From 1a747d8a600d598b21488ce7730a31d69a14fb48 Mon Sep 17 00:00:00 2001 From: Joshua Hoblitt Date: Thu, 30 Jan 2025 12:12:05 -0700 Subject: [PATCH] object: add CephObjectStoreUser.spec.keys Signed-off-by: Joshua Hoblitt (cherry picked from commit 6b2c357871e7c5e4fbe9e183dab8b1e85e1395ef) --- Documentation/CRDs/specification.md | 137 ++++ .../Object-Storage-RGW/object-storage.md | 64 ++ PendingReleaseNotes.md | 6 + .../charts/rook-ceph/templates/resources.yaml | 76 ++ deploy/examples/crds.yaml | 76 ++ pkg/apis/ceph.rook.io/v1/types.go | 23 + .../ceph.rook.io/v1/zz_generated.deepcopy.go | 55 ++ pkg/operator/ceph/object/user/controller.go | 313 +++++++- .../ceph/object/user/controller_test.go | 27 +- tests/integration/ceph_object_test.go | 15 +- .../object/user/userkeys/userkeys.go | 749 ++++++++++++++++++ 11 files changed, 1499 insertions(+), 42 deletions(-) create mode 100644 tests/integration/object/user/userkeys/userkeys.go diff --git a/Documentation/CRDs/specification.md b/Documentation/CRDs/specification.md index 737792932..6cf3bcde8 100644 --- a/Documentation/CRDs/specification.md +++ b/Documentation/CRDs/specification.md @@ -2086,6 +2086,21 @@ ObjectUserQuotaSpec +keys
+ + +[]ObjectUserKey + + + + +(Optional) +

Allows specifying credentials for the user. If not provided, the operator +will generate them.

+ + + + clusterNamespace
string @@ -10410,6 +10425,21 @@ ObjectUserQuotaSpec +keys
+ + +[]ObjectUserKey + + + + +(Optional) +

Allows specifying credentials for the user. If not provided, the operator +will generate them.

+ + + + clusterNamespace
string @@ -10472,6 +10502,19 @@ int64

ObservedGeneration is the latest generation observed by the controller.

+ + +keys
+ + +[]SecretReference + + + + +(Optional) + +

ObjectUserCapSpec @@ -10684,6 +10727,51 @@ string +

ObjectUserKey +

+

+(Appears on:ObjectStoreUserSpec) +

+
+

ObjectUserKey defines a set of rgw user access credentials to be retrieved +from secret resources.

+
+ + + + + + + + + + + + + + + + + +
FieldDescription
+accessKeyRef
+ + +Kubernetes core/v1.SecretKeySelector + + +		 +

Secret key selector for the access_key (commonly referred to as AWS_ACCESS_KEY_ID).

+
+secretKeyRef
+ + +Kubernetes core/v1.SecretKeySelector + + +		 +

Secret key selector for the secret_key (commonly referred to as AWS_SECRET_ACCESS_KEY).

+

ObjectUserQuotaSpec

@@ -12396,6 +12484,55 @@ int32 +

SecretReference +

+

+(Appears on:ObjectStoreUserStatus) +

+
+
+ + + + + + + + + + + + + + + + + + + + + +
FieldDescription
+,secretReference
+ + +Kubernetes core/v1.SecretReference + + +		 +
+uid
+ +k8s.io/apimachinery/pkg/types.UID + +		 +
+resourceVersion
+ +string + +		 +

SecuritySpec

diff --git a/Documentation/Storage-Configuration/Object-Storage-RGW/object-storage.md b/Documentation/Storage-Configuration/Object-Storage-RGW/object-storage.md index 2aedd6187..5eb89365b 100644 --- a/Documentation/Storage-Configuration/Object-Storage-RGW/object-storage.md +++ b/Documentation/Storage-Configuration/Object-Storage-RGW/object-storage.md @@ -588,6 +588,70 @@ kubectl -n rook-ceph get secret rook-ceph-object-user-my-store-my-user -o jsonpa kubectl -n rook-ceph get secret rook-ceph-object-user-my-store-my-user -o jsonpath='{.data.SecretKey}' | base64 --decode ``` +### Managing User S3 Credentials + +The default behavior of `CephObjectStoreUser` is to place the S3 credentials generated when an RGW user is created into a Kubernetes Secret. +The s3 credential(s) may also be explicitly set via `.spec.keys`. +This field requires at least one keypair must and there is no maximum limit. +When `.spec.keys` is present, no `Secret` is created for the user. +If an existing `CephObjectStoreUser` is being transited to using explicit keys, the existing `Secret` will be deleted. +Conversely, if a `.spec.keys` is patched out of an existing `CephObjectStoreUser` all except one of keypairs set on the user will be purged and a `Secret` will be created holding the only remaining keypair. + +!!! important + When `.spec.keys` is not set, all but **one** keypair will be removed from the rgw user. + When `.spec.keys` is set, any keypair not explicitly specified will be removed from the rgw user. + +Example of explicitly managing the rgw user's keypairs: + +```yaml +--- +apiVersion: ceph.rook.io/v1 +kind: CephObjectStoreUser +metadata: + name: foo + namespace: rook-ceph +spec: + store: my-store + clusterNamespace: rook-ceph + keys: + - acccessKeyRef + name: foo-s3 + key: AWS_ACCESS_KEY_ID + secretKeyRef + name: foo-s3 + key: AWS_SECRET_ACCESS_KEY + - acccessKeyRef + name: bar-s3 + key: AWS_ACCESS_KEY_ID + secretKeyRef + name: bar-s3 + key: AWS_SECRET_ACCESS_KEY +``` + +```yaml +--- +apiVersion: v1 +kind: Secret +metadata: + name: foo-s3 + namespace: rook-ceph +data: + AWS_ACCESS_KEY_ID: baz + AWS_SECRET_ACCESS_KEY:baz +``` + +```yaml +--- +apiVersion: v1 +kind: Secret +metadata: + name: bar-s3 + namespace: rook-ceph +data: + AWS_ACCESS_KEY_ID: qux + AWS_SECRET_ACCESS_KEY: quxbar +``` + ## Enable TLS TLS is critical for securing object storage data access, and it is assumed as a default by many S3 diff --git a/PendingReleaseNotes.md b/PendingReleaseNotes.md index 192b31730..11608157c 100644 --- a/PendingReleaseNotes.md +++ b/PendingReleaseNotes.md @@ -12,6 +12,12 @@ Object: new `ROOK_OBC_ALLOW_ADDITIONAL_CONFIG_FIELDS` to enable users to set potentially risky options. See https://github.com/rook/rook/pull/15376 for more information. +- Add credentials/keys management to CephObjectStoreUser. Note that this will + cause manually created credentials/keys on an rgw user to be purged. This + behavior is necessary to ensure that keys that were explicitly declared and + then undeclared are purged. This could be a user observable regression. + (see [#15359](https://github.com/rook/rook/issues/15359) + ## Features - Support external mons for local Rook cluster (see [#14733](https://github.com/rook/rook/issues/14733)). diff --git a/deploy/charts/rook-ceph/templates/resources.yaml b/deploy/charts/rook-ceph/templates/resources.yaml index 2338398a8..a6ae5e771 100644 --- a/deploy/charts/rook-ceph/templates/resources.yaml +++ b/deploy/charts/rook-ceph/templates/resources.yaml @@ -13048,6 +13048,61 @@ spec: displayName: description: The display name for the ceph users type: string + keys: + description: |- + Allows specifying credentials for the user. If not provided, the operator + will generate them. + items: + description: |- + ObjectUserKey defines a set of rgw user access credentials to be retrieved + from secret resources. + properties: + accessKeyRef: + description: Secret key selector for the access_key (commonly referred to as AWS_ACCESS_KEY_ID). + properties: + key: + description: The key of the secret to select from. Must be a valid secret key. + type: string + name: + default: "" + description: |- + Name of the referent. + This field is effectively required, but due to backwards compatibility is + allowed to be empty. Instances of this type with an empty value here are + almost certainly wrong. + More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + type: string + optional: + description: Specify whether the Secret or its key must be defined + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + secretKeyRef: + description: Secret key selector for the secret_key (commonly referred to as AWS_SECRET_ACCESS_KEY). + properties: + key: + description: The key of the secret to select from. Must be a valid secret key. + type: string + name: + default: "" + description: |- + Name of the referent. + This field is effectively required, but due to backwards compatibility is + allowed to be empty. Instances of this type with an empty value here are + almost certainly wrong. + More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + type: string + optional: + description: Specify whether the Secret or its key must be defined + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object + type: array quotas: description: ObjectUserQuotaSpec can be used to set quotas for the object store user to limit their usage. See the [Ceph docs](https://docs.ceph.com/en/latest/radosgw/admin/?#quota-management) for more nullable: true @@ -13084,6 +13139,27 @@ spec: type: string nullable: true type: object + keys: + items: + properties: + name: + description: name is unique within a namespace to reference a secret resource. + type: string + namespace: + description: namespace defines the space within which the secret name must be unique. + type: string + resourceVersion: + type: string + uid: + description: |- + UID is a type that holds unique ID values, including UUIDs. Because we + don't ONLY use UUIDs, this is an alias to string. Being a type captures + intent and helps make sure that UIDs and names do not get conflated. + type: string + type: object + x-kubernetes-map-type: atomic + nullable: true + type: array observedGeneration: description: ObservedGeneration is the latest generation observed by the controller. format: int64 diff --git a/deploy/examples/crds.yaml b/deploy/examples/crds.yaml index b91ca97c9..01aa2fb90 100644 --- a/deploy/examples/crds.yaml +++ b/deploy/examples/crds.yaml @@ -13038,6 +13038,61 @@ spec: displayName: description: The display name for the ceph users type: string + keys: + description: |- + Allows specifying credentials for the user. If not provided, the operator + will generate them. + items: + description: |- + ObjectUserKey defines a set of rgw user access credentials to be retrieved + from secret resources. + properties: + accessKeyRef: + description: Secret key selector for the access_key (commonly referred to as AWS_ACCESS_KEY_ID). + properties: + key: + description: The key of the secret to select from. Must be a valid secret key. + type: string + name: + default: "" + description: |- + Name of the referent. + This field is effectively required, but due to backwards compatibility is + allowed to be empty. Instances of this type with an empty value here are + almost certainly wrong. + More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + type: string + optional: + description: Specify whether the Secret or its key must be defined + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + secretKeyRef: + description: Secret key selector for the secret_key (commonly referred to as AWS_SECRET_ACCESS_KEY). + properties: + key: + description: The key of the secret to select from. Must be a valid secret key. + type: string + name: + default: "" + description: |- + Name of the referent. + This field is effectively required, but due to backwards compatibility is + allowed to be empty. Instances of this type with an empty value here are + almost certainly wrong. + More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + type: string + optional: + description: Specify whether the Secret or its key must be defined + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object + type: array quotas: description: ObjectUserQuotaSpec can be used to set quotas for the object store user to limit their usage. See the [Ceph docs](https://docs.ceph.com/en/latest/radosgw/admin/?#quota-management) for more nullable: true @@ -13074,6 +13129,27 @@ spec: type: string nullable: true type: object + keys: + items: + properties: + name: + description: name is unique within a namespace to reference a secret resource. + type: string + namespace: + description: namespace defines the space within which the secret name must be unique. + type: string + resourceVersion: + type: string + uid: + description: |- + UID is a type that holds unique ID values, including UUIDs. Because we + don't ONLY use UUIDs, this is an alias to string. Being a type captures + intent and helps make sure that UIDs and names do not get conflated. + type: string + type: object + x-kubernetes-map-type: atomic + nullable: true + type: array observedGeneration: description: ObservedGeneration is the latest generation observed by the controller. format: int64 diff --git a/pkg/apis/ceph.rook.io/v1/types.go b/pkg/apis/ceph.rook.io/v1/types.go index ace762dce..72162221d 100755 --- a/pkg/apis/ceph.rook.io/v1/types.go +++ b/pkg/apis/ceph.rook.io/v1/types.go @@ -22,6 +22,7 @@ import ( v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" ) // *************************************************************************** @@ -2002,6 +2003,15 @@ type ObjectStoreUserStatus struct { // ObservedGeneration is the latest generation observed by the controller. // +optional ObservedGeneration int64 `json:"observedGeneration,omitempty"` + // +optional + // +nullable + Keys []SecretReference `json:"keys,omitempty"` +} + +type SecretReference struct { + v1.SecretReference `json:",secretReference"` + UID types.UID `json:"uid,omitempty"` + ResourceVersion string `json:"resourceVersion,omitempty"` } // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object @@ -2027,6 +2037,10 @@ type ObjectStoreUserSpec struct { // +optional // +nullable Quotas *ObjectUserQuotaSpec `json:"quotas,omitempty"` + // Allows specifying credentials for the user. If not provided, the operator + // will generate them. + // +optional + Keys []ObjectUserKey `json:"keys,omitempty"` // The namespace where the parent CephCluster and CephObjectStore are found // +optional ClusterNamespace string `json:"clusterNamespace,omitempty"` @@ -2117,6 +2131,15 @@ type ObjectUserQuotaSpec struct { MaxObjects *int64 `json:"maxObjects,omitempty"` } +// ObjectUserKey defines a set of rgw user access credentials to be retrieved +// from secret resources. +type ObjectUserKey struct { + // Secret key selector for the access_key (commonly referred to as AWS_ACCESS_KEY_ID). + AccessKeyRef *v1.SecretKeySelector `json:"accessKeyRef,omitempty"` + // Secret key selector for the secret_key (commonly referred to as AWS_SECRET_ACCESS_KEY). + SecretKeyRef *v1.SecretKeySelector `json:"secretKeyRef,omitempty"` +} + // +genclient // +genclient:noStatus // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object diff --git a/pkg/apis/ceph.rook.io/v1/zz_generated.deepcopy.go b/pkg/apis/ceph.rook.io/v1/zz_generated.deepcopy.go index db505e945..556739e25 100644 --- a/pkg/apis/ceph.rook.io/v1/zz_generated.deepcopy.go +++ b/pkg/apis/ceph.rook.io/v1/zz_generated.deepcopy.go @@ -3948,6 +3948,13 @@ func (in *ObjectStoreUserSpec) DeepCopyInto(out *ObjectStoreUserSpec) { *out = new(ObjectUserQuotaSpec) (*in).DeepCopyInto(*out) } + if in.Keys != nil { + in, out := &in.Keys, &out.Keys + *out = make([]ObjectUserKey, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } return } @@ -3971,6 +3978,11 @@ func (in *ObjectStoreUserStatus) DeepCopyInto(out *ObjectStoreUserStatus) { (*out)[key] = val } } + if in.Keys != nil { + in, out := &in.Keys, &out.Keys + *out = make([]SecretReference, len(*in)) + copy(*out, *in) + } return } @@ -4000,6 +4012,32 @@ func (in *ObjectUserCapSpec) DeepCopy() *ObjectUserCapSpec { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ObjectUserKey) DeepCopyInto(out *ObjectUserKey) { + *out = *in + if in.AccessKeyRef != nil { + in, out := &in.AccessKeyRef, &out.AccessKeyRef + *out = new(corev1.SecretKeySelector) + (*in).DeepCopyInto(*out) + } + if in.SecretKeyRef != nil { + in, out := &in.SecretKeyRef, &out.SecretKeyRef + *out = new(corev1.SecretKeySelector) + (*in).DeepCopyInto(*out) + } + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ObjectUserKey. +func (in *ObjectUserKey) DeepCopy() *ObjectUserKey { + if in == nil { + return nil + } + out := new(ObjectUserKey) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ObjectUserQuotaSpec) DeepCopyInto(out *ObjectUserQuotaSpec) { *out = *in @@ -4660,6 +4698,23 @@ func (in *SanitizeDisksSpec) DeepCopy() *SanitizeDisksSpec { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *SecretReference) DeepCopyInto(out *SecretReference) { + *out = *in + out.SecretReference = in.SecretReference + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SecretReference. +func (in *SecretReference) DeepCopy() *SecretReference { + if in == nil { + return nil + } + out := new(SecretReference) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *SecuritySpec) DeepCopyInto(out *SecuritySpec) { *out = *in diff --git a/pkg/operator/ceph/object/user/controller.go b/pkg/operator/ceph/object/user/controller.go index fd6c5d5dd..bf3aef339 100644 --- a/pkg/operator/ceph/object/user/controller.go +++ b/pkg/operator/ceph/object/user/controller.go @@ -21,31 +21,36 @@ import ( "context" "fmt" "reflect" + "slices" + "sort" "github.com/ceph/go-ceph/rgw/admin" - opcontroller "github.com/rook/rook/pkg/operator/ceph/controller" - "github.com/rook/rook/pkg/operator/ceph/reporting" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/types" - "k8s.io/client-go/tools/record" - "sigs.k8s.io/controller-runtime/pkg/controller" - "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" - "sigs.k8s.io/controller-runtime/pkg/handler" - "sigs.k8s.io/controller-runtime/pkg/manager" - "sigs.k8s.io/controller-runtime/pkg/reconcile" - "sigs.k8s.io/controller-runtime/pkg/source" - "github.com/coreos/pkg/capnslog" "github.com/pkg/errors" - cephv1 "github.com/rook/rook/pkg/apis/ceph.rook.io/v1" - "github.com/rook/rook/pkg/clusterd" - cephclient "github.com/rook/rook/pkg/daemon/ceph/client" - "github.com/rook/rook/pkg/operator/ceph/object" - "github.com/rook/rook/pkg/operator/k8sutil" corev1 "k8s.io/api/core/v1" kerrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/fields" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/tools/record" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/handler" + "sigs.k8s.io/controller-runtime/pkg/manager" + "sigs.k8s.io/controller-runtime/pkg/predicate" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + "sigs.k8s.io/controller-runtime/pkg/source" + + cephv1 "github.com/rook/rook/pkg/apis/ceph.rook.io/v1" + "github.com/rook/rook/pkg/clusterd" + cephclient "github.com/rook/rook/pkg/daemon/ceph/client" + opcontroller "github.com/rook/rook/pkg/operator/ceph/controller" + "github.com/rook/rook/pkg/operator/ceph/object" + "github.com/rook/rook/pkg/operator/ceph/reporting" + "github.com/rook/rook/pkg/operator/k8sutil" ) const ( @@ -124,6 +129,76 @@ func add(mgr manager.Manager, r reconcile.Reconciler) error { return err } + // watch secrets referenced by CephObjectStoreUser.spec.keys + const ( + secretNameField = "spec.keys.secretNames" + ) + + err = mgr.GetFieldIndexer().IndexField(context.TODO(), &cephv1.CephObjectStoreUser{TypeMeta: controllerTypeMeta}, secretNameField, func(obj client.Object) []string { + var secretNames []string + for _, k := range obj.(*cephv1.CephObjectStoreUser).Spec.Keys { + if k.AccessKeyRef != nil && k.AccessKeyRef.Name != "" { + secretNames = append(secretNames, k.AccessKeyRef.Name) + } + if k.SecretKeyRef != nil && k.SecretKeyRef.Name != "" { + secretNames = append(secretNames, k.SecretKeyRef.Name) + } + } + slices.Sort(secretNames) + return slices.Compact(secretNames) + }) + if err != nil { + return errors.Wrapf(err, "failed to setup IndexField for CephObjectStoreUser.Spec.Keys") + } + + // Always trigger a reconcile when a secret is deleted. This will cause a + // reconciliation failure to happen immediately in hopes of alerting the end + // user to the configuration problem. + changedOrDeleted := predicate.Or( + predicate.TypedResourceVersionChangedPredicate[*corev1.Secret]{}, + predicate.TypedFuncs[*corev1.Secret]{ + DeleteFunc: func(e event.TypedDeleteEvent[*corev1.Secret]) bool { + return true + }, + }, + ) + + err = c.Watch( + source.Kind( + mgr.GetCache(), + &corev1.Secret{}, + handler.TypedEnqueueRequestsFromMapFunc( + func(ctx context.Context, secret *corev1.Secret) []reconcile.Request { + referencingUsers := &cephv1.CephObjectStoreUserList{} + err := r.(*ReconcileObjectStoreUser).client.List(ctx, referencingUsers, &client.ListOptions{ + FieldSelector: fields.OneTermEqualSelector(secretNameField, secret.GetName()), + Namespace: secret.GetNamespace(), + }) + if err != nil { + logger.Errorf("failed to list CephObjectStoreUser(s) while handling event for secret %q in namespace %q. %v", secret.GetName(), secret.GetNamespace(), err) + return []reconcile.Request{} + } + + requests := make([]reconcile.Request, len(referencingUsers.Items)) + for i, item := range referencingUsers.Items { + requests[i] = reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: item.GetName(), + Namespace: item.GetNamespace(), + }, + } + } + logger.Tracef("CephObjectStoreUsers referencing Secret %q in namespace %q: %v", secret.GetName(), secret.GetNamespace(), requests) + return requests + }, + ), + changedOrDeleted, + ), + ) + if err != nil { + return errors.Wrapf(err, "failed to configure watch for Secret(s)") + } + return nil } @@ -224,6 +299,18 @@ func (r *ReconcileObjectStoreUser) reconcile(request reconcile.Request) (reconci // Generate user config userConfig := generateUserConfig(cephObjectStoreUser) + referencedSecrets := &map[types.UID]*corev1.Secret{} + // Set any provided key pair(s) + if len(cephObjectStoreUser.Spec.Keys) > 0 { + var keys *[]admin.UserKeySpec + keys, referencedSecrets, err = r.generateUserKeySpec(cephObjectStoreUser) + if err != nil { + return reconcile.Result{}, *cephObjectStoreUser, errors.Wrapf(err, "failed to generate UserKeySpec for %q", cephObjectStoreUser.Name) + } + + userConfig.Keys = *keys + } + // DELETE: the CR was deleted if !cephObjectStoreUser.GetDeletionTimestamp().IsZero() { logger.Debugf("deleting object store user %q", request.NamespacedName) @@ -257,6 +344,11 @@ func (r *ReconcileObjectStoreUser) reconcile(request reconcile.Request) (reconci return reconcileResponse, *cephObjectStoreUser, err } + // Update status of referenced secrets only after the rgw user has + // reconciled. Update even when no secrets are referenced as this could be a + // transition from explicit keys -> automatic secret generation. + r.updateKeyStatus(request.NamespacedName, referencedSecrets) + // CREATE/UPDATE KUBERNETES SECRET store, err := r.getObjectStore(cephObjectStoreUser.Spec.Store) if err != nil { @@ -293,7 +385,8 @@ func (r *ReconcileObjectStoreUser) createOrUpdateCephUser(u *cephv1.CephObjectSt logCreateOrUpdate := fmt.Sprintf("retrieved existing ceph object user %q", u.Name) var user admin.User var err error - user, err = r.objContext.AdminOpsClient.GetUser(r.opManagerContext, *userConfig) + // lookup user by name only and not by access key + user, err = r.objContext.AdminOpsClient.GetUser(r.opManagerContext, admin.User{ID: u.Name}) if err != nil { if errors.Is(err, admin.ErrNoSuchUser) { user, err = r.objContext.AdminOpsClient.CreateUser(r.opManagerContext, *userConfig) @@ -362,12 +455,20 @@ func (r *ReconcileObjectStoreUser) createOrUpdateCephUser(u *cephv1.CephObjectSt return errors.Wrapf(err, "failed to set quotas for user %q", u.Name) } - // Set access and secret key if len(userConfig.Keys) == 0 { - userConfig.Keys = make([]admin.UserKeySpec, 1) + // use the keys already set on the user & remove all but one key + if len(user.Keys) == 0 { + // something is wrong, there should be at least one key + return errors.Wrapf(err, "no keys set for user %q", u.Name) + } + + userConfig.Keys = []admin.UserKeySpec{user.Keys[0]} + logger.Debugf("reducing user %q keypairs to %v", u.Name, userConfig.Keys) + } + + if err := r.reconcileUserKeys(u.Name, userConfig.Keys); err != nil { + return errors.Wrapf(err, "failed to reconcile keys for user %q", u.Name) } - userConfig.Keys[0].AccessKey = user.Keys[0].AccessKey - userConfig.Keys[0].SecretKey = user.Keys[0].SecretKey logger.Info(logCreateOrUpdate) return nil @@ -549,14 +650,12 @@ func (r *ReconcileObjectStoreUser) reconcileCephUserSecret(cephObjectStoreUser * secret := r.generateCephUserSecret(cephObjectStoreUser, userConfig, tlsSecretName) // Set owner ref to the object store user object - err := controllerutil.SetControllerReference(cephObjectStoreUser, secret, r.scheme) - if err != nil { + if err := controllerutil.SetControllerReference(cephObjectStoreUser, secret, r.scheme); err != nil { return reconcile.Result{}, errors.Wrapf(err, "failed to set owner reference of ceph object user secret %q", secret.Name) } // Create Kubernetes Secret - err = opcontroller.CreateOrUpdateObject(r.opManagerContext, r.client, secret) - if err != nil { + if err := opcontroller.CreateOrUpdateObject(r.opManagerContext, r.client, secret); err != nil { return reconcile.Result{}, errors.Wrapf(err, "failed to create or update ceph object user %q secret", secret.Name) } @@ -703,3 +802,165 @@ func (r *ReconcileObjectStoreUser) updateStatus(observedGeneration int64, name t } logger.Debugf("object store user %q status updated to %q", name, status) } + +// updates `.status.keys`. This functionality is not included as part of +// updateStatus() so that the list of referenced secrets, if any, can be +// updated at the same time the rgw user key set is reconciled. This avoids the +// need to regenerate the list of referenced secrets a second time when the +// reconcile has completed and the overall resource status is updated. +func (r *ReconcileObjectStoreUser) updateKeyStatus(name types.NamespacedName, referencedSecrets *map[types.UID]*corev1.Secret) { + user := &cephv1.CephObjectStoreUser{} + if err := r.client.Get(r.opManagerContext, name, user); err != nil { + if kerrors.IsNotFound(err) { + logger.Debug("CephObjectStoreUser resource not found. Ignoring since object must be deleted.") + return + } + logger.Warningf("failed to retrieve CephObjectStoreUser %q to update .status.keys. %v", name, err) + return + } + if user.Status == nil { + user.Status = &cephv1.ObjectStoreUserStatus{} + } + + logger.Debugf("updating CephObjectStoreUser %q .status.keys to %+v.", name.Name, referencedSecrets) + + keyStatus := []cephv1.SecretReference{} + + for _, secret := range *referencedSecrets { + keyStatus = append(keyStatus, cephv1.SecretReference{ + SecretReference: corev1.SecretReference{ + Name: secret.Name, + Namespace: secret.Namespace, + }, + UID: secret.UID, + ResourceVersion: secret.ResourceVersion, + }) + } + + // assume map key ordering is unstable between reconciles and sort the slice + // by secret name + sort.Slice(keyStatus, func(i, j int) bool { + return keyStatus[i].Name < keyStatus[j].Name + }) + + user.Status.Keys = keyStatus + + if err := reporting.UpdateStatus(r.client, user); err != nil { + logger.Warningf("failed to update CephObjectStoreUser %q .status.keys. %v", name, err) + return + } + logger.Debugf("updated CephObjectStoreUser %q .status.keys.", name) +} + +// getSecretValue returns the value of key in a kubernetes secret +func (r *ReconcileObjectStoreUser) getSecretValue(selector *corev1.SecretKeySelector, namespace string) (string, *corev1.Secret, error) { + secret := &corev1.Secret{} + namespacedName := types.NamespacedName{ + Name: selector.Name, + Namespace: namespace, + } + if err := r.client.Get(r.opManagerContext, namespacedName, secret); err != nil { + return "", nil, errors.Wrapf(err, "failed to get secret %q", namespacedName) + } + value, ok := secret.Data[selector.Key] + if !ok { + return "", nil, errors.Errorf("failed to find key %q in secret %q", selector.Key, namespacedName) + } + + return string(value), secret, nil +} + +// reconcileUserKeys ensures the user's RGW keys match exactly the targetKeys slice. Any keys set on the user but not present in targetKeys are purged. +func (r *ReconcileObjectStoreUser) reconcileUserKeys(userID string, targetKeys []admin.UserKeySpec) error { + ctx := r.opManagerContext + client := r.objContext.AdminOpsClient + + // fetch the current user keys + userInfo, err := client.GetUser(ctx, admin.User{ID: userID}) + if err != nil { + return errors.Wrapf(err, "failed to get user %q", userID) + } + + // create a lookup for the targetkeys (by AccessKey) + targetMap := make(map[string]admin.UserKeySpec, len(targetKeys)) + for _, k := range targetKeys { + targetMap[k.AccessKey] = k + } + + syncdKeys := make([]admin.UserKeySpec, len(targetKeys)) + + // remove any keys configured for the user that aren't present in targetKeys + for _, existingKey := range userInfo.Keys { + targetKey, found := targetMap[existingKey.AccessKey] + if !found { + // RemoveKey() requires the UID to be set but GetUser() returns the list of keys with only .User set + rmKey := existingKey + rmKey.UID = userID + + if err := client.RemoveKey(ctx, rmKey); err != nil { + return errors.Wrapf(err, "failed to remove key %q from user %q", existingKey.AccessKey, userID) + } + logger.Debugf("removed key %q from user %q as it is not in the target list", existingKey.AccessKey, userID) + continue + } + if existingKey.SecretKey != targetKey.SecretKey { + // key exists but needs to be updated; delete it so it will be recreated + // RemoveKey() requires the UID to be set but GetUser() returns the list of keys with only .User set + rmKey := existingKey + rmKey.UID = userID + + if err := client.RemoveKey(ctx, rmKey); err != nil { + return errors.Wrapf(err, "failed to remove key %q from user %q", existingKey.AccessKey, userID) + } + logger.Debugf("removed key %q from user %q needs as it needs to be recreated", existingKey.AccessKey, userID) + continue + } + // else key exists and is correct, no action needed + // defer removal from targetMap until after the loop to avoid changing the map while iterating + syncdKeys = append(syncdKeys, targetKey) + } + + // remove any keys from targetKeys that are already in sync + for _, k := range syncdKeys { + delete(targetMap, k.AccessKey) + } + + // create each desired key + for _, k := range targetMap { + if _, err := client.CreateKey(ctx, k); err != nil { + return errors.Wrapf(err, "failed to create key %q for user %q", k.AccessKey, userID) + } + logger.Debugf("created key %q for user %q", k.AccessKey, userID) + } + + return nil +} + +// Construct a []admin.UserKeySpec from all secrets referenced from CephObjectStoreUser.Spec.Keys +func (r *ReconcileObjectStoreUser) generateUserKeySpec(user *cephv1.CephObjectStoreUser) (*[]admin.UserKeySpec, *map[types.UID]*corev1.Secret, error) { + referencedSecrets := make(map[types.UID]*corev1.Secret) + + keys := make([]admin.UserKeySpec, len(user.Spec.Keys)) + for _, key := range user.Spec.Keys { + accessKey, secret, err := r.getSecretValue(key.AccessKeyRef, user.Namespace) + if err != nil { + return nil, nil, errors.Wrapf(err, "failed to get secret value") + } + referencedSecrets[secret.UID] = secret + + secretKey, secret, err := r.getSecretValue(key.SecretKeyRef, user.Namespace) + if err != nil { + return nil, nil, errors.Wrapf(err, "failed to get secret value") + } + referencedSecrets[secret.UID] = secret + + keys = append(keys, admin.UserKeySpec{ + UID: user.Name, + AccessKey: accessKey, + SecretKey: secretKey, + KeyType: "s3", + }) + } + + return &keys, &referencedSecrets, nil +} diff --git a/pkg/operator/ceph/object/user/controller_test.go b/pkg/operator/ceph/object/user/controller_test.go index 26d0c7a85..4ae6702a4 100644 --- a/pkg/operator/ceph/object/user/controller_test.go +++ b/pkg/operator/ceph/object/user/controller_test.go @@ -37,6 +37,7 @@ import ( cephobject "github.com/rook/rook/pkg/operator/ceph/object" exectest "github.com/rook/rook/pkg/util/exec/test" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -495,7 +496,6 @@ func TestCreateOrUpdateCephUser(t *testing.T) { } adminClient, err := admin.New("rook-ceph-rgw-my-store.mycluster.svc", "53S6B9S809NUP19IJ2K3", "1bXPegzsGClvoGAiJdHQD1uOW2sQBLAZM9j9VtXR", mockClient) assert.NoError(t, err) - userConfig := generateUserConfig(objectUser) r := &ReconcileObjectStoreUser{ objContext: &cephobject.AdminOpsContext{ AdminOpsClient: adminClient, @@ -506,20 +506,21 @@ func TestCreateOrUpdateCephUser(t *testing.T) { assert.NoError(t, err) t.Run("user with empty name", func(t *testing.T) { + userConfig := &admin.User{} err = r.createOrUpdateCephUser(objectUser, userConfig) assert.Error(t, err) }) t.Run("user without any Quotas or Capabilities", func(t *testing.T) { objectUser.Name = name - userConfig = generateUserConfig(objectUser) + userConfig := generateUserConfig(objectUser) err = r.createOrUpdateCephUser(objectUser, userConfig) assert.NoError(t, err) }) t.Run("setting MaxBuckets for the user", func(t *testing.T) { objectUser.Spec.Quotas = &cephv1.ObjectUserQuotaSpec{MaxBuckets: &maxbucket} - userConfig = generateUserConfig(objectUser) + userConfig := generateUserConfig(objectUser) err = r.createOrUpdateCephUser(objectUser, userConfig) assert.NoError(t, err) }) @@ -532,7 +533,7 @@ func TestCreateOrUpdateCephUser(t *testing.T) { Roles: "*", Info: "read, write", } - userConfig = generateUserConfig(objectUser) + userConfig := generateUserConfig(objectUser) err = r.createOrUpdateCephUser(objectUser, userConfig) assert.NoError(t, err) }) @@ -541,31 +542,36 @@ func TestCreateOrUpdateCephUser(t *testing.T) { t.Run("setting MaxObjects for the user", func(t *testing.T) { objectUser.Spec.Capabilities = nil objectUser.Spec.Quotas = &cephv1.ObjectUserQuotaSpec{MaxObjects: &maxobject} - userConfig = generateUserConfig(objectUser) + userConfig := generateUserConfig(objectUser) + require.NoError(t, err) err = r.createOrUpdateCephUser(objectUser, userConfig) assert.NoError(t, err) }) t.Run("setting MaxSize for the user", func(t *testing.T) { objectUser.Spec.Quotas = &cephv1.ObjectUserQuotaSpec{MaxSize: &maxsize} - userConfig = generateUserConfig(objectUser) + userConfig := generateUserConfig(objectUser) + require.NoError(t, err) err = r.createOrUpdateCephUser(objectUser, userConfig) assert.NoError(t, err) }) t.Run("resetting MaxSize and MaxObjects for the user", func(t *testing.T) { objectUser.Spec.Quotas = nil - userConfig = generateUserConfig(objectUser) + userConfig := generateUserConfig(objectUser) + require.NoError(t, err) err = r.createOrUpdateCephUser(objectUser, userConfig) assert.NoError(t, err) }) t.Run("setting both MaxSize and MaxObjects for the user", func(t *testing.T) { objectUser.Spec.Quotas = &cephv1.ObjectUserQuotaSpec{MaxObjects: &maxobject, MaxSize: &maxsize} - userConfig = generateUserConfig(objectUser) + userConfig := generateUserConfig(objectUser) + require.NoError(t, err) err = r.createOrUpdateCephUser(objectUser, userConfig) assert.NoError(t, err) }) t.Run("resetting MaxSize and MaxObjects again for the user", func(t *testing.T) { objectUser.Spec.Quotas = nil - userConfig = generateUserConfig(objectUser) + userConfig := generateUserConfig(objectUser) + require.NoError(t, err) err = r.createOrUpdateCephUser(objectUser, userConfig) assert.NoError(t, err) }) @@ -578,7 +584,8 @@ func TestCreateOrUpdateCephUser(t *testing.T) { Info: "read, write", } objectUser.Spec.Quotas = &cephv1.ObjectUserQuotaSpec{MaxBuckets: &maxbucket, MaxObjects: &maxobject, MaxSize: &maxsize} - userConfig = generateUserConfig(objectUser) + userConfig := generateUserConfig(objectUser) + require.NoError(t, err) err = r.createOrUpdateCephUser(objectUser, userConfig) assert.NoError(t, err) }) diff --git a/tests/integration/ceph_object_test.go b/tests/integration/ceph_object_test.go index 70c2ef930..3d18c86a3 100644 --- a/tests/integration/ceph_object_test.go +++ b/tests/integration/ceph_object_test.go @@ -27,6 +27,13 @@ import ( "github.com/aws/aws-sdk-go/service/s3" "github.com/google/go-cmp/cmp" "github.com/kube-object-storage/lib-bucket-provisioner/pkg/apis/objectbucket.io/v1alpha1" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/stretchr/testify/suite" + v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + cephv1 "github.com/rook/rook/pkg/apis/ceph.rook.io/v1" "github.com/rook/rook/pkg/daemon/ceph/client" rgw "github.com/rook/rook/pkg/operator/ceph/object" @@ -34,12 +41,7 @@ import ( "github.com/rook/rook/tests/framework/installer" "github.com/rook/rook/tests/framework/utils" "github.com/rook/rook/tests/integration/object/bucketowner" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - "github.com/stretchr/testify/suite" - v1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "github.com/rook/rook/tests/integration/object/user/userkeys" ) const ( @@ -153,6 +155,7 @@ func runObjectE2ETest(helper *clients.TestClient, k8sh *utils.K8sHelper, install testObjectStoreOperations(s, helper, k8sh, namespace, storeName, swiftAndKeystone) bucketowner.TestObjectBucketClaimBucketOwner(s.T(), k8sh, installer, logger, tlsEnable) + userkeys.TestObjectStoreUserKeys(s.T(), k8sh, installer, logger, tlsEnable) bucketNotificationTestStoreName := "bucket-notification-" + storeName createCephObjectStore(s.T(), helper, k8sh, installer, namespace, bucketNotificationTestStoreName, 1, tlsEnable, swiftAndKeystone) diff --git a/tests/integration/object/user/userkeys/userkeys.go b/tests/integration/object/user/userkeys/userkeys.go new file mode 100644 index 000000000..da06e4f6e --- /dev/null +++ b/tests/integration/object/user/userkeys/userkeys.go @@ -0,0 +1,749 @@ +/* +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 userkeys + +import ( + "context" + "crypto/tls" + "encoding/json" + "fmt" + "net/http" + "testing" + "time" + + "github.com/ceph/go-ceph/rgw/admin" + "github.com/coreos/pkg/capnslog" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" + + cephv1 "github.com/rook/rook/pkg/apis/ceph.rook.io/v1" + "github.com/rook/rook/tests/framework/installer" + "github.com/rook/rook/tests/framework/utils" +) + +var ( + defaultName = "test-userkeys" + + ns = &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: defaultName, + }, + } + + objectStore = &cephv1.CephObjectStore{ + ObjectMeta: metav1.ObjectMeta{ + Name: defaultName, + // the CephObjectstore must be in the same ns as the CephCluster + Namespace: "object-ns", + }, + Spec: cephv1.ObjectStoreSpec{ + MetadataPool: cephv1.PoolSpec{ + Replicated: cephv1.ReplicatedSpec{ + Size: 1, + RequireSafeReplicaSize: false, + }, + }, + DataPool: cephv1.PoolSpec{ + Replicated: cephv1.ReplicatedSpec{ + Size: 1, + RequireSafeReplicaSize: false, + }, + }, + Gateway: cephv1.GatewaySpec{ + Port: 80, + Instances: 1, + }, + AllowUsersInNamespaces: []string{ns.Name}, + }, + } + + objectStoreSvc = &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: objectStore.Name, + Namespace: objectStore.Namespace, + }, + Spec: corev1.ServiceSpec{ + Selector: map[string]string{ + "app": "rook-ceph-rgw", + "rook_cluster": objectStore.Namespace, + "rook_object_store": objectStore.Name, + }, + Ports: []corev1.ServicePort{ + { + Name: "http", + Port: 80, + Protocol: corev1.ProtocolTCP, + TargetPort: intstr.FromInt(8080), + }, + }, + SessionAffinity: corev1.ServiceAffinityNone, + Type: corev1.ServiceTypeNodePort, + }, + } + + secret1 = &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: defaultName + "-secret1", + Namespace: ns.Name, + }, + Data: map[string][]byte{ + "AWS_ACCESS_KEY_ID": []byte("P1ANF2BP4K2LPGOR5SBB"), + "AWS_SECRET_ACCESS_KEY": []byte("yfCHPhhOed0vJkqZsGEODyJrdKqHD09OMTWCnwjX"), + }, + } + + secret2 = &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: defaultName + "-secret2", + Namespace: ns.Name, + }, + Data: map[string][]byte{ + "AWS_ACCESS_KEY_ID": []byte("2I6RPUTQFMNCSYEXZ6VM"), + "AWS_SECRET_ACCESS_KEY": []byte("uY066SWPfaOVlDeYc7GJyOTfkDejyDdXrqehS6wx"), + }, + } + + secret3 = &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: defaultName + "-secret3", + Namespace: ns.Name, + }, + Data: map[string][]byte{ + "AWS_ACCESS_KEY_ID": []byte("J4D0P20F3EDR51OSND7Y"), + "AWS_SECRET_ACCESS_KEY": []byte("jn89OpkXNoDlIHVVQ23mE2DZgPmuDK3WVH5ExOvQ"), + }, + } + + secret4 = &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: defaultName + "-secret4", + Namespace: ns.Name, + }, + Data: map[string][]byte{ + "AWS_ACCESS_KEY_ID": []byte("MPZX7DG5WJWQ6VPCSLYT"), + "AWS_SECRET_ACCESS_KEY": []byte("phh7DIxnLPeSD2V6FUouhmnWrKlKRD5dBykyXozX"), + }, + } + + secret5 = &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: defaultName + "-secret5", + Namespace: ns.Name, + }, + Data: map[string][]byte{ + "AWS_ACCESS_KEY_ID": []byte("7TNSSANCO5KXK23IPT91"), + "AWS_SECRET_ACCESS_KEY": []byte("HksEDf0hEh3PtTvl7s9x6CyXfkWuY8eAMYAcvH5l"), + }, + } + + osu1 = cephv1.CephObjectStoreUser{ + ObjectMeta: metav1.ObjectMeta{ + Name: defaultName + "-user1", + Namespace: ns.Name, + }, + Spec: cephv1.ObjectStoreUserSpec{ + Store: objectStore.Name, + ClusterNamespace: objectStore.Namespace, + Keys: []cephv1.ObjectUserKey{ + { + AccessKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: secret1.Name, + }, + Key: "AWS_ACCESS_KEY_ID", + }, + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: secret1.Name, + }, + Key: "AWS_SECRET_ACCESS_KEY", + }, + }, + { + AccessKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: secret2.Name, + }, + Key: "AWS_ACCESS_KEY_ID", + }, + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: secret2.Name, + }, + Key: "AWS_SECRET_ACCESS_KEY", + }, + }, + { + AccessKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: secret3.Name, + }, + Key: "AWS_ACCESS_KEY_ID", + }, + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: secret3.Name, + }, + Key: "AWS_SECRET_ACCESS_KEY", + }, + }, + }, + }, + } +) + +// generate the secret name the operator is expected to generate for the CephObjectStoreUser +func generateObjectStoreUserSecretName(osu cephv1.CephObjectStoreUser) string { + return "rook-ceph-object-user-" + osu.Namespace + "-" + osu.Name +} + +// find a UserKeySpec by AccessKey value +func findUserKeySpec(keys []admin.UserKeySpec, accessKey string) (admin.UserKeySpec, error) { + for _, k := range keys { + if k.AccessKey == accessKey { + return k, nil + } + } + return admin.UserKeySpec{}, fmt.Errorf("UserKeySpec with AccessKey %q not found", accessKey) +} + +func checkStatusKeys(t *testing.T, k8sh *utils.K8sHelper, osu cephv1.CephObjectStoreUser, expectedSecrets []*corev1.Secret) { + t.Run(fmt.Sprintf("cephObjectStoreUser %q has .status.keys set", osu.Name), func(t *testing.T) { + ctx := context.TODO() + + liveOsu, err := k8sh.RookClientset.CephV1().CephObjectStoreUsers(ns.Name).Get(ctx, osu.Name, metav1.GetOptions{}) + require.NoError(t, err) + + require.NotNil(t, liveOsu.Status) + assert.Len(t, liveOsu.Status.Keys, len(expectedSecrets)) + + for _, secret := range expectedSecrets { + secretRef, err := func(secretName string, keys []cephv1.SecretReference) (cephv1.SecretReference, error) { + for _, secretRef := range keys { + if secretRef.Name == secretName { + return secretRef, nil + } + } + return cephv1.SecretReference{}, fmt.Errorf("secretReference for secret %q not found in CephObjectStoreUser.status.keys", secret.Name) + }(secret.Name, liveOsu.Status.Keys) + require.NoError(t, err) + + // fetch the live secret for UID and ResourceVersion + liveSecret, err := k8sh.Clientset.CoreV1().Secrets(ns.Name).Get(ctx, secret.Name, metav1.GetOptions{}) + require.NoError(t, err) + + assert.Equal(t, liveSecret.Name, secretRef.Name) + assert.Equal(t, liveSecret.Namespace, secretRef.Namespace) + assert.Equal(t, liveSecret.UID, secretRef.UID) + assert.Equal(t, liveSecret.ResourceVersion, secretRef.ResourceVersion) + } + }) +} + +func checkRgwUserKeys(t *testing.T, adminClient *admin.API, osu cephv1.CephObjectStoreUser, expectedSecrets []*corev1.Secret, accessKeyName, secretKeyName string) { + t.Run(fmt.Sprintf("rgw user %q has keys set", osu.Name), func(t *testing.T) { + ctx := context.TODO() + + for _, secret := range expectedSecrets { + var keySpec admin.UserKeySpec + + // assume that the .Phase doesn't change when updating keys + inSync := utils.Retry(40, time.Second, fmt.Sprintf("CephObjectStoreUser has key in sync with secret %q ", secret.Name), func() bool { + liveUser, err := adminClient.GetUser(ctx, admin.User{ID: osu.Name}) + if err != nil { + return false + } + keySpec, err = findUserKeySpec(liveUser.Keys, string(secret.Data[accessKeyName])) + return err == nil + }) + require.True(t, inSync) + + assert.Equal(t, string(secret.Data[accessKeyName]), keySpec.AccessKey) + assert.Equal(t, string(secret.Data[secretKeyName]), keySpec.SecretKey) + } + + // check that no extra keys are present + liveUser, err := adminClient.GetUser(ctx, admin.User{ID: osu.Name}) + require.NoError(t, err) + assert.Equal(t, len(expectedSecrets), len(liveUser.Keys)) + }) +} + +func TestObjectStoreUserKeys(t *testing.T, k8sh *utils.K8sHelper, installer *installer.CephInstaller, logger *capnslog.PackageLogger, tlsEnable bool) { + t.Run("ObjectStoreUser keys", func(t *testing.T) { + if tlsEnable { + // Skip testing with and without TLS to reduce test time + t.Skip("skipping test for TLS enabled clusters") + } + + var adminClient *admin.API + ctx := context.TODO() + + t.Run(fmt.Sprintf("create ns %q", ns.Name), func(t *testing.T) { + _, err := k8sh.Clientset.CoreV1().Namespaces().Create(ctx, ns, metav1.CreateOptions{}) + require.NoError(t, err) + }) + + t.Run(fmt.Sprintf("create CephObjectStore %q", objectStore.Name), func(t *testing.T) { + objectStore, err := k8sh.RookClientset.CephV1().CephObjectStores(objectStore.Namespace).Create(ctx, objectStore, metav1.CreateOptions{}) + require.NoError(t, err) + + osReady := utils.Retry(180, time.Second, "CephObjectStore is Ready", func() bool { + liveOs, err := k8sh.RookClientset.CephV1().CephObjectStores(objectStore.Namespace).Get(ctx, objectStore.Name, metav1.GetOptions{}) + if err != nil { + return false + } + + if liveOs.Status == nil { + return false + } + + return liveOs.Status.Phase == cephv1.ConditionReady + }) + require.True(t, osReady) + }) + + t.Run(fmt.Sprintf("create svc %q", objectStoreSvc.Name), func(t *testing.T) { + _, err := k8sh.Clientset.CoreV1().Services(objectStore.Namespace).Create(ctx, objectStoreSvc, metav1.CreateOptions{}) + require.NoError(t, err) + }) + + t.Run(fmt.Sprintf("create secret %q", secret1.Name), func(t *testing.T) { + _, err := k8sh.Clientset.CoreV1().Secrets(ns.Name).Create(ctx, secret1, metav1.CreateOptions{}) + require.NoError(t, err) + }) + + t.Run(fmt.Sprintf("create secret %q", secret2.Name), func(t *testing.T) { + _, err := k8sh.Clientset.CoreV1().Secrets(ns.Name).Create(ctx, secret2, metav1.CreateOptions{}) + require.NoError(t, err) + }) + + t.Run(fmt.Sprintf("create secret %q", secret3.Name), func(t *testing.T) { + _, err := k8sh.Clientset.CoreV1().Secrets(ns.Name).Create(ctx, secret3, metav1.CreateOptions{}) + require.NoError(t, err) + }) + + t.Run(fmt.Sprintf("create secret %q", secret4.Name), func(t *testing.T) { + _, err := k8sh.Clientset.CoreV1().Secrets(ns.Name).Create(ctx, secret4, metav1.CreateOptions{}) + require.NoError(t, err) + }) + + t.Run(fmt.Sprintf("create secret %q", secret5.Name), func(t *testing.T) { + _, err := k8sh.Clientset.CoreV1().Secrets(ns.Name).Create(ctx, secret5, metav1.CreateOptions{}) + require.NoError(t, err) + }) + + t.Run(fmt.Sprintf("create CephObjectStoreUser %q", osu1.Name), func(t *testing.T) { + _, err := k8sh.RookClientset.CephV1().CephObjectStoreUsers(ns.Name).Create(ctx, &osu1, metav1.CreateOptions{}) + require.NoError(t, err) + + // user creation may be slow right after rgw start up + osuReady := utils.Retry(120, time.Second, "CephObjectStoreUser is Ready", func() bool { + liveOsu, err := k8sh.RookClientset.CephV1().CephObjectStoreUsers(ns.Name).Get(ctx, osu1.Name, metav1.GetOptions{}) + if err != nil { + return false + } + + if liveOsu.Status == nil { + return false + } + + return liveOsu.Status.Phase == string(cephv1.ConditionReady) + }) + require.True(t, osuReady) + }) + + t.Run("setup rgw admin api client", func(t *testing.T) { + err, output := installer.Execute("radosgw-admin", []string{"user", "info", "--uid=dashboard-admin", fmt.Sprintf("--rgw-realm=%s", objectStore.Name)}, objectStore.Namespace) + require.NoError(t, err) + + // extract api creds from json output + var userInfo map[string]interface{} + err = json.Unmarshal([]byte(output), &userInfo) + require.NoError(t, err) + + s3AccessKey, ok := userInfo["keys"].([]interface{})[0].(map[string]interface{})["access_key"].(string) + require.True(t, ok) + require.NotEmpty(t, s3AccessKey) + + s3SecretKey, ok := userInfo["keys"].([]interface{})[0].(map[string]interface{})["secret_key"].(string) + require.True(t, ok) + require.NotEmpty(t, s3SecretKey) + + // extract rgw endpoint from k8s svc + svc, err := k8sh.Clientset.CoreV1().Services(objectStore.Namespace).Get(ctx, objectStore.Name, metav1.GetOptions{}) + require.NoError(t, err) + + schema := "http://" + httpClient := &http.Client{} + + if tlsEnable { + schema = "https://" + httpClient.Transport = &http.Transport{ + TLSClientConfig: &tls.Config{ + // nolint:gosec // skip TLS verification as this is a test + InsecureSkipVerify: true, + }, + } + } + s3Endpoint := schema + svc.Spec.ClusterIP + ":80" + + logger.Infof("endpoint (%s) Accesskey (%s) secret (%s)", s3Endpoint, s3AccessKey, s3SecretKey) + + adminClient, err = admin.New(s3Endpoint, s3AccessKey, s3SecretKey, httpClient) + require.NoError(t, err) + + // verify that admin api is working + _, err = adminClient.GetInfo(ctx) + require.NoError(t, err) + }) + + { + secrets := []*corev1.Secret{secret1, secret2, secret3} + + checkRgwUserKeys(t, adminClient, osu1, secrets, "AWS_ACCESS_KEY_ID", "AWS_SECRET_ACCESS_KEY") + checkStatusKeys(t, k8sh, osu1, secrets) + } + + t.Run(fmt.Sprintf("update keys on CephObjectStoreUser %q", osu1.Name), func(t *testing.T) { + liveOsu, err := k8sh.RookClientset.CephV1().CephObjectStoreUsers(ns.Name).Get(ctx, osu1.Name, metav1.GetOptions{}) + require.NoError(t, err) + + liveOsu.Spec.Keys = []cephv1.ObjectUserKey{ + { + AccessKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: secret4.Name, + }, + Key: "AWS_ACCESS_KEY_ID", + }, + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: secret4.Name, + }, + Key: "AWS_SECRET_ACCESS_KEY", + }, + }, + { + AccessKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: secret5.Name, + }, + Key: "AWS_ACCESS_KEY_ID", + }, + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: secret5.Name, + }, + Key: "AWS_SECRET_ACCESS_KEY", + }, + }, + } + + _, err = k8sh.RookClientset.CephV1().CephObjectStoreUsers(ns.Name).Update(ctx, liveOsu, metav1.UpdateOptions{}) + require.NoError(t, err) + }) + + { + secrets := []*corev1.Secret{secret4, secret5} + + checkRgwUserKeys(t, adminClient, osu1, secrets, "AWS_ACCESS_KEY_ID", "AWS_SECRET_ACCESS_KEY") + checkStatusKeys(t, k8sh, osu1, secrets) + } + + // test transition from explicit keys -> automatic secret creation + + // when all explicit keys are removed from CephObjectStoreUser, one should + // be left in place and the operator should [still] create a k8s secret for it + t.Run(fmt.Sprintf("remove all keys set on CephObjectStoreUser %q", osu1.Name), func(t *testing.T) { + liveOsu, err := k8sh.RookClientset.CephV1().CephObjectStoreUsers(ns.Name).Get(ctx, osu1.Name, metav1.GetOptions{}) + require.NoError(t, err) + + liveOsu.Spec.Keys = nil + + _, err = k8sh.RookClientset.CephV1().CephObjectStoreUsers(ns.Name).Update(ctx, liveOsu, metav1.UpdateOptions{}) + require.NoError(t, err) + + // wait for the number of keys to drop to 1 + inSync := utils.Retry(40, time.Second, "CephObjectStoreUser has 1 key", func() bool { + liveUser, err := adminClient.GetUser(ctx, admin.User{ID: osu1.Name}) + if err != nil { + return false + } + return len(liveUser.Keys) == 1 + }) + require.True(t, inSync) + }) + + // keys updated on user + { + // fetch automatic secret as it should be the only key set on the rgw user + secretName := generateObjectStoreUserSecretName(osu1) + liveSecret, err := k8sh.Clientset.CoreV1().Secrets(ns.Name).Get(ctx, secretName, metav1.GetOptions{}) + require.NoError(t, err) + + secrets := []*corev1.Secret{liveSecret} + + checkRgwUserKeys(t, adminClient, osu1, secrets, "AccessKey", "SecretKey") + } + + t.Run(fmt.Sprintf("cephObjectStoreUser %q .status.keys is unset", osu1.Name), func(t *testing.T) { + liveOsu, err := k8sh.RookClientset.CephV1().CephObjectStoreUsers(ns.Name).Get(ctx, osu1.Name, metav1.GetOptions{}) + require.NoError(t, err) + + require.NotNil(t, liveOsu.Status) + assert.Len(t, liveOsu.Status.Keys, 0) + }) + + // test transition automatic secret creation -> explicit keys + t.Run(fmt.Sprintf("add keys to CephObjectStoreUser %q", osu1.Name), func(t *testing.T) { + liveOsu, err := k8sh.RookClientset.CephV1().CephObjectStoreUsers(ns.Name).Get(ctx, osu1.Name, metav1.GetOptions{}) + require.NoError(t, err) + + liveOsu.Spec.Keys = []cephv1.ObjectUserKey{ + { + AccessKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: secret1.Name, + }, + Key: "AWS_ACCESS_KEY_ID", + }, + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: secret1.Name, + }, + Key: "AWS_SECRET_ACCESS_KEY", + }, + }, + { + AccessKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: secret2.Name, + }, + Key: "AWS_ACCESS_KEY_ID", + }, + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: secret2.Name, + }, + Key: "AWS_SECRET_ACCESS_KEY", + }, + }, + { + AccessKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: secret3.Name, + }, + Key: "AWS_ACCESS_KEY_ID", + }, + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: secret3.Name, + }, + Key: "AWS_SECRET_ACCESS_KEY", + }, + }, + { + AccessKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: secret4.Name, + }, + Key: "AWS_ACCESS_KEY_ID", + }, + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: secret4.Name, + }, + Key: "AWS_SECRET_ACCESS_KEY", + }, + }, + { + AccessKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: secret5.Name, + }, + Key: "AWS_ACCESS_KEY_ID", + }, + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: secret5.Name, + }, + Key: "AWS_SECRET_ACCESS_KEY", + }, + }, + } + + _, err = k8sh.RookClientset.CephV1().CephObjectStoreUsers(ns.Name).Update(ctx, liveOsu, metav1.UpdateOptions{}) + require.NoError(t, err) + }) + + { + secrets := []*corev1.Secret{secret1, secret2, secret3, secret4, secret5} + + checkRgwUserKeys(t, adminClient, osu1, secrets, "AWS_ACCESS_KEY_ID", "AWS_SECRET_ACCESS_KEY") + checkStatusKeys(t, k8sh, osu1, secrets) + } + + // updating a secret already referenced by a CephObjectStoreUser should trigger a reconcile + t.Run(fmt.Sprintf("update secret %q data", secret1.Name), func(t *testing.T) { + liveSecret, err := k8sh.Clientset.CoreV1().Secrets(secret1.Namespace).Get(ctx, secret1.Name, metav1.GetOptions{}) + require.NoError(t, err) + + liveSecret.Data = map[string][]byte{ + "AWS_ACCESS_KEY_ID": []byte("foo"), + "AWS_SECRET_ACCESS_KEY": []byte("bar"), + } + + _, err = k8sh.Clientset.CoreV1().Secrets(secret1.Namespace).Update(ctx, liveSecret, metav1.UpdateOptions{}) + require.NoError(t, err) + }) + + { + // fetch secret that we modified + liveSecret, err := k8sh.Clientset.CoreV1().Secrets(secret1.Namespace).Get(ctx, secret1.Name, metav1.GetOptions{}) + require.NoError(t, err) + + secrets := []*corev1.Secret{liveSecret, secret2, secret3, secret4, secret5} + + checkRgwUserKeys(t, adminClient, osu1, secrets, "AWS_ACCESS_KEY_ID", "AWS_SECRET_ACCESS_KEY") + checkStatusKeys(t, k8sh, osu1, secrets) + } + + // deleting a secret referenced by a CephObjectStoreUser should trigger a reconcile (which fails) + t.Run(fmt.Sprintf("delete secret %q", secret1.Name), func(t *testing.T) { + err := k8sh.Clientset.CoreV1().Secrets(ns.Name).Delete(ctx, secret1.Name, metav1.DeleteOptions{}) + require.NoError(t, err) + }) + + t.Run(fmt.Sprintf("CephObjectStoreUser %q has phase ReconcileFailed", osu1.Name), func(t *testing.T) { + // user creation may be slow right after rgw start up + osuReady := utils.Retry(40, time.Second, "CephObjectStoreUser is ReconcileFailed", func() bool { + liveOsu, err := k8sh.RookClientset.CephV1().CephObjectStoreUsers(ns.Name).Get(ctx, osu1.Name, metav1.GetOptions{}) + if err != nil { + return false + } + + if liveOsu.Status == nil { + return false + } + + return liveOsu.Status.Phase == string(cephv1.ReconcileFailed) + }) + require.True(t, osuReady) + }) + + // recreate secret to allow CephObjectStoreUser to reconcile so it can be deleted + t.Run(fmt.Sprintf("create secret %q", secret1.Name), func(t *testing.T) { + _, err := k8sh.Clientset.CoreV1().Secrets(ns.Name).Create(ctx, secret1, metav1.CreateOptions{}) + require.NoError(t, err) + }) + + t.Run(fmt.Sprintf("delete CephObjectStoreUser %q", osu1.Name), func(t *testing.T) { + err := k8sh.RookClientset.CephV1().CephObjectStoreUsers(ns.Name).Delete(ctx, osu1.Name, metav1.DeleteOptions{}) + require.NoError(t, err) + + absent := utils.Retry(40, time.Second, "CephObjectStoreUser is absent", func() bool { + _, err := k8sh.RookClientset.CephV1().CephObjectStoreUsers(ns.Name).Get(ctx, osu1.Name, metav1.GetOptions{}) + return err != nil + }) + assert.True(t, absent) + }) + + t.Run(fmt.Sprintf("no CephObjectStoreUsers in ns %q", ns.Name), func(t *testing.T) { + osus, err := k8sh.RookClientset.CephV1().CephObjectStoreUsers(ns.Name).List(ctx, metav1.ListOptions{}) + require.NoError(t, err) + + assert.Len(t, osus.Items, 0) + }) + + t.Run(fmt.Sprintf("user %q does not exist", osu1.Name), func(t *testing.T) { + _, err := adminClient.GetUser(ctx, admin.User{ID: osu1.Name}) + require.ErrorIs(t, err, admin.ErrNoSuchUser) + }) + + // CephObjectStoreUser removal did not delete any secret resources + t.Run(fmt.Sprintf("secret %q still exists", secret2.Name), func(t *testing.T) { + _, err := k8sh.Clientset.CoreV1().Secrets(ns.Name).Get(ctx, secret2.Name, metav1.GetOptions{}) + require.NoError(t, err) + }) + + t.Run(fmt.Sprintf("secret %q still exists", secret3.Name), func(t *testing.T) { + _, err := k8sh.Clientset.CoreV1().Secrets(ns.Name).Get(ctx, secret3.Name, metav1.GetOptions{}) + require.NoError(t, err) + }) + + t.Run(fmt.Sprintf("secret %q still exists", secret4.Name), func(t *testing.T) { + _, err := k8sh.Clientset.CoreV1().Secrets(ns.Name).Get(ctx, secret4.Name, metav1.GetOptions{}) + require.NoError(t, err) + }) + + t.Run(fmt.Sprintf("secret %q still exists", secret5.Name), func(t *testing.T) { + _, err := k8sh.Clientset.CoreV1().Secrets(ns.Name).Get(ctx, secret5.Name, metav1.GetOptions{}) + require.NoError(t, err) + }) + + t.Run(fmt.Sprintf("delete secret %q", secret5.Name), func(t *testing.T) { + err := k8sh.Clientset.CoreV1().Secrets(ns.Name).Delete(ctx, secret5.Name, metav1.DeleteOptions{}) + require.NoError(t, err) + }) + + t.Run(fmt.Sprintf("delete secret %q", secret4.Name), func(t *testing.T) { + err := k8sh.Clientset.CoreV1().Secrets(ns.Name).Delete(ctx, secret4.Name, metav1.DeleteOptions{}) + require.NoError(t, err) + }) + + t.Run(fmt.Sprintf("delete secret %q", secret3.Name), func(t *testing.T) { + err := k8sh.Clientset.CoreV1().Secrets(ns.Name).Delete(ctx, secret3.Name, metav1.DeleteOptions{}) + require.NoError(t, err) + }) + + t.Run(fmt.Sprintf("delete secret %q", secret2.Name), func(t *testing.T) { + err := k8sh.Clientset.CoreV1().Secrets(ns.Name).Delete(ctx, secret2.Name, metav1.DeleteOptions{}) + require.NoError(t, err) + }) + + t.Run(fmt.Sprintf("delete secret %q", secret1.Name), func(t *testing.T) { + err := k8sh.Clientset.CoreV1().Secrets(ns.Name).Delete(ctx, secret1.Name, metav1.DeleteOptions{}) + require.NoError(t, err) + }) + + t.Run(fmt.Sprintf("no secrets in ns %q", ns.Name), func(t *testing.T) { + secrets, err := k8sh.Clientset.CoreV1().Secrets(ns.Name).List(ctx, metav1.ListOptions{}) + require.NoError(t, err) + + assert.Len(t, secrets.Items, 0) + }) + + t.Run(fmt.Sprintf("delete svc %q", objectStoreSvc.Name), func(t *testing.T) { + err := k8sh.Clientset.CoreV1().Services(objectStore.Namespace).Delete(ctx, objectStoreSvc.Name, metav1.DeleteOptions{}) + require.NoError(t, err) + }) + + t.Run(fmt.Sprintf("delete CephObjectStore %q", objectStore.Name), func(t *testing.T) { + err := k8sh.RookClientset.CephV1().CephObjectStores(objectStore.Namespace).Delete(ctx, objectStore.Name, metav1.DeleteOptions{}) + require.NoError(t, err) + }) + + t.Run(fmt.Sprintf("delete ns %q", ns.Name), func(t *testing.T) { + err := k8sh.Clientset.CoreV1().Namespaces().Delete(ctx, ns.Name, metav1.DeleteOptions{}) + require.NoError(t, err) + }) + }) +}