fix: downgrade 404 API responses from error to debug log level by KMI1011 · Pull Request #2776 · tektoncd/pipelines-as-code

KMI1011 · 2026-06-10T19:45:45Z

rh-pre-commit.version: 2.4.0
rh-pre-commit.check-secrets: ENABLED

📝 Description of the Change

OWNERS file being missing lead to displaying a misconstruing Error message. logAPICall logged all API errors at Error level, making missing-file 404s look like real failures. Now, 404 responses are logged at Debug level instead, keeping controller logs clean for normal cases like a missing OWNERS file.

🔗 Linked GitHub Issue

#2653

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 used AI assistance for this PR.
I have not used any 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

linux-foundation-easycla · 2026-06-10T19:45:50Z

The committers listed above are authorized under a signed CLA.

✅ login: KMI1011 / name: KMI1011 (84b0d99)

gemini-code-assist

Code Review

This pull request modifies the GitHub API call logging in pkg/provider/github/profiler.go to log 404 Not Found errors as debug messages instead of error messages. The review feedback points out a potential nil pointer dereference panic when accessing resp.StatusCode if the embedded Response field in github.Response is nil, and suggests adding a check for resp.Response != nil.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

gemini-code-assist · 2026-06-10T19:46:26Z

 	if err != nil {
 		logFields = append(logFields, "error", err.Error())
-		v.Logger.Errorw("GitHub API call failed", logFields...)
+		if resp != nil && resp.StatusCode == http.StatusNotFound {


In Go, github.Response embeds *http.Response anonymously. If resp is non-nil but its embedded Response field is nil, accessing resp.StatusCode will trigger a nil pointer dereference panic. To prevent potential runtime panics, ensure resp.Response is also checked for nil before accessing resp.StatusCode, similar to the check performed in checkRateLimit.

Suggested change

if resp != nil && resp.StatusCode == http.StatusNotFound {

if resp != nil && resp.Response != nil && resp.StatusCode == http.StatusNotFound {

I think it's worth adding.

Will do, I just was wondering, is this portion its, the 'v.Logger.Errorw' line, is it asking for it to be removed from line 139? Because that line is specifically used as an 'else' to bring in a missing OWNER file friendly error code.

I think No, just add nil check

This still needs to be addressed

KMI1011 · 2026-06-10T20:01:35Z

ptal @zakisk

theakshaypant

Thanks @KMI1011 for working on this!
I have some minor comments on these changes.

theakshaypant · 2026-06-11T13:30:25Z

 	}

-	// Log based on success/failure with appropriate level
+	// Plausible replacement for the if err != nil


This comment looks out of place, suggest changing to something like

// Log based on success/failure with appropriate level. // 404 responses are expected in some flows (e.g., missing OWNERS file) // and are logged at debug level to avoid noisy error logs.

+1

With code comments, my rule of thumb is the comment should explain "why does this work in this way" or (when it's not obvious) "why is this necessary".
I might annotate some code with comments about what needs to change and where, when I'm planning a change, but those comments should be removed before the change is opened for review 👍🏼

theakshaypant · 2026-06-11T13:30:57Z

+		if resp != nil && resp.StatusCode == http.StatusNotFound {
+			v.Logger.Debugw("GitHub API call returned not found", logFields...)


Suggest adding a test case for this new branch

aThorp96

Downgrading from Error to Debug might be too severe. For OWNERS files I think it makes sense, but are there other cases where a 404 should result in this error log? Should it be logged at Info or Warning instead of Debug? What do you think @theakshaypant ?

aThorp96 · 2026-06-15T15:22:32Z

 	}

-	// Log based on success/failure with appropriate level
+	// Plausible replacement for the if err != nil


+1

With code comments, my rule of thumb is the comment should explain "why does this work in this way" or (when it's not obvious) "why is this necessary".
I might annotate some code with comments about what needs to change and where, when I'm planning a change, but those comments should be removed before the change is opened for review 👍🏼

aThorp96 · 2026-06-15T17:22:38Z

 	if err != nil {
 		logFields = append(logFields, "error", err.Error())
-		v.Logger.Errorw("GitHub API call failed", logFields...)
+		if resp != nil && resp.StatusCode == http.StatusNotFound {


theakshaypant · 2026-06-16T02:48:23Z

Should it be logged at Info or Warning instead of Debug?

@aThorp96 This was almost a nit from me. I've been going back and forth on this myself.
Since the OWNERS file is outside PaC configuration, not having one isn't an event that an admin needs to be aware of. It's an optional convention file, so I don't think its absence warrants a warning log.
Debug is consistent with the other API call-related logging patterns across the repo (which is why I didn't add this as a comment originally).
As for why I thought it could be an info log: PaC would be keeping an auditor informed: "Hey! We looked for an OWNERS file for permissions but didn't find one." Maybe they'd consider adding one, saves us one API call for the org membership check. But maybe that's just wishful thinking. 🤷🏼‍♂️

aThorp96 · 2026-06-17T23:53:44Z

+Khiar Ibrahim
+


I think this was included by mistake

aThorp96 · 2026-06-17T23:57:49Z

-
-	// Log based on success/failure with appropriate level
+	// Log success/failure appropriately; 404 is debug-only
+	// since a missing OWNERS file is a valid/expected state.


Nitpick, wording, I think there are probably other 404s that are ok

Suggested change

// since a missing OWNERS file is a valid/expected state.

// since it's valid/expected in some cases, such as a missing OWNERS file.

aThorp96 · 2026-06-18T00:42:24Z

 	if err != nil {
 		logFields = append(logFields, "error", err.Error())
-		v.Logger.Errorw("GitHub API call failed", logFields...)
+		if resp != nil && resp.StatusCode == http.StatusNotFound {


This still needs to be addressed

aThorp96 · 2026-06-18T00:44:17Z

I like the additional test case and I appreciate combining it with the existing test case. A couple small feedback comments. The resp.Response != nil check still needs to be addressed

zakisk · 2026-06-18T06:13:43Z

/ok-to-test

codecov · 2026-06-18T06:20:02Z

Codecov Report

❌ Patch coverage is 77.77778% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 59.77%. Comparing base (fff1dac) to head (157386e).

Files with missing lines	Patch %	Lines
pkg/provider/github/profiler.go	77.77%	2 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2776   +/-   ##
=======================================
  Coverage   59.77%   59.77%           
=======================================
  Files         210      210           
  Lines       21135    21141    +6     
=======================================
+ Hits        12633    12638    +5     
- Misses       7707     7708    +1     
  Partials      795      795

☔ 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.

KMI1011 · 2026-06-18T13:23:58Z

I like the additional test case and I appreciate combining it with the existing test case. A couple small feedback comments. The resp.Response != nil check still needs to be addressed

I had planned to keep my update to it, however that was one of the things the pre commit requested I compress. It was the first of my fixes after committing. Should I revert back to the change I initially addressed and apply --no-verify?

aThorp96 · 2026-06-18T13:28:03Z

however that was one of the things the pre commit requested I compress.

pkg/provider/github/profiler.go:141:50: QF1008: could remove embedded field "Response" from selector (staticcheck)
                if resp != nil && resp.Response != nil && resp.Response.StatusCode == http.StatusNotFound {
                                                               ^

I think it's suggesting you compress resp.Response.StatusCode to resp.StatusCode, not that you remove resp.Response != nil

zakisk · 2026-06-19T13:34:57Z

@KMI1011 yes, just use resp.Status == http.StatusNotFound

aThorp96 · 2026-06-24T15:22:49Z

/ok-to-test

Replace duplicate 404 and non-404 error subtests with a table-driven test to satisfy the dupl linter. Fixes tektoncd#2653 rh-pre-commit.version: 2.4.0 rh-pre-commit.check-secrets: ENABLED

theakshaypant · 2026-06-25T06:51:19Z

/ok-to-test

gemini-code-assist Bot reviewed Jun 10, 2026

View reviewed changes

theakshaypant reviewed Jun 11, 2026

View reviewed changes

aThorp96 reviewed Jun 15, 2026

View reviewed changes

KMI1011 force-pushed the fix/2653-owners-api-error-log-level/SRVKP-12338 branch from 84b0d99 to eb36e13 Compare June 17, 2026 18:50

aThorp96 reviewed Jun 18, 2026

View reviewed changes

zakisk added the ok-to-test label Jun 18, 2026

tekton-pac-bot removed the ok-to-test label Jun 18, 2026

KMI1011 force-pushed the fix/2653-owners-api-error-log-level/SRVKP-12338 branch from eb36e13 to 188250d Compare June 23, 2026 13:43

tekton-robot added the needs-rebase label Jun 23, 2026

KMI1011 closed this Jun 23, 2026

KMI1011 force-pushed the fix/2653-owners-api-error-log-level/SRVKP-12338 branch from 188250d to fff1dac Compare June 23, 2026 21:20

KMI1011 reopened this Jun 24, 2026

tekton-robot removed needs-rebase labels Jun 24, 2026

fix: downgrade 404 API responses from error to debug log level

157386e

Replace duplicate 404 and non-404 error subtests with a table-driven test to satisfy the dupl linter. Fixes tektoncd#2653 rh-pre-commit.version: 2.4.0 rh-pre-commit.check-secrets: ENABLED

KMI1011 force-pushed the fix/2653-owners-api-error-log-level/SRVKP-12338 branch from 80a8452 to 157386e Compare June 24, 2026 17:49

	if resp != nil && resp.StatusCode == http.StatusNotFound {
	if resp != nil && resp.Response != nil && resp.StatusCode == http.StatusNotFound {

		if resp != nil && resp.StatusCode == http.StatusNotFound {
		v.Logger.Debugw("GitHub API call returned not found", logFields...)

	// since a missing OWNERS file is a valid/expected state.
	// since it's valid/expected in some cases, such as a missing OWNERS file.

		Khiar Ibrahim

Uh oh!

Conversation

KMI1011 commented Jun 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📝 Description of the Change

🔗 Linked GitHub Issue

🧪 Testing Strategy

🤖 AI Assistance

✅ Submitter Checklist

Uh oh!

linux-foundation-easycla Bot commented Jun 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Jun 10, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

KMI1011 commented Jun 10, 2026

Uh oh!

theakshaypant left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aThorp96 Jun 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

theakshaypant Jun 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aThorp96 left a comment

Choose a reason for hiding this comment

Uh oh!

aThorp96 Jun 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

theakshaypant commented Jun 16, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aThorp96 commented Jun 18, 2026

Uh oh!

zakisk commented Jun 18, 2026

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

KMI1011 commented Jun 18, 2026

Uh oh!

aThorp96 commented Jun 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zakisk commented Jun 19, 2026

Uh oh!

aThorp96 commented Jun 24, 2026

Uh oh!

theakshaypant commented Jun 25, 2026

KMI1011 commented Jun 10, 2026 •

edited

Loading

linux-foundation-easycla Bot commented Jun 10, 2026 •

edited

Loading

aThorp96 Jun 15, 2026 •

edited

Loading

theakshaypant Jun 11, 2026 •

edited

Loading

aThorp96 Jun 15, 2026 •

edited

Loading

codecov Bot commented Jun 18, 2026 •

edited

Loading

aThorp96 commented Jun 18, 2026 •

edited

Loading