fix: add error log on invalid yaml by zakisk · Pull Request #3042 · tektoncd/pipelines-as-code

zakisk · 2026-10-21T19:21:16Z

📝 Description of the Change

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.

🔗 Linked GitHub Issue

Fixes #

🧪 Testing Strategy

🤖 AI Assistance

AI assistance can be used for various tasks, such as code generation,
documentation, or testing.

Please indicate whether you have used AI assistance
for this PR and provide details if applicable.

I have not used any AI assistance for this PR.
I have used AI assistance for this PR.

Important

Slop will be simply rejected, if you are using AI assistance you need to make sure you
understand the code generated and that it meets the project's standards. you
need at least know how to run the code and deploy it (if needed). See
startpaac to make it easy
to deploy and test your code changes.

If the majority of the code in this PR was generated by an AI, please add a Co-authored-by trailer to your commit message.
For example:

Co-authored-by: Claude noreply@anthropic.com

✅ Submitter Checklist

pipelines-as-code · 2026-10-21T19:21:47Z

Paco Review

The PR changes the log level for YAML decode errors in ReadTektonTypes from Debug to Warn. It adds a new warnf helper in logging.go (structurally equivalent to the existing debugf but calling log.Warnf), replaces the single debugf call at the decode-error site with warnf, and updates four test functions to pass a real zap logger (created via zapobserver at InfoLevel) instead of nil.

Paco could not check everything:

Could not load 5 files at PR head fad9332 and base 5a48724 because they were too large, looked like credentials, or could not be read.

No new verified findings. This is not a guarantee that the change is defect-free.

_{Reviewed commit: fad9332. 0 verified findings; 0 summary-only.}

pipelines-as-code

Paco inline comments -- see the Paco Review summary comment for the overview.

Copilot

🟡 Changes recommended

The new logging call can panic with a nil logger, and tests do not assert the error-level output.

2 open issues

Guard error logging when logger is nil · New
Assert captured error logs in decoding failure tests · New

What changed in this PR

Makes invalid Tekton YAML more visible in controller logs, particularly for push events.

Changes:

Adds error-level logging for document decoding failures.
Supplies loggers to existing YAML validation tests.

File	Description
pkg/resolve/resolve.go	Logs decoding failures at error level.
pkg/resolve/resolve_test.go	Adds observer-backed loggers to resolver tests.

_{Review effort: Balanced}

codecov · 2026-10-21T19:24:43Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.98%. Comparing base (1d2b8aa) to head (fad9332).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #3042   +/-   ##
=======================================
  Coverage   91.98%   91.98%           
=======================================
  Files         167      167           
  Lines       12965    12967    +2     
=======================================
+ Hits        11926    11928    +2     
  Misses       1038     1038           
  Partials        1        1

Flag	Coverage Δ
unit-tests	`91.98% <100.00%> (+<0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

theakshaypant · 2026-10-22T05:29:38Z

 		if err != nil {
 			dt, dv := detectAtleastNameOrGenerateNameAndSchemaFromPipelineRun(doc)
 			debugf(log, "ReadTektonTypes: decode error for doc name=%s schema=%s err=%v", dt, dv, err)
+			if log != nil {


How about adding errorf function which replaces the debugf statement completely instead of logging twice, once at debug and again at error?

or maybe even just remove the debugf statement to prevent double logging

dont we have thing like that we can commonize btw?

chmouel · 2026-10-22T11:07:54Z

When i made that change in 3df2338. The decode error was kept at debug on purpose. Some repositories carry a broken file in .tekton/ for a long time, and since we parse that directory on every event (sometimes twice, with the cancel-in-progress path), an error-level line here would repeat on every push for as long as the file stays broken. That makes the controller logs noisy for admins who can't fix the user's repo anyway. If the goal is visibility on push events, emitting a Kubernetes event on the Repository (like we already do for pull requests) or logging at warn level might be a better fit...

zakisk · 2026-10-22T11:17:16Z

emitting a Kubernetes event on the Repository (like we already do for pull requests) or logging at warn level might be a better fit...

we can advice in docs that don't keep any unrelated files in .tekton folder but IMO we should log it as error so that it can be brought to attention. for me I was testing multiline body field where I was getting this error and I was confused about the error because it wasn't red in logs.

zakisk · 2026-10-22T11:25:09Z

go-testing fixed in #3048

chmouel · 2026-10-23T10:41:27Z

@zakisk we had it before and konflux folks complained so we may endup with another ticket "requesting us" to be removed 🙃

zakisk · 2026-10-23T16:14:29Z

@zakisk we had it before and konflux folks complained so we may endup with another ticket "requesting us" to be removed 🙃

made it a warning

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 <zashaikh@redhat.com>

chmouel · 2026-10-26T15:40:48Z

/paco review

Copilot AI balanced review requested due to automatic review settings October 21, 2026 19:21

Copilot started reviewing on behalf of zakisk October 21, 2026 19:21

View session

pipelines-as-code Bot added the paco/review-easy Paco review difficulty label Oct 21, 2026

pipelines-as-code Bot reviewed Oct 21, 2026

View reviewed changes

Comment thread pkg/resolve/resolve.go Resolved

Copilot AI reviewed Oct 21, 2026

View reviewed changes

Comment thread pkg/resolve/resolve.go Resolved

Comment thread pkg/resolve/resolve_test.go Resolved

zakisk force-pushed the error-log-on-bad-yaml branch from 5c859a3 to af659a7 Compare October 22, 2026 04:10

theakshaypant reviewed Oct 22, 2026

View reviewed changes

zakisk force-pushed the error-log-on-bad-yaml branch from af659a7 to 94bb9d3 Compare October 22, 2026 10:07

zakisk force-pushed the error-log-on-bad-yaml branch 2 times, most recently from 4e3e0a3 to 6363f9e Compare October 23, 2026 06:44

zakisk force-pushed the error-log-on-bad-yaml branch from 6363f9e to 975a4c6 Compare October 23, 2026 13:20

zakisk force-pushed the error-log-on-bad-yaml branch from 975a4c6 to 5339b5c Compare October 26, 2026 10:23

chmouel approved these changes Oct 26, 2026

View reviewed changes

zakisk force-pushed the error-log-on-bad-yaml branch from 5339b5c to 2f350f7 Compare October 26, 2026 13:54

Merge branch 'main' into error-log-on-bad-yaml

fad9332

pipelines-as-code Bot added paco/reviewing Paco is reviewing this pull request paco/review-hard Paco review difficulty paco/clean Paco review found nothing to report and removed paco/review-easy Paco review difficulty paco/reviewing Paco is reviewing this pull request labels Oct 26, 2026

zakisk merged commit 5ee6d48 into tektoncd:main Oct 26, 2026
26 checks passed

zakisk deleted the error-log-on-bad-yaml branch October 26, 2026 16:31

Conversation

zakisk commented Oct 21, 2026

📝 Description of the Change

🔗 Linked GitHub Issue

🧪 Testing Strategy

🤖 AI Assistance

✅ Submitter Checklist

Uh oh!

pipelines-as-code Bot commented Oct 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Paco Review

Uh oh!

pipelines-as-code Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

codecov Bot commented Oct 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

theakshaypant Oct 22, 2026

Choose a reason for hiding this comment

Uh oh!

theakshaypant Oct 22, 2026

Choose a reason for hiding this comment

Uh oh!

chmouel Oct 22, 2026

Choose a reason for hiding this comment

Uh oh!

chmouel commented Oct 22, 2026

Uh oh!

zakisk commented Oct 22, 2026

Uh oh!

zakisk commented Oct 22, 2026

Uh oh!

chmouel commented Oct 23, 2026

Uh oh!

zakisk commented Oct 23, 2026

Uh oh!

chmouel commented Oct 26, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

pipelines-as-code Bot commented Oct 21, 2026 •

edited

Loading

codecov Bot commented Oct 21, 2026 •

edited

Loading