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..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,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) } diff --git a/pkg/operator/ceph/client/controller_test.go b/pkg/operator/ceph/client/controller_test.go index 0e61ab872..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" @@ -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"]) + } + }) + } } 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) +}