From 2f350f732ebf1f9397f6ad7711cb88808ee60127 Mon Sep 17 00:00:00 2001 From: Zaki Shaikh Date: Thu, 22 Oct 2026 00:45:45 +0530 Subject: [PATCH] fix: add error log on invalid yaml during resolution of an invalid yaml, PaC logs the error as debug log while it's an error and should be highlighted in controller logs so that user can take care of it. comments are posted for invalid yamls on pull request but for push events it's not easy to find the issue with invalid yaml. Signed-off-by: Zaki Shaikh --- pkg/resolve/logging.go | 6 ++++++ pkg/resolve/resolve.go | 2 +- pkg/resolve/resolve_test.go | 16 ++++++++++++---- 3 files changed, 19 insertions(+), 5 deletions(-) diff --git a/pkg/resolve/logging.go b/pkg/resolve/logging.go index ec1c620ff8..10138e26b4 100644 --- a/pkg/resolve/logging.go +++ b/pkg/resolve/logging.go @@ -9,6 +9,12 @@ func debugf(log *zap.SugaredLogger, format string, args ...any) { log.Debugf(format, args...) } +func warnf(log *zap.SugaredLogger, format string, args ...any) { + if log != nil { + log.Warnf(format, args...) + } +} + func logInfo(log *zap.SugaredLogger, msg string) { if log == nil { return diff --git a/pkg/resolve/resolve.go b/pkg/resolve/resolve.go index 69301e2fa1..ec980432a8 100644 --- a/pkg/resolve/resolve.go +++ b/pkg/resolve/resolve.go @@ -203,7 +203,7 @@ func ReadTektonTypes(ctx context.Context, log *zap.SugaredLogger, data string) ( obj, _, err := decoder.Decode([]byte(doc), nil, nil) if err != nil { dt, dv := detectAtleastNameOrGenerateNameAndSchemaFromPipelineRun(doc) - debugf(log, "ReadTektonTypes: decode error for doc name=%s schema=%s err=%v", dt, dv, err) + warnf(log, "ReadTektonTypes: decode error for doc name=%s schema=%s err=%v", dt, dv, err) types.ValidationErrors = append(types.ValidationErrors, &pacerrors.PacYamlValidations{ Name: dt, Err: fmt.Errorf("error decoding yaml document: %w", err), diff --git a/pkg/resolve/resolve_test.go b/pkg/resolve/resolve_test.go index a4f362f6e5..93c93032b5 100644 --- a/pkg/resolve/resolve_test.go +++ b/pkg/resolve/resolve_test.go @@ -416,9 +416,11 @@ func TestReportBadTektonYaml(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + observer, _ := zapobserver.New(zap.InfoLevel) + logger := zap.New(observer).Sugar() data, err := os.ReadFile("testdata/" + tt.filename + ".yaml") assert.NilError(t, err) - types, err := ReadTektonTypes(context.TODO(), nil, string(data)) + types, err := ReadTektonTypes(context.TODO(), logger, string(data)) assert.NilError(t, err) // Find the validation error by name @@ -510,7 +512,9 @@ spec: taskSpec: invalid-field: "this should cause an error"` - types, err := ReadTektonTypes(context.TODO(), nil, testYaml) + observer, _ := zapobserver.New(zap.InfoLevel) + logger := zap.New(observer).Sugar() + types, err := ReadTektonTypes(context.TODO(), logger, testYaml) assert.NilError(t, err) // Each validation error should have the proper structure @@ -599,7 +603,9 @@ func TestErrorMessageFormat(t *testing.T) { - this: should cause: [an error` - types, err := ReadTektonTypes(context.TODO(), nil, testYaml) + observer, _ := zapobserver.New(zap.InfoLevel) + logger := zap.New(observer).Sugar() + types, err := ReadTektonTypes(context.TODO(), logger, testYaml) assert.NilError(t, err) // Should have at least one validation error @@ -793,7 +799,9 @@ func TestV1Beta1ConversionFixturesAreAcceptedByReadTektonTypes(t *testing.T) { data, err := os.ReadFile("testdata/" + tt.filename + ".yaml") assert.NilError(t, err) - types, err := ReadTektonTypes(context.TODO(), nil, string(data)) + observer, _ := zapobserver.New(zap.InfoLevel) + logger := zap.New(observer).Sugar() + types, err := ReadTektonTypes(context.TODO(), logger, string(data)) assert.NilError(t, err) assert.Equal(t, len(types.PipelineRuns), tt.pipelineRuns) assert.Equal(t, len(types.Pipelines), tt.pipelines)