From 86c65c99283066e922885239d1caa815428ecb3e Mon Sep 17 00:00:00 2001 From: Tanmay Jain Date: Wed, 25 Jun 2025 23:37:22 +0530 Subject: [PATCH 01/13] Inital changes of full solution to disable security --- internal/controller/cluster/access_control.go | 45 ++-- internal/controller/cluster/rack.go | 45 ++-- internal/controller/cluster/reconciler.go | 33 +-- .../v1/aerospikecluster_validating_webhook.go | 24 -- pkg/validation/validate.go | 26 -- test/cluster/access_control_test.go | 234 +++++++++++++++--- test/cluster/utils.go | 5 +- 7 files changed, 271 insertions(+), 141 deletions(-) diff --git a/internal/controller/cluster/access_control.go b/internal/controller/cluster/access_control.go index 19440f8b..917fb803 100644 --- a/internal/controller/cluster/access_control.go +++ b/internal/controller/cluster/access_control.go @@ -37,29 +37,6 @@ func AerospikeAdminCredentials( desiredState, currentState *asdbv1.AerospikeClusterSpec, passwordProvider AerospikeUserPasswordProvider, ) (user, pass string, err error) { - var ( - enabled bool - currentSecurityErr error - desiredSecurityErr error - ) - - enabled, desiredSecurityErr = asdbv1.IsSecurityEnabled(desiredState.AerospikeConfig.Value) - if !enabled { - if currentState.AerospikeConfig != nil { - // It is possible that this is a new cluster and the current state is empty. - enabled, currentSecurityErr = asdbv1.IsSecurityEnabled(currentState.AerospikeConfig.Value) - } - - if currentSecurityErr != nil && desiredSecurityErr != nil { - return "", "", desiredSecurityErr - } - } - - if !enabled { - // Return zero strings if this is not a security enabled cluster. - return "", "", nil - } - if currentState.AerospikeAccessControl == nil { // We haven't yet set up access control. Use default password. return asdbv1.AdminUsername, passwordProvider.GetDefaultPassword(desiredState), nil @@ -190,6 +167,13 @@ func (r *SingleClusterReconciler) reconcileUsers( currentUserNames = append(currentUserNames, userName) } + if len(desired) == 0 { + // If no users are desired, do not drop admin user. + desired[asdbv1.AdminUsername] = asdbv1.AerospikeUserSpec{ + Roles: []string{"user-admin", "sys-admin"}, + } + } + // List users needed in the desired list. requiredUserNames := make([]string, 0, len(desired)) for userName := range desired { @@ -213,9 +197,18 @@ func (r *SingleClusterReconciler) reconcileUsers( for userName := range desired { userSpec := desired[userName] - password, err := passwordProvider.Get(userName, &userSpec) - if err != nil { - return err + var ( + password string + err error + ) + + if userName == asdbv1.AdminUsername && userSpec.SecretName == "" { + password = passwordProvider.GetDefaultPassword(&r.aeroCluster.Spec) + } else { + password, err = passwordProvider.Get(userName, &userSpec) + if err != nil { + return err + } } cmd := aerospikeUserCreateUpdate{ diff --git a/internal/controller/cluster/rack.go b/internal/controller/cluster/rack.go index 20f6ae1f..5a88ae55 100644 --- a/internal/controller/cluster/rack.go +++ b/internal/controller/cluster/rack.go @@ -390,7 +390,7 @@ func (r *SingleClusterReconciler) upgradeOrRollingRestartRack( // This code will run only when security is being enabled in an existing cluster // Update for security is verified by checking the config hash of the pod with the // config hash present in config map - if err := r.handleEnableSecurity(rackState, ignorablePodNames); err != nil { + if err := r.handleClusterSecurity(rackState, ignorablePodNames); err != nil { return found, common.ReconcileError(err) } @@ -1775,14 +1775,15 @@ func (r *SingleClusterReconciler) getCurrentRackList() ( return rackList, nil } -func (r *SingleClusterReconciler) handleEnableSecurity(rackState *RackState, ignorablePodNames sets.Set[string]) error { - if !r.enablingSecurity() { +func (r *SingleClusterReconciler) handleClusterSecurity(rackState *RackState, + ignorablePodNames sets.Set[string]) error { + if !r.needACLReconcile() { // No need to proceed if security is not to be enabling return nil } // Get pods where security-enabled config is applied - securityEnabledPods, err := r.getPodsWithUpdatedConfigForRack(rackState) + securityEnabledPods, err := r.getPodsWithEnabledSecurityForRack(rackState) if err != nil { return err } @@ -1807,11 +1808,14 @@ func (r *SingleClusterReconciler) handleEnableSecurity(rackState *RackState, ign return nil } -func (r *SingleClusterReconciler) enablingSecurity() bool { - return r.aeroCluster.Spec.AerospikeAccessControl != nil && r.aeroCluster.Status.AerospikeAccessControl == nil +func (r *SingleClusterReconciler) needACLReconcile() bool { + specEnabled := r.aeroCluster.Spec.AerospikeAccessControl == nil + statusEnabled := r.aeroCluster.Status.AerospikeAccessControl == nil + + return specEnabled || statusEnabled } -func (r *SingleClusterReconciler) getPodsWithUpdatedConfigForRack(rackState *RackState) ([]corev1.Pod, error) { +func (r *SingleClusterReconciler) getPodsWithEnabledSecurityForRack(rackState *RackState) ([]corev1.Pod, error) { pods, err := r.getOrderedRackPodList(rackState.Rack.ID) if err != nil { return nil, fmt.Errorf("failed to list pods: %v", err) @@ -1822,26 +1826,27 @@ func (r *SingleClusterReconciler) getPodsWithUpdatedConfigForRack(rackState *Rac return nil, nil } - confMap, err := r.getConfigMap(rackState.Rack.ID) - if err != nil { - return nil, err - } + securityEnabledPods := make([]corev1.Pod, 0, len(pods)) - requiredConfHash := confMap.Data[aerospikeConfHashFileName] + for idx := range pods { + statusFromAnnotation := pods[idx].Annotations["aerospikeConf"] - updatedPods := make([]corev1.Pod, 0, len(pods)) + asConfStatus, err := getFlatConfig(r.Log, statusFromAnnotation) + if err != nil { + return nil, fmt.Errorf("failed to load config map by lib: %v", err) + } - for idx := range pods { - podName := pods[idx].Name - podStatus := r.aeroCluster.Status.Pods[podName] + enabled, err := asdbv1.IsSecurityEnabled(*asConfStatus) + if err != nil { + return nil, fmt.Errorf("failed to get cluster security status: %v", err) + } - if podStatus.AerospikeConfigHash == requiredConfHash { - // Config hash is matching, it means config has been applied - updatedPods = append(updatedPods, *pods[idx]) + if enabled { + securityEnabledPods = append(securityEnabledPods, *pods[idx]) } } - return updatedPods, nil + return securityEnabledPods, nil } func isContainerNameInStorageVolumeAttachments( diff --git a/internal/controller/cluster/reconciler.go b/internal/controller/cluster/reconciler.go index b16f327f..17277ce8 100644 --- a/internal/controller/cluster/reconciler.go +++ b/internal/controller/cluster/reconciler.go @@ -331,20 +331,25 @@ func (r *SingleClusterReconciler) validateAndReconcileAccessControl( selectedPods []corev1.Pod, ignorablePodNames sets.Set[string], ) error { - enabled, err := asdbv1.IsSecurityEnabled(r.aeroCluster.Spec.AerospikeConfig.Value) - if err != nil { - return fmt.Errorf("failed to get cluster security status: %v", err) - } - - if !enabled { - r.Log.Info("Cluster is not security enabled, please enable security for this cluster.") - return nil - } - - var conns []*deployment.HostConn + var ( + conns []*deployment.HostConn + err error + ) // Create client if selectedPods == nil { + var enabled bool + + enabled, err = asdbv1.IsSecurityEnabled(r.aeroCluster.Spec.AerospikeConfig.Value) + if err != nil { + return fmt.Errorf("failed to get cluster security status: %v", err) + } + + if !enabled { + r.Log.Info("Cluster is not security enabled, please enable security for this cluster.") + return nil + } + conns, err = r.newAllHostConnWithOption(ignorablePodNames) if err != nil { return fmt.Errorf("failed to get host info: %v", err) @@ -356,6 +361,8 @@ func (r *SingleClusterReconciler) validateAndReconcileAccessControl( } } + r.Log.Info("Reconcile access control for AerospikeCluster, created conn") + hosts := make([]*as.Host, 0, len(conns)) for _, conn := range conns { @@ -512,10 +519,6 @@ func (r *SingleClusterReconciler) getClusterReadinessStatus() (bool, error) { } func (r *SingleClusterReconciler) updateAccessControlStatus() error { - if r.aeroCluster.Spec.AerospikeAccessControl == nil { - return nil - } - r.Log.Info("Update access control status for AerospikeCluster") // Get the old object, it may have been updated in between. diff --git a/internal/webhook/v1/aerospikecluster_validating_webhook.go b/internal/webhook/v1/aerospikecluster_validating_webhook.go index 41a03435..a05f45e6 100644 --- a/internal/webhook/v1/aerospikecluster_validating_webhook.go +++ b/internal/webhook/v1/aerospikecluster_validating_webhook.go @@ -926,26 +926,6 @@ func validateNamespaceConfig( return nil } -func validateSecurityConfigUpdateFromStatus(newSpec, currentStatus *asdbv1.AerospikeConfigSpec) error { - if currentStatus != nil { - currentSecurityEnabled, err := asdbv1.IsSecurityEnabled(currentStatus.Value) - if err != nil { - return err - } - - desiredSecurityEnabled, err := asdbv1.IsSecurityEnabled(newSpec.Value) - if err != nil { - return err - } - - if currentSecurityEnabled && !desiredSecurityEnabled { - return fmt.Errorf("cannot disable cluster security in running cluster") - } - } - - return nil -} - func validateAerospikeConfigUpdate( aslog logr.Logger, incomingSpec, outgoingSpec, currentStatus *asdbv1.AerospikeConfigSpec, @@ -959,10 +939,6 @@ func validateAerospikeConfigUpdate( return err } - if err := validateSecurityConfigUpdateFromStatus(incomingSpec, currentStatus); err != nil { - return err - } - return validateNsConfUpdateFromStatus(incomingSpec, currentStatus) } diff --git a/pkg/validation/validate.go b/pkg/validation/validate.go index 871866e3..a9fa0488 100644 --- a/pkg/validation/validate.go +++ b/pkg/validation/validate.go @@ -631,28 +631,6 @@ func isValueUpdated(m1, m2 map[string]interface{}, key string) bool { return !reflect.DeepEqual(val1, val2) } -func validateSecurityConfigUpdate(oldConfig, newConfig map[string]interface{}) error { - ovflag, err := asdbv1.IsSecurityEnabled(oldConfig) - if err != nil { - return fmt.Errorf( - "failed to validate Security context of old aerospike conf: %w", err, - ) - } - - ivflag, err := asdbv1.IsSecurityEnabled(newConfig) - if err != nil { - return fmt.Errorf( - "failed to validate Security context of new aerospike conf: %w", err, - ) - } - - if !ivflag && ovflag { - return fmt.Errorf("cannot disable cluster security in running cluster") - } - - return nil -} - // ValidateAerospikeConfigUpdate validates the update of aerospikeConfig. // It validates the schema, security, tls, network and namespace configurations for the newConfig // It also validates the update of security, tls, network and namespace configurations. @@ -672,10 +650,6 @@ func ValidateAerospikeConfigUpdate( // ValidateAerospikeConfigUpdateWithoutSchema validates the update of aerospikeConfig except for the schema. // It validates the update of security, tls, network and namespace configurations. func ValidateAerospikeConfigUpdateWithoutSchema(oldConfig, newConfig map[string]interface{}) error { - if err := validateSecurityConfigUpdate(oldConfig, newConfig); err != nil { - return err - } - if err := validateTLSUpdate(oldConfig, newConfig); err != nil { return err } diff --git a/test/cluster/access_control_test.go b/test/cluster/access_control_test.go index ffef9b84..f95a83cc 100644 --- a/test/cluster/access_control_test.go +++ b/test/cluster/access_control_test.go @@ -16,6 +16,7 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/utils/ptr" as "github.com/aerospike/aerospike-client-go/v8" @@ -25,7 +26,7 @@ import ( ) const ( - testClusterSize = 2 + testClusterSize = 4 ) var aerospikeConfigWithSecurity = &asdbv1.AerospikeConfigSpec{ @@ -1492,7 +1493,7 @@ var _ = Describe( ) }, ) - Context( + FContext( "When cluster is deployed", func() { clusterName := fmt.Sprintf("ac-lifecycle-%d", GinkgoParallelProcess()) clusterNamespacedName := test.GetNamespacedName( @@ -1600,6 +1601,194 @@ var _ = Describe( }, ) + It( + "SecurityDisable: should disable security in running cluster", + func() { + accessControl := &asdbv1.AerospikeAccessControlSpec{ + Roles: []asdbv1.AerospikeRoleSpec{ + { + Name: "profiler", + Privileges: []string{ + "read-write-udf.test.users", + "write", + }, + Whitelist: []string{ + "8.8.0.0/16", + }, + }, + }, + Users: []asdbv1.AerospikeUserSpec{ + { + Name: "admin", + SecretName: test.AuthSecretNameForUpdate, + Roles: []string{ + "sys-admin", + "user-admin", + }, + }, + + { + Name: "profileUser", + SecretName: test.AuthSecretNameForUpdate, + Roles: []string{ + "data-admin", + "read-write-udf", + "write", + }, + }, + }, + } + + aerospikeConfigSpec, err := NewAerospikeConfSpec(latestImage) + if err != nil { + Fail( + fmt.Sprintf( + "Invalid Aerospike Config Spec: %v", + err, + ), + ) + } + + aerospikeConfigSpec.setEnableSecurity(true) + + aeroCluster := getAerospikeClusterSpecWithAccessControl( + clusterNamespacedName, accessControl, + aerospikeConfigSpec, + ) + err = testAccessControlReconcile( + aeroCluster, ctx, + ) + if err != nil { + Fail("Security should be enabled") + } + + aerospikeConfigSpec, err = NewAerospikeConfSpec(latestImage) + if err != nil { + Fail( + fmt.Sprintf( + "Invalid Aerospike Config Spec: %v", + err, + ), + ) + } + + aerospikeConfigSpec.setEnableSecurity(false) + accessControl = nil + + // Save cluster variable as well for cleanup. + aeroCluster = getAerospikeClusterSpecWithAccessControl( + clusterNamespacedName, accessControl, + aerospikeConfigSpec, + ) + err = testAccessControlReconcile( + aeroCluster, ctx, + ) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).Should(ContainSubstring("SECURITY_NOT_ENABLED")) + }, + ) + + It( + "SecurityDisable: should disable security in partially security enabled cluster", + func() { + var accessControl *asdbv1.AerospikeAccessControlSpec + + aerospikeConfigSpec, err := NewAerospikeConfSpec(latestImage) + if err != nil { + Fail( + fmt.Sprintf( + "Invalid Aerospike Config Spec: %v", + err, + ), + ) + } + + aerospikeConfigSpec.setEnableSecurity(false) + + // Save cluster variable as well for cleanup. + aeroCluster := getAerospikeClusterSpecWithAccessControl( + clusterNamespacedName, accessControl, + aerospikeConfigSpec, + ) + err = aerospikeClusterCreateUpdate( + k8sClient, aeroCluster, ctx, + ) + Expect(err).ToNot(HaveOccurred()) + + accessControl = &asdbv1.AerospikeAccessControlSpec{ + Roles: []asdbv1.AerospikeRoleSpec{ + { + Name: "profiler", + Privileges: []string{ + "read-write-udf.test.users", + "write", + }, + Whitelist: []string{ + "8.8.0.0/16", + }, + }, + }, + Users: []asdbv1.AerospikeUserSpec{ + { + Name: "admin", + SecretName: test.AuthSecretNameForUpdate, + Roles: []string{ + "sys-admin", + "user-admin", + }, + }, + + { + Name: "profileUser", + SecretName: test.AuthSecretNameForUpdate, + Roles: []string{ + "data-admin", + "read-write-udf", + "write", + }, + }, + }, + } + + aerospikeConfigSpec, err = NewAerospikeConfSpec(latestImage) + if err != nil { + Fail( + fmt.Sprintf( + "Invalid Aerospike Config Spec: %v", + err, + ), + ) + } + + aerospikeConfigSpec.setEnableSecurity(true) + + aeroCluster = getAerospikeClusterSpecWithAccessControl( + clusterNamespacedName, accessControl, + aerospikeConfigSpec, + ) + + err = updateClusterWithNoWait(k8sClient, ctx, aeroCluster) + Expect(err).ToNot(HaveOccurred()) + + time.Sleep(time.Second * 3) + + aerospikeConfigSpec.setEnableSecurity(false) + accessControl = nil + + // Save cluster variable as well for cleanup. + aeroCluster = getAerospikeClusterSpecWithAccessControl( + clusterNamespacedName, accessControl, + aerospikeConfigSpec, + ) + + err = testAccessControlReconcile( + aeroCluster, ctx, + ) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).Should(ContainSubstring("SECURITY_NOT_ENABLED")) + }, + ) + It( "AccessControlLifeCycle", func() { @@ -1718,33 +1907,6 @@ var _ = Describe( ) Expect(err).ToNot(HaveOccurred()) - By("SecurityUpdateReject") - aerospikeConfigSpec, err = NewAerospikeConfSpec(latestImage) - if err != nil { - Fail( - fmt.Sprintf( - "Invalid Aerospike Config Spec: %v", - err, - ), - ) - } - - aerospikeConfigSpec.setEnableSecurity(false) - - aeroCluster = getAerospikeClusterSpecWithAccessControl( - clusterNamespacedName, nil, - aerospikeConfigSpec, - ) - err = testAccessControlReconcile( - aeroCluster, ctx, - ) - if err == nil || !strings.Contains( - err.Error(), - "cannot disable cluster security in running cluster", - ) { - Fail("SecurityUpdate should have failed") - } - By("EnableQuota") accessControl = asdbv1.AerospikeAccessControlSpec{ @@ -2140,6 +2302,14 @@ func getAerospikeClusterSpecWithAccessControl( accessControl *asdbv1.AerospikeAccessControlSpec, aerospikeConfSpec *AerospikeConfSpec, ) *asdbv1.AerospikeCluster { + racks := []asdbv1.Rack{ + { + ID: 1, + }, + { + ID: 2, + }, + } // create Aerospike custom resource return &asdbv1.AerospikeCluster{ ObjectMeta: metav1.ObjectMeta{ @@ -2147,6 +2317,12 @@ func getAerospikeClusterSpecWithAccessControl( Namespace: clusterNamespacedName.Namespace, }, Spec: asdbv1.AerospikeClusterSpec{ + RackConfig: asdbv1.RackConfig{ + Namespaces: []string{"test"}, + Racks: racks, + RollingUpdateBatchSize: &intstr.IntOrString{Type: intstr.Int, + IntVal: int32(2)}, + }, Size: testClusterSize, Image: fmt.Sprintf( "%s:%s", baseImage, aerospikeConfSpec.getVersion(), diff --git a/test/cluster/utils.go b/test/cluster/utils.go index 399da31f..f3dfe3e5 100644 --- a/test/cluster/utils.go +++ b/test/cluster/utils.go @@ -248,6 +248,8 @@ func (acs *AerospikeConfSpec) setEnableSecurity(enableSecurity bool) { if enableSecurity { security := map[string]interface{}{} acs.security = security + } else { + acs.security = nil } } @@ -337,12 +339,13 @@ func NewAerospikeConfSpec(image string) (*AerospikeConfSpec, error) { service := map[string]interface{}{ "feature-key-file": "/etc/aerospike/secret/features.conf", + "proto-fd-max": 1024, } network := getNetworkConfig() namespaces := []interface{}{ map[string]interface{}{ "name": "test", - "replication-factor": 1, + "replication-factor": 2, "storage-engine": map[string]interface{}{ "type": "memory", "data-size": 1073741824, From ef072c46237451c8e646405c815540c0d2c09d68 Mon Sep 17 00:00:00 2001 From: Tanmay Jain Date: Thu, 26 Jun 2025 20:13:30 +0530 Subject: [PATCH 02/13] Restructuring to bring security handling outside of reconcileRack --- internal/controller/cluster/rack.go | 45 ++++++++++++----------- internal/controller/cluster/reconciler.go | 8 ++-- 2 files changed, 27 insertions(+), 26 deletions(-) diff --git a/internal/controller/cluster/rack.go b/internal/controller/cluster/rack.go index 5a88ae55..a491d0bc 100644 --- a/internal/controller/cluster/rack.go +++ b/internal/controller/cluster/rack.go @@ -138,6 +138,13 @@ func (r *SingleClusterReconciler) reconcileRacks() common.ReconcileResult { } } + // Handle enable security just after updating configMap. + // This code will run only when security is being enabled in an existing cluster + // Update for security is verified by checking the aerospike.conf annotation of the pod. + if err = r.handleClusterSecurity(ignorablePodNames); err != nil { + return common.ReconcileError(err) + } + for idx := range rackStateList { state := &rackStateList[idx] found := &appsv1.StatefulSet{} @@ -386,14 +393,6 @@ func (r *SingleClusterReconciler) upgradeOrRollingRestartRack( return found, common.ReconcileError(err) } - // Handle enable security just after updating configMap. - // This code will run only when security is being enabled in an existing cluster - // Update for security is verified by checking the config hash of the pod with the - // config hash present in config map - if err := r.handleClusterSecurity(rackState, ignorablePodNames); err != nil { - return found, common.ReconcileError(err) - } - // Upgrade upgradeNeeded, err := r.isRackUpgradeNeeded(rackState.Rack.ID, ignorablePodNames) if err != nil { @@ -1775,7 +1774,7 @@ func (r *SingleClusterReconciler) getCurrentRackList() ( return rackList, nil } -func (r *SingleClusterReconciler) handleClusterSecurity(rackState *RackState, +func (r *SingleClusterReconciler) handleClusterSecurity( ignorablePodNames sets.Set[string]) error { if !r.needACLReconcile() { // No need to proceed if security is not to be enabling @@ -1783,18 +1782,18 @@ func (r *SingleClusterReconciler) handleClusterSecurity(rackState *RackState, } // Get pods where security-enabled config is applied - securityEnabledPods, err := r.getPodsWithEnabledSecurityForRack(rackState) + securityEnabledPod, err := r.getAnyPodWithEnabledSecurity(ignorablePodNames) if err != nil { return err } - if len(securityEnabledPods) == 0 { - // No security-enabled pods found + if securityEnabledPod == nil { + // No security-enabled pod found return nil } // Setup access control. - if err := r.validateAndReconcileAccessControl(securityEnabledPods, ignorablePodNames); err != nil { + if err := r.validateAndReconcileAccessControl(securityEnabledPod, ignorablePodNames); err != nil { r.Log.Error(err, "Failed to Reconcile access control") r.Recorder.Eventf( r.aeroCluster, corev1.EventTypeWarning, "ACLUpdateFailed", @@ -1815,21 +1814,25 @@ func (r *SingleClusterReconciler) needACLReconcile() bool { return specEnabled || statusEnabled } -func (r *SingleClusterReconciler) getPodsWithEnabledSecurityForRack(rackState *RackState) ([]corev1.Pod, error) { - pods, err := r.getOrderedRackPodList(rackState.Rack.ID) +func (r *SingleClusterReconciler) getAnyPodWithEnabledSecurity( + ignorablePodNames sets.Set[string]) (*corev1.Pod, error) { + podList, err := r.getClusterPodList() if err != nil { return nil, fmt.Errorf("failed to list pods: %v", err) } - if len(pods) == 0 { + if len(podList.Items) == 0 { // No pod found for the rack return nil, nil } - securityEnabledPods := make([]corev1.Pod, 0, len(pods)) + for idx := range podList.Items { + pod := &podList.Items[idx] + if ignorablePodNames.Has(pod.Name) { + continue + } - for idx := range pods { - statusFromAnnotation := pods[idx].Annotations["aerospikeConf"] + statusFromAnnotation := pod.Annotations["aerospikeConf"] asConfStatus, err := getFlatConfig(r.Log, statusFromAnnotation) if err != nil { @@ -1842,11 +1845,11 @@ func (r *SingleClusterReconciler) getPodsWithEnabledSecurityForRack(rackState *R } if enabled { - securityEnabledPods = append(securityEnabledPods, *pods[idx]) + return pod, nil } } - return securityEnabledPods, nil + return nil, nil } func isContainerNameInStorageVolumeAttachments( diff --git a/internal/controller/cluster/reconciler.go b/internal/controller/cluster/reconciler.go index 17277ce8..0a68f7d3 100644 --- a/internal/controller/cluster/reconciler.go +++ b/internal/controller/cluster/reconciler.go @@ -328,7 +328,7 @@ func (r *SingleClusterReconciler) recoverIgnorablePods(ignorablePodNames sets.Se } func (r *SingleClusterReconciler) validateAndReconcileAccessControl( - selectedPods []corev1.Pod, + selectedPod *corev1.Pod, ignorablePodNames sets.Set[string], ) error { var ( @@ -337,7 +337,7 @@ func (r *SingleClusterReconciler) validateAndReconcileAccessControl( ) // Create client - if selectedPods == nil { + if selectedPod == nil { var enabled bool enabled, err = asdbv1.IsSecurityEnabled(r.aeroCluster.Spec.AerospikeConfig.Value) @@ -355,14 +355,12 @@ func (r *SingleClusterReconciler) validateAndReconcileAccessControl( return fmt.Errorf("failed to get host info: %v", err) } } else { - conns, err = r.newPodsHostConnWithOption(selectedPods, ignorablePodNames) + conns, err = r.newPodsHostConnWithOption([]corev1.Pod{*selectedPod}, ignorablePodNames) if err != nil { return fmt.Errorf("failed to get host info: %v", err) } } - r.Log.Info("Reconcile access control for AerospikeCluster, created conn") - hosts := make([]*as.Host, 0, len(conns)) for _, conn := range conns { From f57b6f91a39e49c98d0d3f72a26e7dcfdbd7d08f Mon Sep 17 00:00:00 2001 From: Tanmay Jain Date: Wed, 2 Jul 2025 19:46:45 +0530 Subject: [PATCH 03/13] optimisation --- internal/controller/cluster/pod.go | 32 +++++++++++++++++++ internal/controller/cluster/rack.go | 15 ++++----- .../v1/aerospikecluster_validating_webhook.go | 22 ++++++++++++- test/cluster/access_control_test.go | 22 ++++++++++--- 4 files changed, 76 insertions(+), 15 deletions(-) diff --git a/internal/controller/cluster/pod.go b/internal/controller/cluster/pod.go index 586d72b1..d0e06adc 100644 --- a/internal/controller/cluster/pod.go +++ b/internal/controller/cluster/pod.go @@ -6,6 +6,7 @@ import ( "fmt" "net" "reflect" + "sort" "strconv" "strings" "time" @@ -932,6 +933,37 @@ func (r *SingleClusterReconciler) getClusterPodList() ( return podList, nil } +func (r *SingleClusterReconciler) getOrderedClusterPodList() (*corev1.PodList, error) { + podList, err := r.getClusterPodList() + if err != nil { + return nil, fmt.Errorf("failed to get cluster pod list: %v", err) + } + + sort.Slice(podList.Items, func(i, j int) bool { + // Split strings into parts + partsI := strings.Split(podList.Items[i].Name, "-") + partsJ := strings.Split(podList.Items[i].Name, "-") + + // Get the last two elements as integers + n := len(partsI) + firstI, _ := strconv.Atoi(partsI[n-2]) + secondI, _ := strconv.Atoi(partsI[n-1]) + + n = len(partsJ) + firstJ, _ := strconv.Atoi(partsJ[n-2]) + secondJ, _ := strconv.Atoi(partsJ[n-1]) + + // Sort by first number ascending + if firstI != firstJ { + return firstI < firstJ + } + // Then by second number descending + return secondI > secondJ + }) + + return podList, nil +} + func (r *SingleClusterReconciler) isAnyPodInImageFailedState(podList []*corev1.Pod, ignorablePodNames sets.Set[string], ) bool { for idx := range podList { diff --git a/internal/controller/cluster/rack.go b/internal/controller/cluster/rack.go index a491d0bc..a4cece69 100644 --- a/internal/controller/cluster/rack.go +++ b/internal/controller/cluster/rack.go @@ -138,13 +138,6 @@ func (r *SingleClusterReconciler) reconcileRacks() common.ReconcileResult { } } - // Handle enable security just after updating configMap. - // This code will run only when security is being enabled in an existing cluster - // Update for security is verified by checking the aerospike.conf annotation of the pod. - if err = r.handleClusterSecurity(ignorablePodNames); err != nil { - return common.ReconcileError(err) - } - for idx := range rackStateList { state := &rackStateList[idx] found := &appsv1.StatefulSet{} @@ -393,6 +386,10 @@ func (r *SingleClusterReconciler) upgradeOrRollingRestartRack( return found, common.ReconcileError(err) } + if err := r.handleClusterSecurity(ignorablePodNames); err != nil { + return found, common.ReconcileError(err) + } + // Upgrade upgradeNeeded, err := r.isRackUpgradeNeeded(rackState.Rack.ID, ignorablePodNames) if err != nil { @@ -1811,12 +1808,12 @@ func (r *SingleClusterReconciler) needACLReconcile() bool { specEnabled := r.aeroCluster.Spec.AerospikeAccessControl == nil statusEnabled := r.aeroCluster.Status.AerospikeAccessControl == nil - return specEnabled || statusEnabled + return specEnabled != statusEnabled } func (r *SingleClusterReconciler) getAnyPodWithEnabledSecurity( ignorablePodNames sets.Set[string]) (*corev1.Pod, error) { - podList, err := r.getClusterPodList() + podList, err := r.getOrderedClusterPodList() if err != nil { return nil, fmt.Errorf("failed to list pods: %v", err) } diff --git a/internal/webhook/v1/aerospikecluster_validating_webhook.go b/internal/webhook/v1/aerospikecluster_validating_webhook.go index a05f45e6..81858bd3 100644 --- a/internal/webhook/v1/aerospikecluster_validating_webhook.go +++ b/internal/webhook/v1/aerospikecluster_validating_webhook.go @@ -81,7 +81,7 @@ func (acv *AerospikeClusterCustomValidator) ValidateDelete(_ context.Context, ob return nil, nil } -// ValidateUpdate implements webhook.CustomValidator so a webhook will be registered for the type +// ValidateUpdate implement webhook.CustomValidator so a webhook will be registered for the type func (acv *AerospikeClusterCustomValidator) ValidateUpdate(_ context.Context, oldObj, newObj runtime.Object, ) (admission.Warnings, error) { aerospikeCluster, ok := newObj.(*asdbv1.AerospikeCluster) @@ -147,6 +147,10 @@ func (acv *AerospikeClusterCustomValidator) ValidateUpdate(_ context.Context, ol return warnings, err } + if err := validateAccessControlUpdate(&oldObject.Spec, &aerospikeCluster.Spec, &aerospikeCluster.Status); err != nil { + return warnings, err + } + // Validate AerospikeConfig update if err := validateAerospikeConfigUpdate( aslog, aerospikeCluster.Spec.AerospikeConfig, oldObject.Spec.AerospikeConfig, @@ -159,6 +163,22 @@ func (acv *AerospikeClusterCustomValidator) ValidateUpdate(_ context.Context, ol return warnings, validateRackUpdate(aslog, oldObject, aerospikeCluster) } +func validateAccessControlUpdate( + oldSpec *asdbv1.AerospikeClusterSpec, + newSpec *asdbv1.AerospikeClusterSpec, + status *asdbv1.AerospikeClusterStatus, +) error { + // Prevent removal of access control before status is updated + if oldSpec.AerospikeAccessControl != nil && + newSpec.AerospikeAccessControl == nil && + status.AerospikeAccessControl == nil { + return fmt.Errorf( + "cannot remove AerospikeAccessControl: status has not yet been updated with the current configuration") + } + + return nil +} + func validate(aslog logr.Logger, cluster *asdbv1.AerospikeCluster) (admission.Warnings, error) { aslog.V(1).Info("Validate AerospikeCluster spec", "obj.Spec", cluster.Spec) diff --git a/test/cluster/access_control_test.go b/test/cluster/access_control_test.go index f95a83cc..1fe0abfe 100644 --- a/test/cluster/access_control_test.go +++ b/test/cluster/access_control_test.go @@ -1742,9 +1742,7 @@ var _ = Describe( Name: "profileUser", SecretName: test.AuthSecretNameForUpdate, Roles: []string{ - "data-admin", - "read-write-udf", - "write", + "profiler", }, }, }, @@ -1770,8 +1768,6 @@ var _ = Describe( err = updateClusterWithNoWait(k8sClient, ctx, aeroCluster) Expect(err).ToNot(HaveOccurred()) - time.Sleep(time.Second * 3) - aerospikeConfigSpec.setEnableSecurity(false) accessControl = nil @@ -1784,6 +1780,22 @@ var _ = Describe( err = testAccessControlReconcile( aeroCluster, ctx, ) + + Expect(err).To(HaveOccurred()) + Expect(err.Error()).Should(ContainSubstring( + "status has not yet been updated with the current configuration")) + + time.Sleep(time.Minute * 1) + + aeroCluster = getAerospikeClusterSpecWithAccessControl( + clusterNamespacedName, accessControl, + aerospikeConfigSpec, + ) + + err = testAccessControlReconcile( + aeroCluster, ctx, + ) + Expect(err).To(HaveOccurred()) Expect(err.Error()).Should(ContainSubstring("SECURITY_NOT_ENABLED")) }, From edeb73c8c9dbb325d5e3c14b503552d82dfc9bb7 Mon Sep 17 00:00:00 2001 From: Tanmay Jain Date: Wed, 2 Jul 2025 19:51:37 +0530 Subject: [PATCH 04/13] removing focus from test --- test/cluster/access_control_test.go | 2 +- test/cluster/utils.go | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/test/cluster/access_control_test.go b/test/cluster/access_control_test.go index 1fe0abfe..894f7ce1 100644 --- a/test/cluster/access_control_test.go +++ b/test/cluster/access_control_test.go @@ -1493,7 +1493,7 @@ var _ = Describe( ) }, ) - FContext( + Context( "When cluster is deployed", func() { clusterName := fmt.Sprintf("ac-lifecycle-%d", GinkgoParallelProcess()) clusterNamespacedName := test.GetNamespacedName( diff --git a/test/cluster/utils.go b/test/cluster/utils.go index f3dfe3e5..9374af56 100644 --- a/test/cluster/utils.go +++ b/test/cluster/utils.go @@ -339,7 +339,6 @@ func NewAerospikeConfSpec(image string) (*AerospikeConfSpec, error) { service := map[string]interface{}{ "feature-key-file": "/etc/aerospike/secret/features.conf", - "proto-fd-max": 1024, } network := getNetworkConfig() namespaces := []interface{}{ From 2d77128e266baaffa69a2bf148f2d3a41693bddd Mon Sep 17 00:00:00 2001 From: Tanmay Jain Date: Tue, 8 Jul 2025 15:52:29 +0530 Subject: [PATCH 05/13] Getting Security enabled pods from server get config instead of pod's annotation. --- .../controller/cluster/aero_info_calls.go | 17 +++++--- internal/controller/cluster/pod.go | 32 --------------- internal/controller/cluster/rack.go | 41 ++++++++----------- internal/controller/cluster/service.go | 10 +++++ 4 files changed, 39 insertions(+), 61 deletions(-) diff --git a/internal/controller/cluster/aero_info_calls.go b/internal/controller/cluster/aero_info_calls.go index 4156e861..a0baf97b 100644 --- a/internal/controller/cluster/aero_info_calls.go +++ b/internal/controller/cluster/aero_info_calls.go @@ -104,6 +104,17 @@ func (r *SingleClusterReconciler) quiescePods( return deployment.InfoQuiesce(r.Log, policy, allHostConns, selectedHostConns, r.removedNamespaces(nodesNamespaces)) } +func (r *SingleClusterReconciler) getClusterSecurityConfig( + policy *as.ClientPolicy, podList *corev1.PodList, ignorablePodNames sets.Set[string], +) (map[string]deployment.InfoResult, error) { + hostConns, err := r.newPodsHostConnWithOption(podList.Items, ignorablePodNames) + if err != nil { + return nil, err + } + + return deployment.GetInfoOnHosts(r.Log, policy, hostConns, "get-config:context=security") +} + // TODO: Check only for migration func (r *SingleClusterReconciler) waitForClusterStability( policy *as.ClientPolicy, allHostConns []*deployment.HostConn, @@ -235,11 +246,7 @@ func (r *SingleClusterReconciler) newPodsHostConnWithOption(pods []corev1.Pod, i func (r *SingleClusterReconciler) newAsConn(pod *corev1.Pod) *deployment.ASConn { // Use pod IP and direct service port from within the operator for info calls. - tlsName, port := r.getServiceTLSNameAndPortIfConfigured() - - if tlsName == "" || port == nil { - port = asdbv1.GetServicePort(r.aeroCluster.Spec.AerospikeConfig) - } + tlsName, port := r.getResolvedTLSNameAndPort() host := pod.Status.PodIP asConn := &deployment.ASConn{ diff --git a/internal/controller/cluster/pod.go b/internal/controller/cluster/pod.go index d0e06adc..586d72b1 100644 --- a/internal/controller/cluster/pod.go +++ b/internal/controller/cluster/pod.go @@ -6,7 +6,6 @@ import ( "fmt" "net" "reflect" - "sort" "strconv" "strings" "time" @@ -933,37 +932,6 @@ func (r *SingleClusterReconciler) getClusterPodList() ( return podList, nil } -func (r *SingleClusterReconciler) getOrderedClusterPodList() (*corev1.PodList, error) { - podList, err := r.getClusterPodList() - if err != nil { - return nil, fmt.Errorf("failed to get cluster pod list: %v", err) - } - - sort.Slice(podList.Items, func(i, j int) bool { - // Split strings into parts - partsI := strings.Split(podList.Items[i].Name, "-") - partsJ := strings.Split(podList.Items[i].Name, "-") - - // Get the last two elements as integers - n := len(partsI) - firstI, _ := strconv.Atoi(partsI[n-2]) - secondI, _ := strconv.Atoi(partsI[n-1]) - - n = len(partsJ) - firstJ, _ := strconv.Atoi(partsJ[n-2]) - secondJ, _ := strconv.Atoi(partsJ[n-1]) - - // Sort by first number ascending - if firstI != firstJ { - return firstI < firstJ - } - // Then by second number descending - return secondI > secondJ - }) - - return podList, nil -} - func (r *SingleClusterReconciler) isAnyPodInImageFailedState(podList []*corev1.Pod, ignorablePodNames sets.Set[string], ) bool { for idx := range podList { diff --git a/internal/controller/cluster/rack.go b/internal/controller/cluster/rack.go index a4cece69..e6a5a64b 100644 --- a/internal/controller/cluster/rack.go +++ b/internal/controller/cluster/rack.go @@ -386,8 +386,10 @@ func (r *SingleClusterReconciler) upgradeOrRollingRestartRack( return found, common.ReconcileError(err) } - if err := r.handleClusterSecurity(ignorablePodNames); err != nil { - return found, common.ReconcileError(err) + if failedPods == nil { + if err := r.handleClusterSecurity(ignorablePodNames); err != nil { + return found, common.ReconcileError(err) + } } // Upgrade @@ -1808,41 +1810,32 @@ func (r *SingleClusterReconciler) needACLReconcile() bool { specEnabled := r.aeroCluster.Spec.AerospikeAccessControl == nil statusEnabled := r.aeroCluster.Status.AerospikeAccessControl == nil - return specEnabled != statusEnabled + return !r.IsStatusEmpty() && specEnabled != statusEnabled } func (r *SingleClusterReconciler) getAnyPodWithEnabledSecurity( ignorablePodNames sets.Set[string]) (*corev1.Pod, error) { - podList, err := r.getOrderedClusterPodList() + podList, err := r.getClusterPodList() if err != nil { - return nil, fmt.Errorf("failed to list pods: %v", err) + return nil, err } if len(podList.Items) == 0 { - // No pod found for the rack return nil, nil } - for idx := range podList.Items { - pod := &podList.Items[idx] - if ignorablePodNames.Has(pod.Name) { - continue - } - - statusFromAnnotation := pod.Annotations["aerospikeConf"] - - asConfStatus, err := getFlatConfig(r.Log, statusFromAnnotation) - if err != nil { - return nil, fmt.Errorf("failed to load config map by lib: %v", err) - } + securityConfigs, err := r.getClusterSecurityConfig(r.getClientPolicy(), podList, ignorablePodNames) + if err != nil { + return nil, fmt.Errorf("failed to get security config: %v", err) + } - enabled, err := asdbv1.IsSecurityEnabled(*asConfStatus) - if err != nil { - return nil, fmt.Errorf("failed to get cluster security status: %v", err) - } + for idx := range podList.Items { + _, port := r.getResolvedTLSNameAndPort() - if enabled { - return pod, nil + if security, ok := securityConfigs[hostID(podList.Items[idx].Status.PodIP, int(*port))]; ok { + if securityEnabled, ok := security["enable-security"]; ok && securityEnabled == "true" { + return &podList.Items[idx], nil + } } } diff --git a/internal/controller/cluster/service.go b/internal/controller/cluster/service.go index ce574e17..b769ff53 100644 --- a/internal/controller/cluster/service.go +++ b/internal/controller/cluster/service.go @@ -496,6 +496,16 @@ func (r *SingleClusterReconciler) getServiceTLSNameAndPortIfConfigured() (tlsNam return tlsName, port } +func (r *SingleClusterReconciler) getResolvedTLSNameAndPort() (tlsName string, port *int32) { + tlsName, port = r.getServiceTLSNameAndPortIfConfigured() + + if tlsName == "" || port == nil { + port = asdbv1.GetServicePort(r.aeroCluster.Spec.AerospikeConfig) + } + + return tlsName, port +} + func (r *SingleClusterReconciler) isServiceMetadataUpdated( service *corev1.Service, statusMetadata, From fd1e765b8c3a947d28164e497da8152594bd60c3 Mon Sep 17 00:00:00 2001 From: Tanmay Jain Date: Tue, 8 Jul 2025 17:12:52 +0530 Subject: [PATCH 06/13] nit --- internal/controller/cluster/rack.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/controller/cluster/rack.go b/internal/controller/cluster/rack.go index e6a5a64b..0c6e04ea 100644 --- a/internal/controller/cluster/rack.go +++ b/internal/controller/cluster/rack.go @@ -1807,10 +1807,10 @@ func (r *SingleClusterReconciler) handleClusterSecurity( } func (r *SingleClusterReconciler) needACLReconcile() bool { - specEnabled := r.aeroCluster.Spec.AerospikeAccessControl == nil - statusEnabled := r.aeroCluster.Status.AerospikeAccessControl == nil + isAccessControlSpecNil := r.aeroCluster.Spec.AerospikeAccessControl == nil + isAccessControlStatusNil := r.aeroCluster.Status.AerospikeAccessControl == nil - return !r.IsStatusEmpty() && specEnabled != statusEnabled + return !r.IsStatusEmpty() && isAccessControlSpecNil != isAccessControlStatusNil } func (r *SingleClusterReconciler) getAnyPodWithEnabledSecurity( From bbd9db62eec9ac1ca4ec849f03f7b38bb8ab1e7f Mon Sep 17 00:00:00 2001 From: Tanmay Jain Date: Wed, 9 Jul 2025 19:06:07 +0530 Subject: [PATCH 07/13] update management-lib --- go.mod | 4 ++-- go.sum | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/go.mod b/go.mod index e7adb1f8..5971042a 100644 --- a/go.mod +++ b/go.mod @@ -7,13 +7,14 @@ toolchain go1.23.6 require ( github.com/aerospike/aerospike-backup-service/v3 v3.1.1-0.20250519123935-790c268a2f06 github.com/aerospike/aerospike-client-go/v8 v8.2.2 - github.com/aerospike/aerospike-management-lib v1.7.1-0.20250519063642-57d55e3eddf8 + github.com/aerospike/aerospike-management-lib v1.7.1-0.20250709120612-377d0ff87581 github.com/asaskevich/govalidator v0.0.0-20210307081110-f21760c49a8d github.com/deckarep/golang-set/v2 v2.8.0 github.com/evanphx/json-patch v4.12.0+incompatible github.com/go-logr/logr v1.4.2 github.com/onsi/ginkgo/v2 v2.22.2 github.com/onsi/gomega v1.36.2 + github.com/prometheus/client_golang v1.21.1 github.com/sirupsen/logrus v1.9.3 github.com/stretchr/testify v1.10.0 golang.org/x/crypto v0.38.0 @@ -73,7 +74,6 @@ require ( github.com/mxk/go-flowrate v0.0.0-20140419014527-cca7078d478f // indirect github.com/pkg/errors v0.9.1 // indirect github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect - github.com/prometheus/client_golang v1.21.1 // indirect github.com/prometheus/client_model v0.6.1 // indirect github.com/prometheus/common v0.63.0 // indirect github.com/prometheus/procfs v0.16.0 // indirect diff --git a/go.sum b/go.sum index 7f699c75..a17fa987 100644 --- a/go.sum +++ b/go.sum @@ -4,8 +4,8 @@ github.com/aerospike/aerospike-backup-service/v3 v3.1.1-0.20250519123935-790c268 github.com/aerospike/aerospike-backup-service/v3 v3.1.1-0.20250519123935-790c268a2f06/go.mod h1:9OfmTkDJ8yLMcXawryIHMjLBtSITwm1+XAW0t4XPd5I= github.com/aerospike/aerospike-client-go/v8 v8.2.2 h1:NV1GxB+ATUb1cQtwaIS731A/6EkwuAX4/heh8CpvQOI= github.com/aerospike/aerospike-client-go/v8 v8.2.2/go.mod h1:H6CzKDoHxBj1yY/oQPci1bUIbEx2ATQtJ2GtZ+N64Wg= -github.com/aerospike/aerospike-management-lib v1.7.1-0.20250519063642-57d55e3eddf8 h1:wNMqJ+Xdn3+UiY4t4gC5wISNrT1v5cE3GmneFrKI5XQ= -github.com/aerospike/aerospike-management-lib v1.7.1-0.20250519063642-57d55e3eddf8/go.mod h1:lQggqSKESePgU+FmSRSBUCPcQdeiYS/RreMcHWXMWlU= +github.com/aerospike/aerospike-management-lib v1.7.1-0.20250709120612-377d0ff87581 h1:ZzFsPMDt0aaBSLWCSWE+vwnYFnCvTX8ibAACDfbfu5Y= +github.com/aerospike/aerospike-management-lib v1.7.1-0.20250709120612-377d0ff87581/go.mod h1:lQggqSKESePgU+FmSRSBUCPcQdeiYS/RreMcHWXMWlU= github.com/aerospike/backup-go v0.4.0 h1:Mzo5fTCtIxij2rRi1nhxIkZJjZLRQtzi/5iHSUHWmHk= github.com/aerospike/backup-go v0.4.0/go.mod h1:3KfiQfeDOAIfNajceHqykiQaYavnw4ByU6PjC8ThFsk= github.com/antlr4-go/antlr/v4 v4.13.0 h1:lxCg3LAv+EUK6t1i0y1V6/SLeUi0eKEKdhQAlS8TVTI= From 76c20c33aaf38f491414fe284babbeb2e9b5b75a Mon Sep 17 00:00:00 2001 From: Tanmay Jain Date: Mon, 14 Jul 2025 13:32:38 +0530 Subject: [PATCH 08/13] test jenkins --- test/custom_operator_deployment.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/test/custom_operator_deployment.yaml b/test/custom_operator_deployment.yaml index a198dc4d..8a371c17 100644 --- a/test/custom_operator_deployment.yaml +++ b/test/custom_operator_deployment.yaml @@ -40,3 +40,4 @@ spec: name: aerospike-kubernetes-operator source: aerospike-kubernetes-operator sourceNamespace: test + startingCSV: aerospike-kubernetes-operator.v4.0.2-candidate-pr-389-12 From f9b4655866cfa43db571da1071727148c923fef4 Mon Sep 17 00:00:00 2001 From: Tanmay Jain Date: Tue, 15 Jul 2025 10:43:33 +0530 Subject: [PATCH 09/13] revert jenkin test --- test/cluster/access_control_test.go | 19 ++++++------------- test/custom_operator_deployment.yaml | 1 - 2 files changed, 6 insertions(+), 14 deletions(-) diff --git a/test/cluster/access_control_test.go b/test/cluster/access_control_test.go index 894f7ce1..ea6c5ab6 100644 --- a/test/cluster/access_control_test.go +++ b/test/cluster/access_control_test.go @@ -1688,7 +1688,7 @@ var _ = Describe( }, ) - It( + FIt( "SecurityDisable: should disable security in partially security enabled cluster", func() { var accessControl *asdbv1.AerospikeAccessControlSpec @@ -1785,19 +1785,12 @@ var _ = Describe( Expect(err.Error()).Should(ContainSubstring( "status has not yet been updated with the current configuration")) - time.Sleep(time.Minute * 1) - - aeroCluster = getAerospikeClusterSpecWithAccessControl( - clusterNamespacedName, accessControl, - aerospikeConfigSpec, - ) - - err = testAccessControlReconcile( - aeroCluster, ctx, - ) + Eventually(func(g Gomega) { + err := testAccessControlReconcile(aeroCluster, ctx) + g.Expect(err).To(HaveOccurred()) + g.Expect(err.Error()).To(ContainSubstring("SECURITY_NOT_ENABLED")) + }, 5*time.Minute, 20*time.Second).Should(Succeed()) - Expect(err).To(HaveOccurred()) - Expect(err.Error()).Should(ContainSubstring("SECURITY_NOT_ENABLED")) }, ) diff --git a/test/custom_operator_deployment.yaml b/test/custom_operator_deployment.yaml index 8a371c17..a198dc4d 100644 --- a/test/custom_operator_deployment.yaml +++ b/test/custom_operator_deployment.yaml @@ -40,4 +40,3 @@ spec: name: aerospike-kubernetes-operator source: aerospike-kubernetes-operator sourceNamespace: test - startingCSV: aerospike-kubernetes-operator.v4.0.2-candidate-pr-389-12 From 97723f2e8a2662bc1edd1503b5add4aa71de91b0 Mon Sep 17 00:00:00 2001 From: Tanmay Jain Date: Thu, 17 Jul 2025 16:31:19 +0530 Subject: [PATCH 10/13] addressing comments --- internal/controller/cluster/rack.go | 4 +-- .../v1/aerospikecluster_validating_webhook.go | 3 +- test/cluster/access_control_test.go | 28 +++++++++---------- test/cluster/utils.go | 2 +- 4 files changed, 19 insertions(+), 18 deletions(-) diff --git a/internal/controller/cluster/rack.go b/internal/controller/cluster/rack.go index 0c6e04ea..0ebeb93c 100644 --- a/internal/controller/cluster/rack.go +++ b/internal/controller/cluster/rack.go @@ -1829,9 +1829,9 @@ func (r *SingleClusterReconciler) getAnyPodWithEnabledSecurity( return nil, fmt.Errorf("failed to get security config: %v", err) } - for idx := range podList.Items { - _, port := r.getResolvedTLSNameAndPort() + _, port := r.getResolvedTLSNameAndPort() + for idx := range podList.Items { if security, ok := securityConfigs[hostID(podList.Items[idx].Status.PodIP, int(*port))]; ok { if securityEnabled, ok := security["enable-security"]; ok && securityEnabled == "true" { return &podList.Items[idx], nil diff --git a/internal/webhook/v1/aerospikecluster_validating_webhook.go b/internal/webhook/v1/aerospikecluster_validating_webhook.go index 81858bd3..8937c392 100644 --- a/internal/webhook/v1/aerospikecluster_validating_webhook.go +++ b/internal/webhook/v1/aerospikecluster_validating_webhook.go @@ -173,7 +173,8 @@ func validateAccessControlUpdate( newSpec.AerospikeAccessControl == nil && status.AerospikeAccessControl == nil { return fmt.Errorf( - "cannot remove AerospikeAccessControl: status has not yet been updated with the current configuration") + "cannot remove aerospikeAccessControl: " + + "security enablement is in progress and status has not yet been updated to reflect the current configuration") } return nil diff --git a/test/cluster/access_control_test.go b/test/cluster/access_control_test.go index ea6c5ab6..6e3c0c12 100644 --- a/test/cluster/access_control_test.go +++ b/test/cluster/access_control_test.go @@ -1275,7 +1275,7 @@ var _ = Describe( ) } - aerospikeConfigSpec.setEnableSecurity(false) + aerospikeConfigSpec.setSecurity(false) aeroCluster := getAerospikeClusterSpecWithAccessControl( clusterNamespacedName, &accessControl, @@ -1359,7 +1359,7 @@ var _ = Describe( ) } - aerospikeConfigSpec.setEnableSecurity(true) + aerospikeConfigSpec.setSecurity(true) aeroCluster := getAerospikeClusterSpecWithAccessControl( clusterNamespacedName, &accessControl, @@ -1529,7 +1529,7 @@ var _ = Describe( ) } - aerospikeConfigSpec.setEnableSecurity(false) + aerospikeConfigSpec.setSecurity(false) // Save cluster variable as well for cleanup. aeroCluster := getAerospikeClusterSpecWithAccessControl( @@ -1586,7 +1586,7 @@ var _ = Describe( ) } - aerospikeConfigSpec.setEnableSecurity(true) + aerospikeConfigSpec.setSecurity(true) aeroCluster = getAerospikeClusterSpecWithAccessControl( clusterNamespacedName, accessControl, @@ -1649,7 +1649,7 @@ var _ = Describe( ) } - aerospikeConfigSpec.setEnableSecurity(true) + aerospikeConfigSpec.setSecurity(true) aeroCluster := getAerospikeClusterSpecWithAccessControl( clusterNamespacedName, accessControl, @@ -1672,7 +1672,7 @@ var _ = Describe( ) } - aerospikeConfigSpec.setEnableSecurity(false) + aerospikeConfigSpec.setSecurity(false) accessControl = nil // Save cluster variable as well for cleanup. @@ -1688,7 +1688,7 @@ var _ = Describe( }, ) - FIt( + It( "SecurityDisable: should disable security in partially security enabled cluster", func() { var accessControl *asdbv1.AerospikeAccessControlSpec @@ -1703,7 +1703,7 @@ var _ = Describe( ) } - aerospikeConfigSpec.setEnableSecurity(false) + aerospikeConfigSpec.setSecurity(false) // Save cluster variable as well for cleanup. aeroCluster := getAerospikeClusterSpecWithAccessControl( @@ -1758,7 +1758,7 @@ var _ = Describe( ) } - aerospikeConfigSpec.setEnableSecurity(true) + aerospikeConfigSpec.setSecurity(true) aeroCluster = getAerospikeClusterSpecWithAccessControl( clusterNamespacedName, accessControl, @@ -1768,7 +1768,7 @@ var _ = Describe( err = updateClusterWithNoWait(k8sClient, ctx, aeroCluster) Expect(err).ToNot(HaveOccurred()) - aerospikeConfigSpec.setEnableSecurity(false) + aerospikeConfigSpec.setSecurity(false) accessControl = nil // Save cluster variable as well for cleanup. @@ -1857,7 +1857,7 @@ var _ = Describe( ) } - aerospikeConfigSpec.setEnableSecurity(true) + aerospikeConfigSpec.setSecurity(true) aeroCluster := getAerospikeClusterSpecWithAccessControl( clusterNamespacedName, &accessControl, @@ -1977,7 +1977,7 @@ var _ = Describe( ) } - aerospikeConfigSpec.setEnableSecurity(true) + aerospikeConfigSpec.setSecurity(true) aerospikeConfigSpec.setEnableQuotas(true) aeroCluster = getAerospikeClusterSpecWithAccessControl( @@ -2049,7 +2049,7 @@ var _ = Describe( ), ) } - aerospikeConfigSpec.setEnableSecurity(true) + aerospikeConfigSpec.setSecurity(true) aerospikeConfigSpec.setEnableQuotas(false) aeroCluster = getAerospikeClusterSpecWithAccessControl( @@ -2123,7 +2123,7 @@ var _ = Describe( ), ) } - aerospikeConfigSpec.setEnableSecurity(true) + aerospikeConfigSpec.setSecurity(true) aerospikeConfigSpec.setEnableQuotas(false) aeroCluster = getAerospikeClusterSpecWithAccessControl( diff --git a/test/cluster/utils.go b/test/cluster/utils.go index 958892ee..9afb63be 100644 --- a/test/cluster/utils.go +++ b/test/cluster/utils.go @@ -244,7 +244,7 @@ func (acs *AerospikeConfSpec) getVersion() string { return acs.version } -func (acs *AerospikeConfSpec) setEnableSecurity(enableSecurity bool) { +func (acs *AerospikeConfSpec) setSecurity(enableSecurity bool) { if enableSecurity { security := map[string]interface{}{} acs.security = security From 6b1f3c946b6342af85e926513629b0dc6c9e264f Mon Sep 17 00:00:00 2001 From: Tanmay Jain Date: Tue, 22 Jul 2025 12:23:33 +0530 Subject: [PATCH 11/13] addressing comments --- test/cluster/access_control_test.go | 62 +---------------------------- 1 file changed, 2 insertions(+), 60 deletions(-) diff --git a/test/cluster/access_control_test.go b/test/cluster/access_control_test.go index 6e3c0c12..348e0f5f 100644 --- a/test/cluster/access_control_test.go +++ b/test/cluster/access_control_test.go @@ -59,7 +59,7 @@ var aerospikeConfigWithSecurityWithQuota = &asdbv1.AerospikeConfigSpec{ }, } -var _ = Describe( +var _ = FDescribe( "AccessControl", func() { ctx := goctx.TODO() @@ -1576,16 +1576,6 @@ var _ = Describe( }, } - aerospikeConfigSpec, err = NewAerospikeConfSpec(latestImage) - if err != nil { - Fail( - fmt.Sprintf( - "Invalid Aerospike Config Spec: %v", - err, - ), - ) - } - aerospikeConfigSpec.setSecurity(true) aeroCluster = getAerospikeClusterSpecWithAccessControl( @@ -1662,16 +1652,6 @@ var _ = Describe( Fail("Security should be enabled") } - aerospikeConfigSpec, err = NewAerospikeConfSpec(latestImage) - if err != nil { - Fail( - fmt.Sprintf( - "Invalid Aerospike Config Spec: %v", - err, - ), - ) - } - aerospikeConfigSpec.setSecurity(false) accessControl = nil @@ -1748,16 +1728,6 @@ var _ = Describe( }, } - aerospikeConfigSpec, err = NewAerospikeConfSpec(latestImage) - if err != nil { - Fail( - fmt.Sprintf( - "Invalid Aerospike Config Spec: %v", - err, - ), - ) - } - aerospikeConfigSpec.setSecurity(true) aeroCluster = getAerospikeClusterSpecWithAccessControl( @@ -1783,7 +1753,7 @@ var _ = Describe( Expect(err).To(HaveOccurred()) Expect(err.Error()).Should(ContainSubstring( - "status has not yet been updated with the current configuration")) + "security enablement is in progress")) Eventually(func(g Gomega) { err := testAccessControlReconcile(aeroCluster, ctx) @@ -1967,16 +1937,6 @@ var _ = Describe( }, } - aerospikeConfigSpec, err = NewAerospikeConfSpec(latestImage) - if err != nil { - Fail( - fmt.Sprintf( - "Invalid Aerospike Config Spec: %v", - err, - ), - ) - } - aerospikeConfigSpec.setSecurity(true) aerospikeConfigSpec.setEnableQuotas(true) @@ -2040,15 +2000,6 @@ var _ = Describe( }, } - aerospikeConfigSpec, err = NewAerospikeConfSpec(latestImage) - if err != nil { - Fail( - fmt.Sprintf( - "Invalid Aerospike Config Spec: %v", - err, - ), - ) - } aerospikeConfigSpec.setSecurity(true) aerospikeConfigSpec.setEnableQuotas(false) @@ -2114,15 +2065,6 @@ var _ = Describe( }, } - aerospikeConfigSpec, err = NewAerospikeConfSpec(latestImage) - if err != nil { - Fail( - fmt.Sprintf( - "Invalid Aerospike Config Spec: %v", - err, - ), - ) - } aerospikeConfigSpec.setSecurity(true) aerospikeConfigSpec.setEnableQuotas(false) From 2ad53dec27076f3798835a8b267f91da16403d96 Mon Sep 17 00:00:00 2001 From: Tanmay Jain Date: Tue, 22 Jul 2025 12:31:52 +0530 Subject: [PATCH 12/13] nit --- test/cluster/access_control_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/cluster/access_control_test.go b/test/cluster/access_control_test.go index 348e0f5f..1337785c 100644 --- a/test/cluster/access_control_test.go +++ b/test/cluster/access_control_test.go @@ -59,7 +59,7 @@ var aerospikeConfigWithSecurityWithQuota = &asdbv1.AerospikeConfigSpec{ }, } -var _ = FDescribe( +var _ = Describe( "AccessControl", func() { ctx := goctx.TODO() From 8a5a804a7bc8c48bb42be08e26d54876d3783fe7 Mon Sep 17 00:00:00 2001 From: Tanmay Jain Date: Thu, 24 Jul 2025 11:20:01 +0530 Subject: [PATCH 13/13] fix testcase --- test/cluster/cluster_helper.go | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/test/cluster/cluster_helper.go b/test/cluster/cluster_helper.go index 4a342647..d641a1ac 100644 --- a/test/cluster/cluster_helper.go +++ b/test/cluster/cluster_helper.go @@ -1408,14 +1408,7 @@ func aerospikeClusterCreateUpdateWithTO( current.Spec.AerospikeConfig.Value = desired.Spec.AerospikeConfig.DeepCopy().Value - if err := k8sClient.Update(ctx, current); err != nil { - return err - } - - return waitForAerospikeCluster( - k8sClient, ctx, desired, int(desired.Spec.Size), retryInterval, timeout, - []asdbv1.AerospikeClusterPhase{asdbv1.AerospikeClusterCompleted}, - ) + return updateClusterWithTO(k8sClient, ctx, current, timeout) } func aerospikeClusterCreateUpdate(