CM-1200: Adding agentic .md files for Cert-manager repository#454
CM-1200: Adding agentic .md files for Cert-manager repository#454arun717 wants to merge 2 commits into
Conversation
Introduce AGENTS.md, ARCHITECTURE.md, and CONTRIBUTING.md as the component entry point, and ignore local QE output/coverage artifacts. Co-authored-by: Cursor <cursoragent@cursor.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: arun717 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
WalkthroughThis PR adds a comprehensive documentation suite for the cert-manager-operator repository: new AGENTS.md, ARCHITECTURE.md, CLAUDE.md, CONTRIBUTING.md, and several docs/*.md guideline files; rewrites README.md; removes obsolete cloud_credentials.md, operand_metrics.md, and proxy.md; and updates .gitignore. ChangesDocumentation restructuring
Estimated code review effort: 2 (Simple) | ~15 minutes Related PRs: None identified. Suggested labels: documentation Suggested reviewers: None identified. 🐰 PoemA rabbit hops through docs anew, 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@arun717: 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. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 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 `@AGENTS.md`:
- Around line 173-177: The operand version bump guidance is too narrow because
it only points to make update-manifests and can leave generated artifacts stale;
update the AGENTS.md instructions to reference the full update flow used by the
repo, namely make update followed by make verify, so the version bump process
covers manifests plus bindata/generated code consistently.
- Around line 157-160: The naming guidance is too broad and incorrectly applies
the “cluster” CR name to every operand. Update the Naming section to split the
rule by operand type, keeping the `cluster` name for `CertManager` and
`TrustManager` while documenting that `IstioCSR` is the namespaced `default`
singleton; use the existing operand names and `common.ManagedResourceLabelKey`
reference to make the distinction explicit.
- Around line 44-48: The docs index in AGENTS.md still advertises removed
operational pages, so update the layout tree and the nearby docs list to stop
referencing dead paths. Remove the entries for proxy.md, cloud_credentials.md,
and operand_metrics.md, or swap them for the new documentation pages if they
exist. Use the docs index section around the docs/ tree and the linked index
entries as the place to make the cleanup.
In `@CLAUDE.md`:
- Around line 35-39: The operand version bump guidance only mentions make
update-manifests, which can leave generated bindata stale after version changes.
Update the CLAUDE.md instructions to point readers to the full update flow using
make update (or make update && make verify) so the manifest and generated
outputs stay in sync; keep the reference near the operand version bumps section
and the existing update-manifests note.
In `@docs/integration-guidelines.md`:
- Around line 14-15: The reference in docs/integration-guidelines.md points to
the removed docs/proxy.md, so update that link to the new consolidated proxy
guidance or remove it if no replacement is needed. Use the surrounding text
about Subscription patch examples and the docs/integration-guidelines.md section
to locate and replace the stale reference.
- Around line 50-54: The Metrics section still references the removed operand
metrics doc, so update the `docs/integration-guidelines.md` reference to the new
metrics documentation location or drop the bullet entirely. Use the `Metrics`
section and the `docs/operand_metrics.md` mention as the anchor when editing so
the dead link is fully removed from the integration guidance.
In `@docs/security-guidelines.md`:
- Around line 28-39: The operational references in the Cloud credentials and
Trusted CA/proxy sections are stale because docs/cloud_credentials.md and
docs/proxy.md no longer exist. Update the links in the affected markdown to
point to the consolidated documentation targets, or remove those references
entirely if there is no replacement, keeping the surrounding guidance in
docs/security-guidelines.md aligned with the current docs structure.
In `@README.md`:
- Around line 7-13: The README’s CR naming contract is incorrect for IstioCSR:
the table and summary currently say all operands are named cluster, but the
IstioCSR singleton is actually default. Update the IstioCSR row and the closing
summary text so they clearly state the correct singleton name, while keeping the
other operand names unchanged.
- Around line 120-124: The README’s Operational section still points to deleted
doc files, so update the references in the Operational list to the new docs
index or remove the section entirely. Use the README’s existing Operational
heading and linked entries as the location to replace the dead links with a
valid top-level docs reference that matches the current documentation structure.
🪄 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: f8f93898-bf05-4a4b-8469-8b3acda97029
📒 Files selected for processing (15)
.gitignoreAGENTS.mdARCHITECTURE.mdCLAUDE.mdCONTRIBUTING.mdREADME.mddocs/api-contracts-guidelines.mddocs/cloud_credentials.mddocs/error-handling-guidelines.mddocs/integration-guidelines.mddocs/operand_metrics.mddocs/performance-guidelines.mddocs/proxy.mddocs/security-guidelines.mddocs/testing-guidelines.md
💤 Files with no reviewable changes (3)
- docs/operand_metrics.md
- docs/proxy.md
- docs/cloud_credentials.md
| docs/ | ||
| ├── *-guidelines.md # Agent-oriented deep guidelines (see index below) | ||
| ├── proxy.md # Proxy-related behavior (when present) | ||
| ├── cloud_credentials.md # Ambient credentials / cloud secret wiring (when present) | ||
| └── operand_metrics.md # Metrics and monitoring for the operand (when present) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove the stale operational-doc links.
docs/proxy.md, docs/cloud_credentials.md, and docs/operand_metrics.md are removed in this PR, so both the layout tree and the docs index now advertise dead paths. Drop those entries or replace them with the new docs pages.
Also applies to: 216-223
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@AGENTS.md` around lines 44 - 48, The docs index in AGENTS.md still advertises
removed operational pages, so update the layout tree and the nearby docs list to
stop referencing dead paths. Remove the entries for proxy.md,
cloud_credentials.md, and operand_metrics.md, or swap them for the new
documentation pages if they exist. Use the docs index section around the docs/
tree and the linked index entries as the place to make the cleanup.
| ### Naming | ||
|
|
||
| - Operand CR name is always `cluster` (CEL-enforced). | ||
| - Managed operand resources use label key `app` (`common.ManagedResourceLabelKey`). |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Scope the CR naming rule.
IstioCSR is the namespaced default singleton, so the blanket “always cluster” rule is wrong and will mislead contributors. Split this into CertManager/TrustManager vs. IstioCSR.
🧰 Tools
🪛 LanguageTool
[grammar] ~160-~160: Ensure spelling is correct
Context: ... operand resources use label key app (common.ManagedResourceLabelKey). - Package imports: prefer full paths; n...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@AGENTS.md` around lines 157 - 160, The naming guidance is too broad and
incorrectly applies the “cluster” CR name to every operand. Update the Naming
section to split the rule by operand type, keeping the `cluster` name for
`CertManager` and `TrustManager` while documenting that `IstioCSR` is the
namespaced `default` singleton; use the existing operand names and
`common.ManagedResourceLabelKey` reference to make the distinction explicit.
| Operand version bumps: | ||
|
|
||
| ```bash | ||
| make update-manifests # after changing CERT_MANAGER_VERSION etc. in Makefile | ||
| ``` |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Point version bumps at the full update flow.
make update-manifests is too narrow here; the repo's update path also refreshes bindata/generated code, so this instruction can leave stale artifacts behind after a version bump. Use make update (and then make verify) instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@AGENTS.md` around lines 173 - 177, The operand version bump guidance is too
narrow because it only points to make update-manifests and can leave generated
artifacts stale; update the AGENTS.md instructions to reference the full update
flow used by the repo, namely make update followed by make verify, so the
version bump process covers manifests plus bindata/generated code consistently.
| Operand version bumps: | ||
|
|
||
| ```bash | ||
| make update-manifests # after changing CERT_MANAGER_VERSION etc. in Makefile | ||
| ``` |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use the full update flow here too.
This should mention make update (or make update && make verify), not only make update-manifests; otherwise an operand version bump can leave generated bindata stale.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@CLAUDE.md` around lines 35 - 39, The operand version bump guidance only
mentions make update-manifests, which can leave generated bindata stale after
version changes. Update the CLAUDE.md instructions to point readers to the full
update flow using make update (or make update && make verify) so the manifest
and generated outputs stay in sync; keep the reference near the operand version
bumps section and the existing update-manifests note.
| - Prefer patching Subscription for operator env/config — OLM reverts direct Deployment edits. | ||
| - See `docs/proxy.md` for subscription patch examples. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix the stale docs/proxy.md reference.
docs/proxy.md is removed in this PR, so this link will 404. Please retarget it to the new consolidated guidance or drop the reference.
🤖 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 `@docs/integration-guidelines.md` around lines 14 - 15, The reference in
docs/integration-guidelines.md points to the removed docs/proxy.md, so update
that link to the new consolidated proxy guidance or remove it if no replacement
is needed. Use the surrounding text about Subscription patch examples and the
docs/integration-guidelines.md section to locate and replace the stale
reference.
| ## Metrics | ||
|
|
||
| - Operand metrics setup documented in `docs/operand_metrics.md`. | ||
| - Operator metrics via library-go status/metrics patterns. | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove the dead operand-metrics link.
docs/operand_metrics.md is also removed in this PR, so this reference is now broken. Please update it to the new location or remove 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 `@docs/integration-guidelines.md` around lines 50 - 54, The Metrics section
still references the removed operand metrics doc, so update the
`docs/integration-guidelines.md` reference to the new metrics documentation
location or drop the bullet entirely. Use the `Metrics` section and the
`docs/operand_metrics.md` mention as the anchor when editing so the dead link is
fully removed from the integration guidance.
| ## Cloud credentials | ||
|
|
||
| - AWS/GCP ambient credentials use Cloud Credential Operator via `CredentialsRequest` (`pkg/controller/certmanager/credentials_request.go`). | ||
| - Cloud credential secret name is passed at runtime via `CloudCredentialSecret` flag (see `pkg/operator/starter.go`). | ||
| - See `docs/cloud_credentials.md` for operational setup. | ||
|
|
||
| ## Trusted CA / proxy | ||
|
|
||
| - Trusted CA ConfigMap name is a runtime arg (`TrustedCAConfigMapName` in `pkg/operator/starter.go`). | ||
| - Prefer patching OLM Subscription env vars over editing Deployments directly (OLM reverts direct edits). | ||
| - See `docs/proxy.md` for proxy and CA injection patterns. | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Update the deleted operational-doc links.
Both docs/cloud_credentials.md and docs/proxy.md are removed in this PR, so these references will break. Please point them at the consolidated docs or delete the stale links.
🤖 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 `@docs/security-guidelines.md` around lines 28 - 39, The operational references
in the Cloud credentials and Trusted CA/proxy sections are stale because
docs/cloud_credentials.md and docs/proxy.md no longer exist. Update the links in
the affected markdown to point to the consolidated documentation targets, or
remove those references entirely if there is no replacement, keeping the
surrounding guidance in docs/security-guidelines.md aligned with the current
docs structure.
| | Operand CR | Kind | Description | | ||
| |------------|------|-------------| | ||
| | `cluster` | CertManager | Core cert-manager deployment (controller, webhook, cainjector) | | ||
| | `cluster` | TrustManager | Bundle distribution for trust anchors (TechPreview) | | ||
| | `cluster` | IstioCSR | Certificate signing for Istio workloads (TechPreview) | | ||
|
|
||
| The Operator automatically deploys a cluster-scoped `CertManager` object named `cluster` if it's missing (with default values). | ||
| All CRs use API group `operator.openshift.io/v1alpha1` and must be named `cluster`. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Fix the IstioCSR singleton name.
The table and summary say every CR is cluster, but IstioCSR is actually the namespaced default singleton. Update this so the CR naming contract matches the architecture docs.
🤖 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 `@README.md` around lines 7 - 13, The README’s CR naming contract is incorrect
for IstioCSR: the table and summary currently say all operands are named
cluster, but the IstioCSR singleton is actually default. Update the IstioCSR row
and the closing summary text so they clearly state the correct singleton name,
while keeping the other operand names unchanged.
| ### Operational | ||
|
|
||
| ```asciidoc | ||
| apiVersion: operator.openshift.io/v1alpha1 | ||
| kind: CertManager | ||
| metadata: | ||
| name: cluster | ||
| spec: | ||
| managementState: "Managed" | ||
| unsupportedConfigOverrides: | ||
| # Here's an example to supply custom DNS settings. | ||
| controller: | ||
| args: | ||
| - "--dns01-recursive-nameservers=1.1.1.1:53" | ||
| - "--dns01-recursive-nameservers-only" | ||
| ``` | ||
| ## Metrics and Monitoring | ||
| - [docs/proxy.md](docs/proxy.md) — egress proxy and trusted CA configuration | ||
| - [docs/cloud_credentials.md](docs/cloud_credentials.md) — Cloud Credential Operator integration | ||
| - [docs/operand_metrics.md](docs/operand_metrics.md) — Prometheus metrics setup |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Drop the dead operational-doc links.
docs/proxy.md, docs/cloud_credentials.md, and docs/operand_metrics.md are deleted in this PR, so the README now links to files that no longer exist. Replace that section with the new docs index or remove 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 `@README.md` around lines 120 - 124, The README’s Operational section still
points to deleted doc files, so update the references in the Operational list to
the new docs index or remove the section entirely. Use the README’s existing
Operational heading and linked entries as the location to replace the dead links
with a valid top-level docs reference that matches the current documentation
structure.
arun717
changed the title
openshift-ci-robot
added
the
jira/valid-reference
|
@arun717: This pull request references CM-1200 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. |
2 participants
Summary by CodeRabbit
Documentation
Chores