diff --git a/api/v1/aerospikecluster_types.go b/api/v1/aerospikecluster_types.go index 32a28c781..04b8e4da6 100644 --- a/api/v1/aerospikecluster_types.go +++ b/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"` + + // IsSecurityEnabled is true if security is enabled in the pod + IsSecurityEnabled bool `json:"isSecurityEnabled"` } // +kubebuilder:object:root=true diff --git a/api/v1/aerospikecluster_validating_webhook.go b/api/v1/aerospikecluster_validating_webhook.go index fe340aee2..c15d55aff 100644 --- a/api/v1/aerospikecluster_validating_webhook.go +++ b/api/v1/aerospikecluster_validating_webhook.go @@ -119,7 +119,7 @@ func (c *AerospikeCluster) ValidateUpdate(oldObj runtime.Object) (admission.Warn if err := validateAerospikeConfigUpdate( aslog, incomingVersion, outgoingVersion, c.Spec.AerospikeConfig, old.Spec.AerospikeConfig, - c.Status.AerospikeConfig, + c.Status.AerospikeConfig, c.Status.Pods, ); err != nil { return nil, err } @@ -443,7 +443,7 @@ func (c *AerospikeCluster) validateRackUpdate( if err := validateAerospikeConfigUpdate( aslog, incomingVersion, outgoingVersion, &newRack.AerospikeConfig, &oldRack.AerospikeConfig, - rackStatusConfig, + rackStatusConfig, c.Status.Pods, ); err != nil { return fmt.Errorf( "invalid update in Rack(ID: %d) aerospikeConfig: %v", @@ -1273,7 +1273,7 @@ func getNamespaceReplicationFactor(nsConf map[string]interface{}) (int, error) { } func validateSecurityConfigUpdate( - newVersion, oldVersion string, newSpec, oldSpec *AerospikeConfigSpec, + newVersion, oldVersion string, newSpec, oldSpec *AerospikeConfigSpec, podStatus map[string]AerospikePodStatus, ) error { nv, err := lib.CompareVersions(newVersion, "5.7.0") if err != nil { @@ -1285,30 +1285,39 @@ func validateSecurityConfigUpdate( return err } + isSecurityEnabledPodExist := false + + for pod := range podStatus { + if podStatus[pod].IsSecurityEnabled { + isSecurityEnabledPodExist = true + break + } + } + if nv >= 0 || ov >= 0 { - return validateSecurityContext(newVersion, oldVersion, newSpec, oldSpec) + return validateSecurityContext(newVersion, oldVersion, newSpec, oldSpec, isSecurityEnabledPodExist) } - return validateEnableSecurityConfig(newSpec, oldSpec) + return validateEnableSecurityConfig(newSpec, oldSpec, isSecurityEnabledPodExist) } -func validateEnableSecurityConfig(newConfSpec, oldConfSpec *AerospikeConfigSpec) error { +func validateEnableSecurityConfig(newConfSpec, oldConfSpec *AerospikeConfigSpec, isSecurityEnabledPodExist bool) 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)) && + isSecurityEnabledPodExist { + return fmt.Errorf("cannot disable cluster security in running cluster") } } @@ -1316,7 +1325,7 @@ func validateEnableSecurityConfig(newConfSpec, oldConfSpec *AerospikeConfigSpec) } func validateSecurityContext( - newVersion, oldVersion string, newSpec, oldSpec *AerospikeConfigSpec, + newVersion, oldVersion string, newSpec, oldSpec *AerospikeConfigSpec, isSecurityEnabledPodExist bool, ) error { ovflag, err := IsSecurityEnabled(oldVersion, oldSpec) if err != nil { @@ -1337,8 +1346,8 @@ func validateSecurityContext( } } - if ivflag != ovflag { - return fmt.Errorf("cannot update cluster security config enable-security was changed") + if !ivflag && ovflag && isSecurityEnabledPodExist { + return fmt.Errorf("cannot disable cluster security in running cluster") } return nil @@ -1347,11 +1356,13 @@ func validateSecurityContext( func validateAerospikeConfigUpdate( aslog logr.Logger, incomingVersion, outgoingVersion string, incomingSpec, outgoingSpec, currentStatus *AerospikeConfigSpec, + podStatus map[string]AerospikePodStatus, ) error { aslog.Info("Validate AerospikeConfig update") if err := validateSecurityConfigUpdate( incomingVersion, outgoingVersion, incomingSpec, outgoingSpec, + podStatus, ); err != nil { return err } diff --git a/api/v1/utils.go b/api/v1/utils.go index e59bbe9a4..bea471b81 100644 --- a/api/v1/utils.go +++ b/api/v1/utils.go @@ -63,7 +63,7 @@ const ( AerospikeInitContainerRegistryEnvVar = "AEROSPIKE_KUBERNETES_INIT_REGISTRY" AerospikeInitContainerDefaultRegistry = "docker.io" AerospikeInitContainerDefaultRegistryNamespace = "aerospike" - AerospikeInitContainerDefaultRepoAndTag = "aerospike-kubernetes-init:2.1.2" + AerospikeInitContainerDefaultRepoAndTag = "aerospike-kubernetes-init:2.2.0-dev1" AerospikeAppLabel = "app" AerospikeCustomResourceLabel = "aerospike.com/cr" AerospikeRackIDLabel = "aerospike.com/rack-id" @@ -205,7 +205,7 @@ func IsSecurityEnabled( return false, nil } - if errors.Is(err, internalerrors.ErrInvalidOrEmpty) && retval >= 0 { + if errors.Is(err, internalerrors.ErrInvalidOrEmpty) { return true, nil } diff --git a/config/crd/bases/asdb.aerospike.com_aerospikeclusters.yaml b/config/crd/bases/asdb.aerospike.com_aerospikeclusters.yaml index 4fe9c6c14..25ad29de4 100644 --- a/config/crd/bases/asdb.aerospike.com_aerospikeclusters.yaml +++ b/config/crd/bases/asdb.aerospike.com_aerospikeclusters.yaml @@ -14255,6 +14255,10 @@ spec: items: type: string type: array + isSecurityEnabled: + description: IsSecurityEnabled is true if security is enabled + in the pod + type: boolean networkPolicyHash: description: NetworkPolicyHash is ripemd160 hash of NetworkPolicy used by this pod @@ -14278,6 +14282,7 @@ spec: required: - aerospikeConfigHash - image + - isSecurityEnabled - networkPolicyHash - podIP - podPort diff --git a/controllers/access_control.go b/controllers/access_control.go index 96fea04ae..4c673ec34 100644 --- a/controllers/access_control.go +++ b/controllers/access_control.go @@ -37,38 +37,34 @@ func AerospikeAdminCredentials( desiredState, currentState *asdbv1.AerospikeClusterSpec, passwordProvider AerospikeUserPasswordProvider, ) (user, pass string, err error) { - var enabled bool - - outgoingVersion, err := asdbv1.GetImageVersion(currentState.Image) - if err != nil { - incomingVersion, newErr := asdbv1.GetImageVersion(desiredState.Image) - if newErr != nil { - return "", "", newErr - } + var ( + enabled bool + currentSecurityErr error + desiredSecurityErr error + ) - enabled, newErr = asdbv1.IsSecurityEnabled( + incomingVersion, incomingVersionErr := asdbv1.GetImageVersion(desiredState.Image) + if incomingVersionErr == nil { + enabled, 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 !enabled { + outgoingVersion, outgoingVersionErr := asdbv1.GetImageVersion(currentState.Image) + if outgoingVersionErr == nil { + // It is possible that this is a new cluster and current state is empty. + enabled, currentSecurityErr = asdbv1.IsSecurityEnabled( + outgoingVersion, currentState.AerospikeConfig, ) - if newErr != nil { - return "", "", newErr - } + } else { + currentSecurityErr = outgoingVersionErr + } + + if currentSecurityErr != nil && desiredSecurityErr != nil { + return "", "", desiredSecurityErr } } diff --git a/controllers/reconciler.go b/controllers/reconciler.go index bef0c522b..18496827d 100644 --- a/controllers/reconciler.go +++ b/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,62 @@ 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].IsSecurityEnabled { + 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.enablingSecurity() { + return nil // No need to proceed if security is not to be enabling + } + + securityEnabledPods, err := r.getSecurityEnabledPods() + if err != nil { + return err + } + + if len(securityEnabledPods) == 0 { + return nil // No security-enabled pods found + } + + 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 +} diff --git a/helm-charts/aerospike-kubernetes-operator/crds/customresourcedefinition_aerospikeclusters.asdb.aerospike.com.yaml b/helm-charts/aerospike-kubernetes-operator/crds/customresourcedefinition_aerospikeclusters.asdb.aerospike.com.yaml index 4fe9c6c14..25ad29de4 100644 --- a/helm-charts/aerospike-kubernetes-operator/crds/customresourcedefinition_aerospikeclusters.asdb.aerospike.com.yaml +++ b/helm-charts/aerospike-kubernetes-operator/crds/customresourcedefinition_aerospikeclusters.asdb.aerospike.com.yaml @@ -14255,6 +14255,10 @@ spec: items: type: string type: array + isSecurityEnabled: + description: IsSecurityEnabled is true if security is enabled + in the pod + type: boolean networkPolicyHash: description: NetworkPolicyHash is ripemd160 hash of NetworkPolicy used by this pod @@ -14278,6 +14282,7 @@ spec: required: - aerospikeConfigHash - image + - isSecurityEnabled - networkPolicyHash - podIP - podPort diff --git a/test/access_control_test.go b/test/access_control_test.go index 70e2a237c..13240a62d 100644 --- a/test/access_control_test.go +++ b/test/access_control_test.go @@ -1627,7 +1627,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 @@ -1713,11 +1713,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 { @@ -1891,7 +1888,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") }