Skip to content

Commit

Permalink
fix: Remove MONACO_FEAT_DOCUMENTS and `MONACO_FEAT_DELETE_DOCUMENTS…
Browse files Browse the repository at this point in the history
…` feature flags
  • Loading branch information
arthurpitman committed Jan 7, 2025
1 parent 4fdb3c3 commit 934c05b
Show file tree
Hide file tree
Showing 13 changed files with 33 additions and 112 deletions.
9 changes: 3 additions & 6 deletions cmd/monaco/download/download_command.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
20 changes: 9 additions & 11 deletions cmd/monaco/download/download_configs.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
}

Expand Down
6 changes: 0 additions & 6 deletions cmd/monaco/generate/deletefile/deletefile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Expand Down Expand Up @@ -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")

Expand Down Expand Up @@ -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")

Expand All @@ -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")

Expand Down Expand Up @@ -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")

Expand Down Expand Up @@ -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")

Expand Down
2 changes: 0 additions & 2 deletions cmd/monaco/integrationtest/v2/all_configs_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

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

Expand Down
8 changes: 1 addition & 7 deletions cmd/monaco/integrationtest/v2/documents_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand All @@ -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)
Expand Down
1 change: 0 additions & 1 deletion cmd/monaco/integrationtest/v2/dry-run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
8 changes: 0 additions & 8 deletions internal/featureflags/temporary.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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,
Expand Down
20 changes: 8 additions & 12 deletions pkg/delete/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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() {
Expand Down
2 changes: 0 additions & 2 deletions pkg/delete/delete_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
6 changes: 1 addition & 5 deletions pkg/deploy/deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
19 changes: 7 additions & 12 deletions pkg/persistence/config/internal/persistence/type_definition.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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() {
Expand Down
41 changes: 4 additions & 37 deletions pkg/persistence/config/loader/config_loader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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: `
Expand Down Expand Up @@ -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: `
Expand Down Expand Up @@ -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: `
Expand Down Expand Up @@ -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: `
Expand All @@ -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{
Expand Down
3 changes: 0 additions & 3 deletions pkg/persistence/config/writer/config_writer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down

0 comments on commit 934c05b

Please sign in to comment.