OAPE-878: Add agentic documentation and coding guidelines - #465
Conversation
Provide AGENTS.md, ai-docs architecture/domain/ADRs, and deep-dive guideline playbooks so agents and contributors can follow operator-specific patterns without rediscovering them from code.
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
|
[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 |
|
@arun717: This pull request references OAPE-878 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. |
WalkthroughAdded documentation for the Cert Manager Operator. The documentation covers repository navigation, architecture, APIs, controllers, platform integrations, security, FIPS, packaging, development, and testing workflows. ChangesOperator documentation
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 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. |
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (2)
ai-docs/CERT_MANAGER_OPERATOR_DEVELOPMENT.md (1)
10-15: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winRemove the redundant
make generatecommand.
make buildalready runsgenerate,fmt, andvet.make verifyrunsfmtandvetagain. Usemake build && make update-manifests && make verifyfor this workflow.🤖 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/CERT_MANAGER_OPERATOR_DEVELOPMENT.md` around lines 10 - 15, Update the documented development workflow by removing the redundant make generate command from the command chain, leaving make build && make update-manifests && make verify because make build already performs generation.docs/security-guidelines.md (1)
22-24: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReword to remove the repeated "only".
The sentence at line 23 uses "only" twice close together: "are not user-configurable ... only expose an
egress-only API surface for new NetworkPolicy config." Rephrase to remove the repetition and make the rule clearer.✏️ Proposed wording fix
- ports it already runs (`cert_manager_networkpolicy.go`); only expose an `egress`-only API surface for - new NetworkPolicy config. + ports it already runs (`cert_manager_networkpolicy.go`); expose only an `egress` API surface for + new NetworkPolicy config.🤖 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 22 - 24, Reword the NetworkPolicy guidance near the ingress rule so “only” is not repeated in the same sentence. Preserve the meaning that ingress is derived by the operator and that new configuration exposes an egress-only API surface, using clearer phrasing around the existing cert_manager_networkpolicy.go reference.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 `@ai-docs/architecture/components.md`:
- Around line 39-40: The FeatureGate error behavior is inconsistent across the
documentation; align all three references with setupFeatureGates: retry
discovery failures, fail closed by treating unavailable gates as disabled, and
keep the operator running without starting optional controllers until gates are
known enabled. Update ai-docs/architecture/components.md lines 39-40,
ai-docs/decisions/adr-0003-feature-gates.md line 18, and
docs/operator-controllers-guidelines.md lines 164-166 to describe this same
persistent-error behavior.
In `@docs/api-contracts-guidelines.md`:
- Around line 29-31: The documentation contradicts itself: lines 29-31 instruct
contributors to add operand CRD YAML under config/crd/bases/, while lines
157-160 prohibit manual edits to all files in that directory. Clarify the
guidance by identifying which files in config/crd/bases/ are controller-gen
output and which are operand source manifests, then update both sections to
specify the correct source path for operand CRDs and document the generation
command that produces auto-generated CRD files so contributors understand where
to place manual changes versus what gets regenerated.
In `@docs/error-handling-guidelines.md`:
- Line 4: The link fragment in the reference on line 4
(`#7-library-go-vs-ctrl-runtime-do-not-mix-patterns`) does not match the anchor
generated by the corresponding heading on line 140. Verify the actual anchor
that the heading generates (based on markdown anchor generation rules) and
update the link fragment in the reference on line 4 to match it correctly, or
alternatively add an explicit anchor attribute to the heading on line 140 to
match the expected fragment.
In `@docs/olm-packaging-guidelines.md`:
- Around line 137-148: Align the “Verify scripts” table and nearby instructions
with the existing bindata verification flow: add the bindata verifier and its
`make verify-bindata` target to the table, or explicitly instruct authors to run
`make verify-bindata` directly rather than implying `make verify-scripts` covers
it. Preserve the existing CRD guidance.
- Around line 84-87: The packaging guidelines should distinguish generated
bundle metadata validation from runtime image-map validation: require
operator-sdk bundle validate for relatedImages, plus source-level or unit-test
coverage confirming every RELATED_IMAGE_* variable has a corresponding
imageEnvMap (or equivalent constants) mapping. Update the guidance around adding
new operand images without implying bundle validation checks Go runtime
mappings.
- Around line 57-62: Update the olm.skipRange guidance in the packaging
guidelines so z-stream releases use the candidate version as the exclusive upper
bound (for example, <1.20.1 for 1.20.1), while minor releases use the next
minor’s .0 bound (for example, <1.21.0 for 1.21.0). Preserve the existing
lower-bound and replaces guidance.
In `@docs/testing-guidelines.md`:
- Around line 27-30: Update the test-unit documentation and corresponding
Makefile exclusion rule to use directory alternatives rather than a character
class, matching only test/e2e, test/apis, and test/utils with a following slash
or end of path. Keep the listed test-unit behavior consistent with the Makefile.
- Around line 81-85: Update the CtrlClient example in the testing guidelines to
use valid Go interface syntax with complete method signatures and
parameter/return types, or clearly mark the example as pseudocode using a non-Go
code fence or prose. Preserve the counterfeiter directives only if the example
remains intended to be executable Go.
---
Nitpick comments:
In `@ai-docs/CERT_MANAGER_OPERATOR_DEVELOPMENT.md`:
- Around line 10-15: Update the documented development workflow by removing the
redundant make generate command from the command chain, leaving make build &&
make update-manifests && make verify because make build already performs
generation.
In `@docs/security-guidelines.md`:
- Around line 22-24: Reword the NetworkPolicy guidance near the ingress rule so
“only” is not repeated in the same sentence. Preserve the meaning that ingress
is derived by the operator and that new configuration exposes an egress-only API
surface, using clearer phrasing around the existing
cert_manager_networkpolicy.go reference.
🪄 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: dc6081ff-90f9-41fd-bca6-bd299ff4168b
📒 Files selected for processing (22)
AGENTS.mdai-docs/CERT_MANAGER_OPERATOR_DEVELOPMENT.mdai-docs/CERT_MANAGER_OPERATOR_TESTING.mdai-docs/architecture/components.mdai-docs/decisions/adr-0001-dual-controller-frameworks.mdai-docs/decisions/adr-0002-apply-strategies.mdai-docs/decisions/adr-0003-feature-gates.mdai-docs/decisions/adr-template.mdai-docs/domain/certmanager.mdai-docs/domain/istiocsr.mdai-docs/domain/trustmanager.mdai-docs/exec-plans/README.mdai-docs/references/ecosystem.mdai-docs/references/enhancements.mddocs/api-contracts-guidelines.mddocs/error-handling-guidelines.mddocs/fips-guidelines.mddocs/integration-guidelines.mddocs/olm-packaging-guidelines.mddocs/operator-controllers-guidelines.mddocs/security-guidelines.mddocs/testing-guidelines.md
| 5. `setupFeatureGates` from `--unsupported-addon-features` + optional `featuregates/cluster` read (fail-closed retries; does not abort operator on persistent FeatureGate errors) | ||
| 6. If IstioCSR and/or TrustManager enabled → `NewControllerManager` → Start in goroutine |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Resolve the inconsistent FeatureGate failure mode.
The documents describe opposite behavior for FeatureGate discovery errors. This can change whether optional controllers start when FeatureGate data is unavailable.
ai-docs/architecture/components.md#L39-L40: document the behavior implemented bysetupFeatureGates.ai-docs/decisions/adr-0003-feature-gates.md#L18-L18: align the ADR with the verified startup behavior.docs/operator-controllers-guidelines.md#L164-L166: align the implementation guidance with the same behavior.
#!/bin/bash
set -euo pipefail
rg -n -C 12 \
'setupFeatureGates|unsupported-addon-features|FeatureGateState|IsTrustManagerFeatureGateEnabled|IsIstioCSRFeatureGateEnabled' \
pkg/operator/starter.go \
pkg/operator/setup_manager.go \
pkg/features📍 Affects 3 files
ai-docs/architecture/components.md#L39-L40(this comment)ai-docs/decisions/adr-0003-feature-gates.md#L18-L18docs/operator-controllers-guidelines.md#L164-L166
🤖 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` around lines 39 - 40, The FeatureGate
error behavior is inconsistent across the documentation; align all three
references with setupFeatureGates: retry discovery failures, fail closed by
treating unavailable gates as disabled, and keep the operator running without
starting optional controllers until gates are known enabled. Update
ai-docs/architecture/components.md lines 39-40,
ai-docs/decisions/adr-0003-feature-gates.md line 18, and
docs/operator-controllers-guidelines.md lines 164-166 to describe this same
persistent-error behavior.
| - Adding a new operand CRD (e.g. a new `Bundle`-like resource) means adding a YAML base | ||
| under `config/crd/bases/` and wiring it into `config/crd/kustomization.yaml`, **not** | ||
| adding a Go type in `api/operator/v1alpha1/`. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Resolve the contradictory config/crd/bases/ guidance.
Lines 29-31 require contributors to add operand CRD YAML under config/crd/bases/. Lines 157-160 prohibit manual edits to every file in that directory. Identify which files are controller-gen output and which files are operand source manifests. Document the correct source path and generation command.
Also applies to: 157-160
🤖 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/api-contracts-guidelines.md` around lines 29 - 31, The documentation
contradicts itself: lines 29-31 instruct contributors to add operand CRD YAML
under config/crd/bases/, while lines 157-160 prohibit manual edits to all files
in that directory. Clarify the guidance by identifying which files in
config/crd/bases/ are controller-gen output and which are operand source
manifests, then update both sections to specify the correct source path for
operand CRDs and document the generation command that produces auto-generated
CRD files so contributors understand where to place manual changes versus what
gets regenerated.
| # Error Handling Guidelines | ||
|
|
||
| Scope: the controller-runtime (ctrl-runtime) stack — **IstioCSR** and **TrustManager**. CertManager | ||
| uses library-go and follows a different model (see [§7](#7-library-go-vs-ctrl-runtime-do-not-mix-patterns)). |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix the invalid §7 link fragment.
The link on Line 4 does not match the generated anchor for the heading on Line 140. Use the valid fragment or add an explicit anchor.
Proposed fix
-[§7](`#7-library-go-vs-ctrl-runtime-do-not-mix-patterns`)
+[§7](`#7-library-go-vs-ctrl-runtime--do-not-mix-patterns`)📝 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.
| uses library-go and follows a different model (see [§7](#7-library-go-vs-ctrl-runtime-do-not-mix-patterns)). | |
| uses library-go and follows a different model (see [§7](`#7-library-go-vs-ctrl-runtime--do-not-mix-patterns`)). |
🧰 Tools
🪛 markdownlint-cli2 (0.23.1)
[warning] 4-4: Link fragments should be valid
(MD051, link-fragments)
🤖 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/error-handling-guidelines.md` at line 4, The link fragment in the
reference on line 4 (`#7-library-go-vs-ctrl-runtime-do-not-mix-patterns`) does
not match the anchor generated by the corresponding heading on line 140. Verify
the actual anchor that the heading generates (based on markdown anchor
generation rules) and update the link fragment in the reference on line 4 to
match it correctly, or alternatively add an explicit anchor attribute to the
heading on line 140 to match the expected fragment.
Source: Linters/SAST tools
| - `replaces` must point at the exact previous CSV name shipped in the same channel (upgrade edge). | ||
| - `olm.skipRange` should cover all previously-published z-streams so upgrades from any older | ||
| installed version resolve correctly; bump the lower bound as old minors are dropped and the | ||
| upper bound to the new `<X.Y.0>` being released. | ||
| - Update **both** when cutting a new minor/z-stream; keep `bundle/metadata/annotations.yaml` | ||
| channel labels consistent with the release train. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
# Read the OLM packaging guidelines file to see exact text at lines 57-62 and 163-169
cat -n docs/olm-packaging-guidelines.md | sed -n '50,75p'Repository: openshift/cert-manager-operator
Length of output: 1948
🏁 Script executed:
# Also check lines 163-169
cat -n docs/olm-packaging-guidelines.md | sed -n '160,175p'Repository: openshift/cert-manager-operator
Length of output: 1948
🏁 Script executed:
# Look for actual CSV files in the repository to see real skipRange usage
find . -name "*.yaml" -o -name "*.yml" | grep -E "(csv|bundle|manifests)" | head -20Repository: openshift/cert-manager-operator
Length of output: 1948
🏁 Script executed:
# Search for skipRange usage in the repository
rg "skipRange" --type yaml -A 2 -B 2Repository: openshift/cert-manager-operator
Length of output: 1948
🌐 Web query:
OLM skipRange semver range semantics operator-sdk
💡 Result:
In Operator Lifecycle Manager (OLM), olm.skipRange is an annotation used within a ClusterServiceVersion (CSV) to define a range of semantic versions (semver) that can be skipped during an automatic operator upgrade [1][2][3]. When a channel head contains this annotation, if an installed operator's version falls within the specified range, OLM will automatically upgrade the operator to the latest entry in that channel, effectively skipping the intermediate versions [4][5][3]. Key technical details regarding olm.skipRange include: Semver Semantics: OLM utilizes the blang/semver Go library to parse and evaluate the range string [1][2]. The format must adhere to the semver range syntax supported by that library (e.g., ">=1.0.0 <1.0.3") [1][4][2]. Operational Behavior: - Precedence: When searching for updates, OLM evaluates update paths based on specific criteria. The channel head with a skipRange annotation is prioritized if the currently installed version falls within the defined range [4][5][3]. - Pruning: A significant consequence of using skipRange is that versions within the skipped range may be pruned from the update graph, making them unavailable for new installations via the spec.startingCSV field in a Subscription [1][2]. - Best Practice: To maintain installability of past versions while still leveraging skipRange, it is recommended to also define a replaces field in the CSV that points to the immediate previous version [1][2]. Operator SDK Integration: While the Operator SDK provides tooling to build and manage operators—including generating the manifests required by OLM—olm.skipRange is an OLM-specific feature [6]. The SDK facilitates the creation of the necessary CSVs and metadata where this annotation is applied, but the actual logic for evaluating the range and managing the upgrade graph is handled by the OLM controller (or the newer OLM v1/Operator Controller) in the cluster [7][8][6]. When transitioning to OLM v1 (Operator Controller), the behavior remains similar to legacy OLM for compatibility, though OLM v1 evaluates upgrade paths differently as it does not rely on a strict replaces chain [7][8]. OLM v1 identifies all entries that have replace, skip, or skipRange values covering the currently installed version to determine valid upgrade paths [7][8][9].
Citations:
- 1: https://v0-18-z.olm.operatorframework.io/docs/concepts/olm-architecture/operator-catalog/creating-an-update-graph/
- 2: https://olm.operatorframework.io/docs/concepts/olm-architecture/operator-catalog/creating-an-update-graph/
- 3: https://github.com/operator-framework/operator-lifecycle-manager/blob/master/doc/design/how-to-update-operators.md
- 4: Support semver ranges of versions to skip in the head of a channel operator-framework/operator-lifecycle-manager#834
- 5: https://docs.okd.io/4.22/operators/understanding/olm/olm-workflow.html
- 6: https://github.com/operator-framework/operator-sdk/blob/master/proposals/sdk-integration-with-olm.md
- 7: https://operator-framework.github.io/operator-controller/concepts/upgrade-support/
- 8: https://docs.okd.io/latest/extensions/ce/update-paths.html
- 9: https://docs.okd.io/4.22/extensions/ce/update-paths.html
🌐 Web query:
OLM olm.skipRange exclusive upper bound version comparison
💡 Result:
In Operator Lifecycle Manager (OLM), the olm.skipRange annotation uses the blang/semver Go library to define version ranges [1][2]. The version comparison for the exclusive upper bound follows standard semantic versioning (SemVer) rules [1][3]. When you specify an exclusive upper bound, such as <1.0.3, the range excludes any version that is equal to or greater than 1.0.3 [1][2]. For example, in the range >=1.0.0 <1.0.3, OLM considers any installed version that satisfies this range (e.g., 1.0.0, 1.0.1, 1.0.2) as eligible for an update to the bundle containing the skipRange annotation [1][3][2]. Key considerations regarding skipRange: - Inclusive/Exclusive semantics: The range follows standard SemVer logic, where < acts as an exclusive upper bound [1][3]. - Unbound ranges: It is recommended to avoid using unbound ranges (e.g., <1.0.3) if possible to prevent unintended update behavior [1][2]. - Catalog presence: Versions within the skipRange do not need to be present in the operator catalog for the bundle to be successfully added [1][2]. - Impact on installation: Using skipRange alone can prune previous versions from the update graph, making them unavailable for installation via a Subscription's startingCSV [1][2]. To maintain support for installing past versions while enabling skipRange, it is common practice to use skipRange in conjunction with a replaces field that connects to the immediate previous version in the update chain [1][2].
Citations:
- 1: https://v0-18-z.olm.operatorframework.io/docs/concepts/olm-architecture/operator-catalog/creating-an-update-graph/
- 2: https://olm.operatorframework.io/docs/concepts/olm-architecture/operator-catalog/creating-an-update-graph/
- 3: https://github.com/operator-framework/operator-lifecycle-manager/blob/master/doc/design/how-to-update-operators.md
Clarify the olm.skipRange upper bound for z-stream releases.
The documentation states to "bump the upper bound to the new <X.Y.0> being released" for both minor and z-stream releases. This rule is incorrect for z-stream releases. For a 1.20.1 release, using <1.20.0> excludes version 1.20.0, preventing users on that version from upgrading. Use the candidate version as the exclusive upper bound instead: <1.20.1> for a 1.20.1 z-stream release, and <1.21.0> for a 1.21.0 minor release. These bounds allow all previously-published versions in that minor series to upgrade correctly.
🤖 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/olm-packaging-guidelines.md` around lines 57 - 62, Update the
olm.skipRange guidance in the packaging guidelines so z-stream releases use the
candidate version as the exclusive upper bound (for example, <1.20.1 for
1.20.1), while minor releases use the next minor’s .0 bound (for example,
<1.21.0 for 1.21.0). Preserve the existing lower-bound and replaces guidance.
| Adding a **new** operand image requires: a new `RELATED_IMAGE_*` env in | ||
| `config/manager/manager.yaml`, an entry in the `imageEnvMap` (or equivalent constants file) that | ||
| reads it, and re-running `make bundle` so the corresponding `relatedImages` entry is generated — | ||
| missing any of these will make `operator-sdk bundle validate` / disconnected mirroring fail. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- target document ---'
cat -n docs/olm-packaging-guidelines.md | sed -n '70,190p'
printf '%s\n' '--- imageEnvMap and RELATED_IMAGE references ---'
rg -n -S 'imageEnvMap|RELATED_IMAGE_|relatedImages|bundle validate|make bundle' . \
-g '!vendor' -g '!node_modules' -g '!dist' -g '!build' | sed -n '1,240p'
printf '%s\n' '--- relevant tracked files ---'
git ls-files | rg '(^|/)(manager\.yaml|.*image.*map.*|.*constants.*|.*test.*|Makefile|bundle.*|config/.*)' | sed -n '1,240p'Repository: openshift/cert-manager-operator
Length of output: 1948
🏁 Script executed:
#!/bin/bash
set -eu
cat -n docs/olm-packaging-guidelines.md | sed -n '70,190p'
rg -n -S 'imageEnvMap|RELATED_IMAGE_|relatedImages|bundle validate|make bundle' . \
-g '!vendor' -g '!node_modules' -g '!dist' -g '!build' | sed -n '1,240p'Repository: openshift/cert-manager-operator
Length of output: 1948
🌐 Web query:
github openshift cert-manager-operator docs/olm-packaging-guidelines.md imageEnvMap RELATED_IMAGE_
💡 Result:
In the context of OpenShift and Operator Lifecycle Manager (OLM) packaging, the environment variable naming convention RELATED_IMAGE_ is a mandatory technical requirement used to identify container images associated with an operator [1][2]. This naming convention serves several critical functions in the OpenShift ecosystem: 1. Identification for Mirroring: It allows the OpenShift Build Service (OSBS) and other catalog-management tools to automatically discover all container images required by the operator [1]. This ensures these images can be correctly mirrored to the marketplace or private registries, which is essential for deployments in disconnected (air-gapped) or proxied environments [1][3][4]. 2. Automated Digest-Pinning: When an operator bundle is generated using tools like the Operator SDK (typically with the --use-image-digests flag), the build process scans for environment variables prefixed with RELATED_IMAGE_ [3][4][5]. It automatically resolves the image tags defined in these variables to their immutable content digests (SHA) and populates the spec.relatedImages section of the ClusterServiceVersion (CSV) [3][6][4]. 3. Configuration and Consistency: Using these variables ensures that the operand images used by the operator can be dynamically injected or overridden by the platform if necessary [1]. Developers are encouraged to use this prefix to ensure their operators remain compliant with Red Hat certification standards and function correctly across different OpenShift environments [3][2][5]. While spec.relatedImages in the CSV provides a static list of images for OLM catalog tools, the RELATED_IMAGE_* environment variables provide a runtime mechanism for the operator to know which image references to use, maintaining synchronization between the deployment configuration and the operator's runtime requirements [3][6][4]. Top results: [1][3][2][4][5]
Citations:
- 1: https://github.com/jsm84/rhm-env-example
- 2: Operator operand env var does not conform to OSBS naming requirements keycloak/keycloak#24017
- 3: bundle: nice UX around disconnected setup ocp-release-operator-sdk#113
- 4: https://github.com/opdev/simple-disconnected-operator
- 5: Helm operators generated by operator-sdk lack spec.relatedImages and thus fail RH certification operator-framework/operator-sdk#6482
- 6: How to sync
RELATED_IMAGEenvironment variables withspec.relatedImageswhen using image digests? operator-framework/operator-sdk#6923
🌐 Web query:
site:github.com/openshift/cert-manager-operator "Adding a new operand image requires"
💡 Result:
In the context of Kubernetes Operators, an operand is the application or software service that the operator manages [1][2]. Adding a new operand image generally involves configuring the operator to deploy the desired container image for the managed service [3]. While the specific implementation depends on the operator design, it typically requires the following steps: 1. Identify the Configuration Interface: Check the Custom Resource Definition (CRD) or configuration objects (e.g., a CertManager object) used by the operator to define its managed resources [1][4]. 2. Define the Operand Image: Operators are often configured to use specific images via environment variables, custom resource fields, or override configurations [3][5][6]. For example, in some implementations, related images are defined as environment variables passed to the operator container, or they can be overridden via the operator's configuration spec [3][4]. 3. Apply the Configuration: Use the Kubernetes API or command-line tools (such as oc or kubectl) to update the relevant Custom Resource (CR) with the new image reference [1][4]. This often involves updating a spec section that controls deployment configurations, such as environment variables, resource requirements, or custom arguments [7][4][6]. If you are working with a specific operator, consult its official documentation or the repository's README, as many operators require you to build the operator with specific image references or modify specific fields within the operator's managed CR to point to the desired image [1][2][4].
Citations:
- 1: https://github.com/openshift/cert-manager-operator
- 2: https://github.com/openshift/cert-manager-operator/blob/ae53ee1a5c3a515497f70435e9aaedba0a118f0d/README.md
- 3: 072db14
- 4: https://github.com/openshift/cert-manager-operator/blob/master/README.md
- 5: c49fd4e
- 6: 7d53547
- 7: [cert-manager-1.15] CM-411: Add support of NodeSelector and Tolerations to CertManager CRD for configuring the operand pods #214
🌐 Web query:
site:github.com/openshift/cert-manager-operator/blob "operator-sdk bundle validate" "imageEnvMap"
💡 Result:
There is no evidence in the provided search results to support the existence of an imageEnvMap option or feature within the operator-sdk bundle validate command. The search results discuss the configuration of the cert-manager-operator, including its Dockerfile, Makefile, and Go modules, but do not contain references to imageEnvMap. The operator-sdk bundle validate command is used to verify operator bundles against OLM standards, and users should typically consult the official Operator SDK documentation for a comprehensive list of available command flags and validation criteria.
Citations:
- 1: https://github.com/openshift/cert-manager-operator/blob/master/README.md
- 2: https://github.com/openshift/cert-manager-operator/blob/ac9cc761499788acf145f83ebeb2a7ee80dfa6a4/PROJECT
- 3: https://github.com/openshift/cert-manager-operator/blob/ae53ee1a5c3a515497f70435e9aaedba0a118f0d/README.md
- 4: https://github.com/openshift/cert-manager-operator/blob/master/go.mod
- 5: https://github.com/openshift/cert-manager-operator/blob/78de13d53779ba7b3a097978557f019bb1a9063a/Dockerfile
🌐 Web query:
https://raw.githubusercontent.com/openshift/cert-manager-operator/master/docs/olm-packaging-guidelines.md
💡 Result:
The document olm-packaging-guidelines.md, referenced in the context of the cert-manager-operator repository, refers to the standardized procedures and requirements for packaging an Operator for use with Red Hat's Operator Lifecycle Manager (OLM) [1][2]. In the context of the OpenShift and Operator Framework ecosystems, OLM packaging refers to the bundle format—a standardized way to package Operator metadata, Kubernetes manifests, and CRDs into a container image that OLM can use to install, manage, and upgrade software on a cluster [3][4][5]. Key components and practices associated with OLM packaging include: 1. Bundle Format: An Operator bundle is a container image containing the metadata and manifests (e.g., ClusterServiceVersion (CSV), CRDs, and roles) for a specific version of an Operator [1][6][4]. 2. ClusterServiceVersion (CSV): This is the primary entry point for OLM packaging, describing the Operator, its dependencies, install strategies, and runtime requirements [6][7]. 3. Catalog Images: Bundles are organized into catalogs, which are stored as container images and queried by OLM to provide available Operators to the cluster via OperatorHub [1][3][5]. 4. Tooling: The opm CLI tool is the standard utility provided by the Operator Framework for building, managing, and maintaining these catalogs and bundles [3][4][8]. 5. Guidelines and Compliance: When packaging Operators for OpenShift, authors must often comply with specific metadata requirements (such as OCP version annotations) and validation criteria to ensure compatibility and successful distribution via community or product catalogs [9]. For official technical guidance, developers typically refer to the Operator Framework's OLM documentation or the specific OpenShift documentation regarding operator packaging and development [3][7][8].
Citations:
- 1: https://github.com/cert-manager/cert-manager-olm
- 2: https://github.com/cert-manager/cert-manager-olm/blob/master/README.md
- 3: https://docs.okd.io/latest/operators/understanding/olm-packaging-format.html
- 4: https://docs.okd.io/4.9/operators/understanding/olm-packaging-format.html
- 5: https://docs.redhat.com/en/documentation/openshift_container_platform/4.12/html-sinble/operators/administrator-tasks
- 6: https://operator-framework.github.io/olm-book/docs/packaging-an-operator.html
- 7: https://olm.operatorframework.io/docs/
- 8: https://docs.redhat.com/en/documentation/openshift_container_platform/4.14/pdf/operators/developing-operators
- 9: https://redhat-openshift-ecosystem.github.io/operator-pipelines/users/packaging-required-criteria-ocp/
🌐 Web query:
Operator SDK bundle validate validates relatedImages CSV environment variables runtime image map
💡 Result:
The Operator SDK bundle validate command ensures that your bundle adheres to specific requirements, including the spec.relatedImages field in your ClusterServiceVersion (CSV) [1][2]. This field is used to list images required by the operator, which must be referenced by digest (SHA) to support disconnected environments [3][4]. Key aspects of how relatedImages and environment variables interact include: Related Images Discovery: The Operator SDK can automatically discover RELATED_IMAGE_ environment variables defined in your manager.yaml (or other container environments) and add them to the spec.relatedImages list in your generated CSV [5]. This is a recommended practice to ensure that your operator correctly advertises its required images to the Operator Lifecycle Manager (OLM) [5]. Environment Variables vs. Static Definitions: While you can define related images statically in your base CSV, it is highly recommended to use RELATED_IMAGE_ environment variables [5][6]. Using these variables ensures that when you build your bundle (specifically when using the --use-image-digests flag), the generated CSV is correctly populated with the image digests derived from your environment [7][3][4]. Validating with bundle validate: The operator-sdk bundle validate command checks the integrity and format of your bundle [2]. While it does not automatically "map" environment variables at runtime, it validates that your CSV meets the criteria for catalog submission [1]. To ensure your images are properly validated as digests, you should run make bundle with USE_IMAGE_DIGESTS=true [3][4]. This flag instructs the SDK to resolve images to their SHA digests and populate spec.relatedImages accordingly [3][4]. If you encounter certification or validation errors (e.g., missing spec.relatedImages), ensure that you have enabled digest resolution in your Makefile via BUNDLE_GEN_FLAGS or the USE_IMAGE_DIGESTS variable [4][8]. Note that spec.relatedImages is used primarily during catalog creation and for disconnected environment support; it does not play a direct role in cluster runtime image mapping, which relies on the RELATED_IMAGE_ environment variables themselves being correctly injected into your operator's deployment [5][9][8].
Citations:
- 1: https://master.sdk.operatorframework.io/docs/olm-integration/generation/
- 2: https://sdk.operatorframework.io/docs/cli/operator-sdk_bundle_validate/
- 3: https://docs.okd.io/4.17/operators/operator_sdk/osdk-generating-csvs.html
- 4: https://docs.okd.io/4.18/operators/operator_sdk/osdk-generating-csvs.html
- 5: Don't override relatedImages in config CSV. operator-framework/operator-sdk#5763
- 6: Don't override relatedImages in base CSV. #5763 operator-framework/operator-sdk#5765
- 7: How to sync
RELATED_IMAGEenvironment variables withspec.relatedImageswhen using image digests? operator-framework/operator-sdk#6923 - 8: Helm operators generated by operator-sdk lack spec.relatedImages and thus fail RH certification operator-framework/operator-sdk#6482
- 9:
generate bundle: explicitly support.spec.relatedImagesand.spec.skipsoperator-framework/operator-sdk#4884
Separate bundle metadata validation from runtime image-map validation.
operator-sdk bundle validate checks generated bundle metadata. It does not inspect the Go imageEnvMap or constants that consume each RELATED_IMAGE_* variable. Require both bundle validation and source-level or unit-test validation for every mapping. A missing runtime mapping can pass bundle validation and still break disconnected operand image resolution.
🤖 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/olm-packaging-guidelines.md` around lines 84 - 87, The packaging
guidelines should distinguish generated bundle metadata validation from runtime
image-map validation: require operator-sdk bundle validate for relatedImages,
plus source-level or unit-test coverage confirming every RELATED_IMAGE_*
variable has a corresponding imageEnvMap (or equivalent constants) mapping.
Update the guidance around adding new operand images without implying bundle
validation checks Go runtime mappings.
| ## Verify scripts | ||
|
|
||
| | Script | Checks | Wired into `make verify-scripts`? | | ||
| |---|---|---| | ||
| | `hack/verify-bundle.sh` | Runs `make bundle` and fails if the tree has uncommitted diffs. | Yes | | ||
| | `hack/verify-deepcopy.sh` | Generated `zz_generated.deepcopy.go` is current. | Yes | | ||
| | `hack/verify-clientgen.sh` | Generated clientset/informers/listers/applyconfigurations are current. | Yes | | ||
| | `hack/verify-crds.sh` / `verify-crds-version-upgrade.sh` | CRD manifests match `config/crd/bases` and upgrades are compatible. | No — run directly | | ||
|
|
||
| Run `make verify` (or `make verify-scripts`) locally before sending a PR that touches | ||
| `api/`, `config/`, or `bindata/`; also run `hack/verify-crds*.sh` directly if operand CRDs changed, | ||
| since they are not invoked by any Makefile target in this repo. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Align bindata verification instructions.
Line 154 says bindata verification runs through make verify-scripts, but the verification table does not list a bindata verifier. Line 130 names a separate make verify-bindata target. Add the bindata verifier to the table, or instruct authors to run make verify-bindata directly.
Also applies to: 152-156
🤖 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/olm-packaging-guidelines.md` around lines 137 - 148, Align the “Verify
scripts” table and nearby instructions with the existing bindata verification
flow: add the bindata verifier and its `make verify-bindata` target to the
table, or explicitly instruct authors to run `make verify-bindata` directly
rather than implying `make verify-scripts` covers it. Preserve the existing CRD
guidance.
| make test-unit # go test ./... (excludes test/e2e, test/apis, test/utils) | ||
| make test-apis # envtest + ginkgo against api/operator/v1alpha1/tests fixtures | ||
| make test-e2e # ginkgo e2e suite against a live cluster (requires oc login) | ||
| make test-e2e-debug-cluster # dump operator/operand pod status, CSVs, CRDs on failure |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Fix the test-unit exclusion rule or document its actual behavior.
The supplied Makefile uses grep -vE 'test/[e2e|apis|utils]'. The square brackets define a character class, not directory alternatives. This can exclude unrelated paths such as test/integration and fail to exclude test/helpers.
Use grep -vE 'test/(e2e|apis|utils)(/|$)' if these are the intended directories. Then keep this guide consistent with the Makefile.
Also applies to: 48-48
🤖 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/testing-guidelines.md` around lines 27 - 30, Update the test-unit
documentation and corresponding Makefile exclusion rule to use directory
alternatives rather than a character class, matching only test/e2e, test/apis,
and test/utils with a following slash or end of path. Keep the listed test-unit
behavior consistent with the Makefile.
| ```go | ||
| //go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 -generate | ||
| //counterfeiter:generate -o fakes . CtrlClient | ||
| type CtrlClient interface { Get, List, StatusUpdate, Update, UpdateWithRetry, Create, Delete, Patch, Exists } | ||
| ``` |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
# Read the testing-guidelines.md file around lines 81-85
sed -n '75,90p' docs/testing-guidelines.mdRepository: openshift/cert-manager-operator
Length of output: 1948
Use valid Go syntax or mark this block as pseudocode.
The interface definition type CtrlClient interface { Get, List, StatusUpdate, ... } is not valid Go syntax. A Go interface requires method signatures with parameter and return types. Replace the abbreviated method list with actual method signatures, or if this is pseudocode, use a non-Go code fence or move it into prose.
🤖 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/testing-guidelines.md` around lines 81 - 85, Update the CtrlClient
example in the testing guidelines to use valid Go interface syntax with complete
method signatures and parameter/return types, or clearly mark the example as
pseudocode using a non-Go code fence or prose. Preserve the counterfeiter
directives only if the example remains intended to be executable Go.
Summary
AGENTS.mdandai-docs/(domain, architecture, ADRs, development/testing) for AI-assisted contribution to cert-manager-operator.docs/*-guidelines.mdcovering controllers, OLM packaging, FIPS, security, testing, and related topics.Test plan
AGENTS.mdintoai-docs/pkg/operator/starter.goSummary by CodeRabbit