From 3b0541391303b095ff31fd4af8fa4f6c7c44d78b Mon Sep 17 00:00:00 2001 From: Arthur Pitman Date: Tue, 7 Jan 2025 15:59:25 +0100 Subject: [PATCH] fix: Remove `MONACO_FEAT_DOCUMENTS` and `MONACO_FEAT_DELETE_DOCUMENTS` feature flags --- cmd/monaco/download/download_command.go | 9 ++-- cmd/monaco/download/download_configs.go | 20 ++++----- .../generate/deletefile/deletefile_test.go | 6 --- .../v2/all_configs_integration_test.go | 2 - .../v2/documents_integration_test.go | 8 +--- cmd/monaco/integrationtest/v2/dry-run_test.go | 1 - internal/featureflags/temporary.go | 8 ---- pkg/delete/delete.go | 20 ++++----- pkg/delete/delete_test.go | 2 - pkg/deploy/deploy.go | 6 +-- .../internal/persistence/type_definition.go | 19 ++++----- .../config/loader/config_loader_test.go | 41 ++----------------- .../config/writer/config_writer_test.go | 3 -- 13 files changed, 33 insertions(+), 112 deletions(-) diff --git a/cmd/monaco/download/download_command.go b/cmd/monaco/download/download_command.go index 4254f4d2c..a1c432f84 100644 --- a/cmd/monaco/download/download_command.go +++ b/cmd/monaco/download/download_command.go @@ -87,16 +87,13 @@ func GetDownloadCommand(fs afero.Fs, command Command) (cmd *cobra.Command) { cmd.Flags().BoolVar(&f.onlyAPIs, "only-apis", false, "Download only classic configuration APIs. Deprecated configuration APIs will not be included.") cmd.Flags().BoolVar(&f.onlySettings, "only-settings", false, "Download only settings 2.0 objects") cmd.Flags().BoolVar(&f.onlyAutomation, "only-automation", false, "Only download automation objects, skip all other configuration types") + cmd.Flags().BoolVar(&f.onlyDocuments, "only-documents", false, "Only download documents, skip all other configuration types") // combinations cmd.MarkFlagsMutuallyExclusive("settings-schema", "only-apis", "only-settings", "only-automation") cmd.MarkFlagsMutuallyExclusive("api", "only-apis", "only-settings", "only-automation") - - if featureflags.Documents.Enabled() { - cmd.Flags().BoolVar(&f.onlyDocuments, "only-documents", false, "Only download documents, skip all other configuration types") - cmd.MarkFlagsMutuallyExclusive("settings-schema", "only-apis", "only-settings", "only-automation", "only-documents") - cmd.MarkFlagsMutuallyExclusive("api", "only-apis", "only-settings", "only-automation", "only-documents") - } + cmd.MarkFlagsMutuallyExclusive("settings-schema", "only-apis", "only-settings", "only-automation", "only-documents") + cmd.MarkFlagsMutuallyExclusive("api", "only-apis", "only-settings", "only-automation", "only-documents") if featureflags.OpenPipeline.Enabled() { cmd.Flags().BoolVar(&f.onlyOpenPipeline, "only-openpipeline", false, "Only download openpipeline configurations, skip all other configuration types") diff --git a/cmd/monaco/download/download_configs.go b/cmd/monaco/download/download_configs.go index b841f02a3..17651db39 100644 --- a/cmd/monaco/download/download_configs.go +++ b/cmd/monaco/download/download_configs.go @@ -298,18 +298,16 @@ func downloadConfigs(clientSet *client.ClientSet, apisToDownload api.APIs, opts copyConfigs(configs, bucketCfgs) } - if featureflags.Documents.Enabled() { - if shouldDownloadDocuments(opts) { - if opts.auth.OAuth != nil { - log.Info("Downloading documents") - documentCfgs, err := fn.documentDownload(clientSet.DocumentClient, opts.projectName) - if err != nil { - return nil, err - } - copyConfigs(configs, documentCfgs) - } else if opts.onlyDocuments { - return nil, errors.New("can't download documents: no OAuth credentials configured") + if shouldDownloadDocuments(opts) { + if opts.auth.OAuth != nil { + log.Info("Downloading documents") + documentCfgs, err := fn.documentDownload(clientSet.DocumentClient, opts.projectName) + if err != nil { + return nil, err } + copyConfigs(configs, documentCfgs) + } else if opts.onlyDocuments { + return nil, errors.New("can't download documents: no OAuth credentials configured") } } diff --git a/cmd/monaco/generate/deletefile/deletefile_test.go b/cmd/monaco/generate/deletefile/deletefile_test.go index fcefcadfe..232dd6a74 100644 --- a/cmd/monaco/generate/deletefile/deletefile_test.go +++ b/cmd/monaco/generate/deletefile/deletefile_test.go @@ -76,7 +76,6 @@ func TestInvalidCommandUsage(t *testing.T) { func TestGeneratesValidDeleteFile(t *testing.T) { t.Setenv("TOKEN", "some-value") - t.Setenv(featureflags.Documents.EnvName(), "1") t.Setenv(featureflags.OpenPipeline.EnvName(), "1") t.Setenv(featureflags.Segments.EnvName(), "1") @@ -114,7 +113,6 @@ func TestGeneratesValidDeleteFile(t *testing.T) { func TestGeneratesValidDeleteFileWithCustomValues(t *testing.T) { t.Setenv("TOKEN", "some-value") - t.Setenv(featureflags.Documents.EnvName(), "1") t.Setenv(featureflags.OpenPipeline.EnvName(), "1") t.Setenv(featureflags.Segments.EnvName(), "1") @@ -144,7 +142,6 @@ func TestGeneratesValidDeleteFileWithCustomValues(t *testing.T) { func TestGeneratesValidDeleteFileWithFilter(t *testing.T) { t.Setenv("TOKEN", "some-value") - t.Setenv(featureflags.Documents.EnvName(), "1") t.Setenv(featureflags.OpenPipeline.EnvName(), "1") t.Setenv(featureflags.Segments.EnvName(), "1") @@ -167,7 +164,6 @@ func TestGeneratesValidDeleteFileWithFilter(t *testing.T) { func TestGeneratesValidDeleteFile_ForSpecificEnv(t *testing.T) { t.Setenv("TOKEN", "some-value") - t.Setenv(featureflags.Documents.EnvName(), "1") t.Setenv(featureflags.OpenPipeline.EnvName(), "1") t.Setenv(featureflags.Segments.EnvName(), "1") @@ -238,7 +234,6 @@ func TestGeneratesValidDeleteFile_ForSingleProject(t *testing.T) { func TestGeneratesValidDeleteFile_OmittingClassicConfigsWithNonStringNames(t *testing.T) { t.Setenv("TOKEN", "some-value") - t.Setenv(featureflags.Documents.EnvName(), "1") t.Setenv(featureflags.OpenPipeline.EnvName(), "1") t.Setenv(featureflags.Segments.EnvName(), "1") @@ -280,7 +275,6 @@ func assertDeleteEntries(t *testing.T, entries map[string][]pointer.DeletePointe func TestDoesNotOverwriteExistingFiles(t *testing.T) { t.Setenv("TOKEN", "some-value") - t.Setenv(featureflags.Documents.EnvName(), "1") t.Setenv(featureflags.OpenPipeline.EnvName(), "1") t.Setenv(featureflags.Segments.EnvName(), "1") diff --git a/cmd/monaco/integrationtest/v2/all_configs_integration_test.go b/cmd/monaco/integrationtest/v2/all_configs_integration_test.go index 59be8d1db..0e434f6e7 100644 --- a/cmd/monaco/integrationtest/v2/all_configs_integration_test.go +++ b/cmd/monaco/integrationtest/v2/all_configs_integration_test.go @@ -38,7 +38,6 @@ func TestIntegrationAllConfigsClassic(t *testing.T) { manifest := configFolder + "manifest.yaml" t.Setenv(featureflags.OpenPipeline.EnvName(), "true") - t.Setenv(featureflags.Documents.EnvName(), "true") targetEnvironment := "classic_env" @@ -56,7 +55,6 @@ func TestIntegrationAllConfigsPlatform(t *testing.T) { manifest := configFolder + "manifest.yaml" t.Setenv(featureflags.OpenPipeline.EnvName(), "true") - t.Setenv(featureflags.Documents.EnvName(), "true") targetEnvironment := "platform_env" diff --git a/cmd/monaco/integrationtest/v2/documents_integration_test.go b/cmd/monaco/integrationtest/v2/documents_integration_test.go index bf515b6c2..eea026d03 100644 --- a/cmd/monaco/integrationtest/v2/documents_integration_test.go +++ b/cmd/monaco/integrationtest/v2/documents_integration_test.go @@ -30,7 +30,6 @@ import ( "github.com/dynatrace/dynatrace-configuration-as-code/v2/cmd/monaco/integrationtest" "github.com/dynatrace/dynatrace-configuration-as-code/v2/cmd/monaco/integrationtest/utils/monaco" - "github.com/dynatrace/dynatrace-configuration-as-code/v2/internal/featureflags" manifestloader "github.com/dynatrace/dynatrace-configuration-as-code/v2/pkg/manifest/loader" ) @@ -46,12 +45,7 @@ func TestDocuments(t *testing.T) { manifestPath := configFolder + "manifest.yaml" environment := "platform_env" - envVars := map[string]string{ - featureflags.Documents.EnvName(): "true", - featureflags.DeleteDocuments.EnvName(): "true", - } - - RunIntegrationWithCleanupGivenEnvs(t, configFolder, manifestPath, environment, "Documents", envVars, func(fs afero.Fs, testContext TestContext) { + RunIntegrationWithCleanup(t, configFolder, manifestPath, environment, "Documents", func(fs afero.Fs, testContext TestContext) { // deploy err := monaco.RunWithFSf(fs, "monaco deploy %s --project=project --verbose", manifestPath) assert.NoError(t, err) diff --git a/cmd/monaco/integrationtest/v2/dry-run_test.go b/cmd/monaco/integrationtest/v2/dry-run_test.go index f71afef8d..2c749c1de 100644 --- a/cmd/monaco/integrationtest/v2/dry-run_test.go +++ b/cmd/monaco/integrationtest/v2/dry-run_test.go @@ -37,7 +37,6 @@ func TestDryRun(t *testing.T) { envVars := map[string]string{ featureflags.OpenPipeline.EnvName(): "true", - featureflags.Documents.EnvName(): "true", } RunIntegrationWithCleanupGivenEnvs(t, configFolder, manifest, specificEnvironment, "AllConfigs", envVars, func(fs afero.Fs, _ TestContext) { diff --git a/internal/featureflags/temporary.go b/internal/featureflags/temporary.go index 6caa98ecd..59c9ce29f 100644 --- a/internal/featureflags/temporary.go +++ b/internal/featureflags/temporary.go @@ -20,12 +20,6 @@ const ( // SkipReadOnlyAccountGroupUpdates toggles whether updates to read-only account groups are skipped or not. // Introduced: 2024-03-29; v2.13.0 SkipReadOnlyAccountGroupUpdates FeatureFlag = "MONACO_SKIP_READ_ONLY_ACCOUNT_GROUP_UPDATES" - // Documents toggles whether documents are downloaded and / or deployed. - // Introduced: 2024-04-16; v2.14.0 - Documents FeatureFlag = "MONACO_FEAT_DOCUMENTS" - // DeleteDocuments toggles whether documents are deleted - // Introduced: 2024-04-16; v2.14.2 - DeleteDocuments FeatureFlag = "MONACO_FEAT_DELETE_DOCUMENTS" // PersistSettingsOrder toggles whether insertAfter config parameter is persisted for ordered settings. // Introduced: 2024-05-15; v2.14.0 PersistSettingsOrder FeatureFlag = "MONACO_FEAT_PERSIST_SETTINGS_ORDER" @@ -46,8 +40,6 @@ const ( // These should always be removed after release of a feature, or some stabilization period if needed. var temporaryDefaultValues = map[FeatureFlag]defaultValue{ SkipReadOnlyAccountGroupUpdates: false, - Documents: true, - DeleteDocuments: true, PersistSettingsOrder: true, OpenPipeline: true, IgnoreSkippedConfigs: false, diff --git a/pkg/delete/delete.go b/pkg/delete/delete.go index e0d74cde1..fbca3729e 100644 --- a/pkg/delete/delete.go +++ b/pkg/delete/delete.go @@ -97,12 +97,10 @@ func deleteConfig(ctx context.Context, clients client.ClientSet, t string, entri } log.WithCtxFields(ctx).WithFields(field.Type(t)).Warn("Skipped deletion of %d Grail Bucket configuration(s) as API client was unavailable.", len(entries)) } else if t == "document" { - if featureflags.Documents.Enabled() && featureflags.DeleteDocuments.Enabled() { - if clients.DocumentClient != nil { - return document.Delete(ctx, clients.DocumentClient, entries) - } - log.WithCtxFields(ctx).WithFields(field.Type(t)).Warn("Skipped deletion of %d Document configuration(s) as API client was unavailable.", len(entries)) + if clients.DocumentClient != nil { + return document.Delete(ctx, clients.DocumentClient, entries) } + log.WithCtxFields(ctx).WithFields(field.Type(t)).Warn("Skipped deletion of %d Document configuration(s) as API client was unavailable.", len(entries)) } else if t == string(config.SegmentID) { if featureflags.Segments.Enabled() { if clients.SegmentClient != nil { @@ -156,13 +154,11 @@ func All(ctx context.Context, clients client.ClientSet, apis api.APIs) error { errCount++ } - if featureflags.Documents.Enabled() && featureflags.DeleteDocuments.Enabled() { - if clients.DocumentClient == nil { - log.Warn("Skipped deletion of Documents configurations as appropriate client was unavailable.") - } else if err := document.DeleteAll(ctx, clients.DocumentClient); err != nil { - log.Error("Failed to delete all Document configurations: %v", err) - errCount++ - } + if clients.DocumentClient == nil { + log.Warn("Skipped deletion of Documents configurations as appropriate client was unavailable.") + } else if err := document.DeleteAll(ctx, clients.DocumentClient); err != nil { + log.Error("Failed to delete all Document configurations: %v", err) + errCount++ } if featureflags.Segments.Enabled() { diff --git a/pkg/delete/delete_test.go b/pkg/delete/delete_test.go index e61219b83..42164c55c 100644 --- a/pkg/delete/delete_test.go +++ b/pkg/delete/delete_test.go @@ -1050,8 +1050,6 @@ func TestDeleteClassicKeyUserActionsWeb(t *testing.T) { } func TestDelete_Documents(t *testing.T) { - t.Setenv(featureflags.Documents.EnvName(), "true") - t.Setenv(featureflags.DeleteDocuments.EnvName(), "true") t.Run("delete via coordinate", func(t *testing.T) { given := pointer.DeletePointer{ Type: "document", diff --git a/pkg/deploy/deploy.go b/pkg/deploy/deploy.go index ec77a5823..00f278c74 100644 --- a/pkg/deploy/deploy.go +++ b/pkg/deploy/deploy.go @@ -300,11 +300,7 @@ func deployConfig(ctx context.Context, c *config.Config, clientset *client.Clien resolvedEntity, deployErr = bucket.Deploy(ctx, clientset.BucketClient, properties, renderedConfig, c) case config.DocumentType: - if featureflags.Documents.Enabled() { - resolvedEntity, deployErr = document.Deploy(ctx, clientset.DocumentClient, properties, renderedConfig, c) - } else { - deployErr = fmt.Errorf("unknown config-type (ID: %q)", c.Type.ID()) - } + resolvedEntity, deployErr = document.Deploy(ctx, clientset.DocumentClient, properties, renderedConfig, c) case config.OpenPipelineType: if featureflags.OpenPipeline.Enabled() { diff --git a/pkg/persistence/config/internal/persistence/type_definition.go b/pkg/persistence/config/internal/persistence/type_definition.go index cec4e425d..bb402f8f2 100644 --- a/pkg/persistence/config/internal/persistence/type_definition.go +++ b/pkg/persistence/config/internal/persistence/type_definition.go @@ -109,10 +109,7 @@ func (c *TypeDefinition) UnmarshalYAML(unmarshal func(interface{}) error) error "api": c.parseApiType, "settings": c.parseSettingsType, "automation": c.parseAutomation, - } - - if featureflags.Documents.Enabled() { - unmarshalers["document"] = c.parseDocumentType + "document": c.parseDocumentType, } if featureflags.OpenPipeline.Enabled() { @@ -317,14 +314,12 @@ func (c TypeDefinition) MarshalYAML() (interface{}, error) { return BucketType, nil case config.DocumentType: - if featureflags.Documents.Enabled() { - return map[string]any{ - "document": DocumentDefinition{ - Kind: t.Kind, - Private: t.Private, - }, - }, nil - } + return map[string]any{ + "document": DocumentDefinition{ + Kind: t.Kind, + Private: t.Private, + }, + }, nil case config.OpenPipelineType: if featureflags.OpenPipeline.Enabled() { diff --git a/pkg/persistence/config/loader/config_loader_test.go b/pkg/persistence/config/loader/config_loader_test.go index ba129fd09..76d864bbe 100644 --- a/pkg/persistence/config/loader/config_loader_test.go +++ b/pkg/persistence/config/loader/config_loader_test.go @@ -1451,10 +1451,7 @@ configs: wantErrorsContain: []string{"missing property"}, }, { - name: "Document dashboard config with FF on", - envVars: map[string]string{ - featureflags.Documents.EnvName(): "true", - }, + name: "Document dashboard config with FF on", filePathArgument: "test-file.yaml", filePathOnDisk: "test-file.yaml", fileContentOnDisk: ` @@ -1487,10 +1484,7 @@ configs: }, }, { - name: "Document private dashboard config with FF on", - envVars: map[string]string{ - featureflags.Documents.EnvName(): "true", - }, + name: "Document private dashboard config with FF on", filePathArgument: "test-file.yaml", filePathOnDisk: "test-file.yaml", fileContentOnDisk: ` @@ -1524,10 +1518,7 @@ configs: }, }, { - name: "Document notebook config with FF on", - envVars: map[string]string{ - featureflags.Documents.EnvName(): "true", - }, + name: "Document notebook config with FF on", filePathArgument: "test-file.yaml", filePathOnDisk: "test-file.yaml", fileContentOnDisk: ` @@ -1560,10 +1551,7 @@ configs: }, }, { - name: "Document config with invalid type with FF on", - envVars: map[string]string{ - featureflags.Documents.EnvName(): "true", - }, + name: "Document config with invalid type with FF on", filePathArgument: "test-file.yaml", filePathOnDisk: "test-file.yaml", fileContentOnDisk: ` @@ -1580,27 +1568,6 @@ configs: "unknown document kind \"other\"", }, }, - { - name: "Document config with FF off", - envVars: map[string]string{ - featureflags.Documents.EnvName(): "false", - }, - filePathArgument: "test-file.yaml", - filePathOnDisk: "test-file.yaml", - fileContentOnDisk: ` -configs: -- id: dashboard-id - config: - name: Test dashboard - originObjectId: ext-ID-123 - template: 'profile.json' - type: - document: - kind: dashboard`, - wantErrorsContain: []string{ - "unknown config-type \"document\"", - }, - }, { name: "OpenPipeline config with FF off", envVars: map[string]string{ diff --git a/pkg/persistence/config/writer/config_writer_test.go b/pkg/persistence/config/writer/config_writer_test.go index 4763d919e..94a206398 100644 --- a/pkg/persistence/config/writer/config_writer_test.go +++ b/pkg/persistence/config/writer/config_writer_test.go @@ -1333,9 +1333,6 @@ func TestWriteConfigs(t *testing.T) { "project/document-dashboard/b.json", "project/document-notebook/a.json", }, - envVars: map[string]string{ - featureflags.Documents.EnvName(): "true", - }, }, { name: "OpenPipeline",