From 908a9b8e8616b15d57e53de29f56f280319baa38 Mon Sep 17 00:00:00 2001 From: Tanmay Jain Date: Wed, 14 Feb 2024 17:37:51 +0530 Subject: [PATCH 01/10] enable security dynamically --- api/v1/aerospikecluster_types.go | 3 + api/v1/aerospikecluster_validating_webhook.go | 99 +------------------ api/v1/utils.go | 6 +- .../asdb.aerospike.com_aerospikeclusters.yaml | 5 + controllers/access_control.go | 57 ++++++----- controllers/rack.go | 28 ++++++ controllers/reconciler.go | 11 +++ 7 files changed, 84 insertions(+), 125 deletions(-) diff --git a/api/v1/aerospikecluster_types.go b/api/v1/aerospikecluster_types.go index 9346610f5..bf673a0e3 100644 --- a/api/v1/aerospikecluster_types.go +++ b/api/v1/aerospikecluster_types.go @@ -827,6 +827,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 diff --git a/api/v1/aerospikecluster_validating_webhook.go b/api/v1/aerospikecluster_validating_webhook.go index 986bd7428..0a8e205fa 100644 --- a/api/v1/aerospikecluster_validating_webhook.go +++ b/api/v1/aerospikecluster_validating_webhook.go @@ -19,7 +19,6 @@ package v1 import ( "crypto/x509" "encoding/pem" - "errors" "fmt" "os" "path/filepath" @@ -38,7 +37,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/webhook" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" - internalerrors "github.com/aerospike/aerospike-kubernetes-operator/errors" "github.com/aerospike/aerospike-management-lib/asconfig" "github.com/aerospike/aerospike-management-lib/deployment" ) @@ -116,8 +114,7 @@ func (c *AerospikeCluster) ValidateUpdate(oldObj runtime.Object) (admission.Warn // Validate AerospikeConfig update if err := validateAerospikeConfigUpdate( - aslog, incomingVersion, outgoingVersion, - c.Spec.AerospikeConfig, old.Spec.AerospikeConfig, + aslog, c.Spec.AerospikeConfig, old.Spec.AerospikeConfig, c.Status.AerospikeConfig, ); err != nil { return nil, err @@ -392,16 +389,6 @@ func (c *AerospikeCluster) validateRackUpdate( return nil } - outgoingVersion, err := GetImageVersion(old.Spec.Image) - if err != nil { - return err - } - - incomingVersion, err := GetImageVersion(c.Spec.Image) - if err != nil { - return err - } - // Old racks cannot be updated // Also need to exclude a default rack with default rack ID. No need to check here, // user should not provide or update default rackID @@ -435,8 +422,7 @@ func (c *AerospikeCluster) validateRackUpdate( // Validate aerospikeConfig update if err := validateAerospikeConfigUpdate( - aslog, incomingVersion, outgoingVersion, - &newRack.AerospikeConfig, &oldRack.AerospikeConfig, + aslog, &newRack.AerospikeConfig, &oldRack.AerospikeConfig, rackStatusConfig, ); err != nil { return fmt.Errorf( @@ -1266,90 +1252,11 @@ func getNamespaceReplicationFactor(nsConf map[string]interface{}) (int, error) { return rf, nil } -func validateSecurityConfigUpdate( - newVersion, oldVersion string, newSpec, oldSpec *AerospikeConfigSpec, -) error { - nv, err := lib.CompareVersions(newVersion, "5.7.0") - if err != nil { - return err - } - - ov, err := lib.CompareVersions(oldVersion, "5.7.0") - if err != nil { - return err - } - - if nv >= 0 || ov >= 0 { - return validateSecurityContext(newVersion, oldVersion, newSpec, oldSpec) - } - - return validateEnableSecurityConfig(newSpec, oldSpec) -} - -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 { - 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") - } - } - - return nil -} - -func validateSecurityContext( - newVersion, oldVersion string, newSpec, oldSpec *AerospikeConfigSpec, -) error { - ovflag, err := IsSecurityEnabled(oldVersion, oldSpec) - if err != nil { - if !errors.Is(err, internalerrors.ErrNotFound) { - return fmt.Errorf( - "validateEnableSecurityConfig got an error - oldVersion: %s: %w", - oldVersion, err, - ) - } - } - - ivflag, err := IsSecurityEnabled(newVersion, newSpec) - if err != nil { - if !errors.Is(err, internalerrors.ErrNotFound) { - return fmt.Errorf( - "validateEnableSecurityConfig got an error: %w", err, - ) - } - } - - if ivflag != ovflag { - return fmt.Errorf("cannot update cluster security config enable-security was changed") - } - - return nil -} - func validateAerospikeConfigUpdate( - aslog logr.Logger, incomingVersion, outgoingVersion string, - incomingSpec, outgoingSpec, currentStatus *AerospikeConfigSpec, + aslog logr.Logger, incomingSpec, outgoingSpec, currentStatus *AerospikeConfigSpec, ) error { aslog.Info("Validate AerospikeConfig update") - if err := validateSecurityConfigUpdate( - incomingVersion, outgoingVersion, incomingSpec, outgoingSpec, - ); err != nil { - return err - } - newConf := incomingSpec.Value oldConf := outgoingSpec.Value diff --git a/api/v1/utils.go b/api/v1/utils.go index ff11d8281..0c6e7cff2 100644 --- a/api/v1/utils.go +++ b/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 } diff --git a/config/crd/bases/asdb.aerospike.com_aerospikeclusters.yaml b/config/crd/bases/asdb.aerospike.com_aerospikeclusters.yaml index 723aa4f7a..68ec6296a 100644 --- a/config/crd/bases/asdb.aerospike.com_aerospikeclusters.yaml +++ b/config/crd/bases/asdb.aerospike.com_aerospikeclusters.yaml @@ -14208,6 +14208,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. @@ -14220,6 +14224,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 diff --git a/controllers/access_control.go b/controllers/access_control.go index 96fea04ae..52f552fe8 100644 --- a/controllers/access_control.go +++ b/controllers/access_control.go @@ -37,38 +37,43 @@ 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 { + // If security is enabled in either current or desired state, return security enabled credentials. + enabled = currentSecurityEnabled || desiredSecurityEnabled + } else if currentSecurityErr != nil && desiredSecurityErr != nil { + return "", "", desiredSecurityErr + } else { + if currentSecurityErr != nil { + enabled = desiredSecurityEnabled + } else { + enabled = currentSecurityEnabled } } diff --git a/controllers/rack.go b/controllers/rack.go index 66e269349..17843896d 100644 --- a/controllers/rack.go +++ b/controllers/rack.go @@ -120,6 +120,34 @@ func (r *SingleClusterReconciler) reconcileRacks() reconcileResult { } } + if r.aeroCluster.Status.Pods != nil { + securityDisabledPodNames := r.getsecurityDisabledPodNames() + + // Setup access control. + if err := r.validateAndReconcileAccessControl(securityDisabledPodNames.Union(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 reconcileError(err) + } + + // 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, + ) + + return reconcileError(err) + } + } + for idx := range rackStateList { state := &rackStateList[idx] found := &appsv1.StatefulSet{} diff --git a/controllers/reconciler.go b/controllers/reconciler.go index 58b4417bb..586a7116c 100644 --- a/controllers/reconciler.go +++ b/controllers/reconciler.go @@ -919,3 +919,14 @@ func (r *SingleClusterReconciler) AddAPIVersionLabel(ctx context.Context) error return r.Client.Update(ctx, aeroCluster, updateOption) } + +func (r *SingleClusterReconciler) getsecurityDisabledPodNames() sets.Set[string] { + securityDisabledPods := sets.Set[string]{} + for podName := range r.aeroCluster.Status.Pods { + if !r.aeroCluster.Status.Pods[podName].SecurityEnabled { + securityDisabledPods.Insert(podName) + } + } + + return securityDisabledPods +} From ec4b349502027360b8b799bf9a18ca0b71019fb8 Mon Sep 17 00:00:00 2001 From: Tanmay Jain Date: Mon, 19 Feb 2024 14:04:31 +0530 Subject: [PATCH 02/10] disallow disabling security in running cluster --- api/v1/aerospikecluster_validating_webhook.go | 106 ++++++++++++++++- controllers/access_control.go | 13 +-- controllers/rack.go | 28 ----- controllers/reconciler.go | 109 +++++++++++++----- test/access_control_test.go | 11 +- 5 files changed, 193 insertions(+), 74 deletions(-) diff --git a/api/v1/aerospikecluster_validating_webhook.go b/api/v1/aerospikecluster_validating_webhook.go index 0a8e205fa..0fa995e08 100644 --- a/api/v1/aerospikecluster_validating_webhook.go +++ b/api/v1/aerospikecluster_validating_webhook.go @@ -19,6 +19,7 @@ package v1 import ( "crypto/x509" "encoding/pem" + "errors" "fmt" "os" "path/filepath" @@ -37,6 +38,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/webhook" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + internalerrors "github.com/aerospike/aerospike-kubernetes-operator/errors" "github.com/aerospike/aerospike-management-lib/asconfig" "github.com/aerospike/aerospike-management-lib/deployment" ) @@ -114,7 +116,8 @@ func (c *AerospikeCluster) ValidateUpdate(oldObj runtime.Object) (admission.Warn // Validate AerospikeConfig update if err := validateAerospikeConfigUpdate( - aslog, c.Spec.AerospikeConfig, old.Spec.AerospikeConfig, + aslog, incomingVersion, outgoingVersion, + c.Spec.AerospikeConfig, old.Spec.AerospikeConfig, c.Status.AerospikeConfig, ); err != nil { return nil, err @@ -389,6 +392,16 @@ func (c *AerospikeCluster) validateRackUpdate( return nil } + outgoingVersion, err := GetImageVersion(old.Spec.Image) + if err != nil { + return err + } + + incomingVersion, err := GetImageVersion(c.Spec.Image) + if err != nil { + return err + } + // Old racks cannot be updated // Also need to exclude a default rack with default rack ID. No need to check here, // user should not provide or update default rackID @@ -422,7 +435,8 @@ func (c *AerospikeCluster) validateRackUpdate( // Validate aerospikeConfig update if err := validateAerospikeConfigUpdate( - aslog, &newRack.AerospikeConfig, &oldRack.AerospikeConfig, + aslog, incomingVersion, outgoingVersion, + &newRack.AerospikeConfig, &oldRack.AerospikeConfig, rackStatusConfig, ); err != nil { return fmt.Errorf( @@ -1252,11 +1266,97 @@ func getNamespaceReplicationFactor(nsConf map[string]interface{}) (int, error) { return rf, nil } +func validateSecurityConfigUpdate( + newVersion, oldVersion string, newSpec, oldSpec *AerospikeConfigSpec, +) error { + nv, err := lib.CompareVersions(newVersion, "5.7.0") + if err != nil { + return err + } + + ov, err := lib.CompareVersions(oldVersion, "5.7.0") + if err != nil { + return err + } + + if nv >= 0 || ov >= 0 { + return validateSecurityContext(newVersion, oldVersion, newSpec, oldSpec) + } + + return validateEnableSecurityConfig(newSpec, oldSpec) +} + +func validateEnableSecurityConfig(newConfSpec, oldConfSpec *AerospikeConfigSpec) error { + newConf := newConfSpec.Value + oldConf := oldConfSpec.Value + 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 && oldSecFlag.(bool) && !newEnableSecurityFlagFound { + return fmt.Errorf("cannot disable cluster security in running cluster") + } + + if oldEnableSecurityFlagFound && newEnableSecurityFlagFound || !reflect.DeepEqual( + oldSecFlag, newSecFlag, + ) { + if oldSecFlag.(bool) && !newSecFlag.(bool) { + return fmt.Errorf("cannot disable cluster security in running cluster") + } + } + } + + return nil +} + +func validateSecurityContext( + newVersion, oldVersion string, newSpec, oldSpec *AerospikeConfigSpec, +) error { + ovflag, err := IsSecurityEnabled(oldVersion, oldSpec) + if err != nil { + if !errors.Is(err, internalerrors.ErrNotFound) { + return fmt.Errorf( + "validateEnableSecurityConfig got an error - oldVersion: %s: %w", + oldVersion, err, + ) + } + } + + ivflag, err := IsSecurityEnabled(newVersion, newSpec) + if err != nil { + if !errors.Is(err, internalerrors.ErrNotFound) { + return fmt.Errorf( + "validateEnableSecurityConfig got an error: %w", err, + ) + } + } + + if !ivflag && ovflag { + return fmt.Errorf("cannot disable cluster security in running cluster") + } + + return nil +} + func validateAerospikeConfigUpdate( - aslog logr.Logger, incomingSpec, outgoingSpec, currentStatus *AerospikeConfigSpec, + aslog logr.Logger, incomingVersion, outgoingVersion string, + incomingSpec, outgoingSpec, currentStatus *AerospikeConfigSpec, ) error { aslog.Info("Validate AerospikeConfig update") + if err := validateSecurityConfigUpdate( + incomingVersion, outgoingVersion, incomingSpec, outgoingSpec, + ); err != nil { + return err + } + newConf := incomingSpec.Value oldConf := outgoingSpec.Value diff --git a/controllers/access_control.go b/controllers/access_control.go index 52f552fe8..a362ff5ed 100644 --- a/controllers/access_control.go +++ b/controllers/access_control.go @@ -64,19 +64,12 @@ func AerospikeAdminCredentials( desiredSecurityErr = incomingVersionErr } - if currentSecurityErr == nil && desiredSecurityErr == nil { - // If security is enabled in either current or desired state, return security enabled credentials. - enabled = currentSecurityEnabled || desiredSecurityEnabled - } else if currentSecurityErr != nil && desiredSecurityErr != nil { + if currentSecurityErr != nil && desiredSecurityErr != nil { return "", "", desiredSecurityErr - } else { - if currentSecurityErr != nil { - enabled = desiredSecurityEnabled - } else { - enabled = currentSecurityEnabled - } } + enabled = currentSecurityEnabled || desiredSecurityEnabled + if !enabled { // Return zero strings if this is not a security enabled cluster. return "", "", nil diff --git a/controllers/rack.go b/controllers/rack.go index 17843896d..66e269349 100644 --- a/controllers/rack.go +++ b/controllers/rack.go @@ -120,34 +120,6 @@ func (r *SingleClusterReconciler) reconcileRacks() reconcileResult { } } - if r.aeroCluster.Status.Pods != nil { - securityDisabledPodNames := r.getsecurityDisabledPodNames() - - // Setup access control. - if err := r.validateAndReconcileAccessControl(securityDisabledPodNames.Union(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 reconcileError(err) - } - - // 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, - ) - - return reconcileError(err) - } - } - for idx := range rackStateList { state := &rackStateList[idx] found := &appsv1.StatefulSet{} diff --git a/controllers/reconciler.go b/controllers/reconciler.go index 586a7116c..e1a96405a 100644 --- a/controllers/reconciler.go +++ b/controllers/reconciler.go @@ -100,6 +100,34 @@ func (r *SingleClusterReconciler) Reconcile() (ctrl.Result, error) { return reconcile.Result{}, err } + if r.aeroCluster.Status.Pods != nil && r.enablingSecurity() { + securityEnabledPods, err := r.getsecurityEnabledPods() + if err != nil { + return reconcile.Result{}, 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 reconcile.Result{}, 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 reconcile.Result{}, err + } + } + } + // Reconcile all racks if res := r.reconcileRacks(); !res.isSuccess { if res.err != nil { @@ -144,7 +172,7 @@ func (r *SingleClusterReconciler) Reconcile() (ctrl.Result, error) { return reconcile.Result{}, e } - if err := deployment.InfoQuiesceUndo( + if err = deployment.InfoQuiesceUndo( r.Log, r.getClientPolicy(), allHostConns, ); err != nil { @@ -152,8 +180,14 @@ func (r *SingleClusterReconciler) Reconcile() (ctrl.Result, error) { return reconcile.Result{}, err } + podList, err := r.getClusterPodList() + if err != nil { + return reconcile.Result{}, err + } + // Setup access control. - if err := r.validateAndReconcileAccessControl(ignorablePodNames); err != nil { + // Assuming all pods must be security enabled or disabled. + if err := r.validateAndReconcileAccessControl(podList.Items, ignorablePodNames); err != nil { r.Log.Error(err, "Failed to Reconcile access control") r.Recorder.Eventf( r.aeroCluster, corev1.EventTypeWarning, "ACLUpdateFailed", @@ -164,18 +198,6 @@ func (r *SingleClusterReconciler) Reconcile() (ctrl.Result, error) { return reconcile.Result{}, err } - // 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, - ) - - return reconcile.Result{}, err - } - // Use policy from spec after setting up access control policy := r.getClientPolicy() @@ -264,7 +286,8 @@ func (r *SingleClusterReconciler) recoverIgnorablePods() reconcileResult { return reconcileSuccess() } -func (r *SingleClusterReconciler) validateAndReconcileAccessControl(ignorablePodNames sets.Set[string]) error { +func (r *SingleClusterReconciler) validateAndReconcileAccessControl(securityEnabledPods []corev1.Pod, + ignorablePodNames sets.Set[string]) error { version, err := asdbv1.GetImageVersion(r.aeroCluster.Spec.Image) if err != nil { return err @@ -282,8 +305,10 @@ func (r *SingleClusterReconciler) validateAndReconcileAccessControl(ignorablePod return nil } + var conns []*deployment.HostConn + // Create client - conns, err := r.newAllHostConnWithOption(ignorablePodNames) + conns, err = r.newPodsHostConnWithOption(securityEnabledPods, ignorablePodNames) if err != nil { return fmt.Errorf("failed to get host info: %v", err) } @@ -302,6 +327,7 @@ func (r *SingleClusterReconciler) validateAndReconcileAccessControl(ignorablePod // Create policy using status, status has current connection info clientPolicy := r.getClientPolicy() + aeroClient, err := as.NewClientWithPolicyAndHost(clientPolicy, hosts...) if err != nil { @@ -315,15 +341,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 { @@ -920,13 +961,29 @@ func (r *SingleClusterReconciler) AddAPIVersionLabel(ctx context.Context) error return r.Client.Update(ctx, aeroCluster, updateOption) } -func (r *SingleClusterReconciler) getsecurityDisabledPodNames() sets.Set[string] { - securityDisabledPods := sets.Set[string]{} +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 { - securityDisabledPods.Insert(podName) + 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 securityDisabledPods + return securityEnabledPods, nil +} + +func (r *SingleClusterReconciler) enablingSecurity() bool { + if r.aeroCluster.Spec.AerospikeAccessControl != nil && r.aeroCluster.Status.AerospikeAccessControl == nil { + return true + } + + return false } diff --git a/test/access_control_test.go b/test/access_control_test.go index 08eb10ebf..b205b7843 100644 --- a/test/access_control_test.go +++ b/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") } From 19aa4ad1987053e15b407e71b6a7ca9ad438c379 Mon Sep 17 00:00:00 2001 From: Tanmay Jain Date: Tue, 20 Feb 2024 17:47:27 +0530 Subject: [PATCH 03/10] adding securityEnabled flag in helm crd --- api/v1/aerospikecluster_validating_webhook.go | 10 +--------- ...efinition_aerospikeclusters.asdb.aerospike.com.yaml | 5 +++++ 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/api/v1/aerospikecluster_validating_webhook.go b/api/v1/aerospikecluster_validating_webhook.go index 0fa995e08..8dec672f2 100644 --- a/api/v1/aerospikecluster_validating_webhook.go +++ b/api/v1/aerospikecluster_validating_webhook.go @@ -1300,17 +1300,9 @@ func validateEnableSecurityConfig(newConfSpec, oldConfSpec *AerospikeConfigSpec) oldSecFlag, oldEnableSecurityFlagFound := oldSec.(map[string]interface{})["enable-security"] newSecFlag, newEnableSecurityFlagFound := newSec.(map[string]interface{})["enable-security"] - if oldEnableSecurityFlagFound && oldSecFlag.(bool) && !newEnableSecurityFlagFound { + if oldEnableSecurityFlagFound && oldSecFlag.(bool) && (!newEnableSecurityFlagFound || !newSecFlag.(bool)) { return fmt.Errorf("cannot disable cluster security in running cluster") } - - if oldEnableSecurityFlagFound && newEnableSecurityFlagFound || !reflect.DeepEqual( - oldSecFlag, newSecFlag, - ) { - if oldSecFlag.(bool) && !newSecFlag.(bool) { - return fmt.Errorf("cannot disable cluster security in running cluster") - } - } } 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 2d462a3a7..070d4e753 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 @@ -14209,6 +14209,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. @@ -14221,6 +14225,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 From fba1862eb1b49b18f03c8b60a83436823edac657 Mon Sep 17 00:00:00 2001 From: Tanmay Jain Date: Fri, 23 Feb 2024 16:46:23 +0530 Subject: [PATCH 04/10] small restructuring --- controllers/reconciler.go | 32 +++++++++++++++----------------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/controllers/reconciler.go b/controllers/reconciler.go index e1a96405a..bb9d24c51 100644 --- a/controllers/reconciler.go +++ b/controllers/reconciler.go @@ -101,7 +101,7 @@ func (r *SingleClusterReconciler) Reconcile() (ctrl.Result, error) { } if r.aeroCluster.Status.Pods != nil && r.enablingSecurity() { - securityEnabledPods, err := r.getsecurityEnabledPods() + securityEnabledPods, err := r.getSecurityEnabledPods() if err != nil { return reconcile.Result{}, err } @@ -180,14 +180,9 @@ func (r *SingleClusterReconciler) Reconcile() (ctrl.Result, error) { return reconcile.Result{}, err } - podList, err := r.getClusterPodList() - if err != nil { - return reconcile.Result{}, err - } - // Setup access control. // Assuming all pods must be security enabled or disabled. - if err := r.validateAndReconcileAccessControl(podList.Items, ignorablePodNames); err != nil { + 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", @@ -286,7 +281,7 @@ func (r *SingleClusterReconciler) recoverIgnorablePods() reconcileResult { return reconcileSuccess() } -func (r *SingleClusterReconciler) validateAndReconcileAccessControl(securityEnabledPods []corev1.Pod, +func (r *SingleClusterReconciler) validateAndReconcileAccessControl(selectedPods []corev1.Pod, ignorablePodNames sets.Set[string]) error { version, err := asdbv1.GetImageVersion(r.aeroCluster.Spec.Image) if err != nil { @@ -308,9 +303,16 @@ func (r *SingleClusterReconciler) validateAndReconcileAccessControl(securityEnab var conns []*deployment.HostConn // Create client - conns, err = r.newPodsHostConnWithOption(securityEnabledPods, 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)) @@ -961,7 +963,7 @@ func (r *SingleClusterReconciler) AddAPIVersionLabel(ctx context.Context) error return r.Client.Update(ctx, aeroCluster, updateOption) } -func (r *SingleClusterReconciler) getsecurityEnabledPods() ([]corev1.Pod, error) { +func (r *SingleClusterReconciler) getSecurityEnabledPods() ([]corev1.Pod, error) { securityEnabledPods := make([]corev1.Pod, 0, len(r.aeroCluster.Status.Pods)) for podName := range r.aeroCluster.Status.Pods { @@ -981,9 +983,5 @@ func (r *SingleClusterReconciler) getsecurityEnabledPods() ([]corev1.Pod, error) } func (r *SingleClusterReconciler) enablingSecurity() bool { - if r.aeroCluster.Spec.AerospikeAccessControl != nil && r.aeroCluster.Status.AerospikeAccessControl == nil { - return true - } - - return false + return r.aeroCluster.Spec.AerospikeAccessControl != nil && r.aeroCluster.Status.AerospikeAccessControl == nil } From f26f3f0f8a062442c739a8252e37d12cd16f29d8 Mon Sep 17 00:00:00 2001 From: Tanmay Jain Date: Mon, 4 Mar 2024 15:45:16 +0530 Subject: [PATCH 05/10] fixing lint --- ...rnetes-operator.clusterserviceversion.yaml | 7 ++- controllers/reconciler.go | 60 +++++++++++-------- 2 files changed, 39 insertions(+), 28 deletions(-) diff --git a/config/manifests/bases/aerospike-kubernetes-operator.clusterserviceversion.yaml b/config/manifests/bases/aerospike-kubernetes-operator.clusterserviceversion.yaml index e9cec80a0..64dcdffae 100644 --- a/config/manifests/bases/aerospike-kubernetes-operator.clusterserviceversion.yaml +++ b/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. diff --git a/controllers/reconciler.go b/controllers/reconciler.go index 8a0fa6184..8ca37bec9 100644 --- a/controllers/reconciler.go +++ b/controllers/reconciler.go @@ -119,32 +119,8 @@ func (r *SingleClusterReconciler) Reconcile() (result ctrl.Result, recErr error) return reconcile.Result{}, recErr } - if r.aeroCluster.Status.Pods != nil && r.enablingSecurity() { - securityEnabledPods, err := r.getSecurityEnabledPods() - if err != nil { - return reconcile.Result{}, 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 reconcile.Result{}, 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 reconcile.Result{}, err - } - } + if err := r.handleEnableSecurity(); err != nil { + return reconcile.Result{}, err } // Reconcile all racks @@ -1046,3 +1022,35 @@ func (r *SingleClusterReconciler) getSecurityEnabledPods() ([]corev1.Pod, error) 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 +} From 572f8bf903d82a2bb5f00d77787b82fe4dd53875 Mon Sep 17 00:00:00 2001 From: Tanmay Jain Date: Mon, 18 Mar 2024 13:25:33 +0530 Subject: [PATCH 06/10] addressing comments --- api/v1/aerospikecluster_types.go | 4 +- .../asdb.aerospike.com_aerospikeclusters.yaml | 10 ++-- controllers/reconciler.go | 50 ++++++++++--------- ..._aerospikeclusters.asdb.aerospike.com.yaml | 10 ++-- 4 files changed, 39 insertions(+), 35 deletions(-) diff --git a/api/v1/aerospikecluster_types.go b/api/v1/aerospikecluster_types.go index fd01c5703..5e42a1c79 100644 --- a/api/v1/aerospikecluster_types.go +++ b/api/v1/aerospikecluster_types.go @@ -872,8 +872,8 @@ 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"` + // IsSecurityEnabled is true if security is enabled in the pod + IsSecurityEnabled bool `json:"isSecurityEnabled"` } // +kubebuilder:object:root=true diff --git a/config/crd/bases/asdb.aerospike.com_aerospikeclusters.yaml b/config/crd/bases/asdb.aerospike.com_aerospikeclusters.yaml index 757c0d37a..648fde7c5 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 @@ -14270,10 +14274,6 @@ 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,11 +14282,11 @@ spec: required: - aerospikeConfigHash - image + - isSecurityEnabled - networkPolicyHash - 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 diff --git a/controllers/reconciler.go b/controllers/reconciler.go index 8ca37bec9..18496827d 100644 --- a/controllers/reconciler.go +++ b/controllers/reconciler.go @@ -1004,7 +1004,7 @@ 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 { + if r.aeroCluster.Status.Pods[podName].IsSecurityEnabled { pod := &corev1.Pod{} podName := types.NamespacedName{Name: podName, Namespace: r.aeroCluster.Namespace} @@ -1024,32 +1024,36 @@ func (r *SingleClusterReconciler) enablingSecurity() bool { } func (r *SingleClusterReconciler) handleEnableSecurity() error { - if r.aeroCluster.Status.Pods != nil && r.enablingSecurity() { - securityEnabledPods, err := r.getSecurityEnabledPods() - if err != nil { - return err - } + if !r.enablingSecurity() { + return nil // No need to proceed if security is not to be enabling + } - 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") + securityEnabledPods, err := r.getSecurityEnabledPods() + if err != nil { + return err + } - return err - } + if len(securityEnabledPods) == 0 { + return nil // No security-enabled pods found + } - // 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, - ) + ignorablePodNames, err := r.getIgnorablePods(nil, getConfiguredRackStateList(r.aeroCluster)) + if err != nil { + r.Log.Error(err, "Failed to determine pods to be ignored") - return err - } - } + 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 757c0d37a..648fde7c5 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 @@ -14270,10 +14274,6 @@ 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,11 +14282,11 @@ spec: required: - aerospikeConfigHash - image + - isSecurityEnabled - networkPolicyHash - 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 From 01c36b20e169ca74df2f46fd4d584f1f282f2173 Mon Sep 17 00:00:00 2001 From: Tanmay Jain Date: Tue, 19 Mar 2024 11:00:25 +0530 Subject: [PATCH 07/10] addressing comments --- api/v1/aerospikecluster_validating_webhook.go | 30 ++++++++++----- api/v1/utils.go | 2 +- controllers/access_control.go | 38 +++++++++---------- 3 files changed, 40 insertions(+), 30 deletions(-) diff --git a/api/v1/aerospikecluster_validating_webhook.go b/api/v1/aerospikecluster_validating_webhook.go index c967a6db4..09d75b0c0 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, isSecurityEnabledPodExist bool, ) error { nv, err := lib.CompareVersions(newVersion, "5.7.0") if err != nil { @@ -1286,13 +1286,13 @@ func validateSecurityConfigUpdate( } 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 oldSec, oldSecConfFound := oldConf["security"] @@ -1306,7 +1306,8 @@ func validateEnableSecurityConfig(newConfSpec, oldConfSpec *AerospikeConfigSpec) oldSecFlag, oldEnableSecurityFlagFound := oldSec.(map[string]interface{})["enable-security"] newSecFlag, newEnableSecurityFlagFound := newSec.(map[string]interface{})["enable-security"] - if oldEnableSecurityFlagFound && oldSecFlag.(bool) && (!newEnableSecurityFlagFound || !newSecFlag.(bool)) { + if oldEnableSecurityFlagFound && oldSecFlag.(bool) && (!newEnableSecurityFlagFound || !newSecFlag.(bool)) && + isSecurityEnabledPodExist { return fmt.Errorf("cannot disable cluster security in running cluster") } } @@ -1315,7 +1316,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 { @@ -1336,7 +1337,7 @@ func validateSecurityContext( } } - if !ivflag && ovflag { + if !ivflag && ovflag && isSecurityEnabledPodExist { return fmt.Errorf("cannot disable cluster security in running cluster") } @@ -1346,11 +1347,22 @@ func validateSecurityContext( func validateAerospikeConfigUpdate( aslog logr.Logger, incomingVersion, outgoingVersion string, incomingSpec, outgoingSpec, currentStatus *AerospikeConfigSpec, + podStatus map[string]AerospikePodStatus, ) error { aslog.Info("Validate AerospikeConfig update") + isSecurityEnabledPodExist := false + + for pod := range podStatus { + if podStatus[pod].IsSecurityEnabled { + isSecurityEnabledPodExist = true + break + } + } + if err := validateSecurityConfigUpdate( incomingVersion, outgoingVersion, incomingSpec, outgoingSpec, + isSecurityEnabledPodExist, ); err != nil { return err } diff --git a/api/v1/utils.go b/api/v1/utils.go index f0fe16d25..e99a1a1d4 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 = "tanmayj10" - AerospikeInitContainerDefaultRepoAndTag = "aerospike-kubernetes-init:2.1.1-secenabled" + AerospikeInitContainerDefaultRepoAndTag = "aerospike-kubernetes-init:2.1.1-secenabled1" AerospikeAppLabel = "app" AerospikeCustomResourceLabel = "aerospike.com/cr" AerospikeRackIDLabel = "aerospike.com/rack-id" diff --git a/controllers/access_control.go b/controllers/access_control.go index a362ff5ed..4c673ec34 100644 --- a/controllers/access_control.go +++ b/controllers/access_control.go @@ -38,37 +38,35 @@ func AerospikeAdminCredentials( passwordProvider AerospikeUserPasswordProvider, ) (user, pass string, err error) { var ( - enabled bool - currentSecurityEnabled bool - desiredSecurityEnabled bool - currentSecurityErr error - desiredSecurityErr error + enabled bool + currentSecurityErr error + desiredSecurityErr error ) - 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 - } - incomingVersion, incomingVersionErr := asdbv1.GetImageVersion(desiredState.Image) if incomingVersionErr == nil { - desiredSecurityEnabled, desiredSecurityErr = asdbv1.IsSecurityEnabled( + enabled, desiredSecurityErr = asdbv1.IsSecurityEnabled( incomingVersion, desiredState.AerospikeConfig, ) } else { desiredSecurityErr = incomingVersionErr } - if currentSecurityErr != nil && desiredSecurityErr != nil { - return "", "", desiredSecurityErr - } + 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, + ) + } else { + currentSecurityErr = outgoingVersionErr + } - enabled = currentSecurityEnabled || desiredSecurityEnabled + if currentSecurityErr != nil && desiredSecurityErr != nil { + return "", "", desiredSecurityErr + } + } if !enabled { // Return zero strings if this is not a security enabled cluster. From 13756a18b6cb622fdf1a8806c83589510a6c6140 Mon Sep 17 00:00:00 2001 From: Tanmay Jain Date: Tue, 19 Mar 2024 13:21:12 +0530 Subject: [PATCH 08/10] correcting init tag --- api/v1/utils.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/v1/utils.go b/api/v1/utils.go index 9ad644f8e..b95881795 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-dev" + AerospikeInitContainerDefaultRepoAndTag = "aerospike-kubernetes-init:2.2.0-dev" AerospikeAppLabel = "app" AerospikeCustomResourceLabel = "aerospike.com/cr" AerospikeRackIDLabel = "aerospike.com/rack-id" From 96cc9e7909a5837fe2a65ec3e7914369887b1d59 Mon Sep 17 00:00:00 2001 From: Tanmay Jain Date: Tue, 19 Mar 2024 14:39:49 +0530 Subject: [PATCH 09/10] correcting init tag --- api/v1/utils.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/v1/utils.go b/api/v1/utils.go index b95881795..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.2.0-dev" + AerospikeInitContainerDefaultRepoAndTag = "aerospike-kubernetes-init:2.2.0-dev1" AerospikeAppLabel = "app" AerospikeCustomResourceLabel = "aerospike.com/cr" AerospikeRackIDLabel = "aerospike.com/rack-id" From 6cb83579bac88330fb561cda356025076c5a5938 Mon Sep 17 00:00:00 2001 From: Tanmay Jain Date: Tue, 19 Mar 2024 16:26:18 +0530 Subject: [PATCH 10/10] addressing comment --- api/v1/aerospikecluster_validating_webhook.go | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/api/v1/aerospikecluster_validating_webhook.go b/api/v1/aerospikecluster_validating_webhook.go index 09d75b0c0..c15d55aff 100644 --- a/api/v1/aerospikecluster_validating_webhook.go +++ b/api/v1/aerospikecluster_validating_webhook.go @@ -1273,7 +1273,7 @@ func getNamespaceReplicationFactor(nsConf map[string]interface{}) (int, error) { } func validateSecurityConfigUpdate( - newVersion, oldVersion string, newSpec, oldSpec *AerospikeConfigSpec, isSecurityEnabledPodExist bool, + newVersion, oldVersion string, newSpec, oldSpec *AerospikeConfigSpec, podStatus map[string]AerospikePodStatus, ) error { nv, err := lib.CompareVersions(newVersion, "5.7.0") if err != nil { @@ -1285,6 +1285,15 @@ 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, isSecurityEnabledPodExist) } @@ -1351,18 +1360,9 @@ func validateAerospikeConfigUpdate( ) error { aslog.Info("Validate AerospikeConfig update") - isSecurityEnabledPodExist := false - - for pod := range podStatus { - if podStatus[pod].IsSecurityEnabled { - isSecurityEnabledPodExist = true - break - } - } - if err := validateSecurityConfigUpdate( incomingVersion, outgoingVersion, incomingSpec, outgoingSpec, - isSecurityEnabledPodExist, + podStatus, ); err != nil { return err }