Add README.md documentation for cmd/ tools#5259
Draft
Prucek wants to merge 1 commit into
Draft
Conversation
Contributor
|
Pipeline controller notification For optional jobs, comment This repository is configured in: automatic mode |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (69)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
💤 Files with no reviewable changes (1)
👮 Files not reviewed due to content moderation or server errors (68)
📝 Walkthrough🚥 Pre-merge checks | ❌ 12❌ Failed checks (12 inconclusive)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
Contributor
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Prucek 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
Prucek
changed the title
Prucek
force-pushed
the
rename-agents-to-readme
branch
from
f3bde1b to
f96265a
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 20
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (18)
cmd/private-org-sync/README.md (2)
121-200:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRefresh the old example to the current CLI.
The example still uses
--config-pathand--git-dir, but the new flags table documents--config-dirinstead. Please update or drop this example so it matches the current invocation.🤖 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 `@cmd/private-org-sync/README.md` around lines 121 - 200, The example commands in the README.md file use outdated CLI flags that do not match the current documentation. Update the first command example that currently uses --config-path and --git-dir flags to use --config-dir instead, making it consistent with the flags table documentation and the second example command which already uses the correct --config-dir flag. Ensure all example invocations reflect the current CLI interface.
76-97:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winBring the legacy deployment notes up to date.
This appendix still says missing destination repositories are ignored unconditionally and that merge-conflict divergence is silently tolerated. That conflicts with the new
--fail-on-missing-destination/ error-handling section above, so readers now get two different behavior contracts.🤖 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 `@cmd/private-org-sync/README.md` around lines 76 - 97, The deployment notes section still describes outdated behavior regarding missing destination repositories and merge conflict handling. Update this section to reflect the current implementation by removing the statement that missing destinations are unconditionally ignored and instead reference the `--fail-on-missing-destination` flag mentioned in the error-handling section above. Similarly, update the merge conflict statement to align with the current error-handling behavior documented elsewhere in the README to ensure consistency and clarity for readers about the actual tool behavior.cmd/branchingconfigmanagers/README.md (1)
254-345:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove the obsolete contract appendix.
The trailing contract/deployment block is still marked incomplete and leaves
TODO: What period. That undermines the new top-level docs and keeps unfinished guidance in the canonical README.🤖 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 `@cmd/branchingconfigmanagers/README.md` around lines 254 - 345, Remove the entire "Config Manager Contract" section from the README.md file, which includes all subsections from "## Config Manager Contract" through the end of the "Deploying New Config Managers" section. This section is incomplete and marked with NOTE and TODO comments that undermine the documentation. Delete all content related to the contract definition, product lifecycle data structure, requirements for config managers, and deployment instructions that follow the incomplete deployment note.cmd/config-brancher/README.md (1)
94-98:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate the retained bump-mode section to match
--bump-release.The new summary/table uses
--bump-release, but this legacy block still documents--bump. That leaves two different flag names for the same mode. Please rewrite or remove this tail section so the README stays self-consistent.Proposed fix
- In the bumping mode, it moves the development branch to promote to the version in the `--bump` flag, enabling the + In bump mode (`--bump-release`), it moves the development branch to promote to the target version, enabling the promotion in the release branch that used to match the dev branch version and disabling promotion in the release branch that now matches the dev branch version.🤖 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 `@cmd/config-brancher/README.md` around lines 94 - 98, The "Bumping configuration" section in the README still references the `--bump` flag while the rest of the documentation uses `--bump-release`. Update this legacy section by replacing all instances of `--bump` with `--bump-release` to maintain consistency throughout the README, or alternatively consider removing or rewriting this section if the information is already adequately covered in the new summary/table using the correct flag name.cmd/autoowners/README.md (1)
106-107:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix the
--self-approveusage snippet.The usage block shows
-self-approve approved, but the flags table documents--self-approveas a boolean flag. That extra positional value is misleading and should be removed or regenerated from the actual CLI help.♻️ Suggested fix
- -self-approve approved + -self-approve🤖 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 `@cmd/autoowners/README.md` around lines 106 - 107, The usage snippet for the `--self-approve` flag in the README.md incorrectly shows `-self-approve approved` with a positional argument, but the flags table documents it as a boolean flag without any arguments. Remove the `approved` positional value from the usage example at line 106-107 so that it correctly shows `-self-approve` as a standalone boolean flag, or regenerate the usage documentation from the actual CLI help output to ensure consistency with the flag's actual implementation.cmd/sippy-config-generator/README.md (1)
65-97:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winTrim the legacy manual-run appendix.
The new spec-style sections already cover purpose, flow, flags, and deployment; this older block duplicates that guidance and makes the README harder to scan.
🤖 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 `@cmd/sippy-config-generator/README.md` around lines 65 - 97, Remove the legacy manual-run appendix section from the README that includes the Related, Example invocation, Commit the output, and Customization subsections. These sections duplicate information already covered in the newer spec-style sections of the documentation. Delete this entire block to improve the README's readability and reduce redundant guidance.cmd/testgrid-config-generator/README.md (1)
75-132:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove or rewrite the legacy appendix.
It repeats older manual-run guidance and conflicts with the new spec-style sections above, so readers get mixed instructions.
🤖 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 `@cmd/testgrid-config-generator/README.md` around lines 75 - 132, The legacy appendix section starting with "Note: Go 1.13 is required." and continuing through the end of the diff repeats older manual-run guidance and conflicts with the new spec-style sections above it, creating mixed instructions for readers. Remove this entire legacy appendix section or completely rewrite it to eliminate the duplication and conflicts with the newer documentation structure. The section includes the Go 1.13 note, build instructions using the go build command, the manual run example with testgrid-config-generator flags, and verification steps that appear to duplicate guidance from earlier in the document.cmd/ci-secret-bootstrap/README.md (1)
68-141:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRemove the stale Bitwarden/kubeconfig section.
This block still documents the old Bitwarden mappings,
--bw-password-path/--bw-user, and kubeconfig-context targeting. That directly conflicts with the Vault/GSM model above and leaves the README with two different contracts.🤖 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 `@cmd/ci-secret-bootstrap/README.md` around lines 68 - 141, The "Args and config.yaml" section and the "Run" section of the README document the legacy Bitwarden/kubeconfig approach with outdated parameters like --bw-password-path, --bw-user, and context-based cluster targeting, which conflicts with the current Vault/GSM model described earlier in the documentation. Remove the entire section starting from "## Args and config.yaml" through the end of the "## Run" section (including the code example with the ci-secret-bootstrap command) to eliminate this documentation contradiction and provide a single, consistent contract for the tool's usage.cmd/ci-secret-generator/README.md (1)
72-125:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDelete the legacy Bitwarden example.
The bottom half still shows Bitwarden item/attachment/password mappings and
--bw-*flags, but the new intro says generation uploads to Vault and optionally GSM. Users will follow the wrong CLI/config shape if this stays.🤖 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 `@cmd/ci-secret-generator/README.md` around lines 72 - 125, Remove the legacy Bitwarden documentation and examples from the README file. Delete the YAML configuration examples that show Bitwarden attribute and password mappings (the sections demonstrating password as an attribute name and field configurations), and remove the Run section that shows the Bitwarden CLI usage with flags like --bw-password-path and --bw-user. Replace these removed sections with documentation that reflects the current behavior of uploading secrets to Vault and optionally GSM instead, ensuring the CLI flags and configuration examples match the actual tool functionality.cmd/vault-secret-collection-manager/README.md (1)
98-139:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRemove the old endpoint docs.
This legacy section still says
PATCH /secretcollection/:nameedits members, while the authoritative table above usesPUT /secretcollection/:name/members. Keeping both makes the API contract ambiguous.🤖 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 `@cmd/vault-secret-collection-manager/README.md` around lines 98 - 139, Remove the outdated endpoint documentation in the Endpoints section of the README that describes PATCH /secretcollection/:name for changing secret collection members. This endpoint documentation is inconsistent with the authoritative API contract which uses PUT /secretcollection/:name/members instead. Delete the bullet point containing the PATCH endpoint description to eliminate the ambiguity and keep only the correct current endpoint in the documentation.cmd/gsm-secret-sync/README.md (1)
72-125:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRewrite the old Bitwarden usage block.
This legacy section still talks about Bitwarden entries and
--bw-password-path/--bw-user, which conflicts with the new Vault/GCP workflow described above. Please remove or replace it so the README doesn't advertise two incompatible backends.🤖 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 `@cmd/gsm-secret-sync/README.md` around lines 72 - 125, Remove the legacy Bitwarden usage block from the README that contains references to `--bw-password-path` and `--bw-user` flags, as it conflicts with the new GCP and Vault workflow documented in the current sections. Search the entire file for any mentions of Bitwarden-related configuration, flags, environment variables, or usage examples and delete them completely to ensure the README only documents the current GCP-based backend and doesn't advertise incompatible or deprecated functionality.cmd/ocp-build-data-enforcer/README.md (1)
54-67:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRewrite the deployment bullets.
This section reads like a blend of two tools:
ocp-build-data-enforceris config-driven and works from fetched Dockerfiles, but it does not iterate a fixedopenshift-4.21/imagestree or directly edit Dockerfiles as written here. Please trim this to the actual deployment flow. This differs fromcmd/ocp-build-data-enforcer/main.go.🤖 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 `@cmd/ocp-build-data-enforcer/README.md` around lines 54 - 67, The Deployment section contains inaccurate details that do not match the actual implementation of ocp-build-data-enforcer as defined in main.go. Rewrite the deployment bullets to accurately describe the tool's config-driven workflow, removing the misleading references to iterating over fixed openshift-4.21/images trees and directly editing Dockerfiles. Ensure the rewritten bullets reflect the actual deployment flow and purpose of the enforcer tool as implemented in the codebase.cmd/registry-replacer/README.md (1)
59-67:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSeparate the optional PR flow from the main flow.
This deployment section mixes the default registry-replacement path with the optional
--ensure-correct-promotion-dockerfilebehavior, and the last bullet makes it sound like the tool edits the Dockerfile itself. In practice it rewrites ci-operator config from fetched Dockerfile content, with PR creation gated separately. Tighten this so operators don't infer the wrong action. This differs fromcmd/registry-replacer/main.go.🤖 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 `@cmd/registry-replacer/README.md` around lines 59 - 67, The Deployment section conflates the core registry-replacement behavior with the optional `--ensure-correct-promotion-dockerfile` behavior and PR creation flow. Restructure this section to clearly separate the mandatory registry-replacement logic (which finds ci-operator configs, downloads Dockerfiles, updates configs to replace registry references, and removes invalid/obsolete replacements) from the optional PR flow (which is gated by the `--ensure-correct-promotion-dockerfile` flag and handles pushing to forks and creating PRs via the GitHub API). Clarify that the tool rewrites ci-operator configs based on fetched Dockerfile content rather than directly editing the Dockerfile itself, ensuring the documentation aligns with the actual behavior in main.go.cmd/prow-job-dispatcher/README.md (1)
67-72:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDrop the stale dispatcher description.
This block still describes the old "last seven days / least jobs wins" algorithm and conflicts with the updated dispatch logic above and in the flag table. Keeping both versions in the same README makes the scheduler behavior ambiguous.
♻️ Suggested cleanup
- * It starts off by figuring out how many runs of each Prow jobs we had in the last seven days by querying the Prometheus instance in Prow-monitoring stack. - * It groups all jobs from a Prow job file together and will always try to put all of them on the same cluster. - * If a job has config stating it must be on a specific cluster, that will always be respected. This could lead to a job with tests on different clusters. We should not have many of those cases. - * If all e2e jobs in a group run on the same cloud provider, it will only consider clusters on that cloud provider, if any. Otherwise, all build clusters are considered. - * It will then choose the cluster with the least number of jobs, based on the Prometheus metrics and the already dispatched jobs.🤖 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 `@cmd/prow-job-dispatcher/README.md` around lines 67 - 72, The README contains a stale description of the dispatcher algorithm that references the old "last seven days / least jobs wins" approach and conflicts with the updated dispatch logic described elsewhere in the document. Remove the entire stale block that begins with "It starts off by figuring out how many runs of each Prow jobs we had in the last seven days" through "based on the Prometheus metrics and the already dispatched jobs." This will eliminate the ambiguity caused by having two conflicting versions of the scheduler behavior in the same README.cmd/job-run-aggregator/README.md (1)
109-117:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove the stale
create-tablesexample.
create-tablesis not one of the documented subcommands here, so this snippet will fail as written. It also duplicates thecreate-releasesexample above it.🤖 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 `@cmd/job-run-aggregator/README.md` around lines 109 - 117, Remove the final stale example from the README.md file that uses the create-tables subcommand (the block starting with the comment "This will create the Jobs table:"). This example duplicates the create-releases example shown immediately before it and references a subcommand that is not documented or supported, which will cause it to fail when executed. Delete both the comment line and the associated dlv exec command line that follows it.cmd/image-graph-generator/README.md (1)
79-89:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRegenerate the usage snippet.
This block still lists
-ci-operator-configs-path/-image-mirroring-path, butcmd/image-graph-generator/main.goonly accepts--release-repoand--graphql-endpoint-address. As written, copying the example will fail immediately.🤖 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 `@cmd/image-graph-generator/README.md` around lines 79 - 89, The usage documentation snippet in the README shows command-line flags that no longer match the actual implementation in main.go. Update the usage block to reflect the current flags accepted by the image-graph-generator tool, removing the obsolete -ci-operator-configs-path and -image-mirroring-path flags and replacing them with the actual flags currently implemented (--release-repo and --graphql-endpoint-address) to ensure the documentation accurately represents what users will encounter when running the command.cmd/slack-bot/README.md (1)
70-77:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRestore the
hack/prefix in the sample command.The command currently omits the directory named in the prose, so readers will hit a missing-file error if they copy it verbatim.
🛠️ Suggested fix
-Run the `hack/local-slack-bot.sh` script like so: `RELEASE_REPO_DIR=<your openshift/release repo dir> bash local-slack-bot.sh` +Run the `hack/local-slack-bot.sh` script like so: `RELEASE_REPO_DIR=<your openshift/release repo dir> bash hack/local-slack-bot.sh`🤖 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 `@cmd/slack-bot/README.md` around lines 70 - 77, The command example in the Local testing section is missing the directory prefix for the script path. Update the bash command that runs the local-slack-bot.sh script to include the hack/ directory prefix before the script name so that readers following the instructions verbatim will correctly reference the script location at hack/local-slack-bot.sh.cmd/repo-init/README.md (1)
164-176:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix the local testing command path.
The prose points to
hack/local-repo-init-ui.sh, but the sample commands omithack/, so the copy-paste example fails from the repo root.🛠️ Suggested fix
-The easiest way to run the API/UI locally is to execute the `/hack/local-repo-init-ui.sh` script, like this: +The easiest way to run the API/UI locally is to execute the `hack/local-repo-init-ui.sh` script, like this: ```shell -repo-init --mode=cli --release-repo=/path/to/release/repo +./hack/local-repo-init-ui.sh start@@
-./local-repo-init-ui.sh stop
+./hack/local-repo-init-ui.sh stop</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@cmd/repo-init/README.mdaround lines 164 - 176, The shell command examples
in the README.md file for running local-repo-init-ui.sh are missing the
directory path prefix. Both the start and stop command examples currently show
./local-repo-init-ui.sh but should include the full relative path as
./hack/local-repo-init-ui.sh to match the prose description and allow the
copy-paste examples to work correctly from the repository root. Update both
command instances to include the hack/ directory prefix so users can
successfully execute them without encountering a file not found error.</details> <!-- cr-comment:v1:7c51526aea6d6f0492a9b906 --> </blockquote></details> </blockquote></details>🧹 Nitpick comments (7)
cmd/cluster-init/README.md (1)
98-110: ⚡ Quick winRemove the legacy usage block.
The trailing Create/Update/Create PR section duplicates the new lifecycle docs and reintroduces flags that are not covered above. Please fold the surviving examples into the flag tables or drop this block so the README has one source of truth.
🤖 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 `@cmd/cluster-init/README.md` around lines 98 - 110, Remove the legacy usage documentation block containing the "## Create", "## Update", and "## Create PR" sections as these duplicate the lifecycle documentation covered elsewhere in the README and reintroduce flags like --release-repo, --cluster-name, --update, --create-pr, --github-token-path, and --self-approve that should be documented in a single location. Either consolidate these examples into the flag reference tables if they provide unique value, or delete the entire block to establish a single source of truth for the tool's usage documentation.cmd/blocking-issue-creator/README.md (2)
56-85: ⚡ Quick winRemove the stale legacy sections.
The older “What it does / Why it exists / How it works / How is it deployed” block repeats the structured docs above and makes the README look like it has two canonical descriptions. Drop it unless it adds information that is missing from the new sections.
🤖 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 `@cmd/blocking-issue-creator/README.md` around lines 56 - 85, The README contains duplicate sections "What it does", "Why it exists", "How it works", and "How is it deployed" that repeat information already covered in the structured documentation above. Remove these four legacy section headers and their content from the README, unless the information they contain is unique and not already present in the structured sections above. If all the content is already covered, simply delete the entire legacy block to avoid redundancy and maintain a single canonical description.
18-19: ⚡ Quick winVerify the rate-limit numbers.
The flow says 5 seconds between repos to stay within 30 requests/minute, but the deployment note says 300 requests/minute burst/sustained. Please confirm which limit is real and keep one set of numbers in the README.
Also applies to: 51-52
🤖 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 `@cmd/blocking-issue-creator/README.md` around lines 18 - 19, The README contains conflicting GitHub API rate limit numbers: one section mentions 30 requests/minute as the secondary rate limit while another section (also noted in the deployment notes) references 300 requests/minute for burst/sustained rate limits. Verify the correct GitHub API rate limit that applies to this tool's use case, then update both occurrences (the rate limiting description around line 18-19 and the deployment notes around line 51-52) to use consistent and accurate numbers throughout the documentation. Ensure all rate limit references align with the actual GitHub API limits being respected by the tool.cmd/check-gh-automation/README.md (1)
66-103: ⚡ Quick winMake the auth examples match the new flag table.
The legacy examples at the bottom still use a different GitHub auth shape than the table above. Please verify the current invocation and update or remove the examples so the README doesn’t document two competing modes.
🤖 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 `@cmd/check-gh-automation/README.md` around lines 66 - 103, The README examples for running check-gh-automation are using inconsistent GitHub authentication parameters compared to the flag table documented above. Review the flag table that defines the current authentication options, then update all command examples (including those in the "Pass Prow Config Options", "Determine Modified Repos from Candidate and JobSpec", and "Pass specific Repo(s) to check" sections) to consistently use the same GitHub authentication flags as specified in the flag table. Remove or update any legacy examples that document competing or outdated authentication modes to ensure the README presents a single, consistent invocation pattern.cmd/ci-images-mirror/README.md (1)
80-149: ⚡ Quick winTrim the old narrative below the new sections.
The legacy block after the deployment section repeats the workflow in a different voice and brings back the older “temporary” / two-tag explanation. Please remove or merge it so the README presents a single, consistent flow.
🤖 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 `@cmd/ci-images-mirror/README.md` around lines 80 - 149, The README contains redundant legacy narrative that repeats the workflow explanation in a different voice and contradicts the updated information that states the tool is no longer temporary. Identify and remove the legacy block of text that appears after the deployment section and reiterates the two-tag mirroring explanation and "temporary" language. Ensure the README maintains a single, consistent voice by keeping only the current narrative sections that describe the tool functionality, extensions, and operational procedures without duplication.cmd/promoted-image-governor/README.md (1)
56-107: ⚡ Quick winDelete the duplicated legacy sections.
The old “What it does / Why it exists / How it works / How is it deployed” material below the new structured doc repeats the same content and makes the README look like it has two canonical descriptions. Keep one version.
🤖 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 `@cmd/promoted-image-governor/README.md` around lines 56 - 107, The README contains duplicated sections with the same content described in "What it does", "Why it exists", "How it works", and "How is it deployed". Remove the legacy/old version of these sections (keeping only the newer, better-structured version) to eliminate redundancy and maintain a single canonical description of the promoted-image-governor tool's purpose, functionality, and deployment.cmd/github-ldap-user-group-creator/README.md (1)
76-110: ⚡ Quick winDrop the legacy README block below the new summary.
It repeats the same tool description with outdated terminology (
github-id-group,github-id, etc.), which makes the README self-contradictory. Keep a single source of truth in the new top section.🤖 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 `@cmd/github-ldap-user-group-creator/README.md` around lines 76 - 110, The README contains duplicate documentation with outdated terminology that contradicts the newly added summary section. Remove the legacy README block that appears after the "How is it deployed" section, which repeats the tool description using outdated terms such as "github-id-group" and "github-id". Keep only the new comprehensive summary section ("What it does", "Why it exists", "How it works", "Deleting users", and "How is it deployed") to maintain a single source of truth in the documentation.🤖 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 `@cmd/auto-config-brancher/README.md`: - Line 22: The README documents a --rebalancer-cron flag and its behavior (lines 22 and 48), but this flag is not implemented anywhere in the codebase. Either remove all references to the --rebalancer-cron flag and the optional rebalancer step from the README documentation, or implement the feature by adding the flag definition to FutureOptions in main.go and adding the logic to check if the current time is within +/-1 hour of the cron schedule and prepend rebalancer as step 0 when appropriate. In `@cmd/backport-verifier/README.md`: - Around line 37-48: In the "On `/validate-backports` command" section, replace the abbreviated label name `unvalidated` with the fully qualified label name `backports/unvalidated-commits` in the text describing which label gets applied when the repo has no upstream configured. This ensures consistency with the complete label names shown in the Labels table below. In `@cmd/bugzilla-backporter/README.md`: - Line 20: The README.md documentation on line 20 lists the Prometheus metric prefix as `bugzillabackporter`, but the actual implementation in main.go exposes metrics under the prefix `ci-operator-bugzilla-backporter`. Update the README.md file to replace `bugzillabackporter` with the correct metric prefix `ci-operator-bugzilla-backporter` to ensure the documentation accurately reflects what is implemented and prevents confusion when setting up dashboards or alerts. In `@cmd/ci-operator/README.md`: - Around line 18-24: The namespace handling documentation describes a fixed pattern that does not match the actual contract. Update the overview section describing namespace handling to clarify that the namespace default comes from JOB_SPEC and that {id} substitution only applies when the --namespace flag explicitly includes it, rather than stating it always uses ci-op-{hash}. Additionally, update the corresponding flag table documentation to reflect this conditional behavior so the README accurately documents when each namespace pattern is used. In `@cmd/config-change-trigger/README.md`: - Around line 12-15: In the README.md file describing the Base version configuration, locate the text that currently describes `baseSHA^1` as "the merge base" in the parenthetical explanation. Reword this text to accurately describe `baseSHA^1` as the parent commit of the base SHA rather than the merge base, making it clear that `baseSHA^1` refers to the parent commit using language like "the parent commit of the base SHA (`baseSHA^1`)". In `@cmd/determinize-peribolos/README.md`: - Around line 24-28: Add blank lines around the flags table to comply with markdownlint requirements. Insert a blank line before the "## Flags" header to separate it from preceding content, and insert another blank line after the table's last row to separate it from any following content. This ensures proper markdown formatting with appropriate whitespace around structured elements. In `@cmd/determinize-prow-config/README.md`: - Around line 36-56: Add a language tag to the code fence displaying the sharded directory structure by specifying a language identifier (such as yaml or text) immediately after the opening triple backticks. Additionally, add blank lines before the Flags section header and around the flags table to provide proper visual separation between different content blocks in the markdown document. In `@cmd/dptp-controller-manager/README.md`: - Around line 34-38: The annotation key constant TTLAnnotationKey in the serviceaccount_secret_refresher controller contains a spelling error: `serviaccount` should be `serviceaccount`. Locate the TTLAnnotationKey constant definition in pkg/controller/serviceaccount_secret_refresher/serviceaccount_secret_refresher.go and correct the typo from `serviaccount-secret-rotator.openshift.io/delete-after` to `serviceaccount-secret-rotator.openshift.io/delete-after`. Then search the entire codebase for all usages of TTLAnnotationKey and any hardcoded references to the misspelled annotation key to ensure consistency throughout the implementation. In `@cmd/generate-registry-metadata/README.md`: - Around line 41-45: The Flags section table is missing required blank lines for Markdownlint compliance. Add a blank line before the "## Flags" heading and another blank line after the table ends (after the row containing the registry flag and its description). This provides the necessary whitespace around the table structure that Markdownlint expects. In `@cmd/multi-arch-builder-controller/README.md`: - Around line 76-79: The Requirements section in the README lists `manifest-tool` binary as a required dependency, but the actual implementation uses `manifestpusher.PushImageWithManifest()` and `oc image mirror` instead (as shown in main.go). Remove the bullet point mentioning the manifest-tool binary from the Requirements section, keeping only the valid requirement about target registry credentials mounted on /.docker/config.json. In `@cmd/ocp-build-data-enforcer/README.md`: - Line 26: The README.md documentation at line 26 does not accurately reflect the actual behavior of processDockerfile. The current wording suggests that content.source.git.branch.target is evaluated unconditionally with a fallback to the repo default branch, but the actual implementation only uses content.source.git.branch.target when it starts with openshift-, otherwise falling back to the repo default branch. Update the branch-selection note to accurately describe this conditional logic and match the implementation in cmd/ocp-build-data-enforcer/main.go. In `@cmd/payload-testing-ui/README.md`: - Around line 39-50: The README documents that the --port flag defaults to 8080, but the deployment section states the service runs on port 8000, creating a contradiction. Either update the README's Deployment section to explicitly note that the deployment overrides the default port flag to 8000, or update the actual deployment configuration to use port 8080 (the documented default). Choose whichever approach aligns with the intended deployment configuration, but ensure the documentation accurately reflects the actual port being used. In `@cmd/qci-appci/README.md`: - Around line 40-53: The flags table in the README.md file is not properly separated from the surrounding content with blank lines, which causes Markdownlint validation to fail. Add a blank line after the "## Flags" heading and before the table begins with the column headers (Flag, Default, What it controls), and also add a blank line after the last table row to properly separate the table from any content that follows it. In `@cmd/result-aggregator/README.md`: - Around line 31-33: The Authentication section in the README.md file incorrectly describes the password file format as "CSV format, colon-delimited" when CSV actually refers to comma-separated values. Reword the description to remove the CSV reference and clearly state that each line contains username:password pairs in a colon-delimited format, without mentioning CSV, to avoid ambiguity about the actual file structure. In `@cmd/serviceaccount-secret-rotation-trigger/README.md`: - Around line 20-24: In the README.md file under the "Clear SA secret references" section, the current description incorrectly claims that the Kubernetes control plane will automatically re-create default token and image pull secrets. Revise this statement to accurately reflect that clearing the `secrets` and `imagePullSecrets` references only removes the references themselves, and that the tool actually triggers a refresher controller to handle the recreation of these secrets, rather than implying Kubernetes does this automatically. In `@cmd/slack-bot/README.md`: - Around line 37-54: The Flags table in the README.md file is missing blank line separation from the surrounding markdown headings, which violates markdownlint rules. Add a blank line after the "## Flags" heading and before the table begins, and add a blank line after the table ends (before any subsequent content). This ensures proper markdown formatting and keeps the documentation build clean. In `@cmd/sprint-automation/README.md`: - Around line 45-55: The Flags table in the README is missing blank lines for proper markdown formatting which causes markdownlint violations. Add a blank line between the "## Flags" heading and the table header row that starts with the pipe characters, and add another blank line after the last row of the table (the kubeconfig row) before any following content. This separation will ensure the table is properly formatted and the README passes linting. In `@cmd/sync-rover-groups/README.md`: - Around line 3-6: Update the "What" section in the README.md to clarify that the GitHub-to-Kerberos user mappings YAML file is only output when the --github-users-file flag is set. Modify the wording to indicate that one file is always emitted (the resolved group memberships), while the second file (user mappings) is optional and depends on whether the --github-users-file flag is provided. This ensures readers understand the actual default behavior of the sync-rover-groups tool. In `@cmd/testgrid-config-generator/README.md`: - Around line 56-65: Add blank lines around the flags table in the README.md to comply with markdown linting rules. Insert a blank line before the "## Flags" section header and another blank line after the last row of the table. This ensures proper whitespace separation between the table and surrounding content. In `@cmd/vault-subpath-proxy/README.md`: - Around line 4-46: The documentation has inconsistent terminology about the secret sync timing: the overview states "synchronously syncs the secret data" but the detailed flow section under "KV write validation and sync (RoundTripper)" explicitly states "After a successful write (2xx response), asynchronously syncs the secret data to all Kubernetes clusters". Update the overview section to use "asynchronously" instead of "synchronously" to match the detailed flow description and accurately reflect that Kubernetes sync occurs after the Vault write completes and returns a response, not during the request processing. --- Outside diff comments: In `@cmd/autoowners/README.md`: - Around line 106-107: The usage snippet for the `--self-approve` flag in the README.md incorrectly shows `-self-approve approved` with a positional argument, but the flags table documents it as a boolean flag without any arguments. Remove the `approved` positional value from the usage example at line 106-107 so that it correctly shows `-self-approve` as a standalone boolean flag, or regenerate the usage documentation from the actual CLI help output to ensure consistency with the flag's actual implementation. In `@cmd/branchingconfigmanagers/README.md`: - Around line 254-345: Remove the entire "Config Manager Contract" section from the README.md file, which includes all subsections from "## Config Manager Contract" through the end of the "Deploying New Config Managers" section. This section is incomplete and marked with NOTE and TODO comments that undermine the documentation. Delete all content related to the contract definition, product lifecycle data structure, requirements for config managers, and deployment instructions that follow the incomplete deployment note. In `@cmd/ci-secret-bootstrap/README.md`: - Around line 68-141: The "Args and config.yaml" section and the "Run" section of the README document the legacy Bitwarden/kubeconfig approach with outdated parameters like --bw-password-path, --bw-user, and context-based cluster targeting, which conflicts with the current Vault/GSM model described earlier in the documentation. Remove the entire section starting from "## Args and config.yaml" through the end of the "## Run" section (including the code example with the ci-secret-bootstrap command) to eliminate this documentation contradiction and provide a single, consistent contract for the tool's usage. In `@cmd/ci-secret-generator/README.md`: - Around line 72-125: Remove the legacy Bitwarden documentation and examples from the README file. Delete the YAML configuration examples that show Bitwarden attribute and password mappings (the sections demonstrating password as an attribute name and field configurations), and remove the Run section that shows the Bitwarden CLI usage with flags like --bw-password-path and --bw-user. Replace these removed sections with documentation that reflects the current behavior of uploading secrets to Vault and optionally GSM instead, ensuring the CLI flags and configuration examples match the actual tool functionality. In `@cmd/config-brancher/README.md`: - Around line 94-98: The "Bumping configuration" section in the README still references the `--bump` flag while the rest of the documentation uses `--bump-release`. Update this legacy section by replacing all instances of `--bump` with `--bump-release` to maintain consistency throughout the README, or alternatively consider removing or rewriting this section if the information is already adequately covered in the new summary/table using the correct flag name. In `@cmd/gsm-secret-sync/README.md`: - Around line 72-125: Remove the legacy Bitwarden usage block from the README that contains references to `--bw-password-path` and `--bw-user` flags, as it conflicts with the new GCP and Vault workflow documented in the current sections. Search the entire file for any mentions of Bitwarden-related configuration, flags, environment variables, or usage examples and delete them completely to ensure the README only documents the current GCP-based backend and doesn't advertise incompatible or deprecated functionality. In `@cmd/image-graph-generator/README.md`: - Around line 79-89: The usage documentation snippet in the README shows command-line flags that no longer match the actual implementation in main.go. Update the usage block to reflect the current flags accepted by the image-graph-generator tool, removing the obsolete -ci-operator-configs-path and -image-mirroring-path flags and replacing them with the actual flags currently implemented (--release-repo and --graphql-endpoint-address) to ensure the documentation accurately represents what users will encounter when running the command. In `@cmd/job-run-aggregator/README.md`: - Around line 109-117: Remove the final stale example from the README.md file that uses the create-tables subcommand (the block starting with the comment "This will create the Jobs table:"). This example duplicates the create-releases example shown immediately before it and references a subcommand that is not documented or supported, which will cause it to fail when executed. Delete both the comment line and the associated dlv exec command line that follows it. In `@cmd/ocp-build-data-enforcer/README.md`: - Around line 54-67: The Deployment section contains inaccurate details that do not match the actual implementation of ocp-build-data-enforcer as defined in main.go. Rewrite the deployment bullets to accurately describe the tool's config-driven workflow, removing the misleading references to iterating over fixed openshift-4.21/images trees and directly editing Dockerfiles. Ensure the rewritten bullets reflect the actual deployment flow and purpose of the enforcer tool as implemented in the codebase. In `@cmd/private-org-sync/README.md`: - Around line 121-200: The example commands in the README.md file use outdated CLI flags that do not match the current documentation. Update the first command example that currently uses --config-path and --git-dir flags to use --config-dir instead, making it consistent with the flags table documentation and the second example command which already uses the correct --config-dir flag. Ensure all example invocations reflect the current CLI interface. - Around line 76-97: The deployment notes section still describes outdated behavior regarding missing destination repositories and merge conflict handling. Update this section to reflect the current implementation by removing the statement that missing destinations are unconditionally ignored and instead reference the `--fail-on-missing-destination` flag mentioned in the error-handling section above. Similarly, update the merge conflict statement to align with the current error-handling behavior documented elsewhere in the README to ensure consistency and clarity for readers about the actual tool behavior. In `@cmd/prow-job-dispatcher/README.md`: - Around line 67-72: The README contains a stale description of the dispatcher algorithm that references the old "last seven days / least jobs wins" approach and conflicts with the updated dispatch logic described elsewhere in the document. Remove the entire stale block that begins with "It starts off by figuring out how many runs of each Prow jobs we had in the last seven days" through "based on the Prometheus metrics and the already dispatched jobs." This will eliminate the ambiguity caused by having two conflicting versions of the scheduler behavior in the same README. In `@cmd/registry-replacer/README.md`: - Around line 59-67: The Deployment section conflates the core registry-replacement behavior with the optional `--ensure-correct-promotion-dockerfile` behavior and PR creation flow. Restructure this section to clearly separate the mandatory registry-replacement logic (which finds ci-operator configs, downloads Dockerfiles, updates configs to replace registry references, and removes invalid/obsolete replacements) from the optional PR flow (which is gated by the `--ensure-correct-promotion-dockerfile` flag and handles pushing to forks and creating PRs via the GitHub API). Clarify that the tool rewrites ci-operator configs based on fetched Dockerfile content rather than directly editing the Dockerfile itself, ensuring the documentation aligns with the actual behavior in main.go. In `@cmd/repo-init/README.md`: - Around line 164-176: The shell command examples in the README.md file for running local-repo-init-ui.sh are missing the directory path prefix. Both the start and stop command examples currently show ./local-repo-init-ui.sh but should include the full relative path as ./hack/local-repo-init-ui.sh to match the prose description and allow the copy-paste examples to work correctly from the repository root. Update both command instances to include the hack/ directory prefix so users can successfully execute them without encountering a file not found error. In `@cmd/sippy-config-generator/README.md`: - Around line 65-97: Remove the legacy manual-run appendix section from the README that includes the Related, Example invocation, Commit the output, and Customization subsections. These sections duplicate information already covered in the newer spec-style sections of the documentation. Delete this entire block to improve the README's readability and reduce redundant guidance. In `@cmd/slack-bot/README.md`: - Around line 70-77: The command example in the Local testing section is missing the directory prefix for the script path. Update the bash command that runs the local-slack-bot.sh script to include the hack/ directory prefix before the script name so that readers following the instructions verbatim will correctly reference the script location at hack/local-slack-bot.sh. In `@cmd/testgrid-config-generator/README.md`: - Around line 75-132: The legacy appendix section starting with "Note: Go 1.13 is required." and continuing through the end of the diff repeats older manual-run guidance and conflicts with the new spec-style sections above it, creating mixed instructions for readers. Remove this entire legacy appendix section or completely rewrite it to eliminate the duplication and conflicts with the newer documentation structure. The section includes the Go 1.13 note, build instructions using the go build command, the manual run example with testgrid-config-generator flags, and verification steps that appear to duplicate guidance from earlier in the document. In `@cmd/vault-secret-collection-manager/README.md`: - Around line 98-139: Remove the outdated endpoint documentation in the Endpoints section of the README that describes PATCH /secretcollection/:name for changing secret collection members. This endpoint documentation is inconsistent with the authoritative API contract which uses PUT /secretcollection/:name/members instead. Delete the bullet point containing the PATCH endpoint description to eliminate the ambiguity and keep only the correct current endpoint in the documentation. --- Nitpick comments: In `@cmd/blocking-issue-creator/README.md`: - Around line 56-85: The README contains duplicate sections "What it does", "Why it exists", "How it works", and "How is it deployed" that repeat information already covered in the structured documentation above. Remove these four legacy section headers and their content from the README, unless the information they contain is unique and not already present in the structured sections above. If all the content is already covered, simply delete the entire legacy block to avoid redundancy and maintain a single canonical description. - Around line 18-19: The README contains conflicting GitHub API rate limit numbers: one section mentions 30 requests/minute as the secondary rate limit while another section (also noted in the deployment notes) references 300 requests/minute for burst/sustained rate limits. Verify the correct GitHub API rate limit that applies to this tool's use case, then update both occurrences (the rate limiting description around line 18-19 and the deployment notes around line 51-52) to use consistent and accurate numbers throughout the documentation. Ensure all rate limit references align with the actual GitHub API limits being respected by the tool. In `@cmd/check-gh-automation/README.md`: - Around line 66-103: The README examples for running check-gh-automation are using inconsistent GitHub authentication parameters compared to the flag table documented above. Review the flag table that defines the current authentication options, then update all command examples (including those in the "Pass Prow Config Options", "Determine Modified Repos from Candidate and JobSpec", and "Pass specific Repo(s) to check" sections) to consistently use the same GitHub authentication flags as specified in the flag table. Remove or update any legacy examples that document competing or outdated authentication modes to ensure the README presents a single, consistent invocation pattern. In `@cmd/ci-images-mirror/README.md`: - Around line 80-149: The README contains redundant legacy narrative that repeats the workflow explanation in a different voice and contradicts the updated information that states the tool is no longer temporary. Identify and remove the legacy block of text that appears after the deployment section and reiterates the two-tag mirroring explanation and "temporary" language. Ensure the README maintains a single, consistent voice by keeping only the current narrative sections that describe the tool functionality, extensions, and operational procedures without duplication. In `@cmd/cluster-init/README.md`: - Around line 98-110: Remove the legacy usage documentation block containing the "## Create", "## Update", and "## Create PR" sections as these duplicate the lifecycle documentation covered elsewhere in the README and reintroduce flags like --release-repo, --cluster-name, --update, --create-pr, --github-token-path, and --self-approve that should be documented in a single location. Either consolidate these examples into the flag reference tables if they provide unique value, or delete the entire block to establish a single source of truth for the tool's usage documentation. In `@cmd/github-ldap-user-group-creator/README.md`: - Around line 76-110: The README contains duplicate documentation with outdated terminology that contradicts the newly added summary section. Remove the legacy README block that appears after the "How is it deployed" section, which repeats the tool description using outdated terms such as "github-id-group" and "github-id". Keep only the new comprehensive summary section ("What it does", "Why it exists", "How it works", "Deleting users", and "How is it deployed") to maintain a single source of truth in the documentation. In `@cmd/promoted-image-governor/README.md`: - Around line 56-107: The README contains duplicated sections with the same content described in "What it does", "Why it exists", "How it works", and "How is it deployed". Remove the legacy/old version of these sections (keeping only the newer, better-structured version) to eliminate redundancy and maintain a single canonical description of the promoted-image-governor tool's purpose, functionality, and deployment.🪄 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 YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID:
a3c6be60-f3e1-4080-9631-b907f56023ce📒 Files selected for processing (78)
cmd/auto-config-brancher/README.mdcmd/auto-peribolos-sync/README.mdcmd/auto-testgrid-generator/README.mdcmd/autoowners/README.mdcmd/autopublicizeconfig/README.mdcmd/backport-verifier/README.mdcmd/blocking-issue-creator/README.mdcmd/branchingconfigmanagers/README.mdcmd/bugzilla-backporter/README.mdcmd/check-cluster-profiles-config/README.mdcmd/check-gh-automation/README.mdcmd/ci-images-mirror/README.mdcmd/ci-operator-checkconfig/README.mdcmd/ci-operator-config-mirror/README.mdcmd/ci-operator-configresolver/README.mdcmd/ci-operator-prowgen/README.mdcmd/ci-operator-yaml-creator/README.mdcmd/ci-operator/README.mdcmd/ci-scheduling-webhook/README.mdcmd/ci-secret-bootstrap/README.mdcmd/ci-secret-generator/README.mdcmd/cluster-display/README.mdcmd/cluster-init/README.mdcmd/clusterimageset-updater/README.mdcmd/config-brancher/README.mdcmd/config-change-trigger/README.mdcmd/config-shard-validator/README.mdcmd/cvp-trigger/README.mdcmd/determinize-ci-operator/README.mdcmd/determinize-peribolos/README.mdcmd/determinize-prow-config/README.mdcmd/docgen/README.mdcmd/dptp-controller-manager/README.mdcmd/dptp-pools-cm/README.mdcmd/entrypoint-wrapper/README.mdcmd/generate-registry-metadata/README.mdcmd/github-ldap-user-group-creator/README.mdcmd/gpu-scheduling-webhook/README.mdcmd/gsm-secret-sync/README.mdcmd/helpdesk-faq/README.mdcmd/image-graph-generator/README.mdcmd/job-run-aggregator/README.mdcmd/job-trigger-controller-manager/README.mdcmd/ldap-users-from-github-owners-files/README.mdcmd/lensserver/README.mdcmd/multi-arch-builder-controller/README.mdcmd/multi-pr-prow-plugin/README.mdcmd/ocp-build-data-enforcer/README.mdcmd/payload-testing-prow-plugin/README.mdcmd/payload-testing-ui/README.mdcmd/pipeline-controller/README.mdcmd/pj-rehearse/README.mdcmd/pod-scaler/README.mdcmd/pr-reminder/README.mdcmd/prcreator/README.mdcmd/private-org-peribolos-sync/README.mdcmd/private-org-sync/README.mdcmd/private-prow-configs-mirror/README.mdcmd/promoted-image-governor/README.mdcmd/prow-job-dispatcher/README.mdcmd/publicize/README.mdcmd/qci-appci/README.mdcmd/rebalancer/README.mdcmd/registry-replacer/README.mdcmd/release/README.mdcmd/repo-brancher/README.mdcmd/repo-init/README.mdcmd/result-aggregator/README.mdcmd/retester/README.mdcmd/sanitize-prow-jobs/README.mdcmd/serviceaccount-secret-rotation-trigger/README.mdcmd/sippy-config-generator/README.mdcmd/slack-bot/README.mdcmd/sprint-automation/README.mdcmd/sync-rover-groups/README.mdcmd/testgrid-config-generator/README.mdcmd/vault-secret-collection-manager/README.mdcmd/vault-subpath-proxy/README.md🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
openshift/release(manual)openshift/ci-docs(manual)openshift/release-controller(manual)openshift/ci-chat-bot(manual)
Comment on lines
+37
to
+48
| ### On `/validate-backports` command | ||
| Same flow as above, but if the repo has no upstream configured, posts a comment saying so and applies the `unvalidated` label. | ||
|
|
||
| ### Labels | ||
| Based on the validation result, the plugin ensures exactly one of two labels is present: | ||
|
|
||
| | Label | Meaning | | ||
| |---|---| | ||
| | `backports/validated-commits` | All commits reference merged upstream PRs and no errors occurred | | ||
| | `backports/unvalidated-commits` | At least one commit is invalid or encountered an error | | ||
|
|
||
| When one label is added, the other is removed. The validated label is only applied when there are zero invalid commits AND zero errors. |
There was a problem hiding this comment.
Use the full label name here.
unvalidated is ambiguous and does not match the fully qualified label names listed below. Spell out backports/unvalidated-commits so the workflow stays consistent.
🤖 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 `@cmd/backport-verifier/README.md` around lines 37 - 48, In the "On
`/validate-backports` command" section, replace the abbreviated label name
`unvalidated` with the fully qualified label name
`backports/unvalidated-commits` in the text describing which label gets applied
when the repo has no upstream configured. This ensures consistency with the
complete label names shown in the Labels table below.
| | `/bug` | Return bug details as JSON | | ||
| | `/help` | Help/debug endpoint | | ||
|
|
||
| 5. Exports Prometheus metrics under `bugzillabackporter` (request duration, response size). |
There was a problem hiding this comment.
Fix the metric prefix.
main.go exposes Prometheus metrics under ci-operator-bugzilla-backporter; bugzillabackporter is the wrong name here and will mislead anyone wiring dashboards or alerts from this README.
🤖 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 `@cmd/bugzilla-backporter/README.md` at line 20, The README.md documentation on
line 20 lists the Prometheus metric prefix as `bugzillabackporter`, but the
actual implementation in main.go exposes metrics under the prefix
`ci-operator-bugzilla-backporter`. Update the README.md file to replace
`bugzillabackporter` with the correct metric prefix
`ci-operator-bugzilla-backporter` to ensure the documentation accurately
reflects what is implemented and prevents confusion when setting up dashboards
or alerts.
f96265a to
e7d4266
Compare
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 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 `@cmd/branchingconfigmanagers/README.md`:
- Around line 214-219: The Deployment section overstates flag support by saying
all sub-managers accept --overwrite-time or an equivalent, but only
fast-forwarding-config-manager is documented with it. Update the wording in the
README deployment guidance to scope this flag to the specific managers that
actually implement it, and reference the per-manager flag documentation rather
than implying universal support.
In `@cmd/ci-operator-prowgen/README.md`:
- Around line 3-5: The README intro for ci-operator-prowgen overstates that it
produces every Prow job definition. Update the wording in the opening
description to reflect actual ownership: it generates Prow job YAML from
ci-operator configs and .config.prowgen files, while preserving hand-maintained
infra job files via --known-infra-file; adjust the summary near the current
“What” section to avoid claiming full coverage.
In `@cmd/ci-secret-generator/README.md`:
- Around line 57-58: The flag description for the GSM sync option uses the wrong
tool name and still refers to cluster-init, which is misleading. Update the
`--enable-gsm-sync` description in the ci-secret-generator README to refer to
the correct GSM sync behavior for this command, and keep the
`--gsm-credentials-file` description aligned with that tool’s terminology.
- Around line 83-92: The YAML example under the ci-secret-generator README has
an invalid shape because the second item's fields block is written as a mapping
instead of the same list structure used by item_name and the first example.
Update the example so the second item’s fields entry is also a list of field
objects, keeping the same schema as the other item examples, so the sample
config parses correctly.
In `@cmd/pj-rehearse/README.md`:
- Around line 70-85: The fenced diagram block in the README is missing a
language tag, which markdownlint flags as an unlabeled code block. Update that
fence to use text so the block stays lint-clean, and keep the surrounding
content in the same section unchanged.
In `@cmd/private-prow-configs-mirror/README.md`:
- Around line 18-31: The README table is attached directly to the “Config
sections mirrored” heading, which triggers markdownlint. Update the markdown in
the README section by adding a blank line between the heading and the table so
the rendered structure is valid and lint-clean. Use the existing heading text
and table content as-is; only adjust the spacing around that table.
In `@cmd/release/README.md`:
- Around line 32-54: The Markdown tables under the `release registry step`,
`release registry chain`, and `release registry workflow` sections need a blank
line before each table to satisfy markdownlint. Update the README content around
those subcommand headings so the tables are separated from the preceding
paragraph by an empty line, keeping the `release registry` documentation
lint-clean.
In `@cmd/vault-secret-collection-manager/README.md`:
- Around line 13-21: The collection creation flow in the README-described PUT
/secretcollection/:name path only treats the Vault group as the idempotency
boundary, so a retry cannot heal missing policy or placeholder state. Update the
collection-creation logic so the existing-state check in the create flow (around
the managed group/policy/placeholder steps) converges the full resource set: if
the group already exists, verify and recreate any missing policy, membership, or
placeholder entry instead of returning 409 immediately. Use the existing
secret-collection-manager-managed-<name> naming in the group/policy/placeholder
creation path to locate the relevant code.
🪄 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 YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: fcec3644-3639-45fc-a41b-05641b075fbe
📒 Files selected for processing (69)
cmd/auto-config-brancher/README.mdcmd/auto-peribolos-sync/README.mdcmd/auto-testgrid-generator/README.mdcmd/autoowners/README.mdcmd/autopublicizeconfig/README.mdcmd/blocking-issue-creator/README.mdcmd/branchingconfigmanagers/README.mdcmd/bugzilla-backporter/README.mdcmd/check-cluster-profiles-config/README.mdcmd/check-gh-automation/README.mdcmd/ci-images-mirror/README.mdcmd/ci-operator-checkconfig/README.mdcmd/ci-operator-config-mirror/README.mdcmd/ci-operator-configresolver/README.mdcmd/ci-operator-prowgen/README.mdcmd/ci-operator-yaml-creator/README.mdcmd/ci-operator/README.mdcmd/ci-secret-bootstrap/README.mdcmd/ci-secret-generator/README.mdcmd/cluster-display/README.mdcmd/cluster-init/README.mdcmd/clusterimageset-updater/README.mdcmd/config-brancher/README.mdcmd/config-change-trigger/README.mdcmd/config-shard-validator/README.mdcmd/cvp-trigger/README.mdcmd/determinize-ci-operator/README.mdcmd/determinize-prow-config/README.mdcmd/docgen/README.mdcmd/dptp-controller-manager/README.mdcmd/dptp-pools-cm/README.mdcmd/entrypoint-wrapper/README.mdcmd/generate-registry-metadata/README.mdcmd/github-ldap-user-group-creator/README.mdcmd/gsm-secret-sync/README.mdcmd/image-graph-generator/README.mdcmd/job-run-aggregator/README.mdcmd/job-trigger-controller-manager/README.mdcmd/lensserver/README.mdcmd/multi-arch-builder-controller/README.mdcmd/multi-pr-prow-plugin/README.mdcmd/ocp-build-data-enforcer/README.mdcmd/payload-testing-prow-plugin/README.mdcmd/payload-testing-ui/README.mdcmd/pj-rehearse/README.mdcmd/pod-scaler/README.mdcmd/prcreator/README.mdcmd/private-org-peribolos-sync/README.mdcmd/private-org-sync/README.mdcmd/private-prow-configs-mirror/README.mdcmd/promoted-image-governor/README.mdcmd/prow-job-dispatcher/README.mdcmd/qci-appci/README.mdcmd/rebalancer/README.mdcmd/registry-replacer/README.mdcmd/release/README.mdcmd/repo-brancher/README.mdcmd/repo-init/README.mdcmd/result-aggregator/README.mdcmd/sanitize-prow-jobs/README.mdcmd/serviceaccount-secret-rotation-trigger/README.mdcmd/sippy-config-generator/README.mdcmd/slack-bot/README.mdcmd/sprint-automation/README.mdcmd/sync-rover-groups/README.mdcmd/testgrid-config-generator/README.mdcmd/vault-secret-collection-manager/README.mdcmd/vault-subpath-proxy/README.mdimages/auto-config-brancher/Dockerfile
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
openshift/release(manual)openshift/ci-docs(manual)openshift/release-controller(manual)openshift/ci-chat-bot(manual)
💤 Files with no reviewable changes (1)
- images/auto-config-brancher/Dockerfile
✅ Files skipped from review due to trivial changes (43)
- cmd/cluster-display/README.md
- cmd/config-change-trigger/README.md
- cmd/check-cluster-profiles-config/README.md
- cmd/bugzilla-backporter/README.md
- cmd/auto-testgrid-generator/README.md
- cmd/payload-testing-prow-plugin/README.md
- cmd/result-aggregator/README.md
- cmd/determinize-prow-config/README.md
- cmd/determinize-ci-operator/README.md
- cmd/job-trigger-controller-manager/README.md
- cmd/repo-init/README.md
- cmd/ci-operator-config-mirror/README.md
- cmd/rebalancer/README.md
- cmd/docgen/README.md
- cmd/ci-operator-configresolver/README.md
- cmd/prow-job-dispatcher/README.md
- cmd/multi-arch-builder-controller/README.md
- cmd/autopublicizeconfig/README.md
- cmd/promoted-image-governor/README.md
- cmd/image-graph-generator/README.md
- cmd/autoowners/README.md
- cmd/cvp-trigger/README.md
- cmd/entrypoint-wrapper/README.md
- cmd/qci-appci/README.md
- cmd/gsm-secret-sync/README.md
- cmd/generate-registry-metadata/README.md
- cmd/private-org-sync/README.md
- cmd/slack-bot/README.md
- cmd/config-shard-validator/README.md
- cmd/serviceaccount-secret-rotation-trigger/README.md
- cmd/pod-scaler/README.md
- cmd/sync-rover-groups/README.md
- cmd/registry-replacer/README.md
- cmd/check-gh-automation/README.md
- cmd/sanitize-prow-jobs/README.md
- cmd/sippy-config-generator/README.md
- cmd/ci-images-mirror/README.md
- cmd/config-brancher/README.md
- cmd/ci-operator-checkconfig/README.md
- cmd/lensserver/README.md
- cmd/blocking-issue-creator/README.md
- cmd/payload-testing-ui/README.md
- cmd/auto-peribolos-sync/README.md
Comment on lines
+214
to
+219
| ## Deployment | ||
| All sub-managers run as periodic Prow jobs. The typical pattern is: | ||
| 1. Periodic job runs the config manager against a checkout of openshift/release | ||
| 2. If it exits 0 (config now matches policy), the `prcreator` tool commits changes and opens a PR | ||
|
|
||
| All accept `--overwrite-time` (or equivalent) for testing lifecycle transitions. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Tighten the --overwrite-time claim.
The deployment section says all sub-managers accept --overwrite-time (or an equivalent), but the per-manager tables above only document that flag for fast-forwarding-config-manager. Please scope this to the managers that actually support it.
🤖 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 `@cmd/branchingconfigmanagers/README.md` around lines 214 - 219, The Deployment
section overstates flag support by saying all sub-managers accept
--overwrite-time or an equivalent, but only fast-forwarding-config-manager is
documented with it. Update the wording in the README deployment guidance to
scope this flag to the specific managers that actually implement it, and
reference the per-manager flag documentation rather than implying universal
support.
Comment on lines
+3
to
+5
| ## What | ||
| Generates Prow job YAML definitions (presubmits, postsubmits, periodics) from ci-operator configs and `.config.prowgen` files. This is the `make jobs` engine in the openshift/release repo — every Prow job definition is produced by this tool. | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Avoid overclaiming full job coverage.
--known-infra-file explicitly preserves hand-maintained infra job files, so ci-operator-prowgen does not generate every Prow job definition. Rephrase the intro to match the tool’s actual ownership.
🤖 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 `@cmd/ci-operator-prowgen/README.md` around lines 3 - 5, The README intro for
ci-operator-prowgen overstates that it produces every Prow job definition.
Update the wording in the opening description to reflect actual ownership: it
generates Prow job YAML from ci-operator configs and .config.prowgen files,
while preserving hand-maintained infra job files via --known-infra-file; adjust
the summary near the current “What” section to avoid claiming full coverage.
Comment on lines
+57
to
+58
| | `--enable-gsm-sync` | `false` | Enable syncing cluster-init secrets to GSM | | ||
| | `--gsm-credentials-file` | `""` | Path to GCP service account credentials (required if GSM sync enabled and not dry-run) | |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use the correct tool name in this flag description.
cluster-init is unrelated here, so this reads like copy/paste from another README and makes the GSM sync flag misleading.
♻️ Proposed fix
-| `--enable-gsm-sync` | false | Enable syncing cluster-init secrets to GSM |
+| `--enable-gsm-sync` | false | Enable syncing generated secrets to GSM |📝 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.
Suggested change
| | `--enable-gsm-sync` | `false` | Enable syncing cluster-init secrets to GSM | | |
| | `--gsm-credentials-file` | `""` | Path to GCP service account credentials (required if GSM sync enabled and not dry-run) | | |
| | `--enable-gsm-sync` | `false` | Enable syncing generated secrets to GSM | | |
| | `--gsm-credentials-file` | `""` | Path to GCP service account credentials (required if GSM sync enabled and not dry-run) | |
🤖 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 `@cmd/ci-secret-generator/README.md` around lines 57 - 58, The flag description
for the GSM sync option uses the wrong tool name and still refers to
cluster-init, which is misleading. Update the `--enable-gsm-sync` description in
the ci-secret-generator README to refer to the correct GSM sync behavior for
this command, and keep the `--gsm-credentials-file` description aligned with
that tool’s terminology.
Comment on lines
83
to
92
| ```yaml | ||
| - item_name: first_item | ||
| fields: | ||
| - name: field1 | ||
| cmd: echo -n secret | ||
| attribute: | ||
| name: password | ||
| cmd: echo -n new_password | ||
| - item_name: second_item | ||
| fields: | ||
| name: field2 | ||
| cmd: echo -n field2_contents | ||
| ``` |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Fix the YAML example shape.
The second item's fields block is not a list, so the sample config will not parse as written.
🐛 Proposed fix
- item_name: second_item
fields:
- name: field2
- cmd: echo -n field2_contents
+ - name: field2
+ cmd: echo -n field2_contents📝 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.
Suggested change
| ```yaml | |
| - item_name: first_item | |
| fields: | |
| - name: field1 | |
| cmd: echo -n secret | |
| attribute: | |
| name: password | |
| cmd: echo -n new_password | |
| - item_name: second_item | |
| fields: | |
| name: field2 | |
| cmd: echo -n field2_contents | |
| ``` |
🤖 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 `@cmd/ci-secret-generator/README.md` around lines 83 - 92, The YAML example
under the ci-secret-generator README has an invalid shape because the second
item's fields block is written as a mapping instead of the same list structure
used by item_name and the first example. Update the example so the second item’s
fields entry is also a list of field objects, keeping the same schema as the
other item examples, so the sample config parses correctly.
Comment on lines
+70
to
+85
| ``` | ||
| PR opened | ||
| ├─ no affected jobs → auto-add rehearsals-ack | ||
| └─ affected jobs found → post list, wait for user | ||
| ├─ /pj-rehearse → run rehearsals (label not added) | ||
| ├─ /pj-rehearse auto-ack → run + add label on all-pass | ||
| ├─ /pj-rehearse skip → add label (opt out) | ||
| ├─ /pj-rehearse ack → add label (manual ack) | ||
| └─ no action → label absent, blocks merge | ||
|
|
||
| New push | ||
| ├─ abort active rehearsals | ||
| ├─ remove rehearsals-ack (unless sticky-label-authors) | ||
| ├─ remove network-access-rehearsals-ok | ||
| └─ recompute + repost affected list | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Label the fence with a language.
Markdownlint will flag this unlabeled code block. Mark it as text so the docs stay lint-clean.
Proposed fix
-```
+```text📝 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.
Suggested change
| ``` | |
| PR opened | |
| ├─ no affected jobs → auto-add rehearsals-ack | |
| └─ affected jobs found → post list, wait for user | |
| ├─ /pj-rehearse → run rehearsals (label not added) | |
| ├─ /pj-rehearse auto-ack → run + add label on all-pass | |
| ├─ /pj-rehearse skip → add label (opt out) | |
| ├─ /pj-rehearse ack → add label (manual ack) | |
| └─ no action → label absent, blocks merge | |
| New push | |
| ├─ abort active rehearsals | |
| ├─ remove rehearsals-ack (unless sticky-label-authors) | |
| ├─ remove network-access-rehearsals-ok | |
| └─ recompute + repost affected list | |
| ``` |
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 70-70: 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 `@cmd/pj-rehearse/README.md` around lines 70 - 85, The fenced diagram block in
the README is missing a language tag, which markdownlint flags as an unlabeled
code block. Update that fence to use text so the block stays lint-clean, and
keep the surrounding content in the same section unchanged.
Source: Linters/SAST tools
Comment on lines
+18
to
+31
| ### Config sections mirrored | ||
| | Section | Behavior | | ||
| |---|---| | ||
| | **Branch protection** | Delete existing `openshift-priv` org entry, then copy per-repo rules from source orgs into `openshift-priv` repos | | ||
| | **Tide context policy** | Copy per-repo context policies to `openshift-priv` repos | | ||
| | **Tide merge type** | Mirror org/repo and org/repo@branch merge method settings; drop stale `openshift-priv` repo-level entries | | ||
| | **Tide queries** | Add `openshift-priv/{repo}` to every Tide query that includes the public `{org}/{repo}`, remove stale priv entries | | ||
| | **PR status base URLs** | Copy per-repo PR status URLs to `openshift-priv` | | ||
| | **Plank decoration configs** | Copy per-repo decoration configs to `openshift-priv` | | ||
| | **Job URL prefix config** | Copy per-repo job URL prefixes to `openshift-priv` | | ||
| | **Approve plugin** | Add `openshift-priv/{repo}` to approve plugin repo lists | | ||
| | **LGTM plugin** | Add `openshift-priv/{repo}` to LGTM plugin repo lists | | ||
| | **Bugzilla plugin** | Copy per-repo Bugzilla options to `openshift-priv` | | ||
| | **Plugins** | Compute the union of org-level and repo-level plugins for each mirrored repo, extract common plugins to `openshift-priv` org level, keep repo-specific differences at repo level | |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Separate the table from the heading.
Markdownlint will flag this table unless there is a blank line before it. Insert one to keep the README lint-clean.
Proposed fix
### Config sections mirrored
+
| Section | Behavior |📝 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.
Suggested change
| ### Config sections mirrored | |
| | Section | Behavior | | |
| |---|---| | |
| | **Branch protection** | Delete existing `openshift-priv` org entry, then copy per-repo rules from source orgs into `openshift-priv` repos | | |
| | **Tide context policy** | Copy per-repo context policies to `openshift-priv` repos | | |
| | **Tide merge type** | Mirror org/repo and org/repo@branch merge method settings; drop stale `openshift-priv` repo-level entries | | |
| | **Tide queries** | Add `openshift-priv/{repo}` to every Tide query that includes the public `{org}/{repo}`, remove stale priv entries | | |
| | **PR status base URLs** | Copy per-repo PR status URLs to `openshift-priv` | | |
| | **Plank decoration configs** | Copy per-repo decoration configs to `openshift-priv` | | |
| | **Job URL prefix config** | Copy per-repo job URL prefixes to `openshift-priv` | | |
| | **Approve plugin** | Add `openshift-priv/{repo}` to approve plugin repo lists | | |
| | **LGTM plugin** | Add `openshift-priv/{repo}` to LGTM plugin repo lists | | |
| | **Bugzilla plugin** | Copy per-repo Bugzilla options to `openshift-priv` | | |
| | **Plugins** | Compute the union of org-level and repo-level plugins for each mirrored repo, extract common plugins to `openshift-priv` org level, keep repo-specific differences at repo level | | |
| ### Config sections mirrored | |
| | Section | Behavior | | |
| |---|---| | |
| | **Branch protection** | Delete existing `openshift-priv` org entry, then copy per-repo rules from source orgs into `openshift-priv` repos | | |
| | **Tide context policy** | Copy per-repo context policies to `openshift-priv` repos | | |
| | **Tide merge type** | Mirror org/repo and org/repo@branch merge method settings; drop stale `openshift-priv` repo-level entries | | |
| | **Tide queries** | Add `openshift-priv/{repo}` to every Tide query that includes the public `{org}/{repo}`, remove stale priv entries | | |
| | **PR status base URLs** | Copy per-repo PR status URLs to `openshift-priv` | | |
| | **Plank decoration configs** | Copy per-repo decoration configs to `openshift-priv` | | |
| | **Job URL prefix config** | Copy per-repo job URL prefixes to `openshift-priv` | | |
| | **Approve plugin** | Add `openshift-priv/{repo}` to approve plugin repo lists | | |
| | **LGTM plugin** | Add `openshift-priv/{repo}` to LGTM plugin repo lists | | |
| | **Bugzilla plugin** | Copy per-repo Bugzilla options to `openshift-priv` | | |
| | **Plugins** | Compute the union of org-level and repo-level plugins for each mirrored repo, extract common plugins to `openshift-priv` org level, keep repo-specific differences at repo level | |
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 19-19: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
🤖 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 `@cmd/private-prow-configs-mirror/README.md` around lines 18 - 31, The README
table is attached directly to the “Config sections mirrored” heading, which
triggers markdownlint. Update the markdown in the README section by adding a
blank line between the heading and the table so the rendered structure is valid
and lint-clean. Use the existing heading text and table content as-is; only
adjust the spacing around that table.
Source: Linters/SAST tools
Comment on lines
+32
to
+54
| #### `release registry step [names...]` | ||
| | Flag | What it does | | ||
| |---|---| | ||
| | `--list` / `-l` | List all step names | | ||
| | `--resolve` / `-r` | Print resolved step (for steps, same as unresolved) | | ||
| | `--tree` | Print step name in tree format | | ||
| | (default) | Print YAML of the step definition | | ||
|
|
||
| #### `release registry chain [names...]` | ||
| | Flag | What it does | | ||
| |---|---| | ||
| | `--list` / `-l` | List all chain names | | ||
| | `--resolve` / `-r` | Resolve chain (inline all referenced steps and sub-chains) | | ||
| | `--tree` | Display chain hierarchy with indentation | | ||
| | (default) | Print YAML of the chain definition | | ||
|
|
||
| #### `release registry workflow [names...]` | ||
| | Flag | What it does | | ||
| |---|---| | ||
| | `--list` / `-l` | List all workflow names | | ||
| | `--resolve` / `-r` | Resolve workflow (inline all pre/test/post steps and chains) | | ||
| | `--tree` | Display workflow hierarchy: pre/test/post sections with chains and steps indented | | ||
| | (default) | Print YAML of the workflow definition | |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add blank lines before the subcommand tables.
Markdownlint is flagging the tables under step, chain, and workflow. A blank line before each one will clear the warning and keep the README lint-clean.
Proposed fix
#### `release registry step [names...]`
+
| Flag | What it does |
@@
#### `release registry chain [names...]`
+
| Flag | What it does |
@@
#### `release registry workflow [names...]`
+
| Flag | What it does |📝 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.
Suggested change
| #### `release registry step [names...]` | |
| | Flag | What it does | | |
| |---|---| | |
| | `--list` / `-l` | List all step names | | |
| | `--resolve` / `-r` | Print resolved step (for steps, same as unresolved) | | |
| | `--tree` | Print step name in tree format | | |
| | (default) | Print YAML of the step definition | | |
| #### `release registry chain [names...]` | |
| | Flag | What it does | | |
| |---|---| | |
| | `--list` / `-l` | List all chain names | | |
| | `--resolve` / `-r` | Resolve chain (inline all referenced steps and sub-chains) | | |
| | `--tree` | Display chain hierarchy with indentation | | |
| | (default) | Print YAML of the chain definition | | |
| #### `release registry workflow [names...]` | |
| | Flag | What it does | | |
| |---|---| | |
| | `--list` / `-l` | List all workflow names | | |
| | `--resolve` / `-r` | Resolve workflow (inline all pre/test/post steps and chains) | | |
| | `--tree` | Display workflow hierarchy: pre/test/post sections with chains and steps indented | | |
| | (default) | Print YAML of the workflow definition | | |
| #### `release registry step [names...]` | |
| | Flag | What it does | | |
| |---|---| | |
| | `--list` / `-l` | List all step names | | |
| | `--resolve` / `-r` | Print resolved step (for steps, same as unresolved) | | |
| | `--tree` | Print step name in tree format | | |
| | (default) | Print YAML of the step definition | | |
| #### `release registry chain [names...]` | |
| | Flag | What it does | | |
| |---|---| | |
| | `--list` / `-l` | List all chain names | | |
| | `--resolve` / `-r` | Resolve chain (inline all referenced steps and sub-chains) | | |
| | `--tree` | Display chain hierarchy with indentation | | |
| | (default) | Print YAML of the chain definition | | |
| #### `release registry workflow [names...]` | |
| | Flag | What it does | | |
| |---|---| | |
| | `--list` / `-l` | List all workflow names | | |
| | `--resolve` / `-r` | Resolve workflow (inline all pre/test/post steps and chains) | | |
| | `--tree` | Display workflow hierarchy: pre/test/post sections with chains and steps indented | | |
| | (default) | Print YAML of the workflow definition | |
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 33-33: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
[warning] 41-41: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
[warning] 49-49: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
🤖 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 `@cmd/release/README.md` around lines 32 - 54, The Markdown tables under the
`release registry step`, `release registry chain`, and `release registry
workflow` sections need a blank line before each table to satisfy markdownlint.
Update the README content around those subcommand headings so the tables are
separated from the preceding paragraph by an empty line, keeping the `release
registry` documentation lint-clean.
Source: Linters/SAST tools
Comment on lines
+13
to
+21
| #### Creating a collection (`PUT /secretcollection/:name`) | ||
| 1. Validates the name matches `^[a-z0-9-]+$`. | ||
| 2. Checks for an existing Vault group with the prefixed name (`secret-collection-manager-managed-<name>`). If it exists, returns 409 Conflict (idempotency is on group creation, not policy). | ||
| 3. Looks up the requesting user by alias in Vault. If the user does not exist, creates a new Vault identity entity and alias for them. | ||
| 4. Creates a Vault policy granting: | ||
| - `list`, `delete` on `secret/metadata/self-managed/<name>/*` | ||
| - `create`, `update`, `read` on `secret/data/self-managed/<name>/*` | ||
| 5. Creates a Vault identity group named `secret-collection-manager-managed-<name>` with the requesting user as the sole member and the policy attached. | ||
| 6. Creates a placeholder KV entry at `secret/self-managed/<name>/placeholder` so the collection is visible in the Vault web console (bypasses the censoring plugin's minimum-length rules). |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Make collection creation converge on existing state.
Returning 409 as soon as the group exists leaves a collection half-built if policy or placeholder creation fails after the group is created. A retry then cannot repair the missing state, so the endpoint is not idempotent across the full resource set.
🤖 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 `@cmd/vault-secret-collection-manager/README.md` around lines 13 - 21, The
collection creation flow in the README-described PUT /secretcollection/:name
path only treats the Vault group as the idempotency boundary, so a retry cannot
heal missing policy or placeholder state. Update the collection-creation logic
so the existing-state check in the create flow (around the managed
group/policy/placeholder steps) converges the full resource set: if the group
already exists, verify and recreate any missing policy, membership, or
placeholder entry instead of returning 409 immediately. Use the existing
secret-collection-manager-managed-<name> naming in the group/policy/placeholder
creation path to locate the relevant code.
e7d4266 to
93ee8cd
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
cmd/private-prow-configs-mirror/README.md (1)
18-19: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winSeparate the table from the heading.
Markdownlint still flags this table because it follows the heading immediately. Add a blank line before it.
Proposed fix
### Config sections mirrored + | Section | Behavior |🤖 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 `@cmd/private-prow-configs-mirror/README.md` around lines 18 - 19, The Markdown table under the “Config sections mirrored” heading is placed immediately after the heading, which triggers markdownlint. Update the README so the table is separated from the heading by a blank line; keep the surrounding section text unchanged and adjust only the formatting around that table.Source: Linters/SAST tools
cmd/release/README.md (1)
32-54: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winSeparate the subcommand tables from their headings.
Markdownlint still flags the
step,chain, andworkflowtables because each one follows its heading immediately. Add a blank line before each table.Proposed fix
#### `release registry step [names...]` + | Flag | What it does | @@ #### `release registry chain [names...]` + | Flag | What it does | @@ #### `release registry workflow [names...]` + | Flag | What it does |🤖 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 `@cmd/release/README.md` around lines 32 - 54, The markdown tables under the release registry subcommand headings are too tightly attached to their headings, so markdownlint still flags them. In the README section for release registry step, chain, and workflow, add a blank line after each heading and before the corresponding table so the tables are separated from the headings and render cleanly.Source: Linters/SAST tools
cmd/pj-rehearse/README.md (1)
70-85: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winLabel the fenced block.
markdownlint still flags this diagram block because the fence has no language tag. Use
texthere.Proposed fix
-``` +```text🤖 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 `@cmd/pj-rehearse/README.md` around lines 70 - 85, The fenced diagram block in the README lacks a language tag and is still triggering markdownlint. Update the code fence for the PR/rehearsal flow diagram to use the text language identifier so it is treated as plain text; locate the affected fenced block in the README and change the opening fence accordingly.Source: Linters/SAST tools
🤖 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 `@cmd/branchingconfigmanagers/README.md`:
- Around line 58-69: The markdown tables in the README are still directly
attached to their section headings, which triggers MD058; update each affected
section so there is a blank line between the heading and the table, and a blank
line after the table before the next paragraph or section. Apply this to all
listed table sections in the branching config manager docs, not just the Flags
section.
In `@cmd/github-ldap-user-group-creator/README.md`:
- Around line 3-78: The README wording incorrectly says this job runs as a
Kubernetes CronJob, but `periodic-github-ldap-user-group-creator` is a Prow
periodic job. Update the Deployment section and any other references in this
document to describe it as a periodic Prow job running on app.ci, keeping the
rest of the deployment and pipeline description intact.
---
Duplicate comments:
In `@cmd/pj-rehearse/README.md`:
- Around line 70-85: The fenced diagram block in the README lacks a language tag
and is still triggering markdownlint. Update the code fence for the PR/rehearsal
flow diagram to use the text language identifier so it is treated as plain text;
locate the affected fenced block in the README and change the opening fence
accordingly.
In `@cmd/private-prow-configs-mirror/README.md`:
- Around line 18-19: The Markdown table under the “Config sections mirrored”
heading is placed immediately after the heading, which triggers markdownlint.
Update the README so the table is separated from the heading by a blank line;
keep the surrounding section text unchanged and adjust only the formatting
around that table.
In `@cmd/release/README.md`:
- Around line 32-54: The markdown tables under the release registry subcommand
headings are too tightly attached to their headings, so markdownlint still flags
them. In the README section for release registry step, chain, and workflow, add
a blank line after each heading and before the corresponding table so the tables
are separated from the headings and render cleanly.
🪄 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 YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: da597c51-fe15-46f2-8a0a-13e7c3a6a01b
📒 Files selected for processing (69)
cmd/auto-config-brancher/README.mdcmd/auto-peribolos-sync/README.mdcmd/auto-testgrid-generator/README.mdcmd/autoowners/README.mdcmd/autopublicizeconfig/README.mdcmd/blocking-issue-creator/README.mdcmd/branchingconfigmanagers/README.mdcmd/bugzilla-backporter/README.mdcmd/check-cluster-profiles-config/README.mdcmd/check-gh-automation/README.mdcmd/ci-images-mirror/README.mdcmd/ci-operator-checkconfig/README.mdcmd/ci-operator-config-mirror/README.mdcmd/ci-operator-configresolver/README.mdcmd/ci-operator-prowgen/README.mdcmd/ci-operator-yaml-creator/README.mdcmd/ci-operator/README.mdcmd/ci-secret-bootstrap/README.mdcmd/ci-secret-generator/README.mdcmd/cluster-display/README.mdcmd/cluster-init/README.mdcmd/clusterimageset-updater/README.mdcmd/config-brancher/README.mdcmd/config-change-trigger/README.mdcmd/config-shard-validator/README.mdcmd/cvp-trigger/README.mdcmd/determinize-ci-operator/README.mdcmd/determinize-prow-config/README.mdcmd/docgen/README.mdcmd/dptp-controller-manager/README.mdcmd/dptp-pools-cm/README.mdcmd/entrypoint-wrapper/README.mdcmd/generate-registry-metadata/README.mdcmd/github-ldap-user-group-creator/README.mdcmd/gsm-secret-sync/README.mdcmd/image-graph-generator/README.mdcmd/job-run-aggregator/README.mdcmd/job-trigger-controller-manager/README.mdcmd/lensserver/README.mdcmd/multi-arch-builder-controller/README.mdcmd/multi-pr-prow-plugin/README.mdcmd/ocp-build-data-enforcer/README.mdcmd/payload-testing-prow-plugin/README.mdcmd/payload-testing-ui/README.mdcmd/pj-rehearse/README.mdcmd/pod-scaler/README.mdcmd/prcreator/README.mdcmd/private-org-peribolos-sync/README.mdcmd/private-org-sync/README.mdcmd/private-prow-configs-mirror/README.mdcmd/promoted-image-governor/README.mdcmd/prow-job-dispatcher/README.mdcmd/qci-appci/README.mdcmd/rebalancer/README.mdcmd/registry-replacer/README.mdcmd/release/README.mdcmd/repo-brancher/README.mdcmd/repo-init/README.mdcmd/result-aggregator/README.mdcmd/sanitize-prow-jobs/README.mdcmd/serviceaccount-secret-rotation-trigger/README.mdcmd/sippy-config-generator/README.mdcmd/slack-bot/README.mdcmd/sprint-automation/README.mdcmd/sync-rover-groups/README.mdcmd/testgrid-config-generator/README.mdcmd/vault-secret-collection-manager/README.mdcmd/vault-subpath-proxy/README.mdimages/auto-config-brancher/Dockerfile
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
openshift/release(manual)openshift/ci-docs(manual)openshift/release-controller(manual)openshift/ci-chat-bot(manual)
💤 Files with no reviewable changes (1)
- images/auto-config-brancher/Dockerfile
✅ Files skipped from review due to trivial changes (42)
- cmd/bugzilla-backporter/README.md
- cmd/check-cluster-profiles-config/README.md
- cmd/ci-operator-config-mirror/README.md
- cmd/serviceaccount-secret-rotation-trigger/README.md
- cmd/generate-registry-metadata/README.md
- cmd/job-trigger-controller-manager/README.md
- cmd/config-change-trigger/README.md
- cmd/cluster-display/README.md
- cmd/autopublicizeconfig/README.md
- cmd/result-aggregator/README.md
- cmd/rebalancer/README.md
- cmd/payload-testing-prow-plugin/README.md
- cmd/entrypoint-wrapper/README.md
- cmd/determinize-prow-config/README.md
- cmd/ci-operator-configresolver/README.md
- cmd/auto-testgrid-generator/README.md
- cmd/docgen/README.md
- cmd/config-shard-validator/README.md
- cmd/determinize-ci-operator/README.md
- cmd/promoted-image-governor/README.md
- cmd/auto-peribolos-sync/README.md
- cmd/autoowners/README.md
- cmd/sync-rover-groups/README.md
- cmd/repo-init/README.md
- cmd/pod-scaler/README.md
- cmd/gsm-secret-sync/README.md
- cmd/multi-arch-builder-controller/README.md
- cmd/qci-appci/README.md
- cmd/image-graph-generator/README.md
- cmd/cvp-trigger/README.md
- cmd/ci-images-mirror/README.md
- cmd/sanitize-prow-jobs/README.md
- cmd/check-gh-automation/README.md
- cmd/payload-testing-ui/README.md
- cmd/slack-bot/README.md
- cmd/prow-job-dispatcher/README.md
- cmd/lensserver/README.md
- cmd/registry-replacer/README.md
- cmd/blocking-issue-creator/README.md
- cmd/private-org-sync/README.md
- cmd/ci-operator-checkconfig/README.md
- cmd/sippy-config-generator/README.md
🚧 Files skipped from review as they are similar to previous changes (1)
- cmd/config-brancher/README.md
Comment on lines
+58
to
+69
| #### Flags | ||
| | Flag | Default | What it controls | | ||
| |---|---|---| | ||
| | `--prow-config-dir` | (required) | Path to Prow configuration directory | | ||
| | `--sharded-prow-config-base-dir` | (required) | Base dir for sharded prow config output | | ||
| | `--lifecycle-phase` | (required) | One of: `branching`, `pre-general-availability`, `general-availability`, `acknowledge-critical-fixes-only`, `revert-critical-fixes-only`, `verified` | | ||
| | `--current-release` | (required for branching/pre-GA/GA) | Current OCP version, e.g. `4.17` | | ||
| | `--excluded-repos-config` | (empty) | Path to GA excluded repos config (repos allowed to skip future-branch exclusion checks) | | ||
| | `--repos-guarded-by-ack-critical-fixes` | (required for ack-critical-fixes) | Path to newline-separated list of repos | | ||
| | `--verified-opt-in` | (required for verified) | YAML file of repos to opt into verified label (`org: [repo1, repo2]`) | | ||
| | `--verified-opt-out` | (required for verified) | YAML file of repos to opt out of verified label | | ||
| | `--ci-operator-config-dir` | (required for verified) | Path to ci-operator config dir for auto-discovering OCP-promoting repos | |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add blank lines around the section tables.
These tables are still attached directly to their headings, so markdownlint MD058 will keep firing until each table is separated by blank lines.
Also applies to: 91-97, 115-125, 140-149, 167-175, 202-208
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 59-59: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
🤖 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 `@cmd/branchingconfigmanagers/README.md` around lines 58 - 69, The markdown
tables in the README are still directly attached to their section headings,
which triggers MD058; update each affected section so there is a blank line
between the heading and the table, and a blank line after the table before the
next paragraph or section. Apply this to all listed table sections in the
branching config manager docs, not just the Flags section.
Source: Linters/SAST tools
Comment on lines
+3
to
+78
| ## What | ||
| Batch job that synchronizes OpenShift `Group` resources across all CI clusters to match the canonical set of Rover/LDAP group memberships and GitHub-to-Kerberos identity mappings. It creates three categories of groups: | ||
|
|
||
| `github-ldap-user-group-creator` is a tool to maintain the groups on CI clusters. | ||
| 1. **Per-GitHub-user groups** (`<login>-group`) -- one member, the user's Kerberos ID, on all clusters except `hive` | ||
| 2. **Rover groups** (e.g. `test-platform-ci-admins`, team groups) -- resolved memberships from LDAP, with optional cluster targeting and renaming | ||
| 3. **openshift-priv admins group** (`openshift-priv-admins`) -- admins/members of the openshift-priv GitHub org, mapped to Kerberos IDs, on `app.ci` only | ||
|
|
||
| It also optionally deletes OpenShift `User` and `Identity` objects for people no longer present in Rover, and uploads user records to BigQuery for analytics. | ||
|
|
||
| ## Why it exists | ||
| This is the second half of a two-stage pipeline: `sync-rover-groups` resolves LDAP memberships and writes YAML files, then this tool consumes those files and applies the resulting groups to clusters. | ||
|
|
||
| The groups are used | ||
| - by `ci-operator` which promotes the group to the admins of the namespace created for the test. | ||
| - in [the manifests](https://github.com/openshift/release/tree/main/clusters) of CI clusters | ||
| in the release repo. | ||
| ## How it works -- full flow | ||
|
|
||
| ## How it works | ||
| 1. **Load inputs**: | ||
| - Read the GitHub-to-Kerberos user mapping from `--github-users-file` (YAML array of `rover.User` with `uid`, `github_username`, `cost_center`) | ||
| - Read the resolved Rover group memberships from `--groups-file` (YAML map of group name to Kerberos ID list) | ||
| - Optionally load peribolos config to extract openshift-priv org admins/members | ||
| - Optionally load group config for cluster targeting and renaming | ||
|
|
||
| `github-ldap-user-group-creator` reads | ||
| 2. **BigQuery upload**: Insert all user records into the `ci_analysis_us.users` table in the `openshift-gce-devel` GCP project with the current timestamp. This enables analytics on CI user populations over time. | ||
|
|
||
| 3. **Build GitHub-to-Kerberos mapping**: Call `rover.MapGithubToKerberos()` to create a `map[string]string` from GitHub login to Kerberos ID. | ||
|
|
||
| 4. **Safety checks**: | ||
| - Verify the `test-platform-ci-admins` group exists in the groups file and has at least 3 members; fatal if not | ||
| - Warn if no openshift-priv admins were found | ||
|
|
||
| 5. **Optionally delete invalid users** (`--delete-invalid-users`): For each cluster, list all OpenShift `User` objects. Delete any whose name is not a known Kerberos ID, along with their associated `Identity` objects. Hard-coded exceptions: `backplane-cluster-admin` is always skipped, ci-admins members are never deleted (safety valve). | ||
|
|
||
| 6. **Build group map** (`makeGroups`): | ||
| - Per-GitHub-user groups on all clusters except `hive` | ||
| - openshift-priv admins group on `app.ci` only (requires `--peribolos-config`). Logs errors for admins with no GitHub-to-Kerberos mapping unless they are in `--skip-ocp-priv-admin`. | ||
| - Rover groups on their configured clusters (all clusters by default, overridable via config). Groups can be renamed via `rename_to` in the config, with the original name stored in a `rover-group-name` label. | ||
|
|
||
| 7. **Ensure groups on clusters** (`ensureGroups`): | ||
| - First pass: list all groups on each cluster labeled `dptp.openshift.io/requester: github-ldap-user-group-creator`. Delete any that are no longer in the desired set or no longer targeted to that cluster. Never deletes `test-platform-ci-admins`. | ||
| - Second pass: concurrently (up to `--concurrency` goroutines via semaphore), upsert each group on its target clusters. Validates group names are non-empty and members have no duplicates or blanks before upserting. | ||
| - Upsert logic: attempt `Create`; if `AlreadyExists` and members differ, `Delete` then `Create` (OpenShift Group objects cannot be updated for the `Users` field). If members match, no-op. | ||
| - Retries with exponential backoff (4 steps, factor 2, starting at 1 second). | ||
| - Skips clusters disabled by Prow config. | ||
|
|
||
| ## Flags | ||
|
|
||
| | Flag | Default | What it controls | | ||
| |---|---|---| | ||
| | `--github-users-file` | (required) | YAML file with GitHub-to-Kerberos user mappings (produced by `sync-rover-groups`) | | ||
| | `--groups-file` | (required) | YAML file with resolved Rover group memberships (produced by `sync-rover-groups`) | | ||
| | `--gcp-credentials-file` | (required) | GCP service account JSON for BigQuery writes | | ||
| | `--config-file` | `""` | Group config YAML for per-group cluster targeting and renaming | | ||
| | `--peribolos-config` | `""` | Peribolos org config file; used to extract openshift-priv admins | | ||
| | `--org-from-peribolos-config` | `openshift-priv` | Org to read admins/members from in the peribolos config | | ||
| | `--skip-ocp-priv-admin` | (empty) | GitHub logins to exclude from the openshift-priv-admins group (repeatable) | | ||
| | `--dry-run` | `true` | When true, log intended changes without modifying clusters | | ||
| | `--delete-invalid-users` | `false` | Delete OpenShift User/Identity objects for users not in Rover | | ||
| | `--concurrency` | `60` | Max concurrent goroutines for group upsert operations | | ||
| | `--log-level` | `info` | Log verbosity | | ||
| | Kubernetes flags | (in-cluster) | `--kubeconfig`, `--context`, etc. via Prow's `KubernetesOptions` | | ||
|
|
||
| ## Key files | ||
|
|
||
| - `cmd/github-ldap-user-group-creator/main.go` -- all logic: option parsing, group construction (`makeGroups`), cluster synchronization (`ensureGroups`, `upsertGroup`), BigQuery upload, user deletion | ||
| - `pkg/group/config.go` -- `Config` type with `ClusterGroups` and per-group `Target` (cluster targeting via `ResolveClusters`, `RenameTo`) | ||
| - `pkg/rover/types.go` -- `User` type (`UID`, `GitHubUsername`, `CostCenter`), `MapGithubToKerberos()` helper | ||
| - `pkg/rover/bigquery.go` -- `UserItem` type with `Save()` for BigQuery insertion | ||
|
|
||
| ## Deployment | ||
| Runs as a periodic Prow job (CronJob) on app.ci. Consumes output files from `sync-rover-groups` which runs earlier in the pipeline. Requires kubeconfigs for all CI build clusters and GCP credentials for BigQuery. | ||
|
|
||
| All groups it creates are labeled `dptp.openshift.io/requester: github-ldap-user-group-creator` for ownership tracking and cleanup. | ||
|
|
||
| ## Related | ||
| - `cmd/sync-rover-groups` -- upstream: produces the `--groups-file` and `--github-users-file` this tool consumes | ||
| - `pkg/api` -- constants: `CIAdminsGroupName`, `DPTPRequesterLabel`, `GitHubUserGroup()`, cluster names | ||
|
|
||
| --- | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Drop the CronJob wording.
periodic-github-ldap-user-group-creator is a Prow periodic job, not a Kubernetes CronJob; the current wording misstates the deployment object and can confuse operators.
Suggested fix
-Runs as a periodic Prow job (CronJob) on app.ci.
+Runs as a periodic Prow job on app.ci.📝 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.
Suggested change
| ## What | |
| Batch job that synchronizes OpenShift `Group` resources across all CI clusters to match the canonical set of Rover/LDAP group memberships and GitHub-to-Kerberos identity mappings. It creates three categories of groups: | |
| `github-ldap-user-group-creator` is a tool to maintain the groups on CI clusters. | |
| 1. **Per-GitHub-user groups** (`<login>-group`) -- one member, the user's Kerberos ID, on all clusters except `hive` | |
| 2. **Rover groups** (e.g. `test-platform-ci-admins`, team groups) -- resolved memberships from LDAP, with optional cluster targeting and renaming | |
| 3. **openshift-priv admins group** (`openshift-priv-admins`) -- admins/members of the openshift-priv GitHub org, mapped to Kerberos IDs, on `app.ci` only | |
| It also optionally deletes OpenShift `User` and `Identity` objects for people no longer present in Rover, and uploads user records to BigQuery for analytics. | |
| ## Why it exists | |
| This is the second half of a two-stage pipeline: `sync-rover-groups` resolves LDAP memberships and writes YAML files, then this tool consumes those files and applies the resulting groups to clusters. | |
| The groups are used | |
| - by `ci-operator` which promotes the group to the admins of the namespace created for the test. | |
| - in [the manifests](https://github.com/openshift/release/tree/main/clusters) of CI clusters | |
| in the release repo. | |
| ## How it works -- full flow | |
| ## How it works | |
| 1. **Load inputs**: | |
| - Read the GitHub-to-Kerberos user mapping from `--github-users-file` (YAML array of `rover.User` with `uid`, `github_username`, `cost_center`) | |
| - Read the resolved Rover group memberships from `--groups-file` (YAML map of group name to Kerberos ID list) | |
| - Optionally load peribolos config to extract openshift-priv org admins/members | |
| - Optionally load group config for cluster targeting and renaming | |
| `github-ldap-user-group-creator` reads | |
| 2. **BigQuery upload**: Insert all user records into the `ci_analysis_us.users` table in the `openshift-gce-devel` GCP project with the current timestamp. This enables analytics on CI user populations over time. | |
| 3. **Build GitHub-to-Kerberos mapping**: Call `rover.MapGithubToKerberos()` to create a `map[string]string` from GitHub login to Kerberos ID. | |
| 4. **Safety checks**: | |
| - Verify the `test-platform-ci-admins` group exists in the groups file and has at least 3 members; fatal if not | |
| - Warn if no openshift-priv admins were found | |
| 5. **Optionally delete invalid users** (`--delete-invalid-users`): For each cluster, list all OpenShift `User` objects. Delete any whose name is not a known Kerberos ID, along with their associated `Identity` objects. Hard-coded exceptions: `backplane-cluster-admin` is always skipped, ci-admins members are never deleted (safety valve). | |
| 6. **Build group map** (`makeGroups`): | |
| - Per-GitHub-user groups on all clusters except `hive` | |
| - openshift-priv admins group on `app.ci` only (requires `--peribolos-config`). Logs errors for admins with no GitHub-to-Kerberos mapping unless they are in `--skip-ocp-priv-admin`. | |
| - Rover groups on their configured clusters (all clusters by default, overridable via config). Groups can be renamed via `rename_to` in the config, with the original name stored in a `rover-group-name` label. | |
| 7. **Ensure groups on clusters** (`ensureGroups`): | |
| - First pass: list all groups on each cluster labeled `dptp.openshift.io/requester: github-ldap-user-group-creator`. Delete any that are no longer in the desired set or no longer targeted to that cluster. Never deletes `test-platform-ci-admins`. | |
| - Second pass: concurrently (up to `--concurrency` goroutines via semaphore), upsert each group on its target clusters. Validates group names are non-empty and members have no duplicates or blanks before upserting. | |
| - Upsert logic: attempt `Create`; if `AlreadyExists` and members differ, `Delete` then `Create` (OpenShift Group objects cannot be updated for the `Users` field). If members match, no-op. | |
| - Retries with exponential backoff (4 steps, factor 2, starting at 1 second). | |
| - Skips clusters disabled by Prow config. | |
| ## Flags | |
| | Flag | Default | What it controls | | |
| |---|---|---| | |
| | `--github-users-file` | (required) | YAML file with GitHub-to-Kerberos user mappings (produced by `sync-rover-groups`) | | |
| | `--groups-file` | (required) | YAML file with resolved Rover group memberships (produced by `sync-rover-groups`) | | |
| | `--gcp-credentials-file` | (required) | GCP service account JSON for BigQuery writes | | |
| | `--config-file` | `""` | Group config YAML for per-group cluster targeting and renaming | | |
| | `--peribolos-config` | `""` | Peribolos org config file; used to extract openshift-priv admins | | |
| | `--org-from-peribolos-config` | `openshift-priv` | Org to read admins/members from in the peribolos config | | |
| | `--skip-ocp-priv-admin` | (empty) | GitHub logins to exclude from the openshift-priv-admins group (repeatable) | | |
| | `--dry-run` | `true` | When true, log intended changes without modifying clusters | | |
| | `--delete-invalid-users` | `false` | Delete OpenShift User/Identity objects for users not in Rover | | |
| | `--concurrency` | `60` | Max concurrent goroutines for group upsert operations | | |
| | `--log-level` | `info` | Log verbosity | | |
| | Kubernetes flags | (in-cluster) | `--kubeconfig`, `--context`, etc. via Prow's `KubernetesOptions` | | |
| ## Key files | |
| - `cmd/github-ldap-user-group-creator/main.go` -- all logic: option parsing, group construction (`makeGroups`), cluster synchronization (`ensureGroups`, `upsertGroup`), BigQuery upload, user deletion | |
| - `pkg/group/config.go` -- `Config` type with `ClusterGroups` and per-group `Target` (cluster targeting via `ResolveClusters`, `RenameTo`) | |
| - `pkg/rover/types.go` -- `User` type (`UID`, `GitHubUsername`, `CostCenter`), `MapGithubToKerberos()` helper | |
| - `pkg/rover/bigquery.go` -- `UserItem` type with `Save()` for BigQuery insertion | |
| ## Deployment | |
| Runs as a periodic Prow job (CronJob) on app.ci. Consumes output files from `sync-rover-groups` which runs earlier in the pipeline. Requires kubeconfigs for all CI build clusters and GCP credentials for BigQuery. | |
| All groups it creates are labeled `dptp.openshift.io/requester: github-ldap-user-group-creator` for ownership tracking and cleanup. | |
| ## Related | |
| - `cmd/sync-rover-groups` -- upstream: produces the `--groups-file` and `--github-users-file` this tool consumes | |
| - `pkg/api` -- constants: `CIAdminsGroupName`, `DPTPRequesterLabel`, `GitHubUserGroup()`, cluster names | |
| --- | |
| ## Deployment | |
| Runs as a periodic Prow job on app.ci. Consumes output files from `sync-rover-groups` which runs earlier in the pipeline. Requires kubeconfigs for all CI build clusters and GCP credentials for BigQuery. |
🧰 Tools
🪛 LanguageTool
[uncategorized] ~17-~17: The official name of this software platform is spelled with a capital “H”.
Context: ...(YAML array of rover.User with uid, github_username, cost_center) - Read the...
(GITHUB)
🤖 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 `@cmd/github-ldap-user-group-creator/README.md` around lines 3 - 78, The README
wording incorrectly says this job runs as a Kubernetes CronJob, but
`periodic-github-ldap-user-group-creator` is a Prow periodic job. Update the
Deployment section and any other references in this document to describe it as a
periodic Prow job running on app.ci, keeping the rest of the deployment and
pipeline description intact.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Prucek
force-pushed
the
rename-agents-to-readme
branch
from
93ee8cd to
569942a
Compare
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
1 participant
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.
Summary
cmd/directories with standardized documentation (what, how it works, flags, key files, deployment)openshift/releasemanifests and corrected where inaccurate🤖 Generated with Claude Code
This PR standardizes and expands documentation across many OpenShift CI tool directories under
cmd/by updating/addingREADME.mdfiles. In practical terms, it helps CI operators and contributors quickly understand what each tool does end-to-end, which CLI flags/flags defaults matter, which key source files drive behavior, and how each tool is deployed or invoked in CI.For numerous tools, the READMEs were rewritten into consistent sections (purpose/background, full workflow, flags, key files, and deployment/runtime expectations), including clarifying PR-creation/update rules, commit/reconcile behavior, and operational constraints. Several tools’ “deployment” descriptions were also corrected after verification against
openshift/releasemanifests to align documentation with actual Prow job/container behavior.Additionally, there is one build image change:
images/auto-config-brancher/Dockerfilewas updated to remove therebalancerbinary copy step.