Skip to content

Commit 0e7c3bb

Browse files
arthurpitmanLaubi
andauthored
feat: Create references only in string values in JSON templates (#1672)
* feat: Create references only in string values in JSON templates * feat: Rename feature flag * fix: Return original JSON if no change * refactor: More descriptive variable names * test: Complete tests * fix: Feature flag description --------- Co-authored-by: David Laubreiter <david.laubreiter@dynatrace.com>
1 parent a11a804 commit 0e7c3bb

File tree

13 files changed

+447
-10
lines changed

13 files changed

+447
-10
lines changed

cmd/monaco/integrationtest/v2/integration_test_utils.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,19 @@ func RunIntegrationWithCleanupOnGivenFs(t *testing.T, testFs afero.Fs, configFol
8888
runIntegrationWithCleanup(t, opts, testFunc)
8989
}
9090

91+
func RunIntegrationWithCleanupOnGivenFsAndEnvs(t *testing.T, testFs afero.Fs, configFolder, manifestPath, specificEnvironment, suffixTest string, envVars map[string]string, testFunc TestFunc) {
92+
opts := TestOptions{
93+
fs: testFs,
94+
configFolder: configFolder,
95+
manifestPath: manifestPath,
96+
specificEnvironment: specificEnvironment,
97+
suffix: suffixTest,
98+
envVars: envVars,
99+
}
100+
101+
runIntegrationWithCleanup(t, opts, testFunc)
102+
}
103+
91104
func RunIntegrationWithCleanupGivenEnvs(t *testing.T, configFolder, manifestPath, specificEnvironment, suffixTest string, envVars map[string]string, testFunc TestFunc) {
92105
opts := TestOptions{
93106
fs: testutils.CreateTestFileSystem(),

cmd/monaco/integrationtest/v2/references_test.go

Lines changed: 88 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,20 +23,104 @@ import (
2323
"strings"
2424
"testing"
2525

26+
"github.com/spf13/afero"
27+
"github.com/stretchr/testify/assert"
28+
"github.com/stretchr/testify/require"
29+
"golang.org/x/exp/maps"
30+
2631
"github.com/dynatrace/dynatrace-configuration-as-code/v2/cmd/monaco/integrationtest/utils/monaco"
2732
"github.com/dynatrace/dynatrace-configuration-as-code/v2/cmd/monaco/runner"
33+
"github.com/dynatrace/dynatrace-configuration-as-code/v2/internal/featureflags"
2834
"github.com/dynatrace/dynatrace-configuration-as-code/v2/internal/testutils"
2935
"github.com/dynatrace/dynatrace-configuration-as-code/v2/pkg/api"
3036
"github.com/dynatrace/dynatrace-configuration-as-code/v2/pkg/config"
3137
valueParam "github.com/dynatrace/dynatrace-configuration-as-code/v2/pkg/config/parameter/value"
3238
manifestloader "github.com/dynatrace/dynatrace-configuration-as-code/v2/pkg/manifest/loader"
3339
project "github.com/dynatrace/dynatrace-configuration-as-code/v2/pkg/project/v2"
34-
"github.com/spf13/afero"
35-
"github.com/stretchr/testify/assert"
36-
"github.com/stretchr/testify/require"
37-
"golang.org/x/exp/maps"
3840
)
3941

42+
// TestOnlyStringReferences explicitly tests the "only create references in string values" feature.
43+
// The test creates an network zone with the ID "0", and when the feature flag is turned off, Monaco creates references to it often such as in coordinates for dashboard tiles.
44+
// When turned on, such references are not created.
45+
func TestOnlyStringReferences(t *testing.T) {
46+
tests := []struct {
47+
name string
48+
envVars map[string]string
49+
validate func(t *testing.T, dashboardParameters config.Parameters)
50+
}{
51+
{
52+
name: "With feature flag enabled",
53+
envVars: map[string]string{featureflags.OnlyCreateReferencesInStringValues.EnvName(): "true"},
54+
validate: func(t *testing.T, dashboardParameters config.Parameters) {
55+
assert.Len(t, dashboardParameters, 1, "dashboard should only have one parameter")
56+
_, ok := dashboardParameters["name"]
57+
assert.True(t, ok, "dashboard should only have a name parameter")
58+
},
59+
},
60+
{
61+
name: "With feature flag disabled",
62+
envVars: map[string]string{featureflags.OnlyCreateReferencesInStringValues.EnvName(): "false"},
63+
validate: func(t *testing.T, dashboardParameters config.Parameters) {
64+
assert.Len(t, dashboardParameters, 2, "dashboard should have two parameters")
65+
_, ok := dashboardParameters["name"]
66+
assert.True(t, ok, "dashboard should have a name parameter")
67+
68+
_, ok = dashboardParameters["networkzone__0__id"]
69+
assert.True(t, ok, "dashboard should reference a network zone")
70+
},
71+
},
72+
}
73+
74+
for _, tt := range tests {
75+
t.Run(tt.name, func(t *testing.T) {
76+
77+
configFolder := "test-resources/string-references/"
78+
manifestFile := configFolder + "manifest.yaml"
79+
80+
fs := testutils.CreateTestFileSystem()
81+
82+
proj := "string-references"
83+
env := "classic_env"
84+
85+
RunIntegrationWithCleanupOnGivenFsAndEnvs(t, fs, configFolder, manifestFile, env, "string_references", tt.envVars, func(fs afero.Fs, ctx TestContext) {
86+
87+
// upsert
88+
err := monaco.RunWithFSf(fs, "monaco deploy %s --environment=%s --project=%s --verbose", manifestFile, env, proj)
89+
require.NoError(t, err, "create: did not expect error")
90+
91+
// download
92+
err = monaco.RunWithFSf(fs, "monaco download --manifest=%s --environment=%s --project=proj --output-folder=download --verbose %s", manifestFile, env, "--api=dashboard,network-zone")
93+
require.NoError(t, err, "download: did not expect error")
94+
95+
// load downloaded project
96+
mani, errs := manifestloader.Load(&manifestloader.Context{
97+
Fs: fs,
98+
ManifestPath: "download/manifest.yaml",
99+
Opts: manifestloader.Options{RequireEnvironmentGroups: true},
100+
})
101+
assert.Empty(t, errs, "unexpected error loading manifest")
102+
103+
projects, errs := project.LoadProjects(fs, project.ProjectLoaderContext{
104+
KnownApis: api.NewAPIs().GetApiNameLookup(),
105+
WorkingDir: "download",
106+
Manifest: mani,
107+
ParametersSerde: config.DefaultParameterParsers,
108+
}, nil)
109+
assert.Empty(t, errs, "unexpected error loading project")
110+
111+
// find dashboard
112+
projectAndEnvName := "proj_" + env // for manifest downloads proj + env name
113+
confsPerType := findConfigs(t, projects, projectAndEnvName)
114+
dashboard := findConfig(t, confsPerType, "dashboard", "Dashboard 0_"+ctx.suffix)
115+
116+
require.NotEmpty(t, dashboard.Coordinate.ConfigId)
117+
118+
tt.validate(t, dashboard.Parameters)
119+
})
120+
})
121+
}
122+
}
123+
40124
func TestReferencesAreResolvedOnDownload(t *testing.T) {
41125

42126
envs := []string{"classic_env", "platform_env"}
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
manifestVersion: 1.0
2+
3+
projects:
4+
- name: string-references
5+
6+
environmentGroups:
7+
- name: default
8+
environments:
9+
- name: classic_env
10+
url:
11+
type: environment
12+
value: URL_ENVIRONMENT_1
13+
auth:
14+
token:
15+
name: TOKEN_ENVIRONMENT_1
16+
- name: platform_env
17+
url:
18+
type: environment
19+
value: PLATFORM_URL_ENVIRONMENT_2
20+
auth:
21+
token:
22+
name: TOKEN_ENVIRONMENT_2
23+
oAuth:
24+
clientId:
25+
name: OAUTH_CLIENT_ID
26+
clientSecret:
27+
name: OAUTH_CLIENT_SECRET
28+
tokenEndpoint:
29+
type: environment
30+
value: OAUTH_TOKEN_ENDPOINT
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
configs:
2+
- id: network-zones-enabled
3+
config:
4+
template: network-zones-enabled.json
5+
skip: false
6+
type:
7+
settings:
8+
schema: builtin:networkzones
9+
schemaVersion: 1.0.2
10+
scope: environment
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
{
2+
"enabled": true
3+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
configs:
2+
- id: dashboard1
3+
type:
4+
api: dashboard
5+
config:
6+
name: Dashboard 0
7+
template: dashboard.json
8+
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
{
2+
"dashboardMetadata": {
3+
"name": "{{.name}}",
4+
"shared": true,
5+
"owner": "nobody@dynatrace.com"
6+
},
7+
"tiles": [
8+
{
9+
"name": "Markdown",
10+
"tileType": "MARKDOWN",
11+
"configured": true,
12+
"bounds": {
13+
"top": 0,
14+
"left": 0,
15+
"width": 304,
16+
"height": 152
17+
},
18+
"tileFilter": {},
19+
"isAutoRefreshDisabled": false,
20+
"markdown": "0"
21+
}
22+
]
23+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
configs:
2+
- id: "0" #monaco-test:no-replace
3+
config:
4+
name: "0" #monaco-test:no-replace
5+
template: zone.json
6+
parameters:
7+
networkZoneFeatEnabled:
8+
configType: builtin:networkzones
9+
configId: network-zones-enabled
10+
property: name
11+
type: reference
12+
type:
13+
api: network-zone
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
{
2+
"description": "Network zone",
3+
"alternativeZones": [],
4+
"fallbackMode": "ANY_ACTIVE_GATE"
5+
}

internal/featureflags/temporary.go

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -36,16 +36,21 @@ const (
3636
// ServiceUsers toggles whether account service users configurations are downloaded and / or deployed.
3737
// Introduced: v2.18.0
3838
ServiceUsers FeatureFlag = "MONACO_FEAT_SERVICE_USERS"
39+
// OnlyCreateReferencesInStringValues toggles whether references are created arbitarily in JSON templates
40+
// or when enabled only in string values within the JSON.
41+
// Introduced: v2.19.0
42+
OnlyCreateReferencesInStringValues FeatureFlag = "MONACO_FEAT_ONLY_CREATE_REFERENCES_IN_STRINGS"
3943
)
4044

4145
// temporaryDefaultValues defines temporary feature flags and their default values.
4246
// It is suitable for features that are hidden during development or have some uncertainty.
4347
// These should always be removed after release of a feature, or some stabilization period if needed.
4448
var temporaryDefaultValues = map[FeatureFlag]defaultValue{
45-
SkipReadOnlyAccountGroupUpdates: false,
46-
PersistSettingsOrder: true,
47-
OpenPipeline: true,
48-
IgnoreSkippedConfigs: false,
49-
Segments: true,
50-
ServiceUsers: false,
49+
SkipReadOnlyAccountGroupUpdates: false,
50+
PersistSettingsOrder: true,
51+
OpenPipeline: true,
52+
IgnoreSkippedConfigs: false,
53+
Segments: true,
54+
ServiceUsers: false,
55+
OnlyCreateReferencesInStringValues: false,
5156
}

0 commit comments

Comments
 (0)