chore(SREP-4482, SREP-4486, SREP-4800: Boilerplate Update for Agentic SDLC Rollout)#724
Merged
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@charlesgong: This pull request references SREP-4482 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set. This pull request references SREP-4486 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set. This pull request references SREP-4800 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR updates build container images, reconfigures pre-commit hooks with a tiered golden-rules approach and secret scanning, explicitly handles status conditions and operations in core logic for clarity, rotates development certificates, and cleans up manifests and organizational metadata. ChangesInfrastructure, Linting, and Code Quality
Core Logic Condition Handling
Development Certificates and Manifest Cleanup
🎯 3 (Moderate) | ⏱️ ~20 minutes Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 2 warnings)
✅ Passed checks (12 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
❌ Your patch status has failed because the patch coverage (0.00%) is below the target coverage (50.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #724 +/- ##
==========================================
- Coverage 59.24% 59.21% -0.03%
==========================================
Files 62 62
Lines 4125 4127 +2
==========================================
Hits 2444 2444
- Misses 1532 1534 +2
Partials 149 149
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
controllers/addon/phase_observe_operatorresource.go (1)
97-104:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMissing fallback in CSV phase switch causes false-ready path.
The switch no longer handles empty/unexpected phases, so unresolved CSV states can fall through as success (
resultNil) instead of staying unready/retrying.Proposed fix
switch phase { case operatorsv1alpha1.CSVPhaseSucceeded: // do nothing here case operatorsv1alpha1.CSVPhaseFailed: message = "failed" case operatorsv1alpha1.CSVPhasePending, operatorsv1alpha1.CSVPhaseInstallReady, operatorsv1alpha1.CSVPhaseInstalling, operatorsv1alpha1.CSVPhaseUnknown, operatorsv1alpha1.CSVPhaseReplacing, operatorsv1alpha1.CSVPhaseDeleting, operatorsv1alpha1.CSVPhaseAny: message = "unknown/pending" +default: + message = "unknown/pending" }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@controllers/addon/phase_observe_operatorresource.go` around lines 97 - 104, The switch on the CSV state variable phase (using operatorsv1alpha1.CSVPhaseSucceeded/Failed/Pending/etc.) lacks a default/fallback, so unexpected or empty phases fall through as success; update the switch in controllers/addon/phase_observe_operatorresource.go to include a default case that sets message (the same variable used for status) to an "unknown/pending" or equivalent non-ready value and ensure the calling code does not treat that path as resultNil/success (i.e., cause a retry or mark not-ready) so unresolved CSV states don't incorrectly signal readiness.deploy-extras/development/01-metrics-server-tls-secret.yaml (1)
1-14:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winStale validity comment after cert rotation.
The header comment claims this cert is valid until
May 21 12:11:09 3021 GMT, but the rotated cert inca-bundle.crt/tls.crtdecodes toNotAfter: Jan 6 03:42:55 2034 GMT(~10 years fromNotBefore: Jan 9 2024). Update the comment so dev users don't assume a far-future expiry.📝 Proposed fix
# This Secret is only for testing / dev. -# This cert is valid till May 21 12:11:09 3021 GMT +# This cert is valid till Jan 6 03:42:55 2034 GMT # When deployed as an OLM Bundle, OLM will handle injecting TLS secrets # CN = addon-operator-metrics.addon-operator.svc🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deploy-extras/development/01-metrics-server-tls-secret.yaml` around lines 1 - 14, The top header comment is stale; update the comment above the Secret (metadata.name: manager-metrics-tls) to reflect the certificate's actual NotAfter value (NotAfter: Jan 6 03:42:55 2034 GMT) instead of "May 21 12:11:09 3021 GMT" so devs won't be misled; edit the first few comment lines to state the correct expiry (and optionally note NotBefore: Jan 9 2024) while leaving the rest of the Secret (ca-bundle.crt, tls.crt, tls.key) untouched.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.claude/commands/pre-commit.md:
- Around line 77-84: The fenced code block that begins with the triple backticks
around the "PRE-COMMIT SUMMARY" text lacks a language identifier; update the
opening fence in .claude/commands/pre-commit.md so it includes a language token
(for example "text" or "plain") immediately after the ``` to satisfy markdown
linting. Locate the block which contains the "PRE-COMMIT SUMMARY" header and
change the opening ``` to ```text (or another appropriate language) so the
linter recognizes the code fence.
---
Outside diff comments:
In `@controllers/addon/phase_observe_operatorresource.go`:
- Around line 97-104: The switch on the CSV state variable phase (using
operatorsv1alpha1.CSVPhaseSucceeded/Failed/Pending/etc.) lacks a
default/fallback, so unexpected or empty phases fall through as success; update
the switch in controllers/addon/phase_observe_operatorresource.go to include a
default case that sets message (the same variable used for status) to an
"unknown/pending" or equivalent non-ready value and ensure the calling code does
not treat that path as resultNil/success (i.e., cause a retry or mark not-ready)
so unresolved CSV states don't incorrectly signal readiness.
In `@deploy-extras/development/01-metrics-server-tls-secret.yaml`:
- Around line 1-14: The top header comment is stale; update the comment above
the Secret (metadata.name: manager-metrics-tls) to reflect the certificate's
actual NotAfter value (NotAfter: Jan 6 03:42:55 2034 GMT) instead of "May 21
12:11:09 3021 GMT" so devs won't be misled; edit the first few comment lines to
state the correct expiry (and optionally note NotBefore: Jan 9 2024) while
leaving the rest of the Secret (ca-bundle.crt, tls.crt, tls.key) untouched.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: e6ddd226-f5cf-4464-a3f0-79b9780fd956
⛔ Files ignored due to path filters (14)
boilerplate/_data/backing-image-tagis excluded by!boilerplate/**boilerplate/_data/last-boilerplate-commitis excluded by!boilerplate/**boilerplate/openshift/golang-osd-operator/.codecov.ymlis excluded by!boilerplate/**boilerplate/openshift/golang-osd-operator/OWNERS_ALIASESis excluded by!boilerplate/**boilerplate/openshift/golang-osd-operator/README.mdis excluded by!boilerplate/**boilerplate/openshift/golang-osd-operator/TEST_README.mdis excluded by!boilerplate/**boilerplate/openshift/golang-osd-operator/app-sre.mdis excluded by!boilerplate/**boilerplate/openshift/golang-osd-operator/csv-generate/csv-generate.shis excluded by!boilerplate/**boilerplate/openshift/golang-osd-operator/golangci.ymlis excluded by!boilerplate/**boilerplate/openshift/golang-osd-operator/olm_pko_migration.pyis excluded by!boilerplate/**boilerplate/openshift/golang-osd-operator/pre-commit-config.yamlis excluded by!boilerplate/**boilerplate/openshift/golang-osd-operator/standard.mkis excluded by!boilerplate/**boilerplate/openshift/golang-osd-operator/test_olm_pko_migration.pyis excluded by!boilerplate/**boilerplate/openshift/golang-osd-operator/updateis excluded by!boilerplate/**
📒 Files selected for processing (25)
.ci-operator.yaml.claude/commands/pre-commit.md.codecov.yml.pre-commit-config.yamlOWNERS_ALIASESbuild/Dockerfilebuild/Dockerfile.olm-registrybuild/Dockerfile.webhookcontrollers/addon/monitoring_stack_reconciler.gocontrollers/addon/phase_observe_operatorresource.godeploy-extras/development/01-metrics-server-tls-secret.yamldeploy-extras/development/webhook/00-tls-secret.yamldeploy-extras/development/webhook/validatingwebhookconfig.yamldeploy/80_addon-sermon-fedaration-token.yamldeploy_pko/.test-fixtures/config-with-proxy/Cleanup-OLM-Job.yamldeploy_pko/Cleanup-OLM-Job.yamlfips.gohack/hypershift/package/.test-fixtures/namespace-scope/hcp/addon-operator.yamlhack/hypershift/package/hcp/addon-operator.yaml.gotmplhack/hypershift/package/manifest.yamlintegration/fixtures_test.gointegration/metrics_collection_test.gointegration/monitoring_stack_test.gointernal/metrics/recorder.gointernal/webhooks/addon_webhook.go
💤 Files with no reviewable changes (2)
- integration/fixtures_test.go
- OWNERS_ALIASES
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.pre-commit-config.yaml:
- Around line 41-52: The hooks list for the pre-commit-hooks repo is missing the
large-file protection; add the check-added-large-files hook to the existing
hooks under repo: https://github.com/pre-commit/pre-commit-hooks rev: v5.0.0 by
inserting an entry with id: check-added-large-files (optionally with args like
--maxkb=<value> to match repo policy) alongside check-merge-conflict,
trailing-whitespace, end-of-file-fixer and check-yaml so large files are
rejected at commit time; update the hooks block that currently contains id:
check-merge-conflict and id: trailing-whitespace to include id:
check-added-large-files.
- Around line 76-99: Add a new pre-commit hook entry alongside the existing
go-build and go-mod-tidy hooks: create a hook with id "go-fmt" (name "go fmt")
that uses the system language and executes the Go formatter over staged Go files
(targeting .go files or using types: [go]); ensure it runs before other checks,
sets appropriate file matching (e.g., .go files), and configures pass_filenames
so formatting applies to staged files, keeping the hook consistent with the
existing go-mod-tidy and go-build entries.
In `@AGENTS.md`:
- Around line 1-3: The document's top-level heading is incorrect: replace the
first-line title "# CLAUDE.md" with "# AGENTS.md" so the file title matches the
filename and conforms to the guideline for AGENTS.md; update only the heading
line at the top of the file.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 35d0e577-e9ed-4322-b055-7f639e4c18a6
⛔ Files ignored due to path filters (14)
boilerplate/_data/backing-image-tagis excluded by!boilerplate/**boilerplate/_data/last-boilerplate-commitis excluded by!boilerplate/**boilerplate/openshift/golang-osd-operator/.codecov.ymlis excluded by!boilerplate/**boilerplate/openshift/golang-osd-operator/OWNERS_ALIASESis excluded by!boilerplate/**boilerplate/openshift/golang-osd-operator/README.mdis excluded by!boilerplate/**boilerplate/openshift/golang-osd-operator/TEST_README.mdis excluded by!boilerplate/**boilerplate/openshift/golang-osd-operator/app-sre.mdis excluded by!boilerplate/**boilerplate/openshift/golang-osd-operator/csv-generate/csv-generate.shis excluded by!boilerplate/**boilerplate/openshift/golang-osd-operator/golangci.ymlis excluded by!boilerplate/**boilerplate/openshift/golang-osd-operator/olm_pko_migration.pyis excluded by!boilerplate/**boilerplate/openshift/golang-osd-operator/pre-commit-config.yamlis excluded by!boilerplate/**boilerplate/openshift/golang-osd-operator/standard.mkis excluded by!boilerplate/**boilerplate/openshift/golang-osd-operator/test_olm_pko_migration.pyis excluded by!boilerplate/**boilerplate/openshift/golang-osd-operator/updateis excluded by!boilerplate/**
📒 Files selected for processing (20)
.ci-operator.yaml.claude/commands/pre-commit.md.codecov.yml.pre-commit-config.yamlAGENTS.mdCLAUDE.mdOWNERS_ALIASESbuild/Dockerfilebuild/Dockerfile.olm-registrybuild/Dockerfile.webhookdeploy-extras/development/01-metrics-server-tls-secret.yamldeploy-extras/development/webhook/00-tls-secret.yamldeploy-extras/development/webhook/validatingwebhookconfig.yamldeploy/80_addon-sermon-fedaration-token.yamldeploy_pko/.test-fixtures/config-with-proxy/Cleanup-OLM-Job.yamldeploy_pko/Cleanup-OLM-Job.yamlfips.gohack/hypershift/package/.test-fixtures/namespace-scope/hcp/addon-operator.yamlhack/hypershift/package/hcp/addon-operator.yaml.gotmplhack/hypershift/package/manifest.yaml
✅ Files skipped from review due to trivial changes (12)
- build/Dockerfile.webhook
- CLAUDE.md
- hack/hypershift/package/manifest.yaml
- .codecov.yml
- fips.go
- deploy_pko/Cleanup-OLM-Job.yaml
- hack/hypershift/package/.test-fixtures/namespace-scope/hcp/addon-operator.yaml
- OWNERS_ALIASES
- hack/hypershift/package/hcp/addon-operator.yaml.gotmpl
- .ci-operator.yaml
- deploy_pko/.test-fixtures/config-with-proxy/Cleanup-OLM-Job.yaml
- deploy/80_addon-sermon-fedaration-token.yaml
🚧 Files skipped from review as they are similar to previous changes (3)
- build/Dockerfile.olm-registry
- build/Dockerfile
- deploy-extras/development/webhook/validatingwebhookconfig.yaml
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
controllers/addon/phase_observe_operatorresource.go (1)
97-104:⚠️ Potential issue | 🔴 Critical | ⚖️ Poor tradeoffMissing explicit handling for empty CSV phase.
The
getCSVPhasefunction can return an empty string whencsvReferenceis nil (line 115) or when the CSV has no "Succeeded" condition (line 128). With the removal of thedefaultcase, an empty phase will not match any case in this switch, leavingmessageas an empty string. This causes the check at line 106 to fail, and the function returnsresultNil, nilwithout callingreportUnreadyCSV, effectively treating the addon as ready.An empty phase likely indicates the CSV was not found in the operator status or is malformed, which should be treated as "not ready" rather than implicitly allowing the addon to appear available.
🔧 Proposed fix to handle empty phase
var message string switch phase { case operatorsv1alpha1.CSVPhaseSucceeded: // do nothing here case operatorsv1alpha1.CSVPhaseFailed: message = "failed" case operatorsv1alpha1.CSVPhasePending, operatorsv1alpha1.CSVPhaseInstallReady, operatorsv1alpha1.CSVPhaseInstalling, operatorsv1alpha1.CSVPhaseUnknown, operatorsv1alpha1.CSVPhaseReplacing, operatorsv1alpha1.CSVPhaseDeleting, operatorsv1alpha1.CSVPhaseAny: message = "unknown/pending" +case "": + message = "missing or unknown" +default: + message = "unexpected phase" }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@controllers/addon/phase_observe_operatorresource.go` around lines 97 - 104, The switch in getCSVPhase leaves message empty for an empty CSV phase, causing the function to return early and not call reportUnreadyCSV; update the switch in getCSVPhase (and/or the caller logic around reportUnreadyCSV) to explicitly handle an empty phase (e.g., add a case "" or a default branch) and set message to the same "unknown/pending" (or another "not ready") string so that reportUnreadyCSV is invoked when csvReference is nil or the CSV lacks a Succeeded condition.
♻️ Duplicate comments (3)
.pre-commit-config.yaml (2)
59-69: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winAdd required large-file protection hook.
The
check-added-large-fileshook is missing from the pre-commit-hooks configuration. This allows oversized artifacts to slip into commits.➕ Add large-file check
- id: check-yaml name: YAML syntax (deploy/) files: ^deploy/.*\.ya?ml$ args: [--allow-multiple-documents] + - id: check-added-large-files + args: [--maxkb=500]As per coding guidelines, "Pre-commit hooks configured in .pre-commit-config.yaml should include go-fmt, go-mod-tidy, and large file checks".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.pre-commit-config.yaml around lines 59 - 69, The pre-commit config is missing the check-added-large-files hook; add the hook entry (id: check-added-large-files) under the hooks list for the pre-commit-hooks repo block so added files exceeding the allowed size are blocked. Include sensible args (e.g., --maxkb or similar flag) per pre-commit-hooks docs and ensure it sits alongside existing hooks like check-merge-conflict and trailing-whitespace so it runs for all commits. Reference the repo block that currently contains hooks: check-merge-conflict, trailing-whitespace, end-of-file-fixer, check-yaml.
102-143: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winAdd required
go-fmthook.The
go-fmthook is missing from the local hooks configuration. Whilego-mod-tidyis present, formatting enforcement is required by policy.➕ Add go-fmt hook
- id: go-build name: go build language: system entry: bash -c 'T=$(command -v timeout || command -v gtimeout || echo); ${T:+$T 30s} go build ./...' types: [go] pass_filenames: false + # ----------------------------------------------------------------------- + # Go formatting check | target < 2s | auto-fix + # ----------------------------------------------------------------------- + - id: go-fmt + name: go fmt + language: system + entry: bash -c 'gofmt -w -l .' + types: [go] + pass_filenames: false + # ----------------------------------------------------------------------- # 5. DEPENDENCY DRIFT | target < 10s | errorAs per coding guidelines, "Pre-commit hooks configured in .pre-commit-config.yaml should include go-fmt, go-mod-tidy, and large file checks".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.pre-commit-config.yaml around lines 102 - 143, The local pre-commit hooks are missing a go-fmt hook; add a new hook with id "go-fmt" (adjacent to the existing "go-mod-tidy" and "rbac-wildcard-check" entries) that runs the Go formatter (e.g., using "gofmt -s -w" or equivalent) as a system-language hook, targets Go files (files: '(\.go$)' or types: [go]), and sets pass_filenames:false and an appropriate timeout similar to "go-build"/"go-mod-tidy" so formatting is enforced by the pre-commit pipeline..claude/commands/pre-commit.md (1)
47-84:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winClarify attempt counting terminology in workflow and report.
The attempt counting logic is internally consistent but the final report template may confuse implementers. Line 47 starts
attempt_countat 1, line 64 references "2 fix-and-retry attempts," and line 57 stops when count reaches 3 (1 initial + 2 retries). However, line 84's report template says "Attempts: of 2 maximum" without clarifying whether N counts total attempts (1-3) or retry attempts (0-2).📝 Recommended clarification
-Attempts: <N> of 2 maximum +Retry attempts: <N> of 2 maximum (total runs: <N+1>)Or alternatively, use total attempt terminology consistently throughout:
-**Stop retrying when:** -- All hooks pass → report success -- `attempt_count` reaches 3 → stop, escalate to human (see Step 6) +**Stop retrying when:** +- All hooks pass → report success +- Total attempts reach 3 (initial + 2 retries) → stop, escalate to human (see Step 6)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.claude/commands/pre-commit.md around lines 47 - 84, The workflow uses attempt_count starting at 1 but mixes "retries" and "total attempts" terminology; update language and the final report to be consistent by choosing total-attempts wording: revise all references (e.g., the variable attempt_count, the stop condition "stop when attempt_count reaches 3", and the sentence "2 fix-and-retry attempts") to read "2 retries (3 total attempts)" and change the final report template line from "Attempts: <N> of 2 maximum" to "Attempts: <N> of 3 total (including initial)"; ensure any logic/checks that compare attempt_count use the same convention (stop when attempt_count >= 3 or when retries >= 2) and reflect that in the human-escalation text and summary output so implementers unambiguously know whether N counts retries or total attempts.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@build/Dockerfile.webhook`:
- Line 15: The Dockerfile uses a pinned Red Hat UBI base image tag in the FROM
instruction (registry.access.redhat.com/ubi9/ubi-minimal:9.8-1779809423); change
that pinned tag to a floating Red Hat-managed tag (for example use :9.8 or :9)
so the base image receives RH-managed updates and security patches—update the
FROM line in the Dockerfile accordingly and ensure no other lines reference the
specific build identifier.
---
Outside diff comments:
In `@controllers/addon/phase_observe_operatorresource.go`:
- Around line 97-104: The switch in getCSVPhase leaves message empty for an
empty CSV phase, causing the function to return early and not call
reportUnreadyCSV; update the switch in getCSVPhase (and/or the caller logic
around reportUnreadyCSV) to explicitly handle an empty phase (e.g., add a case
"" or a default branch) and set message to the same "unknown/pending" (or
another "not ready") string so that reportUnreadyCSV is invoked when
csvReference is nil or the CSV lacks a Succeeded condition.
---
Duplicate comments:
In @.claude/commands/pre-commit.md:
- Around line 47-84: The workflow uses attempt_count starting at 1 but mixes
"retries" and "total attempts" terminology; update language and the final report
to be consistent by choosing total-attempts wording: revise all references
(e.g., the variable attempt_count, the stop condition "stop when attempt_count
reaches 3", and the sentence "2 fix-and-retry attempts") to read "2 retries (3
total attempts)" and change the final report template line from "Attempts: <N>
of 2 maximum" to "Attempts: <N> of 3 total (including initial)"; ensure any
logic/checks that compare attempt_count use the same convention (stop when
attempt_count >= 3 or when retries >= 2) and reflect that in the
human-escalation text and summary output so implementers unambiguously know
whether N counts retries or total attempts.
In @.pre-commit-config.yaml:
- Around line 59-69: The pre-commit config is missing the
check-added-large-files hook; add the hook entry (id: check-added-large-files)
under the hooks list for the pre-commit-hooks repo block so added files
exceeding the allowed size are blocked. Include sensible args (e.g., --maxkb or
similar flag) per pre-commit-hooks docs and ensure it sits alongside existing
hooks like check-merge-conflict and trailing-whitespace so it runs for all
commits. Reference the repo block that currently contains hooks:
check-merge-conflict, trailing-whitespace, end-of-file-fixer, check-yaml.
- Around line 102-143: The local pre-commit hooks are missing a go-fmt hook; add
a new hook with id "go-fmt" (adjacent to the existing "go-mod-tidy" and
"rbac-wildcard-check" entries) that runs the Go formatter (e.g., using "gofmt -s
-w" or equivalent) as a system-language hook, targets Go files (files: '(\.go$)'
or types: [go]), and sets pass_filenames:false and an appropriate timeout
similar to "go-build"/"go-mod-tidy" so formatting is enforced by the pre-commit
pipeline.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 7b6d43f2-51bf-4474-999d-1c926a1d6686
⛔ Files ignored due to path filters (11)
boilerplate/_data/backing-image-tagis excluded by!boilerplate/**boilerplate/_data/last-boilerplate-commitis excluded by!boilerplate/**boilerplate/openshift/golang-osd-operator/.codecov.ymlis excluded by!boilerplate/**boilerplate/openshift/golang-osd-operator/OWNERS_ALIASESis excluded by!boilerplate/**boilerplate/openshift/golang-osd-operator/dependabot.ymlis excluded by!boilerplate/**boilerplate/openshift/golang-osd-operator/docs/pre-commit.mdis excluded by!boilerplate/**boilerplate/openshift/golang-osd-operator/golangci.ymlis excluded by!boilerplate/**boilerplate/openshift/golang-osd-operator/olm_pko_migration.pyis excluded by!boilerplate/**boilerplate/openshift/golang-osd-operator/pre-commit-config.yamlis excluded by!boilerplate/**boilerplate/openshift/golang-osd-operator/standard.mkis excluded by!boilerplate/**boilerplate/openshift/golang-osd-operator/updateis excluded by!boilerplate/**
📒 Files selected for processing (25)
.ci-operator.yaml.claude/commands/pre-commit.md.codecov.yml.pre-commit-config.yamlOWNERS_ALIASESbuild/Dockerfilebuild/Dockerfile.olm-registrybuild/Dockerfile.webhookcontrollers/addon/monitoring_stack_reconciler.gocontrollers/addon/phase_observe_operatorresource.godeploy-extras/development/01-metrics-server-tls-secret.yamldeploy-extras/development/webhook/00-tls-secret.yamldeploy-extras/development/webhook/validatingwebhookconfig.yamldeploy/80_addon-sermon-fedaration-token.yamldeploy_pko/.test-fixtures/config-with-proxy/Cleanup-OLM-Job.yamldeploy_pko/Cleanup-OLM-Job.yamlfips.gohack/hypershift/package/.test-fixtures/namespace-scope/hcp/addon-operator.yamlhack/hypershift/package/hcp/addon-operator.yaml.gotmplhack/hypershift/package/manifest.yamlintegration/fixtures_test.gointegration/metrics_collection_test.gointegration/monitoring_stack_test.gointernal/metrics/recorder.gointernal/webhooks/addon_webhook.go
💤 Files with no reviewable changes (1)
- integration/fixtures_test.go
✅ Files skipped from review due to trivial changes (10)
- hack/hypershift/package/manifest.yaml
- controllers/addon/monitoring_stack_reconciler.go
- deploy/80_addon-sermon-fedaration-token.yaml
- .codecov.yml
- hack/hypershift/package/.test-fixtures/namespace-scope/hcp/addon-operator.yaml
- .ci-operator.yaml
- deploy_pko/.test-fixtures/config-with-proxy/Cleanup-OLM-Job.yaml
- deploy_pko/Cleanup-OLM-Job.yaml
- OWNERS_ALIASES
- integration/metrics_collection_test.go
🚧 Files skipped from review as they are similar to previous changes (5)
- hack/hypershift/package/hcp/addon-operator.yaml.gotmpl
- build/Dockerfile
- deploy-extras/development/webhook/validatingwebhookconfig.yaml
- fips.go
- build/Dockerfile.olm-registry
charlesgong
force-pushed
the
SREP-4485
branch
5 times, most recently
from
3c9e724 to
20a02d5
Compare
Updates boilerplate to image-v8.3.6, which introduces Tier 1 pre-commit hooks for OSD operators (SREP-4485 / golden rules: SREP-4450): - File hygiene: check-added-large-files, check-merge-conflict, trailing-whitespace, end-of-file-fixer, YAML syntax (deploy/) - Secrets detection: gitleaks - Static analysis: golangci-lint (system binary, skipped in CI via PRE_COMMIT_CI=1; CI uses dedicated lint job instead) - Formatting: go fmt - Compile check: go build - Dependency drift: go mod tidy (go.mod only, 300s timeout for cold cache) - RBAC: wildcard permission check in deploy/ manifests CI tooling image (deploy-extras/docker/Dockerfile.src.ci) pre-warms GOCACHE and GOMODCACHE and chmod's both world-accessible so OpenShift's non-root UID can use them. Also fixes pre-existing nolint directive format issues found by the new nolintlint rule: removes unused //nolint directives in integration/fixtures_test.go and corrects leading-space format in integration/metrics_collection_test.go. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Contributor
|
/test images |
Contributor
Author
|
/test images |
1 similar comment
Contributor
Author
|
/test images |
Contributor
Author
|
/test images |
eqrx
suggested changes
Jun 2, 2026
erdii
approved these changes
Jun 9, 2026
Contributor
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: charlesgong, erdii The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
Member
|
/lgtm |
Contributor
|
@charlesgong: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What type of PR is this?
boilerplate
What this PR does / why we need it?
This PR moves the changes introduced in boilerplate for Agentic SDLC Rollout into MVP for ocm-agent-operator.
Related BP MRs
Which Jira/Github issue(s) this PR fixes?
Part of Rollout for Agentic SDLC -
Special notes for your reviewer:
Pre-checks (if applicable):
Summary by CodeRabbit
Chores
Bug Fixes
Tests