chore(SREP-4482, SREP-4486, SREP-4800: Boilerplate Update for Agentic SDLC Rollout)#724
chore(SREP-4482, SREP-4486, SREP-4800: Boilerplate Update for Agentic SDLC Rollout)#724charlesgong wants to merge 7 commits into
Conversation
Adds .pre-commit-config.yaml with Tier 1 common hooks mirroring ci/prow/lint. Golden rules: SREP-4450 Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…c SDLC Rollout - Update golangci.yml with expanded linter set - Update standard.mk, pre-commit-config.yaml - Update .codecov.yml and OWNERS_ALIASES - Fix errcheck in fips.go (_, _ = fmt.Println) Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
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. |
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository: openshift/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: ⛔ Files ignored due to path filters (14)
📒 Files selected for processing (24)
WalkthroughThis PR updates build infrastructure (boilerplate and base images), establishes a tiered pre-commit hook configuration aligned to golden rules with agentic documentation, enables code coverage thresholds, adds comprehensive developer documentation, refreshes TLS certificates for development deployments, and performs hygiene cleanup across Kubernetes manifests and team aliases. ChangesInfrastructure, Build, and Developer Experience Updates
🎯 2 (Simple) | ⏱️ ~12 minutes Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (10 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 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: charlesgong The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #724 +/- ##
=======================================
Coverage 59.24% 59.24%
=======================================
Files 62 62
Lines 4125 4125
=======================================
Hits 2444 2444
Misses 1532 1532
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
| ``` | ||
| PRE-COMMIT SUMMARY | ||
| ================== | ||
| Passed: <list of hook IDs> | ||
| Auto-fixed: <list of hook IDs> → files staged | ||
| Fixed: <list of hook IDs> → changes applied | ||
| Failed: <list of hook IDs> → escalated to human | ||
| Attempts: <N> of 2 maximum |
There was a problem hiding this comment.
Add language specification to fenced code block.
The fenced code block lacks a language identifier. Add a language specification to satisfy markdown linting rules.
📝 Proposed fix
-```
+```text
PRE-COMMIT SUMMARY
==================
Passed: <list of hook IDs>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ``` | |
| PRE-COMMIT SUMMARY | |
| ================== | |
| Passed: <list of hook IDs> | |
| Auto-fixed: <list of hook IDs> → files staged | |
| Fixed: <list of hook IDs> → changes applied | |
| Failed: <list of hook IDs> → escalated to human | |
| Attempts: <N> of 2 maximum |
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 77-77: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 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 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.
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
| - repo: https://github.com/pre-commit/pre-commit-hooks | ||
| rev: v5.0.0 | ||
| hooks: | ||
| - id: check-merge-conflict | ||
| - id: trailing-whitespace | ||
| args: [--markdown-linebreak-ext=md] | ||
| - id: end-of-file-fixer | ||
| - id: check-yaml | ||
| name: YAML syntax (deploy/) | ||
| files: ^deploy/.*\.ya?ml$ | ||
| args: [--allow-multiple-documents] | ||
|
|
There was a problem hiding this comment.
Add required large-file protection hook.
check-added-large-files is missing, so oversized artifacts can slip into commits. Please add a large-file check in this hook set to match repository policy.
As per coding guidelines ".pre-commit-config.yaml: 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 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.
| - repo: local | ||
| hooks: | ||
|
|
||
| # ----------------------------------------------------------------------- | ||
| # 4. COMPILE CHECK | target < 10s cached | error | golden rules 3, 4 | ||
| # ----------------------------------------------------------------------- | ||
| - id: go-build | ||
| name: go build | ||
| language: system | ||
| entry: bash -c 'T=$(command -v timeout || command -v gtimeout || echo); ${T:+$T 90s} go build ./...' | ||
| types: [go] | ||
| pass_filenames: false | ||
|
|
||
| # ----------------------------------------------------------------------- | ||
| # 5. DEPENDENCY DRIFT | target < 10s | error | golden rules 3, 5, 8 | ||
| # Fix: run 'go mod tidy' and stage go.mod and go.sum. | ||
| # ----------------------------------------------------------------------- | ||
| - id: go-mod-tidy | ||
| name: go mod tidy | ||
| language: system | ||
| entry: bash -c 'T=$(command -v timeout || command -v gtimeout || echo); ${T:+$T 120s} go mod tidy && git diff --exit-code go.mod go.sum' | ||
| files: '(\.go$|go\.(mod|sum)$)' | ||
| exclude: '^vendor/' | ||
| pass_filenames: false |
There was a problem hiding this comment.
Restore required go-fmt hook.
go-mod-tidy is present, but go-fmt is not configured. Please add a go-fmt pre-commit hook to keep formatting enforcement aligned with policy.
As per coding guidelines ".pre-commit-config.yaml: 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 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.
| # CLAUDE.md | ||
|
|
||
| This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository. |
There was a problem hiding this comment.
Fix top-level document title to match file name.
Line 1 currently says # CLAUDE.md in AGENTS.md; rename it to # AGENTS.md to avoid confusion in navigation/search.
As per coding guidelines "AGENTS.md: Document agents and their capabilities in AGENTS.md".
🤖 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 `@AGENTS.md` around lines 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.
|
@charlesgong: all tests passed! 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. |
3 participants
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
Documentation