CNTRLPLANE-3718: Add AI SDLC context files - #362
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@oceanc80: This pull request references CNTRLPLANE-3718 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: openshift/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
WalkthroughAdds repository documentation for AI guidance, service-ca-operator architecture, contribution rules, and README structure and links. ChangesRepository documentation refresh
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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 `@ARCHITECTURE.md`:
- Around line 14-65: The fenced diagram blocks in ARCHITECTURE.md are missing
language labels, triggering markdownlint warnings. Update each fenced block in
the architecture diagram section to use a text label (for example, on the
opening fence) so the markdown stays lint-clean; this applies to both diagram
fences referenced in the document.
- Around line 104-115: The rotation process description overstates the scope of
the CA bundle ConfigMap updates. In the ARCHITECTURE.md rotation steps, revise
the text around manageSignerCABundle to describe updating the single shared
signing-cabundle ConfigMap rather than one per namespace, while keeping the
per-namespace behavior attributed to the injected resources and controller
re-injection flow.
- Around line 147-149: The Service Annotations description misattributes Secret
creation to the operator instead of the controller. Update the wording in the
Service Annotations section so the
`service.beta.openshift.io/serving-cert-secret-name` entry says the controller
creates the Secret, while keeping the
`service.beta.openshift.io/serving-cert-signed-by` explanation aligned with the
controller’s responsibility.
In `@CONTRIBUTING.md`:
- Line 31: The CONTRIBUTING guidance uses the गैरstandard spelling “MacOS” in
user-facing text and should be made consistent with the canonical “macOS”.
Update the wording in the affected guidance so it uses “macOS” everywhere it
appears, including the related occurrence noted by the review, and keep the rest
of the sentence unchanged.
- Around line 175-179: Rename the “Building the test binary” section so it
matches the actual `make build` target, which builds the full binaries rather
than just a test binary. Update the heading in the CONTRIBUTING section to
something like “Building the binaries” and keep the existing `make build`
example under it, using the section title as the unique locator for the change.
- Around line 141-143: The resource list in CONTRIBUTING.md is malformed because
the links are collapsed together with stray control characters, so fix the
Markdown list formatting. Update the section containing the OpenShift PR
resources so each URL is its own bullet item, keeping the surrounding text
intact and ensuring the list renders cleanly in Markdown.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: e3746bfa-2924-46cf-9adf-02e6238d175b
📒 Files selected for processing (3)
AGENTS.mdARCHITECTURE.mdCONTRIBUTING.md
| ``` | ||
| ┌─────────────────────────────────────────────────────────────┐ | ||
| │ Operator Process (openshift-service-ca-operator namespace) │ | ||
| │ │ | ||
| │ ┌────────────────────────────────────────────────────┐ │ | ||
| │ │ pkg/operator/ │ │ | ||
| │ │ │ │ | ||
| │ │ • Manages controller Deployment lifecycle │ │ | ||
| │ │ • Creates/rotates signing CA keypair (Secret) │ │ | ||
| │ │ • Maintains CA bundle ConfigMap │ │ | ||
| │ │ • Reports ClusterOperator status │ │ | ||
| │ │ • Detects feature gates → forwards to controller │ │ | ||
| │ └────────────────────────────────────────────────────┘ │ | ||
| └─────────────────────────────────────────────────────────────┘ | ||
| │ | ||
| │ Deploys & manages | ||
| ↓ | ||
| ┌─────────────────────────────────────────────────────────────┐ | ||
| │ Controller Process (openshift-service-ca namespace) │ | ||
| │ │ | ||
| │ ┌─────────────────────────────────────────────────────┐ │ | ||
| │ │ pkg/controller/servingcert/ │ │ | ||
| │ │ Serving Cert Signer │ │ | ||
| │ │ • Watches Services with serving-cert annotation │ │ | ||
| │ │ • Generates TLS cert/key signed by service CA │ │ | ||
| │ │ • Creates Secret with tls.crt and tls.key │ │ | ||
| │ │ • Supports headless services (SAN wildcards) │ │ | ||
| │ └─────────────────────────────────────────────────────┘ │ | ||
| │ │ | ||
| │ ┌─────────────────────────────────────────────────────┐ │ | ||
| │ │ pkg/controller/cabundleinjector/ │ │ | ||
| │ │ CA Bundle Injector │ │ | ||
| │ │ • ConfigMap injector (service-ca.crt data key) │ │ | ||
| │ │ • APIService injector (spec.caBundle field) │ │ | ||
| │ │ • CRD injector (conversion webhook caBundle) │ │ | ||
| │ │ • MutatingWebhookConfiguration injector │ │ | ||
| │ │ • ValidatingWebhookConfiguration injector │ │ | ||
| │ │ • Legacy vulnerable injection (4.7 upgrade path) │ │ | ||
| │ └─────────────────────────────────────────────────────┘ │ | ||
| └─────────────────────────────────────────────────────────────┘ | ||
| │ | ||
| │ Uses | ||
| ↓ | ||
| ┌─────────────────────────────────────────────────────────────┐ | ||
| │ Signing CA Secret (openshift-service-ca namespace) │ | ||
| │ signing-key │ | ||
| │ • tls.crt — Current signing CA certificate │ | ||
| │ • tls.key — Current signing CA private key │ | ||
| │ • ca-bundle.crt — Full CA bundle (current + old CAs) │ | ||
| │ • intermediate-ca.crt — Post-rotation bridge cert │ | ||
| └─────────────────────────────────────────────────────────────┘ | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Label the fenced diagrams for markdownlint.
Both fenced blocks are unlabeled, matching the static-analysis warning. Adding text keeps the docs lint-clean.
Also applies to: 168-170
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 14-14: 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 `@ARCHITECTURE.md` around lines 14 - 65, The fenced diagram blocks in
ARCHITECTURE.md are missing language labels, triggering markdownlint warnings.
Update each fenced block in the architecture diagram section to use a text label
(for example, on the opening fence) so the markdown stays lint-clean; this
applies to both diagram fences referenced in the document.
Source: Linters/SAST tools
| For more information regarding more general OpenShift pull request processes, the following resources are helpful: - https://docs.ci.openshift.org/architecture/jira - https://docs.ci.openshift.org/ | ||
| - https://steps.ci.openshift.org/ | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix the malformed resource list.
Those links are collapsed into one line with stray control characters, so the Markdown won’t render cleanly. Split them into separate bullets.
♻️ Proposed fix
-For more information regarding more general OpenShift pull request processes, the following resources are helpful:��- https://docs.ci.openshift.org/architecture/jira�- https://docs.ci.openshift.org/
-- https://steps.ci.openshift.org/
+For more information regarding more general OpenShift pull request processes, the following resources are helpful:
+- https://docs.ci.openshift.org/architecture/jira
+- https://docs.ci.openshift.org/
+- https://steps.ci.openshift.org/📝 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.
| For more information regarding more general OpenShift pull request processes, the following resources are helpful:��- https://docs.ci.openshift.org/architecture/jira�- https://docs.ci.openshift.org/ | |
| - https://steps.ci.openshift.org/ | |
| For more information regarding more general OpenShift pull request processes, the following resources are helpful: | |
| - https://docs.ci.openshift.org/architecture/jira | |
| - https://docs.ci.openshift.org/ | |
| - https://steps.ci.openshift.org/ |
🤖 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 `@CONTRIBUTING.md` around lines 141 - 143, The resource list in CONTRIBUTING.md
is malformed because the links are collapsed together with stray control
characters, so fix the Markdown list formatting. Update the section containing
the OpenShift PR resources so each URL is its own bullet item, keeping the
surrounding text intact and ensuring the list renders cleanly in Markdown.
| ### Building the test binary | ||
|
|
||
| ```bash | ||
| make build | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Rename this section to match the actual build target.
make build is the repo’s full build command, so “Building the test binary” is a bit misleading. Consider renaming this to “Building the binaries” or similar.
♻️ Proposed fix
-### Building the test binary
+### Building the binaries📝 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.
| ### Building the test binary | |
| ```bash | |
| make build | |
| ``` | |
| ### Building the binaries | |
🤖 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 `@CONTRIBUTING.md` around lines 175 - 179, Rename the “Building the test
binary” section so it matches the actual `make build` target, which builds the
full binaries rather than just a test binary. Update the heading in the
CONTRIBUTING section to something like “Building the binaries” and keep the
existing `make build` example under it, using the section title as the unique
locator for the change.
Source: Coding guidelines
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@ARCHITECTURE.md`:
- Around line 20-23: The remaining fenced diagram blocks in ARCHITECTURE.md are
still unlabeled and trigger MD040; update each opening fence to use a text
language tag. Locate the two diagram fences around the service-ca-operator
architecture diagram and add the same label to the other remaining fence in the
document so all fenced blocks are explicitly tagged.
- Around line 31-37: The architecture table has incorrect controller package
paths, with entries like servingcert/controller/ and cabundleinjector/* missing
the pkg/controller/ prefix. Update the affected rows in ARCHITECTURE.md to point
to the real pkg/controller/... locations for the serving cert and CA injector
controllers, using the existing component names (for example, Serving Cert
Signer, ConfigMap CA Injector, APIService CA Injector, Webhook CA Injectors, and
CRD CA Injector) to keep the table accurate.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 672c53a1-8de6-43e4-8a80-287867792e47
📒 Files selected for processing (3)
AGENTS.mdARCHITECTURE.mdREADME.md
| ``` | ||
| service-ca-operator operator → pkg/operator/ → manages CA + controller Deployment | ||
| service-ca-operator controller → pkg/controller/ → signs certs, injects bundles | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Label the remaining diagram fences.
Both unlabeled fenced blocks still trigger MD040; add text to each opening fence.
Fix
-```
+```textBased on the markdownlint warning, these fences still need a language tag.
Also applies to: 89-95
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 20-20: 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 `@ARCHITECTURE.md` around lines 20 - 23, The remaining fenced diagram blocks in
ARCHITECTURE.md are still unlabeled and trigger MD040; update each opening fence
to use a text language tag. Locate the two diagram fences around the
service-ca-operator architecture diagram and add the same label to the other
remaining fence in the document so all fenced blocks are explicitly tagged.
Source: Linters/SAST tools
| | Serving Cert Signer | `servingcert/controller/` | Services, Secrets | Creates TLS Secrets for annotated Services | | ||
| | Serving Cert Updater | `servingcert/controller/` | Services, Secrets | Refreshes certs approaching expiry | | ||
| | ConfigMap CA Injector | `cabundleinjector/configmap.go` | ConfigMaps | Injects CA bundle into annotated ConfigMaps | | ||
| | APIService CA Injector | `cabundleinjector/apiservice.go` | APIServices | Sets `spec.caBundle` on annotated APIServices | | ||
| | Webhook CA Injectors | `cabundleinjector/admissionwebhook.go` | Mutating/ValidatingWebhookConfigs | Sets `caBundle` on annotated webhooks | | ||
| | CRD CA Injector | `cabundleinjector/crd.go` | CRDs | Sets conversion webhook `caBundle` | | ||
| | Legacy Vulnerable Injector | `cabundleinjector/configmap.go` | ConfigMaps named `openshift-service-ca.crt` | Injects legacy bundle for pre-4.7 upgraded clusters | |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix the controller package paths.
The table should point at the real pkg/controller/... directories; servingcert/controller/ is malformed, and the other rows drop the same prefix.
Fix
-| Serving Cert Signer | `servingcert/controller/` | Services, Secrets | Creates TLS Secrets for annotated Services |
+| Serving Cert Signer | `pkg/controller/servingcert/` | Services, Secrets | Creates TLS Secrets for annotated Services |As per coding guidelines, controller packages live under pkg/controller/....
🤖 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 `@ARCHITECTURE.md` around lines 31 - 37, The architecture table has incorrect
controller package paths, with entries like servingcert/controller/ and
cabundleinjector/* missing the pkg/controller/ prefix. Update the affected rows
in ARCHITECTURE.md to point to the real pkg/controller/... locations for the
serving cert and CA injector controllers, using the existing component names
(for example, Serving Cert Signer, ConfigMap CA Injector, APIService CA
Injector, Webhook CA Injectors, and CRD CA Injector) to keep the table accurate.
Source: Coding guidelines
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: everettraven 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
approved
|
@oceanc80: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
/lgtm |
openshift-ci-robot
added
the
verified
|
@everettraven: The DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
CLAUDE.md was added in PR openshift#333 as a standalone file. PR openshift#362 later added AGENTS.md (with the same content restructured) plus ARCHITECTURE.md and CONTRIBUTING.md, which together cover everything in the original CLAUDE.md. Replace the standalone file with a symlink so Claude Code discovers the same content as other AI tools reading AGENTS.md — one source of truth, zero duplication.
3 participants
Adds AGENTS.md, ARCHITECTURE.md, and CONTRIBUTING.md files to provide guidance to both AI agents and human contributors
Summary by CodeRabbit