From 226dc7c76eb87c1e86d4fa298e679e621223349d Mon Sep 17 00:00:00 2001 From: Simon Lauger Date: Thu, 19 Mar 2026 21:27:36 +0100 Subject: [PATCH 01/14] feat: support IMAGE_TAG for install and stack targets Allow 'make install IMAGE_TAG=' and 'make stack IMAGE_TAG=' to deploy operator and stack with a specific remote image tag (e.g. a CI-built commit SHA). When IMAGE_TAG is set, HELM_SET and STACK_HELM_SET are automatically populated with repository, tag, and pullPolicy=Always. Existing local-install/local-stack targets are unaffected as they override HELM_SET/STACK_HELM_SET via target-specific variables. --- Makefile | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index 5704338..b269113 100644 --- a/Makefile +++ b/Makefile @@ -4,6 +4,7 @@ OPENVOX_CODE_IMG ?= ghcr.io/slauger/openvox-code:latest OPENVOX_AGENT_IMG ?= ghcr.io/slauger/openvox-agent:latest OPENVOX_MOCK_IMG ?= ghcr.io/slauger/openvox-mock:latest NAMESPACE ?= openvox-system +IMAGE_REGISTRY ?= ghcr.io/slauger CONTAINER_TOOL ?= $(shell which podman 2>/dev/null || which docker 2>/dev/null) CONTROLLER_GEN = go tool controller-gen GOVULNCHECK = go tool govulncheck @@ -80,8 +81,15 @@ STACK_VALUES ?= charts/openvox-stack/ci/single-node-values.yaml ##@ Deployment +# When IMAGE_TAG is set (e.g. make install IMAGE_TAG=487ea36), configure +# helm to pull that specific image from the registry. +ifdef IMAGE_TAG +HELM_SET ?= --set image.repository=$(IMAGE_REGISTRY)/openvox-operator --set image.tag=$(IMAGE_TAG) --set image.pullPolicy=Always +STACK_HELM_SET ?= --set config.image.repository=$(IMAGE_REGISTRY)/openvox-server --set config.image.tag=$(IMAGE_TAG) --set config.image.pullPolicy=Always +endif + .PHONY: install -install: manifests ## Install operator via Helm with default images. +install: manifests ## Install operator via Helm (supports IMAGE_TAG=). helm upgrade --install openvox-operator charts/openvox-operator \ --namespace $(NAMESPACE) --create-namespace $(HELM_SET) @@ -90,7 +98,7 @@ local-install: HELM_SET := --set image.tag=$(LOCAL_TAG) --set image.pullPolicy=N local-install: install ## Install operator via Helm with local images (no build). .PHONY: stack -stack: ## Deploy openvox-stack via Helm with default images. +stack: ## Deploy openvox-stack via Helm (supports IMAGE_TAG=). helm upgrade --install openvox-stack charts/openvox-stack \ --namespace $(STACK_NAMESPACE) --create-namespace \ --values $(STACK_VALUES) $(STACK_HELM_SET) @@ -156,7 +164,6 @@ ci: lint vet test check-manifests vulncheck helm-lint ## Run all CI checks local ##@ E2E -IMAGE_REGISTRY ?= ghcr.io/slauger IMAGE_TAG ?= $(LOCAL_TAG) E2E_CHAINSAW = IMAGE_TAG=$(IMAGE_TAG) IMAGE_REGISTRY=$(IMAGE_REGISTRY) $(CHAINSAW) test --config tests/e2e/chainsaw-config.yaml From 1dd47cd38955263e352393872dc9fb5009952549 Mon Sep 17 00:00:00 2001 From: Simon Lauger Date: Thu, 19 Mar 2026 21:31:34 +0100 Subject: [PATCH 02/14] feat: use multi-server layout as default stack values Change the default stack values to include a dedicated CA server and separate compiler servers (2 replicas), which better reflects a realistic production-like setup. Point STACK_VALUES to the chart defaults instead of the CI single-node values. --- Makefile | 2 +- charts/openvox-stack/values.yaml | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index b269113..261b7f2 100644 --- a/Makefile +++ b/Makefile @@ -77,7 +77,7 @@ local-deploy: local-build local-install ## Build images and deploy operator via @echo "Operator deployed with openvox-operator:$(LOCAL_TAG)" STACK_NAMESPACE ?= openvox -STACK_VALUES ?= charts/openvox-stack/ci/single-node-values.yaml +STACK_VALUES ?= charts/openvox-stack/values.yaml ##@ Deployment diff --git a/charts/openvox-stack/values.yaml b/charts/openvox-stack/values.yaml index b959edb..1222373 100644 --- a/charts/openvox-stack/values.yaml +++ b/charts/openvox-stack/values.yaml @@ -75,6 +75,20 @@ servers: memory: 1Gi limits: memory: 2Gi + - name: server + ca: false + server: true + poolRefs: [server] + certificate: + certname: server + dnsAltNames: [] + replicas: 2 + resources: + requests: + cpu: 500m + memory: 1Gi + limits: + memory: 2Gi gateway: name: "" # shared Gateway resource name From f8f59f2b1129329000ad34a0e42e5ed9bb8bad5f Mon Sep 17 00:00:00 2001 From: Simon Lauger Date: Mon, 16 Mar 2026 23:46:43 +0100 Subject: [PATCH 03/14] feat: add external CA support (#54) Add spec.external to CertificateAuthority, allowing the operator to delegate CSR signing and CRL fetching to an external Puppet/OpenVox CA running outside the cluster with optional mTLS client authentication. --- api/v1alpha1/certificateauthority_types.go | 36 ++++- api/v1alpha1/zz_generated.deepcopy.go | 20 +++ ....voxpupuli.org_certificateauthorities.yaml | 36 +++++ ....voxpupuli.org_certificateauthorities.yaml | 36 +++++ .../certificateauthority-external.yaml | 41 ++++++ docs/guides/ca-import.md | 135 ++++++++++++++++++ internal/controller/certificate_controller.go | 26 ++-- internal/controller/certificate_signing.go | 88 +++++++++--- .../certificateauthority_controller.go | 62 ++++++++ .../controller/certificateauthority_crl.go | 42 ++++-- internal/controller/constants.go | 16 +-- internal/controller/testutil_test.go | 1 - 12 files changed, 489 insertions(+), 50 deletions(-) create mode 100644 config/samples/certificateauthority-external.yaml create mode 100644 docs/guides/ca-import.md diff --git a/api/v1alpha1/certificateauthority_types.go b/api/v1alpha1/certificateauthority_types.go index a562865..2a47124 100644 --- a/api/v1alpha1/certificateauthority_types.go +++ b/api/v1alpha1/certificateauthority_types.go @@ -31,7 +31,33 @@ type CertificateAuthorityList struct { Items []CertificateAuthority `json:"items"` } +// ExternalCASpec configures an external CA running outside the cluster. +// When set, the operator delegates CSR signing and CRL fetching to the external CA URL +// instead of managing its own CA infrastructure. +type ExternalCASpec struct { + // URL is the base URL of the external Puppet/OpenVox CA (e.g. "https://puppet-ca.example.com:8140"). + // +kubebuilder:validation:Pattern=`^https?://` + URL string `json:"url"` + + // CASecretRef references a Secret containing the CA certificate in key "ca_crt.pem". + // Used to verify the external CA's TLS certificate. + // +optional + CASecretRef string `json:"caSecretRef,omitempty"` + + // TLSSecretRef references a Secret containing "tls.crt" and "tls.key" for mTLS + // client authentication against the external CA. + // +optional + TLSSecretRef string `json:"tlsSecretRef,omitempty"` + + // InsecureSkipVerify disables TLS certificate verification for the external CA. + // Only use this for testing or when the CA uses a self-signed certificate + // and no CASecretRef is provided. + // +optional + InsecureSkipVerify bool `json:"insecureSkipVerify,omitempty"` +} + // CertificateAuthoritySpec defines the desired state of CertificateAuthority. +// +kubebuilder:validation:XValidation:rule="!(has(self.external) && has(self.storage) && self.storage.size != ” && self.storage.size != '1Gi')",message="external and custom storage are mutually exclusive" type CertificateAuthoritySpec struct { // TTL is the CA certificate TTL as a duration string. // Supported units: s (seconds), m (minutes), h (hours), d (days), y (years). @@ -87,16 +113,24 @@ type CertificateAuthoritySpec struct { // IntermediateCA configures an intermediate CA setup. // +optional IntermediateCA IntermediateCASpec `json:"intermediateCA,omitempty"` + + // External configures an external CA running outside the cluster. + // When set, the operator skips PVC/Job-based CA setup and delegates + // CSR signing and CRL fetching to the external CA URL. + // Mutually exclusive with custom storage settings. + // +optional + External *ExternalCASpec `json:"external,omitempty"` } // CertificateAuthorityPhase represents the current lifecycle phase of a CertificateAuthority. -// +kubebuilder:validation:Enum=Pending;Initializing;Ready;Error +// +kubebuilder:validation:Enum=Pending;Initializing;Ready;External;Error type CertificateAuthorityPhase string const ( CertificateAuthorityPhasePending CertificateAuthorityPhase = "Pending" CertificateAuthorityPhaseInitializing CertificateAuthorityPhase = "Initializing" CertificateAuthorityPhaseReady CertificateAuthorityPhase = "Ready" + CertificateAuthorityPhaseExternal CertificateAuthorityPhase = "External" CertificateAuthorityPhaseError CertificateAuthorityPhase = "Error" ) diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 90a9259..fcbccf7 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -210,6 +210,11 @@ func (in *CertificateAuthoritySpec) DeepCopyInto(out *CertificateAuthoritySpec) out.Storage = in.Storage in.Resources.DeepCopyInto(&out.Resources) out.IntermediateCA = in.IntermediateCA + if in.External != nil { + in, out := &in.External, &out.External + *out = new(ExternalCASpec) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new CertificateAuthoritySpec. @@ -471,6 +476,21 @@ func (in *ConfigStatus) DeepCopy() *ConfigStatus { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ExternalCASpec) DeepCopyInto(out *ExternalCASpec) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ExternalCASpec. +func (in *ExternalCASpec) DeepCopy() *ExternalCASpec { + if in == nil { + return nil + } + out := new(ExternalCASpec) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *GatewayReference) DeepCopyInto(out *GatewayReference) { *out = *in diff --git a/charts/openvox-operator/crds/openvox.voxpupuli.org_certificateauthorities.yaml b/charts/openvox-operator/crds/openvox.voxpupuli.org_certificateauthorities.yaml index 0269dc0..e59c189 100644 --- a/charts/openvox-operator/crds/openvox.voxpupuli.org_certificateauthorities.yaml +++ b/charts/openvox-operator/crds/openvox.voxpupuli.org_certificateauthorities.yaml @@ -87,6 +87,37 @@ spec: description: EnableInfraCRL enables infrastructure CRL for compile server revocation. type: boolean + external: + description: |- + External configures an external CA running outside the cluster. + When set, the operator skips PVC/Job-based CA setup and delegates + CSR signing and CRL fetching to the external CA URL. + Mutually exclusive with custom storage settings. + properties: + caSecretRef: + description: |- + CASecretRef references a Secret containing the CA certificate in key "ca_crt.pem". + Used to verify the external CA's TLS certificate. + type: string + insecureSkipVerify: + description: |- + InsecureSkipVerify disables TLS certificate verification for the external CA. + Only use this for testing or when the CA uses a self-signed certificate + and no CASecretRef is provided. + type: boolean + tlsSecretRef: + description: |- + TLSSecretRef references a Secret containing "tls.crt" and "tls.key" for mTLS + client authentication against the external CA. + type: string + url: + description: URL is the base URL of the external Puppet/OpenVox + CA (e.g. "https://puppet-ca.example.com:8140"). + pattern: ^https?:// + type: string + required: + - url + type: object intermediateCA: description: IntermediateCA configures an intermediate CA setup. properties: @@ -183,6 +214,10 @@ spec: Plain numbers are interpreted as seconds for backwards compatibility. type: string type: object + x-kubernetes-validations: + - message: external and custom storage are mutually exclusive + rule: '!(has(self.external) && has(self.storage) && self.storage.size + != ” && self.storage.size != ''1Gi'')' status: description: CertificateAuthorityStatus defines the observed state of CertificateAuthority. @@ -259,6 +294,7 @@ spec: - Pending - Initializing - Ready + - External - Error type: string type: object diff --git a/config/crd/bases/openvox.voxpupuli.org_certificateauthorities.yaml b/config/crd/bases/openvox.voxpupuli.org_certificateauthorities.yaml index 0269dc0..e59c189 100644 --- a/config/crd/bases/openvox.voxpupuli.org_certificateauthorities.yaml +++ b/config/crd/bases/openvox.voxpupuli.org_certificateauthorities.yaml @@ -87,6 +87,37 @@ spec: description: EnableInfraCRL enables infrastructure CRL for compile server revocation. type: boolean + external: + description: |- + External configures an external CA running outside the cluster. + When set, the operator skips PVC/Job-based CA setup and delegates + CSR signing and CRL fetching to the external CA URL. + Mutually exclusive with custom storage settings. + properties: + caSecretRef: + description: |- + CASecretRef references a Secret containing the CA certificate in key "ca_crt.pem". + Used to verify the external CA's TLS certificate. + type: string + insecureSkipVerify: + description: |- + InsecureSkipVerify disables TLS certificate verification for the external CA. + Only use this for testing or when the CA uses a self-signed certificate + and no CASecretRef is provided. + type: boolean + tlsSecretRef: + description: |- + TLSSecretRef references a Secret containing "tls.crt" and "tls.key" for mTLS + client authentication against the external CA. + type: string + url: + description: URL is the base URL of the external Puppet/OpenVox + CA (e.g. "https://puppet-ca.example.com:8140"). + pattern: ^https?:// + type: string + required: + - url + type: object intermediateCA: description: IntermediateCA configures an intermediate CA setup. properties: @@ -183,6 +214,10 @@ spec: Plain numbers are interpreted as seconds for backwards compatibility. type: string type: object + x-kubernetes-validations: + - message: external and custom storage are mutually exclusive + rule: '!(has(self.external) && has(self.storage) && self.storage.size + != ” && self.storage.size != ''1Gi'')' status: description: CertificateAuthorityStatus defines the observed state of CertificateAuthority. @@ -259,6 +294,7 @@ spec: - Pending - Initializing - Ready + - External - Error type: string type: object diff --git a/config/samples/certificateauthority-external.yaml b/config/samples/certificateauthority-external.yaml new file mode 100644 index 0000000..2dfb230 --- /dev/null +++ b/config/samples/certificateauthority-external.yaml @@ -0,0 +1,41 @@ +# External CertificateAuthority example +# +# This example connects to an existing Puppet/OpenVox CA running outside the cluster. +# The operator delegates CSR signing and CRL fetching to the external CA URL +# instead of managing its own CA infrastructure (no PVC, no setup Job). +# +# Prerequisites: +# 1. Create the CA certificate Secret: +# kubectl create secret generic external-ca-cert \ +# --from-file=ca_crt.pem=/path/to/ca_crt.pem +# +# 2. (Optional) For mTLS, create a TLS Secret with client certificate: +# kubectl create secret generic external-ca-tls \ +# --from-file=tls.crt=/path/to/client.pem \ +# --from-file=tls.key=/path/to/client-key.pem +--- +apiVersion: openvox.voxpupuli.org/v1alpha1 +kind: CertificateAuthority +metadata: + name: external-ca +spec: + # CA policy settings still apply to certificates issued via external CA + allowSubjectAltNames: true + allowAuthorizationExtensions: true + enableInfraCRL: true + crlRefreshInterval: 5m + + external: + # Base URL of the external Puppet/OpenVox CA + url: https://puppet-ca.example.com:8140 + + # Secret containing the CA certificate (key: ca_crt.pem) + # Used for TLS verification of the external CA + caSecretRef: external-ca-cert + + # Secret containing client certificate and key for mTLS (keys: tls.crt, tls.key) + # Required if the external CA enforces client certificate authentication + tlsSecretRef: external-ca-tls + + # Set to true to skip TLS verification (not recommended for production) + # insecureSkipVerify: false diff --git a/docs/guides/ca-import.md b/docs/guides/ca-import.md new file mode 100644 index 0000000..e1447d2 --- /dev/null +++ b/docs/guides/ca-import.md @@ -0,0 +1,135 @@ +# Importing or Connecting an External CA + +This guide covers two approaches for using an existing Puppet or OpenVox CA with the operator: + +1. **CA Import** -- copy existing CA data into the operator-managed PVC (one-time migration) +2. **External CA** -- point the operator at a running CA outside the cluster (ongoing delegation) + +## Option A: CA Import (One-Time Migration) + +If you have an existing CA and want the operator to manage it going forward, you can import the CA data into the operator's PVC. + +### Prerequisites + +- An existing Puppet/OpenVox CA with `ca_crt.pem`, `ca_key.pem`, and `ca_crl.pem` +- The operator installed in the cluster + +### Steps + +1. Create the `CertificateAuthority` resource as usual (without `spec.external`): + + ```yaml + apiVersion: openvox.voxpupuli.org/v1alpha1 + kind: CertificateAuthority + metadata: + name: production-ca + spec: + ttl: 5y + storage: + size: 1Gi + ``` + +2. Wait for the PVC to be created, then copy your CA data into it: + + ```bash + # Find the PVC + kubectl get pvc -l openvox.voxpupuli.org/certificate-authority=production-ca + + # Create a temporary pod to copy data + kubectl run ca-import --image=busybox --restart=Never \ + --overrides='{ + "spec": { + "containers": [{ + "name": "ca-import", + "image": "busybox", + "command": ["sleep", "3600"], + "volumeMounts": [{ + "name": "ca-data", + "mountPath": "/ca" + }] + }], + "volumes": [{ + "name": "ca-data", + "persistentVolumeClaim": { + "claimName": "production-ca-data" + } + }] + } + }' + + # Copy CA files + kubectl cp ca_crt.pem ca-import:/ca/ca_crt.pem + kubectl cp ca_key.pem ca-import:/ca/ca_key.pem + kubectl cp ca_crl.pem ca-import:/ca/ca_crl.pem + + # Clean up + kubectl delete pod ca-import + ``` + +3. The CA setup Job will detect existing data and skip regeneration. The operator will create the corresponding Secrets and transition to `Ready`. + +## Option B: External CA (Ongoing Delegation) + +If you have a Puppet/OpenVox CA running outside the cluster and want to keep using it, configure `spec.external` on the `CertificateAuthority` resource. The operator will delegate CSR signing and CRL fetching to the external CA URL. + +### Prerequisites + +- A running Puppet/OpenVox CA accessible from the cluster (e.g. `https://puppet-ca.example.com:8140`) +- The CA's public certificate (`ca_crt.pem`) +- (Optional) A client certificate and key for mTLS authentication + +### Steps + +1. Create Secrets with the CA certificate and optional client credentials: + + ```bash + # CA certificate for TLS verification + kubectl create secret generic external-ca-cert \ + --from-file=ca_crt.pem=/path/to/ca_crt.pem + + # (Optional) Client certificate for mTLS + kubectl create secret generic external-ca-tls \ + --from-file=tls.crt=/path/to/client.pem \ + --from-file=tls.key=/path/to/client-key.pem + ``` + +2. Create the `CertificateAuthority` resource with `spec.external`: + + ```yaml + apiVersion: openvox.voxpupuli.org/v1alpha1 + kind: CertificateAuthority + metadata: + name: external-ca + spec: + allowSubjectAltNames: true + allowAuthorizationExtensions: true + enableInfraCRL: true + crlRefreshInterval: 5m + external: + url: https://puppet-ca.example.com:8140 + caSecretRef: external-ca-cert + tlsSecretRef: external-ca-tls + ``` + +3. The operator will: + - Skip PVC creation and CA setup Job + - Validate the referenced Secrets + - Set the CA phase to `External` + - Periodically fetch the CRL from the external CA + - Route CSR signing requests to the external CA + +### External CA Fields + +| Field | Required | Description | +|-------|----------|-------------| +| `url` | Yes | Base URL of the external CA (e.g. `https://puppet-ca.example.com:8140`) | +| `caSecretRef` | No | Secret name containing `ca_crt.pem` for TLS verification | +| `tlsSecretRef` | No | Secret name containing `tls.crt` and `tls.key` for mTLS client auth | +| `insecureSkipVerify` | No | Skip TLS verification (not recommended for production) | + +### Notes + +- `spec.external` and custom `spec.storage` are mutually exclusive. External CAs do not need local storage. +- The `Certificate` controller accepts both `Ready` and `External` phases as "CA is available", so existing `Certificate` resources work without changes. +- The operator does not manage the external CA's lifecycle (upgrades, backups, etc.). You are responsible for maintaining it. +- CRL refresh still works with external CAs -- the operator fetches the CRL via the Puppet CA HTTP API and stores it in a local Secret. diff --git a/internal/controller/certificate_controller.go b/internal/controller/certificate_controller.go index fe03522..2391521 100644 --- a/internal/controller/certificate_controller.go +++ b/internal/controller/certificate_controller.go @@ -28,8 +28,8 @@ type CertificateReconciler struct { // Event reasons for Certificate. const ( - EventReasonCertificateSigned = "CertificateSigned" - EventReasonCSRWaitingForSigning = "CSRWaitingForSigning" + EventReasonCertificateSigned = "CertificateSigned" + EventReasonCSRWaitingForSigning = "CSRWaitingForSigning" ) // +kubebuilder:rbac:groups=openvox.voxpupuli.org,resources=certificates,verbs=get;list;watch;create;update;patch;delete @@ -70,8 +70,8 @@ func (r *CertificateReconciler) Reconcile(ctx context.Context, req ctrl.Request) return ctrl.Result{}, err } - // Wait for CA to be ready - if ca.Status.Phase != openvoxv1alpha1.CertificateAuthorityPhaseReady { + // Wait for CA to be ready (accept both Ready and External phases) + if ca.Status.Phase != openvoxv1alpha1.CertificateAuthorityPhaseReady && ca.Status.Phase != openvoxv1alpha1.CertificateAuthorityPhaseExternal { logger.Info("waiting for CertificateAuthority to be ready", "ca", ca.Name, "phase", ca.Status.Phase) cert.Status.Phase = openvoxv1alpha1.CertificatePhasePending if statusErr := r.Status().Update(ctx, cert); statusErr != nil { @@ -126,10 +126,17 @@ func (r *CertificateReconciler) SetupWithManager(mgr ctrl.Manager) error { func (r *CertificateReconciler) reconcileCertSigning(ctx context.Context, cert *openvoxv1alpha1.Certificate, ca *openvoxv1alpha1.CertificateAuthority) (ctrl.Result, error) { logger := log.FromContext(ctx) - caServiceName := findCAServiceName(ctx, r.Client, ca, cert.Namespace) - if caServiceName == "" { - logger.Info("waiting for CA server to become available") - return ctrl.Result{RequeueAfter: RequeueIntervalMedium}, nil + // Resolve CA base URL: external URL or internal service discovery + var caBaseURL string + if ca.Spec.External != nil { + caBaseURL = ca.Spec.External.URL + } else { + caServiceName := findCAServiceName(ctx, r.Client, ca, cert.Namespace) + if caServiceName == "" { + logger.Info("waiting for CA server to become available") + return ctrl.Result{RequeueAfter: RequeueIntervalMedium}, nil + } + caBaseURL = fmt.Sprintf("https://%s.%s.svc:8140", caServiceName, cert.Namespace) } cert.Status.Phase = openvoxv1alpha1.CertificatePhaseRequesting @@ -137,7 +144,7 @@ func (r *CertificateReconciler) reconcileCertSigning(ctx context.Context, cert * logger.Error(statusErr, "failed to update Certificate status", "name", cert.Name) } - result, err := r.signCertificate(ctx, cert, ca, caServiceName, cert.Namespace) + result, err := r.signCertificate(ctx, cert, ca, caBaseURL, cert.Namespace) if err != nil { logger.Error(err, "certificate signing failed, will retry") cert.Status.Phase = openvoxv1alpha1.CertificatePhaseError @@ -225,4 +232,3 @@ func (r *CertificateReconciler) extractNotAfter(ctx context.Context, secretName, } return parseCertNotAfter(ctx, secret.Data["cert.pem"]) } - diff --git a/internal/controller/certificate_signing.go b/internal/controller/certificate_signing.go index cd916a2..b0d8033 100644 --- a/internal/controller/certificate_signing.go +++ b/internal/controller/certificate_signing.go @@ -22,12 +22,74 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/log" openvoxv1alpha1 "github.com/slauger/openvox-operator/api/v1alpha1" ) +// caHTTPClientForCA returns an HTTP client configured for the CA. +// For external CAs, it builds an mTLS client from the referenced Secrets. +// For internal CAs, it uses the CA public certificate. +func (r *CertificateReconciler) caHTTPClientForCA(ctx context.Context, ca *openvoxv1alpha1.CertificateAuthority, namespace string) (*http.Client, error) { + if ca.Spec.External != nil { + return buildExternalCAHTTPClient(ctx, r.Client, ca.Spec.External, namespace) + } + + caCertPEM, err := r.getCAPublicCert(ctx, ca, namespace) + if err != nil { + return nil, fmt.Errorf("loading CA certificate: %w", err) + } + return caHTTPClient(caCertPEM) +} + +// buildExternalCAHTTPClient creates an HTTP client for an external CA with optional mTLS and CA verification. +func buildExternalCAHTTPClient(ctx context.Context, reader client.Reader, ext *openvoxv1alpha1.ExternalCASpec, namespace string) (*http.Client, error) { + tlsConfig := &tls.Config{ + InsecureSkipVerify: ext.InsecureSkipVerify, //nolint:gosec // user-controlled option for external CAs + } + + // Load CA certificate for server verification + if ext.CASecretRef != "" { + secret := &corev1.Secret{} + if err := reader.Get(ctx, types.NamespacedName{Name: ext.CASecretRef, Namespace: namespace}, secret); err != nil { + return nil, fmt.Errorf("getting CA Secret %s: %w", ext.CASecretRef, err) + } + caCertPEM := secret.Data["ca_crt.pem"] + if len(caCertPEM) > 0 { + pool := x509.NewCertPool() + if !pool.AppendCertsFromPEM(caCertPEM) { + return nil, fmt.Errorf("failed to parse CA certificate from Secret %s", ext.CASecretRef) + } + tlsConfig.RootCAs = pool + } + } + + // Load client certificate for mTLS + if ext.TLSSecretRef != "" { + secret := &corev1.Secret{} + if err := reader.Get(ctx, types.NamespacedName{Name: ext.TLSSecretRef, Namespace: namespace}, secret); err != nil { + return nil, fmt.Errorf("getting TLS Secret %s: %w", ext.TLSSecretRef, err) + } + certPEM := secret.Data["tls.crt"] + keyPEM := secret.Data["tls.key"] + if len(certPEM) == 0 || len(keyPEM) == 0 { + return nil, fmt.Errorf("TLS Secret %s missing tls.crt or tls.key", ext.TLSSecretRef) + } + clientCert, err := tls.X509KeyPair(certPEM, keyPEM) + if err != nil { + return nil, fmt.Errorf("parsing client certificate from Secret %s: %w", ext.TLSSecretRef, err) + } + tlsConfig.Certificates = []tls.Certificate{clientCert} + } + + return &http.Client{ + Timeout: HTTPClientTimeout, + Transport: &http.Transport{TLSClientConfig: tlsConfig}, + }, nil +} + const ( // AnnotationCSRPollAttempts tracks the number of CSR poll attempts on the pending Secret. AnnotationCSRPollAttempts = "openvox.voxpupuli.org/csr-poll-attempts" @@ -82,7 +144,7 @@ func (r *CertificateReconciler) getCAPublicCert(ctx context.Context, ca *openvox // submitCSR generates an RSA key (or reuses an existing one from a pending Secret), // submits the CSR to the Puppet CA, and stores the private key in a pending Secret. // Returns the pending Secret name. -func (r *CertificateReconciler) submitCSR(ctx context.Context, cert *openvoxv1alpha1.Certificate, ca *openvoxv1alpha1.CertificateAuthority, caServiceName, namespace string) (ctrl.Result, error) { +func (r *CertificateReconciler) submitCSR(ctx context.Context, cert *openvoxv1alpha1.Certificate, ca *openvoxv1alpha1.CertificateAuthority, caBaseURL, namespace string) (ctrl.Result, error) { logger := log.FromContext(ctx) certname := cert.Spec.Certname @@ -151,15 +213,10 @@ func (r *CertificateReconciler) submitCSR(ctx context.Context, cert *openvoxv1al } csrPEM := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE REQUEST", Bytes: csrDER}) - caCertPEM, err := r.getCAPublicCert(ctx, ca, namespace) - if err != nil { - return ctrl.Result{RequeueAfter: RequeueIntervalShort}, fmt.Errorf("loading CA certificate: %w", err) - } - httpClient, err := caHTTPClient(caCertPEM) + httpClient, err := r.caHTTPClientForCA(ctx, ca, namespace) if err != nil { - return ctrl.Result{}, fmt.Errorf("creating CA HTTP client: %w", err) + return ctrl.Result{RequeueAfter: RequeueIntervalShort}, fmt.Errorf("creating CA HTTP client: %w", err) } - caBaseURL := fmt.Sprintf("https://%s.%s.svc:8140", caServiceName, namespace) csrURL := fmt.Sprintf("%s/puppet-ca/v1/certificate_request/%s?environment=production", caBaseURL, certname) logger.Info("submitting CSR to CA", "url", csrURL, "certname", certname) @@ -192,21 +249,16 @@ func (r *CertificateReconciler) submitCSR(ctx context.Context, cert *openvoxv1al } // fetchSignedCert checks if the CA has signed the certificate. Returns the PEM cert or nil. -func (r *CertificateReconciler) fetchSignedCert(ctx context.Context, cert *openvoxv1alpha1.Certificate, ca *openvoxv1alpha1.CertificateAuthority, caServiceName, namespace string) ([]byte, error) { +func (r *CertificateReconciler) fetchSignedCert(ctx context.Context, cert *openvoxv1alpha1.Certificate, ca *openvoxv1alpha1.CertificateAuthority, caBaseURL, namespace string) ([]byte, error) { certname := cert.Spec.Certname if certname == "" { certname = "puppet" } - caCertPEM, err := r.getCAPublicCert(ctx, ca, namespace) - if err != nil { - return nil, fmt.Errorf("loading CA certificate: %w", err) - } - httpClient, err := caHTTPClient(caCertPEM) + httpClient, err := r.caHTTPClientForCA(ctx, ca, namespace) if err != nil { return nil, fmt.Errorf("creating CA HTTP client: %w", err) } - caBaseURL := fmt.Sprintf("https://%s.%s.svc:8140", caServiceName, namespace) certURL := fmt.Sprintf("%s/puppet-ca/v1/certificate/%s?environment=production", caBaseURL, certname) req, err := http.NewRequestWithContext(ctx, http.MethodGet, certURL, nil) @@ -239,16 +291,16 @@ func (r *CertificateReconciler) fetchSignedCert(ctx context.Context, cert *openv // signCertificate is the non-blocking orchestrator. It submits the CSR (if not already done), // checks for the signed cert, and returns RequeueAfter if still waiting. -func (r *CertificateReconciler) signCertificate(ctx context.Context, cert *openvoxv1alpha1.Certificate, ca *openvoxv1alpha1.CertificateAuthority, caServiceName, namespace string) (ctrl.Result, error) { +func (r *CertificateReconciler) signCertificate(ctx context.Context, cert *openvoxv1alpha1.Certificate, ca *openvoxv1alpha1.CertificateAuthority, caBaseURL, namespace string) (ctrl.Result, error) { logger := log.FromContext(ctx) // Step 1: Ensure CSR is submitted and key is persisted - if result, err := r.submitCSR(ctx, cert, ca, caServiceName, namespace); err != nil { + if result, err := r.submitCSR(ctx, cert, ca, caBaseURL, namespace); err != nil { return result, err } // Step 2: Check if cert is signed (non-blocking, single attempt) - signedCertPEM, err := r.fetchSignedCert(ctx, cert, ca, caServiceName, namespace) + signedCertPEM, err := r.fetchSignedCert(ctx, cert, ca, caBaseURL, namespace) if err != nil { logger.Info("failed to fetch signed cert, will retry", "error", err) return ctrl.Result{RequeueAfter: RequeueIntervalMedium}, nil diff --git a/internal/controller/certificateauthority_controller.go b/internal/controller/certificateauthority_controller.go index a5c615b..fd4fd8b 100644 --- a/internal/controller/certificateauthority_controller.go +++ b/internal/controller/certificateauthority_controller.go @@ -31,6 +31,7 @@ type CertificateAuthorityReconciler struct { // Event reasons for CertificateAuthority. const ( EventReasonCAInitialized = "CAInitialized" + EventReasonCAExternal = "CAExternal" EventReasonCRLRefreshed = "CRLRefreshed" EventReasonCRLRefreshFailed = "CRLRefreshFailed" ) @@ -65,6 +66,11 @@ func (r *CertificateAuthorityReconciler) Reconcile(ctx context.Context, req ctrl } } + // External CA: delegate to dedicated reconciler, skip PVC/Job/Config + if ca.Spec.External != nil { + return r.reconcileExternalCA(ctx, ca) + } + // Resolve Config referencing this CA cfg := r.findConfigForCA(ctx, ca) if cfg == nil { @@ -181,6 +187,62 @@ func (r *CertificateAuthorityReconciler) SetupWithManager(mgr ctrl.Manager) erro Complete(r) } +// reconcileExternalCA handles CertificateAuthority resources configured with spec.external. +// It validates the CA Secret (if referenced), sets the External phase, and triggers CRL refresh. +func (r *CertificateAuthorityReconciler) reconcileExternalCA(ctx context.Context, ca *openvoxv1alpha1.CertificateAuthority) (ctrl.Result, error) { + logger := log.FromContext(ctx) + + ext := ca.Spec.External + caSecretName := fmt.Sprintf("%s-ca", ca.Name) + + // If caSecretRef is set, validate the Secret exists and has ca_crt.pem + if ext.CASecretRef != "" { + caSecretName = ext.CASecretRef + secret := &corev1.Secret{} + if err := r.Get(ctx, types.NamespacedName{Name: ext.CASecretRef, Namespace: ca.Namespace}, secret); err != nil { + if errors.IsNotFound(err) { + logger.Info("waiting for CA Secret referenced by external CA", "secret", ext.CASecretRef) + return ctrl.Result{RequeueAfter: RequeueIntervalShort}, nil + } + return ctrl.Result{}, fmt.Errorf("getting CA Secret %s: %w", ext.CASecretRef, err) + } + if len(secret.Data["ca_crt.pem"]) == 0 { + logger.Info("CA Secret missing ca_crt.pem key", "secret", ext.CASecretRef) + return ctrl.Result{RequeueAfter: RequeueIntervalShort}, nil + } + } + + wasExternal := ca.Status.Phase == openvoxv1alpha1.CertificateAuthorityPhaseExternal + ca.Status.Phase = openvoxv1alpha1.CertificateAuthorityPhaseExternal + ca.Status.CASecretName = caSecretName + ca.Status.NotAfter = r.extractCANotAfter(ctx, caSecretName, ca.Namespace) + + meta.SetStatusCondition(&ca.Status.Conditions, metav1.Condition{ + Type: openvoxv1alpha1.ConditionCAReady, + Status: metav1.ConditionTrue, + Reason: "ExternalCA", + Message: fmt.Sprintf("External CA configured at %s", ext.URL), + LastTransitionTime: metav1.Now(), + }) + + if err := r.Status().Update(ctx, ca); err != nil { + return ctrl.Result{}, err + } + + if !wasExternal { + r.Recorder.Eventf(ca, nil, corev1.EventTypeNormal, EventReasonCAExternal, "Reconcile", "External CA configured at %s", ext.URL) + } + + // CRL refresh from external CA + crlResult, err := r.reconcileCRLRefresh(ctx, ca) + if err != nil { + logger.Info("CRL refresh failed, will retry", "error", err) + r.Recorder.Eventf(ca, nil, corev1.EventTypeWarning, EventReasonCRLRefreshFailed, "Reconcile", "CRL refresh failed: %v", err) + return ctrl.Result{RequeueAfter: RequeueIntervalCRL}, nil + } + return crlResult, nil +} + // extractCANotAfter reads the ca_crt.pem from the CA Secret and returns its NotAfter time. func (r *CertificateAuthorityReconciler) extractCANotAfter(ctx context.Context, secretName, namespace string) *metav1.Time { secret := &corev1.Secret{} diff --git a/internal/controller/certificateauthority_crl.go b/internal/controller/certificateauthority_crl.go index 12473a0..1b23414 100644 --- a/internal/controller/certificateauthority_crl.go +++ b/internal/controller/certificateauthority_crl.go @@ -29,13 +29,20 @@ func (r *CertificateAuthorityReconciler) reconcileCRLRefresh(ctx context.Context interval = parsed } - caServiceName := findCAServiceName(ctx, r.Client, ca, ca.Namespace) - if caServiceName == "" { - logger.Info("CA service not yet available, skipping CRL refresh") - return ctrl.Result{RequeueAfter: interval}, nil + // Resolve CA base URL: external URL or internal service discovery + var caBaseURL string + if ca.Spec.External != nil { + caBaseURL = ca.Spec.External.URL + } else { + caServiceName := findCAServiceName(ctx, r.Client, ca, ca.Namespace) + if caServiceName == "" { + logger.Info("CA service not yet available, skipping CRL refresh") + return ctrl.Result{RequeueAfter: interval}, nil + } + caBaseURL = fmt.Sprintf("https://%s.%s.svc:8140", caServiceName, ca.Namespace) } - crlPEM, err := r.fetchCRL(ctx, ca, caServiceName, ca.Namespace) + crlPEM, err := r.fetchCRL(ctx, ca, caBaseURL, ca.Namespace) if err != nil { return ctrl.Result{}, fmt.Errorf("fetching CRL: %w", err) } @@ -65,17 +72,13 @@ func (r *CertificateAuthorityReconciler) getCAPublicCert(ctx context.Context, ca } // fetchCRL retrieves the CRL from the CA HTTP API. -func (r *CertificateAuthorityReconciler) fetchCRL(ctx context.Context, ca *openvoxv1alpha1.CertificateAuthority, caServiceName, namespace string) ([]byte, error) { - caCertPEM, err := r.getCAPublicCert(ctx, ca, namespace) - if err != nil { - return nil, fmt.Errorf("loading CA certificate: %w", err) - } - httpClient, err := caHTTPClient(caCertPEM) +func (r *CertificateAuthorityReconciler) fetchCRL(ctx context.Context, ca *openvoxv1alpha1.CertificateAuthority, caBaseURL, namespace string) ([]byte, error) { + httpClient, err := r.caHTTPClientForCA(ctx, ca, namespace) if err != nil { return nil, fmt.Errorf("creating CA HTTP client: %w", err) } - crlURL := fmt.Sprintf("https://%s.%s.svc:8140/puppet-ca/v1/certificate_revocation_list/ca?environment=production", caServiceName, namespace) + crlURL := fmt.Sprintf("%s/puppet-ca/v1/certificate_revocation_list/ca?environment=production", caBaseURL) req, err := http.NewRequestWithContext(ctx, http.MethodGet, crlURL, nil) if err != nil { return nil, fmt.Errorf("building CRL request: %w", err) @@ -100,6 +103,21 @@ func (r *CertificateAuthorityReconciler) fetchCRL(ctx context.Context, ca *openv return body, nil } +// caHTTPClientForCA returns an HTTP client configured for the CA. +// For external CAs, it builds an mTLS client from the referenced Secrets. +// For internal CAs, it uses the CA public certificate. +func (r *CertificateAuthorityReconciler) caHTTPClientForCA(ctx context.Context, ca *openvoxv1alpha1.CertificateAuthority, namespace string) (*http.Client, error) { + if ca.Spec.External != nil { + return buildExternalCAHTTPClient(ctx, r.Client, ca.Spec.External, namespace) + } + + caCertPEM, err := r.getCAPublicCert(ctx, ca, namespace) + if err != nil { + return nil, fmt.Errorf("loading CA certificate: %w", err) + } + return caHTTPClient(caCertPEM) +} + // updateCRLSecret creates or updates the CRL secret with fresh CRL data. func (r *CertificateAuthorityReconciler) updateCRLSecret(ctx context.Context, ca *openvoxv1alpha1.CertificateAuthority, name string, crlPEM []byte) error { return createOrUpdateSecret(ctx, r.Client, r.Scheme, ca, name, ca.Namespace, caLabels(ca.Name), map[string][]byte{ diff --git a/internal/controller/constants.go b/internal/controller/constants.go index adbcbea..8b7b2c9 100644 --- a/internal/controller/constants.go +++ b/internal/controller/constants.go @@ -24,12 +24,12 @@ const RSAKeySize = 4096 // CA setup Job defaults. const ( - CAJobBackoffLimit = int32(3) - DefaultCAStorageGi = "1Gi" - CASetupRunAsUser = int64(1001) - CASetupRunAsGroup = int64(0) - ServerRunAsUser = int64(1001) - ServerRunAsGroup = int64(0) + CAJobBackoffLimit = int32(3) + DefaultCAStorageGi = "1Gi" + CASetupRunAsUser = int64(1001) + CASetupRunAsGroup = int64(0) + ServerRunAsUser = int64(1001) + ServerRunAsGroup = int64(0) ) // CA setup Job resource defaults (JRuby/JVM workload). @@ -42,7 +42,7 @@ const ( // HPA and PDB defaults for Server resources. const ( - DefaultHPAMaxReplicas = int32(5) - DefaultHPATargetCPU = int32(75) + DefaultHPAMaxReplicas = int32(5) + DefaultHPATargetCPU = int32(75) DefaultPDBMinAvailable = 1 ) diff --git a/internal/controller/testutil_test.go b/internal/controller/testutil_test.go index c730091..14debc7 100644 --- a/internal/controller/testutil_test.go +++ b/internal/controller/testutil_test.go @@ -437,4 +437,3 @@ func newReportProcessorReconciler(c client.Client) *ReportProcessorReconciler { Recorder: testRecorder(), } } - From 90a15318e7cd8b5716b28bed0cb9b30836e644e6 Mon Sep 17 00:00:00 2001 From: Simon Lauger Date: Mon, 16 Mar 2026 23:58:44 +0100 Subject: [PATCH 04/14] fix: replace Unicode smart quote in CEL validation rule with ASCII single quotes --- api/v1alpha1/certificateauthority_types.go | 2 +- .../crds/openvox.voxpupuli.org_certificateauthorities.yaml | 2 +- .../crd/bases/openvox.voxpupuli.org_certificateauthorities.yaml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/api/v1alpha1/certificateauthority_types.go b/api/v1alpha1/certificateauthority_types.go index 2a47124..42a5605 100644 --- a/api/v1alpha1/certificateauthority_types.go +++ b/api/v1alpha1/certificateauthority_types.go @@ -57,7 +57,7 @@ type ExternalCASpec struct { } // CertificateAuthoritySpec defines the desired state of CertificateAuthority. -// +kubebuilder:validation:XValidation:rule="!(has(self.external) && has(self.storage) && self.storage.size != ” && self.storage.size != '1Gi')",message="external and custom storage are mutually exclusive" +// +kubebuilder:validation:XValidation:rule="!(has(self.external) && has(self.storage) && self.storage.size != '' && self.storage.size != '1Gi')",message="external and custom storage are mutually exclusive" type CertificateAuthoritySpec struct { // TTL is the CA certificate TTL as a duration string. // Supported units: s (seconds), m (minutes), h (hours), d (days), y (years). diff --git a/charts/openvox-operator/crds/openvox.voxpupuli.org_certificateauthorities.yaml b/charts/openvox-operator/crds/openvox.voxpupuli.org_certificateauthorities.yaml index e59c189..219e24d 100644 --- a/charts/openvox-operator/crds/openvox.voxpupuli.org_certificateauthorities.yaml +++ b/charts/openvox-operator/crds/openvox.voxpupuli.org_certificateauthorities.yaml @@ -217,7 +217,7 @@ spec: x-kubernetes-validations: - message: external and custom storage are mutually exclusive rule: '!(has(self.external) && has(self.storage) && self.storage.size - != ” && self.storage.size != ''1Gi'')' + != '''' && self.storage.size != ''1Gi'')' status: description: CertificateAuthorityStatus defines the observed state of CertificateAuthority. diff --git a/config/crd/bases/openvox.voxpupuli.org_certificateauthorities.yaml b/config/crd/bases/openvox.voxpupuli.org_certificateauthorities.yaml index e59c189..219e24d 100644 --- a/config/crd/bases/openvox.voxpupuli.org_certificateauthorities.yaml +++ b/config/crd/bases/openvox.voxpupuli.org_certificateauthorities.yaml @@ -217,7 +217,7 @@ spec: x-kubernetes-validations: - message: external and custom storage are mutually exclusive rule: '!(has(self.external) && has(self.storage) && self.storage.size - != ” && self.storage.size != ''1Gi'')' + != '''' && self.storage.size != ''1Gi'')' status: description: CertificateAuthorityStatus defines the observed state of CertificateAuthority. From 4c4ae6d0761709cb3450fa3c51ae25caf58b8d68 Mon Sep 17 00:00:00 2001 From: Simon Lauger Date: Tue, 17 Mar 2026 00:01:57 +0100 Subject: [PATCH 05/14] fix: use CEL size() instead of string comparison in XValidation rule Avoids Go 1.26 go fmt smart quote regression (#30). --- api/v1alpha1/certificateauthority_types.go | 2 +- .../crds/openvox.voxpupuli.org_certificateauthorities.yaml | 4 ++-- .../bases/openvox.voxpupuli.org_certificateauthorities.yaml | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/api/v1alpha1/certificateauthority_types.go b/api/v1alpha1/certificateauthority_types.go index 42a5605..d2bb895 100644 --- a/api/v1alpha1/certificateauthority_types.go +++ b/api/v1alpha1/certificateauthority_types.go @@ -57,7 +57,7 @@ type ExternalCASpec struct { } // CertificateAuthoritySpec defines the desired state of CertificateAuthority. -// +kubebuilder:validation:XValidation:rule="!(has(self.external) && has(self.storage) && self.storage.size != '' && self.storage.size != '1Gi')",message="external and custom storage are mutually exclusive" +// +kubebuilder:validation:XValidation:rule="!(has(self.external) && has(self.storage) && size(self.storage.size) > 0 && self.storage.size != '1Gi')",message="external and custom storage are mutually exclusive" type CertificateAuthoritySpec struct { // TTL is the CA certificate TTL as a duration string. // Supported units: s (seconds), m (minutes), h (hours), d (days), y (years). diff --git a/charts/openvox-operator/crds/openvox.voxpupuli.org_certificateauthorities.yaml b/charts/openvox-operator/crds/openvox.voxpupuli.org_certificateauthorities.yaml index 219e24d..e5c8aef 100644 --- a/charts/openvox-operator/crds/openvox.voxpupuli.org_certificateauthorities.yaml +++ b/charts/openvox-operator/crds/openvox.voxpupuli.org_certificateauthorities.yaml @@ -216,8 +216,8 @@ spec: type: object x-kubernetes-validations: - message: external and custom storage are mutually exclusive - rule: '!(has(self.external) && has(self.storage) && self.storage.size - != '''' && self.storage.size != ''1Gi'')' + rule: '!(has(self.external) && has(self.storage) && size(self.storage.size) + > 0 && self.storage.size != ''1Gi'')' status: description: CertificateAuthorityStatus defines the observed state of CertificateAuthority. diff --git a/config/crd/bases/openvox.voxpupuli.org_certificateauthorities.yaml b/config/crd/bases/openvox.voxpupuli.org_certificateauthorities.yaml index 219e24d..e5c8aef 100644 --- a/config/crd/bases/openvox.voxpupuli.org_certificateauthorities.yaml +++ b/config/crd/bases/openvox.voxpupuli.org_certificateauthorities.yaml @@ -216,8 +216,8 @@ spec: type: object x-kubernetes-validations: - message: external and custom storage are mutually exclusive - rule: '!(has(self.external) && has(self.storage) && self.storage.size - != '''' && self.storage.size != ''1Gi'')' + rule: '!(has(self.external) && has(self.storage) && size(self.storage.size) + > 0 && self.storage.size != ''1Gi'')' status: description: CertificateAuthorityStatus defines the observed state of CertificateAuthority. From 23dc7f36a399e4439984cf0bcab3b2cdfc7a0862 Mon Sep 17 00:00:00 2001 From: Simon Lauger Date: Thu, 19 Mar 2026 09:29:38 +0100 Subject: [PATCH 06/14] test: add unit tests for external CA support Add 13 new tests covering reconcileExternalCA, buildExternalCAHTTPClient, and external CA phase acceptance in the certificate controller. - testutil_test.go: add caOption type with withExternal, withExternalCASecret, withExternalTLSSecret, withExternalInsecureSkipVerify options - certificateauthority_controller_test.go: 6 tests for external CA reconciler (basic flow, custom CA secret, missing secret, missing key, no PVC/Job, no Config) - certificate_controller_test.go: 1 test for External phase acceptance - certificate_signing_test.go: 6 tests for buildExternalCAHTTPClient (minimal, insecure skip verify, CA secret, missing secret, mTLS, missing key) --- .../controller/certificate_controller_test.go | 32 ++++ .../controller/certificate_signing_test.go | 151 +++++++++++++++++ .../certificateauthority_controller_test.go | 153 ++++++++++++++++++ internal/controller/testutil_test.go | 43 ++++- 4 files changed, 378 insertions(+), 1 deletion(-) diff --git a/internal/controller/certificate_controller_test.go b/internal/controller/certificate_controller_test.go index 95f14cc..9d9d37c 100644 --- a/internal/controller/certificate_controller_test.go +++ b/internal/controller/certificate_controller_test.go @@ -129,3 +129,35 @@ func TestCertReconcile_PhasePending(t *testing.T) { t.Errorf("expected phase %q, got %q", openvoxv1alpha1.CertificatePhasePending, updated.Status.Phase) } } + +func TestCertReconcile_CAExternalPhase_Accepted(t *testing.T) { + cert := newCertificate("my-cert", "ext-ca", "") + ca := newCertificateAuthority("ext-ca", withExternal("https://puppet-ca.example.com:8140")) + ca.Status.Phase = openvoxv1alpha1.CertificateAuthorityPhaseExternal + // Pre-create the TLS secret so reconcile completes without HTTP calls + tlsSecret := newSecret("my-cert-tls", map[string][]byte{ + "cert.pem": []byte("signed-cert"), + "key.pem": []byte("private-key"), + }) + + c := setupTestClient(cert, ca, tlsSecret) + r := newCertificateReconciler(c) + + res, err := r.Reconcile(testCtx(), testRequest("my-cert")) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + // Should NOT requeue with 10s (that would mean CA was treated as not-ready) + if res.RequeueAfter == 10*time.Second { + t.Error("certificate should not wait for CA when phase is External") + } + + updated := &openvoxv1alpha1.Certificate{} + if err := c.Get(testCtx(), types.NamespacedName{Name: "my-cert", Namespace: testNamespace}, updated); err != nil { + t.Fatalf("failed to get Certificate: %v", err) + } + if updated.Status.Phase != openvoxv1alpha1.CertificatePhaseSigned { + t.Errorf("expected phase %q, got %q", openvoxv1alpha1.CertificatePhaseSigned, updated.Status.Phase) + } +} diff --git a/internal/controller/certificate_signing_test.go b/internal/controller/certificate_signing_test.go index 3f2e07c..3b6d9a3 100644 --- a/internal/controller/certificate_signing_test.go +++ b/internal/controller/certificate_signing_test.go @@ -1,8 +1,17 @@ package controller import ( + "crypto/rand" + "crypto/rsa" + "crypto/x509" + "crypto/x509/pkix" + "encoding/pem" + "math/big" + "net/http" "testing" "time" + + openvoxv1alpha1 "github.com/slauger/openvox-operator/api/v1alpha1" ) func TestCSRPollBackoff(t *testing.T) { @@ -32,3 +41,145 @@ func TestCSRPollBackoff(t *testing.T) { } } } + +// generateTestCert creates a self-signed CA certificate and key pair for testing. +// Returns PEM-encoded certificate, PEM-encoded private key. +func generateTestCert(t *testing.T) ([]byte, []byte) { + t.Helper() + key, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + t.Fatalf("generating test key: %v", err) + } + tmpl := &x509.Certificate{ + SerialNumber: big.NewInt(1), + Subject: pkix.Name{CommonName: "test-ca"}, + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(24 * time.Hour), + IsCA: true, + KeyUsage: x509.KeyUsageCertSign | x509.KeyUsageCRLSign, + } + certDER, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, &key.PublicKey, key) + if err != nil { + t.Fatalf("creating test certificate: %v", err) + } + certPEM := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: certDER}) + keyPEM := pem.EncodeToMemory(&pem.Block{Type: "RSA PRIVATE KEY", Bytes: x509.MarshalPKCS1PrivateKey(key)}) + return certPEM, keyPEM +} + +func TestBuildExternalCAHTTPClient_Minimal(t *testing.T) { + ext := &openvoxv1alpha1.ExternalCASpec{ + URL: "https://puppet-ca.example.com:8140", + } + c := setupTestClient() + + httpClient, err := buildExternalCAHTTPClient(testCtx(), c, ext, testNamespace) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if httpClient == nil { + t.Fatal("expected non-nil HTTP client") + } + + transport := httpClient.Transport.(*http.Transport) + if transport.TLSClientConfig.InsecureSkipVerify { + t.Error("expected InsecureSkipVerify=false") + } + if transport.TLSClientConfig.RootCAs != nil { + t.Error("expected no custom RootCAs") + } +} + +func TestBuildExternalCAHTTPClient_InsecureSkipVerify(t *testing.T) { + ext := &openvoxv1alpha1.ExternalCASpec{ + URL: "https://puppet-ca.example.com:8140", + InsecureSkipVerify: true, + } + c := setupTestClient() + + httpClient, err := buildExternalCAHTTPClient(testCtx(), c, ext, testNamespace) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + transport := httpClient.Transport.(*http.Transport) + if !transport.TLSClientConfig.InsecureSkipVerify { + t.Error("expected InsecureSkipVerify=true") + } +} + +func TestBuildExternalCAHTTPClient_WithCASecret(t *testing.T) { + certPEM, _ := generateTestCert(t) + caSecret := newSecret("ca-secret", map[string][]byte{ + "ca_crt.pem": certPEM, + }) + ext := &openvoxv1alpha1.ExternalCASpec{ + URL: "https://puppet-ca.example.com:8140", + CASecretRef: "ca-secret", + } + c := setupTestClient(caSecret) + + httpClient, err := buildExternalCAHTTPClient(testCtx(), c, ext, testNamespace) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + transport := httpClient.Transport.(*http.Transport) + if transport.TLSClientConfig.RootCAs == nil { + t.Error("expected custom RootCAs pool") + } +} + +func TestBuildExternalCAHTTPClient_CASecretNotFound(t *testing.T) { + ext := &openvoxv1alpha1.ExternalCASpec{ + URL: "https://puppet-ca.example.com:8140", + CASecretRef: "missing-secret", + } + c := setupTestClient() + + _, err := buildExternalCAHTTPClient(testCtx(), c, ext, testNamespace) + if err == nil { + t.Fatal("expected error when CA secret is missing") + } +} + +func TestBuildExternalCAHTTPClient_WithTLSSecret(t *testing.T) { + certPEM, keyPEM := generateTestCert(t) + tlsSecret := newSecret("tls-secret", map[string][]byte{ + "tls.crt": certPEM, + "tls.key": keyPEM, + }) + ext := &openvoxv1alpha1.ExternalCASpec{ + URL: "https://puppet-ca.example.com:8140", + TLSSecretRef: "tls-secret", + } + c := setupTestClient(tlsSecret) + + httpClient, err := buildExternalCAHTTPClient(testCtx(), c, ext, testNamespace) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + transport := httpClient.Transport.(*http.Transport) + if len(transport.TLSClientConfig.Certificates) != 1 { + t.Errorf("expected 1 client certificate, got %d", len(transport.TLSClientConfig.Certificates)) + } +} + +func TestBuildExternalCAHTTPClient_TLSSecretMissingKey(t *testing.T) { + certPEM, _ := generateTestCert(t) + // Secret has tls.crt but missing tls.key + tlsSecret := newSecret("tls-secret", map[string][]byte{ + "tls.crt": certPEM, + }) + ext := &openvoxv1alpha1.ExternalCASpec{ + URL: "https://puppet-ca.example.com:8140", + TLSSecretRef: "tls-secret", + } + c := setupTestClient(tlsSecret) + + _, err := buildExternalCAHTTPClient(testCtx(), c, ext, testNamespace) + if err == nil { + t.Fatal("expected error when TLS secret is missing tls.key") + } +} diff --git a/internal/controller/certificateauthority_controller_test.go b/internal/controller/certificateauthority_controller_test.go index 8228d55..88aa724 100644 --- a/internal/controller/certificateauthority_controller_test.go +++ b/internal/controller/certificateauthority_controller_test.go @@ -351,3 +351,156 @@ func TestResolveCAJobResources_Custom(t *testing.T) { t.Errorf("expected memory limit 2Gi, got %s", res.Limits.Memory().String()) } } + +// --- External CA tests --- + +func TestCAReconcile_ExternalCA_Basic(t *testing.T) { + ca := newCertificateAuthority("ext-ca", withExternal("https://puppet-ca.example.com:8140")) + ca.Status.Phase = "" // reset to trigger initial phase + c := setupTestClient(ca) + r := newCertificateAuthorityReconciler(c) + + if _, err := r.Reconcile(testCtx(), testRequest("ext-ca")); err != nil { + t.Fatalf("reconcile error: %v", err) + } + + updated := &openvoxv1alpha1.CertificateAuthority{} + if err := c.Get(testCtx(), types.NamespacedName{Name: "ext-ca", Namespace: testNamespace}, updated); err != nil { + t.Fatalf("failed to get CA: %v", err) + } + + if updated.Status.Phase != openvoxv1alpha1.CertificateAuthorityPhaseExternal { + t.Errorf("expected phase %q, got %q", openvoxv1alpha1.CertificateAuthorityPhaseExternal, updated.Status.Phase) + } + if updated.Status.CASecretName != "ext-ca-ca" { + t.Errorf("expected CASecretName %q, got %q", "ext-ca-ca", updated.Status.CASecretName) + } + + // Verify CAReady condition with ExternalCA reason + found := false + for _, cond := range updated.Status.Conditions { + if cond.Type == openvoxv1alpha1.ConditionCAReady { + found = true + if cond.Status != "True" { + t.Errorf("expected condition status True, got %q", cond.Status) + } + if cond.Reason != "ExternalCA" { + t.Errorf("expected condition reason ExternalCA, got %q", cond.Reason) + } + } + } + if !found { + t.Error("CAReady condition not set") + } +} + +func TestCAReconcile_ExternalCA_WithCASecretRef(t *testing.T) { + ca := newCertificateAuthority("ext-ca", + withExternal("https://puppet-ca.example.com:8140"), + withExternalCASecret("my-custom-ca-secret"), + ) + ca.Status.Phase = "" + caSecret := newSecret("my-custom-ca-secret", map[string][]byte{ + "ca_crt.pem": []byte("ca-cert-data"), + }) + c := setupTestClient(ca, caSecret) + r := newCertificateAuthorityReconciler(c) + + if _, err := r.Reconcile(testCtx(), testRequest("ext-ca")); err != nil { + t.Fatalf("reconcile error: %v", err) + } + + updated := &openvoxv1alpha1.CertificateAuthority{} + if err := c.Get(testCtx(), types.NamespacedName{Name: "ext-ca", Namespace: testNamespace}, updated); err != nil { + t.Fatalf("failed to get CA: %v", err) + } + + if updated.Status.CASecretName != "my-custom-ca-secret" { + t.Errorf("expected CASecretName %q, got %q", "my-custom-ca-secret", updated.Status.CASecretName) + } +} + +func TestCAReconcile_ExternalCA_CASecretNotFound(t *testing.T) { + ca := newCertificateAuthority("ext-ca", + withExternal("https://puppet-ca.example.com:8140"), + withExternalCASecret("missing-secret"), + ) + ca.Status.Phase = "" + c := setupTestClient(ca) + r := newCertificateAuthorityReconciler(c) + + res, err := r.Reconcile(testCtx(), testRequest("ext-ca")) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if res.RequeueAfter != 5*time.Second { + t.Errorf("expected requeue after 5s, got %v", res.RequeueAfter) + } +} + +func TestCAReconcile_ExternalCA_CASecretMissingKey(t *testing.T) { + ca := newCertificateAuthority("ext-ca", + withExternal("https://puppet-ca.example.com:8140"), + withExternalCASecret("bad-secret"), + ) + ca.Status.Phase = "" + // Secret exists but lacks the ca_crt.pem key + badSecret := newSecret("bad-secret", map[string][]byte{ + "wrong-key": []byte("data"), + }) + c := setupTestClient(ca, badSecret) + r := newCertificateAuthorityReconciler(c) + + res, err := r.Reconcile(testCtx(), testRequest("ext-ca")) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if res.RequeueAfter != 5*time.Second { + t.Errorf("expected requeue after 5s, got %v", res.RequeueAfter) + } +} + +func TestCAReconcile_ExternalCA_SkipsPVCAndJob(t *testing.T) { + ca := newCertificateAuthority("ext-ca", withExternal("https://puppet-ca.example.com:8140")) + ca.Status.Phase = "" + c := setupTestClient(ca) + r := newCertificateAuthorityReconciler(c) + + if _, err := r.Reconcile(testCtx(), testRequest("ext-ca")); err != nil { + t.Fatalf("reconcile error: %v", err) + } + + // No PVC should be created + pvc := &corev1.PersistentVolumeClaim{} + if err := c.Get(testCtx(), types.NamespacedName{Name: "ext-ca-data", Namespace: testNamespace}, pvc); err == nil { + t.Error("expected no PVC for external CA, but one was created") + } + + // No Job should be created + job := &batchv1.Job{} + if err := c.Get(testCtx(), types.NamespacedName{Name: "ext-ca-ca-setup", Namespace: testNamespace}, job); err == nil { + t.Error("expected no Job for external CA, but one was created") + } +} + +func TestCAReconcile_ExternalCA_NoConfigRequired(t *testing.T) { + // External CA should work without any Config object (unlike internal CA which requires it) + ca := newCertificateAuthority("ext-ca", withExternal("https://puppet-ca.example.com:8140")) + ca.Status.Phase = "" + c := setupTestClient(ca) // no Config object + r := newCertificateAuthorityReconciler(c) + + if _, err := r.Reconcile(testCtx(), testRequest("ext-ca")); err != nil { + t.Fatalf("reconcile error: %v", err) + } + + updated := &openvoxv1alpha1.CertificateAuthority{} + if err := c.Get(testCtx(), types.NamespacedName{Name: "ext-ca", Namespace: testNamespace}, updated); err != nil { + t.Fatalf("failed to get CA: %v", err) + } + + // Should reach External phase without a Config + if updated.Status.Phase != openvoxv1alpha1.CertificateAuthorityPhaseExternal { + t.Errorf("expected phase %q, got %q", openvoxv1alpha1.CertificateAuthorityPhaseExternal, updated.Status.Phase) + } +} diff --git a/internal/controller/testutil_test.go b/internal/controller/testutil_test.go index 14debc7..89ee297 100644 --- a/internal/controller/testutil_test.go +++ b/internal/controller/testutil_test.go @@ -279,7 +279,45 @@ func newCertificate(name, authorityRef string, phase openvoxv1alpha1.Certificate return cert } -func newCertificateAuthority(name string) *openvoxv1alpha1.CertificateAuthority { +type caOption func(*openvoxv1alpha1.CertificateAuthority) + +func withExternal(url string) caOption { + return func(ca *openvoxv1alpha1.CertificateAuthority) { + if ca.Spec.External == nil { + ca.Spec.External = &openvoxv1alpha1.ExternalCASpec{} + } + ca.Spec.External.URL = url + } +} + +func withExternalCASecret(ref string) caOption { + return func(ca *openvoxv1alpha1.CertificateAuthority) { + if ca.Spec.External == nil { + ca.Spec.External = &openvoxv1alpha1.ExternalCASpec{} + } + ca.Spec.External.CASecretRef = ref + } +} + +func withExternalTLSSecret(ref string) caOption { + return func(ca *openvoxv1alpha1.CertificateAuthority) { + if ca.Spec.External == nil { + ca.Spec.External = &openvoxv1alpha1.ExternalCASpec{} + } + ca.Spec.External.TLSSecretRef = ref + } +} + +func withExternalInsecureSkipVerify(v bool) caOption { + return func(ca *openvoxv1alpha1.CertificateAuthority) { + if ca.Spec.External == nil { + ca.Spec.External = &openvoxv1alpha1.ExternalCASpec{} + } + ca.Spec.External.InsecureSkipVerify = v + } +} + +func newCertificateAuthority(name string, opts ...caOption) *openvoxv1alpha1.CertificateAuthority { ca := &openvoxv1alpha1.CertificateAuthority{ ObjectMeta: metav1.ObjectMeta{ Name: name, @@ -296,6 +334,9 @@ func newCertificateAuthority(name string) *openvoxv1alpha1.CertificateAuthority } ca.Status.Phase = openvoxv1alpha1.CertificateAuthorityPhaseReady ca.Status.CASecretName = name + "-ca" + for _, o := range opts { + o(ca) + } return ca } From 6774a546b4058fb8e821d871051e1e00af9025f2 Mon Sep 17 00:00:00 2001 From: Simon Lauger Date: Thu, 19 Mar 2026 21:03:14 +0100 Subject: [PATCH 07/14] feat: add dedicated ClusterIP Service for CertificateAuthority The operator connects to the CA server over HTTPS for CSR submission and CRL refresh. Previously, it discovered the CA endpoint via a complex chain (CA -> Config -> Server -> Pool Service), producing an FQDN that was never added to the CA certificate SANs, causing TLS failures. The CA controller now creates its own ClusterIP Service with a deterministic name (ca.Name) and auto-injects the service FQDN into the CA server certificate DNSAltNames. This eliminates the fragile service discovery via findCAServiceName() and fixes x509 SAN validation errors. Changes: - Add status.serviceName to CertificateAuthorityStatus - Add reconcileCAService() creating a ClusterIP Service (port 8140, selector: openvox.voxpupuli.org/ca=true) - Add ensureCAServiceDNSAltName() injecting ..svc into the CA server Certificate DNSAltNames - Replace findCAServiceName() with direct ca.Name in certificate and CRL controllers - Remove findCAServiceName() from helpers.go - Remove unused RBAC markers (configs/servers/pools) from Certificate controller - Add unit tests for Service creation, SAN injection, idempotency, status.serviceName, and external CA exclusion --- api/v1alpha1/certificateauthority_types.go | 5 + ....voxpupuli.org_certificateauthorities.yaml | 5 + ....voxpupuli.org_certificateauthorities.yaml | 5 + internal/controller/certificate_controller.go | 12 +- .../certificateauthority_controller.go | 14 +- .../certificateauthority_controller_test.go | 163 ++++++++++++++++++ .../controller/certificateauthority_crl.go | 9 +- .../certificateauthority_service.go | 109 ++++++++++++ internal/controller/helpers.go | 42 ----- 9 files changed, 304 insertions(+), 60 deletions(-) create mode 100644 internal/controller/certificateauthority_service.go diff --git a/api/v1alpha1/certificateauthority_types.go b/api/v1alpha1/certificateauthority_types.go index d2bb895..10a94d4 100644 --- a/api/v1alpha1/certificateauthority_types.go +++ b/api/v1alpha1/certificateauthority_types.go @@ -145,6 +145,11 @@ type CertificateAuthorityStatus struct { // +optional CASecretName string `json:"caSecretName,omitempty"` + // ServiceName is the name of the ClusterIP Service created for internal operator + // communication with the CA (CSR signing, CRL refresh). + // +optional + ServiceName string `json:"serviceName,omitempty"` + // NotAfter is the expiration time of the CA certificate. // +optional NotAfter *metav1.Time `json:"notAfter,omitempty"` diff --git a/charts/openvox-operator/crds/openvox.voxpupuli.org_certificateauthorities.yaml b/charts/openvox-operator/crds/openvox.voxpupuli.org_certificateauthorities.yaml index e5c8aef..7a5b858 100644 --- a/charts/openvox-operator/crds/openvox.voxpupuli.org_certificateauthorities.yaml +++ b/charts/openvox-operator/crds/openvox.voxpupuli.org_certificateauthorities.yaml @@ -297,6 +297,11 @@ spec: - External - Error type: string + serviceName: + description: |- + ServiceName is the name of the ClusterIP Service created for internal operator + communication with the CA (CSR signing, CRL refresh). + type: string type: object type: object served: true diff --git a/config/crd/bases/openvox.voxpupuli.org_certificateauthorities.yaml b/config/crd/bases/openvox.voxpupuli.org_certificateauthorities.yaml index e5c8aef..7a5b858 100644 --- a/config/crd/bases/openvox.voxpupuli.org_certificateauthorities.yaml +++ b/config/crd/bases/openvox.voxpupuli.org_certificateauthorities.yaml @@ -297,6 +297,11 @@ spec: - External - Error type: string + serviceName: + description: |- + ServiceName is the name of the ClusterIP Service created for internal operator + communication with the CA (CSR signing, CRL refresh). + type: string type: object type: object served: true diff --git a/internal/controller/certificate_controller.go b/internal/controller/certificate_controller.go index 2391521..257b815 100644 --- a/internal/controller/certificate_controller.go +++ b/internal/controller/certificate_controller.go @@ -36,9 +36,6 @@ const ( // +kubebuilder:rbac:groups=openvox.voxpupuli.org,resources=certificates/status,verbs=get;update;patch // +kubebuilder:rbac:groups=openvox.voxpupuli.org,resources=certificates/finalizers,verbs=update // +kubebuilder:rbac:groups=openvox.voxpupuli.org,resources=certificateauthorities,verbs=get;list;watch -// +kubebuilder:rbac:groups=openvox.voxpupuli.org,resources=configs,verbs=get;list;watch -// +kubebuilder:rbac:groups=openvox.voxpupuli.org,resources=servers,verbs=get;list;watch -// +kubebuilder:rbac:groups=openvox.voxpupuli.org,resources=pools,verbs=get;list;watch // +kubebuilder:rbac:groups="",resources=secrets,verbs=get;list;watch;create;update;patch;delete func (r *CertificateReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { @@ -126,17 +123,12 @@ func (r *CertificateReconciler) SetupWithManager(mgr ctrl.Manager) error { func (r *CertificateReconciler) reconcileCertSigning(ctx context.Context, cert *openvoxv1alpha1.Certificate, ca *openvoxv1alpha1.CertificateAuthority) (ctrl.Result, error) { logger := log.FromContext(ctx) - // Resolve CA base URL: external URL or internal service discovery + // Resolve CA base URL: external URL or internal CA Service var caBaseURL string if ca.Spec.External != nil { caBaseURL = ca.Spec.External.URL } else { - caServiceName := findCAServiceName(ctx, r.Client, ca, cert.Namespace) - if caServiceName == "" { - logger.Info("waiting for CA server to become available") - return ctrl.Result{RequeueAfter: RequeueIntervalMedium}, nil - } - caBaseURL = fmt.Sprintf("https://%s.%s.svc:8140", caServiceName, cert.Namespace) + caBaseURL = fmt.Sprintf("https://%s.%s.svc:8140", ca.Name, cert.Namespace) } cert.Status.Phase = openvoxv1alpha1.CertificatePhaseRequesting diff --git a/internal/controller/certificateauthority_controller.go b/internal/controller/certificateauthority_controller.go index fd4fd8b..eab8dc3 100644 --- a/internal/controller/certificateauthority_controller.go +++ b/internal/controller/certificateauthority_controller.go @@ -42,7 +42,7 @@ const ( // +kubebuilder:rbac:groups=openvox.voxpupuli.org,resources=certificates,verbs=get;list;watch // +kubebuilder:rbac:groups=openvox.voxpupuli.org,resources=configs,verbs=get;list;watch // +kubebuilder:rbac:groups=openvox.voxpupuli.org,resources=servers,verbs=get;list;watch -// +kubebuilder:rbac:groups=openvox.voxpupuli.org,resources=pools,verbs=get;list;watch +// +kubebuilder:rbac:groups="",resources=services,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups="",resources=persistentvolumeclaims;secrets;serviceaccounts,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=batch,resources=jobs,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=roles;rolebindings,verbs=get;list;watch;create;update;patch;delete @@ -89,6 +89,16 @@ func (r *CertificateAuthorityReconciler) Reconcile(ctx context.Context, req ctrl return ctrl.Result{}, fmt.Errorf("finding certificates for CA: %w", err) } + // Step 2a: Reconcile CA Service for internal operator communication + if err := r.reconcileCAService(ctx, ca); err != nil { + return ctrl.Result{}, fmt.Errorf("reconciling CA Service: %w", err) + } + + // Step 2b: Inject CA Service FQDN into CA server Certificate SANs + if err := r.ensureCAServiceDNSAltName(ctx, ca, certs); err != nil { + return ctrl.Result{}, fmt.Errorf("ensuring CA Service DNS alt name: %w", err) + } + // Step 3: Ensure RBAC for CA setup job if err := r.reconcileCASetupRBAC(ctx, ca, certs); err != nil { return ctrl.Result{}, fmt.Errorf("reconciling CA setup RBAC: %w", err) @@ -108,6 +118,7 @@ func (r *CertificateAuthorityReconciler) Reconcile(ctx context.Context, req ctrl caSecretName := fmt.Sprintf("%s-ca", ca.Name) ca.Status.Phase = openvoxv1alpha1.CertificateAuthorityPhaseReady ca.Status.CASecretName = caSecretName + ca.Status.ServiceName = ca.Name ca.Status.NotAfter = r.extractCANotAfter(ctx, caSecretName, ca.Namespace) meta.SetStatusCondition(&ca.Status.Conditions, metav1.Condition{ Type: openvoxv1alpha1.ConditionCAReady, @@ -161,6 +172,7 @@ func (r *CertificateAuthorityReconciler) SetupWithManager(mgr ctrl.Manager) erro For(&openvoxv1alpha1.CertificateAuthority{}). Owns(&corev1.PersistentVolumeClaim{}). Owns(&batchv1.Job{}). + Owns(&corev1.Service{}). Owns(&corev1.ServiceAccount{}). Watches(&openvoxv1alpha1.Certificate{}, handler.EnqueueRequestsFromMapFunc( func(ctx context.Context, obj client.Object) []ctrl.Request { diff --git a/internal/controller/certificateauthority_controller_test.go b/internal/controller/certificateauthority_controller_test.go index 88aa724..4e2efab 100644 --- a/internal/controller/certificateauthority_controller_test.go +++ b/internal/controller/certificateauthority_controller_test.go @@ -460,6 +460,169 @@ func TestCAReconcile_ExternalCA_CASecretMissingKey(t *testing.T) { } } +// --- CA Service tests --- + +func TestCAReconcile_ServiceCreation(t *testing.T) { + ca := newCertificateAuthority("test-ca") + ca.Status.Phase = "" + cfg := caPrereqs("test-ca") + c := setupTestClient(ca, cfg) + r := newCertificateAuthorityReconciler(c) + + if _, err := r.Reconcile(testCtx(), testRequest("test-ca")); err != nil { + t.Fatalf("reconcile error: %v", err) + } + + svc := &corev1.Service{} + if err := c.Get(testCtx(), types.NamespacedName{Name: "test-ca", Namespace: testNamespace}, svc); err != nil { + t.Fatalf("Service not created: %v", err) + } + + // Verify name + if svc.Name != "test-ca" { + t.Errorf("expected Service name %q, got %q", "test-ca", svc.Name) + } + + // Verify port + if len(svc.Spec.Ports) != 1 { + t.Fatalf("expected 1 port, got %d", len(svc.Spec.Ports)) + } + if svc.Spec.Ports[0].Port != 8140 { + t.Errorf("expected port 8140, got %d", svc.Spec.Ports[0].Port) + } + if svc.Spec.Ports[0].TargetPort.IntValue() != 8140 { + t.Errorf("expected targetPort 8140, got %d", svc.Spec.Ports[0].TargetPort.IntValue()) + } + + // Verify selector + if svc.Spec.Selector[LabelCA] != "true" { + t.Errorf("expected selector %s=true, got %v", LabelCA, svc.Spec.Selector) + } + + // Verify labels + if svc.Labels[LabelCertificateAuthority] != "test-ca" { + t.Errorf("expected CA label, got %v", svc.Labels) + } + + // Verify owner reference + if len(svc.OwnerReferences) == 0 { + t.Fatal("expected owner reference on Service") + } + if svc.OwnerReferences[0].Name != "test-ca" { + t.Errorf("expected owner ref name %q, got %q", "test-ca", svc.OwnerReferences[0].Name) + } +} + +func TestCAReconcile_SANInjection(t *testing.T) { + ca := newCertificateAuthority("test-ca") + ca.Status.Phase = "" + cfg := caPrereqs("test-ca") + // Create a Server with ca:true pointing to the cert + server := newServer("ca-server", withCA(true), withServerRole(true)) + server.Spec.ConfigRef = "production" + server.Spec.CertificateRef = "ca-cert" + cert := newCertificate("ca-cert", "test-ca", openvoxv1alpha1.CertificatePhasePending) + c := setupTestClient(ca, cfg, server, cert) + r := newCertificateAuthorityReconciler(c) + + if _, err := r.Reconcile(testCtx(), testRequest("test-ca")); err != nil { + t.Fatalf("reconcile error: %v", err) + } + + // Verify the FQDN was injected into the Certificate's DNSAltNames + updatedCert := &openvoxv1alpha1.Certificate{} + if err := c.Get(testCtx(), types.NamespacedName{Name: "ca-cert", Namespace: testNamespace}, updatedCert); err != nil { + t.Fatalf("failed to get Certificate: %v", err) + } + + expectedFQDN := "test-ca.default.svc" + found := false + for _, san := range updatedCert.Spec.DNSAltNames { + if san == expectedFQDN { + found = true + break + } + } + if !found { + t.Errorf("expected FQDN %q in DNSAltNames, got %v", expectedFQDN, updatedCert.Spec.DNSAltNames) + } +} + +func TestCAReconcile_SANInjectionIdempotent(t *testing.T) { + ca := newCertificateAuthority("test-ca") + ca.Status.Phase = "" + cfg := caPrereqs("test-ca") + server := newServer("ca-server", withCA(true), withServerRole(true)) + server.Spec.ConfigRef = "production" + server.Spec.CertificateRef = "ca-cert" + cert := newCertificate("ca-cert", "test-ca", openvoxv1alpha1.CertificatePhasePending) + // Pre-inject the FQDN + cert.Spec.DNSAltNames = []string{"test-ca.default.svc"} + c := setupTestClient(ca, cfg, server, cert) + r := newCertificateAuthorityReconciler(c) + + if _, err := r.Reconcile(testCtx(), testRequest("test-ca")); err != nil { + t.Fatalf("reconcile error: %v", err) + } + + // Verify no duplicate was added + updatedCert := &openvoxv1alpha1.Certificate{} + if err := c.Get(testCtx(), types.NamespacedName{Name: "ca-cert", Namespace: testNamespace}, updatedCert); err != nil { + t.Fatalf("failed to get Certificate: %v", err) + } + + count := 0 + for _, san := range updatedCert.Spec.DNSAltNames { + if san == "test-ca.default.svc" { + count++ + } + } + if count != 1 { + t.Errorf("expected FQDN once in DNSAltNames, found %d times: %v", count, updatedCert.Spec.DNSAltNames) + } +} + +func TestCAReconcile_StatusServiceName(t *testing.T) { + ca := newCertificateAuthority("test-ca") + ca.Status.Phase = "" + cfg := caPrereqs("test-ca") + caSecret := newSecret("test-ca-ca", map[string][]byte{ + "ca_crt.pem": []byte("ca-cert"), + }) + c := setupTestClient(ca, cfg, caSecret) + r := newCertificateAuthorityReconciler(c) + + if _, err := r.Reconcile(testCtx(), testRequest("test-ca")); err != nil { + t.Fatalf("reconcile error: %v", err) + } + + updated := &openvoxv1alpha1.CertificateAuthority{} + if err := c.Get(testCtx(), types.NamespacedName{Name: "test-ca", Namespace: testNamespace}, updated); err != nil { + t.Fatalf("failed to get CA: %v", err) + } + + if updated.Status.ServiceName != "test-ca" { + t.Errorf("expected status.serviceName %q, got %q", "test-ca", updated.Status.ServiceName) + } +} + +func TestCAReconcile_ExternalCA_NoService(t *testing.T) { + ca := newCertificateAuthority("ext-ca", withExternal("https://puppet-ca.example.com:8140")) + ca.Status.Phase = "" + c := setupTestClient(ca) + r := newCertificateAuthorityReconciler(c) + + if _, err := r.Reconcile(testCtx(), testRequest("ext-ca")); err != nil { + t.Fatalf("reconcile error: %v", err) + } + + // No Service should be created for external CA + svc := &corev1.Service{} + if err := c.Get(testCtx(), types.NamespacedName{Name: "ext-ca", Namespace: testNamespace}, svc); err == nil { + t.Error("expected no Service for external CA, but one was created") + } +} + func TestCAReconcile_ExternalCA_SkipsPVCAndJob(t *testing.T) { ca := newCertificateAuthority("ext-ca", withExternal("https://puppet-ca.example.com:8140")) ca.Status.Phase = "" diff --git a/internal/controller/certificateauthority_crl.go b/internal/controller/certificateauthority_crl.go index 1b23414..fb713be 100644 --- a/internal/controller/certificateauthority_crl.go +++ b/internal/controller/certificateauthority_crl.go @@ -29,17 +29,12 @@ func (r *CertificateAuthorityReconciler) reconcileCRLRefresh(ctx context.Context interval = parsed } - // Resolve CA base URL: external URL or internal service discovery + // Resolve CA base URL: external URL or internal CA Service var caBaseURL string if ca.Spec.External != nil { caBaseURL = ca.Spec.External.URL } else { - caServiceName := findCAServiceName(ctx, r.Client, ca, ca.Namespace) - if caServiceName == "" { - logger.Info("CA service not yet available, skipping CRL refresh") - return ctrl.Result{RequeueAfter: interval}, nil - } - caBaseURL = fmt.Sprintf("https://%s.%s.svc:8140", caServiceName, ca.Namespace) + caBaseURL = fmt.Sprintf("https://%s.%s.svc:8140", ca.Name, ca.Namespace) } crlPEM, err := r.fetchCRL(ctx, ca, caBaseURL, ca.Namespace) diff --git a/internal/controller/certificateauthority_service.go b/internal/controller/certificateauthority_service.go new file mode 100644 index 0000000..827e59a --- /dev/null +++ b/internal/controller/certificateauthority_service.go @@ -0,0 +1,109 @@ +package controller + +import ( + "context" + "fmt" + "slices" + + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/intstr" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + "sigs.k8s.io/controller-runtime/pkg/log" + + openvoxv1alpha1 "github.com/slauger/openvox-operator/api/v1alpha1" +) + +// reconcileCAService creates or updates a ClusterIP Service for internal operator +// communication with the CA (CSR signing, CRL refresh). +func (r *CertificateAuthorityReconciler) reconcileCAService(ctx context.Context, ca *openvoxv1alpha1.CertificateAuthority) error { + logger := log.FromContext(ctx) + svcName := ca.Name + + labels := caLabels(ca.Name) + selector := map[string]string{ + LabelCA: "true", + } + + svc := &corev1.Service{} + err := r.Get(ctx, types.NamespacedName{Name: svcName, Namespace: ca.Namespace}, svc) + if errors.IsNotFound(err) { + logger.Info("creating CA Service", "name", svcName) + svc = &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: svcName, + Namespace: ca.Namespace, + Labels: labels, + }, + Spec: corev1.ServiceSpec{ + Type: corev1.ServiceTypeClusterIP, + Selector: selector, + Ports: []corev1.ServicePort{ + { + Name: "https", + Port: 8140, + TargetPort: intstr.FromInt32(8140), + Protocol: corev1.ProtocolTCP, + }, + }, + }, + } + if err := controllerutil.SetControllerReference(ca, svc, r.Scheme); err != nil { + return err + } + return r.Create(ctx, svc) + } else if err != nil { + return err + } + + // Update existing service + svc.Labels = labels + svc.Spec.Type = corev1.ServiceTypeClusterIP + svc.Spec.Selector = selector + if len(svc.Spec.Ports) == 0 { + svc.Spec.Ports = []corev1.ServicePort{{}} + } + svc.Spec.Ports[0].Name = "https" + svc.Spec.Ports[0].Port = 8140 + svc.Spec.Ports[0].TargetPort = intstr.FromInt32(8140) + svc.Spec.Ports[0].Protocol = corev1.ProtocolTCP + return r.Update(ctx, svc) +} + +// ensureCAServiceDNSAltName injects the CA Service FQDN into the CA server Certificate's +// DNSAltNames so that TLS connections from the operator to the CA service succeed. +func (r *CertificateAuthorityReconciler) ensureCAServiceDNSAltName(ctx context.Context, ca *openvoxv1alpha1.CertificateAuthority, certs []openvoxv1alpha1.Certificate) error { + logger := log.FromContext(ctx) + + serviceFQDN := fmt.Sprintf("%s.%s.svc", ca.Name, ca.Namespace) + + caCert := r.findCAServerCert(ctx, ca, certs) + if caCert == nil { + return nil + } + + if slices.Contains(caCert.Spec.DNSAltNames, serviceFQDN) { + return nil + } + + logger.Info("injecting CA Service FQDN into Certificate DNSAltNames", + "certificate", caCert.Name, "fqdn", serviceFQDN) + caCert.Spec.DNSAltNames = append(caCert.Spec.DNSAltNames, serviceFQDN) + if err := r.Update(ctx, caCert); err != nil { + return fmt.Errorf("updating Certificate %s with CA Service FQDN: %w", caCert.Name, err) + } + + // Reset certificate phase to trigger re-signing with the new alt name + if caCert.Status.Phase == openvoxv1alpha1.CertificatePhaseSigned { + caCert.Status.Phase = openvoxv1alpha1.CertificatePhasePending + if err := r.Status().Update(ctx, caCert); err != nil { + return fmt.Errorf("resetting Certificate %s phase: %w", caCert.Name, err) + } + logger.Info("reset Certificate phase to Pending for re-signing", + "certificate", caCert.Name) + } + + return nil +} diff --git a/internal/controller/helpers.go b/internal/controller/helpers.go index 788ff06..6ccb68b 100644 --- a/internal/controller/helpers.go +++ b/internal/controller/helpers.go @@ -76,48 +76,6 @@ func hashStringMap(data map[string]string) string { return fmt.Sprintf("%x", h.Sum(nil)) } -// findCAServiceName discovers the CA service endpoint by: -// 1. Building the set of Config names that reference this CA -// 2. Finding a running Server with ca:true in one of those Configs -// 3. Finding a Pool referenced by the CA server's poolRefs -// 4. Returning the first matching Pool name as service name -func findCAServiceName(ctx context.Context, reader client.Reader, ca *openvoxv1alpha1.CertificateAuthority, namespace string) string { - logger := log.FromContext(ctx) - - // Build set of Config names referencing this CA - cfgList := &openvoxv1alpha1.ConfigList{} - if err := reader.List(ctx, cfgList, client.InNamespace(namespace)); err != nil { - logger.Error(err, "failed to list Configs for CA service discovery", "namespace", namespace) - return "" - } - configNames := map[string]bool{} - for _, cfg := range cfgList.Items { - if cfg.Spec.AuthorityRef == ca.Name { - configNames[cfg.Name] = true - } - } - - serverList := &openvoxv1alpha1.ServerList{} - if err := reader.List(ctx, serverList, client.InNamespace(namespace)); err != nil { - logger.Error(err, "failed to list Servers for CA service discovery", "namespace", namespace) - return "" - } - - for _, server := range serverList.Items { - if configNames[server.Spec.ConfigRef] && server.Spec.CA { - if server.Status.Phase == openvoxv1alpha1.ServerPhaseRunning { - // Return the first poolRef as the CA service name - if len(server.Spec.PoolRefs) > 0 { - return server.Spec.PoolRefs[0] - } - return "" - } - } - } - - return "" -} - // parseCertNotAfter extracts the NotAfter time from a PEM-encoded certificate. func parseCertNotAfter(ctx context.Context, certPEM []byte) *metav1.Time { logger := log.FromContext(ctx) From 44fe31474e115daa071549954c75f2abe4025096 Mon Sep 17 00:00:00 2001 From: Simon Lauger Date: Thu, 19 Mar 2026 21:03:42 +0100 Subject: [PATCH 08/14] docs: update E2E testing instructions in README Update the E2E section to reflect that tests run on a kind cluster with ghcr.io images. Add workflow commands for building images, document the e2e-stack/e2e-agent/e2e-integration subset targets, and link to the testing docs. --- README.md | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index f57054d..e9d8c06 100644 --- a/README.md +++ b/README.md @@ -203,12 +203,29 @@ Run unit tests: make test ``` -Run E2E tests against the current cluster (requires Docker Desktop Kubernetes or similar). This builds local images, deploys the operator, and runs [Chainsaw](https://kyverno.github.io/chainsaw/) test scenarios (single-node and multi-server): +Run E2E tests against the current cluster. E2E tests require container images in ghcr.io because they run on a kind cluster with the `ImageVolume` feature gate. Build and push images for the current branch via the E2E workflow, then run the tests locally: ```bash +# Build and push images for the current branch +gh workflow run e2e.yaml --ref $(git branch --show-current) + +# Wait for the workflow to finish +gh run watch $(gh run list --workflow=e2e.yaml --limit=1 --json databaseId -q '.[0].databaseId') + +# Run E2E tests make e2e ``` +Subsets of E2E tests can be run separately: + +```bash +make e2e-stack # stack deployment tests (single-node, multi-server) +make e2e-agent # agent tests (basic, broken, idempotent, concurrent) +make e2e-integration # integration tests (enc, report, full) +``` + +See [Testing](docs/development/testing.md) for details. + ### Available Targets | Target | Description | From 4c1710e4c37b938f2a402712c7a08efcaeb2788b Mon Sep 17 00:00:00 2001 From: Simon Lauger Date: Thu, 19 Mar 2026 21:10:48 +0100 Subject: [PATCH 09/14] fix: remove unused test helpers withExternalTLSSecret and withExternalInsecureSkipVerify --- internal/controller/testutil_test.go | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/internal/controller/testutil_test.go b/internal/controller/testutil_test.go index 89ee297..3393aa0 100644 --- a/internal/controller/testutil_test.go +++ b/internal/controller/testutil_test.go @@ -299,24 +299,6 @@ func withExternalCASecret(ref string) caOption { } } -func withExternalTLSSecret(ref string) caOption { - return func(ca *openvoxv1alpha1.CertificateAuthority) { - if ca.Spec.External == nil { - ca.Spec.External = &openvoxv1alpha1.ExternalCASpec{} - } - ca.Spec.External.TLSSecretRef = ref - } -} - -func withExternalInsecureSkipVerify(v bool) caOption { - return func(ca *openvoxv1alpha1.CertificateAuthority) { - if ca.Spec.External == nil { - ca.Spec.External = &openvoxv1alpha1.ExternalCASpec{} - } - ca.Spec.External.InsecureSkipVerify = v - } -} - func newCertificateAuthority(name string, opts ...caOption) *openvoxv1alpha1.CertificateAuthority { ca := &openvoxv1alpha1.CertificateAuthority{ ObjectMeta: metav1.ObjectMeta{ From f74a968fde1e4f2a0a3834773d08f06eff7545f3 Mon Sep 17 00:00:00 2001 From: Simon Lauger Date: Thu, 19 Mar 2026 21:11:57 +0100 Subject: [PATCH 10/14] docs: add subset make targets and controller tests to testing docs --- docs/development/testing.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/docs/development/testing.md b/docs/development/testing.md index a16fdbd..a97614d 100644 --- a/docs/development/testing.md +++ b/docs/development/testing.md @@ -10,6 +10,8 @@ make test This runs `go test ./...` with coverage output. Tests include: +- Controller reconciliation: Config, Server, Pool, Certificate, CertificateAuthority, ReportProcessor (`internal/controller/`) +- Certificate signing and CA HTTP client (`internal/controller/`) - Duration parsing (`api/v1alpha1/duration.go`) - Volume helpers, hash functions, image resolution (`internal/controller/helpers.go`) - Label generation (`internal/controller/labels.go`) @@ -77,6 +79,14 @@ This will: 1. Deploy the operator via Helm (pulling from ghcr.io) 2. Run all Chainsaw test scenarios +Subsets of tests can be run separately: + +```bash +make e2e-stack # stack deployment tests (single-node, multi-server) +make e2e-agent # agent tests (basic, broken, idempotent, concurrent) +make e2e-integration # integration tests (enc, report, full) +``` + ### Test Scenarios Tests are located in `tests/e2e/` with a shared configuration in `tests/e2e/chainsaw-config.yaml`. From 164ba87ff01e709b24076b204c7b4c28d7728935 Mon Sep 17 00:00:00 2001 From: Simon Lauger Date: Thu, 19 Mar 2026 21:21:32 +0100 Subject: [PATCH 11/14] docs: document IMAGE_TAG usage for E2E tests Explain how the E2E workflow tags images with the short git SHA and how to override IMAGE_TAG/IMAGE_REGISTRY when running make e2e locally. --- README.md | 5 ++++- docs/development/testing.md | 25 +++++++++++++++++++++---- 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index e9d8c06..b2cbd28 100644 --- a/README.md +++ b/README.md @@ -212,7 +212,10 @@ gh workflow run e2e.yaml --ref $(git branch --show-current) # Wait for the workflow to finish gh run watch $(gh run list --workflow=e2e.yaml --limit=1 --json databaseId -q '.[0].databaseId') -# Run E2E tests +# Set the image tag (short git SHA, matches what CI built) +export IMAGE_TAG=$(git describe --always) + +# Run E2E tests (uses IMAGE_TAG automatically) make e2e ``` diff --git a/docs/development/testing.md b/docs/development/testing.md index a97614d..8914852 100644 --- a/docs/development/testing.md +++ b/docs/development/testing.md @@ -43,17 +43,15 @@ E2E tests require 5 container images: `openvox-operator`, `openvox-server`, `ope The E2E agent tests deploy Puppet code via OCI volume mounts (`image` volumes), which require the Kubernetes `ImageVolume` feature gate. This feature is default-enabled since Kubernetes 1.35, but Docker Desktop currently ships Kubernetes 1.34 via its built-in kubeadm provider -- and there is no way to inject custom feature gates into that provider. The workaround is to run a kind cluster inside Docker Desktop, where feature gates can be configured via the kind config (`tests/e2e/kind-config.yaml`). However, kind clusters cannot access locally built images directly -- images must be available in a registry. This is why the `e2e.yaml` workflow pushes all images to ghcr.io before tests can run. -Images are pushed to `ghcr.io/slauger/` with a short SHA tag (e.g. `efac063`) and `:latest`. - #### Building Images via CI Trigger the **E2E** workflow to build and push all 5 images for the current branch: ```bash -gh workflow run e2e.yaml +gh workflow run e2e.yaml --ref $(git branch --show-current) ``` -This runs `_container-build.yaml` for each image (multi-arch, hadolint, push to ghcr.io). On `main`, the regular CI workflows (`ci.yaml`, `ci-test-images.yaml`) build the same images automatically. +Images are tagged with the short git SHA of the commit at the branch tip (e.g. `efac063`). This runs `_container-build.yaml` for each image (multi-arch, hadolint, push to ghcr.io). On `main`, the regular CI workflows (`ci.yaml`, `ci-test-images.yaml`) build the same images automatically. #### Building Images Locally @@ -87,6 +85,25 @@ make e2e-agent # agent tests (basic, broken, idempotent, concurrent) make e2e-integration # integration tests (enc, report, full) ``` +### Image Tags + +The E2E workflow tags all images with the short git SHA of the built commit (e.g. `efac063`). The Makefile defaults `IMAGE_TAG` to your local `git describe --always` output, so it matches automatically when your local HEAD is the commit CI built. + +To use a specific tag, export `IMAGE_TAG` once for the session: + +```bash +export IMAGE_TAG=efac063 +make e2e +``` + +Or pass it inline to a single command: + +```bash +IMAGE_TAG=efac063 make e2e-stack +``` + +The `IMAGE_REGISTRY` variable (default: `ghcr.io/slauger`) can be overridden the same way if using a different registry. + ### Test Scenarios Tests are located in `tests/e2e/` with a shared configuration in `tests/e2e/chainsaw-config.yaml`. From 1142ce2cf1eae03de91971693cb230f77bb67619 Mon Sep 17 00:00:00 2001 From: Simon Lauger Date: Thu, 19 Mar 2026 21:26:11 +0100 Subject: [PATCH 12/14] feat: add image_tag input to E2E workflow for custom image tags Add an optional image_tag input to the e2e.yaml workflow that gets passed through to _container-build.yaml. When set, it overrides the default short git SHA tag. This allows building images with descriptive names like 'my-feature' instead of commit hashes. Usage: gh workflow run e2e.yaml -f image_tag=my-feature Update README and testing docs with examples for both the workflow input and the corresponding IMAGE_TAG env var for make targets. --- .github/workflows/_container-build.yaml | 11 +++++++++-- .github/workflows/e2e.yaml | 11 +++++++++++ README.md | 9 ++++++--- docs/development/testing.md | 22 +++++++++++++++------- 4 files changed, 41 insertions(+), 12 deletions(-) diff --git a/.github/workflows/_container-build.yaml b/.github/workflows/_container-build.yaml index 33184ad..327146b 100644 --- a/.github/workflows/_container-build.yaml +++ b/.github/workflows/_container-build.yaml @@ -26,6 +26,11 @@ on: required: false type: boolean default: false + image_tag: + description: 'Custom image tag (default: short git SHA or version tag)' + required: false + type: string + default: '' outputs: version: description: 'Image version tag' @@ -44,10 +49,12 @@ jobs: - name: Determine version id: version run: | - SHORT_SHA=$(echo "${{ github.sha }}" | cut -c1-7) - if [[ "${{ github.ref }}" == refs/tags/v* ]]; then + if [[ -n "${{ inputs.image_tag }}" ]]; then + echo "version=${{ inputs.image_tag }}" >> $GITHUB_OUTPUT + elif [[ "${{ github.ref }}" == refs/tags/v* ]]; then echo "version=${GITHUB_REF#refs/tags/v}" >> $GITHUB_OUTPUT else + SHORT_SHA=$(echo "${{ github.sha }}" | cut -c1-7) echo "version=${SHORT_SHA}" >> $GITHUB_OUTPUT fi diff --git a/.github/workflows/e2e.yaml b/.github/workflows/e2e.yaml index 595c088..2db5f44 100644 --- a/.github/workflows/e2e.yaml +++ b/.github/workflows/e2e.yaml @@ -2,6 +2,12 @@ name: E2E on: workflow_dispatch: + inputs: + image_tag: + description: 'Custom image tag (default: short git SHA)' + required: false + type: string + default: '' permissions: contents: read @@ -18,6 +24,7 @@ jobs: dockerfile: images/openvox-operator/Containerfile context: '.' push: true + image_tag: ${{ inputs.image_tag }} openvox-server: uses: ./.github/workflows/_container-build.yaml @@ -29,6 +36,7 @@ jobs: dockerfile: images/openvox-server/Containerfile context: '.' push: true + image_tag: ${{ inputs.image_tag }} openvox-code: uses: ./.github/workflows/_container-build.yaml @@ -40,6 +48,7 @@ jobs: dockerfile: images/openvox-code/Containerfile context: '.' push: true + image_tag: ${{ inputs.image_tag }} openvox-agent: uses: ./.github/workflows/_container-build.yaml @@ -51,6 +60,7 @@ jobs: dockerfile: images/openvox-agent/Containerfile context: 'images/openvox-agent' push: true + image_tag: ${{ inputs.image_tag }} openvox-mock: uses: ./.github/workflows/_container-build.yaml @@ -62,3 +72,4 @@ jobs: dockerfile: images/openvox-mock/Containerfile context: '.' push: true + image_tag: ${{ inputs.image_tag }} diff --git a/README.md b/README.md index b2cbd28..4b6494a 100644 --- a/README.md +++ b/README.md @@ -206,14 +206,17 @@ make test Run E2E tests against the current cluster. E2E tests require container images in ghcr.io because they run on a kind cluster with the `ImageVolume` feature gate. Build and push images for the current branch via the E2E workflow, then run the tests locally: ```bash -# Build and push images for the current branch +# Build and push images for the current branch (tagged with short git SHA) gh workflow run e2e.yaml --ref $(git branch --show-current) +# Or use a custom image tag +gh workflow run e2e.yaml --ref $(git branch --show-current) -f image_tag=my-feature + # Wait for the workflow to finish gh run watch $(gh run list --workflow=e2e.yaml --limit=1 --json databaseId -q '.[0].databaseId') -# Set the image tag (short git SHA, matches what CI built) -export IMAGE_TAG=$(git describe --always) +# Set the image tag to match what CI built +export IMAGE_TAG=$(git describe --always) # or: export IMAGE_TAG=my-feature # Run E2E tests (uses IMAGE_TAG automatically) make e2e diff --git a/docs/development/testing.md b/docs/development/testing.md index 8914852..c79aaf6 100644 --- a/docs/development/testing.md +++ b/docs/development/testing.md @@ -51,7 +51,13 @@ Trigger the **E2E** workflow to build and push all 5 images for the current bran gh workflow run e2e.yaml --ref $(git branch --show-current) ``` -Images are tagged with the short git SHA of the commit at the branch tip (e.g. `efac063`). This runs `_container-build.yaml` for each image (multi-arch, hadolint, push to ghcr.io). On `main`, the regular CI workflows (`ci.yaml`, `ci-test-images.yaml`) build the same images automatically. +By default, images are tagged with the short git SHA of the commit at the branch tip (e.g. `efac063`). You can pass a custom tag via the `image_tag` input: + +```bash +gh workflow run e2e.yaml --ref $(git branch --show-current) -f image_tag=my-feature +``` + +This runs `_container-build.yaml` for each image (multi-arch, hadolint, push to ghcr.io). On `main`, the regular CI workflows (`ci.yaml`, `ci-test-images.yaml`) build the same images automatically. #### Building Images Locally @@ -87,19 +93,19 @@ make e2e-integration # integration tests (enc, report, full) ### Image Tags -The E2E workflow tags all images with the short git SHA of the built commit (e.g. `efac063`). The Makefile defaults `IMAGE_TAG` to your local `git describe --always` output, so it matches automatically when your local HEAD is the commit CI built. +The E2E workflow tags all images with the short git SHA of the built commit (e.g. `efac063`) by default. A custom tag can be passed via the `image_tag` workflow input (see [Building Images via CI](#building-images-via-ci)). -To use a specific tag, export `IMAGE_TAG` once for the session: +The Makefile defaults `IMAGE_TAG` to your local `git describe --always` output, so it matches automatically when your local HEAD is the commit CI built. To use a custom tag, export `IMAGE_TAG` once for the session: ```bash -export IMAGE_TAG=efac063 +export IMAGE_TAG=my-feature make e2e ``` Or pass it inline to a single command: ```bash -IMAGE_TAG=efac063 make e2e-stack +IMAGE_TAG=my-feature make e2e-stack ``` The `IMAGE_REGISTRY` variable (default: `ghcr.io/slauger`) can be overridden the same way if using a different registry. @@ -207,7 +213,7 @@ Image builds and E2E tests are managed by three workflows: | Workflow | Trigger | What it does | |----------|---------|-------------| -| `e2e.yaml` | `workflow_dispatch` | Builds all 5 images and pushes to ghcr.io | +| `e2e.yaml` | `workflow_dispatch` | Builds all 5 images and pushes to ghcr.io (accepts optional `image_tag` input) | | `ci-test-images.yaml` | Push to `main` (path filter) | Builds agent, code, mock on main | | `cleanup.yaml` | `workflow_dispatch` | Deletes E2E image versions (short SHA tags) | @@ -215,12 +221,14 @@ The typical workflow for validating a feature branch before merging: ```bash # 1. Build all images for the current branch -gh workflow run e2e.yaml +gh workflow run e2e.yaml --ref $(git branch --show-current) +# Or with a custom tag: gh workflow run e2e.yaml --ref $(git branch --show-current) -f image_tag=my-feature # 2. Check build status gh run list --workflow=e2e.yaml --limit=1 # 3. Run E2E tests locally against a cluster that can pull from ghcr.io +export IMAGE_TAG=$(git describe --always) # or: export IMAGE_TAG=my-feature make e2e # 4. Clean up E2E images after merging From 4df83c813e2a519f6eadf5c139a6bb49ece7a184 Mon Sep 17 00:00:00 2001 From: Simon Lauger Date: Thu, 19 Mar 2026 21:37:47 +0100 Subject: [PATCH 13/14] fix: inject CA Service FQDN into job env instead of modifying Certificate CR Move the SAN injection from ensureCAServiceDNSAltName() (which mutated the Certificate CR spec, breaking Helm drift detection) into buildCASetupJob() where the FQDN is appended to the DNS_ALT_NAMES env var transparently. The Certificate CR is never modified by the controller. --- .../certificateauthority_controller.go | 5 -- .../certificateauthority_controller_test.go | 63 +++++++------------ .../controller/certificateauthority_job.go | 12 +++- .../certificateauthority_service.go | 38 ----------- 4 files changed, 35 insertions(+), 83 deletions(-) diff --git a/internal/controller/certificateauthority_controller.go b/internal/controller/certificateauthority_controller.go index eab8dc3..8efb954 100644 --- a/internal/controller/certificateauthority_controller.go +++ b/internal/controller/certificateauthority_controller.go @@ -94,11 +94,6 @@ func (r *CertificateAuthorityReconciler) Reconcile(ctx context.Context, req ctrl return ctrl.Result{}, fmt.Errorf("reconciling CA Service: %w", err) } - // Step 2b: Inject CA Service FQDN into CA server Certificate SANs - if err := r.ensureCAServiceDNSAltName(ctx, ca, certs); err != nil { - return ctrl.Result{}, fmt.Errorf("ensuring CA Service DNS alt name: %w", err) - } - // Step 3: Ensure RBAC for CA setup job if err := r.reconcileCASetupRBAC(ctx, ca, certs); err != nil { return ctrl.Result{}, fmt.Errorf("reconciling CA setup RBAC: %w", err) diff --git a/internal/controller/certificateauthority_controller_test.go b/internal/controller/certificateauthority_controller_test.go index 4e2efab..7455c45 100644 --- a/internal/controller/certificateauthority_controller_test.go +++ b/internal/controller/certificateauthority_controller_test.go @@ -1,6 +1,7 @@ package controller import ( + "strings" "testing" "time" @@ -513,15 +514,15 @@ func TestCAReconcile_ServiceCreation(t *testing.T) { } } -func TestCAReconcile_SANInjection(t *testing.T) { +func TestCAReconcile_JobIncludesServiceFQDN(t *testing.T) { ca := newCertificateAuthority("test-ca") ca.Status.Phase = "" cfg := caPrereqs("test-ca") - // Create a Server with ca:true pointing to the cert server := newServer("ca-server", withCA(true), withServerRole(true)) server.Spec.ConfigRef = "production" server.Spec.CertificateRef = "ca-cert" cert := newCertificate("ca-cert", "test-ca", openvoxv1alpha1.CertificatePhasePending) + cert.Spec.DNSAltNames = []string{"puppet", "ca.example.com"} c := setupTestClient(ca, cfg, server, cert) r := newCertificateAuthorityReconciler(c) @@ -529,57 +530,41 @@ func TestCAReconcile_SANInjection(t *testing.T) { t.Fatalf("reconcile error: %v", err) } - // Verify the FQDN was injected into the Certificate's DNSAltNames - updatedCert := &openvoxv1alpha1.Certificate{} - if err := c.Get(testCtx(), types.NamespacedName{Name: "ca-cert", Namespace: testNamespace}, updatedCert); err != nil { - t.Fatalf("failed to get Certificate: %v", err) + // Verify the Job's DNS_ALT_NAMES env var includes the CA Service FQDN + job := &batchv1.Job{} + if err := c.Get(testCtx(), types.NamespacedName{Name: "test-ca-ca-setup", Namespace: testNamespace}, job); err != nil { + t.Fatalf("Job not created: %v", err) } - expectedFQDN := "test-ca.default.svc" - found := false - for _, san := range updatedCert.Spec.DNSAltNames { - if san == expectedFQDN { - found = true - break - } - } - if !found { - t.Errorf("expected FQDN %q in DNSAltNames, got %v", expectedFQDN, updatedCert.Spec.DNSAltNames) + envMap := map[string]string{} + for _, e := range job.Spec.Template.Spec.Containers[0].Env { + envMap[e.Name] = e.Value } -} -func TestCAReconcile_SANInjectionIdempotent(t *testing.T) { - ca := newCertificateAuthority("test-ca") - ca.Status.Phase = "" - cfg := caPrereqs("test-ca") - server := newServer("ca-server", withCA(true), withServerRole(true)) - server.Spec.ConfigRef = "production" - server.Spec.CertificateRef = "ca-cert" - cert := newCertificate("ca-cert", "test-ca", openvoxv1alpha1.CertificatePhasePending) - // Pre-inject the FQDN - cert.Spec.DNSAltNames = []string{"test-ca.default.svc"} - c := setupTestClient(ca, cfg, server, cert) - r := newCertificateAuthorityReconciler(c) + dnsAltNames := envMap["DNS_ALT_NAMES"] + if dnsAltNames == "" { + t.Fatal("DNS_ALT_NAMES env var not set") + } - if _, err := r.Reconcile(testCtx(), testRequest("test-ca")); err != nil { - t.Fatalf("reconcile error: %v", err) + // Should contain original SANs plus the CA Service FQDN + expectedFQDN := "test-ca.default.svc" + if !strings.Contains(dnsAltNames, expectedFQDN) { + t.Errorf("expected DNS_ALT_NAMES to contain %q, got %q", expectedFQDN, dnsAltNames) + } + if !strings.Contains(dnsAltNames, "puppet") { + t.Errorf("expected DNS_ALT_NAMES to contain original SAN 'puppet', got %q", dnsAltNames) } - // Verify no duplicate was added + // Verify Certificate CR was NOT modified updatedCert := &openvoxv1alpha1.Certificate{} if err := c.Get(testCtx(), types.NamespacedName{Name: "ca-cert", Namespace: testNamespace}, updatedCert); err != nil { t.Fatalf("failed to get Certificate: %v", err) } - - count := 0 for _, san := range updatedCert.Spec.DNSAltNames { - if san == "test-ca.default.svc" { - count++ + if san == expectedFQDN { + t.Error("CA Service FQDN should NOT be injected into Certificate CR spec") } } - if count != 1 { - t.Errorf("expected FQDN once in DNSAltNames, found %d times: %v", count, updatedCert.Spec.DNSAltNames) - } } func TestCAReconcile_StatusServiceName(t *testing.T) { diff --git a/internal/controller/certificateauthority_job.go b/internal/controller/certificateauthority_job.go index 0c3bc56..6fb838b 100644 --- a/internal/controller/certificateauthority_job.go +++ b/internal/controller/certificateauthority_job.go @@ -3,6 +3,7 @@ package controller import ( "context" "fmt" + "slices" "strings" batchv1 "k8s.io/api/batch/v1" @@ -124,7 +125,16 @@ func (r *CertificateAuthorityReconciler) buildCASetupJob(ctx context.Context, ca if caCert.Spec.Certname != "" { certname = caCert.Spec.Certname } - dnsAltNames = strings.Join(caCert.Spec.DNSAltNames, ",") + // Append CA Service FQDN to DNS alt names so the CA server cert + // is valid for internal operator communication (CSR signing, CRL refresh). + // This is done here transparently without modifying the Certificate CR. + serviceFQDN := fmt.Sprintf("%s.%s.svc", ca.Name, ca.Namespace) + altNames := make([]string, len(caCert.Spec.DNSAltNames)) + copy(altNames, caCert.Spec.DNSAltNames) + if !slices.Contains(altNames, serviceFQDN) { + altNames = append(altNames, serviceFQDN) + } + dnsAltNames = strings.Join(altNames, ",") tlsSecretName = fmt.Sprintf("%s-tls", caCert.Name) certResourceName = caCert.Name } diff --git a/internal/controller/certificateauthority_service.go b/internal/controller/certificateauthority_service.go index 827e59a..fedca3c 100644 --- a/internal/controller/certificateauthority_service.go +++ b/internal/controller/certificateauthority_service.go @@ -2,8 +2,6 @@ package controller import ( "context" - "fmt" - "slices" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" @@ -71,39 +69,3 @@ func (r *CertificateAuthorityReconciler) reconcileCAService(ctx context.Context, svc.Spec.Ports[0].Protocol = corev1.ProtocolTCP return r.Update(ctx, svc) } - -// ensureCAServiceDNSAltName injects the CA Service FQDN into the CA server Certificate's -// DNSAltNames so that TLS connections from the operator to the CA service succeed. -func (r *CertificateAuthorityReconciler) ensureCAServiceDNSAltName(ctx context.Context, ca *openvoxv1alpha1.CertificateAuthority, certs []openvoxv1alpha1.Certificate) error { - logger := log.FromContext(ctx) - - serviceFQDN := fmt.Sprintf("%s.%s.svc", ca.Name, ca.Namespace) - - caCert := r.findCAServerCert(ctx, ca, certs) - if caCert == nil { - return nil - } - - if slices.Contains(caCert.Spec.DNSAltNames, serviceFQDN) { - return nil - } - - logger.Info("injecting CA Service FQDN into Certificate DNSAltNames", - "certificate", caCert.Name, "fqdn", serviceFQDN) - caCert.Spec.DNSAltNames = append(caCert.Spec.DNSAltNames, serviceFQDN) - if err := r.Update(ctx, caCert); err != nil { - return fmt.Errorf("updating Certificate %s with CA Service FQDN: %w", caCert.Name, err) - } - - // Reset certificate phase to trigger re-signing with the new alt name - if caCert.Status.Phase == openvoxv1alpha1.CertificatePhaseSigned { - caCert.Status.Phase = openvoxv1alpha1.CertificatePhasePending - if err := r.Status().Update(ctx, caCert); err != nil { - return fmt.Errorf("resetting Certificate %s phase: %w", caCert.Name, err) - } - logger.Info("reset Certificate phase to Pending for re-signing", - "certificate", caCert.Name) - } - - return nil -} From 4970694e5dbf2d6c91118d04a141932a6f00329e Mon Sep 17 00:00:00 2001 From: Simon Lauger Date: Thu, 19 Mar 2026 21:49:48 +0100 Subject: [PATCH 14/14] fix: rename CA internal service to -internal to avoid Pool naming conflict The CA controller's internal ClusterIP Service was named , which conflicts with the Pool controller's Service for the ca pool. Rename to -internal so the Pool name stays available for user-configured service types (ClusterIP, LoadBalancer, NodePort). --- internal/controller/certificate_controller.go | 2 +- .../certificateauthority_controller.go | 2 +- .../certificateauthority_controller_test.go | 16 ++++++++-------- internal/controller/certificateauthority_crl.go | 2 +- internal/controller/certificateauthority_job.go | 2 +- .../controller/certificateauthority_service.go | 2 +- internal/controller/helpers.go | 6 ++++++ 7 files changed, 19 insertions(+), 13 deletions(-) diff --git a/internal/controller/certificate_controller.go b/internal/controller/certificate_controller.go index 257b815..231333b 100644 --- a/internal/controller/certificate_controller.go +++ b/internal/controller/certificate_controller.go @@ -128,7 +128,7 @@ func (r *CertificateReconciler) reconcileCertSigning(ctx context.Context, cert * if ca.Spec.External != nil { caBaseURL = ca.Spec.External.URL } else { - caBaseURL = fmt.Sprintf("https://%s.%s.svc:8140", ca.Name, cert.Namespace) + caBaseURL = fmt.Sprintf("https://%s.%s.svc:8140", caInternalServiceName(ca.Name), cert.Namespace) } cert.Status.Phase = openvoxv1alpha1.CertificatePhaseRequesting diff --git a/internal/controller/certificateauthority_controller.go b/internal/controller/certificateauthority_controller.go index 8efb954..bd5a31f 100644 --- a/internal/controller/certificateauthority_controller.go +++ b/internal/controller/certificateauthority_controller.go @@ -113,7 +113,7 @@ func (r *CertificateAuthorityReconciler) Reconcile(ctx context.Context, req ctrl caSecretName := fmt.Sprintf("%s-ca", ca.Name) ca.Status.Phase = openvoxv1alpha1.CertificateAuthorityPhaseReady ca.Status.CASecretName = caSecretName - ca.Status.ServiceName = ca.Name + ca.Status.ServiceName = caInternalServiceName(ca.Name) ca.Status.NotAfter = r.extractCANotAfter(ctx, caSecretName, ca.Namespace) meta.SetStatusCondition(&ca.Status.Conditions, metav1.Condition{ Type: openvoxv1alpha1.ConditionCAReady, diff --git a/internal/controller/certificateauthority_controller_test.go b/internal/controller/certificateauthority_controller_test.go index 7455c45..6fd3fff 100644 --- a/internal/controller/certificateauthority_controller_test.go +++ b/internal/controller/certificateauthority_controller_test.go @@ -475,13 +475,13 @@ func TestCAReconcile_ServiceCreation(t *testing.T) { } svc := &corev1.Service{} - if err := c.Get(testCtx(), types.NamespacedName{Name: "test-ca", Namespace: testNamespace}, svc); err != nil { + if err := c.Get(testCtx(), types.NamespacedName{Name: "test-ca-internal", Namespace: testNamespace}, svc); err != nil { t.Fatalf("Service not created: %v", err) } // Verify name - if svc.Name != "test-ca" { - t.Errorf("expected Service name %q, got %q", "test-ca", svc.Name) + if svc.Name != "test-ca-internal" { + t.Errorf("expected Service name %q, got %q", "test-ca-internal", svc.Name) } // Verify port @@ -546,8 +546,8 @@ func TestCAReconcile_JobIncludesServiceFQDN(t *testing.T) { t.Fatal("DNS_ALT_NAMES env var not set") } - // Should contain original SANs plus the CA Service FQDN - expectedFQDN := "test-ca.default.svc" + // Should contain original SANs plus the CA internal Service FQDN + expectedFQDN := "test-ca-internal.default.svc" if !strings.Contains(dnsAltNames, expectedFQDN) { t.Errorf("expected DNS_ALT_NAMES to contain %q, got %q", expectedFQDN, dnsAltNames) } @@ -586,8 +586,8 @@ func TestCAReconcile_StatusServiceName(t *testing.T) { t.Fatalf("failed to get CA: %v", err) } - if updated.Status.ServiceName != "test-ca" { - t.Errorf("expected status.serviceName %q, got %q", "test-ca", updated.Status.ServiceName) + if updated.Status.ServiceName != "test-ca-internal" { + t.Errorf("expected status.serviceName %q, got %q", "test-ca-internal", updated.Status.ServiceName) } } @@ -603,7 +603,7 @@ func TestCAReconcile_ExternalCA_NoService(t *testing.T) { // No Service should be created for external CA svc := &corev1.Service{} - if err := c.Get(testCtx(), types.NamespacedName{Name: "ext-ca", Namespace: testNamespace}, svc); err == nil { + if err := c.Get(testCtx(), types.NamespacedName{Name: "ext-ca-internal", Namespace: testNamespace}, svc); err == nil { t.Error("expected no Service for external CA, but one was created") } } diff --git a/internal/controller/certificateauthority_crl.go b/internal/controller/certificateauthority_crl.go index fb713be..83ee100 100644 --- a/internal/controller/certificateauthority_crl.go +++ b/internal/controller/certificateauthority_crl.go @@ -34,7 +34,7 @@ func (r *CertificateAuthorityReconciler) reconcileCRLRefresh(ctx context.Context if ca.Spec.External != nil { caBaseURL = ca.Spec.External.URL } else { - caBaseURL = fmt.Sprintf("https://%s.%s.svc:8140", ca.Name, ca.Namespace) + caBaseURL = fmt.Sprintf("https://%s.%s.svc:8140", caInternalServiceName(ca.Name), ca.Namespace) } crlPEM, err := r.fetchCRL(ctx, ca, caBaseURL, ca.Namespace) diff --git a/internal/controller/certificateauthority_job.go b/internal/controller/certificateauthority_job.go index 6fb838b..900b4cf 100644 --- a/internal/controller/certificateauthority_job.go +++ b/internal/controller/certificateauthority_job.go @@ -128,7 +128,7 @@ func (r *CertificateAuthorityReconciler) buildCASetupJob(ctx context.Context, ca // Append CA Service FQDN to DNS alt names so the CA server cert // is valid for internal operator communication (CSR signing, CRL refresh). // This is done here transparently without modifying the Certificate CR. - serviceFQDN := fmt.Sprintf("%s.%s.svc", ca.Name, ca.Namespace) + serviceFQDN := fmt.Sprintf("%s.%s.svc", caInternalServiceName(ca.Name), ca.Namespace) altNames := make([]string, len(caCert.Spec.DNSAltNames)) copy(altNames, caCert.Spec.DNSAltNames) if !slices.Contains(altNames, serviceFQDN) { diff --git a/internal/controller/certificateauthority_service.go b/internal/controller/certificateauthority_service.go index fedca3c..970b7e5 100644 --- a/internal/controller/certificateauthority_service.go +++ b/internal/controller/certificateauthority_service.go @@ -18,7 +18,7 @@ import ( // communication with the CA (CSR signing, CRL refresh). func (r *CertificateAuthorityReconciler) reconcileCAService(ctx context.Context, ca *openvoxv1alpha1.CertificateAuthority) error { logger := log.FromContext(ctx) - svcName := ca.Name + svcName := caInternalServiceName(ca.Name) labels := caLabels(ca.Name) selector := map[string]string{ diff --git a/internal/controller/helpers.go b/internal/controller/helpers.go index 6ccb68b..c0a33f7 100644 --- a/internal/controller/helpers.go +++ b/internal/controller/helpers.go @@ -136,6 +136,12 @@ func createOrUpdateSecret(ctx context.Context, c client.Client, scheme *runtime. return c.Update(ctx, secret) } +// caInternalServiceName returns the name of the internal ClusterIP Service +// created by the CA controller for operator communication (CSR signing, CRL refresh). +func caInternalServiceName(caName string) string { + return fmt.Sprintf("%s-internal", caName) +} + // resolveCode determines the code source for a Server. // Priority: Server override > Config default. func resolveCode(server *openvoxv1alpha1.Server, cfg *openvoxv1alpha1.Config) *openvoxv1alpha1.CodeSpec {