Skip to content

chore: fix new unnecessary gosec warnings - #1461

Open
jgwest wants to merge 1 commit into
redhat-developer:masterfrom
jgwest:fix-gosec-oct-2026
Open

jgwest wants to merge 1 commit into
redhat-developer:masterfrom
jgwest:fix-gosec-oct-2026

Conversation

@jgwest

@jgwest jgwest commented Oct 23, 2026 •

Copy link
Copy Markdown
Member

What does this PR do?

  • A new version of gosec is reporting new warnings, causing CI failures
  • This PR adds the appropriate tags to handle these warnings

PR acceptance criteria:

  • Documentation was updated and verified using make serve-docs
  • Unit tests were updated
  • E2E tests were updated

What type of PR is this?
/kind failing-ci

Signed-off-by: Jonathan West <jgwest@gmail.com>
@openshift-ci openshift-ci Bot added the kind/failing-ci OpenShift CI failure label Oct 23, 2026
@coderabbitai

coderabbitai Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
📥 Commits

Reviewing files that changed from the base of the PR and between b61fc0a and 8b39d45.

📒 Files selected for processing: 7
  • argocd-operator/controllers/argocd/dex.go
  • argocd-operator/controllers/argocd/image_updater.go
  • argocd-operator/controllers/argocdagent/agent/deployment.go
  • argocd-operator/controllers/argocdagent/deployment.go
  • argocd-operator/tests/auxiliary/smtplistener/cmd/listener/serve.go
  • argocd-operator/tests/ginkgo/fixture/argocd/fixture.go
  • argocd-operator/tests/ginkgo/fixture/gitserver/repo.go
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.

📝 Summary

Summary by CodeRabbit

  • Bug Fixes
    • Failures during configuration reloads triggered by SIGHUP and log reopening triggered by SIGUSR1 are now reported in logs rather than silently ignored. This makes signal-related operational problems easier to notice and diagnose. The underlying operations and handling of other signals remain unchanged; only failure reporting has changed.

The diff adds gosec annotations to credential-like literals and test fixture code. It also logs errors returned by configuration reload and log reopening in the SMTP listener’s SIGHUP and SIGUSR1 handlers.

Changes: Gosec annotations

Layer / File(s) Summary
Operator configuration and deployment annotations
argocd-operator/controllers/argocd/dex.go, argocd-operator/controllers/argocd/image_updater.go, argocd-operator/controllers/argocdagent/agent/deployment.go, argocd-operator/controllers/argocdagent/deployment.go
Added #nosec G101 annotations to the Dex clientSecret placeholder and Secret resource names. The configuration values and volume behavior remain unchanged.
Test fixture annotations
argocd-operator/tests/ginkgo/fixture/argocd/fixture.go, argocd-operator/tests/ginkgo/fixture/gitserver/repo.go
Added #nosec G124 and #nosec G204 comments for the outbound cookie and Git command construction. Executable behavior remains unchanged.

SMTP listener signal error logging

Layer / File(s) Summary
Log signal operation errors
argocd-operator/tests/auxiliary/smtplistener/cmd/listener/serve.go
The SIGHUP and SIGUSR1 handlers now log errors returned by configuration reload and log reopening. Previously, both errors were ignored.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Other

Suggested reviewers: olivergondza, jaideepr97

Merge Risk: ⚪ Minimal · up to 8b39d

The PR is mergeable with no identified user-facing or operational risk.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 7 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title clearly identifies the gosec warning fixes, which match the main changes.
Description check Passed The description accurately states that the PR addresses new gosec warnings by adding tags.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.

Comment @coderabbitai help to get the list of available commands.

@openshift-ci
openshift-ci Bot requested review from Rizwana777 and jparsai October 23, 2026 12:08
@chengfang

Copy link
Copy Markdown
Contributor

/lgtm
/approve

@openshift-ci

openshift-ci Bot commented Oct 23, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: chengfang

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ranakan19 ranakan19 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!
/lgtm


req = req.WithContext(ctx)

// #nosec G124 -- this is an outbound request cookie (client to server); Secure/HttpOnly/SameSite are response-only attributes and don't apply here

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@ranakan19

Copy link
Copy Markdown
Contributor

/retest
terraform error; failed to create cluster

@chengfang

Copy link
Copy Markdown
Contributor

/test v4.14-kuttl-parallel

@olivergondza

Copy link
Copy Markdown
Collaborator

/test v4.14-ci-index-gitops-operator-bundle

@olivergondza

Copy link
Copy Markdown
Collaborator

/test v4.14-images

@openshift-ci

openshift-ci Bot commented Oct 24, 2026 •

Copy link
Copy Markdown

@jgwest: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/v4.19-kuttl-sequential 8b39d45 link unknown /test v4.19-kuttl-sequential
ci/prow/v4.19-kuttl-parallel 8b39d45 link unknown /test v4.19-kuttl-parallel

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

@olivergondza

Copy link
Copy Markdown
Collaborator

/retest-required

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants