Merge pull request #16065 from rook/mergify/bp/release-1.17/pr-16059

ceph: Provide an option to specify the secretName in the cephClient CR (backport #16059)

Documentation/CRDs/specification.md

@@ -592,6 +592,32 @@ string
 </tr>
 <tr>
 <td>
+<code>secretName</code><br/>
+<em>
+string
+</em>
+</td>
+<td>
+<em>(Optional)</em>
+<p>SecretName is the name of the secret created for this ceph client.
+If not specified, the default name is &ldquo;rook-ceph-client-&rdquo; as a prefix to the CR name.</p>
+</td>
+</tr>
+<tr>
+<td>
+<code>removeSecret</code><br/>
+<em>
+bool
+</em>
+</td>
+<td>
+<em>(Optional)</em>
+<p>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.</p>
+</td>
+</tr>
+<tr>
+<td>
 <code>caps</code><br/>
 <em>
 map[string]string
@@ -4556,6 +4582,32 @@ string
 </tr>
 <tr>
 <td>
+<code>secretName</code><br/>
+<em>
+string
+</em>
+</td>
+<td>
+<em>(Optional)</em>
+<p>SecretName is the name of the secret created for this ceph client.
+If not specified, the default name is &ldquo;rook-ceph-client-&rdquo; as a prefix to the CR name.</p>
+</td>
+</tr>
+<tr>
+<td>
+<code>removeSecret</code><br/>
+<em>
+bool
+</em>
+</td>
+<td>
+<em>(Optional)</em>
+<p>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.</p>
+</td>
+</tr>
+<tr>
+<td>
 <code>caps</code><br/>
 <em>
 map[string]string

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

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

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"`
 }

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,10 +405,17 @@ 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
 }
 
 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)
 }

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"
@@ -317,5 +320,223 @@ 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 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{
+		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"])
+}
+
+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"])
+			}
+		})
+	}
 }

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
+}

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)
+}