From 0e992a9e3f738970e359bc210640f59c550d5bfc Mon Sep 17 00:00:00 2001 From: Madhu Rajanna Date: Thu, 26 Jun 2025 15:47:32 +0200 Subject: [PATCH 1/2] core: provide option to specify secret name When cephClient CR is created by User, the Rook is creating an User and creating an secret with its own predefined name + CR name, With this we are allowing user to specify the secretName Signed-off-by: Madhu Rajanna (cherry picked from commit d6d2e604fcf1a0df57869a512f77d1a0bbb6c8df) --- Documentation/CRDs/specification.md | 52 +++++++++++++++++++ .../charts/rook-ceph/templates/resources.yaml | 13 +++++ deploy/examples/crds.yaml | 13 +++++ pkg/apis/ceph.rook.io/v1/types.go | 10 ++++ pkg/operator/ceph/client/controller.go | 3 ++ pkg/operator/ceph/client/controller_test.go | 18 ++++++- 6 files changed, 108 insertions(+), 1 deletion(-) diff --git a/Documentation/CRDs/specification.md b/Documentation/CRDs/specification.md index 1aa4888d5..ed9c73bb6 100644 --- a/Documentation/CRDs/specification.md +++ b/Documentation/CRDs/specification.md @@ -592,6 +592,32 @@ string +secretName
+ +string + + + +(Optional) +

SecretName is the name of the secret created for this ceph client. +If not specified, the default name is “rook-ceph-client-” as a prefix to the CR name.

+ + + + +removeSecret
+ +bool + + + +(Optional) +

RemoveSecret indicates whether the current secret for this ceph client should be removed or not. +If true, the K8s secret will be deleted, but the cephx keyring will remain until the CR is deleted.

+ + + + caps
map[string]string @@ -4556,6 +4582,32 @@ string +secretName
+ +string + + + +(Optional) +

SecretName is the name of the secret created for this ceph client. +If not specified, the default name is “rook-ceph-client-” as a prefix to the CR name.

+ + + + +removeSecret
+ +bool + + + +(Optional) +

RemoveSecret indicates whether the current secret for this ceph client should be removed or not. +If true, the K8s secret will be deleted, but the cephx keyring will remain until the CR is deleted.

+ + + + caps
map[string]string diff --git a/deploy/charts/rook-ceph/templates/resources.yaml b/deploy/charts/rook-ceph/templates/resources.yaml index 53601c457..feddc45bd 100644 --- a/deploy/charts/rook-ceph/templates/resources.yaml +++ b/deploy/charts/rook-ceph/templates/resources.yaml @@ -1293,6 +1293,19 @@ spec: x-kubernetes-preserve-unknown-fields: true name: type: string + removeSecret: + description: |- + RemoveSecret indicates whether the current secret for this ceph client should be removed or not. + If true, the K8s secret will be deleted, but the cephx keyring will remain until the CR is deleted. + type: boolean + secretName: + description: |- + SecretName is the name of the secret created for this ceph client. + If not specified, the default name is "rook-ceph-client-" as a prefix to the CR name. + type: string + x-kubernetes-validations: + - message: SecretName is immutable and cannot be changed + rule: self == oldSelf required: - caps type: object diff --git a/deploy/examples/crds.yaml b/deploy/examples/crds.yaml index 65e299706..13f6dba30 100644 --- a/deploy/examples/crds.yaml +++ b/deploy/examples/crds.yaml @@ -1292,6 +1292,19 @@ spec: x-kubernetes-preserve-unknown-fields: true name: type: string + removeSecret: + description: |- + RemoveSecret indicates whether the current secret for this ceph client should be removed or not. + If true, the K8s secret will be deleted, but the cephx keyring will remain until the CR is deleted. + type: boolean + secretName: + description: |- + SecretName is the name of the secret created for this ceph client. + If not specified, the default name is "rook-ceph-client-" as a prefix to the CR name. + type: string + x-kubernetes-validations: + - message: SecretName is immutable and cannot be changed + rule: self == oldSelf required: - caps type: object diff --git a/pkg/apis/ceph.rook.io/v1/types.go b/pkg/apis/ceph.rook.io/v1/types.go index 538a28840..538feac96 100755 --- a/pkg/apis/ceph.rook.io/v1/types.go +++ b/pkg/apis/ceph.rook.io/v1/types.go @@ -3007,6 +3007,16 @@ type CephClientList struct { type ClientSpec struct { // +optional Name string `json:"name,omitempty"` + // SecretName is the name of the secret created for this ceph client. + // If not specified, the default name is "rook-ceph-client-" as a prefix to the CR name. + // +kubebuilder:validation:XValidation:message="SecretName is immutable and cannot be changed",rule="self == oldSelf" + // +optional + SecretName string `json:"secretName,omitempty"` + + // RemoveSecret indicates whether the current secret for this ceph client should be removed or not. + // If true, the K8s secret will be deleted, but the cephx keyring will remain until the CR is deleted. + // +optional + RemoveSecret bool `json:"removeSecret,omitempty"` // +kubebuilder:pruning:PreserveUnknownFields Caps map[string]string `json:"caps"` } diff --git a/pkg/operator/ceph/client/controller.go b/pkg/operator/ceph/client/controller.go index 8f3e64bbd..0c3fb4dea 100644 --- a/pkg/operator/ceph/client/controller.go +++ b/pkg/operator/ceph/client/controller.go @@ -399,5 +399,8 @@ func generateStatusInfo(client *cephv1.CephClient) map[string]string { } func generateCephUserSecretName(client *cephv1.CephClient) string { + if client.Spec.SecretName != "" { + return client.Spec.SecretName // return the secret name as requested by user. + } return fmt.Sprintf("rook-ceph-client-%s", client.Name) } diff --git a/pkg/operator/ceph/client/controller_test.go b/pkg/operator/ceph/client/controller_test.go index 0e61ab872..fe5a09ff2 100644 --- a/pkg/operator/ceph/client/controller_test.go +++ b/pkg/operator/ceph/client/controller_test.go @@ -317,5 +317,21 @@ func TestBuildUpdateStatusInfo(t *testing.T) { statusInfo := generateStatusInfo(cephClient) assert.NotEmpty(t, statusInfo["secretName"]) - assert.Equal(t, "rook-ceph-client-client-ocp", statusInfo["secretName"]) + assert.Equal(t, generateCephUserSecretName(cephClient), statusInfo["secretName"]) +} + +func TestCustomSecretname(t *testing.T) { + secretName := "test-secret" + cephClient := &cephv1.CephClient{ + ObjectMeta: metav1.ObjectMeta{ + Name: "client-ocp", + }, + Spec: cephv1.ClientSpec{ + SecretName: secretName, + }, + } + + statusInfo := generateStatusInfo(cephClient) + assert.NotEmpty(t, statusInfo["secretName"]) + assert.Equal(t, secretName, statusInfo["secretName"]) } From 9f28101cc345cee6ad4759cceca9a61db7a0dbfe Mon Sep 17 00:00:00 2001 From: Madhu Rajanna Date: Fri, 27 Jun 2025 06:10:45 +0200 Subject: [PATCH 2/2] core: update secret handling for cephclient If RemoveSecret is set remove the secret. if not set, do below check if the current cephClient is owner if secret is present, if not return as its owned by other controller, if its not present create or update the secret. Signed-off-by: Madhu Rajanna (cherry picked from commit d37297475e045e30074143b4fdce9fb7b76fd9ef) --- pkg/operator/ceph/client/controller.go | 61 +++-- pkg/operator/ceph/client/controller_test.go | 205 +++++++++++++++ pkg/operator/k8sutil/secret.go | 97 +++++++ pkg/operator/k8sutil/secret_test.go | 273 ++++++++++++++++++++ 4 files changed, 613 insertions(+), 23 deletions(-) create mode 100644 pkg/operator/k8sutil/secret_test.go diff --git a/pkg/operator/ceph/client/controller.go b/pkg/operator/ceph/client/controller.go index 0c3fb4dea..1c36a46a7 100644 --- a/pkg/operator/ceph/client/controller.go +++ b/pkg/operator/ceph/client/controller.go @@ -285,34 +285,45 @@ func (r *ReconcileCephClient) createOrUpdateClient(cephClient *cephv1.CephClient }, Type: k8sutil.RookType, } + return r.reconcileCephClientSecret(cephClient, secret) +} - // Set CephClient owner ref to the Secret - err = controllerutil.SetControllerReference(cephClient, secret, r.scheme) - if err != nil { - return errors.Wrapf(err, "failed to set owner reference to ceph client secret %q", secret.Name) +func (r *ReconcileCephClient) reconcileCephClientSecret( + cephClient *cephv1.CephClient, + secret *v1.Secret, +) error { + // Fetch existing secret + _, getSecretErr := r.context.Clientset.CoreV1(). + Secrets(secret.Namespace). + Get(r.clusterInfo.Context, secret.Name, metav1.GetOptions{}) + if getSecretErr != nil && !kerrors.IsNotFound(getSecretErr) { + return errors.Wrapf(getSecretErr, "error fetching secret %q", secret.Name) } - // Create or Update Kubernetes Secret - _, err = r.context.Clientset.CoreV1().Secrets(cephClient.Namespace).Get(r.clusterInfo.Context, secret.Name, metav1.GetOptions{}) - if err != nil { - if kerrors.IsNotFound(err) { - logger.Debugf("creating secret for %q", secret.Name) - if _, err := r.context.Clientset.CoreV1().Secrets(cephClient.Namespace).Create(r.clusterInfo.Context, secret, metav1.CreateOptions{}); err != nil { - return errors.Wrapf(err, "failed to create secret for %q", secret.Name) - } - logger.Infof("created client %q", cephClient.Name) - return nil + if err := controllerutil.SetControllerReference(cephClient, secret, r.scheme); err != nil { + return errors.Wrapf(err, "failed to set owner reference on secret %q", secret.Name) + } + + // Delete the secret if required + if cephClient.Spec.RemoveSecret { + if getSecretErr == nil { + return k8sutil.DeleteSecretIfOwnedBy(r.clusterInfo.Context, r.context.Clientset, + secret.Name, secret.Namespace, *metav1.GetControllerOf(secret)) } - return errors.Wrapf(err, "failed to get secret for %q", secret.Name) - } - logger.Debugf("updating secret for %s", secret.Name) - _, err = r.context.Clientset.CoreV1().Secrets(cephClient.Namespace).Update(r.clusterInfo.Context, secret, metav1.UpdateOptions{}) - if err != nil { - return errors.Wrapf(err, "failed to update secret for %q", secret.Name) + return nil } - logger.Infof("updated client %q", cephClient.Name) - return nil + if kerrors.IsNotFound(getSecretErr) { + logger.Debugf("creating secret %q", secret.Namespace+"/"+secret.Name) + if _, err := r.context.Clientset.CoreV1(). + Secrets(secret.Namespace).Create(r.clusterInfo.Context, secret, metav1.CreateOptions{}); err != nil { + return errors.Wrapf(err, "failed to create secret %q", secret.Name) + } + logger.Infof("created secret for CephClient %q", cephClient.Namespace+"/"+cephClient.Name) + return nil + } + + return k8sutil.UpdateSecretIfOwnedBy(r.clusterInfo.Context, r.context.Clientset, secret) } // Delete the client @@ -394,7 +405,11 @@ func (r *ReconcileCephClient) updateStatus(observedGeneration int64, name types. func generateStatusInfo(client *cephv1.CephClient) map[string]string { m := make(map[string]string) - m["secretName"] = generateCephUserSecretName(client) + // Set only if the secret is managed by the client + if !client.Spec.RemoveSecret { + m["secretName"] = generateCephUserSecretName(client) + } + return m } diff --git a/pkg/operator/ceph/client/controller_test.go b/pkg/operator/ceph/client/controller_test.go index fe5a09ff2..84e8c0b8c 100644 --- a/pkg/operator/ceph/client/controller_test.go +++ b/pkg/operator/ceph/client/controller_test.go @@ -28,14 +28,17 @@ import ( rookclient "github.com/rook/rook/pkg/client/clientset/versioned/fake" "github.com/rook/rook/pkg/client/clientset/versioned/scheme" "github.com/rook/rook/pkg/clusterd" + cephclient "github.com/rook/rook/pkg/daemon/ceph/client" "github.com/rook/rook/pkg/operator/k8sutil" testop "github.com/rook/rook/pkg/operator/test" exectest "github.com/rook/rook/pkg/util/exec/test" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" + k8sfake "k8s.io/client-go/kubernetes/fake" "k8s.io/client-go/tools/record" "sigs.k8s.io/controller-runtime/pkg/client/fake" "sigs.k8s.io/controller-runtime/pkg/reconcile" @@ -320,6 +323,20 @@ func TestBuildUpdateStatusInfo(t *testing.T) { assert.Equal(t, generateCephUserSecretName(cephClient), statusInfo["secretName"]) } +func TestRemoveSecretUpdateStatusInfo(t *testing.T) { + cephClient := &cephv1.CephClient{ + ObjectMeta: metav1.ObjectMeta{ + Name: "client-ocp", + }, + Spec: cephv1.ClientSpec{ + RemoveSecret: true, + }, + } + + statusInfo := generateStatusInfo(cephClient) + assert.Empty(t, statusInfo) +} + func TestCustomSecretname(t *testing.T) { secretName := "test-secret" cephClient := &cephv1.CephClient{ @@ -335,3 +352,191 @@ func TestCustomSecretname(t *testing.T) { assert.NotEmpty(t, statusInfo["secretName"]) assert.Equal(t, secretName, statusInfo["secretName"]) } + +func TestReconcileCephClient_reconcileCephClientSecret(t *testing.T) { + ownerController := true + tests := []struct { + name string + removeSecret bool + existingSecret *v1.Secret + expectDelete bool + expectCreate bool + expectUpdate bool + expectError bool + }{ + { + name: "delete if removeSecret is set and owned by same cephClient", + removeSecret: true, + existingSecret: &v1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-secret", + Namespace: "rook-ceph", + OwnerReferences: []metav1.OwnerReference{{ + APIVersion: "ceph.rook.io/v1", + Kind: "CephClient", + Name: "test-client", + Controller: &ownerController, + }}, + }, + }, + expectDelete: true, + }, + { + name: "skip delete if not owned by the current cephClient", + removeSecret: true, + existingSecret: &v1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "client.test", + Namespace: "rook-ceph", + OwnerReferences: []metav1.OwnerReference{{ + APIVersion: "ceph.rook.io/v1", + Kind: "CephClient", + Name: "another-client", + Controller: &ownerController, + }}, + }, + }, + expectDelete: false, + }, + { + name: "create if secret not found", + removeSecret: false, + existingSecret: nil, + expectCreate: true, + }, + { + name: "update the secret if owned by the cephClient", + removeSecret: false, + existingSecret: &v1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-secret", + Namespace: "rook-ceph", + ResourceVersion: "123", + OwnerReferences: []metav1.OwnerReference{{ + APIVersion: "ceph.rook.io/v1", + Kind: "CephClient", + Name: "test-client", + Controller: &ownerController, + }}, + }, + }, + expectUpdate: true, + }, + { + name: "error if another ceph client is owned and removeSecret is set", + removeSecret: false, + existingSecret: &v1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-secret", + Namespace: "rook-ceph", + OwnerReferences: []metav1.OwnerReference{{ + APIVersion: "ceph.rook.io/v1", + Kind: "CephClient", + Name: "another-client", + Controller: &ownerController, + }}, + }, + }, + expectError: true, + }, + { + name: "update the secret if owned by another resource", + removeSecret: false, + existingSecret: &v1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-secret", + Namespace: "rook-ceph", + ResourceVersion: "123", + OwnerReferences: []metav1.OwnerReference{{ + APIVersion: "ceph.rook.io/v1", + Kind: "CephCluster", + Name: "test-cluster", + Controller: &ownerController, + }}, + }, + }, + expectError: true, + }, + { + name: "update the secret if there is no owner", + removeSecret: false, + existingSecret: &v1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-secret", + Namespace: "rook-ceph", + ResourceVersion: "123", + }, + }, + expectUpdate: false, + expectError: true, + }, + { + name: "secret already deleted and removeSecret is set", + removeSecret: true, + existingSecret: nil, + expectDelete: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + client := k8sfake.NewSimpleClientset() + scheme := runtime.NewScheme() + assert.NoError(t, cephv1.AddToScheme(scheme)) + r := &ReconcileCephClient{ + context: &clusterd.Context{ + Clientset: client, + }, + scheme: scheme, + clusterInfo: &cephclient.ClusterInfo{ + Context: context.TODO(), + }, + } + + cephClient := &cephv1.CephClient{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-client", + Namespace: "rook-ceph", + }, + TypeMeta: metav1.TypeMeta{ + APIVersion: "ceph.rook.io/v1", + Kind: "CephClient", + }, + Spec: cephv1.ClientSpec{ + RemoveSecret: tt.removeSecret, + }, + } + + secret := &v1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-secret", + Namespace: "rook-ceph", + }, + StringData: map[string]string{"foo": "bar"}, + } + + if tt.existingSecret != nil { + _, err := client.CoreV1().Secrets("rook-ceph").Create(context.TODO(), tt.existingSecret, metav1.CreateOptions{}) + require.NoError(t, err) + } + + err := r.reconcileCephClientSecret(cephClient, secret) + if tt.expectError { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + + secrets, _ := client.CoreV1().Secrets("rook-ceph").List(context.TODO(), metav1.ListOptions{}) + if tt.expectDelete { + assert.Empty(t, secrets.Items) + } + if tt.expectCreate { + assert.Len(t, secrets.Items, 1) + } + if tt.expectUpdate { + assert.Equal(t, "bar", secrets.Items[0].StringData["foo"]) + } + }) + } +} diff --git a/pkg/operator/k8sutil/secret.go b/pkg/operator/k8sutil/secret.go index 3a68ac4af..2161d82e7 100644 --- a/pkg/operator/k8sutil/secret.go +++ b/pkg/operator/k8sutil/secret.go @@ -23,6 +23,7 @@ import ( v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/client-go/kubernetes" ) @@ -45,3 +46,99 @@ func CreateOrUpdateSecret(ctx context.Context, clientset kubernetes.Interface, s } return s, err } + +// DeleteSecretIfOwnedBy deletes a Kubernetes Secret if its controller OwnerReference matches the provided owner. +func DeleteSecretIfOwnedBy(ctx context.Context, clientset kubernetes.Interface, secretName, namespace string, owner metav1.OwnerReference) error { + secret, err := clientset.CoreV1(). + Secrets(namespace). + Get(ctx, secretName, metav1.GetOptions{}) + if errors.IsNotFound(err) { + return nil + } + if err != nil { + return fmt.Errorf("failed to get secret %s. %w", secretName, err) + } + + secretOwner := metav1.GetControllerOf(secret) + if secretOwner == nil { + // if secret is unowned, assume it is user created and not own-able by Rook + return nil + } + + if !IsSameOwnerReference(*secretOwner, owner) { + logger.Debugf("secret %q is already owned by %s %q, skipping deletion", secret.Namespace+"/"+secret.Name, secretOwner.Kind, secretOwner.Name) + return nil + } + // secret is owned by this ceph client remove it + err = clientset.CoreV1(). + Secrets(secret.Namespace). + Delete(ctx, secret.Name, metav1.DeleteOptions{}) + if err != nil && !errors.IsNotFound(err) { + return fmt.Errorf("failed to delete secret %s. %w", secret.Name, err) + } + logger.Infof("removed secret %q", secret.Namespace+"/"+secret.Name) + + return nil +} + +// UpdateSecretIfOwnedBy fetches the latest version of the given Secret from the cluster, +// verifies controller ownership, applies any in-memory modifications from the input secret, +// and updates it if ownership matches. +func UpdateSecretIfOwnedBy(ctx context.Context, clientset kubernetes.Interface, secret *v1.Secret) error { + // Fetch latest version of the secret + existingSecret, err := clientset.CoreV1(). + Secrets(secret.Namespace). + Get(ctx, secret.Name, metav1.GetOptions{}) + if err != nil { + return fmt.Errorf("failed to fetch secret %q: %w", secret.Name, err) + } + + // Check controller ownership + existingSecretOwner := metav1.GetControllerOf(existingSecret) + if existingSecretOwner == nil { + // Secret is unowned + return fmt.Errorf("no owner founf on secret %q", existingSecret.Namespace+"/"+existingSecret.Name) + } + + secretOwner := metav1.GetControllerOf(secret) + if secretOwner == nil { + return fmt.Errorf("the ownerReference is not set on %q", secret.Name) + } + + if !IsSameOwnerReference(*existingSecretOwner, *secretOwner) { + return fmt.Errorf("secret %q is already owned by %s %q", existingSecret.Namespace+"/"+existingSecret.Name, secretOwner.Kind, secretOwner.Name) + } + + // Apply input from the secret to the existingSecret + existingSecret.Data = secret.Data + existingSecret.StringData = secret.StringData + existingSecret.Labels = secret.Labels + existingSecret.Annotations = secret.Annotations + + // Update the secret + _, err = clientset.CoreV1(). + Secrets(existingSecret.Namespace). + Update(ctx, existingSecret, metav1.UpdateOptions{}) + if err != nil { + return fmt.Errorf("failed to update secret %q: %w", existingSecret.Name, err) + } + + logger.Infof("updated secret %q", existingSecret.Namespace+"/"+existingSecret.Name) + return nil +} + +// IsSameOwnerReference returns true if the two OwnerReferences refer to the same Kubernetes resource. +// It compares Group, Kind, and Name fields. It ignores the UID field. +func IsSameOwnerReference(ownerRef, resourceRef metav1.OwnerReference) bool { + ownerRefGroupVersion, err := schema.ParseGroupVersion(ownerRef.APIVersion) + if err != nil { + return false + } + resourceGroupVersion, err := schema.ParseGroupVersion(resourceRef.APIVersion) + if err != nil { + return false + } + return ownerRefGroupVersion.Group == resourceGroupVersion.Group && + ownerRef.Kind == resourceRef.Kind && + ownerRef.Name == resourceRef.Name +} diff --git a/pkg/operator/k8sutil/secret_test.go b/pkg/operator/k8sutil/secret_test.go new file mode 100644 index 000000000..d62bec1ef --- /dev/null +++ b/pkg/operator/k8sutil/secret_test.go @@ -0,0 +1,273 @@ +/* +Copyright 2025 The Rook Authors. All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package k8sutil + +import ( + "context" + "fmt" + "testing" + + "github.com/stretchr/testify/assert" + 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" + k8sfake "k8s.io/client-go/kubernetes/fake" + k8stesting "k8s.io/client-go/testing" + "k8s.io/utils/ptr" +) + +func TestIsSameOwnerReference(t *testing.T) { + // Matching owner references + refA := metav1.OwnerReference{ + APIVersion: "ceph.rook.io/v1", + Kind: "CephClient", + Name: "my-client", + } + refB := metav1.OwnerReference{ + APIVersion: "ceph.rook.io/v1", + Kind: "CephClient", + Name: "my-client", + } + + // Mismatched owner references + refC := metav1.OwnerReference{ + APIVersion: "ceph.rook.io/v1", + Kind: "CephClient", + Name: "different-client", + } + + // Test matching references + assert.True(t, IsSameOwnerReference(refA, refB), "expected owner references to match") + + // Test mismatched references + assert.False(t, IsSameOwnerReference(refA, refC), "expected owner references to not match") +} + +func TestDeleteSecretIfOwnedBy(t *testing.T) { + owner := metav1.OwnerReference{ + APIVersion: "v1", + Kind: "CephCluster", + Name: "ceph-cluster", + Controller: ptr.To(true), + } + + tests := []struct { + name string + secret *corev1.Secret + getSecretErr error + deleteSecretErr error + expectedDelete bool + expectedErr bool + }{ + { + name: "secret not found", + getSecretErr: k8serrors.NewNotFound(schema.GroupResource{Group: "", Resource: "secrets"}, "my-secret"), + expectedDelete: false, + expectedErr: false, + }, + { + name: "error getting secret", + getSecretErr: fmt.Errorf("API error"), + expectedDelete: false, + expectedErr: true, + }, + { + name: "secret has no owner", + secret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-secret", + Namespace: "default", + }, + }, + expectedDelete: false, + expectedErr: false, + }, + { + name: "secret has different owner", + secret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-secret", + Namespace: "default", + OwnerReferences: []metav1.OwnerReference{{ + APIVersion: "v1", + Kind: "CephCluster", + Name: "other-ceph-cluster", + Controller: ptr.To(true), + }}, + }, + }, + expectedDelete: false, + expectedErr: false, + }, + { + name: "secret owned by caller, deletion succeeds", + secret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-secret", + Namespace: "default", + OwnerReferences: []metav1.OwnerReference{{ + APIVersion: "v1", + Kind: "CephCluster", + Name: "my-ceph-cluster", + Controller: ptr.To(true), + }}, + }, + }, + expectedDelete: true, + expectedErr: false, + }, + { + name: "secret owned by caller, deletion returns error", + secret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-secret", + Namespace: "default", + OwnerReferences: []metav1.OwnerReference{owner}, + }, + }, + deleteSecretErr: fmt.Errorf("delete error"), + expectedDelete: true, + expectedErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + client := k8sfake.NewSimpleClientset() + + if tt.getSecretErr != nil || tt.secret != nil { + client.Fake.PrependReactor("get", "secrets", func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { + if tt.getSecretErr != nil { + return true, nil, tt.getSecretErr + } + return true, tt.secret, nil + }) + } + + if tt.expectedDelete { + client.Fake.PrependReactor("delete", "secrets", func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { + return true, nil, tt.deleteSecretErr + }) + } + + err := DeleteSecretIfOwnedBy(context.TODO(), client, "my-secret", "default", owner) + if tt.expectedErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + }) + } +} + +func TestUpdateSecretIfOwnedBy(t *testing.T) { + ctx := context.TODO() + client := k8sfake.NewSimpleClientset() + + expectedOwner := metav1.OwnerReference{ + APIVersion: "v1", + Kind: "CephCluster", + Name: "rook-ceph", + Controller: ptr.To(true), + } + + namespace := "default" + // 1. Secret owned by expected controller → should be updated + secret1 := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-secret", + Namespace: namespace, + OwnerReferences: []metav1.OwnerReference{ + expectedOwner, + }, + }, + Data: map[string][]byte{ + "foo": []byte("bar"), + }, + } + + // Create the secret + _, err := client.CoreV1().Secrets(namespace).Create(ctx, secret1, metav1.CreateOptions{}) + assert.NoError(t, err) + + // Modify it locally before update + secret1.Data["foo"] = []byte("updated") + + err = UpdateSecretIfOwnedBy(ctx, client, secret1) + assert.NoError(t, err) + + updated, _ := client.CoreV1().Secrets(namespace).Get(ctx, secret1.Name, metav1.GetOptions{}) + assert.Equal(t, []byte("updated"), updated.Data["foo"]) + + // 2. Secret without any owner + secret2 := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "without-owner", + Namespace: namespace, + }, + Data: map[string][]byte{ + "key": []byte("val"), + }, + } + _, err = client.CoreV1().Secrets(namespace).Create(ctx, secret2, metav1.CreateOptions{}) + assert.NoError(t, err) + + secret2.Data["key"] = []byte("should-not-update") + + err = UpdateSecretIfOwnedBy(ctx, client, secret2) + assert.Error(t, err) + + // 3. Secret owned by someone else → should not be updated + otherOwner := metav1.OwnerReference{ + APIVersion: "v1", + Kind: "OtherController", + Name: "other", + Controller: ptr.To(true), + } + secret3 := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "owned-by-others", + Namespace: namespace, + OwnerReferences: []metav1.OwnerReference{ + otherOwner, + }, + }, + Data: map[string][]byte{ + "key": []byte("before"), + }, + } + _, err = client.CoreV1().Secrets(namespace).Create(ctx, secret3, metav1.CreateOptions{}) + assert.NoError(t, err) + + secret3.Data["key"] = []byte("should-not-update") + + secret3.OwnerReferences = []metav1.OwnerReference{expectedOwner} + err = UpdateSecretIfOwnedBy(ctx, client, secret3) + assert.Error(t, err) + + // 4. Secret does not exist → should return error + secret4 := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "nonexistent", + Namespace: namespace, + }, + } + err = UpdateSecretIfOwnedBy(ctx, client, secret4) + assert.Error(t, err) +}