From 9c72054c3e362fc0756a8ab901f37560f401c2d1 Mon Sep 17 00:00:00 2001 From: Sudhanshu Ranjan Date: Mon, 13 May 2024 13:32:16 +0530 Subject: [PATCH] Remove isSecurityEnabled flag. Determine enabled security by comparing the config hash of the configMap and the pod. --- api/v1/aerospikecluster_types.go | 4 - api/v1/aerospikecluster_validating_webhook.go | 55 +++---------- api/v1/utils.go | 4 +- .../asdb.aerospike.com_aerospikeclusters.yaml | 4 - controllers/rack.go | 77 +++++++++++++++++++ controllers/reconciler.go | 63 --------------- ..._aerospikeclusters.asdb.aerospike.com.yaml | 4 - 7 files changed, 90 insertions(+), 121 deletions(-) diff --git a/api/v1/aerospikecluster_types.go b/api/v1/aerospikecluster_types.go index bd9c833ba..13196062f 100644 --- a/api/v1/aerospikecluster_types.go +++ b/api/v1/aerospikecluster_types.go @@ -902,10 +902,6 @@ type AerospikePodStatus struct { //nolint:govet // for readability // DynamicConfigUpdateStatus is the status of dynamic config update operation. // Empty "" status means successful update. DynamicConfigUpdateStatus DynamicConfigUpdateStatus `json:"dynamicConfigUpdateStatus,omitempty"` - - // IsSecurityEnabled is true if security is enabled in the pod - // +optional - 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 8d6478b0b..a4cb3d191 100644 --- a/api/v1/aerospikecluster_validating_webhook.go +++ b/api/v1/aerospikecluster_validating_webhook.go @@ -117,7 +117,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.Pods, + c.Status.AerospikeConfig, ); err != nil { return nil, err } @@ -441,7 +441,7 @@ func (c *AerospikeCluster) validateRackUpdate( if err := validateAerospikeConfigUpdate( aslog, incomingVersion, outgoingVersion, &newRack.AerospikeConfig, &oldRack.AerospikeConfig, - rackStatusConfig, c.Status.Pods, + rackStatusConfig, ); err != nil { return fmt.Errorf( "invalid update in Rack(ID: %d) aerospikeConfig: %v", @@ -947,7 +947,7 @@ func readNamesFromLocalCertificate(clientCertSpec *AerospikeOperatorClientCertSp return result, err } - if len(cert.Subject.CommonName) > 0 { + if cert.Subject.CommonName != "" { result[cert.Subject.CommonName] = struct{}{} } @@ -1251,8 +1251,7 @@ func getNamespaceReplicationFactor(nsConf map[string]interface{}) (int, error) { } func validateSecurityConfigUpdate( - newVersion, oldVersion string, newSpec, oldSpec, currentStatus *AerospikeConfigSpec, - podStatus map[string]AerospikePodStatus) error { + newVersion, oldVersion string, newSpec, oldSpec, currentStatus *AerospikeConfigSpec) error { if currentStatus != nil { currentSecurityConfig, err := IsSecurityEnabled(oldVersion, currentStatus) if err != nil { @@ -1280,14 +1279,13 @@ func validateSecurityConfigUpdate( } if nv >= 0 || ov >= 0 { - return validateSecurityContext(newVersion, oldVersion, newSpec, oldSpec, podStatus) + return validateSecurityContext(newVersion, oldVersion, newSpec, oldSpec) } - return validateEnableSecurityConfig(newSpec, oldSpec, podStatus) + return validateEnableSecurityConfig(newSpec, oldSpec) } -func validateEnableSecurityConfig(newConfSpec, oldConfSpec *AerospikeConfigSpec, - podStatus map[string]AerospikePodStatus) error { +func validateEnableSecurityConfig(newConfSpec, oldConfSpec *AerospikeConfigSpec) error { newConf := newConfSpec.Value oldConf := oldConfSpec.Value oldSec, oldSecConfFound := oldConf["security"] @@ -1302,14 +1300,7 @@ func validateEnableSecurityConfig(newConfSpec, oldConfSpec *AerospikeConfigSpec, newSecFlag, newEnableSecurityFlagFound := newSec.(map[string]interface{})["enable-security"] if oldEnableSecurityFlagFound && oldSecFlag.(bool) && (!newEnableSecurityFlagFound || !newSecFlag.(bool)) { - isSecurityEnabledPodExist, err := isSecurityEnabledPodExist(podStatus) - if err != nil { - return fmt.Errorf("cannot disable cluster security in running cluster, %s", err.Error()) - } - - if isSecurityEnabledPodExist { - return fmt.Errorf("cannot disable cluster security in running cluster") - } + return fmt.Errorf("cannot disable cluster security in running cluster") } } @@ -1317,8 +1308,7 @@ func validateEnableSecurityConfig(newConfSpec, oldConfSpec *AerospikeConfigSpec, } func validateSecurityContext( - newVersion, oldVersion string, newSpec, oldSpec *AerospikeConfigSpec, podStatus map[string]AerospikePodStatus, -) error { + newVersion, oldVersion string, newSpec, oldSpec *AerospikeConfigSpec) error { ovflag, err := IsSecurityEnabled(oldVersion, oldSpec) if err != nil { if !errors.Is(err, internalerrors.ErrNotFound) { @@ -1339,44 +1329,21 @@ func validateSecurityContext( } if !ivflag && ovflag { - isSecurityEnabledPodExist, err := isSecurityEnabledPodExist(podStatus) - if err != nil { - return fmt.Errorf("cannot disable cluster security, %s", err.Error()) - } - - if isSecurityEnabledPodExist { - return fmt.Errorf("cannot disable cluster security in running cluster") - } + return fmt.Errorf("cannot disable cluster security in running cluster") } return nil } -func isSecurityEnabledPodExist(podStatus map[string]AerospikePodStatus) (bool, error) { - if len(podStatus) == 0 { - return false, fmt.Errorf("pod status is unknown") - } - - for pod := range podStatus { - if podStatus[pod].IsSecurityEnabled { - return true, nil - } - } - - return false, nil -} - 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, - currentStatus, podStatus, - ); err != nil { + currentStatus); err != nil { return err } diff --git a/api/v1/utils.go b/api/v1/utils.go index 653145710..10bbaf8d9 100644 --- a/api/v1/utils.go +++ b/api/v1/utils.go @@ -62,8 +62,8 @@ const ( AerospikeInitContainerName = "aerospike-init" AerospikeInitContainerRegistryEnvVar = "AEROSPIKE_KUBERNETES_INIT_REGISTRY" AerospikeInitContainerDefaultRegistry = "docker.io" - AerospikeInitContainerDefaultRegistryNamespace = "aerospike" - AerospikeInitContainerDefaultRepoAndTag = "aerospike-kubernetes-init:2.2.0-dev3" + AerospikeInitContainerDefaultRegistryNamespace = "sud82" + AerospikeInitContainerDefaultRepoAndTag = "aerospike-kubernetes-init-nightly:2.1.0-dev1" AerospikeAppLabel = "app" AerospikeAppLabelValue = "aerospike-cluster" AerospikeCustomResourceLabel = "aerospike.com/cr" diff --git a/config/crd/bases/asdb.aerospike.com_aerospikeclusters.yaml b/config/crd/bases/asdb.aerospike.com_aerospikeclusters.yaml index 18bd0f51a..899c699e0 100644 --- a/config/crd/bases/asdb.aerospike.com_aerospikeclusters.yaml +++ b/config/crd/bases/asdb.aerospike.com_aerospikeclusters.yaml @@ -14291,10 +14291,6 @@ 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 diff --git a/controllers/rack.go b/controllers/rack.go index 36a391847..849afb636 100644 --- a/controllers/rack.go +++ b/controllers/rack.go @@ -373,6 +373,14 @@ func (r *SingleClusterReconciler) upgradeOrRollingRestartRack(found *appsv1.Stat return found, 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.handleEnableSecurity(rackState, ignorablePodNames); err != nil { + return found, reconcileError(err) + } + // Upgrade upgradeNeeded, err := r.isRackUpgradeNeeded(rackState.Rack.ID, ignorablePodNames) if err != nil { @@ -1735,6 +1743,75 @@ func (r *SingleClusterReconciler) getCurrentRackList() ( return rackList, nil } +func (r *SingleClusterReconciler) handleEnableSecurity(rackState *RackState, ignorablePodNames sets.Set[string]) error { + if !r.enablingSecurity() { + // 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) + if err != nil { + return err + } + + if len(securityEnabledPods) == 0 { + // No security-enabled pods found + return nil + } + + // 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 +} + +func (r *SingleClusterReconciler) enablingSecurity() bool { + return r.aeroCluster.Spec.AerospikeAccessControl != nil && r.aeroCluster.Status.AerospikeAccessControl == nil +} + +func (r *SingleClusterReconciler) getPodsWithUpdatedConfigForRack(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) + } + + if len(pods) == 0 { + // No pod found for the rack + return nil, nil + } + + confMap, err := r.getConfigMap(rackState.Rack.ID) + if err != nil { + return nil, err + } + + requiredConfHash := confMap.Data[aerospikeConfHashFileName] + + securityEnabledPods := make([]corev1.Pod, 0, len(r.aeroCluster.Status.Pods)) + + for idx := range pods { + podName := pods[idx].Name + podStatus := r.aeroCluster.Status.Pods[podName] + + if podStatus.AerospikeConfigHash == requiredConfHash { + // Config hash is matching, it means config has been applied + securityEnabledPods = append(securityEnabledPods, *pods[idx]) + } + } + + return securityEnabledPods, nil +} + func isContainerNameInStorageVolumeAttachments( containerName string, mounts []asdbv1.VolumeAttachment, ) bool { diff --git a/controllers/reconciler.go b/controllers/reconciler.go index d5887d8cc..55b60f61a 100644 --- a/controllers/reconciler.go +++ b/controllers/reconciler.go @@ -119,10 +119,6 @@ func (r *SingleClusterReconciler) Reconcile() (result ctrl.Result, recErr error) return reconcile.Result{}, recErr } - if err := r.handleEnableSecurity(); err != nil { - return reconcile.Result{}, err - } - if err := r.handleEnableDynamicConfig(); err != nil { return reconcile.Result{}, err } @@ -988,65 +984,6 @@ 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 -} - func (r *SingleClusterReconciler) handleEnableDynamicConfig() error { if r.IsStatusEmpty() { // We have invalid status. 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 18bd0f51a..899c699e0 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 @@ -14291,10 +14291,6 @@ 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