Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 14 commits into from
Mar 21, 2024
3 changes: 3 additions & 0 deletions api/v1/aerospikecluster_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
43 changes: 27 additions & 16 deletions api/v1/aerospikecluster_validating_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
abhishekdwivedi3060 marked this conversation as resolved.
Show resolved Hide resolved
); err != nil {
return nil, err
}
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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 {
Expand All @@ -1285,38 +1285,47 @@ 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")
}
}

return nil
}

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 {
Expand All @@ -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
Expand All @@ -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
}
Expand Down
4 changes: 2 additions & 2 deletions api/v1/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}

Expand Down
5 changes: 5 additions & 0 deletions config/crd/bases/asdb.aerospike.com_aerospikeclusters.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -14278,6 +14282,7 @@ spec:
required:
- aerospikeConfigHash
- image
- isSecurityEnabled
- networkPolicyHash
- podIP
- podPort
Expand Down
48 changes: 22 additions & 26 deletions controllers/access_control.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we check for the enable security from spec by default? If it is not enabled then look into the status.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea here was to send Aerospike credentials if security is enabled in either the spec or status. Following that, we check for AerospikeAccessControl in the status to determine whether to use user-provided credentials or the default admin credentials.

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
}
}

Expand Down
123 changes: 99 additions & 24 deletions controllers/reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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",
Expand All @@ -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()

Expand Down Expand Up @@ -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
Expand All @@ -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))
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}
Loading
Loading