SPIRE-233, OAPE-880: Add agentic documentation for AI-assisted development - #144
SPIRE-233, OAPE-880: Add agentic documentation for AI-assisted development#144rausingh-rh wants to merge 3 commits into
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@rausingh-rh: This pull request references SPIRE-233 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. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rausingh-rh 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
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdded repository guidance and linked documentation for ZTWIM development, testing, architecture, design decisions, custom resources, execution plans, and ecosystem references. ChangesZTWIM documentation
Estimated code review effort: 2 (Simple) | ~15 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✨ 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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/security-guidelines.md (1)
123-125: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd a language tag to this fence.
This is tripping MD040;
textis sufficient here.Proposed fix
-``` +```text app.kubernetes.io/managed-by = zero-trust-workload-identity-manager🤖 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 123 - 125, The fenced block in the documentation is missing a language tag, causing the MD040 lint issue. Update the markdown fence around the app.kubernetes.io/managed-by example to use a text language tag so the code block is explicitly marked as plain text. Locate the fenced example in the security guidelines content and apply the fix without changing the displayed value.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 `@docs/performance-guidelines.md`:
- Around line 151-157: The CREATE_ONLY_MODE description is too broad and should
not say controllers “skip all Update calls,” since status and OLM condition
writes still happen elsewhere. Update the wording in the performance guidelines
to scope the behavior to managed-resource drift-correction updates only, and
keep the references to ResourceNeedsUpdate and Upgradeable consistent with that
narrower behavior.
In `@docs/security-guidelines.md`:
- Around line 62-64: The fenced snippet in the security guidelines markdown is
missing a language tag and is triggering the MD040 lint warning. Update the
existing fence around the serving-cert annotation text to use the text language
tag so the rendered block remains unchanged while satisfying the markdown
linter.
In `@docs/testing-guidelines.md`:
- Around line 24-35: The outer fenced block in the testing guidelines markdown
is unlabeled, which triggers markdownlint MD040. Update the code block that
contains the tree layout so it uses a text fence, keeping the content in the
existing testing-guidelines section readable while satisfying the linter.
---
Outside diff comments:
In `@docs/security-guidelines.md`:
- Around line 123-125: The fenced block in the documentation is missing a
language tag, causing the MD040 lint issue. Update the markdown fence around the
app.kubernetes.io/managed-by example to use a text language tag so the code
block is explicitly marked as plain text. Locate the fenced example in the
security guidelines content and apply the fix without changing the displayed
value.
🪄 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: 4bb8ad89-3e1e-463c-bcb5-cb11bc37f756
📒 Files selected for processing (8)
AGENTS.mdCLAUDE.mddocs/api-contracts-guidelines.mddocs/error-handling-guidelines.mddocs/integration-guidelines.mddocs/performance-guidelines.mddocs/security-guidelines.mddocs/testing-guidelines.md
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 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 208-210: Update the documentation bullet for
predicate.GenerationChangedPredicate to identify it as a controller-runtime
built-in, replacing the incorrect “standard library” wording while leaving the
predicate usage and surrounding bullets unchanged.
In `@docs/api-contracts-guidelines.md`:
- Around line 176-177: Update the workflow instructions in
docs/api-contracts-guidelines.md to include running make test after generated
artifacts are updated, while retaining the existing make verify step for linting
and formatting validation.
- Around line 205-206: Update the SPIRE dependency update procedure in the
documentation to run make vendor immediately after changing go.mod, then run
make verify. Keep the existing CRD manifest copy step and ensure the sequence
explicitly keeps vendored Go types synchronized with the selected dependency.
- Around line 40-49: Update the marker documentation so `<Kind>` is explicitly
identified as a substitution token, instructing authors to replace it with the
concrete resource kind. Add one concrete marker-block example with the
placeholder replaced in both the XValidation message and CSV displayName while
preserving the required marker set.
- Around line 181-190: Update the “How to Add a New CRD” checklist to
distinguish operand-specific requirements from general CRD steps. Make embedding
CommonConfig, implementing GetConditionalStatus(), and updating ZTWIM
OperandStatus aggregation conditional on the resource being an operand, so
non-operand resources such as ZeroTrustWorkloadIdentityManager do not inherit
those status API requirements.
In `@docs/integration-guidelines.md`:
- Around line 16-22: Add the text language tag to the fenced blocks at
docs/integration-guidelines.md lines 16-22 and 43-51,
docs/security-guidelines.md lines 62-64 and 123-125, and
docs/testing-guidelines.md lines 24-35; leave each block’s contents unchanged.
In `@docs/performance-guidelines.md`:
- Around line 20-21: Update the cache visibility statement near
cacheResourceWithoutReqSelectors to limit it to resources registered in the
label-filtered cacheResources list. Do not claim that all resources without the
managed-by label are invisible, since operator CRs and OperatorCondition remain
tracked.
- Around line 71-73: Update ZTWIMSpecChangedPredicate.UpdateFunc so it accepts
Update events when the resource generation changes and rejects updates that only
modify status; retain the existing Create and Delete behavior. Update the
referenced performance guideline to document that reconciliation occurs for
spec/generation changes but not status-only writes.
In `@docs/security-guidelines.md`:
- Line 85: Update the external-certificate RBAC documentation near
SpireServerExternalCertRoleName to explicitly state that the RoleBinding grants
system:serviceaccount:openshift-ingress:router access to the Secret, while
preserving the existing operator-namespace requirement.
- Around line 43-52: The SCC workflow documentation incorrectly treats the
resource as bindata. Update the relevant integration guidance to identify
pkg/controller/spire-agent/scc.go as the source and document its
Get/Create/Update reconciliation workflow, then align the SPIRE Agent section in
docs/security-guidelines.md with that process without changing the listed
security constraints.
🪄 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: 85459567-bc4c-4870-8776-2095c0024d18
📒 Files selected for processing (8)
AGENTS.mdCLAUDE.mddocs/api-contracts-guidelines.mddocs/error-handling-guidelines.mddocs/integration-guidelines.mddocs/performance-guidelines.mddocs/security-guidelines.mddocs/testing-guidelines.md
🚧 Files skipped from review as they are similar to previous changes (1)
- CLAUDE.md
rausingh-rh
changed the title
There was a problem hiding this comment.
Actionable comments posted: 7
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (14)
AGENTS.md-66-66 (1)
66-66: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winKeep the agent retrieval order consistent.
Change this path to
domain/ → architecture/components.md → ZTWIM_DEVELOPMENT.md, or markdecisions/as optional after that sequence.As per coding guidelines, relevant
ai-docs/documentation must be read in the orderdomain/,architecture/components.md, thenZTWIM_DEVELOPMENT.md.🤖 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` at line 66, Update the “AI Agent Path” guidance in AGENTS.md to require the retrieval order domain/ → architecture/components.md → ZTWIM_DEVELOPMENT.md, removing decisions/ from the mandatory sequence or explicitly marking it optional after these documents.Source: Coding guidelines
AGENTS.md-12-12 (1)
12-12: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winAlign
AGENTS.mdwith the bundle channel.The bundle configuration and
ai-docs/references/ecosystem.mdusealpha. ChangeAGENTS.md#L12fromstable-v1toalpha.🤖 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` at line 12, Update the channel reference in AGENTS.md line 12 from stable-v1 to alpha, matching the existing bundle configuration and ai-docs/references/ecosystem.md lines 59-63; no direct change is required in the ecosystem reference.ai-docs/references/ecosystem.md-7-7 (1)
7-7: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAlign each
Locationlabel with its href.Use the existing
platform/operator-patterns,domain/kubernetes, anddomain/openshiftpaths. Thepractices/testing,practices/security, anddecisionspaths return 404; remove those path-specific labels or link to valid destinations.🤖 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 `@ai-docs/references/ecosystem.md` at line 7, Update the Location labels in ecosystem.md so each label matches its href: retain the valid platform/operator-patterns, domain/kubernetes, and domain/openshift paths, and remove or replace the labels for practices/testing, practices/security, and decisions unless they point to valid destinations.Source: MCP tools
ai-docs/domain/shared-types.md-98-113 (1)
98-113: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winFix the Markdown fence structure in all domain documents.
Several files contain unlabeled or unmatched fences. Markdownlint reports MD040, and unmatched fences can render later content as code.
ai-docs/domain/shared-types.md#L98-L113: label the Kubebuilder blocks asgoand remove the unmatched fence.ai-docs/domain/zero-trust-workload-identity-manager.md#L95-L95: remove the unmatched fence.ai-docs/domain/spiffe-csi-driver.md#L95-L95: remove the unmatched fence.ai-docs/domain/spire-agent.md#L105-L105: remove the unmatched fence.ai-docs/domain/spire-oidc-discovery-provider.md#L90-L90: remove the unmatched fence.ai-docs/domain/spire-server.md#L126-L126: remove the unmatched fence.🤖 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 `@ai-docs/domain/shared-types.md` around lines 98 - 113, Fix Markdown fence structure in all affected domain documents: in ai-docs/domain/shared-types.md lines 98-113, label the Kubebuilder code fences as go and remove the unmatched fence; in ai-docs/domain/zero-trust-workload-identity-manager.md line 95, ai-docs/domain/spiffe-csi-driver.md line 95, ai-docs/domain/spire-agent.md line 105, ai-docs/domain/spire-oidc-discovery-provider.md line 90, and ai-docs/domain/spire-server.md line 126, remove the unmatched fences. Ensure every remaining code block has a matching fence and language label.Source: Linters/SAST tools
ai-docs/architecture/components.md-7-7 (1)
7-7: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd language identifiers to the fenced blocks.
The fences at Lines 7, 112, and 165 trigger Markdownlint MD040. Use
textfor these repository-layout and flow diagrams.As per coding guidelines, run
make verifybefore pushing changes.Also applies to: 112-112, 165-165
🤖 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 `@ai-docs/architecture/components.md` at line 7, Update the fenced code blocks at the referenced locations in components.md to specify the text language identifier, including the repository-layout and flow-diagram fences. Run make verify after the documentation changes.Sources: Coding guidelines, Linters/SAST tools
ai-docs/decisions/adr-0001-controller-runtime-over-library-go.md-41-45 (1)
41-45: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUpdate the stale controller-runtime documentation links.
go.moddeclares v0.22.4, butmain.golinks to v0.19.0. Use v0.22.4 or version-neutral documentation.🤖 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 `@ai-docs/decisions/adr-0001-controller-runtime-over-library-go.md` around lines 41 - 45, Update the controller-runtime documentation reference associated with the manager setup in main.go to use v0.22.4 or a version-neutral URL, keeping it consistent with the dependency declared in go.mod. Leave the other references unchanged.ai-docs/decisions/adr-0002-bindata-for-operand-manifests.md-34-34 (1)
34-34: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse one bindata target consistently. Both targets are valid, but use
make update-bindata-assetsin the ADR to match the architecture documentation.🤖 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 `@ai-docs/decisions/adr-0002-bindata-for-operand-manifests.md` at line 34, Update the ADR’s manifest-generation workflow to consistently reference the make target update-bindata-assets, replacing update-bindata while preserving the surrounding YAML, constant, and generated-file steps.ai-docs/decisions/adr-0002-bindata-for-operand-manifests.md-16-16 (1)
16-16: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCorrect the
MustAssetfailure-timing statement.
assets.MustAsset(path)exists and controllers use it, so the line 16 statement is correct.MustAssetcallsAssetat runtime and panics when lookup or loading fails. It does not catch missing assets at build time. Update line 30 accordingly.🤖 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 `@ai-docs/decisions/adr-0002-bindata-for-operand-manifests.md` at line 16, Update the ADR statement around the MustAsset failure timing to clarify that assets.MustAsset(path) performs lookup at runtime, calling Asset and panicking when lookup or loading fails; do not describe missing assets as being detected at build time.ai-docs/ZTWIM_TESTING.md-81-81 (1)
81-81: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd a language to the directory-tree fence.
The fence at Line 81 has no language identifier. Add
textto satisfy markdownlint MD040.🤖 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 `@ai-docs/ZTWIM_TESTING.md` at line 81, Update the directory-tree code fence in ZTWIM_TESTING.md to include the text language identifier, while preserving the fenced content unchanged.Source: Linters/SAST tools
ai-docs/ZTWIM_DEVELOPMENT.md-217-217 (1)
217-217: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRemove the unmatched code fence.
The fence at Line 217 has no matching opening fence. It creates an empty block and triggers MD040 and MD058. Remove it and keep the blank line before the
---rule.🤖 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 `@ai-docs/ZTWIM_DEVELOPMENT.md` at line 217, Remove the unmatched code fence at the end of ai-docs/ZTWIM_DEVELOPMENT.md, preserving the blank line immediately before the horizontal rule (`---`) so the document no longer contains an empty fenced block.Source: Linters/SAST tools
ai-docs/ZTWIM_DEVELOPMENT.md-174-175 (1)
174-175: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCorrect the
make testprerequisite.The
Makefiletesttarget setsOPERATOR_NAMESPACEitself in Lines 124-126. The current warning incorrectly requires callers to set it when invokingmake test. Limit this warning to directgo testcommands, or explain thatmake testsupplies the variable automatically.🤖 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 `@ai-docs/ZTWIM_DEVELOPMENT.md` around lines 174 - 175, Update the testing prerequisite in ZTWIM_DEVELOPMENT.md to remove the incorrect requirement that callers set OPERATOR_NAMESPACE before invoking make test. State that make test configures OPERATOR_NAMESPACE automatically, and reserve the warning for direct go test commands if needed.ai-docs/ZTWIM_DEVELOPMENT.md-185-185 (1)
185-185: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winFix the Go return example.
return (Result{}, err)is invalid Go syntax. Usereturn Result{}, err.🤖 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 `@ai-docs/ZTWIM_DEVELOPMENT.md` at line 185, Update the “Error with requeue” example in ZTWIM_DEVELOPMENT.md to return the zero-value Result without parentheses: use the valid Go form Result{}, err while preserving the existing error logging example.Source: MCP tools
ai-docs/ZTWIM_DEVELOPMENT.md-176-176 (1)
176-176: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCorrect the
SetControllerReferenceguidance.
controllerutil.SetControllerReferencereturns an error when owner validation, GVK lookup, or controller ownership validation fails. Require callers to check and propagate the error.🤖 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 `@ai-docs/ZTWIM_DEVELOPMENT.md` at line 176, Update the SetControllerReference guidance in ZTWIM_DEVELOPMENT.md to state that callers must check its returned error and propagate failures from owner validation, GVK lookup, or controller ownership validation, rather than claiming it fails silently when the owner type is not registered.Source: MCP tools
ai-docs/ZTWIM_TESTING.md-110-123 (1)
110-123: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDocument the pre-provisioned OpenShift requirement.
make test-e2eonly runsgo test; it does not create a Kind cluster. The suite requiresKUBECONFIGand OpenShift APIs and resources. State that users must provision an OpenShift 4.18+ cluster before running the target. Correct the misleading Makefile comment.🤖 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 `@ai-docs/ZTWIM_TESTING.md` around lines 110 - 123, Update the E2E testing documentation around the make test-e2e commands to state that users must provision a pre-existing OpenShift 4.18+ cluster with the required APIs and resources, and provide KUBECONFIG pointing to it; clarify that the target does not create a Kind cluster. Correct the misleading Makefile comment to reflect this prerequisite.
🧹 Nitpick comments (2)
AGENTS.md (1)
68-68: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAlign Platform links with their visible destinations.
Both files use generic or unresolved hrefs for specific Platform topics. Update each target, or use generic labels consistently. (github.com)
AGENTS.md#L68-L68: use distinct targets forOperator,Testing, andSecurity.ai-docs/references/ecosystem.md#L7-L7: link to the operator-patterns destination.ai-docs/references/ecosystem.md#L21-L21: replace the unresolved testing destination.ai-docs/references/ecosystem.md#L33-L33: replace the unresolved security destination.ai-docs/references/ecosystem.md#L45-L45: link to the Kubernetes destination.ai-docs/references/ecosystem.md#L55-L55: link to the OpenShift destination.ai-docs/references/ecosystem.md#L67-L67: replace the unresolved decisions destination.🤖 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` at line 68, Align every Platform reference with its visible destination: in AGENTS.md lines 68-68, assign distinct valid links for Operator, Testing, and Security; in ai-docs/references/ecosystem.md lines 7-7, 21-21, 33-33, 45-45, 55-55, and 67-67, replace generic or unresolved hrefs with the specified operator-patterns, testing, security, Kubernetes, OpenShift, and decisions destinations respectively.Source: MCP tools
ai-docs/exec-plans/README.md (1)
3-7: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake the execution-plan outline actionable.
These bullets are not links, and the file does not define the listed topics. Link each item to verified Platform guidance or add the missing 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 `@ai-docs/exec-plans/README.md` around lines 3 - 7, Make the table of contents in the execution-plan README actionable by linking each bullet—“What are exec-plans?”, “When to create them”, “How to use them”, “Completion workflow”, and “Template”—to existing verified Platform guidance or by adding corresponding sections in the README. Ensure every listed topic has a valid destination and matches the documented content.
🤖 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`:
- Line 32: Update the CRD documentation in AGENTS.md to state that only the five
ZTWIM-owned CRDs are cluster-scoped singletons named “cluster,” rather than
claiming all CRDs follow this rule. Document the three upstream CRDs separately
using their scope and naming details from ai-docs/references/ecosystem.md.
In `@ai-docs/architecture/components.md`:
- Around line 63-74: Update the “Reconciliation Flow” documentation to reflect
that workload-health checks are controller-specific, since the CSI controller’s
Reconcile flow returns after reconcileDaemonSet without invoking a health check.
Keep the universal steps accurate and identify Step 8 as applicable only to
controllers that implement CheckStatefulSetHealth(), CheckDaemonSetHealth(), or
CheckDeploymentHealth().
In `@ai-docs/domain/shared-types.md`:
- Around line 30-41: Align the condition contract between shared-types.md and
zero-trust-workload-identity-manager.md by verifying the condition referenced as
OperandsAvailable. If it is emitted, add its documented status and reason
semantics alongside the condition types and reasons; otherwise, update the
workload identity manager documentation to use the actual defined condition
name.
In `@ai-docs/domain/spiffe-csi-driver.md`:
- Line 36: Update the “Socket Path Triad” guidance to require alignment only
between SpireAgent.spec.socketPath and SpiffeCSIDriver.spec.agentSocketPath;
clarify that the workload volumeMounts[].mountPath may differ. Add the separate
requirement that pluginName matches volumes[].csi.driver, using the example near
the existing Line 79 guidance.
In `@ai-docs/ZTWIM_DEVELOPMENT.md`:
- Around line 63-64: Update the binary build documentation near the FIPS-aware
description to explicitly state that binaries built without FIPS support must
not be used in CI or production. Preserve the existing local-build warning and
clarify that non-FIPS builds are restricted to local development only.
- Line 177: Update the reconciliation error guidance near the “DO NOT return
nil” rule to distinguish errors that must be returned for controller-runtime
requeue from validation errors and deferred status-update errors that are
handled after setting status or logged. Remove the unqualified “always” wording
and align the rule with the documented behavior in the validation and
status-update sections.
In `@ai-docs/ZTWIM_TESTING.md`:
- Around line 7-14: Update ZTWIM_TESTING.md to document envtest as part of the
unit-test workflow, correcting the claim that it is unused. Revise the direct
package-test guidance near the unit-test commands to include the required
KUBEBUILDER_ASSETS setup, or replace it with the supported Makefile test command
that performs this setup.
---
Minor comments:
In `@AGENTS.md`:
- Line 66: Update the “AI Agent Path” guidance in AGENTS.md to require the
retrieval order domain/ → architecture/components.md → ZTWIM_DEVELOPMENT.md,
removing decisions/ from the mandatory sequence or explicitly marking it
optional after these documents.
- Line 12: Update the channel reference in AGENTS.md line 12 from stable-v1 to
alpha, matching the existing bundle configuration and
ai-docs/references/ecosystem.md lines 59-63; no direct change is required in the
ecosystem reference.
In `@ai-docs/architecture/components.md`:
- Line 7: Update the fenced code blocks at the referenced locations in
components.md to specify the text language identifier, including the
repository-layout and flow-diagram fences. Run make verify after the
documentation changes.
In `@ai-docs/decisions/adr-0001-controller-runtime-over-library-go.md`:
- Around line 41-45: Update the controller-runtime documentation reference
associated with the manager setup in main.go to use v0.22.4 or a version-neutral
URL, keeping it consistent with the dependency declared in go.mod. Leave the
other references unchanged.
In `@ai-docs/decisions/adr-0002-bindata-for-operand-manifests.md`:
- Line 34: Update the ADR’s manifest-generation workflow to consistently
reference the make target update-bindata-assets, replacing update-bindata while
preserving the surrounding YAML, constant, and generated-file steps.
- Line 16: Update the ADR statement around the MustAsset failure timing to
clarify that assets.MustAsset(path) performs lookup at runtime, calling Asset
and panicking when lookup or loading fails; do not describe missing assets as
being detected at build time.
In `@ai-docs/domain/shared-types.md`:
- Around line 98-113: Fix Markdown fence structure in all affected domain
documents: in ai-docs/domain/shared-types.md lines 98-113, label the Kubebuilder
code fences as go and remove the unmatched fence; in
ai-docs/domain/zero-trust-workload-identity-manager.md line 95,
ai-docs/domain/spiffe-csi-driver.md line 95, ai-docs/domain/spire-agent.md line
105, ai-docs/domain/spire-oidc-discovery-provider.md line 90, and
ai-docs/domain/spire-server.md line 126, remove the unmatched fences. Ensure
every remaining code block has a matching fence and language label.
In `@ai-docs/references/ecosystem.md`:
- Line 7: Update the Location labels in ecosystem.md so each label matches its
href: retain the valid platform/operator-patterns, domain/kubernetes, and
domain/openshift paths, and remove or replace the labels for practices/testing,
practices/security, and decisions unless they point to valid destinations.
In `@ai-docs/ZTWIM_DEVELOPMENT.md`:
- Line 217: Remove the unmatched code fence at the end of
ai-docs/ZTWIM_DEVELOPMENT.md, preserving the blank line immediately before the
horizontal rule (`---`) so the document no longer contains an empty fenced
block.
- Around line 174-175: Update the testing prerequisite in ZTWIM_DEVELOPMENT.md
to remove the incorrect requirement that callers set OPERATOR_NAMESPACE before
invoking make test. State that make test configures OPERATOR_NAMESPACE
automatically, and reserve the warning for direct go test commands if needed.
- Line 185: Update the “Error with requeue” example in ZTWIM_DEVELOPMENT.md to
return the zero-value Result without parentheses: use the valid Go form
Result{}, err while preserving the existing error logging example.
- Line 176: Update the SetControllerReference guidance in ZTWIM_DEVELOPMENT.md
to state that callers must check its returned error and propagate failures from
owner validation, GVK lookup, or controller ownership validation, rather than
claiming it fails silently when the owner type is not registered.
In `@ai-docs/ZTWIM_TESTING.md`:
- Line 81: Update the directory-tree code fence in ZTWIM_TESTING.md to include
the text language identifier, while preserving the fenced content unchanged.
- Around line 110-123: Update the E2E testing documentation around the make
test-e2e commands to state that users must provision a pre-existing OpenShift
4.18+ cluster with the required APIs and resources, and provide KUBECONFIG
pointing to it; clarify that the target does not create a Kind cluster. Correct
the misleading Makefile comment to reflect this prerequisite.
---
Nitpick comments:
In `@AGENTS.md`:
- Line 68: Align every Platform reference with its visible destination: in
AGENTS.md lines 68-68, assign distinct valid links for Operator, Testing, and
Security; in ai-docs/references/ecosystem.md lines 7-7, 21-21, 33-33, 45-45,
55-55, and 67-67, replace generic or unresolved hrefs with the specified
operator-patterns, testing, security, Kubernetes, OpenShift, and decisions
destinations respectively.
In `@ai-docs/exec-plans/README.md`:
- Around line 3-7: Make the table of contents in the execution-plan README
actionable by linking each bullet—“What are exec-plans?”, “When to create them”,
“How to use them”, “Completion workflow”, and “Template”—to existing verified
Platform guidance or by adding corresponding sections in the README. Ensure
every listed topic has a valid destination and matches the documented content.
🪄 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: 84ceebf8-ee18-4290-9f87-48bd161575c0
📒 Files selected for processing (16)
AGENTS.mdai-docs/ZTWIM_DEVELOPMENT.mdai-docs/ZTWIM_TESTING.mdai-docs/architecture/components.mdai-docs/decisions/adr-0001-controller-runtime-over-library-go.mdai-docs/decisions/adr-0002-bindata-for-operand-manifests.mdai-docs/decisions/adr-template.mdai-docs/domain/shared-types.mdai-docs/domain/spiffe-csi-driver.mdai-docs/domain/spire-agent.mdai-docs/domain/spire-oidc-discovery-provider.mdai-docs/domain/spire-server.mdai-docs/domain/zero-trust-workload-identity-manager.mdai-docs/exec-plans/README.mdai-docs/references/ecosystem.mdai-docs/references/enhancements.md
|
/retest |
rausingh-rh
changed the title
|
@rausingh-rh: This pull request references SPIRE-233 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set. This pull request references OAPE-880 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 task 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. |
Signed-off-by: iamP1 <nhegde@redhat.com>
|
@rausingh-rh: 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. |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@ai-docs/domain/upstream-spiffe-crds.md`:
- Around line 70-83: Update ai-docs/domain/upstream-spiffe-crds.md at lines
70-83 to add the required spiffeIDTemplate field and replace workloadSelector
with an optional podSelector or namespaceSelector. At lines 94-102, change
bundleEndpointProfile to the object form with type: https_web. At lines 112-123,
represent selectors as selector-syntax strings such as "unix:uid:1000" instead
of type/value objects.
In `@ai-docs/README.md`:
- Around line 9-15: Reorder the documentation reading order in the README.md
file to place ZTWIM_DEVELOPMENT.md before decisions/. The correct sequence
should be: domain/, architecture/components.md, ZTWIM_DEVELOPMENT.md (build
workflow and bootstrap), then decisions/ (architectural decisions), and finally
ZTWIM_TESTING.md. This ensures agents read the required development workflow
before consulting architectural decisions when generating code.
In `@ai-docs/ZTWIM_DEVELOPMENT.md`:
- Around line 29-33: Update the installation guidance near the ZTWIM Deployment
description to state that users must manually create all ZTWIM-owned CRs, rather
than all CRs. Preserve the existing explanation that the ZTWIM controller
aggregates status and does not create operand CRs, while allowing separately
managed resources such as OIDC-created ClusterSPIFFEID resources.
🪄 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: 116cb81d-ac76-4baa-b61d-f5bc94afdee4
📒 Files selected for processing (10)
AGENTS.mdai-docs/README.mdai-docs/ZTWIM_DEVELOPMENT.mdai-docs/domain/shared-types.mdai-docs/domain/spiffe-csi-driver.mdai-docs/domain/spire-agent.mdai-docs/domain/spire-oidc-discovery-provider.mdai-docs/domain/spire-server.mdai-docs/domain/upstream-spiffe-crds.mdai-docs/domain/zero-trust-workload-identity-manager.md
🚧 Files skipped from review as they are similar to previous changes (7)
- ai-docs/domain/spire-oidc-discovery-provider.md
- ai-docs/domain/spire-agent.md
- ai-docs/domain/spire-server.md
- ai-docs/domain/zero-trust-workload-identity-manager.md
- AGENTS.md
- ai-docs/domain/shared-types.md
- ai-docs/domain/spiffe-csi-driver.md
| ```yaml | ||
| apiVersion: spire.spiffe.io/v1alpha1 | ||
| kind: ClusterSPIFFEID | ||
| metadata: | ||
| name: example-workload | ||
| spec: | ||
| className: zero-trust-workload-identity-manager-spire # required for this ZTWIM installation | ||
| workloadSelector: | ||
| k8sServiceAccount: | ||
| name: example-sa | ||
| namespace: example-ns | ||
| dnsNameTemplates: | ||
| - "*.example.com" | ||
| ttl: 3600 |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
# First, find the documentation file and read the examples
head -150 ai-docs/domain/upstream-spiffe-crds.md | tail -100Repository: openshift/zero-trust-workload-identity-manager
Length of output: 4658
🏁 Script executed:
# Find CRD definition files in the repository
find . -path ./vendor -prune -o -name "*.yaml" -type f | grep -i crd | grep -v vendor | head -20Repository: openshift/zero-trust-workload-identity-manager
Length of output: 756
🏁 Script executed:
# Also look for config/crd/bases directory which is mentioned in coding guidelines
ls -la config/crd/bases/ 2>/dev/null | head -20Repository: openshift/zero-trust-workload-identity-manager
Length of output: 1063
🏁 Script executed:
# Read ClusterSPIFFEID CRD schema
cat config/crd/bases/clusterspiffeids-spiffe-crd.yaml | head -200Repository: openshift/zero-trust-workload-identity-manager
Length of output: 9602
🏁 Script executed:
# Read ClusterFederatedTrustDomain CRD schema
cat config/crd/bases/clusterfederatedtrustdomains-spiffe-crd.yamlRepository: openshift/zero-trust-workload-identity-manager
Length of output: 4421
🏁 Script executed:
# Read ClusterStaticEntry CRD schema
cat config/crd/bases/clusterstaticentries-spiffe-crd.yamlRepository: openshift/zero-trust-workload-identity-manager
Length of output: 3851
🏁 Script executed:
# Get more of the ClusterSPIFFEID schema to see workloadSelectorTemplates
cat config/crd/bases/clusterspiffeids-spiffe-crd.yaml | tail -n +200 | head -100Repository: openshift/zero-trust-workload-identity-manager
Length of output: 3429
🏁 Script executed:
# Check for sample files that show correct usage
find config/samples -name "*.yaml" -type f | xargs grep -l "ClusterSPIFFEID\|ClusterFederatedTrustDomain\|ClusterStaticEntry"Repository: openshift/zero-trust-workload-identity-manager
Length of output: 384
🏁 Script executed:
# Read the sample files to see correct field structures
cat config/samples/spire.spiffe.io_v1alpha1_clusterspiffeid.yamlRepository: openshift/zero-trust-workload-identity-manager
Length of output: 738
🏁 Script executed:
cat config/samples/spire.spiffe.io_v1alpha1_clusterfederatedtrustdomain.yamlRepository: openshift/zero-trust-workload-identity-manager
Length of output: 640
🏁 Script executed:
cat config/samples/spire.spiffe.io_v1alpha1_clusterstaticentries.yamlRepository: openshift/zero-trust-workload-identity-manager
Length of output: 759
🏁 Script executed:
# Verify the actual field names in the CRD schema more carefully
python3 << 'PY'
import yaml
# Read ClusterSPIFFEID CRD
with open('config/crd/bases/clusterspiffeids-spiffe-crd.yaml') as f:
crd = yaml.safe_load(f)
spec_props = crd['spec']['versions'][0]['schema']['openAPIV3Schema']['properties']['spec']['properties']
print("=== ClusterSPIFFEID spec properties ===")
print("Keys:", sorted(spec_props.keys()))
print("\nRequired:", crd['spec']['versions'][0]['schema']['openAPIV3Schema']['properties']['spec'].get('required', []))
# Read ClusterFederatedTrustDomain CRD
with open('config/crd/bases/clusterfederatedtrustdomains-spiffe-crd.yaml') as f:
crd = yaml.safe_load(f)
spec_props = crd['spec']['versions'][0]['schema']['openAPIV3Schema']['properties']['spec']['properties']
print("\n=== ClusterFederatedTrustDomain spec properties ===")
print("Keys:", sorted(spec_props.keys()))
print("Required:", crd['spec']['versions'][0]['schema']['openAPIV3Schema']['properties']['spec'].get('required', []))
print("\nbundleEndpointProfile schema:")
print(spec_props['bundleEndpointProfile'])
# Read ClusterStaticEntry CRD
with open('config/crd/bases/clusterstaticentries-spiffe-crd.yaml') as f:
crd = yaml.safe_load(f)
spec_props = crd['spec']['versions'][0]['schema']['openAPIV3Schema']['properties']['spec']['properties']
print("\n=== ClusterStaticEntry spec properties ===")
print("Keys:", sorted(spec_props.keys()))
print("Required:", crd['spec']['versions'][0]['schema']['openAPIV3Schema']['properties']['spec'].get('required', []))
print("\nselectors schema:")
print(spec_props['selectors'])
PYRepository: openshift/zero-trust-workload-identity-manager
Length of output: 301
🏁 Script executed:
# Extract the required fields and property names from the CRD schemas using grep and sed
echo "=== ClusterSPIFFEID required fields ==="
grep -A 20 "required:" config/crd/bases/clusterspiffeids-spiffe-crd.yaml | head -30
echo ""
echo "=== ClusterFederatedTrustDomain required fields ==="
grep -A 20 "required:" config/crd/bases/clusterfederatedtrustdomains-spiffe-crd.yaml | head -30
echo ""
echo "=== ClusterStaticEntry required fields ==="
grep -A 20 "required:" config/crd/bases/clusterstaticentries-spiffe-crd.yaml | head -30Repository: openshift/zero-trust-workload-identity-manager
Length of output: 3751
🏁 Script executed:
# Check if bundleEndpointProfile can be a string or must be an object
echo "=== bundleEndpointProfile details ==="
grep -A 15 "bundleEndpointProfile:" config/crd/bases/clusterfederatedtrustdomains-spiffe-crd.yamlRepository: openshift/zero-trust-workload-identity-manager
Length of output: 913
🏁 Script executed:
# Confirm the actual sample file values for bundleEndpointProfile and compare with doc examples
echo "=== Sample bundleEndpointProfile value ==="
grep -A 2 "bundleEndpointProfile:" config/samples/spire.spiffe.io_v1alpha1_clusterfederatedtrustdomain.yaml
echo ""
echo "=== Doc example bundleEndpointProfile value ==="
grep -A 2 "bundleEndpointProfile:" ai-docs/domain/upstream-spiffe-crds.mdRepository: openshift/zero-trust-workload-identity-manager
Length of output: 355
🏁 Script executed:
# Check if workloadSelector is defined somewhere in the ClusterSPIFFEID schema
grep -i "workloadselector" config/crd/bases/clusterspiffeids-spiffe-crd.yamlRepository: openshift/zero-trust-workload-identity-manager
Length of output: 313
🏁 Script executed:
# Get the full list of spec properties for ClusterSPIFFEID to confirm what's supported
grep -A 200 "properties:" config/crd/bases/clusterspiffeids-spiffe-crd.yaml | grep -E "^\s+[a-z].*:" | head -30Repository: openshift/zero-trust-workload-identity-manager
Length of output: 1203
🏁 Script executed:
# Get the complete spec properties list for ClusterSPIFFEID
sed -n '/spec:/,/status:/p' config/crd/bases/clusterspiffeids-spiffe-crd.yaml | grep -E "^\s+[a-zA-Z]" | head -50Repository: openshift/zero-trust-workload-identity-manager
Length of output: 2462
🏁 Script executed:
# More carefully extract all property keys in ClusterSPIFFEID spec
python3 << 'PY'
import re
with open('config/crd/bases/clusterspiffeids-spiffe-crd.yaml') as f:
content = f.read()
# Find the spec properties section
spec_match = re.search(r'description: ClusterSPIFFEIDSpec.*?properties:\s*(.*?)\s+required:', content, re.DOTALL)
if spec_match:
props_text = spec_match.group(1)
# Extract all keys at the top level (not nested)
keys = re.findall(r'^ (\w+):', props_text, re.MULTILINE)
print("ClusterSPIFFEID spec top-level properties:")
for key in sorted(set(keys)):
print(f" - {key}")
PYRepository: openshift/zero-trust-workload-identity-manager
Length of output: 375
🏁 Script executed:
# Check the actual sample file again more carefully - is workloadSelector really there?
cat -n config/samples/spire.spiffe.io_v1alpha1_clusterspiffeid.yaml | grep -A 5 "spec:"Repository: openshift/zero-trust-workload-identity-manager
Length of output: 346
🏁 Script executed:
# Get complete list of all spec properties in ClusterSPIFFEID schema
python3 << 'PY'
import re
with open('config/crd/bases/clusterspiffeids-spiffe-crd.yaml') as f:
content = f.read()
# Find all lines with property names in the spec section
lines = content.split('\n')
in_spec_props = False
indent_level = None
properties = []
for i, line in enumerate(lines):
if 'description: ClusterSPIFFEIDSpec' in line:
in_spec_props = True
continue
if in_spec_props:
if 'required:' in line and '- ' in lines[i+1] if i+1 < len(lines) else False:
# We've reached the required fields, so we're done with properties
break
# Match property definitions (lines starting with spaces and ending with ':')
if re.match(r'^ \w+:', line):
prop_name = re.search(r'(\w+):', line).group(1)
properties.append(prop_name)
print("All ClusterSPIFFEID spec properties:")
for prop in sorted(set(properties)):
print(f" - {prop}")
PYRepository: openshift/zero-trust-workload-identity-manager
Length of output: 379
🏁 Script executed:
# Check what the sample file structure actually is - maybe it's incomplete or wrong
wc -l config/samples/spire.spiffe.io_v1alpha1_clusterspiffeid.yaml
cat config/samples/spire.spiffe.io_v1alpha1_clusterspiffeid.yamlRepository: openshift/zero-trust-workload-identity-manager
Length of output: 802
🏁 Script executed:
# Get ALL properties from the ClusterSPIFFEID spec, including ones we might have missed
sed -n '/description: ClusterSPIFFEIDSpec/,/required:/p' config/crd/bases/clusterspiffeids-spiffe-crd.yaml | grep -E '^\s{14}[a-zA-Z]' | awk '{print $1}' | sort | uniqRepository: openshift/zero-trust-workload-identity-manager
Length of output: 312
🏁 Script executed:
# Check if workloadSelector appears anywhere in the CRD schema at all
grep -i "workloadselector\|workload.selector" config/crd/bases/clusterspiffeids-spiffe-crd.yamlRepository: openshift/zero-trust-workload-identity-manager
Length of output: 389
🏁 Script executed:
# Now check the documentation examples against the actual properties
echo "=== Doc example workloadSelector ==="
grep -B 5 -A 5 "workloadSelector:" ai-docs/domain/upstream-spiffe-crds.mdRepository: openshift/zero-trust-workload-identity-manager
Length of output: 514
🏁 Script executed:
# Check if the sample files are generated or hand-written
head -5 config/samples/spire.spiffe.io_v1alpha1_clusterspiffeid.yaml
head -5 config/samples/spire.spiffe.io_v1alpha1_clusterfederatedtrustdomain.yaml
head -5 config/samples/spire.spiffe.io_v1alpha1_clusterstaticentries.yamlRepository: openshift/zero-trust-workload-identity-manager
Length of output: 631
🏁 Script executed:
# Check the actual required fields and complete schema for ClusterSPIFFEID spec
sed -n '/description: ClusterSPIFFEIDSpec/,/required:/p' config/crd/bases/clusterspiffeids-spiffe-crd.yaml | tail -20Repository: openshift/zero-trust-workload-identity-manager
Length of output: 1280
🏁 Script executed:
# Let me check if there are more properties in the schema that we haven't extracted yet
# by looking at the full schema more carefully
grep -c "type: string\|type: boolean\|type: object\|type: array" config/crd/bases/clusterspiffeids-spiffe-crd.yaml
grep "spiffeIDTemplate\|podSelector\|ttl\|x509SVIDTTL\|jwtTtl" config/crd/bases/clusterspiffeids-spiffe-crd.yaml | head -10Repository: openshift/zero-trust-workload-identity-manager
Length of output: 318
🏁 Script executed:
# Check the full list of spec properties by looking at the whole schema file more systematically
python3 << 'PY'
import re
with open('config/crd/bases/clusterspiffeids-spiffe-crd.yaml') as f:
content = f.read()
# Find spec properties section
spec_start = content.find('description: ClusterSPIFFEIDSpec')
spec_end = content.find('required:', spec_start)
spec_section = content[spec_start:spec_end]
# Find all top-level property keys (those with 14 spaces of indentation in the properties section)
prop_matches = re.findall(r'\n ([a-zA-Z][a-zA-Z0-9]*?):\n', spec_section)
properties = sorted(set(prop_matches))
print("ClusterSPIFFEID spec properties from schema:")
for p in properties:
print(f" {p}")
PYRepository: openshift/zero-trust-workload-identity-manager
Length of output: 367
🏁 Script executed:
# Check the Git history to see if these sample files are generated
git log --oneline -n 10 config/samples/spire.spiffe.io_v1alpha1_clusterspiffeid.yaml 2>/dev/null | head -5Repository: openshift/zero-trust-workload-identity-manager
Length of output: 223
🏁 Script executed:
# Check if there's any script that generates or validates these sample files
find . -name "*.sh" -o -name "Makefile" | xargs grep -l "sample\|spiffe" 2>/dev/null | head -5Repository: openshift/zero-trust-workload-identity-manager
Length of output: 184
🏁 Script executed:
# Let me search for any documentation or tests that show the correct format
find . -name "*.md" -o -name "*.go" | xargs grep -l "workloadSelectorTemplates\|spiffeIDTemplate" 2>/dev/null | head -10Repository: openshift/zero-trust-workload-identity-manager
Length of output: 366
🏁 Script executed:
# Check if there are integration tests that show correct usage
find . -path ./vendor -prune -o -name "*test*.go" -type f | xargs grep -l "ClusterSPIFFEID\|ClusterFederatedTrustDomain\|ClusterStaticEntry" 2>/dev/null | head -5Repository: openshift/zero-trust-workload-identity-manager
Length of output: 395
🏁 Script executed:
# Verify the selector format by checking upstream SPIRE documentation references
grep -r "selector.*type.*value\|unix:uid" . --include="*.md" 2>/dev/null | head -5Repository: openshift/zero-trust-workload-identity-manager
Length of output: 184
🏁 Script executed:
# Check the upstream SPIRE controller manager API types
cat vendor/github.com/spiffe/spire-controller-manager/api/v1alpha1/clusterspiffeid_types.go | head -150Repository: openshift/zero-trust-workload-identity-manager
Length of output: 6107
🏁 Script executed:
# Check ClusterFederatedTrustDomain and ClusterStaticEntry types
ls -la vendor/github.com/spiffe/spire-controller-manager/api/v1alpha1/ | grep clusterRepository: openshift/zero-trust-workload-identity-manager
Length of output: 757
🏁 Script executed:
# Look at actual ClusterStaticEntry types
grep -A 20 "type ClusterStaticEntrySpec" vendor/github.com/spiffe/spire-controller-manager/api/v1alpha1/clusterstaticentry_types.goRepository: openshift/zero-trust-workload-identity-manager
Length of output: 1181
🏁 Script executed:
# Check ClusterFederatedTrustDomain types
cat vendor/github.com/spiffe/spire-controller-manager/api/v1alpha1/clusterfederatedtrustdomain_types.go | head -100Repository: openshift/zero-trust-workload-identity-manager
Length of output: 3843
🏁 Script executed:
# Look at the e2e tests to see how these CRs are actually used
cat test/e2e/e2e_test.go | grep -A 20 "ClusterSPIFFEID\|ClusterFederatedTrustDomain\|ClusterStaticEntry" | head -50Repository: openshift/zero-trust-workload-identity-manager
Length of output: 3008
🏁 Script executed:
# Check if the sample file is somehow valid despite not being in the schema
# by checking if there are any tests that actually use it
grep -r "workloadSelector" . --include="*.go" --include="*.yaml" 2>/dev/null | grep -v vendor | head -10Repository: openshift/zero-trust-workload-identity-manager
Length of output: 575
🏁 Script executed:
# Final check: validate the doc examples against what we know
echo "=== Summary of actual API structures ==="
echo "ClusterSPIFFEID required field: spiffeIDTemplate (string)"
echo "ClusterSPIFFEID selector fields: podSelector (LabelSelector) or namespaceSelector (LabelSelector)"
echo "ClusterFederatedTrustDomain.bundleEndpointProfile must be object with 'type' field"
echo "ClusterStaticEntry.selectors must be array of strings (e.g., 'unix:uid:1000')"Repository: openshift/zero-trust-workload-identity-manager
Length of output: 542
Fix upstream CRD examples to match generated schemas.
The documentation examples use field shapes that Kubernetes validation rejects. Users copying these manifests will receive schema validation errors.
- ClusterSPIFFEID (lines 70–83): Missing required field
spiffeIDTemplate. RemoveworkloadSelector; usepodSelectorornamespaceSelector(both are optional label selectors) if pod/namespace targeting is needed. - ClusterFederatedTrustDomain (lines 94–102): Change
bundleEndpointProfile: https_webto an object:bundleEndpointProfile: { type: https_web }. - ClusterStaticEntry (lines 112–123): Change selector objects to strings in selector syntax format (e.g.,
selectors: ["unix:uid:1000"]instead of a list of objects withtypeandvaluekeys).
📍 Affects 1 file
ai-docs/domain/upstream-spiffe-crds.md#L70-L83(this comment)ai-docs/domain/upstream-spiffe-crds.md#L94-L102ai-docs/domain/upstream-spiffe-crds.md#L112-L123
🤖 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 `@ai-docs/domain/upstream-spiffe-crds.md` around lines 70 - 83, Update
ai-docs/domain/upstream-spiffe-crds.md at lines 70-83 to add the required
spiffeIDTemplate field and replace workloadSelector with an optional podSelector
or namespaceSelector. At lines 94-102, change bundleEndpointProfile to the
object form with type: https_web. At lines 112-123, represent selectors as
selector-syntax strings such as "unix:uid:1000" instead of type/value objects.
| Follow this order when working on a task: | ||
|
|
||
| 1. **`domain/`** — CRD types, validation rules, field relationships | ||
| 2. **`architecture/components.md`** — controller patterns, reconcile flow, shared utilities | ||
| 3. **`decisions/`** — architectural decisions (controller-runtime, bindata) | ||
| 4. **`ZTWIM_DEVELOPMENT.md`** — build workflow, bootstrap, common tasks | ||
| 5. **`ZTWIM_TESTING.md`** — unit and E2E test patterns (when writing tests) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use the required documentation order.
The Agent Path places decisions/ before ZTWIM_DEVELOPMENT.md. Move decisions/ after ZTWIM_DEVELOPMENT.md so agents read the required development workflow before generating code.
Proposed order
1. **`domain/`** — CRD types, validation rules, field relationships
2. **`architecture/components.md`** — controller patterns, reconcile flow, shared utilities
-3. **`decisions/`** — architectural decisions (controller-runtime, bindata)
-4. **`ZTWIM_DEVELOPMENT.md`** — build workflow, bootstrap, common tasks
-5. **`ZTWIM_TESTING.md`** — unit and E2E test patterns (when writing tests)
+3. **`ZTWIM_DEVELOPMENT.md`** — build workflow, bootstrap, common tasks
+4. **`decisions/`** — architectural decisions (controller-runtime, bindata)
+5. **`ZTWIM_TESTING.md`** — unit and E2E test patterns (when writing tests)As per coding guidelines, **/*: Before generating code, read relevant documentation in ai-docs/ in this order: domain/, architecture/components.md, then ZTWIM_DEVELOPMENT.md.
📝 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.
| Follow this order when working on a task: | |
| 1. **`domain/`** — CRD types, validation rules, field relationships | |
| 2. **`architecture/components.md`** — controller patterns, reconcile flow, shared utilities | |
| 3. **`decisions/`** — architectural decisions (controller-runtime, bindata) | |
| 4. **`ZTWIM_DEVELOPMENT.md`** — build workflow, bootstrap, common tasks | |
| 5. **`ZTWIM_TESTING.md`** — unit and E2E test patterns (when writing tests) | |
| Follow this order when working on a task: | |
| 1. **`domain/`** — CRD types, validation rules, field relationships | |
| 2. **`architecture/components.md`** — controller patterns, reconcile flow, shared utilities | |
| 3. **`ZTWIM_DEVELOPMENT.md`** — build workflow, bootstrap, common tasks | |
| 4. **`decisions/`** — architectural decisions (controller-runtime, bindata) | |
| 5. **`ZTWIM_TESTING.md`** — unit and E2E test patterns (when writing tests) |
🤖 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 `@ai-docs/README.md` around lines 9 - 15, Reorder the documentation reading
order in the README.md file to place ZTWIM_DEVELOPMENT.md before decisions/. The
correct sequence should be: domain/, architecture/components.md,
ZTWIM_DEVELOPMENT.md (build workflow and bootstrap), then decisions/
(architectural decisions), and finally ZTWIM_TESTING.md. This ensures agents
read the required development workflow before consulting architectural decisions
when generating code.
Source: Coding guidelines
| ZTWIM is installed via OLM. After the operator Deployment is running, **users must create all CRs manually** — the ZTWIM controller aggregates status but does **not** create operand CRs. | ||
|
|
||
| ### Required CRs | ||
|
|
||
| All five ZTWIM-owned CRs are cluster-scoped singletons named `cluster`: |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Limit the manual-creation statement to ZTWIM-owned CRs.
Line 29 says users must create all CRs manually. Line 63 states that the OIDC controller creates two operator-managed ClusterSPIFFEID resources. Qualify Line 29 to avoid duplicate ClusterSPIFFEID resources.
Proposed wording
-ZTWIM is installed via OLM. After the operator Deployment is running, **users must create all CRs manually** — the ZTWIM controller aggregates status but does **not** create operand CRs.
+ZTWIM is installed via OLM. After the operator Deployment is running, **users must create all five ZTWIM-owned CRs manually** — the ZTWIM controller aggregates status but does **not** create operand CRs.📝 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.
| ZTWIM is installed via OLM. After the operator Deployment is running, **users must create all CRs manually** — the ZTWIM controller aggregates status but does **not** create operand CRs. | |
| ### Required CRs | |
| All five ZTWIM-owned CRs are cluster-scoped singletons named `cluster`: | |
| ZTWIM is installed via OLM. After the operator Deployment is running, **users must create all five ZTWIM-owned CRs manually** — the ZTWIM controller aggregates status but does **not** create operand CRs. | |
| ### Required CRs | |
| All five ZTWIM-owned CRs are cluster-scoped singletons named `cluster`: |
🤖 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 `@ai-docs/ZTWIM_DEVELOPMENT.md` around lines 29 - 33, Update the installation
guidance near the ZTWIM Deployment description to state that users must manually
create all ZTWIM-owned CRs, rather than all CRs. Preserve the existing
explanation that the ZTWIM controller aggregates status and does not create
operand CRs, while allowing separately managed resources such as OIDC-created
ClusterSPIFFEID resources.
3 participants
Summary
Adds AI-optimized documentation following the ai-helpers component-docs format:
AGENTS.md— Agent onboarding entry pointai-docs/— Component-specific docs: CRD domain concepts, architecture, ADRs, development/testing guides, platform referencesSummary by CodeRabbit