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 selected for processing (1)
WalkthroughUpdates CI and Docker build base images, replaces pre-commit hookset and adds pre-commit procedure docs, rotates dev TLS/CA material, makes explicit condition/operation handling in controllers/webhooks/metrics, removes a few OWNERS entries, and applies minor whitespace/lint adjustments and test lint comments. ChangesBuild infrastructure
Pre-commit tooling and policy
Development TLS and webhook manifests
Runtime behavior, controllers, metrics, and webhooks
OWNERS, tests, lint and whitespace fixes
🎯 3 (Moderate) | ⏱️ ~25 minutes Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning, 1 inconclusive)
✅ Passed checks (9 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❌ Patch coverage is ❌ Your patch status has failed because the patch coverage (33.33%) 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.17% -0.08%
==========================================
Files 62 62
Lines 4125 4130 +5
==========================================
Hits 2444 2444
- Misses 1532 1537 +5
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.
|
@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
Bug Fixes
Tests