From a8bb8c35b3af48f715d8168096fe739a686b3c92 Mon Sep 17 00:00:00 2001 From: George Kechagias Date: Thu, 11 Dec 2025 14:21:46 +0200 Subject: [PATCH 1/3] K8SPSMDB-1413 skip setting controller owner on secrets owned by Certificates created by cert-manager --- pkg/psmdb/tls/certmanager.go | 5 ++ pkg/psmdb/tls/certmanager_test.go | 78 +++++++++++++++++++++++++++++++ 2 files changed, 83 insertions(+) diff --git a/pkg/psmdb/tls/certmanager.go b/pkg/psmdb/tls/certmanager.go index aa14cd746c..2f69b3d513 100644 --- a/pkg/psmdb/tls/certmanager.go +++ b/pkg/psmdb/tls/certmanager.go @@ -313,6 +313,11 @@ func (c *certManagerController) WaitForCerts(ctx context.Context, cr *api.Percon if v, ok := secret.Annotations[cm.CertificateNameKey]; !ok || v != secret.Name { continue } + // cert-manager sets the Certificate as the controller owner. + // In that case, the operator should not set a new controller reference. + if metav1.GetControllerOf(secret) != nil { + continue + } if err = controllerutil.SetControllerReference(cr, secret, c.scheme); err != nil { return errors.Wrap(err, "set controller reference") } diff --git a/pkg/psmdb/tls/certmanager_test.go b/pkg/psmdb/tls/certmanager_test.go index 18fec27ad8..1cf81c88f3 100644 --- a/pkg/psmdb/tls/certmanager_test.go +++ b/pkg/psmdb/tls/certmanager_test.go @@ -6,6 +6,9 @@ import ( cm "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" cmmeta "github.com/cert-manager/cert-manager/pkg/apis/meta/v1" + "github.com/percona/percona-server-mongodb-operator/pkg/version" + "github.com/stretchr/testify/assert" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/kubernetes/scheme" @@ -130,6 +133,81 @@ func TestCreateCertificate(t *testing.T) { }) } +func TestWaitForCertsWithCertManagerManagedSecret(t *testing.T) { + ctx := context.Background() + + cr := &api.PerconaServerMongoDB{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster", + Namespace: "default", + UID: "test-uid-123", + }, + Spec: api.PerconaServerMongoDBSpec{ + CRVersion: version.Version(), + }, + } + + certName := CACertificateSecretName(cr) + certificate := &cm.Certificate{ + ObjectMeta: metav1.ObjectMeta{ + Name: certName, + Namespace: cr.Namespace, + UID: "cert-uid-456", + }, + Spec: cm.CertificateSpec{ + SecretName: certName, + }, + } + + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: certName, + Namespace: cr.Namespace, + Annotations: map[string]string{ + cm.CertificateNameKey: certName, + }, + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: cm.SchemeGroupVersion.String(), + Kind: cm.CertificateKind, + Name: certificate.Name, + UID: certificate.UID, + Controller: pointer(true), + }, + }, + }, + Data: map[string][]byte{ + "ca.crt": []byte("fake-ca-cert"), + "tls.crt": []byte("fake-tls-cert"), + "tls.key": []byte("fake-tls-key"), + }, + } + + s := scheme.Scheme + s.AddKnownTypes(api.SchemeGroupVersion, new(api.PerconaServerMongoDB)) + s.AddKnownTypes(cm.SchemeGroupVersion, new(cm.Certificate)) + s.AddKnownTypes(corev1.SchemeGroupVersion, new(corev1.Secret)) + + cl := fake.NewClientBuilder(). + WithScheme(s). + WithObjects(cr, certificate, secret). + WithStatusSubresource(cr). + Build() + + controller := &certManagerController{ + cl: cl, + scheme: s, + dryRun: false, + } + + err := controller.WaitForCerts(ctx, cr, certName) + assert.NoError(t, err) +} + +func pointer(b bool) *bool { + return &b +} + // creates a fake client to mock API calls with the mock objects func buildFakeClient(objs ...client.Object) CertManagerController { s := scheme.Scheme From 5796c4d6ea1eb4bd20798db98c2473a735ac0280 Mon Sep 17 00:00:00 2001 From: George Kechagias Date: Thu, 11 Dec 2025 15:26:27 +0200 Subject: [PATCH 2/3] improve unit test with more scenarios --- pkg/psmdb/tls/certmanager_test.go | 148 ++++++++++++++++++++---------- 1 file changed, 102 insertions(+), 46 deletions(-) diff --git a/pkg/psmdb/tls/certmanager_test.go b/pkg/psmdb/tls/certmanager_test.go index 1cf81c88f3..0e9c61b5c2 100644 --- a/pkg/psmdb/tls/certmanager_test.go +++ b/pkg/psmdb/tls/certmanager_test.go @@ -133,7 +133,7 @@ func TestCreateCertificate(t *testing.T) { }) } -func TestWaitForCertsWithCertManagerManagedSecret(t *testing.T) { +func TestWaitForCerts(t *testing.T) { ctx := context.Background() cr := &api.PerconaServerMongoDB{ @@ -148,60 +148,116 @@ func TestWaitForCertsWithCertManagerManagedSecret(t *testing.T) { } certName := CACertificateSecretName(cr) - certificate := &cm.Certificate{ - ObjectMeta: metav1.ObjectMeta{ - Name: certName, - Namespace: cr.Namespace, - UID: "cert-uid-456", - }, - Spec: cm.CertificateSpec{ - SecretName: certName, - }, - } - secret := &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: certName, - Namespace: cr.Namespace, - Annotations: map[string]string{ - cm.CertificateNameKey: certName, + tests := map[string]struct { + certificate *cm.Certificate + secret *corev1.Secret + }{ + "with cert-manager managed secret": { + certificate: &cm.Certificate{ + ObjectMeta: metav1.ObjectMeta{ + Name: certName, + Namespace: cr.Namespace, + UID: "cert-uid-456", + }, + Spec: cm.CertificateSpec{ + SecretName: certName, + }, + }, + secret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: certName, + Namespace: cr.Namespace, + Annotations: map[string]string{ + cm.CertificateNameKey: certName, + }, + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: cm.SchemeGroupVersion.String(), + Kind: cm.CertificateKind, + Name: certName, + UID: "cert-uid-456", + Controller: pointer(true), + }, + }, + }, + Data: map[string][]byte{ + "ca.crt": []byte("fake-ca-cert"), + "tls.crt": []byte("fake-tls-cert"), + "tls.key": []byte("fake-tls-key"), + }, + }, + }, + "with cert-manager managed secret but without OwnerReferences": { + certificate: &cm.Certificate{ + ObjectMeta: metav1.ObjectMeta{ + Name: certName, + Namespace: cr.Namespace, + UID: "cert-uid-456", + }, + Spec: cm.CertificateSpec{ + SecretName: certName, + }, }, - OwnerReferences: []metav1.OwnerReference{ - { - APIVersion: cm.SchemeGroupVersion.String(), - Kind: cm.CertificateKind, - Name: certificate.Name, - UID: certificate.UID, - Controller: pointer(true), + secret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: certName, + Namespace: cr.Namespace, + Annotations: map[string]string{ + cm.CertificateNameKey: certName, + }, + }, + Data: map[string][]byte{ + "ca.crt": []byte("fake-ca-cert"), + "tls.crt": []byte("fake-tls-cert"), + "tls.key": []byte("fake-tls-key"), }, }, }, - Data: map[string][]byte{ - "ca.crt": []byte("fake-ca-cert"), - "tls.crt": []byte("fake-tls-cert"), - "tls.key": []byte("fake-tls-key"), + "without cert-manager": { + certificate: nil, + secret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: certName, + Namespace: cr.Namespace, + }, + Data: map[string][]byte{ + "ca.crt": []byte("fake-ca-cert"), + "tls.crt": []byte("fake-tls-cert"), + "tls.key": []byte("fake-tls-key"), + }, + }, }, } - s := scheme.Scheme - s.AddKnownTypes(api.SchemeGroupVersion, new(api.PerconaServerMongoDB)) - s.AddKnownTypes(cm.SchemeGroupVersion, new(cm.Certificate)) - s.AddKnownTypes(corev1.SchemeGroupVersion, new(corev1.Secret)) - - cl := fake.NewClientBuilder(). - WithScheme(s). - WithObjects(cr, certificate, secret). - WithStatusSubresource(cr). - Build() - - controller := &certManagerController{ - cl: cl, - scheme: s, - dryRun: false, + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + s := scheme.Scheme + s.AddKnownTypes(api.SchemeGroupVersion, new(api.PerconaServerMongoDB)) + s.AddKnownTypes(cm.SchemeGroupVersion, new(cm.Certificate)) + s.AddKnownTypes(corev1.SchemeGroupVersion, new(corev1.Secret)) + + objects := []client.Object{cr, tc.secret} + if tc.certificate != nil { + objects = append(objects, tc.certificate) + } + + cl := fake.NewClientBuilder(). + WithScheme(s). + WithObjects(objects...). + WithStatusSubresource(cr). + Build() + + controller := &certManagerController{ + cl: cl, + scheme: s, + dryRun: false, + } + + err := controller.WaitForCerts(ctx, cr, certName) + assert.NoError(t, err) + }) } - - err := controller.WaitForCerts(ctx, cr, certName) - assert.NoError(t, err) } func pointer(b bool) *bool { From d86cf1f649e1c27081add809463fb7790fa8cda7 Mon Sep 17 00:00:00 2001 From: George Kechagias Date: Fri, 12 Dec 2025 12:49:34 +0200 Subject: [PATCH 3/3] fix imports --- pkg/psmdb/tls/certmanager_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/psmdb/tls/certmanager_test.go b/pkg/psmdb/tls/certmanager_test.go index 0e9c61b5c2..a493fbd22f 100644 --- a/pkg/psmdb/tls/certmanager_test.go +++ b/pkg/psmdb/tls/certmanager_test.go @@ -6,7 +6,6 @@ import ( cm "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" cmmeta "github.com/cert-manager/cert-manager/pkg/apis/meta/v1" - "github.com/percona/percona-server-mongodb-operator/pkg/version" "github.com/stretchr/testify/assert" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -16,6 +15,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client/fake" // nolint api "github.com/percona/percona-server-mongodb-operator/pkg/apis/psmdb/v1" + "github.com/percona/percona-server-mongodb-operator/pkg/version" ) func TestCreateIssuer(t *testing.T) {