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.

Sign up for GitHub

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
Merged

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

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
908a9b8
enable security dynamically
tanmayja Feb 14, 2024
ec4b349
disallow disabling security in running cluster
tanmayja Feb 19, 2024
e259de7
Merge branch 'master' into enablesec
tanmayja Feb 19, 2024
19aa4ad
adding securityEnabled flag in helm crd
tanmayja Feb 20, 2024
fba1862
small restructuring
tanmayja Feb 23, 2024
e9a798d
Merge branch 'master' into enablesec
tanmayja Mar 4, 2024
f26f3f0
fixing lint
tanmayja Mar 4, 2024
e04f709
Merge branch 'master' into enablesec
tanmayja Mar 18, 2024
572f8bf
addressing comments
tanmayja Mar 18, 2024
01c36b2
addressing comments
tanmayja Mar 19, 2024
1821f74
Merge branch 'master' into enablesec
tanmayja Mar 19, 2024
13756a1
correcting init tag
tanmayja Mar 19, 2024
96cc9e7
correcting init tag
tanmayja Mar 19, 2024
6cb8357
addressing comment
tanmayja Mar 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 21 additions & 9 deletions api/v1/aerospikecluster_validating_webhook.go
View file
Open in desktop
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, isSecurityEnabledPodExist bool,
) error {
nv, err := lib.CompareVersions(newVersion, "5.7.0")
if err != nil {
Expand All @@ -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"]
Expand All @@ -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")
}
}
Expand All @@ -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 {
Expand All @@ -1336,7 +1337,7 @@ func validateSecurityContext(
}
}

if !ivflag && ovflag {
if !ivflag && ovflag && isSecurityEnabledPodExist {
return fmt.Errorf("cannot disable cluster security in running cluster")
}

Expand All @@ -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
}
}

abhishekdwivedi3060 marked this conversation as resolved.
Show resolved Hide resolved
if err := validateSecurityConfigUpdate(
incomingVersion, outgoingVersion, incomingSpec, outgoingSpec,
isSecurityEnabledPodExist,
); err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion api/v1/utils.go
View file
Open in desktop
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 = "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"
Expand Down
38 changes: 18 additions & 20 deletions controllers/access_control.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
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 {
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.
Expand Down
Loading