Skip to content

Commit

Permalink
Address review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
sud82 committed May 18, 2024
1 parent c130817 commit 9f3df5a
Show file tree
Hide file tree
Showing 5 changed files with 71 additions and 29 deletions.
29 changes: 21 additions & 8 deletions api/v1/aerospikecluster_validating_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ func (c *AerospikeCluster) validate(aslog logr.Logger) error {
return err
}

if err := validateRequiredFileStorageForFeatureConf(
if err := validateRequiredFileStorageForAerospikeConfig(
rack.AerospikeConfig, &rack.Storage,
); err != nil {
return err
Expand Down Expand Up @@ -1797,7 +1797,7 @@ func validateRequiredFileStorageForMetadata(
return nil
}

func validateRequiredFileStorageForFeatureConf(
func validateRequiredFileStorageForAerospikeConfig(
configSpec AerospikeConfigSpec, storage *AerospikeStorageSpec,
) error {
featureKeyFilePaths := getFeatureKeyFilePaths(configSpec)
Expand All @@ -1821,21 +1821,34 @@ func validateRequiredFileStorageForFeatureConf(
}
}

// CA cert related fields are not supported with Secret Manager, so check their mount volume
allPaths = append(allPaths, caPaths...)

if defaultPassFilePath != nil {
allPaths = append(allPaths, *defaultPassFilePath)
if !isSecretManagerPath(*defaultPassFilePath) {
allPaths = append(allPaths, *defaultPassFilePath)
} else {
return fmt.Errorf("default-password-file path doesn't support Secret Manager, path %s", *defaultPassFilePath)
}
}

// CA cert related fields are not supported with Secret Manager, so check their mount volume
allPaths = append(allPaths, caPaths...)

for _, path := range allPaths {
if !storage.isVolumePresentForAerospikePath(filepath.Dir(path)) {
volume := storage.GetVolumeForAerospikePath(filepath.Dir(path))
if volume == nil {
return fmt.Errorf(
"feature-key-file paths or tls paths or default-password-file path "+
"are not mounted - create an entry for '%v' in 'storage.volumes'",
"are not mounted - create an entry for '%s' in 'storage.volumes'",
path,
)
}

if defaultPassFilePath != nil &&
(path == *defaultPassFilePath && volume.Source.Secret == nil) {
return fmt.Errorf(
"default-password-file path %s volume source should be secret in storage config, volume %v",
path, volume,
)
}
}

return nil
Expand Down
6 changes: 0 additions & 6 deletions api/v1/storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -203,12 +203,6 @@ func (s *AerospikeStorageSpec) getAerospikeStorageList(onlyPV bool) (
return blockStorageDeviceList, fileStorageList, nil
}

// isVolumePresentForAerospikePath checks if configuration has a volume defined for given path for Aerospike server
// container.
func (s *AerospikeStorageSpec) isVolumePresentForAerospikePath(path string) bool {
return s.GetVolumeForAerospikePath(path) != nil
}

// GetVolumeForAerospikePath returns volume defined for given path for Aerospike server container.
func (s *AerospikeStorageSpec) GetVolumeForAerospikePath(path string) *VolumeSpec {
var matchedVolume *VolumeSpec
Expand Down
40 changes: 36 additions & 4 deletions test/access_control_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"github.com/go-logr/logr"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/utils/ptr"
Expand Down Expand Up @@ -2161,13 +2162,39 @@ var _ = Describe(
Expect(err).To(HaveOccurred())
})

It("Should fail if volume source is not secret for default-password-file", func() {
aeroCluster := createDummyAerospikeCluster(clusterNamespacedName, 4)
racks := getDummyRackConf(1, 2)
aeroCluster.Spec.RackConfig.Racks = racks
aeroCluster.Spec.RackConfig.Namespaces = []string{"test"}
aeroCluster.Spec.AerospikeConfig.Value["security"] = map[string]interface{}{
"default-password-file": "/etc/aerospike/defaultpass/password.conf",
}
aeroCluster.Spec.Storage.Volumes = append(aeroCluster.Spec.Storage.Volumes, asdbv1.VolumeSpec{
Name: "defaultpass",
Source: asdbv1.VolumeSource{
EmptyDir: &corev1.EmptyDirVolumeSource{},
},
Aerospike: &asdbv1.AerospikeServerVolumeAttachment{
Path: "/etc/aerospike/defaultpass",
},
})

err := k8sClient.Create(ctx, aeroCluster)
Expect(err).To(HaveOccurred())
})

It("Should use default-password-file when configured", func() {
By("Creating cluster")

aeroCluster := createDummyAerospikeCluster(clusterNamespacedName, 4)
racks := getDummyRackConf(1, 2)
aeroCluster.Spec.RackConfig.Racks = racks
aeroCluster.Spec.RackConfig.Namespaces = []string{"test"}
// This file is already added in the storage volume backed by the secret.
aeroCluster.Spec.AerospikeConfig.Value["security"] = map[string]interface{}{
"default-password-file": "/etc/aerospike/secret/password.conf",
}

err := k8sClient.Create(ctx, aeroCluster)
Expect(err).ToNot(HaveOccurred())
Expand All @@ -2194,15 +2221,20 @@ var _ = Describe(
return cerr
}

nodes := client.GetNodeNames()
if len(nodes) == 0 {
return fmt.Errorf("No nodes found")
if !client.IsConnected() {
return fmt.Errorf("Not connected")
}

pkgLog.Info("Connected to cluster", "nodes", nodes)
pkgLog.Info("Connected to cluster")

return nil
}, 3*time.Minute).ShouldNot(HaveOccurred())

By("Try scaleup")
err = scaleUpClusterTest(
k8sClient, ctx, clusterNamespacedName, 1,
)
Expect(err).ToNot(HaveOccurred())
})
})
},
Expand Down
23 changes: 13 additions & 10 deletions test/cluster_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,13 @@ import (

const (
baseImage = "aerospike/aerospike-server-enterprise"
prevServerVersion = "6.4.0.0"
pre6Version = "5.7.0.17"
version6 = "6.0.0.5"
prevServerVersion = "7.0.0.0"
latestServerVersion = "7.1.0.0"
invalidVersion = "3.0.0.4"

post6Version = "7.0.0.0"
pre6Version = "5.7.0.17"
version6 = "6.0.0.5"
)

var (
Expand All @@ -45,9 +47,12 @@ var (
logger = logr.Discard()
prevImage = fmt.Sprintf("%s:%s", baseImage, prevServerVersion)
latestImage = fmt.Sprintf("%s:%s", baseImage, latestServerVersion)
version6Image = fmt.Sprintf("%s:%s", baseImage, version6)
invalidImage = fmt.Sprintf("%s:%s", baseImage, invalidVersion)
pre6Image = fmt.Sprintf("%s:%s", baseImage, pre6Version)

// Storage wipe test
post6Image = fmt.Sprintf("%s:%s", baseImage, post6Version)
version6Image = fmt.Sprintf("%s:%s", baseImage, version6)
pre6Image = fmt.Sprintf("%s:%s", baseImage, pre6Version)
)

func rollingRestartClusterByEnablingTLS(
Expand Down Expand Up @@ -1123,10 +1128,8 @@ func createDummyAerospikeCluster(
"proto-fd-max": defaultProtofdmax,
"auto-pin": "none",
},
"security": map[string]interface{}{
"default-password-file": "/etc/aerospike/secret/password.conf",
},
"network": getNetworkConfig(),
"security": map[string]interface{}{},
"network": getNetworkConfig(),
"namespaces": []interface{}{
getSCNamespaceConfig("test", "/test/dev/xvdf"),
},
Expand Down Expand Up @@ -1467,7 +1470,7 @@ func aerospikeClusterCreateUpdate(
ctx goctx.Context,
) error {
return aerospikeClusterCreateUpdateWithTO(
k8sClient, desired, ctx, retryInterval, getTimeout(1),
k8sClient, desired, ctx, retryInterval, getTimeout(desired.Spec.Size),
)
}

Expand Down
2 changes: 1 addition & 1 deletion test/storage_wipe_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ var _ = Describe(
}
aeroCluster := getStorageWipeAerospikeCluster(
clusterNamespacedName, storageConfig, racks,
latestImage, getAerospikeClusterConfig(),
post6Image, getAerospikeClusterConfig(),
)

aeroCluster.Spec.PodSpec = podSpec
Expand Down

0 comments on commit 9f3df5a

Please sign in to comment.