Skip to content

Add hub-spoke trust config, root CA monitoring, and kubeconfig guidance#773

Open
sebrandon1 wants to merge 1 commit into
openshift-kni:mainfrom
sebrandon1:cert-manager-trust-store-ca-issuers
Open

Add hub-spoke trust config, root CA monitoring, and kubeconfig guidance#773
sebrandon1 wants to merge 1 commit into
openshift-kni:mainfrom
sebrandon1:cert-manager-trust-store-ca-issuers

Conversation

@sebrandon1

@sebrandon1 sebrandon1 commented May 19, 2026

Copy link
Copy Markdown
Contributor

Hub-spoke trust, root CA monitoring, and kubeconfig guidance

Companion RDS documentation MR: reference-design-specifications!192

Hub-spoke CA trust distribution (OCPBUGS-85774)

Adds KlusterletConfig and CA ConfigMap for distributing the cert-manager root CA to managed spokes. These are mandatory configuration with kube-compare validation templates. This ensures spokes trust the hub's root CA (CA:TRUE) instead of the auto-detected leaf serving cert (CA:FALSE), which:

  • Eliminates the chicken-and-egg problem where spokes can't receive ManifestWork updates after a hub CA replacement
  • Makes cert rotations seamless with no ManifestWork timing dependency
  • Supports both greenfield and brownfield hub-spoke deployments

New hub-only CRs:

  • certManagerHubCAConfigMap.yaml -- ConfigMap with the root CA PEM, labeled for the import controller
  • certManagerKlusterletConfig.yaml -- KlusterletConfig with UseCustomCABundles strategy

Kubeconfig trust update guidance (OCPBUGS-85776)

Adds documentation to all three profile READMEs warning that API server cert replacement with a non-publicly-trusted CA invalidates existing kubeconfigs, with step-by-step recovery procedure. Applies to any non-publicly-trusted issuer (including ACME with a private root CA).

Root CA expiration monitoring (OCPBUGS-85777)

Adds a hub-only ACM Policy that enforces a PrometheusRule alerting on root CA expiration:

  • Warning at 90 days before expiry
  • Critical at 30 days before expiry

Also extends the existing certManagerCertificatePolicy to monitor the cert-manager namespace alongside openshift-ingress and openshift-config.

Issuer guidance

ACME is the reference recommendation. Other issuer types (e.g., CA issuer for disconnected environments) are allowable — users may configure their own ClusterIssuer. Currently only ACME issuer is provided in the reference.

Testing

Validated on OCP 4.21.15 hub+spoke (SNO, ZTP-provisioned, bare metal) with cert-manager v1.19.0:

Removed from original PR

Based on reviewer feedback:

  • Self-signed ClusterIssuer -- removed per reviewer recommendation to use ACME with suitable PKI instead
  • Root CA Certificate bootstrap -- only needed for self-signed flow
  • CA ClusterIssuer reference CRs -- removed per feedback that reference CRs should only be provided for ACME; CA issuer is noted as allowable in documentation

@openshift-ci openshift-ci Bot requested review from irinamihai and sabbir-47 May 19, 2026 18:30
@openshift-ci

openshift-ci Bot commented May 19, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: sebrandon1
Once this PR has been reviewed and has the lgtm label, please assign marsik for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@coderabbitai

coderabbitai Bot commented May 19, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds optional internal-CA ClusterIssuer manifests across core/hub/ran, root-CA expiration monitoring (PrometheusRule + Placement + PlacementBinding), expands certificate policy namespace coverage to include cert-manager, updates READMEs with disconnected-PKI/kubeconfig trust guidance, and marks cert-manager CRs optional in compare/kustomize metadata.

Changes

Internal CA Issuers and Root CA Monitoring

Layer / File(s) Summary
Internal CA ClusterIssuer Manifests
telco-core/configuration/reference-crs-kube-compare/optional/cert-manager/certManagerClusterIssuerCA.yaml, telco-core/configuration/reference-crs/optional/cert-manager/certManagerClusterIssuerCA.yaml, telco-hub/configuration/reference-crs-kube-compare/optional/cert-manager/certManagerClusterIssuerCA.yaml, telco-hub/configuration/reference-crs/optional/cert-manager/certManagerClusterIssuerCA.yaml, telco-ran/configuration/source-crs/cert-manager/certManagerClusterIssuerCA.yaml
ClusterIssuer manifests (templated kube-compare variants, standard variants, and RAN ZTP-wave-annotated variant) referencing root-ca-secret as the CA source for internal PKI deployments.
Root CA Expiration Monitoring
telco-hub/configuration/reference-crs/optional/cert-manager/certManagerRootCAExpirationPolicy.yaml, telco-hub/configuration/reference-crs/optional/cert-manager/certManagerRootCAExpirationPolicyPlacement.yaml, telco-hub/configuration/reference-crs/optional/cert-manager/certManagerRootCAExpirationPolicyPlacementBinding.yaml
Adds an ACM Policy that enforces a PrometheusRule with two alerts (warning <90 days, critical <30 days) for the root CA expiration, plus Placement and PlacementBinding to target local clusters.
Certificate Policy Namespace Expansion
telco-hub/configuration/reference-crs-kube-compare/optional/cert-manager/certManagerCertificatePolicy.yaml, telco-hub/configuration/reference-crs/optional/cert-manager/certManagerCertificatePolicy.yaml
monitor-certificates CertificatePolicy templates now include the cert-manager namespace alongside openshift-ingress and openshift-config.

Deployment Documentation and Configuration Updates

Layer / File(s) Summary
Multi-Environment Issuer Deployment Guidance
telco-core/configuration/reference-crs/optional/cert-manager/README.md, telco-hub/configuration/reference-crs/optional/cert-manager/README.md, telco-ran/configuration/source-crs/cert-manager/README.md
README updates introduce a “Certificate Issuers” section describing ACME vs internal-CA options, provide Option A/Option B customization steps (including root-ca-secret creation and Certificate issuerRef changes), add kubeconfig trust recovery steps for API server cert replacement, and document monitoring/trust behaviors.
Configuration and Comparison Management
telco-core/configuration/reference-crs-kube-compare/compare_ignore, telco-core/configuration/reference-crs-kube-compare/optional/cert-manager/certManagerClusterIssuerCA.yaml, telco-hub/configuration/reference-crs-kube-compare/compare_ignore, telco-hub/configuration/reference-crs-kube-compare/metadata.yaml, telco-hub/configuration/reference-crs/optional/cert-manager/kustomization.yaml, telco-ran/configuration/kube-compare-reference/hack/compare_ignore
Kustomization adds commented optional CA/monitoring entries; compare-ignore files updated to exclude user-specific cert-manager CRs from kube-compare; metadata.fieldsToOmit adjusted to include a security.openshift.io annotation and remove obsolete oc-mirror fields.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly summarizes the main changes: adding hub-spoke trust configuration, root CA monitoring setup, and kubeconfig guidance for cert-manager deployments.
Description check ✅ Passed The PR description comprehensively details the changes: adds CA issuer support for disconnected environments, kubeconfig trust recovery procedures, root CA expiration monitoring, and hub-spoke trust documentation.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 8

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
telco-ran/configuration/source-crs/cert-manager/README.md (1)

66-75: ⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Add kubeconfig update step in Deployment Order for custom CA scenarios.

For Option B (self-signed CA) and Option C (existing internal CA), the kubeconfig must be updated to trust the new root CA before applying the APIServer configuration in step 7. Otherwise, administrators will be locked out and unable to run oc commands.

📝 Suggested addition to Deployment Order

Add a new step between 6 and 7:

 6. Wait for certificates to be issued and secrets created
+6a. **(For Option B & C only)** Extract the root CA certificate and update kubeconfig (see "Important: Kubeconfig Trust After API Server Cert Replacement" section below)
 7. Apply the APIServer and IngressController configurations
🤖 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 `@telco-ran/configuration/source-crs/cert-manager/README.md` around lines 66 -
75, Deployment order misses a necessary kubeconfig update for custom CA flows:
for Option B (self-signed CA) and Option C (existing internal CA) update the
cluster-admin kubeconfig to trust the new root CA before applying the APIServer
and IngressController configurations; insert a new step between steps 6 and 7 in
the "Deployment Order" that instructs operators to add the new CA certificate to
their local kubeconfig (or cluster trust store) and verify oc client
connectivity (e.g., confirm oc whoami and oc get pods) so administrators are not
locked out when APIServer changes are applied.
telco-hub/configuration/reference-crs/optional/cert-manager/README.md (1)

66-73: ⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Add kubeconfig update step in Deployment Order for custom CA scenarios.

For Option B (self-signed CA) and Option C (existing internal CA), the kubeconfig must be updated to trust the new root CA before applying the APIServer configuration in step 5. Otherwise, administrators will be locked out and unable to run oc commands.

📝 Suggested additions to Deployment Order

Add two missing steps:

 3. Deploy the ClusterIssuer
-4. Wait for certificates to be issued and secrets created
-5. Apply the APIServer and IngressController configurations
+4. Deploy the Certificate resources
+5. Wait for certificates to be issued and secrets created
+5a. **(For Option B & C only)** Extract the root CA certificate and update kubeconfig (see "Important: Kubeconfig Trust After API Server Cert Replacement" section below)
+6. Apply the APIServer and IngressController configurations
🤖 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 `@telco-hub/configuration/reference-crs/optional/cert-manager/README.md` around
lines 66 - 73, The Deployment Order is missing a kubeconfig update step for
Option B (self-signed CA) and Option C (existing internal CA); before applying
the APIServer configuration in step 5, add a step that updates administrators'
kubeconfig to trust the new root CA (e.g., import the new CA into the cluster
admin kubeconfig or distribute the updated kubeconfig) so `oc` commands continue
to work; reference the existing steps involving ClusterIssuer, APIServer and
IngressController and ensure the new kubeconfig-trust step appears after "Wait
for certificates to be issued and secrets created" and immediately before "Apply
the APIServer and IngressController configurations."
telco-core/configuration/reference-crs/optional/cert-manager/README.md (1)

66-74: ⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Add kubeconfig update step in Deployment Order for custom CA scenarios.

For Option B (self-signed CA) and Option C (existing internal CA), the kubeconfig must be updated to trust the new root CA before applying the APIServer configuration in step 6. Otherwise, administrators will be locked out and unable to run oc commands.

📝 Suggested addition to Deployment Order

Add a new step between 5 and 6:

 5. Wait for certificates to be issued and secrets created
+5a. **(For Option B & C only)** Extract the root CA certificate and update kubeconfig (see "Important: Kubeconfig Trust After API Server Cert Replacement" section below)
 6. Apply the APIServer and IngressController configurations
🤖 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 `@telco-core/configuration/reference-crs/optional/cert-manager/README.md`
around lines 66 - 74, Update the "Deployment Order" section in README.md to
insert a new step between steps 5 and 6 that instructs operators, for Option B
(self-signed CA) and Option C (existing internal CA), to update cluster admin
kubeconfigs to trust the new root CA before applying the APIServer and
IngressController configurations; reference the Deployment Order heading and
explicitly state that kubeconfigs must be updated so admins can run oc commands
and avoid being locked out, and link or mention the mechanism to update
kubeconfigs (e.g., adding the new CA to kubeconfig trust) so it's clear when and
why this step is required.
🤖 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
`@telco-core/configuration/reference-crs-kube-compare/optional/cert-manager/certManagerRootCACertificate.yaml`:
- Line 6: The namespace templating in certManagerRootCACertificate.yaml ("{{
.metadata.namespace | default \"cert-manager\" }}") must be removed so the
Secret is created in cert-manager where ClusterIssuer.spec.ca.secretName looks
for it; update certManagerRootCACertificate.yaml to use a fixed namespace value
of "cert-manager" (or remove the namespace field so it defaults to cert-manager)
so the Secret and ClusterIssuer.spec.ca.secretName resolve correctly.

In `@telco-core/configuration/reference-crs/optional/cert-manager/README.md`:
- Around line 83-111: Update the "Kubeconfig Trust After API Server Cert
Replacement" section to make clear this kubeconfig update is a required
deployment step for Option B (Self-signed CA) and Option C (Existing internal
CA) and must be done before applying the APIServer configuration; add a
prominent note/callout at the top of the section (referencing the section title
and the commands shown, e.g., the oc get secret root-ca-secret ... and oc config
set-cluster ... examples) stating that performing the kubeconfig update prior to
applying the APIServer config prevents being locked out and that running oc get
secret after APIServer replacement will fail with x509 errors.

In
`@telco-hub/configuration/reference-crs-kube-compare/optional/cert-manager/certManagerRootCACertificate.yaml`:
- Line 6: The template sets namespace via metadata.namespace which can place the
root CA Secret outside cert-manager's cluster-resource namespace so
ClusterIssuer.spec.ca.secretName cannot find it; change the resource so the
Secret is created in the cert-manager cluster-resource namespace by removing the
templated metadata.namespace and/or hardcoding namespace: "cert-manager" (or
using the cluster-resource namespace value) so ClusterIssuer.spec.ca.secretName
can always resolve the Secret.

In
`@telco-hub/configuration/reference-crs/optional/cert-manager/certManagerHubCATrustPolicy.yaml`:
- Around line 29-40: The lookup of the CA Secret is running on managed clusters;
change the template to fetch the Secret from the hub by wrapping the lookup in
hub delimiters and applying protect so the hub Secret is resolved server-side:
replace the current (lookup "v1" "Secret" "cert-manager" "root-ca-secret") usage
for variable $hubCA with a hub-scoped lookup (e.g. using the hub lookup ... hub
pattern) and apply | protect, and keep the subsequent conditional if $hubCA
unchanged so the rendered ConfigMap uses the hub CA bundle.

In
`@telco-hub/configuration/reference-crs/optional/cert-manager/certManagerRootCAExpirationPolicy.yaml`:
- Around line 45-55: Update CertManagerRootCAExpirationWarning and
CertManagerRootCAExpirationCritical to scope by both labels and prevent overlap:
add namespace filtering to the metric
certmanager_certificate_expiration_timestamp_seconds by including
namespace="{{...}}" in the expr for both alerts (since the metric has both name
and namespace labels) and change the warning alert expression
(CertManagerRootCAExpirationWarning) so it only matches certificates expiring in
>=30 days and <90 days (e.g., include an explicit lower bound like
certmanager_certificate_expiration_timestamp_seconds{name="root-ca",
namespace="{{...}}"} - time() >= 30 * 24 * 3600 && ... < 90 * 24 * 3600) while
CertManagerRootCAExpirationCritical remains <30d with the same namespace filter;
use the alert names CertManagerRootCAExpirationWarning and
CertManagerRootCAExpirationCritical to locate and update the rules.

In
`@telco-hub/configuration/reference-crs/optional/cert-manager/kustomization.yaml`:
- Around line 9-12: The comment in kustomization.yaml is misleading: instead of
instructing users to "uncomment one" when choosing alternative issuers, clarify
that the self-signed bootstrap flow requires enabling multiple manifests
together (certManagerClusterIssuerSelfSigned.yaml,
certManagerRootCACertificate.yaml, and certManagerClusterIssuerCA.yaml) while
the ACME issuer is an alternative; update the comment to instruct users to
uncomment either the ACME issuer block or the full set of self-signed manifests
(list them explicitly) so operators know to enable all required files for the
self-signed path and avoid partial/invalid configurations.

In `@telco-hub/configuration/reference-crs/optional/cert-manager/README.md`:
- Around line 82-110: Update the "Kubeconfig Trust After API Server Cert
Replacement" section to clarify this is a required deployment step for Option B
and C (not just a recovery step) by adding a prominent note before the steps
that says users must update kubeconfig before applying the APIServer
configuration; reference the section title and the commands shown (oc get secret
root-ca-secret -n cert-manager -o jsonpath=..., oc config set-cluster ...
--certificate-authority=... --embed-certs) and explicitly warn that applying the
APIServer config first will cause oc and API clients to fail with x509 errors
and prevent retrieving the secret.

In `@telco-ran/configuration/source-crs/cert-manager/README.md`:
- Around line 84-112: Add a clear pre-deployment callout to the "Important:
Kubeconfig Trust After API Server Cert Replacement" section stating that the
kubeconfig update is a required deployment step for Option B (Self-signed CA)
and Option C (Existing internal CA) and must be completed before applying the
APIServer configuration (referenced as "step 7 of the deployment order"); update
the title or first paragraph to explicitly warn users that applying the
APIServer config first will lock them out and cause the `oc get secret` command
shown in the example to fail with an x509 error, and ensure the note references
Options B & C and the APIServer deployment step so readers know this is not a
post-incident recovery but a mandatory pre-apply step.

---

Outside diff comments:
In `@telco-core/configuration/reference-crs/optional/cert-manager/README.md`:
- Around line 66-74: Update the "Deployment Order" section in README.md to
insert a new step between steps 5 and 6 that instructs operators, for Option B
(self-signed CA) and Option C (existing internal CA), to update cluster admin
kubeconfigs to trust the new root CA before applying the APIServer and
IngressController configurations; reference the Deployment Order heading and
explicitly state that kubeconfigs must be updated so admins can run oc commands
and avoid being locked out, and link or mention the mechanism to update
kubeconfigs (e.g., adding the new CA to kubeconfig trust) so it's clear when and
why this step is required.

In `@telco-hub/configuration/reference-crs/optional/cert-manager/README.md`:
- Around line 66-73: The Deployment Order is missing a kubeconfig update step
for Option B (self-signed CA) and Option C (existing internal CA); before
applying the APIServer configuration in step 5, add a step that updates
administrators' kubeconfig to trust the new root CA (e.g., import the new CA
into the cluster admin kubeconfig or distribute the updated kubeconfig) so `oc`
commands continue to work; reference the existing steps involving ClusterIssuer,
APIServer and IngressController and ensure the new kubeconfig-trust step appears
after "Wait for certificates to be issued and secrets created" and immediately
before "Apply the APIServer and IngressController configurations."

In `@telco-ran/configuration/source-crs/cert-manager/README.md`:
- Around line 66-75: Deployment order misses a necessary kubeconfig update for
custom CA flows: for Option B (self-signed CA) and Option C (existing internal
CA) update the cluster-admin kubeconfig to trust the new root CA before applying
the APIServer and IngressController configurations; insert a new step between
steps 6 and 7 in the "Deployment Order" that instructs operators to add the new
CA certificate to their local kubeconfig (or cluster trust store) and verify oc
client connectivity (e.g., confirm oc whoami and oc get pods) so administrators
are not locked out when APIServer changes are applied.
🪄 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: Organization UI

Review profile: CHILL

Plan: Enterprise

Run ID: 5a1209f0-501f-4c13-840f-2d96de51484a

📥 Commits

Reviewing files that changed from the base of the PR and between ef013bd and 6ffff6e.

📒 Files selected for processing (27)
  • telco-core/configuration/reference-crs-kube-compare/optional/cert-manager/certManagerClusterIssuerCA.yaml
  • telco-core/configuration/reference-crs-kube-compare/optional/cert-manager/certManagerClusterIssuerSelfSigned.yaml
  • telco-core/configuration/reference-crs-kube-compare/optional/cert-manager/certManagerRootCACertificate.yaml
  • telco-core/configuration/reference-crs/optional/cert-manager/README.md
  • telco-core/configuration/reference-crs/optional/cert-manager/certManagerClusterIssuerCA.yaml
  • telco-core/configuration/reference-crs/optional/cert-manager/certManagerClusterIssuerSelfSigned.yaml
  • telco-core/configuration/reference-crs/optional/cert-manager/certManagerRootCACertificate.yaml
  • telco-hub/configuration/reference-crs-kube-compare/optional/cert-manager/certManagerClusterIssuerCA.yaml
  • telco-hub/configuration/reference-crs-kube-compare/optional/cert-manager/certManagerClusterIssuerSelfSigned.yaml
  • telco-hub/configuration/reference-crs-kube-compare/optional/cert-manager/certManagerRootCACertificate.yaml
  • telco-hub/configuration/reference-crs/optional/cert-manager/README.md
  • telco-hub/configuration/reference-crs/optional/cert-manager/certManagerCertificatePolicy.yaml
  • telco-hub/configuration/reference-crs/optional/cert-manager/certManagerClusterIssuerCA.yaml
  • telco-hub/configuration/reference-crs/optional/cert-manager/certManagerClusterIssuerSelfSigned.yaml
  • telco-hub/configuration/reference-crs/optional/cert-manager/certManagerHubCATrustPolicy.yaml
  • telco-hub/configuration/reference-crs/optional/cert-manager/certManagerHubCATrustPolicyPlacement.yaml
  • telco-hub/configuration/reference-crs/optional/cert-manager/certManagerHubCATrustPolicyPlacementBinding.yaml
  • telco-hub/configuration/reference-crs/optional/cert-manager/certManagerRootCACertificate.yaml
  • telco-hub/configuration/reference-crs/optional/cert-manager/certManagerRootCAExpirationPolicy.yaml
  • telco-hub/configuration/reference-crs/optional/cert-manager/certManagerRootCAExpirationPolicyPlacement.yaml
  • telco-hub/configuration/reference-crs/optional/cert-manager/certManagerRootCAExpirationPolicyPlacementBinding.yaml
  • telco-hub/configuration/reference-crs/optional/cert-manager/certManagerSpokeCATrustCM.yaml
  • telco-hub/configuration/reference-crs/optional/cert-manager/kustomization.yaml
  • telco-ran/configuration/source-crs/cert-manager/README.md
  • telco-ran/configuration/source-crs/cert-manager/certManagerClusterIssuerCA.yaml
  • telco-ran/configuration/source-crs/cert-manager/certManagerClusterIssuerSelfSigned.yaml
  • telco-ran/configuration/source-crs/cert-manager/certManagerRootCACertificate.yaml

Comment thread telco-hub/configuration/reference-crs/optional/cert-manager/kustomization.yaml Outdated
Comment thread telco-ran/configuration/source-crs/cert-manager/README.md Outdated
@sebrandon1 sebrandon1 force-pushed the cert-manager-trust-store-ca-issuers branch 3 times, most recently from 30229a7 to 29b4fd7 Compare May 19, 2026 19:18
@sebrandon1 sebrandon1 changed the title Add self-signed/CA issuers, hub-spoke CA trust, and root CA monitoring Add self-signed/CA issuers, spoke CA trust, and root CA monitoring May 20, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (1)
telco-ran/configuration/source-crs/cert-manager/certManagerRootCACertificate.yaml (1)

6-7: ⚡ Quick win

Use a later ZTP deploy wave for root-ca than selfsigned-issuer.

root-ca currently uses the same wave ("10") as its issuer, which can make first-pass ordering non-deterministic. Set this Certificate to a later wave (for example "11") so issuer creation is deterministically sequenced before issuance.

Proposed change
 metadata:
   name: root-ca
   namespace: cert-manager
   annotations:
-    ran.openshift.io/ztp-deploy-wave: "10"
+    ran.openshift.io/ztp-deploy-wave: "11"
🤖 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
`@telco-ran/configuration/source-crs/cert-manager/certManagerRootCACertificate.yaml`
around lines 6 - 7, The Certificate resource 'root-ca' is annotated with the
same ZTP deploy wave ("10") as its issuer 'selfsigned-issuer', which can cause
non-deterministic ordering; update the annotation on the 'root-ca' Certificate
(certManagerRootCACertificate.yaml, resource name root-ca) to use a later deploy
wave (e.g., change ran.openshift.io/ztp-deploy-wave from "10" to "11") so the
issuer (selfsigned-issuer) is created before the root CA is issued.
🤖 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 `@telco-core/configuration/reference-crs/optional/cert-manager/README.md`:
- Around line 53-56: The docs use inconsistent secret keys: they instruct
creating a Secret named `root-ca-secret` with `tls.crt`/`tls.key` but later
extraction commands expect `ca.crt`; update the README to use the same key name
everywhere (choose either `tls.crt` or `ca.crt`) and make the `root-ca-secret`
creation, the `certManagerClusterIssuerCA.yaml` usage, and the Certificate
`issuerRef` examples all reference that single consistent secret key name so the
kubeconfig/root CA extraction step will succeed in the internal/disconnected CA
flow.

In `@telco-hub/configuration/reference-crs/optional/cert-manager/README.md`:
- Around line 53-56: The README has a key mismatch: the secret is documented as
named root-ca-secret containing tls.crt/tls.key but the kubeconfig trust command
reads ca.crt; update the docs so the secret key names match the command (either
change the secret description to use ca.crt/ca.key or change the command to read
tls.crt/tls.key) and ensure references to root-ca-secret, cert-manager
namespace, certManagerClusterIssuerCA.yaml, and issuerRef: ca-issuer are
consistent across the file (also apply the same fix to the other occurrence
around lines 97-100).

In `@telco-ran/configuration/source-crs/cert-manager/README.md`:
- Around line 53-56: The README is inconsistent: Option C tells users to create
a Secret named root-ca-secret with tls.crt/tls.key, but the kubeconfig
extraction step expects ca.crt and fails to produce /tmp/root-ca.crt; update the
docs so both places use the same key name (either tls.crt or ca.crt). Search for
root-ca-secret, certManagerClusterIssuerCA.yaml, ca-issuer and the kubeconfig
extraction that writes /tmp/root-ca.crt, then change the extraction command or
the Secret key name so they match (and update any references to tls.key/tls.crt
or ca.crt/ca.key accordingly). Ensure the README consistently documents the
chosen key names across Option C and the kubeconfig step.

---

Nitpick comments:
In
`@telco-ran/configuration/source-crs/cert-manager/certManagerRootCACertificate.yaml`:
- Around line 6-7: The Certificate resource 'root-ca' is annotated with the same
ZTP deploy wave ("10") as its issuer 'selfsigned-issuer', which can cause
non-deterministic ordering; update the annotation on the 'root-ca' Certificate
(certManagerRootCACertificate.yaml, resource name root-ca) to use a later deploy
wave (e.g., change ran.openshift.io/ztp-deploy-wave from "10" to "11") so the
issuer (selfsigned-issuer) is created before the root CA is issued.
🪄 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: Organization UI

Review profile: CHILL

Plan: Enterprise

Run ID: b5c17fd7-a763-4751-a732-766cb671e8ab

📥 Commits

Reviewing files that changed from the base of the PR and between 6ffff6e and a0c7607.

📒 Files selected for processing (27)
  • telco-core/configuration/reference-crs-kube-compare/optional/cert-manager/certManagerClusterIssuerCA.yaml
  • telco-core/configuration/reference-crs-kube-compare/optional/cert-manager/certManagerClusterIssuerSelfSigned.yaml
  • telco-core/configuration/reference-crs-kube-compare/optional/cert-manager/certManagerRootCACertificate.yaml
  • telco-core/configuration/reference-crs/optional/cert-manager/README.md
  • telco-core/configuration/reference-crs/optional/cert-manager/certManagerClusterIssuerCA.yaml
  • telco-core/configuration/reference-crs/optional/cert-manager/certManagerClusterIssuerSelfSigned.yaml
  • telco-core/configuration/reference-crs/optional/cert-manager/certManagerRootCACertificate.yaml
  • telco-hub/configuration/reference-crs-kube-compare/compare_ignore
  • telco-hub/configuration/reference-crs-kube-compare/optional/cert-manager/certManagerCertificatePolicy.yaml
  • telco-hub/configuration/reference-crs-kube-compare/optional/cert-manager/certManagerClusterIssuerCA.yaml
  • telco-hub/configuration/reference-crs-kube-compare/optional/cert-manager/certManagerClusterIssuerSelfSigned.yaml
  • telco-hub/configuration/reference-crs-kube-compare/optional/cert-manager/certManagerRootCACertificate.yaml
  • telco-hub/configuration/reference-crs/optional/cert-manager/README.md
  • telco-hub/configuration/reference-crs/optional/cert-manager/certManagerCertificatePolicy.yaml
  • telco-hub/configuration/reference-crs/optional/cert-manager/certManagerClusterIssuerCA.yaml
  • telco-hub/configuration/reference-crs/optional/cert-manager/certManagerClusterIssuerSelfSigned.yaml
  • telco-hub/configuration/reference-crs/optional/cert-manager/certManagerRootCACertificate.yaml
  • telco-hub/configuration/reference-crs/optional/cert-manager/certManagerRootCAExpirationPolicy.yaml
  • telco-hub/configuration/reference-crs/optional/cert-manager/certManagerRootCAExpirationPolicyPlacement.yaml
  • telco-hub/configuration/reference-crs/optional/cert-manager/certManagerRootCAExpirationPolicyPlacementBinding.yaml
  • telco-hub/configuration/reference-crs/optional/cert-manager/certManagerSpokeCATrustCM.yaml
  • telco-hub/configuration/reference-crs/optional/cert-manager/kustomization.yaml
  • telco-ran/configuration/kube-compare-reference/hack/compare_ignore
  • telco-ran/configuration/source-crs/cert-manager/README.md
  • telco-ran/configuration/source-crs/cert-manager/certManagerClusterIssuerCA.yaml
  • telco-ran/configuration/source-crs/cert-manager/certManagerClusterIssuerSelfSigned.yaml
  • telco-ran/configuration/source-crs/cert-manager/certManagerRootCACertificate.yaml
✅ Files skipped from review due to trivial changes (4)
  • telco-hub/configuration/reference-crs-kube-compare/compare_ignore
  • telco-hub/configuration/reference-crs/optional/cert-manager/certManagerRootCACertificate.yaml
  • telco-ran/configuration/kube-compare-reference/hack/compare_ignore
  • telco-hub/configuration/reference-crs/optional/cert-manager/kustomization.yaml

Comment thread telco-core/configuration/reference-crs/optional/cert-manager/README.md Outdated
Comment thread telco-hub/configuration/reference-crs/optional/cert-manager/README.md Outdated
Comment thread telco-ran/configuration/source-crs/cert-manager/README.md Outdated
@sebrandon1 sebrandon1 force-pushed the cert-manager-trust-store-ca-issuers branch from a0c7607 to fdcccb5 Compare May 20, 2026 21:08
@imiller0

Copy link
Copy Markdown
Collaborator

I don't think we should be adding self-signed certificates to the reference configuration. ACME can be used in disconnected environments with suitable issuer/pki

@sebrandon1 sebrandon1 force-pushed the cert-manager-trust-store-ca-issuers branch from fdcccb5 to 4710a27 Compare May 27, 2026 16:11
@sebrandon1 sebrandon1 changed the title Add self-signed/CA issuers, spoke CA trust, and root CA monitoring Add CA issuer, root CA monitoring, and hub-spoke trust docs May 27, 2026
@sebrandon1 sebrandon1 force-pushed the cert-manager-trust-store-ca-issuers branch 4 times, most recently from 4930307 to 644fd05 Compare May 28, 2026 19:19
@sebrandon1 sebrandon1 changed the title Add CA issuer, root CA monitoring, and hub-spoke trust docs Add CA issuer, hub-spoke trust config, and root CA monitoring May 28, 2026
@sebrandon1 sebrandon1 changed the title Add CA issuer, hub-spoke trust config, and root CA monitoring Add hub-spoke trust config, root CA monitoring, and kubeconfig guidance Jun 1, 2026
@sebrandon1 sebrandon1 force-pushed the cert-manager-trust-store-ca-issuers branch from b846cb2 to 40129b5 Compare June 1, 2026 16:33
@openshift-ci openshift-ci Bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 1, 2026
@sebrandon1 sebrandon1 force-pushed the cert-manager-trust-store-ca-issuers branch from 40129b5 to f56c0a8 Compare June 1, 2026 16:34
@openshift-ci openshift-ci Bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 1, 2026
Comment thread telco-core/configuration/reference-crs/optional/cert-manager/README.md Outdated
Comment thread telco-core/configuration/reference-crs/optional/cert-manager/README.md Outdated
Comment thread telco-core/configuration/reference-crs/optional/cert-manager/README.md Outdated
Comment thread telco-core/configuration/reference-crs/optional/cert-manager/README.md Outdated
Comment thread telco-hub/configuration/reference-crs-kube-compare/compare_ignore Outdated
Comment thread telco-hub/configuration/reference-crs/optional/cert-manager/README.md Outdated
Comment thread telco-hub/configuration/reference-crs/optional/cert-manager/README.md Outdated
Comment thread telco-hub/configuration/reference-crs/optional/cert-manager/README.md Outdated
Comment thread telco-ran/configuration/source-crs/cert-manager/README.md Outdated
Comment thread telco-ran/configuration/source-crs/cert-manager/README.md Outdated
@sebrandon1 sebrandon1 force-pushed the cert-manager-trust-store-ca-issuers branch 4 times, most recently from 31dd12b to b9664ab Compare June 2, 2026 15:06
@sebrandon1 sebrandon1 force-pushed the cert-manager-trust-store-ca-issuers branch 2 times, most recently from 191a1bd to 4089b71 Compare June 2, 2026 15:21
Comment thread telco-hub/configuration/reference-crs/optional/cert-manager/kustomization.yaml Outdated
@sebrandon1 sebrandon1 force-pushed the cert-manager-trust-store-ca-issuers branch 3 times, most recently from 3792e2a to 95b799b Compare June 3, 2026 17:18
@sebrandon1 sebrandon1 force-pushed the cert-manager-trust-store-ca-issuers branch from 95b799b to 8ae3687 Compare June 5, 2026 20:42
@openshift-ci openshift-ci Bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 5, 2026
@sebrandon1 sebrandon1 force-pushed the cert-manager-trust-store-ca-issuers branch from 8ae3687 to 60dd692 Compare June 5, 2026 20:46
@openshift-ci openshift-ci Bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 5, 2026
Hub-spoke CA trust distribution via KlusterletConfig and CA ConfigMap,
made mandatory with kube-compare validation templates.
Root CA expiration monitoring via ACM Policy with PrometheusRule alerts.
CertificatePolicy expanded to monitor cert-manager namespace.
Kubeconfig trust documentation added to all profile READMEs, applicable
to any non-publicly-trusted issuer.

ACME is the reference recommendation. Other issuer types are allowable
but only ACME is provided in the reference.
@sebrandon1 sebrandon1 force-pushed the cert-manager-trust-store-ca-issuers branch from 60dd692 to 7b5e6f7 Compare June 8, 2026 15:45
sebrandon1 added a commit to sebrandon1/telco-reference that referenced this pull request Jun 15, 2026
Cert-manager was removed from RAN for 4.22 GA (PR openshift-kni#801, OCPBUGS-86666).
Now that branches have been cut, restore it to main for IBU testing.

Restored files:
- 9 source CRs + README in source-crs/cert-manager/
- 3 kube-compare templates in kube-compare-reference/cert-manager/
- metadata.yaml, compare_ignore, default_value.yaml, acm-common-ranGen.yaml

README updated with ACME-only issuer wording and kubeconfig trust guidance
aligned with hub cert-manager improvements (PR openshift-kni#773).
sebrandon1 added a commit to sebrandon1/telco-reference that referenced this pull request Jun 15, 2026
Cert-manager was removed from RAN for 4.22 GA (PR openshift-kni#801, OCPBUGS-86666).
Now that branches have been cut, restore it to main for IBU testing.

Restored files:
- 9 source CRs + README in source-crs/cert-manager/
- 3 kube-compare templates in kube-compare-reference/cert-manager/
- metadata.yaml, compare_ignore, default_value.yaml, acm-common-ranGen.yaml

README updated with ACME-only issuer wording and kubeconfig trust guidance
aligned with hub cert-manager improvements (PR openshift-kni#773).
sebrandon1 added a commit to sebrandon1/telco-reference that referenced this pull request Jun 15, 2026
Cert-manager was removed from RAN for 4.22 GA (PR openshift-kni#801, OCPBUGS-86666).
Now that branches have been cut, restore it to main for IBU testing.

Restored files:
- 9 source CRs + README in source-crs/cert-manager/
- 3 kube-compare templates in kube-compare-reference/cert-manager/
- metadata.yaml, compare_ignore, default_value.yaml, acm-common-ranGen.yaml

README updated with ACME-only issuer wording and kubeconfig trust guidance
aligned with hub cert-manager improvements (PR openshift-kni#773).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants