diff --git a/e2e-tests/tls-issue-cert-manager/compare/certificate_some-name-tls-issue-ca-cert-updated.yml b/e2e-tests/tls-issue-cert-manager/compare/certificate_some-name-tls-issue-ca-cert-updated.yml new file mode 100644 index 0000000000..0281775631 --- /dev/null +++ b/e2e-tests/tls-issue-cert-manager/compare/certificate_some-name-tls-issue-ca-cert-updated.yml @@ -0,0 +1,19 @@ +apiVersion: cert-manager.io/v1 +kind: Certificate +metadata: + generation: 2 + labels: + app.kubernetes.io/instance: some-name-tls-issue + app.kubernetes.io/managed-by: percona-xtradb-cluster-operator + app.kubernetes.io/name: percona-xtradb-cluster + app.kubernetes.io/part-of: percona-xtradb-cluster + name: some-name-tls-issue-ca-cert +spec: + commonName: some-name-tls-issue-ca + duration: 20000h0m0s + isCA: true + issuerRef: + kind: Issuer + name: some-name-tls-issue-pxc-ca-issuer + renewBefore: 730h0m0s + secretName: some-name-tls-issue-ca-cert diff --git a/e2e-tests/tls-issue-cert-manager/compare/certificate_some-name-tls-issue-ssl-updated.yml b/e2e-tests/tls-issue-cert-manager/compare/certificate_some-name-tls-issue-ssl-updated.yml new file mode 100644 index 0000000000..b3bf63002f --- /dev/null +++ b/e2e-tests/tls-issue-cert-manager/compare/certificate_some-name-tls-issue-ssl-updated.yml @@ -0,0 +1,24 @@ +apiVersion: cert-manager.io/v1 +kind: Certificate +metadata: + generation: 2 + labels: + app.kubernetes.io/instance: some-name-tls-issue + app.kubernetes.io/managed-by: percona-xtradb-cluster-operator + app.kubernetes.io/name: percona-xtradb-cluster + app.kubernetes.io/part-of: percona-xtradb-cluster + name: some-name-tls-issue-ssl +spec: + commonName: some-name-tls-issue-proxysql + dnsNames: + - some-name-tls-issue-pxc + - some-name-tls-issue-proxysql + - '*.some-name-tls-issue-pxc' + - '*.some-name-tls-issue-proxysql' + - test.com + - new-san.example.com + duration: 2160h0m0s + issuerRef: + kind: Issuer + name: some-name-tls-issue-pxc-issuer + secretName: some-name-tls-issue-ssl diff --git a/e2e-tests/tls-issue-cert-manager/conf/some-name-tls-issue-updated.yml b/e2e-tests/tls-issue-cert-manager/conf/some-name-tls-issue-updated.yml new file mode 100644 index 0000000000..0d15edfc64 --- /dev/null +++ b/e2e-tests/tls-issue-cert-manager/conf/some-name-tls-issue-updated.yml @@ -0,0 +1,100 @@ +apiVersion: pxc.percona.com/v1 +kind: PerconaXtraDBCluster +metadata: + name: some-name-tls-issue + finalizers: + - percona.com/delete-pxc-pods-in-order +spec: + tls: + certValidityDuration: 2160h + caValidityDuration: 20000h + SANs: + - test.com + - new-san.example.com + secretsName: my-cluster-secrets + vaultSecretName: some-name-vault + pause: false + pxc: + size: 3 + image: -pxc + resources: + requests: + memory: 0.1G + cpu: 100m + limits: + memory: "2G" + cpu: "1" + volumeSpec: + persistentVolumeClaim: + resources: + requests: + storage: 2Gi + affinity: + antiAffinityTopologyKey: "kubernetes.io/hostname" + proxysql: + enabled: false + size: 2 + image: -proxysql + resources: + requests: + memory: 0.1G + cpu: 100m + limits: + memory: 1G + cpu: 700m + affinity: + antiAffinityTopologyKey: "kubernetes.io/hostname" + volumeSpec: + persistentVolumeClaim: + resources: + requests: + storage: 2Gi + haproxy: + enabled: true + size: 2 + image: -haproxy + resources: + requests: + memory: 0.1G + cpu: 100m + limits: + memory: 1G + cpu: 700m + affinity: + antiAffinityTopologyKey: "kubernetes.io/hostname" + pmm: + enabled: false + image: perconalab/pmm-client:1.17.1 + serverHost: monitoring-service + serverUser: pmm + backup: + image: -backup + storages: + pvc: + type: filesystem + volume: + persistentVolumeClaim: + accessModes: [ "ReadWriteOnce" ] + resources: + requests: + storage: 1Gi + aws-s3: + type: s3 + s3: + region: us-east-1 + bucket: operator-testing + credentialsSecret: aws-s3-secret + minio: + type: s3 + s3: + credentialsSecret: minio-secret + region: us-east-1 + bucket: operator-testing + endpointUrl: http://minio-service:9000/ + gcp-cs: + type: s3 + s3: + credentialsSecret: gcp-cs-secret + region: us-east-1 + bucket: operator-testing + endpointUrl: https://storage.googleapis.com diff --git a/e2e-tests/tls-issue-cert-manager/run b/e2e-tests/tls-issue-cert-manager/run index 341f812aae..9386bf7352 100755 --- a/e2e-tests/tls-issue-cert-manager/run +++ b/e2e-tests/tls-issue-cert-manager/run @@ -17,6 +17,25 @@ check_verify_identity() { bash -c "printf '%s\n' \"${command}\" | mysql -sN $args" } +wait_for_certificate_generation() { + local certificate="$1" + local target_generation="$2" + local retries="${3:-36}" + local current_generation + + for _ in $(seq 1 "$retries"); do + current_generation=$(kubectl_bin get certificate "$certificate" -o jsonpath='{.metadata.generation}' 2>/dev/null || true) + if [ "$current_generation" = "$target_generation" ]; then + return 0 + fi + echo "Certificate $certificate is at generation ${current_generation:-unknown}; waiting for generation $target_generation" + sleep 5 + done + + echo "Certificate $certificate did not reach generation $target_generation after $retries retries" + return 1 +} + trigger_ca_rotation() { desc 'set rotationPolicy=Always on CA certificate' kubectl_bin patch certificate "$cluster-ca-cert" --type=merge \ @@ -138,6 +157,24 @@ main() { desc 'check ssl-internal certificate using HAProxy' check_verify_identity "$cluster-haproxy" + desc 'update tls spec: change duration and add new SAN' + apply_config "$test_dir/conf/$cluster-updated.yml" + + desc 'wait for cert-manager certificate specs to be updated' + wait_for_certificate_generation "$cluster-ca-cert" 2 + wait_for_certificate_generation "$cluster-ssl" 2 + + desc 'check if CA certificate updated' + compare_kubectl certificate/$cluster-ca-cert "-updated" + + desc 'check if SSL certificate updated' + compare_kubectl certificate/$cluster-ssl "-updated" + + wait_cluster_consistency "$cluster" 3 2 + + desc 'check ssl-internal certificate using PXC after update' + check_verify_identity "$cluster-pxc" + desc 'trigger CA rotation and verify leaf cert re-issuance' trigger_ca_rotation diff --git a/pkg/controller/pxc/tls.go b/pkg/controller/pxc/tls.go index 9909153202..2d3c63704d 100644 --- a/pkg/controller/pxc/tls.go +++ b/pkg/controller/pxc/tls.go @@ -16,6 +16,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" logf "sigs.k8s.io/controller-runtime/pkg/log" api "github.com/percona/percona-xtradb-cluster-operator/pkg/apis/pxc/v1" @@ -54,6 +55,10 @@ func (r *ReconcilePerconaXtraDBCluster) reconcileSSL(ctx context.Context, cr *ap if err := r.reconcileCARotation(ctx, cr, &secretObj, &secretInternalObj); err != nil { return errors.Wrap(err, "reconcile CA rotation") } + // reconcile cert-manager Certificate specs on operator upgrade + if err := r.reconcileCertManagerCertificateSpecs(ctx, cr); err != nil { + return fmt.Errorf("reconcile cert-manager certificates: %w", err) + } return nil } else if errSecret != nil && !k8serr.IsNotFound(errSecret) { return fmt.Errorf("get secret: %v", errSecret) @@ -78,157 +83,39 @@ func (r *ReconcilePerconaXtraDBCluster) reconcileSSL(ctx context.Context, cr *ap } func (r *ReconcilePerconaXtraDBCluster) createSSLByCertManager(ctx context.Context, cr *api.PerconaXtraDBCluster) error { - issuerName := naming.IssuerName(cr) - caIssuerName := naming.CAIssuerName(cr) - issuerKind := "Issuer" - issuerGroup := "" - caDuration := &metav1.Duration{Duration: pxctls.DefaultCAValidity} - if cr.Spec.TLS != nil && cr.Spec.TLS.CADuration != nil { - caDuration = cr.Spec.TLS.CADuration - } + cfg := resolveTLSCertConfig(cr) - if cr.Spec.TLS != nil && cr.Spec.TLS.IssuerConf != nil { - issuerKind = cr.Spec.TLS.IssuerConf.Kind - issuerName = cr.Spec.TLS.IssuerConf.Name - issuerGroup = cr.Spec.TLS.IssuerConf.Group - } else { - if err := r.createIssuer(ctx, cr, caIssuerName, ""); err != nil { + if cr.Spec.TLS == nil || cr.Spec.TLS.IssuerConf == nil { + if err := r.createIssuer(ctx, cr, cfg.caIssuerName, ""); err != nil { return err } - caCert := &cm.Certificate{ - ObjectMeta: metav1.ObjectMeta{ - Name: naming.CACertificateName(cr), - Namespace: cr.Namespace, - }, - Spec: cm.CertificateSpec{ - SecretName: naming.CACertificateName(cr), - CommonName: cr.Name + "-ca", - IsCA: true, - IssuerRef: cmmeta.ObjectReference{ - Name: caIssuerName, - Kind: issuerKind, - Group: issuerGroup, - }, - Duration: caDuration, - RenewBefore: &metav1.Duration{Duration: pxctls.DefaultRenewBefore}, - }, - } - if cr.CompareVersionWith("1.16.0") >= 0 { - caCert.Labels = naming.LabelsCluster(cr) - } - if cr.CompareVersionWith("1.20.0") >= 0 { - caCert.Spec.PrivateKey = &cm.CertificatePrivateKey{ - RotationPolicy: cm.RotationPolicyNever, - } - } - - err := r.client.Create(ctx, caCert) - if err != nil && !k8serr.IsAlreadyExists(err) { - return fmt.Errorf("create CA certificate: %v", err) + caCert := buildCACertificate(cr, cfg) + if err := r.createOrUpdateCertificate(ctx, caCert); err != nil { + return fmt.Errorf("create or update CA certificate: %w", err) } if err := r.waitForCerts(ctx, cr.Namespace, caCert.Spec.SecretName); err != nil { return err } - if err := r.createIssuer(ctx, cr, issuerName, caCert.Spec.SecretName); err != nil { + if err := r.createIssuer(ctx, cr, cfg.issuerName, caCert.Spec.SecretName); err != nil { return err } } - duration := &metav1.Duration{Duration: pxctls.DefaultCertValidity} - if cr.Spec.TLS != nil && cr.Spec.TLS.Duration != nil { - duration = cr.Spec.TLS.Duration - } - - kubeCert := &cm.Certificate{ - ObjectMeta: metav1.ObjectMeta{ - Name: naming.SSLCertificateName(cr), - Namespace: cr.Namespace, - }, - Spec: cm.CertificateSpec{ - SecretName: cr.Spec.PXC.SSLSecretName, - CommonName: cr.Name + "-proxysql", - DNSNames: []string{ - cr.Name + "-pxc", - cr.Name + "-proxysql", - "*." + cr.Name + "-pxc", - "*." + cr.Name + "-proxysql", - }, - IssuerRef: cmmeta.ObjectReference{ - Name: issuerName, - Kind: issuerKind, - Group: issuerGroup, - }, - }, - } - if cr.CompareVersionWith("1.16.0") >= 0 { - kubeCert.Labels = naming.LabelsCluster(cr) - } - if cr.Spec.TLS != nil && len(cr.Spec.TLS.SANs) > 0 { - kubeCert.Spec.DNSNames = append(kubeCert.Spec.DNSNames, cr.Spec.TLS.SANs...) - } - if cr.CompareVersionWith("1.19.0") >= 0 { - kubeCert.Spec.Duration = duration - } - if cr.CompareVersionWith("1.20.0") >= 0 { - kubeCert.Spec.PrivateKey = &cm.CertificatePrivateKey{ - RotationPolicy: cm.RotationPolicyNever, - } - } - - err := r.client.Create(ctx, kubeCert) - if err != nil && !k8serr.IsAlreadyExists(err) { - return fmt.Errorf("create certificate: %v", err) + kubeCert := buildSSLCertificate(cr, cfg) + if err := r.createOrUpdateCertificate(ctx, kubeCert); err != nil { + return fmt.Errorf("create or update certificate: %w", err) } if cr.Spec.PXC.SSLSecretName == cr.Spec.PXC.SSLInternalSecretName { return r.waitForCerts(ctx, cr.Namespace, cr.Spec.PXC.SSLSecretName) } - kubeCert = &cm.Certificate{ - ObjectMeta: metav1.ObjectMeta{ - Name: naming.SSLInternalCertificateName(cr), - Namespace: cr.Namespace, - }, - Spec: cm.CertificateSpec{ - SecretName: cr.Spec.PXC.SSLInternalSecretName, - CommonName: cr.Name + "-pxc", - DNSNames: []string{ - cr.Name + "-pxc", - "*." + cr.Name + "-pxc", - cr.Name + "-haproxy-replicas." + cr.Namespace + ".svc.cluster.local", - cr.Name + "-haproxy-replicas." + cr.Namespace, - cr.Name + "-haproxy-replicas", - cr.Name + "-haproxy." + cr.Namespace + ".svc.cluster.local", - cr.Name + "-haproxy." + cr.Namespace, - cr.Name + "-haproxy", - }, - IssuerRef: cmmeta.ObjectReference{ - Name: issuerName, - Kind: issuerKind, - Group: issuerGroup, - }, - }, - } - if cr.Spec.TLS != nil && len(cr.Spec.TLS.SANs) > 0 { - kubeCert.Spec.DNSNames = append(kubeCert.Spec.DNSNames, cr.Spec.TLS.SANs...) - } - if cr.CompareVersionWith("1.16.0") >= 0 { - kubeCert.Labels = naming.LabelsCluster(cr) - } - if cr.CompareVersionWith("1.19.0") >= 0 { - kubeCert.Spec.Duration = duration - } - if cr.CompareVersionWith("1.20.0") >= 0 { - kubeCert.Spec.PrivateKey = &cm.CertificatePrivateKey{ - RotationPolicy: cm.RotationPolicyNever, - } - } - err = r.client.Create(ctx, kubeCert) - if err != nil && !k8serr.IsAlreadyExists(err) { - return fmt.Errorf("create internal certificate: %v", err) + kubeCertInternal := buildSSLInternalCertificate(cr, cfg) + if err := r.createOrUpdateCertificate(ctx, kubeCertInternal); err != nil { + return fmt.Errorf("create or update internal certificate: %w", err) } return r.waitForCerts(ctx, cr.Namespace, cr.Spec.PXC.SSLSecretName, cr.Spec.PXC.SSLInternalSecretName) @@ -375,6 +262,287 @@ func (r *ReconcilePerconaXtraDBCluster) createSSLManualy(ctx context.Context, cr return nil } +func (r *ReconcilePerconaXtraDBCluster) createOrUpdateCertificate(ctx context.Context, desired *cm.Certificate) error { + existing := &cm.Certificate{ + ObjectMeta: metav1.ObjectMeta{ + Name: desired.Name, + Namespace: desired.Namespace, + }, + } + _, err := controllerutil.CreateOrUpdate(ctx, r.client, existing, func() error { + existing.Labels = mergeStringMap(existing.Labels, desired.Labels) + existing.Annotations = mergeStringMap(existing.Annotations, desired.Annotations) + if len(desired.OwnerReferences) > 0 { + existing.OwnerReferences = desired.OwnerReferences + } + + desiredSpec := desired.Spec + if desiredSpec.PrivateKey != nil { + privateKey := *desiredSpec.PrivateKey + desiredSpec.PrivateKey = &privateKey + } + + // Preserve IssuerRef.Group if the API server defaulted it + // (cert-manager >= 1.19 defaults empty group to "cert-manager.io"). + // Without this, every reconcile would see a diff and trigger + // an unnecessary Update, which increments the Certificate's + // generation and may cause cert-manager to re-issue. + if desiredSpec.IssuerRef.Group == "" && existing.Spec.IssuerRef.Group != "" { + desiredSpec.IssuerRef.Group = existing.Spec.IssuerRef.Group + } + + // CA rotation is triggered by patching rotationPolicy=Always. + // Keep the override only while cert-manager is actively issuing. + if shouldPreserveCertificateRotationPolicy(existing, desiredSpec) { + desiredSpec.PrivateKey.RotationPolicy = existing.Spec.PrivateKey.RotationPolicy + } + + existing.Spec = desiredSpec + return nil + }) + return err +} + +func mergeStringMap(existing, desired map[string]string) map[string]string { + if len(existing) == 0 && len(desired) == 0 { + return nil + } + merged := make(map[string]string, len(existing)+len(desired)) + for k, v := range existing { + merged[k] = v + } + for k, v := range desired { + merged[k] = v + } + return merged +} + +func shouldPreserveCertificateRotationPolicy(existing *cm.Certificate, desired cm.CertificateSpec) bool { + if existing.Spec.PrivateKey == nil || desired.PrivateKey == nil { + return false + } + if desired.PrivateKey.RotationPolicy != cm.RotationPolicyNever { + return false + } + if existing.Spec.PrivateKey.RotationPolicy == "" || + existing.Spec.PrivateKey.RotationPolicy == desired.PrivateKey.RotationPolicy { + return false + } + return certificateIsIssuing(existing) +} + +func certificateIsIssuing(cert *cm.Certificate) bool { + for _, condition := range cert.Status.Conditions { + if condition.Type == cm.CertificateConditionIssuing && condition.Status == cmmeta.ConditionTrue { + return true + } + } + return false +} + +// tlsCertConfig holds the resolved TLS configuration for building cert-manager Certificate objects. +type tlsCertConfig struct { + issuerName string + caIssuerName string + issuerKind string + issuerGroup string + caDuration *metav1.Duration + duration *metav1.Duration +} + +func resolveTLSCertConfig(cr *api.PerconaXtraDBCluster) tlsCertConfig { + cfg := tlsCertConfig{ + issuerName: naming.IssuerName(cr), + caIssuerName: naming.CAIssuerName(cr), + issuerKind: "Issuer", + caDuration: &metav1.Duration{Duration: pxctls.DefaultCAValidity}, + duration: &metav1.Duration{Duration: pxctls.DefaultCertValidity}, + } + if cr.Spec.TLS != nil { + if cr.Spec.TLS.IssuerConf != nil { + cfg.issuerKind = cr.Spec.TLS.IssuerConf.Kind + cfg.issuerName = cr.Spec.TLS.IssuerConf.Name + cfg.issuerGroup = cr.Spec.TLS.IssuerConf.Group + } + if cr.Spec.TLS.CADuration != nil { + cfg.caDuration = cr.Spec.TLS.CADuration + } + if cr.Spec.TLS.Duration != nil { + cfg.duration = cr.Spec.TLS.Duration + } + } + return cfg +} + +func buildCACertificate(cr *api.PerconaXtraDBCluster, cfg tlsCertConfig) *cm.Certificate { + caCert := &cm.Certificate{ + ObjectMeta: metav1.ObjectMeta{ + Name: naming.CACertificateName(cr), + Namespace: cr.Namespace, + }, + Spec: cm.CertificateSpec{ + SecretName: naming.CACertificateName(cr), + CommonName: cr.Name + "-ca", + IsCA: true, + IssuerRef: cmmeta.ObjectReference{ + Name: cfg.caIssuerName, + Kind: cfg.issuerKind, + Group: cfg.issuerGroup, + }, + Duration: cfg.caDuration, + RenewBefore: &metav1.Duration{Duration: pxctls.DefaultRenewBefore}, + }, + } + if cr.CompareVersionWith("1.16.0") >= 0 { + caCert.Labels = naming.LabelsCluster(cr) + } + if cr.CompareVersionWith("1.20.0") >= 0 { + caCert.Spec.PrivateKey = &cm.CertificatePrivateKey{ + RotationPolicy: cm.RotationPolicyNever, + } + } + return caCert +} + +func buildSSLCertificate(cr *api.PerconaXtraDBCluster, cfg tlsCertConfig) *cm.Certificate { + kubeCert := &cm.Certificate{ + ObjectMeta: metav1.ObjectMeta{ + Name: naming.SSLCertificateName(cr), + Namespace: cr.Namespace, + }, + Spec: cm.CertificateSpec{ + SecretName: cr.Spec.PXC.SSLSecretName, + CommonName: cr.Name + "-proxysql", + DNSNames: []string{ + cr.Name + "-pxc", + cr.Name + "-proxysql", + "*." + cr.Name + "-pxc", + "*." + cr.Name + "-proxysql", + }, + IssuerRef: cmmeta.ObjectReference{ + Name: cfg.issuerName, + Kind: cfg.issuerKind, + Group: cfg.issuerGroup, + }, + }, + } + if cr.CompareVersionWith("1.16.0") >= 0 { + kubeCert.Labels = naming.LabelsCluster(cr) + } + if cr.Spec.TLS != nil && len(cr.Spec.TLS.SANs) > 0 { + kubeCert.Spec.DNSNames = append(kubeCert.Spec.DNSNames, cr.Spec.TLS.SANs...) + } + if cr.CompareVersionWith("1.19.0") >= 0 { + kubeCert.Spec.Duration = cfg.duration + } + if cr.CompareVersionWith("1.20.0") >= 0 { + kubeCert.Spec.PrivateKey = &cm.CertificatePrivateKey{ + RotationPolicy: cm.RotationPolicyNever, + } + } + return kubeCert +} + +func buildSSLInternalCertificate(cr *api.PerconaXtraDBCluster, cfg tlsCertConfig) *cm.Certificate { + kubeCert := &cm.Certificate{ + ObjectMeta: metav1.ObjectMeta{ + Name: naming.SSLInternalCertificateName(cr), + Namespace: cr.Namespace, + }, + Spec: cm.CertificateSpec{ + SecretName: cr.Spec.PXC.SSLInternalSecretName, + CommonName: cr.Name + "-pxc", + DNSNames: []string{ + cr.Name + "-pxc", + "*." + cr.Name + "-pxc", + cr.Name + "-haproxy-replicas." + cr.Namespace + ".svc.cluster.local", + cr.Name + "-haproxy-replicas." + cr.Namespace, + cr.Name + "-haproxy-replicas", + cr.Name + "-haproxy." + cr.Namespace + ".svc.cluster.local", + cr.Name + "-haproxy." + cr.Namespace, + cr.Name + "-haproxy", + }, + IssuerRef: cmmeta.ObjectReference{ + Name: cfg.issuerName, + Kind: cfg.issuerKind, + Group: cfg.issuerGroup, + }, + }, + } + if cr.Spec.TLS != nil && len(cr.Spec.TLS.SANs) > 0 { + kubeCert.Spec.DNSNames = append(kubeCert.Spec.DNSNames, cr.Spec.TLS.SANs...) + } + if cr.CompareVersionWith("1.16.0") >= 0 { + kubeCert.Labels = naming.LabelsCluster(cr) + } + if cr.CompareVersionWith("1.19.0") >= 0 { + kubeCert.Spec.Duration = cfg.duration + } + if cr.CompareVersionWith("1.20.0") >= 0 { + kubeCert.Spec.PrivateKey = &cm.CertificatePrivateKey{ + RotationPolicy: cm.RotationPolicyNever, + } + } + return kubeCert +} + +// reconcileCertManagerCertificateSpecs updates existing cert-manager Certificate +// CR specs to match the current PXC CR TLS configuration. Unlike createSSLByCertManager, +// it does not create issuers or wait for secrets since they already exist. +// It is a no-op if the Certificate CRs were not created by cert-manager. +func (r *ReconcilePerconaXtraDBCluster) reconcileCertManagerCertificateSpecs(ctx context.Context, cr *api.PerconaXtraDBCluster) error { + // Only reconcile if the SSL certificate was created by cert-manager. + sslCert := &cm.Certificate{} + if err := r.client.Get(ctx, types.NamespacedName{ + Namespace: cr.Namespace, + Name: naming.SSLCertificateName(cr), + }, sslCert); err != nil { + if k8serr.IsNotFound(err) { + // Certificate CR doesn't exist — certs were created manually. + return nil + } + return fmt.Errorf("get ssl certificate: %w", err) + } + + cfg := resolveTLSCertConfig(cr) + + // Update CA certificate spec if using built-in issuer + if cr.Spec.TLS == nil || cr.Spec.TLS.IssuerConf == nil { + // Confirm the CA issuer exists before attempting updates. + caIssuer := &cm.Issuer{} + if err := r.client.Get(ctx, types.NamespacedName{ + Namespace: cr.Namespace, + Name: cfg.caIssuerName, + }, caIssuer); err != nil { + if k8serr.IsNotFound(err) { + return nil + } + return fmt.Errorf("get CA issuer: %w", err) + } + + caCert := buildCACertificate(cr, cfg) + if err := r.createOrUpdateCertificate(ctx, caCert); err != nil { + return fmt.Errorf("update CA certificate: %w", err) + } + } + + // Update SSL certificate spec + kubeCert := buildSSLCertificate(cr, cfg) + if err := r.createOrUpdateCertificate(ctx, kubeCert); err != nil { + return fmt.Errorf("update certificate: %w", err) + } + + // Update SSL internal certificate spec + if cr.Spec.PXC.SSLSecretName != cr.Spec.PXC.SSLInternalSecretName { + kubeCertInternal := buildSSLInternalCertificate(cr, cfg) + if err := r.createOrUpdateCertificate(ctx, kubeCertInternal); err != nil { + return fmt.Errorf("update internal certificate: %w", err) + } + } + + return nil +} + func (r *ReconcilePerconaXtraDBCluster) reconcileCARotation( ctx context.Context, cr *api.PerconaXtraDBCluster, diff --git a/pkg/controller/pxc/tls_test.go b/pkg/controller/pxc/tls_test.go index 743b99d895..406b928af2 100644 --- a/pkg/controller/pxc/tls_test.go +++ b/pkg/controller/pxc/tls_test.go @@ -3,7 +3,11 @@ package pxc import ( "bytes" "testing" + "time" + cm "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" + cmmeta "github.com/cert-manager/cert-manager/pkg/apis/meta/v1" + cmscheme "github.com/cert-manager/cert-manager/pkg/client/clientset/versioned/scheme" "github.com/percona/percona-xtradb-cluster-operator/pkg/apis" api "github.com/percona/percona-xtradb-cluster-operator/pkg/apis/pxc/v1" "github.com/percona/percona-xtradb-cluster-operator/pkg/pxc/app/statefulset" @@ -21,6 +25,122 @@ import ( logf "sigs.k8s.io/controller-runtime/pkg/log" ) +func TestCreateOrUpdateCertificatePreservesExistingFields(t *testing.T) { + ctx := t.Context() + + scheme := runtime.NewScheme() + require.NoError(t, clientgoscheme.AddToScheme(scheme)) + require.NoError(t, cmscheme.AddToScheme(scheme)) + + existing := &cm.Certificate{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster-ca-cert", + Namespace: "default", + Annotations: map[string]string{ + "external": "keep", + }, + Labels: map[string]string{ + "external": "keep", + "app.kubernetes.io/name": "old", + }, + }, + Spec: cm.CertificateSpec{ + SecretName: "old-secret", + IssuerRef: cmmeta.ObjectReference{ + Name: "cluster-ca-issuer", + Kind: "Issuer", + Group: "cert-manager.io", + }, + PrivateKey: &cm.CertificatePrivateKey{ + RotationPolicy: cm.RotationPolicyAlways, + }, + }, + Status: cm.CertificateStatus{ + Conditions: []cm.CertificateCondition{ + { + Type: cm.CertificateConditionIssuing, + Status: cmmeta.ConditionTrue, + }, + }, + }, + } + desired := &cm.Certificate{ + ObjectMeta: metav1.ObjectMeta{ + Name: existing.Name, + Namespace: existing.Namespace, + Annotations: map[string]string{ + "desired": "true", + }, + Labels: map[string]string{ + "app.kubernetes.io/name": "pxc", + "desired": "true", + }, + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: api.SchemeGroupVersion.String(), + Kind: "PerconaXtraDBCluster", + Name: "cluster", + UID: types.UID("cluster-uid"), + }, + }, + }, + Spec: cm.CertificateSpec{ + SecretName: "cluster-ca-cert", + CommonName: "cluster-ca", + IsCA: true, + IssuerRef: cmmeta.ObjectReference{ + Name: "cluster-ca-issuer", + Kind: "Issuer", + }, + Duration: &metav1.Duration{Duration: time.Hour}, + PrivateKey: &cm.CertificatePrivateKey{ + RotationPolicy: cm.RotationPolicyNever, + }, + }, + } + + cl := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(existing). + Build() + r := &ReconcilePerconaXtraDBCluster{ + client: cl, + scheme: cl.Scheme(), + } + + require.NoError(t, r.createOrUpdateCertificate(ctx, desired)) + + actual := &cm.Certificate{} + require.NoError(t, cl.Get(ctx, client.ObjectKeyFromObject(existing), actual)) + require.Equal(t, map[string]string{ + "external": "keep", + "app.kubernetes.io/name": "pxc", + "desired": "true", + }, actual.Labels) + require.Equal(t, map[string]string{ + "external": "keep", + "desired": "true", + }, actual.Annotations) + require.Equal(t, desired.OwnerReferences, actual.OwnerReferences) + require.Equal(t, "cert-manager.io", actual.Spec.IssuerRef.Group) + require.Equal(t, cm.RotationPolicyAlways, actual.Spec.PrivateKey.RotationPolicy) + require.Equal(t, "cluster-ca-cert", actual.Spec.SecretName) + require.Equal(t, "cluster-ca", actual.Spec.CommonName) + require.True(t, actual.Spec.IsCA) + + actual.Status.Conditions = []cm.CertificateCondition{ + { + Type: cm.CertificateConditionIssuing, + Status: cmmeta.ConditionFalse, + }, + } + require.NoError(t, cl.Update(ctx, actual)) + require.NoError(t, r.createOrUpdateCertificate(ctx, desired)) + + require.NoError(t, cl.Get(ctx, client.ObjectKeyFromObject(existing), actual)) + require.Equal(t, cm.RotationPolicyNever, actual.Spec.PrivateKey.RotationPolicy) +} + func TestRotateSSLCertificate(t *testing.T) { ctx := t.Context() cr, err := readDefaultCR("test-cluster", "default")