[KO-290] Allow enabling security in the existing deployed cluster

api/v1/aerospikecluster_types.go

@@ -871,6 +871,9 @@ type AerospikePodStatus struct { //nolint:govet // for readability
 
 	// PodSpecHash is ripemd160 hash of PodSpec used by this pod
 	PodSpecHash string `json:"podSpecHash"`
+
+	// SecurityEnabled is true if security is enabled in the pod
+	SecurityEnabled bool `json:"securityEnabled"`
 }
 
 // +kubebuilder:object:root=true

api/v1/aerospikecluster_validating_webhook.go

@@ -1294,20 +1294,19 @@ func validateSecurityConfigUpdate(
 func validateEnableSecurityConfig(newConfSpec, oldConfSpec *AerospikeConfigSpec) error {
 	newConf := newConfSpec.Value
 	oldConf := oldConfSpec.Value
-	// Security cannot be updated dynamically
-	// TODO: How to enable dynamic security update, need to pass policy for individual nodes.
-	// auth-enabled and auth-disabled node can co-exist
 	oldSec, oldSecConfFound := oldConf["security"]
 	newSec, newSecConfFound := newConf["security"]
 
+	if oldSecConfFound && !newSecConfFound {
+		return fmt.Errorf("cannot remove cluster security config")
+	}
+
 	if oldSecConfFound && newSecConfFound {
 		oldSecFlag, oldEnableSecurityFlagFound := oldSec.(map[string]interface{})["enable-security"]
 		newSecFlag, newEnableSecurityFlagFound := newSec.(map[string]interface{})["enable-security"]
 
-		if oldEnableSecurityFlagFound != newEnableSecurityFlagFound || !reflect.DeepEqual(
-			oldSecFlag, newSecFlag,
-		) {
-			return fmt.Errorf("cannot update cluster security config enable-security was changed")
+		if oldEnableSecurityFlagFound && oldSecFlag.(bool) && (!newEnableSecurityFlagFound || !newSecFlag.(bool)) {
+			return fmt.Errorf("cannot disable cluster security in running cluster")
 		}
 	}
 
@@ -1336,8 +1335,8 @@ func validateSecurityContext(
 		}
 	}
 
-	if ivflag != ovflag {
-		return fmt.Errorf("cannot update cluster security config enable-security was changed")
+	if !ivflag && ovflag {
+		return fmt.Errorf("cannot disable cluster security in running cluster")
 	}
 
 	return nil

api/v1/utils.go

@@ -61,8 +61,8 @@ const (
 	AerospikeInitContainerName                     = "aerospike-init"
 	AerospikeInitContainerRegistryEnvVar           = "AEROSPIKE_KUBERNETES_INIT_REGISTRY"
 	AerospikeInitContainerDefaultRegistry          = "docker.io"
-	AerospikeInitContainerDefaultRegistryNamespace = "aerospike"
-	AerospikeInitContainerDefaultRepoAndTag        = "aerospike-kubernetes-init:2.1.1"
+	AerospikeInitContainerDefaultRegistryNamespace = "tanmayj10"
+	AerospikeInitContainerDefaultRepoAndTag        = "aerospike-kubernetes-init:2.1.1-secenabled"
 	AerospikeAppLabel                              = "app"
 	AerospikeCustomResourceLabel                   = "aerospike.com/cr"
 	AerospikeRackIDLabel                           = "aerospike.com/rack-id"
@@ -204,7 +204,7 @@ func IsSecurityEnabled(
 			return false, nil
 		}
 
-		if errors.Is(err, internalerrors.ErrInvalidOrEmpty) && retval >= 0 {
+		if errors.Is(err, internalerrors.ErrInvalidOrEmpty) {
 			return true, nil
 		}
 

config/crd/bases/asdb.aerospike.com_aerospikeclusters.yaml

@@ -14270,6 +14270,10 @@ spec:
                       description: PodSpecHash is ripemd160 hash of PodSpec used by
                         this pod
                       type: string
+                    securityEnabled:
+                      description: SecurityEnabled is true if security is enabled
+                        in the pod
+                      type: boolean
                     servicePort:
                       description: ServicePort is the port Aerospike clients outside
                         K8s can connect to.
@@ -14282,6 +14286,7 @@ spec:
                   - podIP
                   - podPort
                   - podSpecHash
+                  - securityEnabled
                   type: object
                 description: Pods has Aerospike specific status of the pods. This
                   is map instead of the conventional map as list convention to allow

config/manifests/bases/aerospike-kubernetes-operator.clusterserviceversion.yaml

@@ -51,12 +51,15 @@ spec:
         displayName: Server Image
         path: image
       - description: K8sNodeBlockList is a list of Kubernetes nodes which are not
-          used for Aerospike pods.
+          used for Aerospike pods. Pods are not scheduled on these nodes. Pods are
+          migrated from these nodes if already present. This is useful for the maintenance
+          of Kubernetes nodes.
         displayName: Kubernetes Node BlockList
         path: k8sNodeBlockList
       - description: MaxUnavailable is the percentage/number of pods that can be allowed
           to go down or unavailable before application disruption. This value is used
-          to create PodDisruptionBudget. Defaults to 1.
+          to create PodDisruptionBudget. Defaults to 1. Refer Aerospike documentation
+          for more details.
         displayName: Max Unavailable
         path: maxUnavailable
       - description: Certificates to connect to Aerospike.

controllers/access_control.go

@@ -37,41 +37,39 @@ func AerospikeAdminCredentials(
 	desiredState, currentState *asdbv1.AerospikeClusterSpec,
 	passwordProvider AerospikeUserPasswordProvider,
 ) (user, pass string, err error) {
-	var enabled bool
+	var (
+		enabled                bool
+		currentSecurityEnabled bool
+		desiredSecurityEnabled bool
+		currentSecurityErr     error
+		desiredSecurityErr     error
+	)
 
-	outgoingVersion, err := asdbv1.GetImageVersion(currentState.Image)
-	if err != nil {
-		incomingVersion, newErr := asdbv1.GetImageVersion(desiredState.Image)
-		if newErr != nil {
-			return "", "", newErr
-		}
+	outgoingVersion, outgoingVersionErr := asdbv1.GetImageVersion(currentState.Image)
+	if outgoingVersionErr == nil {
+		// It is possible that this is a new cluster and current state is empty.
+		currentSecurityEnabled, currentSecurityErr = asdbv1.IsSecurityEnabled(
+			outgoingVersion, currentState.AerospikeConfig,
+		)
+	} else {
+		currentSecurityErr = outgoingVersionErr
+	}
 
-		enabled, newErr = asdbv1.IsSecurityEnabled(
+	incomingVersion, incomingVersionErr := asdbv1.GetImageVersion(desiredState.Image)
+	if incomingVersionErr == nil {
+		desiredSecurityEnabled, desiredSecurityErr = asdbv1.IsSecurityEnabled(
 			incomingVersion, desiredState.AerospikeConfig,
 		)
-		if newErr != nil {
-			return "", "", newErr
-		}
 	} else {
-		enabled, err = asdbv1.IsSecurityEnabled(
-			outgoingVersion, currentState.AerospikeConfig,
-		)
-		if err != nil {
-			incomingVersion, newErr := asdbv1.GetImageVersion(desiredState.Image)
-			if newErr != nil {
-				return "", "", newErr
-			}
+		desiredSecurityErr = incomingVersionErr
+	}
 
-			// Its possible this is a new cluster and current state is empty.
-			enabled, newErr = asdbv1.IsSecurityEnabled(
-				incomingVersion, desiredState.AerospikeConfig,
-			)
-			if newErr != nil {
-				return "", "", newErr
-			}
-		}
+	if currentSecurityErr != nil && desiredSecurityErr != nil {
+		return "", "", desiredSecurityErr
 	}
 
+	enabled = currentSecurityEnabled || desiredSecurityEnabled
+
 	if !enabled {
 		// Return zero strings if this is not a security enabled cluster.
 		return "", "", nil

controllers/reconciler.go

@@ -119,6 +119,10 @@ func (r *SingleClusterReconciler) Reconcile() (result ctrl.Result, recErr error)
 		return reconcile.Result{}, recErr
 	}
 
+	if err := r.handleEnableSecurity(); err != nil {
+		return reconcile.Result{}, err
+	}
+
 	// Reconcile all racks
 	if res := r.reconcileRacks(); !res.isSuccess {
 		if res.err != nil {
@@ -192,7 +196,8 @@ func (r *SingleClusterReconciler) Reconcile() (result ctrl.Result, recErr error)
 	}
 
 	// Setup access control.
-	if err = r.validateAndReconcileAccessControl(ignorablePodNames); err != nil {
+	// Assuming all pods must be security enabled or disabled.
+	if err = r.validateAndReconcileAccessControl(nil, ignorablePodNames); err != nil {
 		r.Log.Error(err, "Failed to Reconcile access control")
 		r.Recorder.Eventf(
 			r.aeroCluster, corev1.EventTypeWarning, "ACLUpdateFailed",
@@ -205,20 +210,6 @@ func (r *SingleClusterReconciler) Reconcile() (result ctrl.Result, recErr error)
 		return reconcile.Result{}, recErr
 	}
 
-	// Update the AerospikeCluster status.
-	if err = r.updateAccessControlStatus(); err != nil {
-		r.Log.Error(err, "Failed to update AerospikeCluster access control status")
-		r.Recorder.Eventf(
-			r.aeroCluster, corev1.EventTypeWarning, "StatusUpdateFailed",
-			"Failed to update AerospikeCluster access control status %s/%s",
-			r.aeroCluster.Namespace, r.aeroCluster.Name,
-		)
-
-		recErr = err
-
-		return reconcile.Result{}, recErr
-	}
-
 	// Use policy from spec after setting up access control
 	policy := r.getClientPolicy()
 
@@ -314,7 +305,8 @@ func (r *SingleClusterReconciler) recoverIgnorablePods() reconcileResult {
 	return reconcileSuccess()
 }
 
-func (r *SingleClusterReconciler) validateAndReconcileAccessControl(ignorablePodNames sets.Set[string]) error {
+func (r *SingleClusterReconciler) validateAndReconcileAccessControl(selectedPods []corev1.Pod,
+	ignorablePodNames sets.Set[string]) error {
 	version, err := asdbv1.GetImageVersion(r.aeroCluster.Spec.Image)
 	if err != nil {
 		return err
@@ -332,10 +324,19 @@ func (r *SingleClusterReconciler) validateAndReconcileAccessControl(ignorablePod
 		return nil
 	}
 
+	var conns []*deployment.HostConn
+
 	// Create client
-	conns, err := r.newAllHostConnWithOption(ignorablePodNames)
-	if err != nil {
-		return fmt.Errorf("failed to get host info: %v", err)
+	if selectedPods == nil {
+		conns, err = r.newAllHostConnWithOption(ignorablePodNames)
+		if err != nil {
+			return fmt.Errorf("failed to get host info: %v", err)
+		}
+	} else {
+		conns, err = r.newPodsHostConnWithOption(selectedPods, ignorablePodNames)
+		if err != nil {
+			return fmt.Errorf("failed to get host info: %v", err)
+		}
 	}
 
 	hosts := make([]*as.Host, 0, len(conns))
@@ -365,15 +366,30 @@ func (r *SingleClusterReconciler) validateAndReconcileAccessControl(ignorablePod
 	err = r.reconcileAccessControl(
 		aeroClient, pp,
 	)
-	if err == nil {
+
+	if err != nil {
+		return fmt.Errorf("failed to reconcile access control: %v", err)
+	}
+
+	r.Recorder.Eventf(
+		r.aeroCluster, corev1.EventTypeNormal, "ACLUpdated",
+		"Updated Access Control %s/%s", r.aeroCluster.Namespace,
+		r.aeroCluster.Name,
+	)
+
+	// Update the AerospikeCluster status.
+	if err := r.updateAccessControlStatus(); err != nil {
+		r.Log.Error(err, "Failed to update AerospikeCluster access control status")
 		r.Recorder.Eventf(
-			r.aeroCluster, corev1.EventTypeNormal, "ACLUpdated",
-			"Updated Access Control %s/%s", r.aeroCluster.Namespace,
-			r.aeroCluster.Name,
+			r.aeroCluster, corev1.EventTypeWarning, "StatusUpdateFailed",
+			"Failed to update AerospikeCluster access control status %s/%s",
+			r.aeroCluster.Namespace, r.aeroCluster.Name,
 		)
+
+		return err
 	}
 
-	return err
+	return nil
 }
 
 func (r *SingleClusterReconciler) updateStatus() error {
@@ -983,3 +999,58 @@ func (r *SingleClusterReconciler) AddAPIVersionLabel(ctx context.Context) error
 
 	return r.Client.Update(ctx, aeroCluster, updateOption)
 }
+
+func (r *SingleClusterReconciler) getSecurityEnabledPods() ([]corev1.Pod, error) {
+	securityEnabledPods := make([]corev1.Pod, 0, len(r.aeroCluster.Status.Pods))
+
+	for podName := range r.aeroCluster.Status.Pods {
+		if r.aeroCluster.Status.Pods[podName].SecurityEnabled {
+			pod := &corev1.Pod{}
+			podName := types.NamespacedName{Name: podName, Namespace: r.aeroCluster.Namespace}
+
+			if err := r.Client.Get(context.TODO(), podName, pod); err != nil {
+				return securityEnabledPods, err
+			}
+
+			securityEnabledPods = append(securityEnabledPods, *pod)
+		}
+	}
+
+	return securityEnabledPods, nil
+}
+
+func (r *SingleClusterReconciler) enablingSecurity() bool {
+	return r.aeroCluster.Spec.AerospikeAccessControl != nil && r.aeroCluster.Status.AerospikeAccessControl == nil
+}
+
+func (r *SingleClusterReconciler) handleEnableSecurity() error {
+	if r.aeroCluster.Status.Pods != nil && r.enablingSecurity() {
+		securityEnabledPods, err := r.getSecurityEnabledPods()
+		if err != nil {
+			return err
+		}
+
+		if len(securityEnabledPods) > 0 {
+			ignorablePodNames, err := r.getIgnorablePods(nil, getConfiguredRackStateList(r.aeroCluster))
+			if err != nil {
+				r.Log.Error(err, "Failed to determine pods to be ignored")
+
+				return err
+			}
+
+			// Setup access control.
+			if err := r.validateAndReconcileAccessControl(securityEnabledPods, ignorablePodNames); err != nil {
+				r.Log.Error(err, "Failed to Reconcile access control")
+				r.Recorder.Eventf(
+					r.aeroCluster, corev1.EventTypeWarning, "ACLUpdateFailed",
+					"Failed to setup Access Control %s/%s", r.aeroCluster.Namespace,
+					r.aeroCluster.Name,
+				)
+
+				return err
+			}
+		}
+	}
+
+	return nil
+}

helm-charts/aerospike-kubernetes-operator/crds/customresourcedefinition_aerospikeclusters.asdb.aerospike.com.yaml

@@ -14270,6 +14270,10 @@ spec:
                       description: PodSpecHash is ripemd160 hash of PodSpec used by
                         this pod
                       type: string
+                    securityEnabled:
+                      description: SecurityEnabled is true if security is enabled
+                        in the pod
+                      type: boolean
                     servicePort:
                       description: ServicePort is the port Aerospike clients outside
                         K8s can connect to.
@@ -14282,6 +14286,7 @@ spec:
                   - podIP
                   - podPort
                   - podSpecHash
+                  - securityEnabled
                   type: object
                 description: Pods has Aerospike specific status of the pods. This
                   is map instead of the conventional map as list convention to allow

test/access_control_test.go

@@ -1626,7 +1626,7 @@ var _ = Describe(
 						ctx := goctx.Background()
 
 						It(
-							"SecurityUpdateReject: should fail, Cannot update cluster security config",
+							"SecurityEnable: should enable security in running cluster",
 							func() {
 								var accessControl *asdbv1.AerospikeAccessControlSpec
 
@@ -1712,11 +1712,8 @@ var _ = Describe(
 								err = testAccessControlReconcile(
 									aeroCluster, ctx,
 								)
-								if err == nil || !strings.Contains(
-									err.Error(),
-									"cannot update cluster security config",
-								) {
-									Fail("SecurityUpdate should have failed")
+								if err != nil {
+									Fail("Security should have enabled successfully")
 								}
 
 								if aeroCluster != nil {
@@ -1890,7 +1887,7 @@ var _ = Describe(
 								)
 								if err == nil || !strings.Contains(
 									err.Error(),
-									"cannot update cluster security config",
+									"cannot disable cluster security in running cluster",
 								) {
 									Fail("SecurityUpdate should have failed")
 								}