chore(SREP-4482, SREP-4486, SREP-4800: Boilerplate Update for Agentic SDLC Rollout)#299
chore(SREP-4482, SREP-4486, SREP-4800: Boilerplate Update for Agentic SDLC Rollout)#299charlesgong wants to merge 3 commits into
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. |
|
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 (2)
📒 Files selected for processing (16)
WalkthroughThis PR updates build infrastructure base images across multiple Dockerfiles and CI configuration, introduces a comprehensive pre-commit hooks configuration for code hygiene and static analysis, adjusts Codecov coverage thresholds with explicit targets, and modifies team ownership aliases by removing and adding specific members to multiple teams. ChangesBuild Infrastructure: Base Image Updates
Developer Tooling and Code Quality Configuration
Team Governance
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 12✅ Passed checks (12 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 Additional details and impacted files@@ Coverage Diff @@
## master #299 +/- ##
==========================================
- Coverage 58.34% 58.24% -0.10%
==========================================
Files 26 26
Lines 1942 1947 +5
==========================================
+ Hits 1133 1134 +1
- Misses 731 735 +4
Partials 78 78
🚀 New features to boost your workflow:
|
openshift-ci
Bot
added
the
needs-rebase
|
[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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
build/Dockerfile (1)
9-17:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRun the runtime image as non-root.
The final image still runs as root because no
USERis set. Please switch to a non-root UID/GID beforeENTRYPOINTto reduce container breakout impact.Suggested hardening patch
FROM registry.access.redhat.com/ubi9/ubi-minimal:9.7-1778562320 ENV OPERATOR_PATH=/gcp-project-operator \ OPERATOR_BIN=gcp-project-operator -WORKDIR /root/ +WORKDIR /tmp COPY --from=builder /gcp-project-operator/build/_output/bin/${OPERATOR_BIN} /usr/local/bin/${OPERATOR_BIN} LABEL io.openshift.managed.name="gcp-project-operator" \ io.openshift.managed.description="This operator will be responsible for managing GCP project and credentials." +USER 1001 ENTRYPOINT ["/usr/local/bin/gcp-project-operator"]🤖 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 `@build/Dockerfile` around lines 9 - 17, The Dockerfile currently leaves the container running as root (ENTRYPOINT /usr/local/bin/gcp-project-operator); create and switch to an unprivileged user and group and ensure the operator binary and any required directories are owned by that UID/GID before ENTRYPOINT. Specifically, add steps to create a non-root user/group (e.g., gcpop), chown /usr/local/bin/${OPERATOR_BIN} (and any runtime dirs) to that user, and set USER to that UID/GID so the container runs non-root when launching ENTRYPOINT.
🤖 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 @.codecov.yml:
- Around line 11-14: Replace the static project coverage target in the
.codecov.yml project.default.target (currently "35%") with "auto" so Codecov
uses the base-commit comparison, and keep the threshold
(project.default.threshold) at 1% (or adjust as desired) to allow minor
fluctuations while preventing regressions.
- Around line 15-18: The .codecov.yml currently ignores generated files like
"zz_generated*.go" but doesn't exclude the auto-generated fips boilerplate;
update the ignore list in .codecov.yml (the same section that contains
"zz_generated*.go") to add "fips.go" (or a suitable pattern like "fips*.go") so
the auto-generated file is excluded from coverage checks and won't affect the
patch/threshold metrics.
In @.pre-commit-config.yaml:
- Around line 123-133: The hook labeled id "rbac-wildcard-check" currently runs
entry "bash -c 'make rbac-wildcard-check'" which will block commits on non-zero
exit despite the comment saying "warn-only"; update the hook to match the docs
by wrapping the make invocation with the same timeout guard used by other local
hooks and ensure the hook suppresses non-zero exit codes (so it never fails the
commit) while still emitting diagnostic output (i.e., run the check under a
timeout and always return success), and also update the human-readable
name/comment if you prefer to make it explicit that this is non-blocking; target
the hook definition for changes.
---
Outside diff comments:
In `@build/Dockerfile`:
- Around line 9-17: The Dockerfile currently leaves the container running as
root (ENTRYPOINT /usr/local/bin/gcp-project-operator); create and switch to an
unprivileged user and group and ensure the operator binary and any required
directories are owned by that UID/GID before ENTRYPOINT. Specifically, add steps
to create a non-root user/group (e.g., gcpop), chown
/usr/local/bin/${OPERATOR_BIN} (and any runtime dirs) to that user, and set USER
to that UID/GID so the container runs non-root when launching ENTRYPOINT.
🪄 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: ff01cf10-3a6d-4de9-b082-8942056f79f8
⛔ 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/README.mdis excluded by!boilerplate/**boilerplate/openshift/golang-osd-operator/ensure.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/updateis excluded by!boilerplate/**
📒 Files selected for processing (6)
.ci-operator.yaml.codecov.yml.pre-commit-config.yamlOWNERS_ALIASESbuild/Dockerfilebuild/Dockerfile.olm-registry
| project: | ||
| default: | ||
| target: 35% | ||
| threshold: 1% |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify current coverage levels and check if 35% target is appropriate
# Check recent coverage reports in CI logs or codecov history
echo "Current project coverage per codecov bot: 58.34%"
echo "Proposed target: 35%"
echo "Gap: 23.34%"
echo ""
echo "Recommendation: Consider raising target to ~55-58% to maintain current quality level"Repository: openshift/gcp-project-operator
Length of output: 242
🌐 Web query:
What is the recommended best practice for setting Codecov project coverage targets relative to current coverage levels?
💡 Result:
Recommended best practice: set Codecov “project” coverage targets to auto (relative to the base commit / PR target), with a non-zero threshold (leniency) so coverage can improve steadily without blocking due to small fluctuations. Codecov’s status checks support target: auto, which compares the commit’s coverage against the PR base (parent/base commit) coverage rather than requiring an absolute fixed percentage. This is the intended way to enforce that new changes must maintain or increase project coverage over time [1][2]. If you expect legitimate variance (e.g., flaky tests or other normal churn), keep target: auto but set a threshold that allows a small drop while still requiring improvement relative to the base. Codecov explicitly calls out using threshold to “give some leeway” and shows threshold: 10% as an example for relaxing an auto target [2]. Example (project check relative to current levels): coverage: status: project: default: target: auto threshold: 0% # or a small non-zero value to allow minor drops base: auto This config makes the required “success” level track the current base coverage rather than a static number, aligning the target with how your coverage stands today [1][2].
Citations:
- 1: https://docs.codecov.io/docs/commit-status
- 2: https://docs.codecov.com/v4.6/docs/common-recipe-list
Use target: auto instead of a static percentage for project coverage.
The project coverage target is set to a static 35%, which is significantly below the current coverage of 58.34%. However, the issue isn't just the gap—it's the approach itself. Per Codecov best practices, project targets should use target: auto to compare against the base commit, not a static percentage. This automatically tracks current coverage levels and prevents regression without requiring manual adjustment. Set target: auto with a threshold value (e.g., 1%) to allow minor fluctuations while still enforcing improvement.
🤖 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 @.codecov.yml around lines 11 - 14, Replace the static project coverage
target in the .codecov.yml project.default.target (currently "35%") with "auto"
so Codecov uses the base-commit comparison, and keep the threshold
(project.default.threshold) at 1% (or adjust as desired) to allow minor
fluctuations while preventing regressions.
| patch: | ||
| default: | ||
| target: 50% | ||
| threshold: 1% |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Locate the uncovered line in fips.go mentioned by codecov
# Find fips.go and show recent changes
fd -t f 'fips.go$' --exec echo "File: {}" \; --exec git diff master...HEAD -- {} \;Repository: openshift/gcp-project-operator
Length of output: 144
🏁 Script executed:
# Get file info and content for fips.go
wc -l ./fips.go
head -50 ./fips.goRepository: openshift/gcp-project-operator
Length of output: 352
🏁 Script executed:
# Try alternative git commands to see recent changes
git log --oneline -5 -- ./fips.go
git show HEAD:./fips.go | head -50Repository: openshift/gcp-project-operator
Length of output: 420
🏁 Script executed:
# Check for test files related to fips functionality
fd -t f '.*fips.*test.*' -o '.*test.*fips.*'Repository: openshift/gcp-project-operator
Length of output: 222
🏁 Script executed:
# Check git status for unstaged changes
git status --short ./fips.goRepository: openshift/gcp-project-operator
Length of output: 56
🏁 Script executed:
# Check the actual git state - see what branches exist
git branch -a | head -20Repository: openshift/gcp-project-operator
Length of output: 178
🏁 Script executed:
# Try to see the actual changes in the PR using git diff HEAD~1 or similar
git log --oneline -10Repository: openshift/gcp-project-operator
Length of output: 155
🏁 Script executed:
# Check the .codecov.yml file completely
cat -n .codecov.ymlRepository: openshift/gcp-project-operator
Length of output: 845
🏁 Script executed:
# Look for any exclude patterns in codecov config
rg "exclude|ignore" .codecov.yml -A 2 -B 2Repository: openshift/gcp-project-operator
Length of output: 146
Add fips.go to the .codecov.yml ignore list to exclude auto-generated boilerplate from coverage requirements.
The fips.go file is auto-generated boilerplate marked with "BOILERPLATE GENERATED -- DO NOT EDIT" and only executes when the fips_enabled build tag is active. Standard CI tests don't use this tag, so the code is never executed in normal test runs. Rather than forcing test coverage for this auto-generated conditional code, update the ignore section of .codecov.yml to exclude it—similar to how zz_generated*.go files are already excluded.
🤖 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 @.codecov.yml around lines 15 - 18, The .codecov.yml currently ignores
generated files like "zz_generated*.go" but doesn't exclude the auto-generated
fips boilerplate; update the ignore list in .codecov.yml (the same section that
contains "zz_generated*.go") to add "fips.go" (or a suitable pattern like
"fips*.go") so the auto-generated file is excluded from coverage checks and
won't affect the patch/threshold metrics.
| # 6. RBAC WILDCARD CHECK | target < 5s | warn-only (blocking after cleanup) | ||
| # Rejects wildcard RBAC in deploy/ manifests (verbs/resources: ["*"] | ||
| # or multi-line - '*' format). Logic lives in standard.mk target | ||
| # 'rbac-wildcard-check' for readability and reuse. | ||
| # Fix: replace wildcards with explicit verbs and resource names. | ||
| # ----------------------------------------------------------------------- | ||
| - id: rbac-wildcard-check | ||
| name: RBAC wildcard permissions | ||
| language: system | ||
| entry: bash -c 'make rbac-wildcard-check' | ||
| files: ^deploy/.*\.ya?ml$ |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does pre-commit provide a native warn-only mode for hooks, or do non-zero hook exits always block commits?
💡 Result:
No—pre-commit does not provide a native “warn-only” mode where non-zero hook exits still allow the commit. What happens instead: 1) A hook must effectively “pass” (i.e., pre-commit treats non-zero exit as failure) to avoid blocking the commit. pre-commit docs state that if a commit-msg/prepare-commit-msg hook exits nonzero, “the commit will be aborted.” [1][2] 2) The pre-commit maintainers explicitly discuss that they don’t provide an out-of-the-box warning-only mechanism; the recommended approach is to change the hook command so it exits 0 while emitting warnings (e.g., || true or using the underlying tool’s “exit-zero” option). [3] 3) There is a special-purpose workaround helper pre-commit hazmat ignore-exit-code, introduced in v4.5.0, which can suppress exit-code failures for a hook (with an explicit warning that it’s a “bad idea” to introduce warning noise). [1][2] So in general: non-zero hook exits block commits; “warn-only” is not a normal native mode, but you can achieve it by forcing exit 0 (or by using the hazmat helper). Example patterns from the pre-commit community: - Wrap the hook command so failures don’t propagate: entry: bash -c 'tool ... "$@" || true' -- [3] - For tools that support it, use their --exit-zero/equivalent so the hook exits 0 while still printing issues (example given with flake8). [3] - Or use pre-commit hazmat ignore-exit-code to ignore a hook’s exit code (new in v4.5.0). [1][2]
Citations:
- 1: https://pre-commit.com/
- 2: https://github.com/pre-commit/pre-commit.com/blob/main/sections/advanced.md
- 3: Can I show warning message without blocking the commit? pre-commit/pre-commit#923
Fix documentation/implementation mismatch and add timeout guard.
Line 123 claims "warn-only", but pre-commit does not support a native warn-only mode—non-zero hook exits always block commits. The current entry point (make rbac-wildcard-check) has no exit-code suppression and will therefore block, contradicting the documented behavior. Additionally, this hook lacks the timeout wrapper used by other local hooks in the file.
Suggested fix
- # 6. RBAC WILDCARD CHECK | target < 5s | warn-only (blocking after cleanup)
+ # 6. RBAC WILDCARD CHECK | target < 5s | error
@@
- entry: bash -c 'make rbac-wildcard-check'
+ entry: bash -c 'T=$(command -v timeout || command -v gtimeout || echo); ${T:+$T 30s} make rbac-wildcard-check'📝 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.
| # 6. RBAC WILDCARD CHECK | target < 5s | warn-only (blocking after cleanup) | |
| # Rejects wildcard RBAC in deploy/ manifests (verbs/resources: ["*"] | |
| # or multi-line - '*' format). Logic lives in standard.mk target | |
| # 'rbac-wildcard-check' for readability and reuse. | |
| # Fix: replace wildcards with explicit verbs and resource names. | |
| # ----------------------------------------------------------------------- | |
| - id: rbac-wildcard-check | |
| name: RBAC wildcard permissions | |
| language: system | |
| entry: bash -c 'make rbac-wildcard-check' | |
| files: ^deploy/.*\.ya?ml$ | |
| # 6. RBAC WILDCARD CHECK | target < 5s | error | |
| # Rejects wildcard RBAC in deploy/ manifests (verbs/resources: ["*"] | |
| # or multi-line - '*' format). Logic lives in standard.mk target | |
| # 'rbac-wildcard-check' for readability and reuse. | |
| # Fix: replace wildcards with explicit verbs and resource names. | |
| # ----------------------------------------------------------------------- | |
| - id: rbac-wildcard-check | |
| name: RBAC wildcard permissions | |
| language: system | |
| entry: bash -c 'T=$(command -v timeout || command -v gtimeout || echo); ${T:+$T 30s} make rbac-wildcard-check' | |
| files: ^deploy/.*\.ya?ml$ |
🤖 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 123 - 133, The hook labeled id
"rbac-wildcard-check" currently runs entry "bash -c 'make rbac-wildcard-check'"
which will block commits on non-zero exit despite the comment saying
"warn-only"; update the hook to match the docs by wrapping the make invocation
with the same timeout guard used by other local hooks and ensure the hook
suppresses non-zero exit codes (so it never fails the commit) while still
emitting diagnostic output (i.e., run the check under a timeout and always
return success), and also update the human-readable name/comment if you prefer
to make it explicit that this is non-blocking; target the hook definition for
changes.
|
@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