Skip to content

Commit 7aa1b46

Browse files
chmouelCursor
andcommitted
feat: Introduce error log snippet line config
* Added a new configuration option `error-log-snippet-number-of-lines`. This provides control over the number of lines displayed in error log snippets. * Set the default value for `error-log-snippet-number-of-lines` to 3. * Implemented a maximum character limit of 65536 for all error log snippets. This addressed potential issues with exceeding the GitHub Check Run API character limit for logs. * Removed a previously hardcoded constant for log snippet lines, replacing it with the new configurable setting. * Updated the documentation to describe the new configuration and the character limit. Jira: https://issues.redhat.com/browse/SRVKP-8165 Co-authored-by: Cursor <[email protected]> Signed-off-by: Chmouel Boudjnah <[email protected]>
1 parent 756efe9 commit 7aa1b46

File tree

10 files changed

+276
-43
lines changed

10 files changed

+276
-43
lines changed

config/302-pac-configmap.yaml

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -80,14 +80,18 @@ data:
8080
tekton-dashboard-url: ""
8181

8282
# Enable or disable the feature to show a log snippet of the failed task when there is
83-
# an error in a Pipeline
83+
# an error in a Pipeline. The number of lines shown is controlled by the
84+
# error-log-snippet-number-of-lines setting below.
8485
#
85-
# It will show the last 3 lines of the first container of the first task
86-
# that has error in the pipeline.
87-
#
88-
# you may want to disable this if you think your pipeline may leak some value
86+
# You may want to disable this if you think your pipeline may leak sensitive values.
8987
error-log-snippet: "true"
9088

89+
# The number of lines to display in error log snippets, when `error-log-snippet` is
90+
# set to "true".
91+
# The GitHub Check interface (via the GitHub App) has a 65,535 character limit,
92+
# so consider using a conservative value for this setting.
93+
error-log-snippet-number-of-lines: "3"
94+
9195
# Enable or disable the inspection of container logs to detect error message
9296
# and expose them as annotations on Pull Request. Only Github apps is supported
9397
error-detection-from-container-logs: "true"

docs/content/docs/install/settings.md

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -254,13 +254,22 @@ A few settings are available to configure this feature:
254254
Enable or disable the feature to show a log snippet of the failed task when
255255
there is an error in a PipelineRun.
256256

257-
Due to the constraints of the different GIT provider APIs, it will show the last
258-
3 lines of the first container from the first task that has exited with an
259-
error in the PipelineRun.
257+
Due to the constraints of the different GIT provider APIs, it will show a
258+
configurable number of lines of the first container from the first task that
259+
has exited with an error in the PipelineRun. The number of lines is controlled
260+
by the `error-log-snippet-number-of-lines` setting (see below).
260261

261-
If it find any strings matching the values of secrets attached to the
262+
If it finds any strings matching the values of secrets attached to the
262263
PipelineRun it will replace it with the placeholder `******`
263264

265+
* `error-log-snippet-number-of-lines`
266+
267+
default: `3`
268+
269+
How many lines to show in the error log snippets when `error-log-snippet` is set to `"true"`.
270+
When using GitHub APP the GitHub Check interface [has a limit of 65535 characters](https://docs.github.com/en/rest/checks/runs?apiVersion=2022-11-28#create-a-check-run),
271+
so you may want to be conservative with this setting.
272+
264273
* `error-detection-from-container-logs`
265274

266275
Enable or disable the inspection of the container logs to detect error message

pkg/kubeinteraction/status/task_status.go

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"fmt"
66
"regexp"
77
"strings"
8+
"unicode/utf8"
89

910
pacv1alpha1 "github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/v1alpha1"
1011
"github.com/openshift-pipelines/pipelines-as-code/pkg/kubeinteraction"
@@ -17,6 +18,8 @@ import (
1718

1819
var reasonMessageReplacementRegexp = regexp.MustCompile(`\(image: .*`)
1920

21+
const maxErrorSnippetCharacterLimit = 65535 // This is the maximum size allowed by Github check run logs and may apply to all other providers
22+
2023
// GetTaskRunStatusForPipelineTask takes a minimal embedded status child reference and returns the actual TaskRunStatus
2124
// for the PipelineTask. It returns an error if the child reference's kind isn't TaskRun.
2225
func GetTaskRunStatusForPipelineTask(ctx context.Context, client versioned.Interface, ns string, childRef tektonv1.ChildStatusReference) (*tektonv1.TaskRunStatus, error) {
@@ -113,8 +116,18 @@ func CollectFailedTasksLogSnippet(ctx context.Context, cs *params.Run, kinteract
113116
if strings.HasSuffix(trimmed, " Skipping step because a previous step failed") {
114117
continue
115118
}
116-
// see if a pattern match from errRe
117-
ti.LogSnippet = strings.TrimSpace(trimmed)
119+
// GitHub's character limit is actually in bytes, not unicode characters
120+
// Truncate to maxErrorSnippetCharacterLimit bytes, then trim to last valid UTF-8 boundary
121+
if len(trimmed) > maxErrorSnippetCharacterLimit {
122+
trimmed = trimmed[:maxErrorSnippetCharacterLimit]
123+
// Trim further to last valid rune boundary to ensure valid UTF-8
124+
r, size := utf8.DecodeLastRuneInString(trimmed)
125+
for r == utf8.RuneError && size > 0 {
126+
trimmed = trimmed[:len(trimmed)-size]
127+
r, size = utf8.DecodeLastRuneInString(trimmed)
128+
}
129+
}
130+
ti.LogSnippet = trimmed
118131
}
119132
}
120133
}

pkg/kubeinteraction/status/task_status_test.go

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package status
22

33
import (
44
"testing"
5+
"unicode/utf8"
56

67
"github.com/jonboulle/clockwork"
78
"github.com/openshift-pipelines/pipelines-as-code/pkg/params"
@@ -116,6 +117,141 @@ func TestCollectFailedTasksLogSnippet(t *testing.T) {
116117
}
117118
}
118119

120+
func TestCollectFailedTasksLogSnippetUTF8SafeTruncation(t *testing.T) {
121+
clock := clockwork.NewFakeClock()
122+
123+
tests := []struct {
124+
name string
125+
podOutput string
126+
expectedTruncation bool
127+
expectedLengthRunes int // Expected rune count for non-truncated strings
128+
expectValidUTF8 bool // Should result in valid UTF-8
129+
}{
130+
{
131+
name: "short ascii text",
132+
podOutput: "Error: simple failure message",
133+
expectedTruncation: false,
134+
expectedLengthRunes: 29,
135+
expectValidUTF8: true,
136+
},
137+
{
138+
name: "long ascii text over limit",
139+
podOutput: string(make([]byte, maxErrorSnippetCharacterLimit+100)), // Fill with null bytes which are 1 byte each
140+
expectedTruncation: true,
141+
expectValidUTF8: true,
142+
},
143+
{
144+
name: "utf8 text under limit",
145+
podOutput: "🚀 Error: deployment failed with émojis and spécial chars",
146+
expectedTruncation: false,
147+
expectedLengthRunes: len([]rune("🚀 Error: deployment failed with émojis and spécial chars")),
148+
expectValidUTF8: true,
149+
},
150+
{
151+
name: "utf8 text over limit",
152+
podOutput: "🚀 " + string(make([]rune, maxErrorSnippetCharacterLimit)), // Create string with unicode chars (will be >65535 bytes)
153+
expectedTruncation: true,
154+
expectValidUTF8: true,
155+
},
156+
{
157+
name: "mixed utf8 at boundary",
158+
podOutput: string(make([]rune, maxErrorSnippetCharacterLimit+1)) + "🚀🔥💥",
159+
expectedTruncation: true,
160+
expectValidUTF8: true,
161+
},
162+
}
163+
164+
for _, tt := range tests {
165+
t.Run(tt.name, func(t *testing.T) {
166+
pr := tektontest.MakePRCompletion(clock, "pipeline-newest", "ns", tektonv1.PipelineRunReasonSuccessful.String(), nil, make(map[string]string), 10)
167+
pr.Status.ChildReferences = []tektonv1.ChildStatusReference{
168+
{
169+
TypeMeta: runtime.TypeMeta{
170+
Kind: "TaskRun",
171+
},
172+
Name: "task1",
173+
PipelineTaskName: "task1",
174+
},
175+
}
176+
177+
taskStatus := tektonv1.TaskRunStatusFields{
178+
PodName: "task1",
179+
Steps: []tektonv1.StepState{
180+
{
181+
Name: "step1",
182+
ContainerState: corev1.ContainerState{
183+
Terminated: &corev1.ContainerStateTerminated{
184+
ExitCode: 1,
185+
},
186+
},
187+
},
188+
},
189+
}
190+
191+
tdata := testclient.Data{
192+
TaskRuns: []*tektonv1.TaskRun{
193+
tektontest.MakeTaskRunCompletion(clock, "task1", "ns", "pipeline-newest",
194+
map[string]string{}, taskStatus, knativeduckv1.Conditions{
195+
{
196+
Type: knativeapi.ConditionSucceeded,
197+
Status: corev1.ConditionFalse,
198+
Reason: "Failed",
199+
Message: "task failed",
200+
},
201+
},
202+
10),
203+
},
204+
}
205+
206+
ctx, _ := rtesting.SetupFakeContext(t)
207+
stdata, _ := testclient.SeedTestData(t, ctx, tdata)
208+
cs := &params.Run{Clients: paramclients.Clients{
209+
Tekton: stdata.Pipeline,
210+
}}
211+
212+
intf := &kubernetestint.KinterfaceTest{
213+
GetPodLogsOutput: map[string]string{
214+
"task1": tt.podOutput,
215+
},
216+
}
217+
218+
got := CollectFailedTasksLogSnippet(ctx, cs, intf, pr, 1)
219+
assert.Equal(t, 1, len(got))
220+
221+
snippet := got["task1"].LogSnippet
222+
byteCount := len(snippet)
223+
runeCount := len([]rune(snippet))
224+
225+
if tt.expectedTruncation {
226+
// Should be truncated to at most maxErrorSnippetCharacterLimit bytes
227+
if byteCount > maxErrorSnippetCharacterLimit {
228+
t.Errorf("Expected truncated string to be at most %d bytes, got %d",
229+
maxErrorSnippetCharacterLimit, byteCount)
230+
}
231+
232+
// Verify the string is valid UTF-8 after truncation
233+
assert.True(t, utf8.ValidString(snippet), "Truncated string should be valid UTF-8")
234+
235+
// Should be shorter than original (in bytes)
236+
assert.Less(t, byteCount, len(tt.podOutput),
237+
"Truncated string should be shorter than original")
238+
} else {
239+
// Should match expected length exactly (in runes for non-truncated)
240+
assert.Equal(t, tt.expectedLengthRunes, runeCount,
241+
"Expected string length %d runes, got %d", tt.expectedLengthRunes, runeCount)
242+
243+
// Should match original (no truncation)
244+
assert.Equal(t, tt.podOutput, snippet, "String should not be truncated")
245+
}
246+
247+
// Always verify valid UTF-8
248+
if tt.expectValidUTF8 {
249+
assert.True(t, utf8.ValidString(snippet), "String should be valid UTF-8")
250+
}
251+
})
252+
}
253+
}
254+
119255
func TestGetStatusFromTaskStatusOrFromAsking(t *testing.T) {
120256
testNS := "test"
121257
tests := []struct {

pkg/params/settings/config.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -64,10 +64,11 @@ type Settings struct {
6464
SecretGHAppRepoScoped bool `default:"true" json:"secret-github-app-token-scoped"`
6565
SecretGhAppTokenScopedExtraRepos string `json:"secret-github-app-scope-extra-repos"`
6666

67-
ErrorLogSnippet bool `default:"true" json:"error-log-snippet"`
68-
ErrorDetection bool `default:"true" json:"error-detection-from-container-logs"`
69-
ErrorDetectionNumberOfLines int `default:"50" json:"error-detection-max-number-of-lines"`
70-
ErrorDetectionSimpleRegexp string `default:"^(?P<filename>[^:]*):(?P<line>[0-9]+):(?P<column>[0-9]+)?([ ]*)?(?P<error>.*)" json:"error-detection-simple-regexp"`
67+
ErrorLogSnippet bool `default:"true" json:"error-log-snippet"`
68+
ErrorLogSnippetNumberOfLines int `default:"3" json:"error-log-snippet-number-of-lines"`
69+
ErrorDetection bool `default:"true" json:"error-detection-from-container-logs"`
70+
ErrorDetectionNumberOfLines int `default:"50" json:"error-detection-max-number-of-lines"`
71+
ErrorDetectionSimpleRegexp string `default:"^(?P<filename>[^:]*):(?P<line>[0-9]+):(?P<column>[0-9]+)?([ ]*)?(?P<error>.*)" json:"error-detection-simple-regexp"`
7172

7273
EnableCancelInProgressOnPullRequests bool `json:"enable-cancel-in-progress-on-pull-requests"`
7374
EnableCancelInProgressOnPush bool `json:"enable-cancel-in-progress-on-push"`

pkg/params/settings/config_test.go

Lines changed: 29 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ func TestSyncConfig(t *testing.T) {
3535
SecretGHAppRepoScoped: true,
3636
SecretGhAppTokenScopedExtraRepos: "",
3737
ErrorLogSnippet: true,
38+
ErrorLogSnippetNumberOfLines: 3,
3839
ErrorDetection: true,
3940
ErrorDetectionNumberOfLines: 50,
4041
ErrorDetectionSimpleRegexp: "^(?P<filename>[^:]*):(?P<line>[0-9]+):(?P<column>[0-9]+)?([ ]*)?(?P<error>.*)",
@@ -78,31 +79,34 @@ func TestSyncConfig(t *testing.T) {
7879
"skip-push-event-for-pr-commits": "true",
7980
},
8081
expectedStruct: Settings{
81-
ApplicationName: "pac-pac",
82-
HubCatalogs: nil,
83-
RemoteTasks: false,
84-
MaxKeepRunsUpperLimit: 10,
85-
DefaultMaxKeepRuns: 5,
86-
BitbucketCloudCheckSourceIP: false,
87-
BitbucketCloudAdditionalSourceIP: "some-ip",
88-
TektonDashboardURL: "https://tekton-dashboard",
89-
AutoConfigureNewGitHubRepo: true,
90-
AutoConfigureRepoNamespaceTemplate: "template",
91-
AutoConfigureRepoRepositoryTemplate: "template",
92-
SecretAutoCreation: false,
93-
SecretGHAppRepoScoped: false,
94-
SecretGhAppTokenScopedExtraRepos: "extra-repos",
95-
ErrorLogSnippet: false,
96-
ErrorDetection: false,
97-
ErrorDetectionNumberOfLines: 100,
98-
ErrorDetectionSimpleRegexp: "^(?P<filename>[^:]*):(?P<line>[0-9]+):(?P<column>[0-9]+)?([ ]*)?(?P<error>.*)",
99-
CustomConsoleName: "custom-console",
100-
CustomConsoleURL: "https://custom-console",
101-
CustomConsolePRdetail: "https://custom-console-pr-details",
102-
CustomConsolePRTaskLog: "https://custom-console-pr-tasklog",
103-
CustomConsoleNamespaceURL: "https://custom-console-namespace",
104-
RememberOKToTest: false,
105-
SkipPushEventForPRCommits: true,
82+
ApplicationName: "pac-pac",
83+
HubCatalogs: nil,
84+
RemoteTasks: false,
85+
MaxKeepRunsUpperLimit: 10,
86+
DefaultMaxKeepRuns: 5,
87+
BitbucketCloudCheckSourceIP: false,
88+
BitbucketCloudAdditionalSourceIP: "some-ip",
89+
TektonDashboardURL: "https://tekton-dashboard",
90+
AutoConfigureNewGitHubRepo: true,
91+
AutoConfigureRepoNamespaceTemplate: "template",
92+
AutoConfigureRepoRepositoryTemplate: "template",
93+
SecretAutoCreation: false,
94+
SecretGHAppRepoScoped: false,
95+
SecretGhAppTokenScopedExtraRepos: "extra-repos",
96+
ErrorLogSnippet: false,
97+
ErrorLogSnippetNumberOfLines: 3,
98+
ErrorDetection: false,
99+
ErrorDetectionNumberOfLines: 100,
100+
ErrorDetectionSimpleRegexp: "^(?P<filename>[^:]*):(?P<line>[0-9]+):(?P<column>[0-9]+)?([ ]*)?(?P<error>.*)",
101+
EnableCancelInProgressOnPullRequests: false,
102+
EnableCancelInProgressOnPush: false,
103+
SkipPushEventForPRCommits: true,
104+
CustomConsoleName: "custom-console",
105+
CustomConsoleURL: "https://custom-console",
106+
CustomConsolePRdetail: "https://custom-console-pr-details",
107+
CustomConsolePRTaskLog: "https://custom-console-pr-tasklog",
108+
CustomConsoleNamespaceURL: "https://custom-console-namespace",
109+
RememberOKToTest: false,
106110
},
107111
},
108112
{

pkg/reconciler/status.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ import (
2525

2626
const (
2727
maxPipelineRunStatusRun = 5
28-
logSnippetNumLines = 3
2928
)
3029

3130
var backoffSchedule = []time.Duration{
@@ -81,7 +80,11 @@ func (r *Reconciler) updateRepoRunStatus(ctx context.Context, logger *zap.Sugare
8180
}
8281

8382
func (r *Reconciler) getFailureSnippet(ctx context.Context, pr *tektonv1.PipelineRun) string {
84-
taskinfos := kstatus.CollectFailedTasksLogSnippet(ctx, r.run, r.kinteract, pr, logSnippetNumLines)
83+
lines := int64(settings.DefaultSettings().ErrorLogSnippetNumberOfLines)
84+
if r.run.Info.Pac != nil {
85+
lines = int64(r.run.Info.Pac.ErrorLogSnippetNumberOfLines)
86+
}
87+
taskinfos := kstatus.CollectFailedTasksLogSnippet(ctx, r.run, r.kinteract, pr, lines)
8588
if len(taskinfos) == 0 {
8689
return ""
8790
}

0 commit comments

Comments
 (0)