From 7f9450bf7f9aa25df18c9e305d75c62222234685 Mon Sep 17 00:00:00 2001 From: Mayank Shah Date: Mon, 13 Jul 2026 16:43:36 +0530 Subject: [PATCH 01/17] added spec.issuerConf to CRD Signed-off-by: Mayank Shah --- .../pgv2.percona.com_perconapgclusters.yaml | 19 ++++++++++ .../pgv2.percona.com_perconapgclusters.yaml | 19 ++++++++++ ...eam.pgv2.percona.com_postgresclusters.yaml | 19 ++++++++++ deploy/bundle.yaml | 38 +++++++++++++++++++ deploy/crd.yaml | 38 +++++++++++++++++++ deploy/cw-bundle.yaml | 38 +++++++++++++++++++ .../v1beta1/postgrescluster_types.go | 4 ++ .../v1beta1/zz_generated.deepcopy.go | 6 +++ 8 files changed, 181 insertions(+) diff --git a/build/crd/percona/generated/pgv2.percona.com_perconapgclusters.yaml b/build/crd/percona/generated/pgv2.percona.com_perconapgclusters.yaml index f681b7dc1b..2888c8f3d3 100644 --- a/build/crd/percona/generated/pgv2.percona.com_perconapgclusters.yaml +++ b/build/crd/percona/generated/pgv2.percona.com_perconapgclusters.yaml @@ -28690,6 +28690,25 @@ spec: type: string certValidityDuration: type: string + issuerConf: + description: K8SPG-951 + properties: + group: + description: |- + Group of the issuer being referred to. + Defaults to 'cert-manager.io'. + type: string + kind: + description: |- + Kind of the issuer being referred to. + Defaults to 'Issuer'. + type: string + name: + description: Name of the issuer being referred to. + type: string + required: + - name + type: object pgBackRestCertValidityDuration: type: string type: object diff --git a/config/crd/bases/pgv2.percona.com_perconapgclusters.yaml b/config/crd/bases/pgv2.percona.com_perconapgclusters.yaml index b6cbab607b..02d8d82224 100644 --- a/config/crd/bases/pgv2.percona.com_perconapgclusters.yaml +++ b/config/crd/bases/pgv2.percona.com_perconapgclusters.yaml @@ -29385,6 +29385,25 @@ spec: type: string certValidityDuration: type: string + issuerConf: + description: K8SPG-951 + properties: + group: + description: |- + Group of the issuer being referred to. + Defaults to 'cert-manager.io'. + type: string + kind: + description: |- + Kind of the issuer being referred to. + Defaults to 'Issuer'. + type: string + name: + description: Name of the issuer being referred to. + type: string + required: + - name + type: object pgBackRestCertValidityDuration: type: string type: object diff --git a/config/crd/bases/upstream.pgv2.percona.com_postgresclusters.yaml b/config/crd/bases/upstream.pgv2.percona.com_postgresclusters.yaml index e8ba977e3c..23ceaf2af3 100644 --- a/config/crd/bases/upstream.pgv2.percona.com_postgresclusters.yaml +++ b/config/crd/bases/upstream.pgv2.percona.com_postgresclusters.yaml @@ -28671,6 +28671,25 @@ spec: type: string certValidityDuration: type: string + issuerConf: + description: K8SPG-951 + properties: + group: + description: |- + Group of the issuer being referred to. + Defaults to 'cert-manager.io'. + type: string + kind: + description: |- + Kind of the issuer being referred to. + Defaults to 'Issuer'. + type: string + name: + description: Name of the issuer being referred to. + type: string + required: + - name + type: object pgBackRestCertValidityDuration: type: string type: object diff --git a/deploy/bundle.yaml b/deploy/bundle.yaml index 737b9667eb..bd28ae65a3 100644 --- a/deploy/bundle.yaml +++ b/deploy/bundle.yaml @@ -29682,6 +29682,25 @@ spec: type: string certValidityDuration: type: string + issuerConf: + description: K8SPG-951 + properties: + group: + description: |- + Group of the issuer being referred to. + Defaults to 'cert-manager.io'. + type: string + kind: + description: |- + Kind of the issuer being referred to. + Defaults to 'Issuer'. + type: string + name: + description: Name of the issuer being referred to. + type: string + required: + - name + type: object pgBackRestCertValidityDuration: type: string type: object @@ -66741,6 +66760,25 @@ spec: type: string certValidityDuration: type: string + issuerConf: + description: K8SPG-951 + properties: + group: + description: |- + Group of the issuer being referred to. + Defaults to 'cert-manager.io'. + type: string + kind: + description: |- + Kind of the issuer being referred to. + Defaults to 'Issuer'. + type: string + name: + description: Name of the issuer being referred to. + type: string + required: + - name + type: object pgBackRestCertValidityDuration: type: string type: object diff --git a/deploy/crd.yaml b/deploy/crd.yaml index cee8c24318..40da7d9e18 100644 --- a/deploy/crd.yaml +++ b/deploy/crd.yaml @@ -29682,6 +29682,25 @@ spec: type: string certValidityDuration: type: string + issuerConf: + description: K8SPG-951 + properties: + group: + description: |- + Group of the issuer being referred to. + Defaults to 'cert-manager.io'. + type: string + kind: + description: |- + Kind of the issuer being referred to. + Defaults to 'Issuer'. + type: string + name: + description: Name of the issuer being referred to. + type: string + required: + - name + type: object pgBackRestCertValidityDuration: type: string type: object @@ -66741,6 +66760,25 @@ spec: type: string certValidityDuration: type: string + issuerConf: + description: K8SPG-951 + properties: + group: + description: |- + Group of the issuer being referred to. + Defaults to 'cert-manager.io'. + type: string + kind: + description: |- + Kind of the issuer being referred to. + Defaults to 'Issuer'. + type: string + name: + description: Name of the issuer being referred to. + type: string + required: + - name + type: object pgBackRestCertValidityDuration: type: string type: object diff --git a/deploy/cw-bundle.yaml b/deploy/cw-bundle.yaml index e2ab9b11ab..4f9834295f 100644 --- a/deploy/cw-bundle.yaml +++ b/deploy/cw-bundle.yaml @@ -29682,6 +29682,25 @@ spec: type: string certValidityDuration: type: string + issuerConf: + description: K8SPG-951 + properties: + group: + description: |- + Group of the issuer being referred to. + Defaults to 'cert-manager.io'. + type: string + kind: + description: |- + Kind of the issuer being referred to. + Defaults to 'Issuer'. + type: string + name: + description: Name of the issuer being referred to. + type: string + required: + - name + type: object pgBackRestCertValidityDuration: type: string type: object @@ -66741,6 +66760,25 @@ spec: type: string certValidityDuration: type: string + issuerConf: + description: K8SPG-951 + properties: + group: + description: |- + Group of the issuer being referred to. + Defaults to 'cert-manager.io'. + type: string + kind: + description: |- + Kind of the issuer being referred to. + Defaults to 'Issuer'. + type: string + name: + description: Name of the issuer being referred to. + type: string + required: + - name + type: object pgBackRestCertValidityDuration: type: string type: object diff --git a/pkg/apis/upstream.pgv2.percona.com/v1beta1/postgrescluster_types.go b/pkg/apis/upstream.pgv2.percona.com/v1beta1/postgrescluster_types.go index 43407dd51a..f25900f82a 100644 --- a/pkg/apis/upstream.pgv2.percona.com/v1beta1/postgrescluster_types.go +++ b/pkg/apis/upstream.pgv2.percona.com/v1beta1/postgrescluster_types.go @@ -9,6 +9,7 @@ import ( "fmt" "reflect" + cmmeta "github.com/cert-manager/cert-manager/pkg/apis/meta/v1" gover "github.com/hashicorp/go-version" "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" @@ -230,6 +231,9 @@ type TLSSpec struct { CAValidityDuration *metav1.Duration `json:"caValidityDuration,omitempty"` // +optional PGBackRestCertValidityDuration *metav1.Duration `json:"pgBackRestCertValidityDuration,omitempty"` + // K8SPG-951 + // +optional + IssuerConf *cmmeta.IssuerReference `json:"issuerConf,omitempty"` } // DataSource defines data sources for a new PostgresCluster. diff --git a/pkg/apis/upstream.pgv2.percona.com/v1beta1/zz_generated.deepcopy.go b/pkg/apis/upstream.pgv2.percona.com/v1beta1/zz_generated.deepcopy.go index b3f88c9297..2e5dfbd8d4 100644 --- a/pkg/apis/upstream.pgv2.percona.com/v1beta1/zz_generated.deepcopy.go +++ b/pkg/apis/upstream.pgv2.percona.com/v1beta1/zz_generated.deepcopy.go @@ -9,6 +9,7 @@ package v1beta1 import ( + metav1 "github.com/cert-manager/cert-manager/pkg/apis/meta/v1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -2668,6 +2669,11 @@ func (in *TLSSpec) DeepCopyInto(out *TLSSpec) { *out = new(v1.Duration) **out = **in } + if in.IssuerConf != nil { + in, out := &in.IssuerConf, &out.IssuerConf + *out = new(metav1.IssuerReference) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new TLSSpec. From 187c2f8a64a9cc8db7ae2c626874d60e8f295f2f Mon Sep 17 00:00:00 2001 From: Mayank Shah Date: Tue, 14 Jul 2026 15:46:53 +0530 Subject: [PATCH 02/17] wip Signed-off-by: Mayank Shah --- .../controller/postgrescluster/controller.go | 2 +- .../controller/postgrescluster/instance.go | 31 +- .../postgrescluster/instance_test.go | 43 ++ .../controller/postgrescluster/pgbackrest.go | 27 +- .../postgrescluster/pgbackrest_test.go | 37 ++ internal/controller/postgrescluster/pki.go | 59 ++- .../controller/postgrescluster/pki_test.go | 176 ++++++++ internal/naming/names.go | 24 + internal/naming/names_test.go | 20 + percona/certmanager/certmanager.go | 425 ++++++++++++++---- percona/certmanager/certmanager_test.go | 417 +++++++++++++++++ percona/runtime/runtime.go | 17 + percona/runtime/runtime_test.go | 34 ++ 13 files changed, 1214 insertions(+), 98 deletions(-) create mode 100644 percona/runtime/runtime_test.go diff --git a/internal/controller/postgrescluster/controller.go b/internal/controller/postgrescluster/controller.go index 9ab3477bd1..6c00d5b193 100644 --- a/internal/controller/postgrescluster/controller.go +++ b/internal/controller/postgrescluster/controller.go @@ -300,7 +300,7 @@ func (r *Reconciler) Reconcile( rootCA, err = r.reconcileRootCertificate(ctx, cluster) } - if err == nil && rootCA != nil { + if err == nil { certManagerManaged, certErr := r.isRootCACertManagerManaged(ctx, cluster) if certErr != nil { log.V(1).Info("failed to check if root CA is cert-manager managed, will retry on next reconcile", diff --git a/internal/controller/postgrescluster/instance.go b/internal/controller/postgrescluster/instance.go index f45be198a4..1b7b7a33a5 100644 --- a/internal/controller/postgrescluster/instance.go +++ b/internal/controller/postgrescluster/instance.go @@ -1575,15 +1575,20 @@ func (r *Reconciler) reconcileCertManagerInstanceCertificates( _ = leafCert.Certificate.UnmarshalText(instanceCerts.Data["dns.crt"]) _ = leafCert.PrivateKey.UnmarshalText(instanceCerts.Data["dns.key"]) + caCert, err := instanceCACert(rootCertificateAuth, existing) + if err != nil { + return nil, err + } + err = patroni.InstanceCertificates(ctx, - rootCertificateAuth.Certificate, leafCert.Certificate, + caCert, leafCert.Certificate, leafCert.PrivateKey, instanceCerts) if err != nil { return nil, errors.Wrap(err, "failed to add patroni certificates") } err = pgbackrest.InstanceCertificates(ctx, cluster, - rootCertificateAuth.Certificate, leafCert.Certificate, leafCert.PrivateKey, + caCert, leafCert.Certificate, leafCert.PrivateKey, instanceCerts) if err != nil { return nil, errors.Wrap(err, "failed to add pgbackrest certificates") @@ -1597,6 +1602,28 @@ func (r *Reconciler) reconcileCertManagerInstanceCertificates( return instanceCerts, nil } +// instanceCACert returns the CA certificate to embed alongside an instance's +// leaf certificate. When rootCertificateAuth is set (the operator manages +// the CA itself), it's the source of truth. When it's nil (external issuer — +// see K8SPG-951), this reads the ca.crt cert-manager wrote into the +// instance's own just-issued secret. +func instanceCACert(rootCertificateAuth *pki.RootCertificateAuthority, issuedSecret *corev1.Secret) (pki.Certificate, error) { + if rootCertificateAuth != nil { + return rootCertificateAuth.Certificate, nil + } + + ca := issuedSecret.Data[corev1.ServiceAccountRootCAKey] + if len(ca) == 0 { + return pki.Certificate{}, errors.New("external issuer did not return a CA certificate for the instance") + } + + var caCert pki.Certificate + if err := caCert.UnmarshalText(ca); err != nil { + return pki.Certificate{}, errors.Wrap(err, "failed to parse CA certificate from cert-manager secret") + } + return caCert, nil +} + // reconcileInternalInstanceCertificates creates instance certificates using internal PKI. func (r *Reconciler) reconcileInternalInstanceCertificates( ctx context.Context, cluster *v1beta1.PostgresCluster, diff --git a/internal/controller/postgrescluster/instance_test.go b/internal/controller/postgrescluster/instance_test.go index 868457a709..fe4e1ca889 100644 --- a/internal/controller/postgrescluster/instance_test.go +++ b/internal/controller/postgrescluster/instance_test.go @@ -37,6 +37,7 @@ import ( "github.com/percona/percona-postgresql-operator/v2/internal/controller/runtime" "github.com/percona/percona-postgresql-operator/v2/internal/logging" "github.com/percona/percona-postgresql-operator/v2/internal/naming" + "github.com/percona/percona-postgresql-operator/v2/internal/pki" "github.com/percona/percona-postgresql-operator/v2/internal/testing/cmp" "github.com/percona/percona-postgresql-operator/v2/internal/testing/events" "github.com/percona/percona-postgresql-operator/v2/internal/testing/require" @@ -2158,3 +2159,45 @@ func TestCleanupDisruptionBudgets(t *testing.T) { }) }) } + +func TestInstanceCACert(t *testing.T) { + t.Run("uses rootCertificateAuth when present", func(t *testing.T) { + root, err := pki.NewRootCertificateAuthority() + assert.NilError(t, err) + + caCert, err := instanceCACert(root, &corev1.Secret{}) + assert.NilError(t, err) + + want, err := root.Certificate.MarshalText() + assert.NilError(t, err) + got, err := caCert.MarshalText() + assert.NilError(t, err) + assert.DeepEqual(t, want, got) + }) + + t.Run("parses ca.crt from issued secret when rootCertificateAuth is nil", func(t *testing.T) { + root, err := pki.NewRootCertificateAuthority() + assert.NilError(t, err) + caBytes, err := root.Certificate.MarshalText() + assert.NilError(t, err) + + issuedSecret := &corev1.Secret{Data: map[string][]byte{corev1.ServiceAccountRootCAKey: caBytes}} + + caCert, err := instanceCACert(nil, issuedSecret) + assert.NilError(t, err) + got, err := caCert.MarshalText() + assert.NilError(t, err) + assert.DeepEqual(t, caBytes, got) + }) + + t.Run("errors when issued secret has no ca.crt", func(t *testing.T) { + _, err := instanceCACert(nil, &corev1.Secret{}) + assert.ErrorContains(t, err, "did not return a CA certificate") + }) + + t.Run("errors when ca.crt is not a valid certificate", func(t *testing.T) { + issuedSecret := &corev1.Secret{Data: map[string][]byte{corev1.ServiceAccountRootCAKey: []byte("not a cert")}} + _, err := instanceCACert(nil, issuedSecret) + assert.ErrorContains(t, err, "failed to parse CA certificate") + }) +} diff --git a/internal/controller/postgrescluster/pgbackrest.go b/internal/controller/postgrescluster/pgbackrest.go index 4a51d94709..e5611ca49e 100644 --- a/internal/controller/postgrescluster/pgbackrest.go +++ b/internal/controller/postgrescluster/pgbackrest.go @@ -2322,9 +2322,9 @@ func (r *Reconciler) reconcileCertManagerPGBackRestSecret( // Populate the pgBackRest secret from cert-manager-issued certs. initialize.Map(&intent.Data) - caCert, err := rootCA.Certificate.MarshalText() + caCert, err := pgBackRestCACert(rootCA, clientSecret, repoSecret) if err != nil { - return errors.Wrap(err, "failed to marshal root CA certificate") + return err } intent.Data[pgbackrest.CertAuthoritySecretKey] = caCert intent.Data[pgbackrest.CertClientSecretKey] = clientSecret.Data[corev1.TLSCertKey] @@ -2335,6 +2335,29 @@ func (r *Reconciler) reconcileCertManagerPGBackRestSecret( return nil } +// pgBackRestCACert returns the CA certificate bytes to trust for pgBackRest's +// client/repo TLS. When rootCA is set (the operator manages the CA itself), +// it's the source of truth. When rootCA is nil (external issuer — see +// K8SPG-951), there is no operator-tracked CA; instead this reads the ca.crt +// cert-manager wrote into one of the just-issued leaf secrets, which is +// present as long as the external issuer returns CA data in its response +// (true for CA-backed issuers; not guaranteed for e.g. some ACME issuers). +func pgBackRestCACert(rootCA *pki.RootCertificateAuthority, clientSecret, repoSecret *corev1.Secret) ([]byte, error) { + if rootCA != nil { + caCert, err := rootCA.Certificate.MarshalText() + return caCert, errors.Wrap(err, "failed to marshal root CA certificate") + } + + if ca := clientSecret.Data[corev1.ServiceAccountRootCAKey]; len(ca) > 0 { + return ca, nil + } + if ca := repoSecret.Data[corev1.ServiceAccountRootCAKey]; len(ca) > 0 { + return ca, nil + } + + return nil, errors.New("external issuer did not return a CA certificate for pgBackRest") +} + // +kubebuilder:rbac:groups="",resources="serviceaccounts",verbs={create,patch} // +kubebuilder:rbac:groups="rbac.authorization.k8s.io",resources="roles",verbs={create,patch} // +kubebuilder:rbac:groups="rbac.authorization.k8s.io",resources="rolebindings",verbs={create,patch} diff --git a/internal/controller/postgrescluster/pgbackrest_test.go b/internal/controller/postgrescluster/pgbackrest_test.go index e41a11e83e..363c29a5b2 100644 --- a/internal/controller/postgrescluster/pgbackrest_test.go +++ b/internal/controller/postgrescluster/pgbackrest_test.go @@ -4571,3 +4571,40 @@ func TestBackupsEnabled(t *testing.T) { assert.Assert(t, backupsReconciliationAllowed) }) } + +func TestPgBackRestCACert(t *testing.T) { + t.Run("uses rootCA when present", func(t *testing.T) { + root, err := pki.NewRootCertificateAuthority() + assert.NilError(t, err) + + caCert, err := pgBackRestCACert(root, &corev1.Secret{}, &corev1.Secret{}) + assert.NilError(t, err) + + want, err := root.Certificate.MarshalText() + assert.NilError(t, err) + assert.DeepEqual(t, want, caCert) + }) + + t.Run("falls back to client secret ca.crt when rootCA is nil", func(t *testing.T) { + clientSecret := &corev1.Secret{Data: map[string][]byte{corev1.ServiceAccountRootCAKey: []byte("client-ca-bytes")}} + repoSecret := &corev1.Secret{} + + caCert, err := pgBackRestCACert(nil, clientSecret, repoSecret) + assert.NilError(t, err) + assert.DeepEqual(t, []byte("client-ca-bytes"), caCert) + }) + + t.Run("falls back to repo secret ca.crt when client secret has none", func(t *testing.T) { + clientSecret := &corev1.Secret{} + repoSecret := &corev1.Secret{Data: map[string][]byte{corev1.ServiceAccountRootCAKey: []byte("repo-ca-bytes")}} + + caCert, err := pgBackRestCACert(nil, clientSecret, repoSecret) + assert.NilError(t, err) + assert.DeepEqual(t, []byte("repo-ca-bytes"), caCert) + }) + + t.Run("errors when neither secret has a CA cert", func(t *testing.T) { + _, err := pgBackRestCACert(nil, &corev1.Secret{}, &corev1.Secret{}) + assert.ErrorContains(t, err, "did not return a CA certificate") + }) +} diff --git a/internal/controller/postgrescluster/pki.go b/internal/controller/postgrescluster/pki.go index 7ee3d61f7f..fed716e506 100644 --- a/internal/controller/postgrescluster/pki.go +++ b/internal/controller/postgrescluster/pki.go @@ -42,12 +42,23 @@ func (r *Reconciler) reconcileRootCertificate( ) ( *pki.RootCertificateAuthority, error, ) { + mode, err := certmanager.ResolveIssuerMode(ctx, r.Client, cluster) + if err != nil { + return nil, errors.Wrap(err, "failed to resolve issuer mode") + } + if mode == certmanager.IssuerModeExternal { + return nil, nil + } + const keyCertificate, keyPrivateKey = "root.crt", "root.key" // K8SPG-553 existing := &corev1.Secret{ ObjectMeta: naming.PostgresRootCASecret(cluster), } + if mode == certmanager.IssuerModeManagedCluster { + existing.ObjectMeta = naming.ClusterCACertSecret(cluster, certmanager.CertManagerNamespace()) + } privateKey := keyPrivateKey certificateKey := keyCertificate @@ -64,11 +75,11 @@ func (r *Reconciler) reconcileRootCertificate( } } - err := errors.WithStack( + err = errors.WithStack( r.Client.Get(ctx, client.ObjectKeyFromObject(existing), existing)) // K8SPG-555: we need to check ca certificate from old operator versions // TODO: remove when 2.4.0 will become unsupported - if k8serrors.IsNotFound(err) { + if k8serrors.IsNotFound(err) && mode == certmanager.IssuerModeManagedNamespaced { nn := client.ObjectKeyFromObject(existing) nn.Name = naming.RootCertSecret err = errors.WithStack( @@ -104,6 +115,12 @@ func (r *Reconciler) reconcileRootCertificate( return nil, errors.New("waiting for cert-manager to issue a valid CA certificate") } + if mode == certmanager.IssuerModeManagedCluster { + // The cluster-scoped CA cert/secret is entirely cert-manager's + // responsibility; there is no internal-PKI fallback for it. + return nil, errors.New("waiting for cert-manager to issue a valid CA certificate") + } + root := &pki.RootCertificateAuthority{} if err == nil { @@ -186,8 +203,17 @@ func (r *Reconciler) reconcileCertManagerRootCertificate( return nil, errors.Wrap(err, "error applying CA certificate") } + mode, err := certmanager.ResolveIssuerMode(ctx, r.Client, cluster) + if err != nil { + return nil, errors.Wrap(err, "failed to resolve issuer mode") + } + secretMeta := naming.PostgresRootCASecret(cluster) + if mode == certmanager.IssuerModeManagedCluster { + secretMeta = naming.ClusterCACertSecret(cluster, certmanager.CertManagerNamespace()) + } + // Try to fetch the CA secret created by cert-manager. - secret := &corev1.Secret{ObjectMeta: naming.PostgresRootCASecret(cluster)} + secret := &corev1.Secret{ObjectMeta: secretMeta} if err := r.Client.Get(ctx, client.ObjectKeyFromObject(secret), secret); err != nil { if k8serrors.IsNotFound(err) { log.Info("waiting for cert-manager to issue CA certificate") @@ -335,9 +361,14 @@ func (r *Reconciler) reconcileCertManagerClusterCertificate( ) { c := r.CertManagerCtrlFunc(r.Client, r.Scheme, false) - err := c.ApplyIssuer(ctx, cluster) + mode, err := certmanager.ResolveIssuerMode(ctx, r.Client, cluster) if err != nil { - return nil, errors.Wrap(err, "failed to apply TLS issuer") + return nil, errors.Wrap(err, "failed to resolve issuer mode") + } + if mode != certmanager.IssuerModeExternal { + if err := c.ApplyIssuer(ctx, cluster); err != nil { + return nil, errors.Wrap(err, "failed to apply TLS issuer") + } } primaryDNSNames, err := naming.ServiceDNSNames(ctx, primaryService, cluster.Spec.ClusterServiceDNSSuffix) @@ -385,11 +416,27 @@ func (r *Reconciler) isRootCACertManagerManaged(ctx context.Context, cluster *v1 return false, nil } + mode, err := certmanager.ResolveIssuerMode(ctx, r.Client, cluster) + if err != nil { + return false, errors.Wrap(err, "failed to resolve issuer mode") + } + installed, err := r.isCertManagerInstalled(ctx, cluster.Namespace) - if err != nil || !installed { + if err != nil { return false, err } + if mode != certmanager.IssuerModeManagedNamespaced { + if !installed { + return false, errors.New("cert-manager is required when spec.tls.issuerConf is set") + } + return true, nil + } + + if !installed { + return false, nil + } + rootSecret := &corev1.Secret{ObjectMeta: naming.PostgresRootCASecret(cluster)} err = r.Client.Get(ctx, client.ObjectKeyFromObject(rootSecret), rootSecret) if err != nil { diff --git a/internal/controller/postgrescluster/pki_test.go b/internal/controller/postgrescluster/pki_test.go index 2c61ad1d54..39d4ec101c 100644 --- a/internal/controller/postgrescluster/pki_test.go +++ b/internal/controller/postgrescluster/pki_test.go @@ -12,6 +12,8 @@ import ( "strings" "testing" + cmv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" + cmmeta "github.com/cert-manager/cert-manager/pkg/apis/meta/v1" "github.com/pkg/errors" "gotest.tools/v3/assert" appsv1 "k8s.io/api/apps/v1" @@ -21,6 +23,7 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/rest" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" "github.com/percona/percona-postgresql-operator/v2/internal/naming" "github.com/percona/percona-postgresql-operator/v2/internal/pki" @@ -787,3 +790,176 @@ func getCertFromSecret( fromSecret := &pki.Certificate{} return fromSecret, fromSecret.UnmarshalText(secretCRT) } + +// installedCertManagerController reports cert-manager as installed and lets +// every Apply* call succeed as a no-op — used for K8SPG-951 issuer-mode +// tests that only need isCertManagerInstalled to return true, not real +// Issuer/Certificate object creation (envtest has no cert-manager CRDs). +type installedCertManagerController struct{} + +func (installedCertManagerController) Check(context.Context, *rest.Config, string) error { + return nil +} +func (installedCertManagerController) CertificateExists(context.Context, string, string) (bool, error) { + return false, nil +} +func (installedCertManagerController) ApplyIssuer(context.Context, *v1beta1.PostgresCluster) error { + return nil +} +func (installedCertManagerController) ApplyCAIssuer(context.Context, *v1beta1.PostgresCluster) error { + return nil +} +func (installedCertManagerController) ApplyCACertificate(context.Context, *v1beta1.PostgresCluster) error { + return nil +} +func (installedCertManagerController) ApplyClusterCertificate(context.Context, *v1beta1.PostgresCluster, []string) error { + return nil +} +func (installedCertManagerController) ApplyInstanceCertificate(context.Context, *v1beta1.PostgresCluster, string, []string) error { + return nil +} +func (installedCertManagerController) ApplyPGBouncerCertificate(context.Context, *v1beta1.PostgresCluster, []string) error { + return nil +} +func (installedCertManagerController) ApplyReplicationCertificate(context.Context, *v1beta1.PostgresCluster) error { + return nil +} +func (installedCertManagerController) ApplyPGBackRestClientCertificate(context.Context, *v1beta1.PostgresCluster) error { + return nil +} +func (installedCertManagerController) ApplyPGBackRestRepoCertificate(context.Context, *v1beta1.PostgresCluster, []string) error { + return nil +} + +func installedCertManagerCtrlFunc(_ client.Client, _ *runtime.Scheme, _ bool) certmanager.Controller { + return installedCertManagerController{} +} + +func TestIssuerModeAwareness(t *testing.T) { + _, tClient := setupKubernetes(t) + require.ParallelCapacity(t, 1) + ctx := t.Context() + namespace := require.Namespace(t, tClient).Name + + t.Run("reconcileRootCertificate returns nil for external issuer", func(t *testing.T) { + cluster := testCluster() + cluster.Name = "external-root-cert" + cluster.Namespace = namespace + cluster.Spec.TLS = &v1beta1.TLSSpec{ + IssuerConf: &cmmeta.IssuerReference{Name: "vault-issuer", Kind: "VaultClusterIssuer"}, + } + assert.NilError(t, tClient.Create(ctx, cluster)) + + r := &Reconciler{ + Client: tClient, + Owner: ControllerName, + CertManagerCtrlFunc: certmanager.NewController, + } + + root, err := r.reconcileRootCertificate(ctx, cluster) + assert.NilError(t, err) + assert.Assert(t, root == nil) + }) + + t.Run("isRootCACertManagerManaged errors when cert-manager missing and issuerConf is set", func(t *testing.T) { + cluster := testCluster() + cluster.Name = "external-no-certmanager" + cluster.Namespace = namespace + cluster.Spec.TLS = &v1beta1.TLSSpec{ + IssuerConf: &cmmeta.IssuerReference{Name: "vault-issuer", Kind: "VaultClusterIssuer"}, + } + assert.NilError(t, tClient.Create(ctx, cluster)) + + r := &Reconciler{ + Client: tClient, + Owner: ControllerName, + CertManagerCtrlFunc: mockCertManagerCtrlFunc, + RestConfig: nil, // isCertManagerInstalled short-circuits to false + } + + _, err := r.isRootCACertManagerManaged(ctx, cluster) + assert.ErrorContains(t, err, "cert-manager is required") + }) + + t.Run("isRootCACertManagerManaged returns true immediately for external issuer when cert-manager installed", func(t *testing.T) { + cluster := testCluster() + cluster.Name = "external-with-certmanager" + cluster.Namespace = namespace + cluster.Spec.TLS = &v1beta1.TLSSpec{ + IssuerConf: &cmmeta.IssuerReference{Name: "vault-issuer", Kind: "VaultClusterIssuer"}, + } + assert.NilError(t, tClient.Create(ctx, cluster)) + + r := &Reconciler{ + Client: tClient, + Owner: ControllerName, + CertManagerCtrlFunc: installedCertManagerCtrlFunc, + RestConfig: &rest.Config{}, + } + + managed, err := r.isRootCACertManagerManaged(ctx, cluster) + assert.NilError(t, err) + assert.Assert(t, managed) + }) + + t.Run("isRootCACertManagerManaged returns true immediately for managed cluster issuer when cert-manager installed", func(t *testing.T) { + cluster := testCluster() + cluster.Name = "cluster-scoped-with-certmanager" + cluster.Namespace = namespace + cluster.Spec.TLS = &v1beta1.TLSSpec{ + IssuerConf: &cmmeta.IssuerReference{Name: "shared-tls-issuer", Kind: cmv1.ClusterIssuerKind}, + } + assert.NilError(t, tClient.Create(ctx, cluster)) + + // certmanager.ResolveIssuerMode issues a live Get for the named + // ClusterIssuer. The shared envtest scheme used by setupKubernetes(t) + // (internal/controller/runtime.Scheme) does not register cert-manager + // types, and envtest has no cert-manager CRDs installed either, so + // tClient can't serve that Get (pre-existing gap, out of scope for + // this task's file set). A scheme-complete fake client stands in for + // tClient as this Reconciler's Client so the ClusterIssuer Get + // resolves to NotFound (mode ManagedCluster), matching what happens + // against a real cluster where the operator can read the API but the + // ClusterIssuer doesn't exist yet. + certManagerScheme := runtime.NewScheme() + assert.NilError(t, cmv1.AddToScheme(certManagerScheme)) + fakeClient := fake.NewClientBuilder().WithScheme(certManagerScheme).Build() + + r := &Reconciler{ + Client: fakeClient, + Owner: ControllerName, + CertManagerCtrlFunc: installedCertManagerCtrlFunc, + RestConfig: &rest.Config{}, + } + + managed, err := r.isRootCACertManagerManaged(ctx, cluster) + assert.NilError(t, err) + assert.Assert(t, managed) + }) + + t.Run("reconcileCertManagerClusterCertificate skips ApplyIssuer for external issuer", func(t *testing.T) { + cluster := testCluster() + cluster.Name = "external-cluster-cert" + cluster.Namespace = namespace + cluster.Spec.TLS = &v1beta1.TLSSpec{ + IssuerConf: &cmmeta.IssuerReference{Name: "vault-issuer", Kind: "VaultClusterIssuer"}, + } + assert.NilError(t, tClient.Create(ctx, cluster)) + + recovery := &recoveryCertManagerController{} + r := &Reconciler{ + Client: tClient, + Owner: ControllerName, + CertManagerCtrlFunc: func(_ client.Client, _ *runtime.Scheme, _ bool) certmanager.Controller { + return recovery + }, + } + + primaryService := &corev1.Service{ObjectMeta: metav1.ObjectMeta{Namespace: namespace, Name: "external-cluster-cert-primary"}} + replicaService := &corev1.Service{ObjectMeta: metav1.ObjectMeta{Namespace: namespace, Name: "external-cluster-cert-replicas"}} + + _, err := r.reconcileCertManagerClusterCertificate(ctx, nil, cluster, primaryService, replicaService) + assert.NilError(t, err) + assert.Equal(t, recovery.applyIssuerCalls, 0) + }) +} diff --git a/internal/naming/names.go b/internal/naming/names.go index cca81200ef..416c0dc3da 100644 --- a/internal/naming/names.go +++ b/internal/naming/names.go @@ -656,3 +656,27 @@ func TLSIssuer(cluster *v1beta1.PostgresCluster) metav1.ObjectMeta { Name: cluster.Name + "-tls-issuer", } } + +// ClusterCAIssuer returns the ObjectMeta for the cluster-scoped self-signed +// CA ClusterIssuer used by cert-manager when spec.tls.issuerConf.kind is +// "ClusterIssuer" (K8SPG-951). The name is qualified by cluster name and +// namespace so multiple PostgresClusters sharing this mode don't collide; +// ClusterIssuers have no namespace of their own. +func ClusterCAIssuer(cluster *v1beta1.PostgresCluster) metav1.ObjectMeta { + return metav1.ObjectMeta{ + Name: cluster.Name + "-" + cluster.Namespace + "-ca-issuer", + } +} + +// ClusterCACertSecret returns the ObjectMeta for the CA certificate Secret +// backing a cluster-scoped self-signed CA ClusterIssuer (K8SPG-951). +// certManagerNamespace is cert-manager's shared "cluster resource namespace" +// (see percona/certmanager.CertManagerNamespace) — a ClusterIssuer's +// spec.ca.secretName is resolved there, not in the PostgresCluster's own +// namespace. +func ClusterCACertSecret(cluster *v1beta1.PostgresCluster, certManagerNamespace string) metav1.ObjectMeta { + return metav1.ObjectMeta{ + Namespace: certManagerNamespace, + Name: cluster.Name + "-" + cluster.Namespace + "-cluster-ca-cert", + } +} diff --git a/internal/naming/names_test.go b/internal/naming/names_test.go index 86d3d736be..e45fc6d443 100644 --- a/internal/naming/names_test.go +++ b/internal/naming/names_test.go @@ -332,3 +332,23 @@ func TestPortNamesUniqueAndValid(t *testing.T) { names.Insert(name) } } + +func TestClusterCAIssuer(t *testing.T) { + cluster := &v1beta1.PostgresCluster{} + cluster.Namespace = "postgres-operator" + cluster.Name = "hippo" + + meta := ClusterCAIssuer(cluster) + assert.Equal(t, meta.Name, "hippo-postgres-operator-ca-issuer") + assert.Equal(t, meta.Namespace, "") +} + +func TestClusterCACertSecret(t *testing.T) { + cluster := &v1beta1.PostgresCluster{} + cluster.Namespace = "postgres-operator" + cluster.Name = "hippo" + + meta := ClusterCACertSecret(cluster, "cert-manager") + assert.Equal(t, meta.Name, "hippo-postgres-operator-cluster-ca-cert") + assert.Equal(t, meta.Namespace, "cert-manager") +} diff --git a/percona/certmanager/certmanager.go b/percona/certmanager/certmanager.go index e6149a0e87..4eb997d019 100644 --- a/percona/certmanager/certmanager.go +++ b/percona/certmanager/certmanager.go @@ -2,9 +2,11 @@ package certmanager import ( "context" + "os" "regexp" "time" + "github.com/cert-manager/cert-manager/pkg/apis/certmanager" v1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" cmmeta "github.com/cert-manager/cert-manager/pkg/apis/meta/v1" "github.com/cert-manager/cert-manager/pkg/util/cmapichecker" @@ -47,6 +49,110 @@ const ( DefaultRenewBefore = 30 * 24 * time.Hour ) +// IssuerMode describes how the operator should treat +// cluster.Spec.TLS.IssuerConf (K8SPG-951). +type IssuerMode int + +const ( + // IssuerModeManagedNamespaced: issuerConf is unset, or its Kind is "" or + // "Issuer" — the operator owns a namespaced self-signed CA issuer, CA + // certificate, and CA-backed TLS issuer (the long-standing behavior). + IssuerModeManagedNamespaced IssuerMode = iota + // IssuerModeManagedCluster: issuerConf.Kind is "ClusterIssuer" and the + // operator can read the named ClusterIssuer (or it doesn't exist yet) — + // the operator owns a cluster-scoped self-signed CA ClusterIssuer, a CA + // certificate in cert-manager's shared namespace, and a CA-backed TLS + // ClusterIssuer. + IssuerModeManagedCluster + // IssuerModeExternal: issuerConf.Kind is a third-party kind, or is + // "ClusterIssuer" but the operator is Forbidden from reading it — every + // leaf Certificate references issuerConf directly and the operator + // creates nothing issuer-related. + IssuerModeExternal +) + +// CertManagerNamespace returns cert-manager's shared "cluster resource +// namespace" — where a ClusterIssuer's spec.ca.secretName is resolved from, +// as opposed to the namespace of whatever references the ClusterIssuer. +// Configurable via the CERTMANAGER_NAMESPACE environment variable; defaults +// to "cert-manager". +func CertManagerNamespace() string { + if ns := os.Getenv("CERTMANAGER_NAMESPACE"); ns != "" { + return ns + } + return "cert-manager" +} + +// issuerConf returns cluster.Spec.TLS.IssuerConf, or nil if TLS or +// IssuerConf is unset. +func issuerConf(cluster *v1beta1.PostgresCluster) *cmmeta.IssuerReference { + if cluster.Spec.TLS == nil { + return nil + } + return cluster.Spec.TLS.IssuerConf +} + +// ResolveIssuerMode determines how the operator should handle +// cluster.Spec.TLS.IssuerConf. For Kind == "ClusterIssuer" it performs a +// live Get to check whether the operator can read the named ClusterIssuer: +// Forbidden (no RBAC for clusterissuers.cert-manager.io — the default, +// since none is shipped) downgrades to IssuerModeExternal so the operator +// doesn't try to own something it can't inspect. NotFound is not an error +// here — the operator will create it as part of managing it. +func ResolveIssuerMode(ctx context.Context, cl client.Client, cluster *v1beta1.PostgresCluster) (IssuerMode, error) { + ic := issuerConf(cluster) + if ic == nil { + return IssuerModeManagedNamespaced, nil + } + + switch ic.Kind { + case "", v1.IssuerKind: + return IssuerModeManagedNamespaced, nil + case v1.ClusterIssuerKind: + existing := &v1.ClusterIssuer{} + err := cl.Get(ctx, types.NamespacedName{Name: ic.Name}, existing) + switch { + case err == nil, k8serrors.IsNotFound(err): + return IssuerModeManagedCluster, nil + case k8serrors.IsForbidden(err): + return IssuerModeExternal, nil + default: + return IssuerModeManagedNamespaced, errors.Wrap(err, "failed to get cluster issuer") + } + default: + return IssuerModeExternal, nil + } +} + +// issuerRef resolves what a leaf Certificate's spec.issuerRef should be for +// the given mode: +// - IssuerModeExternal: issuerConf's Name/Kind/Group verbatim (Group +// defaults to "cert-manager.io" when empty — matches cert-manager's own +// IssuerReference default). +// - IssuerModeManagedCluster: the cluster-scoped TLS ClusterIssuer named +// by issuerConf.Name (required by the CRD whenever issuerConf is set). +// - IssuerModeManagedNamespaced: the namespaced TLS Issuer, named by +// issuerConf.Name when set, otherwise the auto-generated name. +func issuerRef(cluster *v1beta1.PostgresCluster, mode IssuerMode) cmmeta.IssuerReference { + switch mode { + case IssuerModeExternal: + ic := issuerConf(cluster) + group := ic.Group + if group == "" { + group = certmanager.GroupName + } + return cmmeta.IssuerReference{Name: ic.Name, Kind: ic.Kind, Group: group} + case IssuerModeManagedCluster: + return cmmeta.IssuerReference{Name: issuerConf(cluster).Name, Kind: v1.ClusterIssuerKind} + default: + name := naming.TLSIssuer(cluster).Name + if ic := issuerConf(cluster); ic != nil && ic.Name != "" { + name = ic.Name + } + return cmmeta.IssuerReference{Name: name, Kind: v1.IssuerKind} + } +} + type controller struct { cl client.Client scheme *runtime.Scheme @@ -112,11 +218,53 @@ func (c *controller) CertificateExists(ctx context.Context, namespace, certName return false, errors.Wrapf(err, "get certificate/%s", certName) } +// ApplyIssuer creates the CA-backed Issuer resource that signs every leaf +// Certificate for the given PostgresCluster (or a cluster-scoped CA-backed +// ClusterIssuer when spec.tls.issuerConf.kind is "ClusterIssuer" — +// K8SPG-951). No-op when the resolved mode is external. func (c *controller) ApplyIssuer(ctx context.Context, cluster *v1beta1.PostgresCluster) error { + mode, err := ResolveIssuerMode(ctx, c.cl, cluster) + if err != nil { + return errors.Wrap(err, "failed to resolve issuer mode") + } + if mode == IssuerModeExternal { + return nil + } + + if mode == IssuerModeManagedCluster { + caSecretName := naming.ClusterCACertSecret(cluster, CertManagerNamespace()).Name + meta := metav1.ObjectMeta{Name: issuerRef(cluster, mode).Name} + + existing := &v1.ClusterIssuer{} + err := c.cl.Get(ctx, types.NamespacedName{Name: meta.Name}, existing) + if err == nil { + return nil + } + if !k8serrors.IsNotFound(err) { + return errors.Wrap(err, "failed to get cluster issuer") + } + + issuer := &v1.ClusterIssuer{ + ObjectMeta: meta, + Spec: v1.IssuerSpec{ + IssuerConfig: v1.IssuerConfig{ + CA: &v1.CAIssuer{SecretName: caSecretName}, + }, + }, + } + if err := c.cl.Create(ctx, issuer); err != nil { + return errors.Wrap(err, "failed to create cluster issuer") + } + return nil + } + meta := naming.TLSIssuer(cluster) + if ic := issuerConf(cluster); ic != nil && ic.Name != "" { + meta.Name = ic.Name + } existing := &v1.Issuer{} - err := c.cl.Get(ctx, types.NamespacedName{Name: meta.Name, Namespace: meta.Namespace}, existing) + err = c.cl.Get(ctx, types.NamespacedName{Name: meta.Name, Namespace: meta.Namespace}, existing) if err == nil { hasOwnerRef, err := controllerutil.HasOwnerReference(existing.OwnerReferences, cluster, c.scheme) if err != nil { @@ -164,12 +312,48 @@ func (c *controller) ApplyIssuer(ctx context.Context, cluster *v1beta1.PostgresC return nil } -// ApplyCAIssuer creates a SelfSigned Issuer resource for the given PostgresCluster. +// ApplyCAIssuer creates a SelfSigned Issuer resource for the given +// PostgresCluster (or a cluster-scoped SelfSigned ClusterIssuer when +// spec.tls.issuerConf.kind is "ClusterIssuer" — K8SPG-951). No-op when the +// resolved mode is external. func (c *controller) ApplyCAIssuer(ctx context.Context, cluster *v1beta1.PostgresCluster) error { + mode, err := ResolveIssuerMode(ctx, c.cl, cluster) + if err != nil { + return errors.Wrap(err, "failed to resolve issuer mode") + } + if mode == IssuerModeExternal { + return nil + } + + spec := v1.IssuerSpec{ + IssuerConfig: v1.IssuerConfig{ + SelfSigned: &v1.SelfSignedIssuer{}, + }, + } + + if mode == IssuerModeManagedCluster { + meta := naming.ClusterCAIssuer(cluster) + + existing := &v1.ClusterIssuer{} + err := c.cl.Get(ctx, types.NamespacedName{Name: meta.Name}, existing) + if err == nil { + return nil + } + if !k8serrors.IsNotFound(err) { + return errors.Wrap(err, "failed to get CA cluster issuer") + } + + issuer := &v1.ClusterIssuer{ObjectMeta: meta, Spec: spec} + if err := c.cl.Create(ctx, issuer); err != nil { + return errors.Wrap(err, "failed to create ca cluster issuer") + } + return nil + } + meta := naming.CAIssuer(cluster) existing := &v1.Issuer{} - err := c.cl.Get(ctx, types.NamespacedName{Name: meta.Name, Namespace: meta.Namespace}, existing) + err = c.cl.Get(ctx, types.NamespacedName{Name: meta.Name, Namespace: meta.Namespace}, existing) if err == nil { hasOwnerRef, err := controllerutil.HasOwnerReference(existing.OwnerReferences, cluster, c.scheme) if err != nil { @@ -195,14 +379,7 @@ func (c *controller) ApplyCAIssuer(ctx context.Context, cluster *v1beta1.Postgre return errors.Wrap(err, "failed to get CA issuer") } - issuer := &v1.Issuer{ - ObjectMeta: meta, - Spec: v1.IssuerSpec{ - IssuerConfig: v1.IssuerConfig{ - SelfSigned: &v1.SelfSignedIssuer{}, - }, - }, - } + issuer := &v1.Issuer{ObjectMeta: meta, Spec: spec} if err := controllerutil.SetControllerReference(cluster, issuer, c.scheme); err != nil { return errors.Wrap(err, "failed to set controller reference") @@ -215,35 +392,62 @@ func (c *controller) ApplyCAIssuer(ctx context.Context, cluster *v1beta1.Postgre return nil } +// ApplyCACertificate creates the self-signed CA Certificate for the given +// PostgresCluster. For IssuerModeManagedCluster (K8SPG-951), it's placed in +// cert-manager's shared namespace under a cluster-qualified name and gets no +// owner reference (it may be shared by other PostgresClusters). No-op for +// IssuerModeExternal. func (c *controller) ApplyCACertificate(ctx context.Context, cluster *v1beta1.PostgresCluster) error { - certName := naming.PostgresRootCASecret(cluster).Name + mode, err := ResolveIssuerMode(ctx, c.cl, cluster) + if err != nil { + return errors.Wrap(err, "failed to resolve issuer mode") + } + if mode == IssuerModeExternal { + return nil + } caDuration := DefaultCertDuration if cluster.Spec.TLS != nil && cluster.Spec.TLS.CAValidityDuration != nil { caDuration = cluster.Spec.TLS.CAValidityDuration.Duration } + clusterScoped := mode == IssuerModeManagedCluster + + var secretMeta metav1.ObjectMeta + var issuerRefValue cmmeta.IssuerReference + if clusterScoped { + secretMeta = naming.ClusterCACertSecret(cluster, CertManagerNamespace()) + issuerRefValue = cmmeta.IssuerReference{Name: naming.ClusterCAIssuer(cluster).Name, Kind: v1.ClusterIssuerKind} + } else { + secretMeta = naming.PostgresRootCASecret(cluster) + issuerRefValue = cmmeta.IssuerReference{Name: naming.CAIssuer(cluster).Name, Kind: v1.IssuerKind} + } + certName := secretMeta.Name + certNamespace := secretMeta.Namespace + existing := &v1.Certificate{} - err := c.cl.Get(ctx, types.NamespacedName{Name: certName, Namespace: cluster.Namespace}, existing) + err = c.cl.Get(ctx, types.NamespacedName{Name: certName, Namespace: certNamespace}, existing) if err == nil { needsUpdate := false - hasOwnerRef, err := controllerutil.HasOwnerReference(existing.OwnerReferences, cluster, c.scheme) - if err != nil { - return errors.Wrap(err, "check owner reference") - } - - if !hasOwnerRef { - gvk := v1beta1.SchemeBuilder.GroupVersion.WithKind("PostgresCluster") - existing.OwnerReferences = []metav1.OwnerReference{{ - APIVersion: gvk.GroupVersion().String(), - Kind: gvk.Kind, - Name: cluster.GetName(), - UID: cluster.GetUID(), - BlockOwnerDeletion: ptr.To(true), - Controller: ptr.To(true), - }} - needsUpdate = true + if !clusterScoped { + hasOwnerRef, err := controllerutil.HasOwnerReference(existing.OwnerReferences, cluster, c.scheme) + if err != nil { + return errors.Wrap(err, "check owner reference") + } + + if !hasOwnerRef { + gvk := v1beta1.SchemeBuilder.GroupVersion.WithKind("PostgresCluster") + existing.OwnerReferences = []metav1.OwnerReference{{ + APIVersion: gvk.GroupVersion().String(), + Kind: gvk.Kind, + Name: cluster.GetName(), + UID: cluster.GetUID(), + BlockOwnerDeletion: ptr.To(true), + Controller: ptr.To(true), + }} + needsUpdate = true + } } if existing.Spec.Duration != nil && existing.Spec.Duration.Duration != caDuration { @@ -264,19 +468,16 @@ func (c *controller) ApplyCACertificate(ctx context.Context, cluster *v1beta1.Po cert := &v1.Certificate{ ObjectMeta: metav1.ObjectMeta{ Name: certName, - Namespace: cluster.Namespace, + Namespace: certNamespace, Labels: naming.WithPerconaLabels(map[string]string{ naming.LabelCluster: cluster.Name, }, cluster.Name, "", cluster.Labels[naming.LabelVersion]), }, Spec: v1.CertificateSpec{ - SecretName: certName, - CommonName: cluster.Name + "-ca", - IsCA: true, - IssuerRef: cmmeta.IssuerReference{ - Name: naming.CAIssuer(cluster).Name, - Kind: v1.IssuerKind, - }, + SecretName: certName, + CommonName: cluster.Name + "-ca", + IsCA: true, + IssuerRef: issuerRefValue, Duration: &metav1.Duration{Duration: caDuration}, RenewBefore: &metav1.Duration{Duration: DefaultRenewBefore}, PrivateKey: &v1.CertificatePrivateKey{ @@ -292,8 +493,10 @@ func (c *controller) ApplyCACertificate(ctx context.Context, cluster *v1beta1.Po }, } - if err := controllerutil.SetControllerReference(cluster, cert, c.scheme); err != nil { - return errors.Wrap(err, "failed to set controller reference") + if !clusterScoped { + if err := controllerutil.SetControllerReference(cluster, cert, c.scheme); err != nil { + return errors.Wrap(err, "failed to set controller reference") + } } if err := c.cl.Create(ctx, cert); err != nil { @@ -310,6 +513,12 @@ func (c *controller) ApplyClusterCertificate(ctx context.Context, cluster *v1bet return errors.New("dnsNames cannot be empty") } + mode, err := ResolveIssuerMode(ctx, c.cl, cluster) + if err != nil { + return errors.Wrap(err, "failed to resolve issuer mode") + } + wantIssuerRef := issuerRef(cluster, mode) + certName := ClusterCertificateName(cluster) certDuration := DefaultCertDuration @@ -318,7 +527,7 @@ func (c *controller) ApplyClusterCertificate(ctx context.Context, cluster *v1bet } existing := &v1.Certificate{} - err := c.cl.Get(ctx, types.NamespacedName{Name: certName, Namespace: cluster.Namespace}, existing) + err = c.cl.Get(ctx, types.NamespacedName{Name: certName, Namespace: cluster.Namespace}, existing) if err == nil { needsUpdate := false @@ -345,6 +554,11 @@ func (c *controller) ApplyClusterCertificate(ctx context.Context, cluster *v1bet needsUpdate = true } + if existing.Spec.IssuerRef != wantIssuerRef { + existing.Spec.IssuerRef = wantIssuerRef + needsUpdate = true + } + if !needsUpdate { return nil } @@ -365,13 +579,10 @@ func (c *controller) ApplyClusterCertificate(ctx context.Context, cluster *v1bet }, cluster.Name, "", cluster.Labels[naming.LabelVersion]), }, Spec: v1.CertificateSpec{ - SecretName: certName, - CommonName: cluster.Name + "-postgres", - DNSNames: dnsNames, - IssuerRef: cmmeta.ObjectReference{ - Name: naming.TLSIssuer(cluster).Name, - Kind: v1.IssuerKind, - }, + SecretName: certName, + CommonName: cluster.Name + "-postgres", + DNSNames: dnsNames, + IssuerRef: wantIssuerRef, Duration: &metav1.Duration{Duration: certDuration}, RenewBefore: &metav1.Duration{Duration: DefaultRenewBefore}, PrivateKey: &v1.CertificatePrivateKey{ @@ -412,6 +623,12 @@ func (c *controller) ApplyInstanceCertificate(ctx context.Context, cluster *v1be return errors.New("dnsNames cannot be empty") } + mode, err := ResolveIssuerMode(ctx, c.cl, cluster) + if err != nil { + return errors.Wrap(err, "failed to resolve issuer mode") + } + wantIssuerRef := issuerRef(cluster, mode) + certName := InstanceCertificateName(instanceName) secretName := instanceName + "-certs" @@ -421,7 +638,7 @@ func (c *controller) ApplyInstanceCertificate(ctx context.Context, cluster *v1be } existing := &v1.Certificate{} - err := c.cl.Get(ctx, types.NamespacedName{Name: certName, Namespace: cluster.Namespace}, existing) + err = c.cl.Get(ctx, types.NamespacedName{Name: certName, Namespace: cluster.Namespace}, existing) if err == nil { needsUpdate := false @@ -448,6 +665,11 @@ func (c *controller) ApplyInstanceCertificate(ctx context.Context, cluster *v1be needsUpdate = true } + if existing.Spec.IssuerRef != wantIssuerRef { + existing.Spec.IssuerRef = wantIssuerRef + needsUpdate = true + } + if !needsUpdate { return nil } @@ -468,13 +690,10 @@ func (c *controller) ApplyInstanceCertificate(ctx context.Context, cluster *v1be }, cluster.Name, "", cluster.Labels[naming.LabelVersion]), }, Spec: v1.CertificateSpec{ - SecretName: secretName, - CommonName: instanceName, - DNSNames: dnsNames, - IssuerRef: cmmeta.IssuerReference{ - Name: naming.TLSIssuer(cluster).Name, - Kind: v1.IssuerKind, - }, + SecretName: secretName, + CommonName: instanceName, + DNSNames: dnsNames, + IssuerRef: wantIssuerRef, Duration: &metav1.Duration{Duration: certDuration}, RenewBefore: &metav1.Duration{Duration: DefaultRenewBefore}, PrivateKey: &v1.CertificatePrivateKey{ @@ -514,6 +733,12 @@ func (c *controller) ApplyPGBouncerCertificate(ctx context.Context, cluster *v1b return errors.New("dnsNames cannot be empty") } + mode, err := ResolveIssuerMode(ctx, c.cl, cluster) + if err != nil { + return errors.Wrap(err, "failed to resolve issuer mode") + } + wantIssuerRef := issuerRef(cluster, mode) + secretMeta := naming.ClusterPGBouncer(cluster) certName := PGBouncerCertificateName(cluster) @@ -523,7 +748,7 @@ func (c *controller) ApplyPGBouncerCertificate(ctx context.Context, cluster *v1b } existing := &v1.Certificate{} - err := c.cl.Get(ctx, types.NamespacedName{Name: certName, Namespace: cluster.Namespace}, existing) + err = c.cl.Get(ctx, types.NamespacedName{Name: certName, Namespace: cluster.Namespace}, existing) if err == nil { needsUpdate := false @@ -550,6 +775,11 @@ func (c *controller) ApplyPGBouncerCertificate(ctx context.Context, cluster *v1b needsUpdate = true } + if existing.Spec.IssuerRef != wantIssuerRef { + existing.Spec.IssuerRef = wantIssuerRef + needsUpdate = true + } + if !needsUpdate { return nil } @@ -570,13 +800,10 @@ func (c *controller) ApplyPGBouncerCertificate(ctx context.Context, cluster *v1b }, cluster.Name, "pgbouncer", cluster.Labels[naming.LabelVersion]), }, Spec: v1.CertificateSpec{ - SecretName: secretMeta.Name + "-frontend-tls", - CommonName: truncateForCommonName(cluster.Name, "-pgbouncer"), - DNSNames: dnsNames, - IssuerRef: cmmeta.IssuerReference{ - Name: naming.TLSIssuer(cluster).Name, - Kind: v1.IssuerKind, - }, + SecretName: secretMeta.Name + "-frontend-tls", + CommonName: truncateForCommonName(cluster.Name, "-pgbouncer"), + DNSNames: dnsNames, + IssuerRef: wantIssuerRef, Duration: &metav1.Duration{Duration: certDuration}, RenewBefore: &metav1.Duration{Duration: DefaultRenewBefore}, PrivateKey: &v1.CertificatePrivateKey{ @@ -612,6 +839,12 @@ func (c *controller) ApplyPGBouncerCertificate(ctx context.Context, cluster *v1b // ApplyReplicationCertificate creates a cert-manager Certificate resource for the replication client. func (c *controller) ApplyReplicationCertificate(ctx context.Context, cluster *v1beta1.PostgresCluster) error { + mode, err := ResolveIssuerMode(ctx, c.cl, cluster) + if err != nil { + return errors.Wrap(err, "failed to resolve issuer mode") + } + wantIssuerRef := issuerRef(cluster, mode) + secretMeta := naming.ReplicationClientCertSecret(cluster) certName := ReplicationCertificateName(cluster) commonName := "_crunchyrepl" @@ -622,7 +855,7 @@ func (c *controller) ApplyReplicationCertificate(ctx context.Context, cluster *v } existing := &v1.Certificate{} - err := c.cl.Get(ctx, types.NamespacedName{Name: certName, Namespace: cluster.Namespace}, existing) + err = c.cl.Get(ctx, types.NamespacedName{Name: certName, Namespace: cluster.Namespace}, existing) if err == nil { needsUpdate := false @@ -649,6 +882,11 @@ func (c *controller) ApplyReplicationCertificate(ctx context.Context, cluster *v needsUpdate = true } + if existing.Spec.IssuerRef != wantIssuerRef { + existing.Spec.IssuerRef = wantIssuerRef + needsUpdate = true + } + if !needsUpdate { return nil } @@ -669,13 +907,10 @@ func (c *controller) ApplyReplicationCertificate(ctx context.Context, cluster *v }, cluster.Name, "", cluster.Labels[naming.LabelVersion]), }, Spec: v1.CertificateSpec{ - SecretName: secretMeta.Name, - CommonName: commonName, - DNSNames: []string{commonName}, - IssuerRef: cmmeta.IssuerReference{ - Name: naming.TLSIssuer(cluster).Name, - Kind: v1.IssuerKind, - }, + SecretName: secretMeta.Name, + CommonName: commonName, + DNSNames: []string{commonName}, + IssuerRef: wantIssuerRef, Duration: &metav1.Duration{Duration: certDuration}, RenewBefore: &metav1.Duration{Duration: DefaultRenewBefore}, PrivateKey: &v1.CertificatePrivateKey{ @@ -712,6 +947,12 @@ func (c *controller) ApplyReplicationCertificate(ctx context.Context, cluster *v // for the pgBackRest client used by all PostgreSQL instances to connect to the // repository host. func (c *controller) ApplyPGBackRestClientCertificate(ctx context.Context, cluster *v1beta1.PostgresCluster) error { + mode, err := ResolveIssuerMode(ctx, c.cl, cluster) + if err != nil { + return errors.Wrap(err, "failed to resolve issuer mode") + } + wantIssuerRef := issuerRef(cluster, mode) + secretMeta := naming.PGBackRestClientCertSecret(cluster) certName := PGBackRestClientCertificateName(cluster) @@ -725,7 +966,7 @@ func (c *controller) ApplyPGBackRestClientCertificate(ctx context.Context, clust commonName := "pgbackrest@" + string(cluster.GetUID()) existing := &v1.Certificate{} - err := c.cl.Get(ctx, types.NamespacedName{Name: certName, Namespace: cluster.Namespace}, existing) + err = c.cl.Get(ctx, types.NamespacedName{Name: certName, Namespace: cluster.Namespace}, existing) if err == nil { needsUpdate := false @@ -758,6 +999,11 @@ func (c *controller) ApplyPGBackRestClientCertificate(ctx context.Context, clust needsUpdate = true } + if existing.Spec.IssuerRef != wantIssuerRef { + existing.Spec.IssuerRef = wantIssuerRef + needsUpdate = true + } + if !needsUpdate { return nil } @@ -777,13 +1023,10 @@ func (c *controller) ApplyPGBackRestClientCertificate(ctx context.Context, clust }, cluster.Name, "", cluster.Labels[naming.LabelVersion]), }, Spec: v1.CertificateSpec{ - SecretName: secretMeta.Name, - CommonName: commonName, - DNSNames: []string{commonName}, - IssuerRef: cmmeta.IssuerReference{ - Name: naming.TLSIssuer(cluster).Name, - Kind: v1.IssuerKind, - }, + SecretName: secretMeta.Name, + CommonName: commonName, + DNSNames: []string{commonName}, + IssuerRef: wantIssuerRef, Duration: &metav1.Duration{Duration: certDuration}, RenewBefore: &metav1.Duration{Duration: DefaultRenewBefore}, PrivateKey: &v1.CertificatePrivateKey{ @@ -822,6 +1065,12 @@ func (c *controller) ApplyPGBackRestRepoCertificate(ctx context.Context, cluster return errors.New("dnsNames cannot be empty") } + mode, err := ResolveIssuerMode(ctx, c.cl, cluster) + if err != nil { + return errors.Wrap(err, "failed to resolve issuer mode") + } + wantIssuerRef := issuerRef(cluster, mode) + secretMeta := naming.PGBackRestRepoCertSecret(cluster) certName := PGBackRestRepoCertificateName(cluster) @@ -831,7 +1080,7 @@ func (c *controller) ApplyPGBackRestRepoCertificate(ctx context.Context, cluster } existing := &v1.Certificate{} - err := c.cl.Get(ctx, types.NamespacedName{Name: certName, Namespace: cluster.Namespace}, existing) + err = c.cl.Get(ctx, types.NamespacedName{Name: certName, Namespace: cluster.Namespace}, existing) if err == nil { needsUpdate := false @@ -858,6 +1107,11 @@ func (c *controller) ApplyPGBackRestRepoCertificate(ctx context.Context, cluster needsUpdate = true } + if existing.Spec.IssuerRef != wantIssuerRef { + existing.Spec.IssuerRef = wantIssuerRef + needsUpdate = true + } + if !needsUpdate { return nil } @@ -877,13 +1131,10 @@ func (c *controller) ApplyPGBackRestRepoCertificate(ctx context.Context, cluster }, cluster.Name, "", cluster.Labels[naming.LabelVersion]), }, Spec: v1.CertificateSpec{ - SecretName: secretMeta.Name, - CommonName: truncateForCommonName(cluster.Name, "-pgbackrest-repo"), - DNSNames: dnsNames, - IssuerRef: cmmeta.IssuerReference{ - Name: naming.TLSIssuer(cluster).Name, - Kind: v1.IssuerKind, - }, + SecretName: secretMeta.Name, + CommonName: truncateForCommonName(cluster.Name, "-pgbackrest-repo"), + DNSNames: dnsNames, + IssuerRef: wantIssuerRef, Duration: &metav1.Duration{Duration: certDuration}, RenewBefore: &metav1.Duration{Duration: DefaultRenewBefore}, PrivateKey: &v1.CertificatePrivateKey{ diff --git a/percona/certmanager/certmanager_test.go b/percona/certmanager/certmanager_test.go index bea8548a93..66ea2d7f04 100644 --- a/percona/certmanager/certmanager_test.go +++ b/percona/certmanager/certmanager_test.go @@ -7,12 +7,15 @@ import ( "time" v1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" + cmmeta "github.com/cert-manager/cert-manager/pkg/apis/meta/v1" "github.com/cert-manager/cert-manager/pkg/util/cmapichecker" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" + k8serrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/client-go/rest" sigs "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" @@ -213,6 +216,65 @@ func TestApplyIssuer(t *testing.T) { err = ctrl.ApplyIssuer(t.Context(), cluster) require.NoError(t, err) }) + + t.Run("skip when external issuer", func(t *testing.T) { + cluster := testCluster() + cluster.Name = "tls-issuer-external" + cluster.Spec.TLS = &v1beta1.TLSSpec{ + IssuerConf: &cmmeta.IssuerReference{Name: "vault-issuer", Kind: "VaultClusterIssuer"}, + } + client := setupFakeClient(t, cluster) + ctrl := NewController(client, client.Scheme(), false) + + err := ctrl.ApplyIssuer(t.Context(), cluster) + require.NoError(t, err) + + list := &v1.IssuerList{} + require.NoError(t, client.List(t.Context(), list)) + assert.Len(t, list.Items, 0) + }) + + t.Run("create cluster-scoped TLS issuer when Kind is ClusterIssuer", func(t *testing.T) { + cluster := testCluster() + cluster.Name = "tls-issuer-cluster-scoped" + cluster.Spec.TLS = &v1beta1.TLSSpec{ + IssuerConf: &cmmeta.IssuerReference{Name: "shared-tls-issuer", Kind: v1.ClusterIssuerKind}, + } + client := setupFakeClient(t, cluster) + ctrl := NewController(client, client.Scheme(), false) + + err := ctrl.ApplyIssuer(t.Context(), cluster) + require.NoError(t, err) + + issuer := &v1.ClusterIssuer{} + err = client.Get(t.Context(), sigs.ObjectKey{Name: "shared-tls-issuer"}, issuer) + require.NoError(t, err) + require.NotNil(t, issuer.Spec.CA) + assert.Equal(t, naming.ClusterCACertSecret(cluster, CertManagerNamespace()).Name, issuer.Spec.CA.SecretName) + assert.Empty(t, issuer.OwnerReferences) + + // idempotent + err = ctrl.ApplyIssuer(t.Context(), cluster) + require.NoError(t, err) + }) + + t.Run("managed namespaced honors issuerConf name override", func(t *testing.T) { + cluster := testCluster() + cluster.Name = "tls-issuer-custom-name" + cluster.Spec.TLS = &v1beta1.TLSSpec{ + IssuerConf: &cmmeta.IssuerReference{Name: "my-custom-issuer-name"}, + } + client := setupFakeClient(t, cluster) + ctrl := NewController(client, client.Scheme(), false) + + err := ctrl.ApplyIssuer(t.Context(), cluster) + require.NoError(t, err) + + issuer := &v1.Issuer{} + err = client.Get(t.Context(), sigs.ObjectKey{Namespace: cluster.Namespace, Name: "my-custom-issuer-name"}, issuer) + require.NoError(t, err) + require.Len(t, issuer.OwnerReferences, 1) + }) } func TestApplyCAIssuer(t *testing.T) { @@ -245,6 +307,47 @@ func TestApplyCAIssuer(t *testing.T) { err = ctrl.ApplyCAIssuer(t.Context(), cluster) require.NoError(t, err) }) + + t.Run("skip when external issuer", func(t *testing.T) { + cluster := testCluster() + cluster.Name = "ca-test-external" + cluster.Spec.TLS = &v1beta1.TLSSpec{ + IssuerConf: &cmmeta.IssuerReference{Name: "vault-issuer", Kind: "VaultClusterIssuer"}, + } + client := setupFakeClient(t, cluster) + ctrl := NewController(client, client.Scheme(), false) + + err := ctrl.ApplyCAIssuer(t.Context(), cluster) + require.NoError(t, err) + + list := &v1.IssuerList{} + require.NoError(t, client.List(t.Context(), list)) + assert.Len(t, list.Items, 0) + }) + + t.Run("create cluster-scoped CA issuer when Kind is ClusterIssuer", func(t *testing.T) { + cluster := testCluster() + cluster.Name = "ca-test-cluster-scoped" + cluster.Spec.TLS = &v1beta1.TLSSpec{ + IssuerConf: &cmmeta.IssuerReference{Name: "shared-tls-issuer", Kind: v1.ClusterIssuerKind}, + } + client := setupFakeClient(t, cluster) + ctrl := NewController(client, client.Scheme(), false) + + err := ctrl.ApplyCAIssuer(t.Context(), cluster) + require.NoError(t, err) + + issuer := &v1.ClusterIssuer{} + meta := naming.ClusterCAIssuer(cluster) + err = client.Get(t.Context(), sigs.ObjectKey{Name: meta.Name}, issuer) + require.NoError(t, err) + assert.NotNil(t, issuer.Spec.SelfSigned) + assert.Empty(t, issuer.OwnerReferences) + + // idempotent + err = ctrl.ApplyCAIssuer(t.Context(), cluster) + require.NoError(t, err) + }) } func TestApplyCACertificate(t *testing.T) { @@ -290,6 +393,50 @@ func TestApplyCACertificate(t *testing.T) { err = ctrl.ApplyCACertificate(t.Context(), cluster) require.NoError(t, err) }) + + t.Run("skip when external issuer", func(t *testing.T) { + cluster := testCluster() + cluster.Name = "ca-cert-external" + cluster.Spec.TLS = &v1beta1.TLSSpec{ + IssuerConf: &cmmeta.IssuerReference{Name: "vault-issuer", Kind: "VaultClusterIssuer"}, + } + client := setupFakeClient(t, cluster) + ctrl := NewController(client, client.Scheme(), false) + + err := ctrl.ApplyCACertificate(t.Context(), cluster) + require.NoError(t, err) + + list := &v1.CertificateList{} + require.NoError(t, client.List(t.Context(), list)) + assert.Len(t, list.Items, 0) + }) + + t.Run("places CA certificate in cert-manager namespace when Kind is ClusterIssuer", func(t *testing.T) { + cluster := testCluster() + cluster.Name = "ca-cert-cluster-scoped" + cluster.Spec.TLS = &v1beta1.TLSSpec{ + IssuerConf: &cmmeta.IssuerReference{Name: "shared-tls-issuer", Kind: v1.ClusterIssuerKind}, + } + client := setupFakeClient(t, cluster) + ctrl := NewController(client, client.Scheme(), false) + + err := ctrl.ApplyCACertificate(t.Context(), cluster) + require.NoError(t, err) + + cert := &v1.Certificate{} + meta := naming.ClusterCACertSecret(cluster, CertManagerNamespace()) + err = client.Get(t.Context(), sigs.ObjectKey{Namespace: meta.Namespace, Name: meta.Name}, cert) + require.NoError(t, err) + assert.Equal(t, meta.Name, cert.Spec.SecretName) + assert.True(t, cert.Spec.IsCA) + assert.Equal(t, naming.ClusterCAIssuer(cluster).Name, cert.Spec.IssuerRef.Name) + assert.Equal(t, v1.ClusterIssuerKind, cert.Spec.IssuerRef.Kind) + assert.Empty(t, cert.OwnerReferences) + + // idempotent + err = ctrl.ApplyCACertificate(t.Context(), cluster) + require.NoError(t, err) + }) } func TestApplyClusterCertificate(t *testing.T) { @@ -1069,3 +1216,273 @@ func TestCustomTLSDurations(t *testing.T) { assert.Equal(t, customCertDuration, cert.Spec.Duration.Duration) }) } + +// forbiddenGetClient wraps a client.Client and returns a Forbidden error from +// Get for *v1.ClusterIssuer, simulating a cluster that hasn't granted RBAC +// for clusterissuers.cert-manager.io (the default — see K8SPG-951's design: +// no such RBAC ships by default in either deployment mode). +type forbiddenGetClient struct { + sigs.Client +} + +func (f *forbiddenGetClient) Get(ctx context.Context, key sigs.ObjectKey, obj sigs.Object, opts ...sigs.GetOption) error { + if _, ok := obj.(*v1.ClusterIssuer); ok { + return k8serrors.NewForbidden( + schema.GroupResource{Group: "cert-manager.io", Resource: "clusterissuers"}, + key.Name, fmt.Errorf("forbidden")) + } + return f.Client.Get(ctx, key, obj, opts...) +} + +func TestCertManagerNamespace(t *testing.T) { + t.Run("defaults to cert-manager", func(t *testing.T) { + t.Setenv("CERTMANAGER_NAMESPACE", "") + assert.Equal(t, "cert-manager", CertManagerNamespace()) + }) + + t.Run("honors CERTMANAGER_NAMESPACE", func(t *testing.T) { + t.Setenv("CERTMANAGER_NAMESPACE", "custom-cm-ns") + assert.Equal(t, "custom-cm-ns", CertManagerNamespace()) + }) +} + +func TestResolveIssuerMode(t *testing.T) { + t.Run("nil TLS returns managed namespaced", func(t *testing.T) { + cluster := testCluster() + cl := setupFakeClient(t, cluster) + + mode, err := ResolveIssuerMode(t.Context(), cl, cluster) + require.NoError(t, err) + assert.Equal(t, IssuerModeManagedNamespaced, mode) + }) + + t.Run("nil IssuerConf returns managed namespaced", func(t *testing.T) { + cluster := testCluster() + cluster.Spec.TLS = &v1beta1.TLSSpec{} + cl := setupFakeClient(t, cluster) + + mode, err := ResolveIssuerMode(t.Context(), cl, cluster) + require.NoError(t, err) + assert.Equal(t, IssuerModeManagedNamespaced, mode) + }) + + t.Run("Kind Issuer returns managed namespaced", func(t *testing.T) { + cluster := testCluster() + cluster.Spec.TLS = &v1beta1.TLSSpec{ + IssuerConf: &cmmeta.IssuerReference{Name: "my-issuer", Kind: v1.IssuerKind}, + } + cl := setupFakeClient(t, cluster) + + mode, err := ResolveIssuerMode(t.Context(), cl, cluster) + require.NoError(t, err) + assert.Equal(t, IssuerModeManagedNamespaced, mode) + }) + + t.Run("Kind ClusterIssuer not found returns managed cluster", func(t *testing.T) { + cluster := testCluster() + cluster.Spec.TLS = &v1beta1.TLSSpec{ + IssuerConf: &cmmeta.IssuerReference{Name: "shared-issuer", Kind: v1.ClusterIssuerKind}, + } + cl := setupFakeClient(t, cluster) + + mode, err := ResolveIssuerMode(t.Context(), cl, cluster) + require.NoError(t, err) + assert.Equal(t, IssuerModeManagedCluster, mode) + }) + + t.Run("Kind ClusterIssuer readable returns managed cluster", func(t *testing.T) { + cluster := testCluster() + cluster.Spec.TLS = &v1beta1.TLSSpec{ + IssuerConf: &cmmeta.IssuerReference{Name: "shared-issuer", Kind: v1.ClusterIssuerKind}, + } + existing := &v1.ClusterIssuer{ObjectMeta: metav1.ObjectMeta{Name: "shared-issuer"}} + cl := setupFakeClient(t, cluster, existing) + + mode, err := ResolveIssuerMode(t.Context(), cl, cluster) + require.NoError(t, err) + assert.Equal(t, IssuerModeManagedCluster, mode) + }) + + t.Run("Kind ClusterIssuer forbidden returns external", func(t *testing.T) { + cluster := testCluster() + cluster.Spec.TLS = &v1beta1.TLSSpec{ + IssuerConf: &cmmeta.IssuerReference{Name: "shared-issuer", Kind: v1.ClusterIssuerKind}, + } + cl := &forbiddenGetClient{Client: setupFakeClient(t, cluster)} + + mode, err := ResolveIssuerMode(t.Context(), cl, cluster) + require.NoError(t, err) + assert.Equal(t, IssuerModeExternal, mode) + }) + + t.Run("third-party Kind returns external", func(t *testing.T) { + cluster := testCluster() + cluster.Spec.TLS = &v1beta1.TLSSpec{ + IssuerConf: &cmmeta.IssuerReference{Name: "vault-issuer", Kind: "VaultClusterIssuer", Group: "vault.example.com"}, + } + cl := setupFakeClient(t, cluster) + + mode, err := ResolveIssuerMode(t.Context(), cl, cluster) + require.NoError(t, err) + assert.Equal(t, IssuerModeExternal, mode) + }) +} + +func TestIssuerRef(t *testing.T) { + t.Run("managed namespaced without issuerConf uses generated name", func(t *testing.T) { + cluster := testCluster() + ref := issuerRef(cluster, IssuerModeManagedNamespaced) + assert.Equal(t, naming.TLSIssuer(cluster).Name, ref.Name) + assert.Equal(t, v1.IssuerKind, ref.Kind) + assert.Equal(t, "", ref.Group) + }) + + t.Run("managed namespaced with issuerConf name override", func(t *testing.T) { + cluster := testCluster() + cluster.Spec.TLS = &v1beta1.TLSSpec{ + IssuerConf: &cmmeta.IssuerReference{Name: "custom-tls-issuer"}, + } + ref := issuerRef(cluster, IssuerModeManagedNamespaced) + assert.Equal(t, "custom-tls-issuer", ref.Name) + assert.Equal(t, v1.IssuerKind, ref.Kind) + }) + + t.Run("managed cluster uses issuerConf name with ClusterIssuer kind", func(t *testing.T) { + cluster := testCluster() + cluster.Spec.TLS = &v1beta1.TLSSpec{ + IssuerConf: &cmmeta.IssuerReference{Name: "shared-issuer", Kind: v1.ClusterIssuerKind}, + } + ref := issuerRef(cluster, IssuerModeManagedCluster) + assert.Equal(t, "shared-issuer", ref.Name) + assert.Equal(t, v1.ClusterIssuerKind, ref.Kind) + }) + + t.Run("external uses issuerConf verbatim with default group", func(t *testing.T) { + cluster := testCluster() + cluster.Spec.TLS = &v1beta1.TLSSpec{ + IssuerConf: &cmmeta.IssuerReference{Name: "vault-issuer", Kind: "VaultClusterIssuer"}, + } + ref := issuerRef(cluster, IssuerModeExternal) + assert.Equal(t, "vault-issuer", ref.Name) + assert.Equal(t, "VaultClusterIssuer", ref.Kind) + assert.Equal(t, "cert-manager.io", ref.Group) + }) + + t.Run("external preserves explicit group", func(t *testing.T) { + cluster := testCluster() + cluster.Spec.TLS = &v1beta1.TLSSpec{ + IssuerConf: &cmmeta.IssuerReference{Name: "vault-issuer", Kind: "VaultClusterIssuer", Group: "vault.example.com"}, + } + ref := issuerRef(cluster, IssuerModeExternal) + assert.Equal(t, "vault.example.com", ref.Group) + }) +} + +func TestApplyCertificateIssuerRefDrift(t *testing.T) { + newIssuerConf := func(kind string) *v1beta1.TLSSpec { + return &v1beta1.TLSSpec{IssuerConf: &cmmeta.IssuerReference{Name: "vault-issuer", Kind: kind}} + } + + t.Run("cluster certificate switches to external issuerRef on update", func(t *testing.T) { + cluster := testCluster() + cluster.Name = "drift-cluster-cert" + client := setupFakeClient(t, cluster) + ctrl := NewController(client, client.Scheme(), false) + + dnsNames := []string{"drift-cluster-cert-primary.test-namespace.svc"} + require.NoError(t, ctrl.ApplyClusterCertificate(t.Context(), cluster, dnsNames)) + + cluster.Spec.TLS = newIssuerConf("VaultClusterIssuer") + require.NoError(t, ctrl.ApplyClusterCertificate(t.Context(), cluster, dnsNames)) + + cert := &v1.Certificate{} + secretName := naming.PostgresTLSSecret(cluster) + require.NoError(t, client.Get(t.Context(), sigs.ObjectKey{Namespace: cluster.Namespace, Name: secretName.Name}, cert)) + assert.Equal(t, "vault-issuer", cert.Spec.IssuerRef.Name) + assert.Equal(t, "VaultClusterIssuer", cert.Spec.IssuerRef.Kind) + }) + + t.Run("instance certificate switches issuerRef on update", func(t *testing.T) { + cluster := testCluster() + cluster.Name = "drift-instance-cert" + client := setupFakeClient(t, cluster) + ctrl := NewController(client, client.Scheme(), false) + + instanceName := "drift-instance-cert-instance-0" + dnsNames := []string{instanceName + ".test-namespace.svc"} + require.NoError(t, ctrl.ApplyInstanceCertificate(t.Context(), cluster, instanceName, dnsNames)) + + cluster.Spec.TLS = newIssuerConf("VaultClusterIssuer") + require.NoError(t, ctrl.ApplyInstanceCertificate(t.Context(), cluster, instanceName, dnsNames)) + + cert := &v1.Certificate{} + require.NoError(t, client.Get(t.Context(), sigs.ObjectKey{Namespace: cluster.Namespace, Name: instanceName + "-cert"}, cert)) + assert.Equal(t, "vault-issuer", cert.Spec.IssuerRef.Name) + }) + + t.Run("pgbouncer certificate switches issuerRef on update", func(t *testing.T) { + cluster := testCluster() + cluster.Name = "drift-pgbouncer-cert" + client := setupFakeClient(t, cluster) + ctrl := NewController(client, client.Scheme(), false) + + dnsNames := []string{"drift-pgbouncer-cert-pgbouncer.test-namespace.svc"} + require.NoError(t, ctrl.ApplyPGBouncerCertificate(t.Context(), cluster, dnsNames)) + + cluster.Spec.TLS = newIssuerConf("VaultClusterIssuer") + require.NoError(t, ctrl.ApplyPGBouncerCertificate(t.Context(), cluster, dnsNames)) + + cert := &v1.Certificate{} + require.NoError(t, client.Get(t.Context(), sigs.ObjectKey{Namespace: cluster.Namespace, Name: cluster.Name + "-pgbouncer-cert"}, cert)) + assert.Equal(t, "vault-issuer", cert.Spec.IssuerRef.Name) + }) + + t.Run("replication certificate switches issuerRef on update", func(t *testing.T) { + cluster := testCluster() + cluster.Name = "drift-replication-cert" + client := setupFakeClient(t, cluster) + ctrl := NewController(client, client.Scheme(), false) + + require.NoError(t, ctrl.ApplyReplicationCertificate(t.Context(), cluster)) + + cluster.Spec.TLS = newIssuerConf("VaultClusterIssuer") + require.NoError(t, ctrl.ApplyReplicationCertificate(t.Context(), cluster)) + + cert := &v1.Certificate{} + require.NoError(t, client.Get(t.Context(), sigs.ObjectKey{Namespace: cluster.Namespace, Name: cluster.Name + "-replication-cert"}, cert)) + assert.Equal(t, "vault-issuer", cert.Spec.IssuerRef.Name) + }) + + t.Run("pgbackrest client certificate switches issuerRef on update", func(t *testing.T) { + cluster := testCluster() + cluster.Name = "drift-pgbr-client-cert" + client := setupFakeClient(t, cluster) + ctrl := NewController(client, client.Scheme(), false) + + require.NoError(t, ctrl.ApplyPGBackRestClientCertificate(t.Context(), cluster)) + + cluster.Spec.TLS = newIssuerConf("VaultClusterIssuer") + require.NoError(t, ctrl.ApplyPGBackRestClientCertificate(t.Context(), cluster)) + + cert := &v1.Certificate{} + require.NoError(t, client.Get(t.Context(), sigs.ObjectKey{Namespace: cluster.Namespace, Name: cluster.Name + "-pgbackrest-client-cert"}, cert)) + assert.Equal(t, "vault-issuer", cert.Spec.IssuerRef.Name) + }) + + t.Run("pgbackrest repo certificate switches issuerRef on update", func(t *testing.T) { + cluster := testCluster() + cluster.Name = "drift-pgbr-repo-cert" + client := setupFakeClient(t, cluster) + ctrl := NewController(client, client.Scheme(), false) + + dnsNames := []string{cluster.Name + "-repo-host-0." + cluster.Name + "-pgbackrest.test-namespace.svc"} + require.NoError(t, ctrl.ApplyPGBackRestRepoCertificate(t.Context(), cluster, dnsNames)) + + cluster.Spec.TLS = newIssuerConf("VaultClusterIssuer") + require.NoError(t, ctrl.ApplyPGBackRestRepoCertificate(t.Context(), cluster, dnsNames)) + + cert := &v1.Certificate{} + require.NoError(t, client.Get(t.Context(), sigs.ObjectKey{Namespace: cluster.Namespace, Name: cluster.Name + "-pgbackrest-repo-cert"}, cert)) + assert.Equal(t, "vault-issuer", cert.Spec.IssuerRef.Name) + }) +} diff --git a/percona/runtime/runtime.go b/percona/runtime/runtime.go index a561e7b6cd..204748a370 100644 --- a/percona/runtime/runtime.go +++ b/percona/runtime/runtime.go @@ -5,8 +5,10 @@ import ( "strings" "time" + cmv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" "k8s.io/client-go/rest" "sigs.k8s.io/controller-runtime/pkg/cache" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/manager" r "github.com/percona/percona-postgresql-operator/v2/internal/controller/runtime" @@ -19,6 +21,19 @@ const refreshInterval time.Duration = 60 * time.Minute const ElectionID string = "08db3feb.percona.com" +// ClientCacheOptions returns the client.CacheOptions CreateRuntimeManager +// applies to every manager it builds. The operator does not request +// cluster-wide list/watch RBAC for clusterissuers.cert-manager.io by +// default (K8SPG-951's managed ClusterIssuer mode), so Get calls for +// ClusterIssuer must bypass the informer cache entirely and hit the API +// server directly, which only needs "get" on the one named object. +// Exported so it's directly unit-testable without constructing a manager. +func ClientCacheOptions() *client.CacheOptions { + return &client.CacheOptions{ + DisableFor: []client.Object{&cmv1.ClusterIssuer{}}, + } +} + // CreateRuntimeManager wraps internal/controller/runtime.NewManager and modifies the given options: // - Fully overwrites the Cache field // - Sets Cache.SyncPeriod to refreshInterval const @@ -42,6 +57,8 @@ func CreateRuntimeManager(config *rest.Config, features feature.MutableGate, opt options.Cache.DefaultNamespaces = namespaces } + options.Client.Cache = ClientCacheOptions() + options.BaseContext = func() context.Context { ctx := context.Background() return feature.NewContext(ctx, features) diff --git a/percona/runtime/runtime_test.go b/percona/runtime/runtime_test.go new file mode 100644 index 0000000000..2c863f4744 --- /dev/null +++ b/percona/runtime/runtime_test.go @@ -0,0 +1,34 @@ +package runtime + +import ( + "testing" + + cmv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/rest" + "sigs.k8s.io/controller-runtime/pkg/manager" + + "github.com/percona/percona-postgresql-operator/v2/internal/feature" +) + +func TestCreateRuntimeManagerDisablesClusterIssuerCache(t *testing.T) { + t.Setenv("WATCH_NAMESPACE", "") + + scheme := runtime.NewScheme() + _ = cmv1.AddToScheme(scheme) + + opts := manager.Options{Scheme: scheme} + mgr, err := CreateRuntimeManager(&rest.Config{Host: "https://127.0.0.1:1"}, feature.NewGate(), opts) + require.NoError(t, err) + require.NotNil(t, mgr) +} + +func TestClientCacheOptionsDisablesClusterIssuer(t *testing.T) { + opts := ClientCacheOptions() + require.NotNil(t, opts) + require.Len(t, opts.DisableFor, 1) + _, ok := opts.DisableFor[0].(*cmv1.ClusterIssuer) + assert.True(t, ok) +} From e347f11c9fe5205cf4364010a61ce9ff4024745f Mon Sep 17 00:00:00 2001 From: Mayank Shah Date: Wed, 15 Jul 2026 13:17:41 +0530 Subject: [PATCH 03/17] cleanup Signed-off-by: Mayank Shah --- internal/controller/postgrescluster/pki.go | 20 +++++-------------- .../controller/postgrescluster/pki_test.go | 2 +- percona/certmanager/certmanager.go | 8 +++----- 3 files changed, 9 insertions(+), 21 deletions(-) diff --git a/internal/controller/postgrescluster/pki.go b/internal/controller/postgrescluster/pki.go index fed716e506..36f79fd627 100644 --- a/internal/controller/postgrescluster/pki.go +++ b/internal/controller/postgrescluster/pki.go @@ -77,17 +77,6 @@ func (r *Reconciler) reconcileRootCertificate( err = errors.WithStack( r.Client.Get(ctx, client.ObjectKeyFromObject(existing), existing)) - // K8SPG-555: we need to check ca certificate from old operator versions - // TODO: remove when 2.4.0 will become unsupported - if k8serrors.IsNotFound(err) && mode == certmanager.IssuerModeManagedNamespaced { - nn := client.ObjectKeyFromObject(existing) - nn.Name = naming.RootCertSecret - err = errors.WithStack( - r.Client.Get(ctx, nn, existing)) - if err == nil { - existing.Name = naming.RootCertSecret - } - } if k8serrors.IsNotFound(err) { err = nil @@ -252,14 +241,14 @@ func (r *Reconciler) reconcileClusterCertificate( } if certManagerManaged { - return r.reconcileCertManagerClusterCertificate(ctx, root, cluster, primaryService, replicaService) + return r.reconcileCertManagerClusterCertificate(ctx, cluster, primaryService, replicaService) } // cluster certificates are not managed by cert-manager // but Certificate object exists due to the bug described in K8SPG-1017 // we need to reconcile them anyway to update ownerRef for K8SPG-1007. if cert := certmanager.ClusterCertificateName(cluster); r.shouldReconcileCertManagerCertificate(ctx, cluster.Namespace, cert) { - _, err := r.reconcileCertManagerClusterCertificate(ctx, root, cluster, primaryService, replicaService) + _, err := r.reconcileCertManagerClusterCertificate(ctx, cluster, primaryService, replicaService) if err != nil { logging.FromContext(ctx).Error(err, "failed to reconcile Certificate", "name", cert) } @@ -353,8 +342,9 @@ func (r *Reconciler) reconcileInternalClusterCertificate( // reconcileCertManagerClusterCertificate creates a cluster certificate using cert-manager. // It first ensures the TLS issuer exists, then creates the cluster Certificate CR. func (r *Reconciler) reconcileCertManagerClusterCertificate( - ctx context.Context, root *pki.RootCertificateAuthority, - cluster *v1beta1.PostgresCluster, primaryService *corev1.Service, + ctx context.Context, + cluster *v1beta1.PostgresCluster, + primaryService *corev1.Service, replicaService *corev1.Service, ) ( *corev1.SecretProjection, error, diff --git a/internal/controller/postgrescluster/pki_test.go b/internal/controller/postgrescluster/pki_test.go index 39d4ec101c..3e3f214d62 100644 --- a/internal/controller/postgrescluster/pki_test.go +++ b/internal/controller/postgrescluster/pki_test.go @@ -958,7 +958,7 @@ func TestIssuerModeAwareness(t *testing.T) { primaryService := &corev1.Service{ObjectMeta: metav1.ObjectMeta{Namespace: namespace, Name: "external-cluster-cert-primary"}} replicaService := &corev1.Service{ObjectMeta: metav1.ObjectMeta{Namespace: namespace, Name: "external-cluster-cert-replicas"}} - _, err := r.reconcileCertManagerClusterCertificate(ctx, nil, cluster, primaryService, replicaService) + _, err := r.reconcileCertManagerClusterCertificate(ctx, cluster, primaryService, replicaService) assert.NilError(t, err) assert.Equal(t, recovery.applyIssuerCalls, 0) }) diff --git a/percona/certmanager/certmanager.go b/percona/certmanager/certmanager.go index 4eb997d019..a4d2a9057f 100644 --- a/percona/certmanager/certmanager.go +++ b/percona/certmanager/certmanager.go @@ -413,14 +413,12 @@ func (c *controller) ApplyCACertificate(ctx context.Context, cluster *v1beta1.Po clusterScoped := mode == IssuerModeManagedCluster - var secretMeta metav1.ObjectMeta - var issuerRefValue cmmeta.IssuerReference + secretMeta := naming.PostgresRootCASecret(cluster) + issuerRefValue := cmmeta.IssuerReference{Name: naming.CAIssuer(cluster).Name, Kind: v1.IssuerKind} + if clusterScoped { secretMeta = naming.ClusterCACertSecret(cluster, CertManagerNamespace()) issuerRefValue = cmmeta.IssuerReference{Name: naming.ClusterCAIssuer(cluster).Name, Kind: v1.ClusterIssuerKind} - } else { - secretMeta = naming.PostgresRootCASecret(cluster) - issuerRefValue = cmmeta.IssuerReference{Name: naming.CAIssuer(cluster).Name, Kind: v1.IssuerKind} } certName := secretMeta.Name certNamespace := secretMeta.Namespace From 6e5dcbd3f588bc723ff878fcea99ce620aa82a7e Mon Sep 17 00:00:00 2001 From: Mayank Shah Date: Wed, 15 Jul 2026 13:34:30 +0530 Subject: [PATCH 04/17] use issuer name for secret name Signed-off-by: Mayank Shah --- internal/naming/names.go | 19 ++++++------------- internal/naming/names_test.go | 11 +++++++++-- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/internal/naming/names.go b/internal/naming/names.go index 416c0dc3da..755b23d929 100644 --- a/internal/naming/names.go +++ b/internal/naming/names.go @@ -657,26 +657,19 @@ func TLSIssuer(cluster *v1beta1.PostgresCluster) metav1.ObjectMeta { } } -// ClusterCAIssuer returns the ObjectMeta for the cluster-scoped self-signed -// CA ClusterIssuer used by cert-manager when spec.tls.issuerConf.kind is -// "ClusterIssuer" (K8SPG-951). The name is qualified by cluster name and -// namespace so multiple PostgresClusters sharing this mode don't collide; -// ClusterIssuers have no namespace of their own. +// K8SPG-951 +// ClusterCAIssuer returns the ObjectMeta for the cluster-scoped CA ClusterIssuer used by cert-manager. func ClusterCAIssuer(cluster *v1beta1.PostgresCluster) metav1.ObjectMeta { return metav1.ObjectMeta{ - Name: cluster.Name + "-" + cluster.Namespace + "-ca-issuer", + Name: cluster.Spec.TLS.IssuerConf.Name + "-ca-issuer", } } -// ClusterCACertSecret returns the ObjectMeta for the CA certificate Secret -// backing a cluster-scoped self-signed CA ClusterIssuer (K8SPG-951). -// certManagerNamespace is cert-manager's shared "cluster resource namespace" -// (see percona/certmanager.CertManagerNamespace) — a ClusterIssuer's -// spec.ca.secretName is resolved there, not in the PostgresCluster's own -// namespace. +// K8SPG-951 +// ClusterTLSIssuer returns the ObjectMeta for the cluster-scoped TLS ClusterIssuer used by cert-manager. func ClusterCACertSecret(cluster *v1beta1.PostgresCluster, certManagerNamespace string) metav1.ObjectMeta { return metav1.ObjectMeta{ Namespace: certManagerNamespace, - Name: cluster.Name + "-" + cluster.Namespace + "-cluster-ca-cert", + Name: cluster.Spec.TLS.IssuerConf.Name + "-ca-cert", } } diff --git a/internal/naming/names_test.go b/internal/naming/names_test.go index e45fc6d443..0834d72d1f 100644 --- a/internal/naming/names_test.go +++ b/internal/naming/names_test.go @@ -8,6 +8,7 @@ import ( "strings" "testing" + cmmeta "github.com/cert-manager/cert-manager/pkg/apis/meta/v1" "gotest.tools/v3/assert" appsv1 "k8s.io/api/apps/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -337,9 +338,12 @@ func TestClusterCAIssuer(t *testing.T) { cluster := &v1beta1.PostgresCluster{} cluster.Namespace = "postgres-operator" cluster.Name = "hippo" + cluster.Spec.TLS = &v1beta1.TLSSpec{ + IssuerConf: &cmmeta.IssuerReference{Name: "shared-tls-issuer"}, + } meta := ClusterCAIssuer(cluster) - assert.Equal(t, meta.Name, "hippo-postgres-operator-ca-issuer") + assert.Equal(t, meta.Name, "shared-tls-issuer-ca-issuer") assert.Equal(t, meta.Namespace, "") } @@ -347,8 +351,11 @@ func TestClusterCACertSecret(t *testing.T) { cluster := &v1beta1.PostgresCluster{} cluster.Namespace = "postgres-operator" cluster.Name = "hippo" + cluster.Spec.TLS = &v1beta1.TLSSpec{ + IssuerConf: &cmmeta.IssuerReference{Name: "shared-tls-issuer"}, + } meta := ClusterCACertSecret(cluster, "cert-manager") - assert.Equal(t, meta.Name, "hippo-postgres-operator-cluster-ca-cert") + assert.Equal(t, meta.Name, "shared-tls-issuer-ca-cert") assert.Equal(t, meta.Namespace, "cert-manager") } From abe1790031e074fbaf5a643b8625b13f2728cfd6 Mon Sep 17 00:00:00 2001 From: Mayank Shah Date: Wed, 15 Jul 2026 13:39:28 +0530 Subject: [PATCH 05/17] trim down unnecessary comments Signed-off-by: Mayank Shah --- percona/certmanager/certmanager.go | 42 ++++-------------------------- 1 file changed, 5 insertions(+), 37 deletions(-) diff --git a/percona/certmanager/certmanager.go b/percona/certmanager/certmanager.go index a4d2a9057f..1704c00ca9 100644 --- a/percona/certmanager/certmanager.go +++ b/percona/certmanager/certmanager.go @@ -49,33 +49,18 @@ const ( DefaultRenewBefore = 30 * 24 * time.Hour ) -// IssuerMode describes how the operator should treat -// cluster.Spec.TLS.IssuerConf (K8SPG-951). type IssuerMode int const ( - // IssuerModeManagedNamespaced: issuerConf is unset, or its Kind is "" or - // "Issuer" — the operator owns a namespaced self-signed CA issuer, CA - // certificate, and CA-backed TLS issuer (the long-standing behavior). + // IssuerModeManagedNamespaced: operator owns and manages a namespaced self-signed Issuer IssuerModeManagedNamespaced IssuerMode = iota - // IssuerModeManagedCluster: issuerConf.Kind is "ClusterIssuer" and the - // operator can read the named ClusterIssuer (or it doesn't exist yet) — - // the operator owns a cluster-scoped self-signed CA ClusterIssuer, a CA - // certificate in cert-manager's shared namespace, and a CA-backed TLS - // ClusterIssuer. + // IssuerModeManagedCluster: operator owns and manages a cluster-scoped self-signed ClusterIssuer IssuerModeManagedCluster - // IssuerModeExternal: issuerConf.Kind is a third-party kind, or is - // "ClusterIssuer" but the operator is Forbidden from reading it — every - // leaf Certificate references issuerConf directly and the operator - // creates nothing issuer-related. + // IssuerModeExternal: operator does nothing for issuer, simply trusts that it exists and uses it to sign certificates IssuerModeExternal ) -// CertManagerNamespace returns cert-manager's shared "cluster resource -// namespace" — where a ClusterIssuer's spec.ca.secretName is resolved from, -// as opposed to the namespace of whatever references the ClusterIssuer. -// Configurable via the CERTMANAGER_NAMESPACE environment variable; defaults -// to "cert-manager". +// CertManagerNamespace returns the namespace where cert-manager is installed. func CertManagerNamespace() string { if ns := os.Getenv("CERTMANAGER_NAMESPACE"); ns != "" { return ns @@ -83,8 +68,6 @@ func CertManagerNamespace() string { return "cert-manager" } -// issuerConf returns cluster.Spec.TLS.IssuerConf, or nil if TLS or -// IssuerConf is unset. func issuerConf(cluster *v1beta1.PostgresCluster) *cmmeta.IssuerReference { if cluster.Spec.TLS == nil { return nil @@ -92,13 +75,7 @@ func issuerConf(cluster *v1beta1.PostgresCluster) *cmmeta.IssuerReference { return cluster.Spec.TLS.IssuerConf } -// ResolveIssuerMode determines how the operator should handle -// cluster.Spec.TLS.IssuerConf. For Kind == "ClusterIssuer" it performs a -// live Get to check whether the operator can read the named ClusterIssuer: -// Forbidden (no RBAC for clusterissuers.cert-manager.io — the default, -// since none is shipped) downgrades to IssuerModeExternal so the operator -// doesn't try to own something it can't inspect. NotFound is not an error -// here — the operator will create it as part of managing it. +// ResolveIssuerMode determines how the operator should handle cluster.Spec.TLS.IssuerConf. func ResolveIssuerMode(ctx context.Context, cl client.Client, cluster *v1beta1.PostgresCluster) (IssuerMode, error) { ic := issuerConf(cluster) if ic == nil { @@ -124,15 +101,6 @@ func ResolveIssuerMode(ctx context.Context, cl client.Client, cluster *v1beta1.P } } -// issuerRef resolves what a leaf Certificate's spec.issuerRef should be for -// the given mode: -// - IssuerModeExternal: issuerConf's Name/Kind/Group verbatim (Group -// defaults to "cert-manager.io" when empty — matches cert-manager's own -// IssuerReference default). -// - IssuerModeManagedCluster: the cluster-scoped TLS ClusterIssuer named -// by issuerConf.Name (required by the CRD whenever issuerConf is set). -// - IssuerModeManagedNamespaced: the namespaced TLS Issuer, named by -// issuerConf.Name when set, otherwise the auto-generated name. func issuerRef(cluster *v1beta1.PostgresCluster, mode IssuerMode) cmmeta.IssuerReference { switch mode { case IssuerModeExternal: From 9d97033464a5df5f85e8ba1b976cc8d58c89c3c4 Mon Sep 17 00:00:00 2001 From: Mayank Shah Date: Wed, 15 Jul 2026 14:21:27 +0530 Subject: [PATCH 06/17] improve ResolveIssuerMode by checking labels Signed-off-by: Mayank Shah --- internal/naming/labels.go | 6 +++++- percona/certmanager/certmanager.go | 21 +++++++++++++++++++-- percona/certmanager/certmanager_test.go | 24 ++++++++++++++++++++++-- 3 files changed, 46 insertions(+), 5 deletions(-) diff --git a/internal/naming/labels.go b/internal/naming/labels.go index ad67a6bc9c..ef1fe0d135 100644 --- a/internal/naming/labels.go +++ b/internal/naming/labels.go @@ -22,6 +22,10 @@ const ( LabelPerconaName = appK8sPrefix + "name" LabelPerconaInstance = appK8sPrefix + "instance" + // LabelPerconaManagedByValue is the value this operator stamps onto + // LabelPerconaManagedBy. + LabelPerconaManagedByValue = "percona-postgresql-operator" + // LabelCluster et al. provides the fundamental labels for Postgres instances LabelCluster = labelPrefix + "cluster" LabelInstance = labelPrefix + "instance" @@ -319,7 +323,7 @@ func WithPerconaLabels(set map[string]string, clusterName, component, crVersion } ls := labels.Set{ - LabelPerconaManagedBy: "percona-postgresql-operator", + LabelPerconaManagedBy: LabelPerconaManagedByValue, LabelPerconaName: "percona-postgresql", LabelPerconaPartOf: "percona-postgresql", } diff --git a/percona/certmanager/certmanager.go b/percona/certmanager/certmanager.go index 1704c00ca9..0a337e8d61 100644 --- a/percona/certmanager/certmanager.go +++ b/percona/certmanager/certmanager.go @@ -89,9 +89,18 @@ func ResolveIssuerMode(ctx context.Context, cl client.Client, cluster *v1beta1.P existing := &v1.ClusterIssuer{} err := cl.Get(ctx, types.NamespacedName{Name: ic.Name}, existing) switch { - case err == nil, k8serrors.IsNotFound(err): + // ClusterIssuer not found, operator will create it + case k8serrors.IsNotFound(err): return IssuerModeManagedCluster, nil + case err == nil: + // ClusterIssuer found, check if the operator created it + if val, ok := existing.GetLabels()[naming.LabelPerconaManagedBy]; ok && val == naming.LabelPerconaManagedByValue { + return IssuerModeManagedCluster, nil + } + // Operator did not create it, it is managed externally + return IssuerModeExternal, nil case k8serrors.IsForbidden(err): + // Operator does not have permission, trust blindly that it exists and managed externally return IssuerModeExternal, nil default: return IssuerModeManagedNamespaced, errors.Wrap(err, "failed to get cluster issuer") @@ -201,7 +210,12 @@ func (c *controller) ApplyIssuer(ctx context.Context, cluster *v1beta1.PostgresC if mode == IssuerModeManagedCluster { caSecretName := naming.ClusterCACertSecret(cluster, CertManagerNamespace()).Name - meta := metav1.ObjectMeta{Name: issuerRef(cluster, mode).Name} + meta := metav1.ObjectMeta{ + Name: issuerRef(cluster, mode).Name, + Labels: map[string]string{ + naming.LabelPerconaManagedBy: naming.LabelPerconaManagedByValue, + }, + } existing := &v1.ClusterIssuer{} err := c.cl.Get(ctx, types.NamespacedName{Name: meta.Name}, existing) @@ -301,6 +315,9 @@ func (c *controller) ApplyCAIssuer(ctx context.Context, cluster *v1beta1.Postgre if mode == IssuerModeManagedCluster { meta := naming.ClusterCAIssuer(cluster) + meta.Labels = map[string]string{ + naming.LabelPerconaManagedBy: naming.LabelPerconaManagedByValue, + } existing := &v1.ClusterIssuer{} err := c.cl.Get(ctx, types.NamespacedName{Name: meta.Name}, existing) diff --git a/percona/certmanager/certmanager_test.go b/percona/certmanager/certmanager_test.go index 66ea2d7f04..ac980d99ab 100644 --- a/percona/certmanager/certmanager_test.go +++ b/percona/certmanager/certmanager_test.go @@ -252,6 +252,7 @@ func TestApplyIssuer(t *testing.T) { require.NotNil(t, issuer.Spec.CA) assert.Equal(t, naming.ClusterCACertSecret(cluster, CertManagerNamespace()).Name, issuer.Spec.CA.SecretName) assert.Empty(t, issuer.OwnerReferences) + assert.Equal(t, naming.LabelPerconaManagedByValue, issuer.Labels[naming.LabelPerconaManagedBy]) // idempotent err = ctrl.ApplyIssuer(t.Context(), cluster) @@ -343,6 +344,7 @@ func TestApplyCAIssuer(t *testing.T) { require.NoError(t, err) assert.NotNil(t, issuer.Spec.SelfSigned) assert.Empty(t, issuer.OwnerReferences) + assert.Equal(t, naming.LabelPerconaManagedByValue, issuer.Labels[naming.LabelPerconaManagedBy]) // idempotent err = ctrl.ApplyCAIssuer(t.Context(), cluster) @@ -1290,12 +1292,15 @@ func TestResolveIssuerMode(t *testing.T) { assert.Equal(t, IssuerModeManagedCluster, mode) }) - t.Run("Kind ClusterIssuer readable returns managed cluster", func(t *testing.T) { + t.Run("Kind ClusterIssuer readable and labeled as ours returns managed cluster", func(t *testing.T) { cluster := testCluster() cluster.Spec.TLS = &v1beta1.TLSSpec{ IssuerConf: &cmmeta.IssuerReference{Name: "shared-issuer", Kind: v1.ClusterIssuerKind}, } - existing := &v1.ClusterIssuer{ObjectMeta: metav1.ObjectMeta{Name: "shared-issuer"}} + existing := &v1.ClusterIssuer{ObjectMeta: metav1.ObjectMeta{ + Name: "shared-issuer", + Labels: map[string]string{naming.LabelPerconaManagedBy: naming.LabelPerconaManagedByValue}, + }} cl := setupFakeClient(t, cluster, existing) mode, err := ResolveIssuerMode(t.Context(), cl, cluster) @@ -1303,6 +1308,21 @@ func TestResolveIssuerMode(t *testing.T) { assert.Equal(t, IssuerModeManagedCluster, mode) }) + t.Run("Kind ClusterIssuer readable but not labeled as ours returns external", func(t *testing.T) { + cluster := testCluster() + cluster.Spec.TLS = &v1beta1.TLSSpec{ + IssuerConf: &cmmeta.IssuerReference{Name: "shared-issuer", Kind: v1.ClusterIssuerKind}, + } + // A pre-existing ClusterIssuer (e.g. ACME-backed) that this operator + // never created — no managed-by label. + existing := &v1.ClusterIssuer{ObjectMeta: metav1.ObjectMeta{Name: "shared-issuer"}} + cl := setupFakeClient(t, cluster, existing) + + mode, err := ResolveIssuerMode(t.Context(), cl, cluster) + require.NoError(t, err) + assert.Equal(t, IssuerModeExternal, mode) + }) + t.Run("Kind ClusterIssuer forbidden returns external", func(t *testing.T) { cluster := testCluster() cluster.Spec.TLS = &v1beta1.TLSSpec{ From 8c941e71e9dd1fb17d0dacb639a6c950df965be4 Mon Sep 17 00:00:00 2001 From: Mayank Shah Date: Wed, 15 Jul 2026 14:22:54 +0530 Subject: [PATCH 07/17] add groupName in issuerRef Signed-off-by: Mayank Shah --- percona/certmanager/certmanager.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/percona/certmanager/certmanager.go b/percona/certmanager/certmanager.go index 0a337e8d61..b901b7e70a 100644 --- a/percona/certmanager/certmanager.go +++ b/percona/certmanager/certmanager.go @@ -120,13 +120,13 @@ func issuerRef(cluster *v1beta1.PostgresCluster, mode IssuerMode) cmmeta.IssuerR } return cmmeta.IssuerReference{Name: ic.Name, Kind: ic.Kind, Group: group} case IssuerModeManagedCluster: - return cmmeta.IssuerReference{Name: issuerConf(cluster).Name, Kind: v1.ClusterIssuerKind} + return cmmeta.IssuerReference{Name: issuerConf(cluster).Name, Kind: v1.ClusterIssuerKind, Group: certmanager.GroupName} default: name := naming.TLSIssuer(cluster).Name if ic := issuerConf(cluster); ic != nil && ic.Name != "" { name = ic.Name } - return cmmeta.IssuerReference{Name: name, Kind: v1.IssuerKind} + return cmmeta.IssuerReference{Name: name, Kind: v1.IssuerKind, Group: certmanager.GroupName} } } From a19b87806458614ac01e3e5ff4ddf91930423015 Mon Sep 17 00:00:00 2001 From: Mayank Shah Date: Wed, 15 Jul 2026 14:24:24 +0530 Subject: [PATCH 08/17] remove unnecessary test Signed-off-by: Mayank Shah --- percona/runtime/runtime_test.go | 34 --------------------------------- 1 file changed, 34 deletions(-) delete mode 100644 percona/runtime/runtime_test.go diff --git a/percona/runtime/runtime_test.go b/percona/runtime/runtime_test.go deleted file mode 100644 index 2c863f4744..0000000000 --- a/percona/runtime/runtime_test.go +++ /dev/null @@ -1,34 +0,0 @@ -package runtime - -import ( - "testing" - - cmv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/client-go/rest" - "sigs.k8s.io/controller-runtime/pkg/manager" - - "github.com/percona/percona-postgresql-operator/v2/internal/feature" -) - -func TestCreateRuntimeManagerDisablesClusterIssuerCache(t *testing.T) { - t.Setenv("WATCH_NAMESPACE", "") - - scheme := runtime.NewScheme() - _ = cmv1.AddToScheme(scheme) - - opts := manager.Options{Scheme: scheme} - mgr, err := CreateRuntimeManager(&rest.Config{Host: "https://127.0.0.1:1"}, feature.NewGate(), opts) - require.NoError(t, err) - require.NotNil(t, mgr) -} - -func TestClientCacheOptionsDisablesClusterIssuer(t *testing.T) { - opts := ClientCacheOptions() - require.NotNil(t, opts) - require.Len(t, opts.DisableFor, 1) - _, ok := opts.DisableFor[0].(*cmv1.ClusterIssuer) - assert.True(t, ok) -} From c7f2e5bc6539663f855cba8c3748fde16443ae6e Mon Sep 17 00:00:00 2001 From: Mayank Shah Date: Wed, 15 Jul 2026 14:26:10 +0530 Subject: [PATCH 09/17] updated cr.yaml Signed-off-by: Mayank Shah --- deploy/cr.yaml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/deploy/cr.yaml b/deploy/cr.yaml index e6d71f7858..0484c82661 100644 --- a/deploy/cr.yaml +++ b/deploy/cr.yaml @@ -62,6 +62,10 @@ spec: # certValidityDuration: 2160h # caValidityDuration: 26280h # pgBackRestCertValidityDuration: 2160h +# issuerConf: +# name: some-selfsigned-issuer +# kind: ClusterIssuer +# group: cert-manager.io # standby: # enabled: true # host: "" From f544698e8feeb856ee30d9072a7c09124f9327d1 Mon Sep 17 00:00:00 2001 From: Mayank Shah Date: Wed, 15 Jul 2026 14:29:08 +0530 Subject: [PATCH 10/17] remove unnecessarily verbose comments Signed-off-by: Mayank Shah --- internal/controller/postgrescluster/instance.go | 5 ----- internal/controller/postgrescluster/pgbackrest.go | 7 ------- percona/runtime/runtime.go | 7 ------- 3 files changed, 19 deletions(-) diff --git a/internal/controller/postgrescluster/instance.go b/internal/controller/postgrescluster/instance.go index 1b7b7a33a5..6a4af57619 100644 --- a/internal/controller/postgrescluster/instance.go +++ b/internal/controller/postgrescluster/instance.go @@ -1602,11 +1602,6 @@ func (r *Reconciler) reconcileCertManagerInstanceCertificates( return instanceCerts, nil } -// instanceCACert returns the CA certificate to embed alongside an instance's -// leaf certificate. When rootCertificateAuth is set (the operator manages -// the CA itself), it's the source of truth. When it's nil (external issuer — -// see K8SPG-951), this reads the ca.crt cert-manager wrote into the -// instance's own just-issued secret. func instanceCACert(rootCertificateAuth *pki.RootCertificateAuthority, issuedSecret *corev1.Secret) (pki.Certificate, error) { if rootCertificateAuth != nil { return rootCertificateAuth.Certificate, nil diff --git a/internal/controller/postgrescluster/pgbackrest.go b/internal/controller/postgrescluster/pgbackrest.go index e5611ca49e..09c9a5fc1c 100644 --- a/internal/controller/postgrescluster/pgbackrest.go +++ b/internal/controller/postgrescluster/pgbackrest.go @@ -2335,13 +2335,6 @@ func (r *Reconciler) reconcileCertManagerPGBackRestSecret( return nil } -// pgBackRestCACert returns the CA certificate bytes to trust for pgBackRest's -// client/repo TLS. When rootCA is set (the operator manages the CA itself), -// it's the source of truth. When rootCA is nil (external issuer — see -// K8SPG-951), there is no operator-tracked CA; instead this reads the ca.crt -// cert-manager wrote into one of the just-issued leaf secrets, which is -// present as long as the external issuer returns CA data in its response -// (true for CA-backed issuers; not guaranteed for e.g. some ACME issuers). func pgBackRestCACert(rootCA *pki.RootCertificateAuthority, clientSecret, repoSecret *corev1.Secret) ([]byte, error) { if rootCA != nil { caCert, err := rootCA.Certificate.MarshalText() diff --git a/percona/runtime/runtime.go b/percona/runtime/runtime.go index 204748a370..0041172ad3 100644 --- a/percona/runtime/runtime.go +++ b/percona/runtime/runtime.go @@ -21,13 +21,6 @@ const refreshInterval time.Duration = 60 * time.Minute const ElectionID string = "08db3feb.percona.com" -// ClientCacheOptions returns the client.CacheOptions CreateRuntimeManager -// applies to every manager it builds. The operator does not request -// cluster-wide list/watch RBAC for clusterissuers.cert-manager.io by -// default (K8SPG-951's managed ClusterIssuer mode), so Get calls for -// ClusterIssuer must bypass the informer cache entirely and hit the API -// server directly, which only needs "get" on the one named object. -// Exported so it's directly unit-testable without constructing a manager. func ClientCacheOptions() *client.CacheOptions { return &client.CacheOptions{ DisableFor: []client.Object{&cmv1.ClusterIssuer{}}, From cbf395936d7f17333d1ca523cd1cdf4cfe91d2f4 Mon Sep 17 00:00:00 2001 From: Mayank Shah Date: Wed, 15 Jul 2026 14:45:41 +0530 Subject: [PATCH 11/17] update e2e test Signed-off-by: Mayank Shah --- .../24-verify-managed-cluster-issuer.yaml | 69 +++++++++++++++++++ ...-verify-tls-connection-cluster-issuer.yaml | 33 +++++++++ ...6-verify-tls-pgbouncer-cluster-issuer.yaml | 31 +++++++++ ...verify-tls-replication-cluster-issuer.yaml | 19 +++++ ...-verify-tls-pgbackrest-cluster-issuer.yaml | 36 ++++++++++ .../99-remove-cluster-gracefully.yaml | 6 ++ 6 files changed, 194 insertions(+) create mode 100644 e2e-tests/tests/cert-manager-tls/24-verify-managed-cluster-issuer.yaml create mode 100644 e2e-tests/tests/cert-manager-tls/25-verify-tls-connection-cluster-issuer.yaml create mode 100644 e2e-tests/tests/cert-manager-tls/26-verify-tls-pgbouncer-cluster-issuer.yaml create mode 100644 e2e-tests/tests/cert-manager-tls/27-verify-tls-replication-cluster-issuer.yaml create mode 100644 e2e-tests/tests/cert-manager-tls/28-verify-tls-pgbackrest-cluster-issuer.yaml diff --git a/e2e-tests/tests/cert-manager-tls/24-verify-managed-cluster-issuer.yaml b/e2e-tests/tests/cert-manager-tls/24-verify-managed-cluster-issuer.yaml new file mode 100644 index 0000000000..28062d7a5e --- /dev/null +++ b/e2e-tests/tests/cert-manager-tls/24-verify-managed-cluster-issuer.yaml @@ -0,0 +1,69 @@ +apiVersion: kuttl.dev/v1beta1 +kind: TestStep +commands: + - script: |- + set -o errexit + set -o xtrace + + source ../../functions + + # ClusterIssuers are cluster-scoped, so the name is qualified by the + # test's namespace to avoid collisions with other test runs sharing + # the same Kubernetes cluster. + issuer_name="${NAMESPACE}-shared-issuer" + + kubectl -n "$NAMESPACE" patch perconapgcluster cert-manager-tls \ + --type=merge \ + -p="{\"spec\":{\"tls\":{\"issuerConf\":{\"name\":\"${issuer_name}\",\"kind\":\"ClusterIssuer\"}}}}" + + wait_cluster_consistency cert-manager-tls + + ca_issuer_ready=$(kubectl get clusterissuer "${issuer_name}-ca-issuer" -o jsonpath='{.status.conditions[?(@.type=="Ready")].status}') + if [[ "$ca_issuer_ready" != "True" ]]; then + echo "Managed CA ClusterIssuer is not ready: $ca_issuer_ready" + exit 1 + fi + + ca_issuer_managed_by=$(kubectl get clusterissuer "${issuer_name}-ca-issuer" -o jsonpath='{.metadata.labels.app\.kubernetes\.io/managed-by}') + if [[ "$ca_issuer_managed_by" != "percona-postgresql-operator" ]]; then + echo "Managed CA ClusterIssuer is missing the operator's managed-by label, got: $ca_issuer_managed_by" + exit 1 + fi + + ca_cert_ready=$(kubectl -n cert-manager get certificate "${issuer_name}-ca-cert" -o jsonpath='{.status.conditions[?(@.type=="Ready")].status}') + if [[ "$ca_cert_ready" != "True" ]]; then + echo "Managed cluster CA certificate is not ready: $ca_cert_ready" + exit 1 + fi + + tls_issuer_ready=$(kubectl get clusterissuer "${issuer_name}" -o jsonpath='{.status.conditions[?(@.type=="Ready")].status}') + if [[ "$tls_issuer_ready" != "True" ]]; then + echo "Managed TLS ClusterIssuer is not ready: $tls_issuer_ready" + exit 1 + fi + + tls_issuer_managed_by=$(kubectl get clusterissuer "${issuer_name}" -o jsonpath='{.metadata.labels.app\.kubernetes\.io/managed-by}') + if [[ "$tls_issuer_managed_by" != "percona-postgresql-operator" ]]; then + echo "Managed TLS ClusterIssuer is missing the operator's managed-by label, got: $tls_issuer_managed_by" + exit 1 + fi + + tls_issuer_secret=$(kubectl get clusterissuer "${issuer_name}" -o jsonpath='{.spec.ca.secretName}') + if [[ "$tls_issuer_secret" != "${issuer_name}-ca-cert" ]]; then + echo "Managed TLS ClusterIssuer references the wrong CA secret, got: $tls_issuer_secret" + exit 1 + fi + + cluster_cert_issuer_name=$(kubectl -n "$NAMESPACE" get certificate cert-manager-tls-cluster-cert -o jsonpath='{.spec.issuerRef.name}') + cluster_cert_issuer_kind=$(kubectl -n "$NAMESPACE" get certificate cert-manager-tls-cluster-cert -o jsonpath='{.spec.issuerRef.kind}') + if [[ "$cluster_cert_issuer_name" != "$issuer_name" || "$cluster_cert_issuer_kind" != "ClusterIssuer" ]]; then + echo "Cluster certificate does not reference the managed ClusterIssuer, got name=$cluster_cert_issuer_name kind=$cluster_cert_issuer_kind" + exit 1 + fi + + cluster_cert_ready=$(kubectl -n "$NAMESPACE" get certificate cert-manager-tls-cluster-cert -o jsonpath='{.status.conditions[?(@.type=="Ready")].status}') + if [[ "$cluster_cert_ready" != "True" ]]; then + echo "Cluster certificate is not ready after switching to the managed ClusterIssuer: $cluster_cert_ready" + exit 1 + fi + timeout: 300 diff --git a/e2e-tests/tests/cert-manager-tls/25-verify-tls-connection-cluster-issuer.yaml b/e2e-tests/tests/cert-manager-tls/25-verify-tls-connection-cluster-issuer.yaml new file mode 100644 index 0000000000..8823376d10 --- /dev/null +++ b/e2e-tests/tests/cert-manager-tls/25-verify-tls-connection-cluster-issuer.yaml @@ -0,0 +1,33 @@ +apiVersion: kuttl.dev/v1beta1 +kind: TestStep +commands: + - script: |- + set -o errexit + set -o xtrace + + source ../../functions + + pg_certificate_data=$(run_comand_on_pod "openssl s_client -connect cert-manager-tls-primary:5432 -starttls postgres <<< '' 2>/dev/null | openssl x509 -noout -subject -issuer -dates") + + echo "PostgreSQL certificate data: $pg_certificate_data" + + if [[ "$pg_certificate_data" != *"subject=CN=cert-manager-tls-postgres"* ]]; then + echo "PostgreSQL certificate CN does not match expected value" + echo "Expected subject to contain: CN=cert-manager-tls-postgres" + exit 1 + fi + + if [[ "$pg_certificate_data" != *"issuer=CN=cert-manager-tls-ca"* ]]; then + echo "PostgreSQL certificate issuer does not match expected value" + echo "Expected issuer to contain: CN=cert-manager-tls-ca" + exit 1 + fi + + ssl_info=$(run_psql_local "SHOW ssl;" "postgres:$(get_psql_user_pass cert-manager-tls-pguser-postgres)@$(get_psql_user_host cert-manager-tls-pguser-postgres)") + echo "SSL status: $ssl_info" + + if [[ "$ssl_info" != *"on"* ]]; then + echo "SSL is not enabled on PostgreSQL after switching to the managed ClusterIssuer" + exit 1 + fi + timeout: 30 diff --git a/e2e-tests/tests/cert-manager-tls/26-verify-tls-pgbouncer-cluster-issuer.yaml b/e2e-tests/tests/cert-manager-tls/26-verify-tls-pgbouncer-cluster-issuer.yaml new file mode 100644 index 0000000000..af344ba8d6 --- /dev/null +++ b/e2e-tests/tests/cert-manager-tls/26-verify-tls-pgbouncer-cluster-issuer.yaml @@ -0,0 +1,31 @@ +apiVersion: kuttl.dev/v1beta1 +kind: TestStep +commands: + - script: |- + set -o errexit + set -o xtrace + + source ../../functions + + pgb_certificate_data=$(run_comand_on_pod "openssl s_client -connect cert-manager-tls-pgbouncer:5432 -starttls postgres <<< '' 2>/dev/null | openssl x509 -noout -subject -issuer") + + echo "PgBouncer certificate data: $pgb_certificate_data" + + if [[ -z "$pgb_certificate_data" ]]; then + echo "Failed to retrieve PgBouncer TLS certificate" + exit 1 + fi + + if [[ "$pgb_certificate_data" != *"issuer=CN=cert-manager-tls-ca"* ]]; then + echo "Unexpected PgBouncer certificate issuer. Expected CN=cert-manager-tls-ca" + echo "Got: $pgb_certificate_data" + exit 1 + fi + + pgb_ssl=$(run_psql_local "SELECT ssl FROM pg_stat_ssl WHERE pid = pg_backend_pid();" "cert-manager-tls:$(get_psql_user_pass cert-manager-tls-pguser-cert-manager-tls)@cert-manager-tls-pgbouncer/postgres") + + if [[ "$pgb_ssl" != *"t"* ]]; then + echo "PgBouncer-to-PostgreSQL connection is not using SSL after switching to the managed ClusterIssuer" + exit 1 + fi + timeout: 30 diff --git a/e2e-tests/tests/cert-manager-tls/27-verify-tls-replication-cluster-issuer.yaml b/e2e-tests/tests/cert-manager-tls/27-verify-tls-replication-cluster-issuer.yaml new file mode 100644 index 0000000000..56e8a90314 --- /dev/null +++ b/e2e-tests/tests/cert-manager-tls/27-verify-tls-replication-cluster-issuer.yaml @@ -0,0 +1,19 @@ +apiVersion: kuttl.dev/v1beta1 +kind: TestStep +commands: + - script: |- + set -o errexit + set -o xtrace + + source ../../functions + + repl_ssl_count=$(run_psql_local \ + "SELECT count(*) FROM pg_stat_ssl s JOIN pg_stat_replication r ON s.pid = r.pid WHERE s.ssl = true;" \ + "postgres:$(get_psql_user_pass cert-manager-tls-pguser-postgres)@cert-manager-tls-primary") + repl_ssl_count=$(echo "$repl_ssl_count" | tr -d '[:space:]') + + if [[ "$repl_ssl_count" -lt 1 ]]; then + echo "No SSL replication connections found after switching to the managed ClusterIssuer, got: $repl_ssl_count" + exit 1 + fi + timeout: 30 diff --git a/e2e-tests/tests/cert-manager-tls/28-verify-tls-pgbackrest-cluster-issuer.yaml b/e2e-tests/tests/cert-manager-tls/28-verify-tls-pgbackrest-cluster-issuer.yaml new file mode 100644 index 0000000000..43db32b66a --- /dev/null +++ b/e2e-tests/tests/cert-manager-tls/28-verify-tls-pgbackrest-cluster-issuer.yaml @@ -0,0 +1,36 @@ +apiVersion: kuttl.dev/v1beta1 +kind: TestStep +commands: + - script: |- + set -o errexit + set -o xtrace + + source ../../functions + + instance=$(kubectl -n "$NAMESPACE" get pod \ + -l postgres-operator.crunchydata.com/cluster=cert-manager-tls,postgres-operator.crunchydata.com/role=primary \ + -o jsonpath='{.items[0].metadata.name}') + + kubectl -n "$NAMESPACE" exec "$instance" -c pgbackrest -- \ + test -f /etc/pgbackrest/server/server-tls.crt + kubectl -n "$NAMESPACE" exec "$instance" -c pgbackrest -- \ + test -f /etc/pgbackrest/server/server-tls.key + kubectl -n "$NAMESPACE" exec "$instance" -c pgbackrest -- \ + test -f /etc/pgbackrest/conf.d/~postgres-operator/tls-ca.crt + + pgbr_certificate_data=$(run_comand_on_pod "openssl s_client -connect ${instance}.cert-manager-tls-pods:8432 <<< '' 2>/dev/null | openssl x509 -noout -subject -issuer") + + if [[ -z "$pgbr_certificate_data" ]]; then + echo "Failed to retrieve pgBackRest TLS certificate" + exit 1 + fi + + if [[ "$pgbr_certificate_data" != *"issuer=CN=cert-manager-tls-ca"* ]]; then + echo "Unexpected pgBackRest certificate issuer. Expected CN=cert-manager-tls-ca" + echo "Got: $pgbr_certificate_data" + exit 1 + fi + + # Verify pgBackRest works over TLS, not just cert file existence + kubectl -n "$NAMESPACE" exec "$instance" -c pgbackrest -- pgbackrest info + timeout: 30 diff --git a/e2e-tests/tests/cert-manager-tls/99-remove-cluster-gracefully.yaml b/e2e-tests/tests/cert-manager-tls/99-remove-cluster-gracefully.yaml index dbc10adc6d..b0dfd3baca 100644 --- a/e2e-tests/tests/cert-manager-tls/99-remove-cluster-gracefully.yaml +++ b/e2e-tests/tests/cert-manager-tls/99-remove-cluster-gracefully.yaml @@ -19,5 +19,11 @@ commands: remove_all_finalizers check_operator_panic destroy_operator + + # Cluster-scoped, so not owned by (and not garbage-collected with) the + # namespaced PostgresCluster/PerconaPGCluster deleted above. + issuer_name="${NAMESPACE}-shared-issuer" + kubectl delete clusterissuer "$issuer_name" "${issuer_name}-ca-issuer" --ignore-not-found=true + destroy_cert_manager timeout: 60 \ No newline at end of file From d123e94d6d9042f96702245aa7c2601d326d7805 Mon Sep 17 00:00:00 2001 From: Mayank Shah Date: Wed, 15 Jul 2026 15:00:47 +0530 Subject: [PATCH 12/17] linting Signed-off-by: Mayank Shah --- percona/certmanager/certmanager_test.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/percona/certmanager/certmanager_test.go b/percona/certmanager/certmanager_test.go index ac980d99ab..47f09fd20a 100644 --- a/percona/certmanager/certmanager_test.go +++ b/percona/certmanager/certmanager_test.go @@ -6,9 +6,11 @@ import ( "testing" "time" + "github.com/cert-manager/cert-manager/pkg/apis/certmanager" v1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" cmmeta "github.com/cert-manager/cert-manager/pkg/apis/meta/v1" "github.com/cert-manager/cert-manager/pkg/util/cmapichecker" + "github.com/pkg/errors" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" @@ -231,7 +233,7 @@ func TestApplyIssuer(t *testing.T) { list := &v1.IssuerList{} require.NoError(t, client.List(t.Context(), list)) - assert.Len(t, list.Items, 0) + assert.Empty(t, list.Items) }) t.Run("create cluster-scoped TLS issuer when Kind is ClusterIssuer", func(t *testing.T) { @@ -323,7 +325,7 @@ func TestApplyCAIssuer(t *testing.T) { list := &v1.IssuerList{} require.NoError(t, client.List(t.Context(), list)) - assert.Len(t, list.Items, 0) + assert.Empty(t, list.Items) }) t.Run("create cluster-scoped CA issuer when Kind is ClusterIssuer", func(t *testing.T) { @@ -410,7 +412,7 @@ func TestApplyCACertificate(t *testing.T) { list := &v1.CertificateList{} require.NoError(t, client.List(t.Context(), list)) - assert.Len(t, list.Items, 0) + assert.Empty(t, list.Items) }) t.Run("places CA certificate in cert-manager namespace when Kind is ClusterIssuer", func(t *testing.T) { @@ -1231,7 +1233,7 @@ func (f *forbiddenGetClient) Get(ctx context.Context, key sigs.ObjectKey, obj si if _, ok := obj.(*v1.ClusterIssuer); ok { return k8serrors.NewForbidden( schema.GroupResource{Group: "cert-manager.io", Resource: "clusterissuers"}, - key.Name, fmt.Errorf("forbidden")) + key.Name, errors.New("forbidden")) } return f.Client.Get(ctx, key, obj, opts...) } @@ -1354,7 +1356,7 @@ func TestIssuerRef(t *testing.T) { ref := issuerRef(cluster, IssuerModeManagedNamespaced) assert.Equal(t, naming.TLSIssuer(cluster).Name, ref.Name) assert.Equal(t, v1.IssuerKind, ref.Kind) - assert.Equal(t, "", ref.Group) + assert.Equal(t, certmanager.GroupName, ref.Group) }) t.Run("managed namespaced with issuerConf name override", func(t *testing.T) { From 4e0ff68ab40e7352bcf0f664fa7d1cbcea174531 Mon Sep 17 00:00:00 2001 From: Mayank Shah Date: Wed, 15 Jul 2026 16:06:29 +0530 Subject: [PATCH 13/17] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- internal/naming/names.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/naming/names.go b/internal/naming/names.go index 755b23d929..651111f207 100644 --- a/internal/naming/names.go +++ b/internal/naming/names.go @@ -666,7 +666,7 @@ func ClusterCAIssuer(cluster *v1beta1.PostgresCluster) metav1.ObjectMeta { } // K8SPG-951 -// ClusterTLSIssuer returns the ObjectMeta for the cluster-scoped TLS ClusterIssuer used by cert-manager. +// ClusterCACertSecret returns the ObjectMeta for the cluster-scoped CA Secret in cert-manager's namespace. func ClusterCACertSecret(cluster *v1beta1.PostgresCluster, certManagerNamespace string) metav1.ObjectMeta { return metav1.ObjectMeta{ Namespace: certManagerNamespace, From 63b3e41ff3e61900a1fcfb43d1dbb3dcdfc2406a Mon Sep 17 00:00:00 2001 From: Mayank Shah Date: Thu, 16 Jul 2026 11:03:56 +0530 Subject: [PATCH 14/17] fix scheme ordering Signed-off-by: Mayank Shah --- cmd/postgres-operator/main.go | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/cmd/postgres-operator/main.go b/cmd/postgres-operator/main.go index f61123c4b1..70a07b6927 100644 --- a/cmd/postgres-operator/main.go +++ b/cmd/postgres-operator/main.go @@ -124,15 +124,6 @@ func main() { ) assertNoError(err) - // Add Percona custom resource types to scheme - assertNoError(v2.AddToScheme(mgr.GetScheme())) - - assertNoError(volumesnapshotv1.AddToScheme(mgr.GetScheme())) - - // K8SPG-552 - // Add Scheme for cert-manager resources like Issuer and Certificate. - assertNoError(certmanagerscheme.AddToScheme(mgr.GetScheme())) - // add all PostgreSQL Operator controllers to the runtime manager err = addControllersToManager(ctx, mgr) assertNoError(err) @@ -355,6 +346,13 @@ func initManager(ctx context.Context) (runtime.Options, error) { } } + // add scheme + scheme := runtime.Scheme + assertNoError(v2.AddToScheme(scheme)) + assertNoError(volumesnapshotv1.AddToScheme(scheme)) + assertNoError(certmanagerscheme.AddToScheme(scheme)) + options.Scheme = scheme + return options, nil } From 491a4d016ea0d72f98b98b73297b1e684e1c62c6 Mon Sep 17 00:00:00 2001 From: Mayank Shah Date: Thu, 16 Jul 2026 15:24:24 +0530 Subject: [PATCH 15/17] unit test fix Signed-off-by: Mayank Shah --- cmd/postgres-operator/main_test.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/cmd/postgres-operator/main_test.go b/cmd/postgres-operator/main_test.go index 4e04c4af2a..690b3db2a8 100644 --- a/cmd/postgres-operator/main_test.go +++ b/cmd/postgres-operator/main_test.go @@ -11,6 +11,8 @@ import ( "gotest.tools/v3/assert" "gotest.tools/v3/assert/cmp" + + "github.com/percona/percona-postgresql-operator/v2/internal/controller/runtime" ) func TestInitManager(t *testing.T) { @@ -42,6 +44,9 @@ func TestInitManager(t *testing.T) { assert.Assert(t, options.RenewDeadline.Seconds() == 40) assert.Assert(t, options.RetryPeriod.Seconds() == 10) + assert.Assert(t, options.Scheme == runtime.Scheme, + "expected the shared scheme to be configured before manager creation") + { options.Cache.SyncPeriod = nil options.Controller.GroupKindConcurrency = nil @@ -51,6 +56,7 @@ func TestInitManager(t *testing.T) { options.LeaseDuration = nil options.RenewDeadline = nil options.RetryPeriod = nil + options.Scheme = nil assert.Assert(t, reflect.ValueOf(options).IsZero(), "expected remaining fields to be unset:\n%+v", options) From f2dc25092bade1103fa0a0f052d0feb9c8e9f566 Mon Sep 17 00:00:00 2001 From: Mayank Shah Date: Fri, 17 Jul 2026 16:36:11 +0530 Subject: [PATCH 16/17] fix panic Signed-off-by: Mayank Shah --- internal/pgbouncer/reconcile.go | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/internal/pgbouncer/reconcile.go b/internal/pgbouncer/reconcile.go index 4a0582b364..6f707741f0 100644 --- a/internal/pgbouncer/reconcile.go +++ b/internal/pgbouncer/reconcile.go @@ -85,12 +85,14 @@ func Secret(ctx context.Context, if inCluster.Spec.Proxy.PGBouncer.CustomTLSSecret == nil { if frontendCertManagerSecret != nil { if err == nil { - outSecret.Data[certFrontendAuthoritySecretKey], err = inRoot.Certificate.MarshalText() + outSecret.Data[certFrontendAuthoritySecretKey], err = frontendAuthorityCert(inRoot, frontendCertManagerSecret) } if err == nil { outSecret.Data[certFrontendSecretKey] = frontendCertManagerSecret.Data[corev1.TLSCertKey] outSecret.Data[certFrontendPrivateKeySecretKey] = frontendCertManagerSecret.Data[corev1.TLSPrivateKeyKey] } + } else if inRoot == nil { + err = errors.New("waiting for cert-manager to issue pgbouncer frontend certificate") } else { leaf := &pki.LeafCertificate{} var dnsNames []string @@ -130,6 +132,20 @@ func Secret(ctx context.Context, return err } +// frontendAuthorityCert returns the CA certificate bytes to trust for the +// PgBouncer frontend certificate. It prefers the internal PKI root, which is +// nil when an external cert-manager issuer is in use; in that case, it falls +// back to the "ca.crt" that cert-manager writes into the frontend Secret. +func frontendAuthorityCert(inRoot *pki.RootCertificateAuthority, frontendCertManagerSecret *corev1.Secret) ([]byte, error) { + if inRoot != nil { + return inRoot.Certificate.MarshalText() + } + if ca := frontendCertManagerSecret.Data[tlsAuthoritySecretKey]; len(ca) > 0 { + return ca, nil + } + return nil, errors.New("external issuer did not return a CA certificate for pgbouncer frontend") +} + // Pod populates a PodSpec with the container and volumes needed to run PgBouncer. func Pod( ctx context.Context, From ec13aa13e2ceebf8e7e1bf634d6251ef465091f2 Mon Sep 17 00:00:00 2001 From: Mayank Shah Date: Fri, 17 Jul 2026 16:55:10 +0530 Subject: [PATCH 17/17] e2e test fixes Signed-off-by: Mayank Shah --- e2e-tests/functions | 5 ++ .../05-deploy-cert-manager.yaml | 2 +- .../24-verify-external-cluster-issuer.yaml | 88 +++++++++++++++++++ .../24-verify-managed-cluster-issuer.yaml | 69 --------------- ...-verify-tls-connection-cluster-issuer.yaml | 2 +- ...6-verify-tls-pgbouncer-cluster-issuer.yaml | 2 +- ...verify-tls-replication-cluster-issuer.yaml | 2 +- .../99-remove-cluster-gracefully.yaml | 9 +- 8 files changed, 103 insertions(+), 76 deletions(-) create mode 100644 e2e-tests/tests/cert-manager-tls/24-verify-external-cluster-issuer.yaml delete mode 100644 e2e-tests/tests/cert-manager-tls/24-verify-managed-cluster-issuer.yaml diff --git a/e2e-tests/functions b/e2e-tests/functions index d84e5cb8a7..febfb71c00 100644 --- a/e2e-tests/functions +++ b/e2e-tests/functions @@ -1175,6 +1175,11 @@ deploy_cert_manager() { sleep 5 done + echo "Waiting for cert-manager mutating webhook to be ready..." + until kubectl get mutatingwebhookconfiguration cert-manager-webhook -o jsonpath='{.webhooks[0].clientConfig.caBundle}' | grep -q '[A-Za-z0-9+/=]'; do + sleep 5 + done + echo "Waiting for cert-manager webhook service to have endpoints..." until kubectl -n cert-manager get endpoints cert-manager-webhook -o jsonpath='{.subsets[*].addresses}' | grep -q '.'; do sleep 5 diff --git a/e2e-tests/tests/cert-manager-tls/05-deploy-cert-manager.yaml b/e2e-tests/tests/cert-manager-tls/05-deploy-cert-manager.yaml index 716800c0cc..abf42b9a58 100644 --- a/e2e-tests/tests/cert-manager-tls/05-deploy-cert-manager.yaml +++ b/e2e-tests/tests/cert-manager-tls/05-deploy-cert-manager.yaml @@ -13,4 +13,4 @@ commands: kubectl -n "$NAMESPACE" delete pod -l postgres-operator.crunchydata.com/role=pgbouncer,postgres-operator.crunchydata.com/cluster=cert-manager-tls wait_cluster_consistency cert-manager-tls - timeout: 120 + timeout: 240 diff --git a/e2e-tests/tests/cert-manager-tls/24-verify-external-cluster-issuer.yaml b/e2e-tests/tests/cert-manager-tls/24-verify-external-cluster-issuer.yaml new file mode 100644 index 0000000000..5d91ebb26f --- /dev/null +++ b/e2e-tests/tests/cert-manager-tls/24-verify-external-cluster-issuer.yaml @@ -0,0 +1,88 @@ +apiVersion: kuttl.dev/v1beta1 +kind: TestStep +commands: + - script: |- + set -o errexit + set -o xtrace + + source ../../functions + + # ClusterIssuers are cluster-scoped, so the name is qualified by the + # test's namespace to avoid collisions with other test runs sharing + # the same Kubernetes cluster. + # + # This ClusterIssuer is created here by the test itself, the way a + # cluster admin managing their own PKI would - NOT by the operator. + # It exercises the "external" issuer mode, where the operator only + # references a pre-existing ClusterIssuer it does not own or manage. + issuer_name="${NAMESPACE}-shared-issuer" + bootstrap_issuer_name="${issuer_name}-bootstrap" + ca_cert_name="${issuer_name}-ca-cert" + + kubectl apply -f - <