Chore: revert deletion of bucket api FF (#1216)

warber · web-flow · commit 11888e6964d2 · 2023-10-04T12:44:58.000+02:00
diff --git a/cmd/monaco/download/download_configs.go b/cmd/monaco/download/download_configs.go
@@ -18,6 +18,7 @@ import (
 	"errors"
 	"fmt"
 	"github.com/dynatrace/dynatrace-configuration-as-code/v2/cmd/monaco/dynatrace"
+	"github.com/dynatrace/dynatrace-configuration-as-code/v2/internal/featureflags"
 	"github.com/dynatrace/dynatrace-configuration-as-code/v2/internal/log"
 	"github.com/dynatrace/dynatrace-configuration-as-code/v2/pkg/api"
 	"github.com/dynatrace/dynatrace-configuration-as-code/v2/pkg/config"
@@ -256,14 +257,16 @@ func downloadConfigs(downloaders downloaders, opts downloadConfigsOptions) (proj
 		}
 	}
 
-	if opts.auth.OAuth != nil {
-		log.Info("Downloading Grail buckets")
+	if featureflags.Buckets().Enabled() {
+		if opts.auth.OAuth != nil {
+			log.Info("Downloading Grail buckets")
 
-		bucketCfgs, err := downloaders.Bucket().Download(opts.projectName)
-		if err != nil {
-			return nil, err
+			bucketCfgs, err := downloaders.Bucket().Download(opts.projectName)
+			if err != nil {
+				return nil, err
+			}
+			copyConfigs(configs, bucketCfgs)
 		}
-		copyConfigs(configs, bucketCfgs)
 	}
 
 	return configs, nil
diff --git a/cmd/monaco/integrationtest/v2/all_configs_integration_test.go b/cmd/monaco/integrationtest/v2/all_configs_integration_test.go
@@ -20,6 +20,7 @@
 package v2
 
 import (
+	"github.com/dynatrace/dynatrace-configuration-as-code/v2/internal/featureflags"
 	"github.com/dynatrace/dynatrace-configuration-as-code/v2/internal/testutils"
 	"testing"
 
@@ -45,6 +46,8 @@ func runAllConfigsTest(t *testing.T, specificEnvironment string) {
 	configFolder := "test-resources/integration-all-configs/"
 	manifest := configFolder + "manifest.yaml"
 
+	t.Setenv(featureflags.Buckets().EnvName(), "true")
+
 	RunIntegrationWithCleanup(t, configFolder, manifest, specificEnvironment, "AllConfigs", func(fs afero.Fs, _ TestContext) {
 
 		// This causes a POST for all configs:
@@ -69,6 +72,7 @@ func runAllConfigsTest(t *testing.T, specificEnvironment string) {
 func TestIntegrationValidationAllConfigs(t *testing.T) {
 
 	t.Setenv("UNIQUE_TEST_SUFFIX", "can-be-nonunique-for-validation")
+	t.Setenv(featureflags.Buckets().EnvName(), "true")
 
 	configFolder := "test-resources/integration-all-configs/"
 	manifest := configFolder + "manifest.yaml"
diff --git a/cmd/monaco/integrationtest/v2/bucket_integration_test.go b/cmd/monaco/integrationtest/v2/bucket_integration_test.go
@@ -21,6 +21,7 @@ package v2
 
 import (
 	"github.com/dynatrace/dynatrace-configuration-as-code/v2/cmd/monaco/integrationtest"
+	"github.com/dynatrace/dynatrace-configuration-as-code/v2/internal/featureflags"
 	"github.com/dynatrace/dynatrace-configuration-as-code/v2/internal/testutils"
 	"github.com/stretchr/testify/assert"
 	"testing"
@@ -31,6 +32,8 @@ import (
 
 // Tests a dry run (validation)
 func TestIntegrationBucketValidation(t *testing.T) {
+
+	t.Setenv(featureflags.Buckets().EnvName(), "1")
 	t.Setenv("UNIQUE_TEST_SUFFIX", "can-be-nonunique-for-validation")
 
 	configFolder := "test-resources/integration-bucket/"
@@ -61,6 +64,7 @@ func TestIntegrationBucket(t *testing.T) {
 	configFolder := "test-resources/integration-bucket/"
 	manifest := configFolder + "manifest.yaml"
 	specificEnvironment := ""
+	t.Setenv(featureflags.Buckets().EnvName(), "1")
 
 	RunIntegrationWithCleanup(t, configFolder, manifest, specificEnvironment, "Buckets", func(fs afero.Fs, _ TestContext) {
 
@@ -85,6 +89,7 @@ func TestIntegrationComplexBucket(t *testing.T) {
 	configFolder := "test-resources/integration-bucket/"
 	manifest := configFolder + "manifest.yaml"
 	specificEnvironment := ""
+	t.Setenv(featureflags.Buckets().EnvName(), "1")
 
 	RunIntegrationWithCleanup(t, configFolder, manifest, specificEnvironment, "ComplexBuckets", func(fs afero.Fs, _ TestContext) {
 
diff --git a/cmd/monaco/integrationtest/v2/config_restore_e2e_test.go b/cmd/monaco/integrationtest/v2/config_restore_e2e_test.go
@@ -22,6 +22,7 @@ import (
 	"fmt"
 	"github.com/dynatrace/dynatrace-configuration-as-code/v2/cmd/monaco/integrationtest"
 	"github.com/dynatrace/dynatrace-configuration-as-code/v2/cmd/monaco/runner"
+	"github.com/dynatrace/dynatrace-configuration-as-code/v2/internal/featureflags"
 	"github.com/dynatrace/dynatrace-configuration-as-code/v2/internal/log"
 	"github.com/dynatrace/dynatrace-configuration-as-code/v2/internal/testutils"
 	"github.com/dynatrace/dynatrace-configuration-as-code/v2/pkg/manifest"
@@ -107,6 +108,8 @@ func TestRestoreConfigs_FromDownloadWithPlatformManifestFile_withPlatformConfigs
 	subsetOfConfigsToDownload := "alerting-profile,management-zone"
 	suffixTest := "_download_automations"
 
+	t.Setenv(featureflags.Buckets().EnvName(), "true")
+
 	testRestoreConfigs(t, initialConfigsFolder, downloadFolder, suffixTest, manifestFile, subsetOfConfigsToDownload, false, execution_downloadConfigs)
 }
 
diff --git a/cmd/monaco/integrationtest/v2/delete_integration_test.go b/cmd/monaco/integrationtest/v2/delete_integration_test.go
@@ -22,6 +22,7 @@ import (
 	"fmt"
 	"github.com/dynatrace/dynatrace-configuration-as-code/v2/cmd/monaco/integrationtest"
 	"github.com/dynatrace/dynatrace-configuration-as-code/v2/cmd/monaco/runner"
+	"github.com/dynatrace/dynatrace-configuration-as-code/v2/internal/featureflags"
 	"github.com/dynatrace/dynatrace-configuration-as-code/v2/internal/testutils"
 	"github.com/dynatrace/dynatrace-configuration-as-code/v2/pkg/config"
 	"github.com/dynatrace/dynatrace-configuration-as-code/v2/pkg/config/coordinate"
@@ -160,6 +161,7 @@ func TestDelete(t *testing.T) {
 }
 
 func TestDeleteSkipsPlatformTypesWhenDeletingFromClassicEnv(t *testing.T) {
+	t.Setenv(featureflags.Buckets().EnvName(), "true")
 
 	configFolder := "test-resources/delete-test-configs/"
 	deployManifestPath := configFolder + "deploy-manifest.yaml"
diff --git a/internal/featureflags/temporary.go b/internal/featureflags/temporary.go
@@ -73,6 +73,15 @@ func DependencyGraphBasedDeployParallel() FeatureFlag {
 	}
 }
 
+// Buckets toggles whether the Grail bucket type can be used.
+// Introduced: (inactive) 2023-08-09 ->
+func Buckets() FeatureFlag {
+	return FeatureFlag{
+		envName:        "MONACO_FEAT_BUCKETS",
+		defaultEnabled: false,
+	}
+}
+
 // UnescapeOnConvert toggles whether converting will remove escape chars from v1 values.
 // Introduced: 2023-09-01; v2.8.0
 func UnescapeOnConvert() FeatureFlag {
diff --git a/pkg/persistence/config/internal/persistence/type_definition.go b/pkg/persistence/config/internal/persistence/type_definition.go
@@ -19,6 +19,7 @@ package persistence
 import (
 	"errors"
 	"fmt"
+	"github.com/dynatrace/dynatrace-configuration-as-code/v2/internal/featureflags"
 	"github.com/dynatrace/dynatrace-configuration-as-code/v2/pkg/config"
 	"github.com/mitchellh/mapstructure"
 )
@@ -54,6 +55,11 @@ func (c *TypeDefinition) UnmarshalYAML(unmarshal func(interface{}) error) error
 	switch v := data.(type) {
 	case string:
 		if v == BucketType {
+			// string was a bucket
+			if !featureflags.Buckets().Enabled() {
+				return fmt.Errorf("failed to parse 'type' section: unknown type %q", v)
+			}
+
 			c.Bucket = v
 			return nil
 		}
diff --git a/pkg/persistence/config/internal/persistence/type_definition_test.go b/pkg/persistence/config/internal/persistence/type_definition_test.go
@@ -19,12 +19,15 @@
 package persistence
 
 import (
+	"github.com/dynatrace/dynatrace-configuration-as-code/v2/internal/featureflags"
 	"github.com/stretchr/testify/assert"
 	"gopkg.in/yaml.v2"
 	"testing"
 )
 
 func Test_typeDefinition_isSound(t *testing.T) {
+	t.Setenv(featureflags.Buckets().EnvName(), "1")
+
 	type fields struct {
 		configType TypeDefinition
 		knownApis  map[string]struct{}
diff --git a/pkg/persistence/config/loader/config_loader_test.go b/pkg/persistence/config/loader/config_loader_test.go
@@ -20,6 +20,7 @@ package loader
 
 import (
 	"fmt"
+	"github.com/dynatrace/dynatrace-configuration-as-code/v2/internal/featureflags"
 	"github.com/dynatrace/dynatrace-configuration-as-code/v2/pkg/config"
 	"github.com/dynatrace/dynatrace-configuration-as-code/v2/pkg/config/coordinate"
 	"github.com/dynatrace/dynatrace-configuration-as-code/v2/pkg/config/parameter"
@@ -884,7 +885,10 @@ configs:
 			},
 		},
 		{
-			name:             "Bucket config",
+			name: "Bucket config with FF on",
+			envVars: map[string]string{
+				featureflags.Buckets().EnvName(): "true",
+			},
 			filePathArgument: "test-file.yaml",
 			filePathOnDisk:   "test-file.yaml",
 			fileContentOnDisk: `
@@ -911,7 +915,26 @@ configs:
 			},
 		},
 		{
-			name:             "Bucket written as api config",
+			name: "Bucket with FF off",
+			envVars: map[string]string{
+				featureflags.Buckets().EnvName(): "false",
+			},
+			filePathArgument: "test-file.yaml",
+			filePathOnDisk:   "test-file.yaml",
+			fileContentOnDisk: `
+configs:
+- id: profile-id
+  config:
+    template: 'profile.json'
+  type: bucket
+`,
+			wantErrorsContain: []string{"failed to parse 'type' section: unknown type \"bucket\""},
+		},
+		{
+			name: "Bucket written as api config",
+			envVars: map[string]string{
+				featureflags.Buckets().EnvName(): "true",
+			},
 			filePathArgument: "test-file.yaml",
 			filePathOnDisk:   "test-file.yaml",
 			fileContentOnDisk: `
