From 4efcf23677d96b0c228027cbb79ad9245170e951 Mon Sep 17 00:00:00 2001 From: Subba Gontla Date: Thu, 12 Nov 2020 12:37:47 -0800 Subject: [PATCH] cspl-558: Improve code coverage for Index management --- pkg/splunk/client/enterprise.go | 5 +- pkg/splunk/client/enterprise_test.go | 2 +- pkg/splunk/controller/configmap_test.go | 60 ++++ pkg/splunk/enterprise/clustermaster.go | 6 +- pkg/splunk/enterprise/clustermaster_test.go | 45 ++- pkg/splunk/enterprise/configuration.go | 17 - pkg/splunk/enterprise/configuration_test.go | 329 +++++++++++++++++++- pkg/splunk/enterprise/util_test.go | 91 ++++++ 8 files changed, 519 insertions(+), 36 deletions(-) diff --git a/pkg/splunk/client/enterprise.go b/pkg/splunk/client/enterprise.go index 5cf7569ae..a61e5e277 100644 --- a/pkg/splunk/client/enterprise.go +++ b/pkg/splunk/client/enterprise.go @@ -629,10 +629,7 @@ func (c *SplunkClient) DecommissionIndexerClusterPeer(enforceCounts bool) error } // BundlePush pushes the CM master apps bundle to all the indexer peers -func (c *SplunkClient) BundlePush(ignoreIdenticalBundle bool, mock bool) error { - if mock { - return nil - } +func (c *SplunkClient) BundlePush(ignoreIdenticalBundle bool) error { endpoint := fmt.Sprintf("%s/services/cluster/master/control/default/apply", c.ManagementURI) reqBody := fmt.Sprintf("&ignore_identical_bundle=%t", ignoreIdenticalBundle) diff --git a/pkg/splunk/client/enterprise_test.go b/pkg/splunk/client/enterprise_test.go index 9f6fc8ef6..42903e887 100644 --- a/pkg/splunk/client/enterprise_test.go +++ b/pkg/splunk/client/enterprise_test.go @@ -179,7 +179,7 @@ func TestBundlePush(t *testing.T) { wantRequest, _ := http.NewRequest("POST", "https://localhost:8089/services/cluster/master/control/default/apply", body) test := func(c SplunkClient) error { - return c.BundlePush(true, false) + return c.BundlePush(true) } splunkClientTester(t, "TestBundlePush", 200, "", wantRequest, test) } diff --git a/pkg/splunk/controller/configmap_test.go b/pkg/splunk/controller/configmap_test.go index 4cdaf981a..3385ca5e0 100644 --- a/pkg/splunk/controller/configmap_test.go +++ b/pkg/splunk/controller/configmap_test.go @@ -19,6 +19,7 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" spltest "github.com/splunk/splunk-operator/pkg/splunk/test" ) @@ -41,3 +42,62 @@ func TestApplyConfigMap(t *testing.T) { } spltest.ReconcileTester(t, "TestApplyConfigMap", ¤t, revised, createCalls, updateCalls, reconcile, false) } + +func TestGetConfigMap(t *testing.T) { + current := corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "defaults", + Namespace: "test", + }, + } + + client := spltest.NewMockClient() + namespacedName := types.NamespacedName{Namespace: current.GetNamespace(), Name: current.GetName()} + + _, err := GetConfigMap(client, namespacedName) + if err == nil { + t.Errorf("Should return an error, when the configMap doesn't exist") + } + + _, err = ApplyConfigMap(client, ¤t) + if err != nil { + t.Errorf("Failed to create the configMap. Error: %s", err.Error()) + } + + _, err = GetConfigMap(client, namespacedName) + if err != nil { + t.Errorf("Should not return an error, when the configMap exists") + } +} + +func TestGetConfigMapResourceVersion(t *testing.T) { + current := corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "defaults", + Namespace: "test", + }, + } + + client := spltest.NewMockClient() + namespacedName := types.NamespacedName{Namespace: current.GetNamespace(), Name: current.GetName()} + + _, err := GetConfigMap(client, namespacedName) + if err == nil { + t.Errorf("Should return an error, when the configMap doesn't exist") + } + + _, err = GetConfigMapResourceVersion(client, namespacedName) + if err == nil { + t.Errorf("Should return an error, when the configMap doesn't exist") + } + + _, err = ApplyConfigMap(client, ¤t) + if err != nil { + t.Errorf("Failed to create the configMap. Error: %s", err.Error()) + } + + _, err = GetConfigMapResourceVersion(client, namespacedName) + if err != nil { + t.Errorf("Should not return an error, when the configMap exists") + } +} diff --git a/pkg/splunk/enterprise/clustermaster.go b/pkg/splunk/enterprise/clustermaster.go index 9fea8bad7..1676db702 100644 --- a/pkg/splunk/enterprise/clustermaster.go +++ b/pkg/splunk/enterprise/clustermaster.go @@ -186,9 +186,7 @@ func CheckIfsmartstoreConfigMapUpdatedToPod(c splcommon.ControllerClient, cr *en command := fmt.Sprintf("cat /mnt/splunk-operator/local/%s", configToken) stdOut, stdErr, err := splutil.PodExecCommand(c, cmPodName, cr.GetNamespace(), []string{"/bin/sh"}, command, false, false) if err != nil || stdErr != "" { - if cr.Spec.Mock == false { - return fmt.Errorf("Failed to check config token value on pod. stdout=%s, stderror=%s, error=%v", stdOut, stdErr, err) - } + return fmt.Errorf("Failed to check config token value on pod. stdout=%s, stderror=%s, error=%v", stdOut, stdErr, err) } configMap, exists := getSmartstoreConfigMap(c, cr, SplunkClusterMaster) @@ -267,5 +265,5 @@ func PushMasterAppsBundle(c splcommon.ControllerClient, cr *enterprisev1.Cluster // Get a Splunk client to execute the REST call splunkClient := splclient.NewSplunkClient(fmt.Sprintf("https://%s:8089", fqdnName), "admin", string(adminPwd)) - return splunkClient.BundlePush(true, cr.Spec.Mock) + return splunkClient.BundlePush(true) } diff --git a/pkg/splunk/enterprise/clustermaster_test.go b/pkg/splunk/enterprise/clustermaster_test.go index 94a3259a1..adba53ad3 100644 --- a/pkg/splunk/enterprise/clustermaster_test.go +++ b/pkg/splunk/enterprise/clustermaster_test.go @@ -16,6 +16,7 @@ package enterprise import ( "fmt" + "strings" "testing" "time" @@ -243,6 +244,13 @@ func TestPerformCmBundlePush(t *testing.T) { client := spltest.NewMockClient() + // When the secret object is not present, should return an error + current.Status.BundlePushTracker.NeedToPushMasterApps = true + err := PerformCmBundlePush(client, ¤t) + if err == nil { + t.Errorf("Should return error, when the secret object is not present") + } + secret, err := splutil.ApplyNamespaceScopedSecretObject(client, "test") if err != nil { t.Errorf(err.Error()) @@ -261,23 +269,36 @@ func TestPerformCmBundlePush(t *testing.T) { Data: map[string]string{configToken: ""}, } + _, err = splctrl.ApplyConfigMap(client, &smartstoreConfigMap) + if err != nil { + t.Errorf(err.Error()) + } + current.Status.BundlePushTracker.NeedToPushMasterApps = true - current.Status.BundlePushTracker.LastCheckInterval = time.Now().Unix() - 100 - _, err = splctrl.ApplyConfigMap(client, &smartstoreConfigMap) + //Re-attempting to push the CM bundle in less than 5 seconds should return an error + current.Status.BundlePushTracker.LastCheckInterval = time.Now().Unix() - 1 + err = PerformCmBundlePush(client, ¤t) + if err == nil { + t.Errorf("Bundle Push Should fail, if attempted to push within 5 seconds interval") + } + //Re-attempting to push the CM bundle after 5 seconds passed, should not return an error + current.Status.BundlePushTracker.LastCheckInterval = time.Now().Unix() - 10 err = PerformCmBundlePush(client, ¤t) - if err != nil { - t.Errorf("Bundle Push failed") + if err != nil && strings.HasPrefix(err.Error(), "Will re-attempt to push the bundle after the 5 seconds") { + t.Errorf("Bundle Push Should not fail if reattempted after 5 seconds interval passed. Error: %s", err.Error()) } + // When the CM Bundle push is not pending, should not return an error + current.Status.BundlePushTracker.NeedToPushMasterApps = false err = PerformCmBundlePush(client, ¤t) if err != nil { - t.Errorf("Bundle Push failed") + t.Errorf("Should not return an error when the Bundle push is not required. Error: %s", err.Error()) } } -func TestPushMasterAppsBundlePush(t *testing.T) { +func TestPushMasterAppsBundle(t *testing.T) { current := enterprisev1.ClusterMaster{ TypeMeta: metav1.TypeMeta{ @@ -296,6 +317,12 @@ func TestPushMasterAppsBundlePush(t *testing.T) { client := spltest.NewMockClient() + //Without global secret object, should return an error + err := PushMasterAppsBundle(client, ¤t) + if err == nil { + t.Errorf("Bundle push should fail, when the secret object is not found") + } + secret, err := splutil.ApplyNamespaceScopedSecretObject(client, "test") if err != nil { t.Errorf(err.Error()) @@ -306,8 +333,10 @@ func TestPushMasterAppsBundlePush(t *testing.T) { t.Errorf(err.Error()) } + //Without password, should return an error + delete(secret.Data, "password") err = PushMasterAppsBundle(client, ¤t) - if err != nil { - t.Errorf("Bundle Push failed") + if err == nil { + t.Errorf("Bundle push should fail, when the password is not found") } } diff --git a/pkg/splunk/enterprise/configuration.go b/pkg/splunk/enterprise/configuration.go index 368b822ed..48f2a4fce 100644 --- a/pkg/splunk/enterprise/configuration.go +++ b/pkg/splunk/enterprise/configuration.go @@ -659,23 +659,6 @@ func updateSplunkPodTemplateWithConfig(client splcommon.ControllerClient, podTem } } -// LogSmartStoreVolumes logs smartstore volumes -func LogSmartStoreVolumes(volumeList []enterprisev1.VolumeSpec) { - scopedLog := logC.WithName("LogSmartStoreVolumes") - //var temp string - for _, volume := range volumeList { - scopedLog.Info("Volume: ", "name: ", volume.Name, "endpoint: ", volume.Endpoint, "path: ", volume.Path) - } -} - -// LogSmartStoreIndexes logs smartstore indexes -func LogSmartStoreIndexes(indexList []enterprisev1.IndexSpec) { - scopedLog := logC.WithName("LogSmartStoreIndexes") - for _, index := range indexList { - scopedLog.Info("Index: ", "name: ", index.Name, "remotePath: ", index.RemotePath, "volumeName", index.VolName) - } -} - // isSmartstoreEnabled checks and returns true if smartstore is configured func isSmartstoreConfigured(smartstore *enterprisev1.SmartStoreSpec) bool { if smartstore == nil { diff --git a/pkg/splunk/enterprise/configuration_test.go b/pkg/splunk/enterprise/configuration_test.go index 9fc58b208..ec37dfb13 100644 --- a/pkg/splunk/enterprise/configuration_test.go +++ b/pkg/splunk/enterprise/configuration_test.go @@ -21,6 +21,9 @@ import ( enterprisev1 "github.com/splunk/splunk-operator/pkg/apis/enterprise/v1beta1" splcommon "github.com/splunk/splunk-operator/pkg/splunk/common" + splctrl "github.com/splunk/splunk-operator/pkg/splunk/controller" + spltest "github.com/splunk/splunk-operator/pkg/splunk/test" + splutil "github.com/splunk/splunk-operator/pkg/splunk/util" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" @@ -457,9 +460,199 @@ func TestValidateSplunkSmartstoreSpec(t *testing.T) { if err != nil { t.Errorf("Smartstore config is optional, should not cause an error. But, got the error: %v", err) } + + // Configuring indexes without volume config should return error + SmartStoreWithoutVolumes := enterprisev1.SmartStoreSpec{ + IndexList: []enterprisev1.IndexSpec{ + {Name: "salesdata1", RemotePath: "remotepath1", + IndexAndGlobalCommonSpec: enterprisev1.IndexAndGlobalCommonSpec{ + VolName: "msos_s2s3_vol"}, + }, + {Name: "salesdata2", RemotePath: "remotepath2", + IndexAndGlobalCommonSpec: enterprisev1.IndexAndGlobalCommonSpec{ + VolName: "msos_s2s3_vol"}, + }, + {Name: "salesdata3", RemotePath: "remotepath3", + IndexAndGlobalCommonSpec: enterprisev1.IndexAndGlobalCommonSpec{ + VolName: "msos_s2s3_vol"}, + }, + }, + } + + err = ValidateSplunkSmartstoreSpec(&SmartStoreWithoutVolumes) + if err == nil { + t.Errorf("Smartstore config without volume details should return error") + } + + // Duplicate volume names should be rejected + SmartStoreWithDuplicateVolumes := enterprisev1.SmartStoreSpec{ + VolList: []enterprisev1.VolumeSpec{ + {Name: "msos_s2s3_vol-1", Endpoint: "https://s3-eu-west-2.amazonaws.com", Path: "testbucket-rs-london", SecretRef: "s3-secret"}, + {Name: "msos_s2s3_vol-2", Endpoint: "https://s3-eu-west-2.amazonaws.com", Path: "testbucket-rs-london", SecretRef: "s3-secret"}, + {Name: "msos_s2s3_vol-1", Endpoint: "https://s3-eu-west-2.amazonaws.com", Path: "testbucket-rs-london", SecretRef: "s3-secret"}, + }, + IndexList: []enterprisev1.IndexSpec{ + {Name: "salesdata1", RemotePath: "remotepath1", + IndexAndGlobalCommonSpec: enterprisev1.IndexAndGlobalCommonSpec{ + VolName: "msos_s2s3_vol"}, + }, + {Name: "salesdata2", RemotePath: "remotepath2", + IndexAndGlobalCommonSpec: enterprisev1.IndexAndGlobalCommonSpec{ + VolName: "msos_s2s3_vol"}, + }, + {Name: "salesdata3", RemotePath: "remotepath3", + IndexAndGlobalCommonSpec: enterprisev1.IndexAndGlobalCommonSpec{ + VolName: "msos_s2s3_vol"}, + }, + }, + } + + err = ValidateSplunkSmartstoreSpec(&SmartStoreWithDuplicateVolumes) + if err == nil { + t.Errorf("Duplicate volume configuration should return an error") + } + + // Defaults with invalid volume reference should return error + SmartStoreDefaultsWithNonExistingVolume := enterprisev1.SmartStoreSpec{ + Defaults: enterprisev1.IndexConfDefaultsSpec{ + IndexAndGlobalCommonSpec: enterprisev1.IndexAndGlobalCommonSpec{ + VolName: "msos_s2s3_vol-2"}, + }, + VolList: []enterprisev1.VolumeSpec{ + {Name: "msos_s2s3_vol-1", Endpoint: "https://s3-eu-west-2.amazonaws.com", Path: "testbucket-rs-london", SecretRef: "s3-secret"}, + }, + } + + err = ValidateSplunkSmartstoreSpec(&SmartStoreDefaultsWithNonExistingVolume) + if err == nil { + t.Errorf("Volume referred in the indexes defaults should be a valid volume") + } + + //Duplicate index names should return an error + SmartStoreWithDuplicateIndexes := enterprisev1.SmartStoreSpec{ + VolList: []enterprisev1.VolumeSpec{ + {Name: "msos_s2s3_vol", Endpoint: "https://s3-eu-west-2.amazonaws.com", Path: "testbucket-rs-london", SecretRef: "s3-secret"}, + }, + IndexList: []enterprisev1.IndexSpec{ + {Name: "salesdata1", RemotePath: "remotepath1", + IndexAndGlobalCommonSpec: enterprisev1.IndexAndGlobalCommonSpec{ + VolName: "msos_s2s3_vol"}, + }, + {Name: "salesdata1", RemotePath: "remotepath2", + IndexAndGlobalCommonSpec: enterprisev1.IndexAndGlobalCommonSpec{ + VolName: "msos_s2s3_vol"}, + }, + }, + } + + err = ValidateSplunkSmartstoreSpec(&SmartStoreWithDuplicateIndexes) + if err == nil { + t.Errorf("Duplicate index names should return an error") + } + + // If the default volume is not configured, then each index should be configured + // with an explicit volume info. If not, should return an error + SmartStoreVolumeMissingBothFromDefaultsAndIndex := enterprisev1.SmartStoreSpec{ + VolList: []enterprisev1.VolumeSpec{ + {Name: "msos_s2s3_vol-1", Endpoint: "https://s3-eu-west-2.amazonaws.com", Path: "testbucket-rs-london", SecretRef: "s3-secret"}, + }, + IndexList: []enterprisev1.IndexSpec{ + {Name: "salesdata1", RemotePath: "remotepath1"}, + {Name: "salesdata2", RemotePath: "remotepath2", + IndexAndGlobalCommonSpec: enterprisev1.IndexAndGlobalCommonSpec{ + VolName: "msos_s2s3_vol"}, + }, + }, + } + + err = ValidateSplunkSmartstoreSpec(&SmartStoreVolumeMissingBothFromDefaultsAndIndex) + if err == nil { + t.Errorf("If no default volume, index with missing volume info should return an error") + } + + // Volume referenced from an index must be a valid volume + SmartStoreIndexesWithInvalidVolumeName := enterprisev1.SmartStoreSpec{ + VolList: []enterprisev1.VolumeSpec{ + {Name: "msos_s2s3_vol-1", Endpoint: "https://s3-eu-west-2.amazonaws.com", Path: "testbucket-rs-london", SecretRef: "s3-secret"}, + }, + IndexList: []enterprisev1.IndexSpec{ + {Name: "salesdata1", RemotePath: "remotepath1", + IndexAndGlobalCommonSpec: enterprisev1.IndexAndGlobalCommonSpec{ + VolName: "msos_s2s3_vol-2"}, + }, + }, + } + + err = ValidateSplunkSmartstoreSpec(&SmartStoreIndexesWithInvalidVolumeName) + if err == nil { + t.Errorf("Index with an invalid volume name should return error") + } } -func TestValidateSplunkSmartstoreCacheManagerSpec(t *testing.T) { +func TestGetSmartstoreIndexesConfig(t *testing.T) { + SmartStoreIndexes := enterprisev1.SmartStoreSpec{ + IndexList: []enterprisev1.IndexSpec{ + {Name: "salesdata1", RemotePath: "remotepath1", + IndexAndGlobalCommonSpec: enterprisev1.IndexAndGlobalCommonSpec{ + MaxGlobalDataSizeMB: 6000, + MaxGlobalRawDataSizeMB: 7000, + VolName: "msos_s2s3_vol"}, + }, + {Name: "salesdata2", RemotePath: "remotepath2", + IndexAndGlobalCommonSpec: enterprisev1.IndexAndGlobalCommonSpec{ + VolName: "msos_s2s3_vol"}, + }, + {Name: "salesdata3", // Missing RemotePath should be filled with the default "$_index_name" + IndexAndGlobalCommonSpec: enterprisev1.IndexAndGlobalCommonSpec{ + MaxGlobalDataSizeMB: 2000, + MaxGlobalRawDataSizeMB: 3000, + VolName: "msos_s2s3_vol"}, + IndexAndCacheManagerCommonSpec: enterprisev1.IndexAndCacheManagerCommonSpec{ + HotlistBloomFilterRecencyHours: 48, + HotlistRecencySecs: 48 * 60 * 60}, + }, + {Name: "salesdata4", // Missing RemotePath should be filled with the default "$_index_name" + IndexAndGlobalCommonSpec: enterprisev1.IndexAndGlobalCommonSpec{ + MaxGlobalDataSizeMB: 4000, + MaxGlobalRawDataSizeMB: 5000, + VolName: "msos_s2s3_vol"}, + IndexAndCacheManagerCommonSpec: enterprisev1.IndexAndCacheManagerCommonSpec{ + HotlistBloomFilterRecencyHours: 24, + HotlistRecencySecs: 24 * 60 * 60}, + }, + }, + } + + expectedINIFormatString := fmt.Sprintf(` +[salesdata1] +remotePath = volume:msos_s2s3_vol/remotepath1 +maxGlobalDataSizeMB = 6000 +maxGlobalRawDataSizeMB = 7000 + +[salesdata2] +remotePath = volume:msos_s2s3_vol/remotepath2 + +[salesdata3] +remotePath = volume:msos_s2s3_vol/$_index_name +hotlist_bloom_filter_recency_hours = 48 +hotlist_recency_secs = 172800 +maxGlobalDataSizeMB = 2000 +maxGlobalRawDataSizeMB = 3000 + +[salesdata4] +remotePath = volume:msos_s2s3_vol/$_index_name +hotlist_bloom_filter_recency_hours = 24 +hotlist_recency_secs = 86400 +maxGlobalDataSizeMB = 4000 +maxGlobalRawDataSizeMB = 5000 +`) + + indexesConfIni := GetSmartstoreIndexesConfig(SmartStoreIndexes.IndexList) + if indexesConfIni != expectedINIFormatString { + t.Errorf("expected: %s, returned: %s", expectedINIFormatString, indexesConfIni) + } +} +func TestGetServerConfigEntries(t *testing.T) { SmartStoreCacheManager := enterprisev1.CacheManagerSpec{ IndexAndCacheManagerCommonSpec: enterprisev1.IndexAndCacheManagerCommonSpec{ @@ -489,9 +682,16 @@ max_concurrent_uploads = 6 if expectedIniContents != serverConfForCacheManager { t.Errorf("Expected: %s \n Received: %s", expectedIniContents, serverConfForCacheManager) } + + // Empty config should return empty string + serverConfForCacheManager = GetServerConfigEntries(nil) + if serverConfForCacheManager != "" { + t.Errorf("Expected empty string, but received: %s", serverConfForCacheManager) + } + } -func TestValidateSplunkSmartstoreDefaultsSpec(t *testing.T) { +func TestGetSmartstoreIndexesDefaults(t *testing.T) { SmartStoreDefaultsConf := enterprisev1.IndexConfDefaultsSpec{ IndexAndGlobalCommonSpec: enterprisev1.IndexAndGlobalCommonSpec{ @@ -520,3 +720,128 @@ maxGlobalRawDataSizeMB = 61440 } } + +func TestCheckIfVolumeExists(t *testing.T) { + SmartStoreConfig := enterprisev1.SmartStoreSpec{ + VolList: []enterprisev1.VolumeSpec{ + {Name: "msos_s2s3_vol", Endpoint: "https://s3-eu-west-2.amazonaws.com", Path: "testbucket-rs-london", SecretRef: "s3-secret"}, + }, + IndexList: []enterprisev1.IndexSpec{ + {Name: "salesdata1", RemotePath: "remotepath1", + IndexAndGlobalCommonSpec: enterprisev1.IndexAndGlobalCommonSpec{ + VolName: "msos_s2s3_vol"}, + }, + {Name: "salesdata2", RemotePath: "remotepath2", + IndexAndGlobalCommonSpec: enterprisev1.IndexAndGlobalCommonSpec{ + VolName: "msos_s2s3_vol"}, + }, + {Name: "salesdata3", RemotePath: "remotepath3", + IndexAndGlobalCommonSpec: enterprisev1.IndexAndGlobalCommonSpec{ + VolName: "msos_s2s3_vol"}, + }, + }, + } + + // Volume that doesn't should error out + _, err := checkIfVolumeExists(SmartStoreConfig.VolList, "random_volume_name") + + if err == nil { + t.Errorf("if the volume doesn't exists, error should be reported") + } + + // Volume that exists should not error out + index := len(SmartStoreConfig.VolList) - 1 + returnedIndex, err := checkIfVolumeExists(SmartStoreConfig.VolList, SmartStoreConfig.VolList[index].Name) + + if err != nil { + t.Errorf("existing volume should not error out. index id: %d, error: %s", index, err.Error()) + } else if index != returnedIndex { + t.Errorf("Expected index: %d, but returned index id: %d", index, returnedIndex) + } +} + +func TestAreRemoteVolumeKeysChanged(t *testing.T) { + cr := enterprisev1.ClusterMaster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "CM", + Namespace: "test", + }, + Spec: enterprisev1.ClusterMasterSpec{ + SmartStore: enterprisev1.SmartStoreSpec{ + VolList: []enterprisev1.VolumeSpec{ + {Name: "msos_s2s3_vol", Endpoint: "https://s3-eu-west-2.amazonaws.com", Path: "testbucket-rs-london", SecretRef: "splunk-test-secret"}, + }, + + IndexList: []enterprisev1.IndexSpec{ + {Name: "salesdata1", RemotePath: "remotepath1", IndexAndGlobalCommonSpec: enterprisev1.IndexAndGlobalCommonSpec{ + VolName: "msos_s2s3_vol"}, + }, + {Name: "salesdata2", RemotePath: "remotepath2", IndexAndGlobalCommonSpec: enterprisev1.IndexAndGlobalCommonSpec{ + VolName: "msos_s2s3_vol"}, + }, + {Name: "salesdata3", RemotePath: "remotepath3", IndexAndGlobalCommonSpec: enterprisev1.IndexAndGlobalCommonSpec{ + VolName: "msos_s2s3_vol"}, + }, + }, + }, + }, + } + + client := spltest.NewMockClient() + var err error + ResourceRev := map[string]string{ + "secret1": "12345", + "secret2": "67890", + } + + // Missing secret object should return an error + keysChanged := AreRemoteVolumeKeysChanged(client, &cr, SplunkClusterMaster, &cr.Spec.SmartStore, ResourceRev, &err) + if err == nil { + t.Errorf("Missing secret object should return an error. keyChangedFlag: %t", keysChanged) + } else if keysChanged { + t.Errorf("When the S3 secret object is not present, Key change should not be reported") + } + + // First time secret version reference should be updated + // Just to simplify the test, assume that the keys are stored as part of the splunk-test-scret + secret, err := splutil.ApplyNamespaceScopedSecretObject(client, "test") + if err != nil { + t.Errorf(err.Error()) + } + + _, err = splctrl.ApplySecret(client, secret) + if err != nil { + t.Errorf(err.Error()) + } + + keysChanged = AreRemoteVolumeKeysChanged(client, &cr, SplunkClusterMaster, &cr.Spec.SmartStore, ResourceRev, &err) + + _, ok := ResourceRev["splunk-test-secret"] + if !ok { + t.Errorf("Failed to update the Resource Version for first time") + } + + // Change the Resource Version, and see if that is being detected + resourceVersion := "3434" + secret.SetResourceVersion(resourceVersion) + + keysChanged = AreRemoteVolumeKeysChanged(client, &cr, SplunkClusterMaster, &cr.Spec.SmartStore, ResourceRev, &err) + resourceVersionUpdated, ok := ResourceRev["splunk-test-secret"] + if !keysChanged || resourceVersion != resourceVersionUpdated { + t.Errorf("Failed detect the secret object change. Key changed: %t, Expected resource version: %s, Updated resource version %s", keysChanged, resourceVersion, resourceVersionUpdated) + } + + // No change on the secret object should return false + keysChanged = AreRemoteVolumeKeysChanged(client, &cr, SplunkClusterMaster, &cr.Spec.SmartStore, ResourceRev, &err) + resourceVersionUpdated, ok = ResourceRev["splunk-test-secret"] + if keysChanged { + t.Errorf("If there is no change on secret object, should return false") + } + + // Empty volume list should return false + cr.Spec.SmartStore.VolList = nil + keysChanged = AreRemoteVolumeKeysChanged(client, &cr, SplunkClusterMaster, &cr.Spec.SmartStore, ResourceRev, &err) + if keysChanged { + t.Errorf("Empty volume should not report a key change") + } +} diff --git a/pkg/splunk/enterprise/util_test.go b/pkg/splunk/enterprise/util_test.go index d837dfc9e..731215537 100644 --- a/pkg/splunk/enterprise/util_test.go +++ b/pkg/splunk/enterprise/util_test.go @@ -202,6 +202,13 @@ func TestApplySmartstoreConfigMap(t *testing.T) { } test(client, &cr, &cr.Spec.SmartStore, `{"metadata":{"name":"splunk-idxCluster--smartstore","namespace":"test","creationTimestamp":null},"data":{"conftoken":"1601945361","indexes.conf":"[default]\nrepFactor = auto\nmaxDataSize = auto\nhomePath = $SPLUNK_DB/$_index_name/db\ncoldPath = $SPLUNK_DB/$_index_name/colddb\nthawedPath = $SPLUNK_DB/$_index_name/thaweddb\n \n[volume:msos_s2s3_vol]\nstorageType = remote\npath = s3://testbucket-rs-london\nremote.s3.access_key = abcdJDckRkxhMEdmSk5FekFRRzBFOXV6bGNldzJSWE9IenhVUy80aa\nremote.s3.secret_key = g4NVp0a29PTzlPdGczWk1vekVUcVBSa0o4NkhBWWMvR1NadDV4YVEy\nremote.s3.endpoint = https://s3-eu-west-2.amazonaws.com\n \n[salesdata1]\nremotePath = volume:msos_s2s3_vol/remotepath1\n\n[salesdata2]\nremotePath = volume:msos_s2s3_vol/remotepath2\n\n[salesdata3]\nremotePath = volume:msos_s2s3_vol/remotepath3\n","server.conf":""}}`) + + // Missing Volume config should return an error + cr.Spec.SmartStore.VolList = nil + _, _, err = ApplySmartstoreConfigMap(client, &cr, &cr.Spec.SmartStore) + if err == nil { + t.Errorf("Configuring Indexes without volumes should return an error") + } } func TestRemoveOwenerReferencesForSecretObjectsReferredBySmartstoreVolumes(t *testing.T) { @@ -214,6 +221,9 @@ func TestRemoveOwenerReferencesForSecretObjectsReferredBySmartstoreVolumes(t *te SmartStore: enterprisev1.SmartStoreSpec{ VolList: []enterprisev1.VolumeSpec{ {Name: "msos_s2s3_vol", Endpoint: "https://s3-eu-west-2.amazonaws.com", Path: "testbucket-rs-london", SecretRef: "splunk-test-secret"}, + {Name: "msos_s2s3_vol_2", Endpoint: "https://s3-eu-west-2.amazonaws.com", Path: "testbucket-rs-london", SecretRef: "splunk-test-secret"}, + {Name: "msos_s2s3_vol_3", Endpoint: "https://s3-eu-west-2.amazonaws.com", Path: "testbucket-rs-london", SecretRef: "splunk-test-secret"}, + {Name: "msos_s2s3_vol_4", Endpoint: "https://s3-eu-west-2.amazonaws.com", Path: "testbucket-rs-london", SecretRef: "splunk-test-secret"}, }, IndexList: []enterprisev1.IndexSpec{ @@ -260,4 +270,85 @@ func TestRemoveOwenerReferencesForSecretObjectsReferredBySmartstoreVolumes(t *te if err != nil { t.Errorf("Couldn't Remove S3 Secret object references %v", err) } + + // If the secret object doesn't exist, should return an error + // Here in the volume references, secrets splunk-test-sec_1, to splunk-test-sec_4 doesn't exist + cr = enterprisev1.ClusterMaster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "idxCluster", + Namespace: "testWithNoSecret", + }, + Spec: enterprisev1.ClusterMasterSpec{ + SmartStore: enterprisev1.SmartStoreSpec{ + VolList: []enterprisev1.VolumeSpec{ + {Name: "msos_s2s3_vol", Endpoint: "https://s3-eu-west-2.amazonaws.com", Path: "testbucket-rs-london", SecretRef: "splunk-test-sec_1"}, + {Name: "msos_s2s3_vol_2", Endpoint: "https://s3-eu-west-2.amazonaws.com", Path: "testbucket-rs-london", SecretRef: "splunk-test-sec_2"}, + {Name: "msos_s2s3_vol_3", Endpoint: "https://s3-eu-west-2.amazonaws.com", Path: "testbucket-rs-london", SecretRef: "splunk-test-sec_3"}, + {Name: "msos_s2s3_vol_4", Endpoint: "https://s3-eu-west-2.amazonaws.com", Path: "testbucket-rs-london", SecretRef: "splunk-test-sec_4"}, + }, + }, + }, + } + + // S3 secret owner reference removal, with non-existing secret objects + err = DeleteOwnerReferencesForS3SecretObjects(client, secret, &cr.Spec.SmartStore) + if err == nil { + t.Errorf("Should report an error, when the secret object referenced in the volume config doesn't exist") + } + + // Smartstore volume config with non-existing secret objects + err = DeleteOwnerReferencesForResources(client, &cr, &cr.Spec.SmartStore) + if err == nil { + t.Errorf("Should report an error, when the secret objects doesn't exist") + } +} + +func TestGetSmartstoreRemoteVolumeSecrets(t *testing.T) { + cr := enterprisev1.ClusterMaster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "CM", + Namespace: "test", + }, + Spec: enterprisev1.ClusterMasterSpec{ + SmartStore: enterprisev1.SmartStoreSpec{ + VolList: []enterprisev1.VolumeSpec{ + {Name: "msos_s2s3_vol", Endpoint: "https://s3-eu-west-2.amazonaws.com", Path: "testbucket-rs-london", SecretRef: "splunk-test-secret"}, + }, + }, + }, + } + + client := spltest.NewMockClient() + + // Just to simplify the test, assume that the keys are stored as part of the splunk-test-secret object, hence create that secret object + secret, err := splutil.ApplyNamespaceScopedSecretObject(client, "test") + if err != nil { + t.Errorf(err.Error()) + } + + _, err = splctrl.ApplySecret(client, secret) + if err != nil { + t.Errorf(err.Error()) + } + + // Missing S3 access key should return error + _, _, _, err = GetSmartstoreRemoteVolumeSecrets(cr.Spec.SmartStore.VolList[0], client, &cr, &cr.Spec.SmartStore) + if err == nil { + t.Errorf("Missing S3 access key should return an error") + } + + secret.Data[s3AccessKey] = []byte("abcdJDckRkxhMEdmSk5FekFRRzBFOXV6bGNldzJSWE9IenhVUy80aa") + + // Missing S3 secret key should return error + _, _, _, err = GetSmartstoreRemoteVolumeSecrets(cr.Spec.SmartStore.VolList[0], client, &cr, &cr.Spec.SmartStore) + if err == nil { + t.Errorf("Missing S3 secret key should return an error") + } + + // When access key and secret keys are present, returned keys should not be empty. Also, should not return an error + secret.Data[s3SecretKey] = []byte("g4NVp0a29PTzlPdGczWk1vekVUcVBSa0o4NkhBWWMvR1NadDV4YVEy") + accessKey, secretKey, _, err := GetSmartstoreRemoteVolumeSecrets(cr.Spec.SmartStore.VolList[0], client, &cr, &cr.Spec.SmartStore) + if accessKey == "" || secretKey == "" || err != nil { + t.Errorf("Missing S3 Keys / Error not expected, when the Secret object with the S3 specific keys are present") + } }