From 9442b0e93b86c6e1c8d30e82e587fc315e84e67c Mon Sep 17 00:00:00 2001 From: Bohdan Siryk Date: Wed, 8 Nov 2023 14:51:19 +0200 Subject: [PATCH] issue-607, cassandra user creation and deletion logic extracted from cassandra controller predicates --- apis/clusters/v1beta1/cassandra_types.go | 5 +- apis/clusters/v1beta1/structs.go | 37 ++++ apis/clusters/v1beta1/structs_test.go | 187 ++++++++++++++++++ .../clusters/v1beta1/zz_generated.deepcopy.go | 38 +++- .../clusters.instaclustr.com_cassandras.yaml | 12 ++ ...lusterresources_v1beta1_cassandrauser.yaml | 37 ++++ .../samples/clusters_v1beta1_cassandra.yaml | 14 +- controllers/clusters/cassandra_controller.go | 120 ++++++----- .../tests/cassandra_plus_users_test.go | 8 +- 9 files changed, 387 insertions(+), 71 deletions(-) create mode 100644 apis/clusters/v1beta1/structs_test.go diff --git a/apis/clusters/v1beta1/cassandra_types.go b/apis/clusters/v1beta1/cassandra_types.go index 71fdf78dc..f7dcd8dfc 100644 --- a/apis/clusters/v1beta1/cassandra_types.go +++ b/apis/clusters/v1beta1/cassandra_types.go @@ -62,14 +62,15 @@ type CassandraSpec struct { PasswordAndUserAuth bool `json:"passwordAndUserAuth,omitempty"` Spark []*Spark `json:"spark,omitempty"` BundledUseOnly bool `json:"bundledUseOnly,omitempty"` - UserRefs []*UserReference `json:"userRefs,omitempty"` + UserRefs UserReferences `json:"userRefs,omitempty"` //+kubebuilder:validate:MaxItems:=1 ResizeSettings []*ResizeSettings `json:"resizeSettings,omitempty"` } // CassandraStatus defines the observed state of Cassandra type CassandraStatus struct { - ClusterStatus `json:",inline"` + ClusterStatus `json:",inline"` + AvailableUsers UserReferences `json:"availableUsers,omitempty"` } type CassandraDataCentre struct { diff --git a/apis/clusters/v1beta1/structs.go b/apis/clusters/v1beta1/structs.go index 1ee503af5..19b331311 100644 --- a/apis/clusters/v1beta1/structs.go +++ b/apis/clusters/v1beta1/structs.go @@ -715,3 +715,40 @@ type UserReference struct { Namespace string `json:"namespace"` Name string `json:"name"` } + +type UserReferences []*UserReference + +// Diff returns difference between two UserReferences. +// added stores elements which are presented in new UserReferences, but aren't presented in old. +// deleted stores elements which aren't presented in new UserReferences, but are presented in old. +func (old UserReferences) Diff(new UserReferences) (added, deleted UserReferences) { + // filtering deleted references + for _, oldRef := range old { + var exists bool + for _, newRef := range new { + if *oldRef == *newRef { + exists = true + } + } + + if !exists { + deleted = append(deleted, oldRef) + } + } + + // filtering added references + for _, newRef := range new { + var exists bool + for _, oldRef := range old { + if *newRef == *oldRef { + exists = true + } + } + + if !exists { + added = append(added, newRef) + } + } + + return added, deleted +} diff --git a/apis/clusters/v1beta1/structs_test.go b/apis/clusters/v1beta1/structs_test.go new file mode 100644 index 000000000..e0a932043 --- /dev/null +++ b/apis/clusters/v1beta1/structs_test.go @@ -0,0 +1,187 @@ +package v1beta1_test + +import ( + "encoding/json" + "reflect" + "testing" + + "github.com/instaclustr/operator/apis/clusters/v1beta1" +) + +func TestUserReferencesDiff(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + old v1beta1.UserReferences + new v1beta1.UserReferences + expectedAdded v1beta1.UserReferences + expectedDeleted v1beta1.UserReferences + }{ + { + name: "nothing changed", + old: v1beta1.UserReferences{ + { + Name: "name1", + Namespace: "namespace1", + }, + }, + new: v1beta1.UserReferences{ + { + Name: "name1", + Namespace: "namespace1", + }, + }, + }, + { + name: "added new reference", + old: v1beta1.UserReferences{ + { + Name: "name1", + Namespace: "namespace1", + }, + }, + new: v1beta1.UserReferences{ + { + Name: "name1", + Namespace: "namespace1", + }, + { + Name: "name2", + Namespace: "namespace2", + }, + }, + expectedAdded: v1beta1.UserReferences{ + { + Name: "name2", + Namespace: "namespace2", + }, + }, + }, + { + name: "deleted old reference", + old: v1beta1.UserReferences{ + { + Name: "name1", + Namespace: "namespace1", + }, + { + Name: "name2", + Namespace: "namespace2", + }, + }, + new: v1beta1.UserReferences{ + { + Name: "name1", + Namespace: "namespace1", + }, + }, + expectedDeleted: v1beta1.UserReferences{ + { + Name: "name2", + Namespace: "namespace2", + }, + }, + }, + { + name: "both slices are nil", + old: nil, + new: nil, + }, + { + name: "deleting the first out of 3", + old: v1beta1.UserReferences{ + { + Name: "name1", + Namespace: "namespace1", + }, + { + Name: "name2", + Namespace: "namespace2", + }, + { + Name: "name3", + Namespace: "namespace3", + }, + }, + new: v1beta1.UserReferences{ + { + Name: "name2", + Namespace: "namespace2", + }, + { + Name: "name3", + Namespace: "namespace3", + }, + }, + expectedDeleted: v1beta1.UserReferences{ + { + Name: "name1", + Namespace: "namespace1", + }, + }, + }, + { + name: "deleting the first and adding a new one", + old: v1beta1.UserReferences{ + { + Name: "name1", + Namespace: "namespace1", + }, + { + Name: "name2", + Namespace: "namespace2", + }, + { + Name: "name3", + Namespace: "namespace3", + }, + }, + new: v1beta1.UserReferences{ + { + Name: "name2", + Namespace: "namespace2", + }, + { + Name: "name3", + Namespace: "namespace3", + }, + { + Name: "name4", + Namespace: "namespace4", + }, + }, + expectedDeleted: v1beta1.UserReferences{ + { + Name: "name1", + Namespace: "namespace1", + }, + }, + expectedAdded: v1beta1.UserReferences{ + { + Name: "name4", + Namespace: "namespace4", + }, + }, + }, + } + + for _, c := range cases { + c := c + t.Run(c.name, func(t *testing.T) { + added, deleted := c.old.Diff(c.new) + + if !reflect.DeepEqual(added, c.expectedAdded) || !reflect.DeepEqual(deleted, c.expectedDeleted) { + t.Errorf("expected added %s, got %s; expected deleted %s, got %s", + toJson(c.expectedAdded), toJson(added), + toJson(c.expectedDeleted), toJson(deleted), + ) + } + }) + } +} + +func toJson(obj any) string { + b, _ := json.Marshal(obj) + return string(b) +} diff --git a/apis/clusters/v1beta1/zz_generated.deepcopy.go b/apis/clusters/v1beta1/zz_generated.deepcopy.go index 8346d5ca2..3ac81cafb 100644 --- a/apis/clusters/v1beta1/zz_generated.deepcopy.go +++ b/apis/clusters/v1beta1/zz_generated.deepcopy.go @@ -482,7 +482,7 @@ func (in *CassandraSpec) DeepCopyInto(out *CassandraSpec) { } if in.UserRefs != nil { in, out := &in.UserRefs, &out.UserRefs - *out = make([]*UserReference, len(*in)) + *out = make(UserReferences, len(*in)) for i := range *in { if (*in)[i] != nil { in, out := &(*in)[i], &(*out)[i] @@ -518,6 +518,17 @@ func (in *CassandraSpec) DeepCopy() *CassandraSpec { func (in *CassandraStatus) DeepCopyInto(out *CassandraStatus) { *out = *in in.ClusterStatus.DeepCopyInto(&out.ClusterStatus) + if in.AvailableUsers != nil { + in, out := &in.AvailableUsers, &out.AvailableUsers + *out = make(UserReferences, len(*in)) + for i := range *in { + if (*in)[i] != nil { + in, out := &(*in)[i], &(*out)[i] + *out = new(UserReference) + **out = **in + } + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new CassandraStatus. @@ -2436,6 +2447,31 @@ func (in *UserReference) DeepCopy() *UserReference { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in UserReferences) DeepCopyInto(out *UserReferences) { + { + in := &in + *out = make(UserReferences, len(*in)) + for i := range *in { + if (*in)[i] != nil { + in, out := &(*in)[i], &(*out)[i] + *out = new(UserReference) + **out = **in + } + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new UserReferences. +func (in UserReferences) DeepCopy() UserReferences { + if in == nil { + return nil + } + out := new(UserReferences) + in.DeepCopyInto(out) + return *out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Zookeeper) DeepCopyInto(out *Zookeeper) { *out = *in diff --git a/config/crd/bases/clusters.instaclustr.com_cassandras.yaml b/config/crd/bases/clusters.instaclustr.com_cassandras.yaml index 862c6bcee..b92d77a37 100644 --- a/config/crd/bases/clusters.instaclustr.com_cassandras.yaml +++ b/config/crd/bases/clusters.instaclustr.com_cassandras.yaml @@ -224,6 +224,18 @@ spec: status: description: CassandraStatus defines the observed state of Cassandra properties: + availableUsers: + items: + properties: + name: + type: string + namespace: + type: string + required: + - name + - namespace + type: object + type: array cdcid: type: string currentClusterOperationStatus: diff --git a/config/samples/clusterresources_v1beta1_cassandrauser.yaml b/config/samples/clusterresources_v1beta1_cassandrauser.yaml index f12a6fd0a..cc800df17 100644 --- a/config/samples/clusterresources_v1beta1_cassandrauser.yaml +++ b/config/samples/clusterresources_v1beta1_cassandrauser.yaml @@ -15,3 +15,40 @@ spec: secretRef: name: "cassandra-user-secret" namespace: "default" +--- +apiVersion: v1 +kind: Secret +metadata: + name: cassandra-user-secret2 +data: + password: NDgxMzU5ODM1NzlmMDU0ZTlhY2I4ZjcxMTMzMzQ1MjM3ZQ== + username: b2xvbG8xCg== +--- + +apiVersion: clusterresources.instaclustr.com/v1beta1 +kind: CassandraUser +metadata: + name: cassandrauser-sample2 +spec: + secretRef: + name: "cassandra-user-secret2" + namespace: "default" +--- +apiVersion: v1 +kind: Secret +metadata: + name: cassandra-user-secret3 +data: + password: NDgxMzU5ODM1NzlmMDU0ZTlhY2I4ZjcxMTMzMzQ1MjM3ZQ== + username: b2xvbG8yCg== +--- + +apiVersion: clusterresources.instaclustr.com/v1beta1 +kind: CassandraUser +metadata: + name: cassandrauser-sample3 +spec: + secretRef: + name: "cassandra-user-secret3" + namespace: "default" +--- \ No newline at end of file diff --git a/config/samples/clusters_v1beta1_cassandra.yaml b/config/samples/clusters_v1beta1_cassandra.yaml index d1e9be089..e28ad3008 100644 --- a/config/samples/clusters_v1beta1_cassandra.yaml +++ b/config/samples/clusters_v1beta1_cassandra.yaml @@ -3,7 +3,7 @@ kind: Cassandra metadata: name: cassandra-cluster spec: - name: "username-Cassandra" + name: "bohdan-Cassandra" version: "4.0.10" privateNetworkCluster: false dataCentres: @@ -37,11 +37,13 @@ spec: pciCompliance: false luceneEnabled: false # can be enabled only on 3.11.13 version of Cassandra passwordAndUserAuth: true -# userRefs: -# - namespace: default -# name: cassandrauser-sample -# - namespace: default -# name: cassandrauser-sample2 + userRefs: + - namespace: default + name: cassandrauser-sample + - namespace: default + name: cassandrauser-sample2 + - namespace: default + name: cassandrauser-sample3 slaTier: "NON_PRODUCTION" # resizeSettings: # - notifySupportContacts: false diff --git a/controllers/clusters/cassandra_controller.go b/controllers/clusters/cassandra_controller.go index de2036fe4..7d5f05482 100644 --- a/controllers/clusters/cassandra_controller.go +++ b/controllers/clusters/cassandra_controller.go @@ -261,7 +261,7 @@ func (r *CassandraReconciler) handleCreateCluster( "Cluster backups check job is started", ) - if cassandra.Spec.UserRefs != nil { + if cassandra.Spec.UserRefs != nil && cassandra.Status.AvailableUsers == nil { err = r.startUsersCreationJob(cassandra) if err != nil { l.Error(err, "Failed to start user creation job") @@ -385,6 +385,34 @@ func (r *CassandraReconciler) handleUpdateCluster( } } + addedRefs, deletedRefs := cassandra.Status.AvailableUsers.Diff(cassandra.Spec.UserRefs) + + for _, ref := range addedRefs { + err = r.handleUsersCreate(ctx, l, cassandra, ref) + if err != nil { + return reconcile.Result{}, err + } + } + + for _, ref := range deletedRefs { + err = r.handleUsersDelete(ctx, l, cassandra, ref) + if err != nil { + return reconcile.Result{}, err + } + } + + if addedRefs != nil || deletedRefs != nil { + cassandra.Status.AvailableUsers = cassandra.Spec.UserRefs + err = r.Status().Patch(ctx, cassandra, patch) + if err != nil { + l.Error(err, "Failed to patch cluster status with created user") + r.EventRecorder.Eventf(cassandra, models.Warning, models.CreationFailed, + "Failed to create a user for the cluster. Reason: %v", err) + + return reconcile.Result{}, err + } + } + cassandra.Annotations[models.ResourceStateAnnotation] = models.UpdatedEvent cassandra.Annotations[models.UpdateQueuedAnnotation] = "" err = r.Patch(ctx, cassandra, patch) @@ -717,6 +745,23 @@ func (r *CassandraReconciler) handleUsersDelete( return err } + patch = c.NewPatch() + + for i, ref := range c.Status.AvailableUsers { + if *ref == *uRef { + c.Status.AvailableUsers = append(c.Status.AvailableUsers[:i], c.Status.AvailableUsers[i+1:]...) + break + } + } + + err = r.Status().Patch(ctx, c, patch) + if err != nil { + l.Error(err, "Failed to patch cluster status with created user", "user ref", uRef) + r.EventRecorder.Eventf(c, models.Warning, models.CreationFailed, + "Failed to create a user for the cluster. Reason: %v", err) + return err + } + l.Info("User has been added to the queue for deletion", "User resource", u.Namespace+"/"+u.Name, "Cassandra resource", c.Namespace+"/"+c.Name) @@ -774,62 +819,6 @@ func (r *CassandraReconciler) handleUsersDetach( return nil } -func (r *CassandraReconciler) handleUserEvent( - newObj *v1beta1.Cassandra, - oldUsers []*v1beta1.UserReference, -) { - ctx := context.TODO() - l := log.FromContext(ctx) - - for _, newUser := range newObj.Spec.UserRefs { - var exist bool - - for _, oldUser := range oldUsers { - - if *newUser == *oldUser { - exist = true - break - } - } - - if exist { - continue - } - - err := r.handleUsersCreate(ctx, l, newObj, newUser) - if err != nil { - l.Error(err, "Cannot create Cassandra user in predicate", "user", newUser) - r.EventRecorder.Eventf(newObj, models.Warning, models.CreatingEvent, - "Cannot create user. Reason: %v", err) - } - - oldUsers = append(oldUsers, newUser) - } - - for _, oldUser := range oldUsers { - var exist bool - - for _, newUser := range newObj.Spec.UserRefs { - - if *oldUser == *newUser { - exist = true - break - } - } - - if exist { - continue - } - - err := r.handleUsersDelete(ctx, l, newObj, oldUser) - if err != nil { - l.Error(err, "Cannot delete Cassandra user", "user", oldUser) - r.EventRecorder.Eventf(newObj, models.Warning, models.CreatingEvent, - "Cannot delete user from cluster. Reason: %v", err) - } - } -} - func (r *CassandraReconciler) startClusterStatusJob(cassandraCluster *v1beta1.Cassandra) error { job := r.newWatchStatusJob(cassandraCluster) @@ -1145,6 +1134,19 @@ func (r *CassandraReconciler) newUsersCreationJob(c *v1beta1.Cassandra) schedule } } + if c.Spec.UserRefs != nil { + patch := c.NewPatch() + c.Status.AvailableUsers = c.Spec.UserRefs + err = r.Status().Patch(ctx, c, patch) + if err != nil { + l.Error(err, "Failed to patch cluster status with created user") + r.EventRecorder.Eventf(c, models.Warning, models.CreationFailed, + "Failed to create a user for the cluster. Reason: %v", err) + + return err + } + } + l.Info("User creation job successfully finished", "resource name", c.Name) r.EventRecorder.Eventf(c, models.Normal, models.Created, "User creation job successfully finished") @@ -1278,10 +1280,6 @@ func (r *CassandraReconciler) SetupWithManager(mgr ctrl.Manager) error { return false } - oldObj := event.ObjectOld.(*v1beta1.Cassandra) - - r.handleUserEvent(newObj, oldObj.Spec.UserRefs) - newObj.Annotations[models.ResourceStateAnnotation] = models.UpdatingEvent return true }, diff --git a/controllers/tests/cassandra_plus_users_test.go b/controllers/tests/cassandra_plus_users_test.go index 816d41974..53dc111f6 100644 --- a/controllers/tests/cassandra_plus_users_test.go +++ b/controllers/tests/cassandra_plus_users_test.go @@ -147,6 +147,8 @@ var _ = Describe("Basic Cassandra User controller + Basic Cassandra cluster cont Name: userManifest1.Name, }} + doneCh := NewChannelWithTimeout(timeout) + Expect(k8sClient.Get(ctx, cassandraNamespacedName1, &cassandra1)).Should(Succeed()) patch := cassandra1.NewPatch() @@ -166,6 +168,7 @@ var _ = Describe("Basic Cassandra User controller + Basic Cassandra cluster cont return true }, timeout, interval).Should(BeTrue()) + <-doneCh }) }) @@ -177,7 +180,9 @@ var _ = Describe("Basic Cassandra User controller + Basic Cassandra cluster cont // removing user cassandra1.Spec.UserRefs = []*v1beta1.UserReference{} Expect(k8sClient.Patch(ctx, &cassandra1, patch)).Should(Succeed()) - By("going to Cassandra(cluster) controller predicate and put user entity to deletion state. " + + + done := NewChannelWithTimeout(timeout) + By("going to Cassandra(cluster) controller and put user entity to deletion state. " + "Finally deletes the user for the corresponded cluster") Eventually(func() bool { if err := k8sClient.Get(ctx, userNamespacedName1, &user1); err != nil { @@ -190,6 +195,7 @@ var _ = Describe("Basic Cassandra User controller + Basic Cassandra cluster cont return true }, timeout, interval).Should(BeTrue()) + <-done }) })