Skip to content

feat(config): Warn when undefined environments or groups are used in overrides #1880

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 28, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions pkg/config/errors/loader_errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package errors

import (
"fmt"

"github.com/dynatrace/dynatrace-configuration-as-code/v2/pkg/config/coordinate"
)

Expand Down
22 changes: 21 additions & 1 deletion pkg/config/loader/config_entry_loader.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (

"github.com/spf13/afero"

"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"
"github.com/dynatrace/dynatrace-configuration-as-code/v2/pkg/config/coordinate"
Expand Down Expand Up @@ -55,12 +56,15 @@ func parseConfigEntry(
return nil, []error{newDefinitionParserError(configId, singleConfigContext, err.Error())}
}

warnForUndefinedGroups(loaderContext, definition.GroupOverrides)
groupOverrideMap := toGroupOverrideMap(definition.GroupOverrides)

warnForUndefinedEnvironments(loaderContext, definition.EnvironmentOverrides)
environmentOverrideMap := toEnvironmentOverrideMap(definition.EnvironmentOverrides)

var results []config.Config
var errs []error
for _, env := range loaderContext.Environments {
for _, env := range loaderContext.Environments.SelectedEnvironments {

result, definitionErrors := parseDefinitionForEnvironment(fs, singleConfigContext, configId, env, definition, groupOverrideMap, environmentOverrideMap)

Expand All @@ -79,6 +83,22 @@ func parseConfigEntry(
return results, nil
}

func warnForUndefinedGroups(loaderContext *configFileLoaderContext, groupOverrides []persistence.GroupOverride) {
for _, group := range groupOverrides {
if _, exists := loaderContext.Environments.AllGroupNames[group.Group]; !exists {
log.Warn("group override references unknown group '%s' which is not defined in the manifest", group.Group)
}
}
}

func warnForUndefinedEnvironments(loaderContext *configFileLoaderContext, environmentOverrides []persistence.EnvironmentOverride) {
for _, environmentOverride := range environmentOverrides {
if _, exists := loaderContext.Environments.AllGroupNames[environmentOverride.Environment]; !exists {
log.Warn("environment override references unknown environment '%s' which is not defined in the manifest", environmentOverride.Environment)
}
}
}

func toEnvironmentOverrideMap(environments []persistence.EnvironmentOverride) map[string]persistence.EnvironmentOverride {
result := make(map[string]persistence.EnvironmentOverride)

Expand Down
2 changes: 1 addition & 1 deletion pkg/config/loader/config_loader.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ import (
type LoaderContext struct {
ProjectId string
Path string
Environments []manifest.EnvironmentDefinition
Environments manifest.Environments
KnownApis map[string]struct{}
ParametersSerDe map[string]parameter.ParameterSerDe
}
Expand Down
22 changes: 15 additions & 7 deletions pkg/config/loader/config_loader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,15 +51,23 @@ func Test_parseConfigs(t *testing.T) {
ProjectId: "project",
Path: "some-dir/",
KnownApis: map[string]struct{}{"some-api": {}, api.DashboardShareSettings: {}},
Environments: []manifest.EnvironmentDefinition{
{
Name: "env name",
URL: manifest.URLDefinition{Type: manifest.ValueURLType, Value: "env url"},
Group: "default",
Auth: manifest.Auth{
Token: &manifest.AuthSecret{Name: "token var"},
Environments: manifest.Environments{
SelectedEnvironments: manifest.EnvironmentDefinitionsByName{
"env name": {
Name: "env name",
URL: manifest.URLDefinition{Type: manifest.ValueURLType, Value: "env url"},
Group: "default",
Auth: manifest.Auth{
Token: &manifest.AuthSecret{Name: "token var"},
},
},
},
AllEnvironmentNames: map[string]struct{}{
"env name": {},
},
AllGroupNames: map[string]struct{}{
"default": {},
},
},
ParametersSerDe: config.DefaultParameterParsers,
}
Expand Down
9 changes: 7 additions & 2 deletions pkg/config/loader/parameter_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,13 @@ func TestParametersAreLoadedAsExpected(t *testing.T) {
fs := afero.NewReadOnlyFs(afero.NewOsFs())

loaderContext := LoaderContext{
Environments: []manifest.EnvironmentDefinition{
{Name: "testEnv"},
Environments: manifest.Environments{
SelectedEnvironments: manifest.EnvironmentDefinitionsByName{
"testEnv": {Name: "testEnv"},
},
AllEnvironmentNames: map[string]struct{}{
"testenv": {},
},
},
KnownApis: map[string]struct{}{"some-api": {}},
ParametersSerDe: config.DefaultParameterParsers,
Expand Down
9 changes: 7 additions & 2 deletions pkg/config/loader/template_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,13 @@ func TestConfigurationTemplatingFromFilesProducesValidJson(t *testing.T) {
fs := afero.NewReadOnlyFs(afero.NewOsFs())

loaderContext := LoaderContext{
Environments: []manifest.EnvironmentDefinition{
{Name: "testEnv"},
Environments: manifest.Environments{
SelectedEnvironments: manifest.EnvironmentDefinitionsByName{
"testEnv": {Name: "testEnv"},
},
AllEnvironmentNames: map[string]struct{}{
"testenv": {},
},
},
KnownApis: map[string]struct{}{"some-api": {}},
ParametersSerDe: config.DefaultParameterParsers,
Expand Down
40 changes: 17 additions & 23 deletions pkg/project/project_loader.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,6 @@ func LoadProjects(ctx context.Context, fs afero.Fs, loaderContext ProjectLoaderC
return nil, []error{fmt.Errorf("no projects defined in manifest")}
}

environments := toEnvironmentSlice(loaderContext.Manifest.Environments.SelectedEnvironments)

projectNamesToLoad, errs := getProjectNamesToLoad(loaderContext.Manifest.Projects, specificProjectNames)

seenProjectNames := make(map[string]struct{}, len(projectNamesToLoad))
Expand All @@ -109,7 +107,7 @@ func LoadProjects(ctx context.Context, fs afero.Fs, loaderContext ProjectLoaderC
continue
}

project, loadProjectErrs := loadProject(ctx, workingDirFs, loaderContext, projectDefinition, environments)
project, loadProjectErrs := loadProject(ctx, workingDirFs, loaderContext, projectDefinition, loaderContext.Manifest.Environments)

if len(loadProjectErrs) > 0 {
errs = append(errs, loadProjectErrs...)
Expand All @@ -119,7 +117,7 @@ func LoadProjects(ctx context.Context, fs afero.Fs, loaderContext ProjectLoaderC

loadedProjects = append(loadedProjects, project)

for _, environment := range environments {
for _, environment := range loaderContext.Manifest.Environments.SelectedEnvironments {
projectNamesToLoad = append(projectNamesToLoad, project.Dependencies[environment.Name]...)
}
}
Expand Down Expand Up @@ -168,17 +166,7 @@ func getProjectNamesToLoad(allProjectsDefinitions manifest.ProjectDefinitionByPr
return projectNamesToLoad, errs
}

func toEnvironmentSlice(environments map[string]manifest.EnvironmentDefinition) []manifest.EnvironmentDefinition {
var result []manifest.EnvironmentDefinition

for _, env := range environments {
result = append(result, env)
}

return result
}

func loadProject(ctx context.Context, fs afero.Fs, loaderContext ProjectLoaderContext, projectDefinition manifest.ProjectDefinition, environments []manifest.EnvironmentDefinition) (Project, []error) {
func loadProject(ctx context.Context, fs afero.Fs, loaderContext ProjectLoaderContext, projectDefinition manifest.ProjectDefinition, environments manifest.Environments) (Project, []error) {
if exists, err := afero.Exists(fs, projectDefinition.Path); err != nil {
formattedErr := fmt.Errorf("failed to load project `%s` (%s): %w", projectDefinition.Name, projectDefinition.Path, err)
report.GetReporterFromContextOrDiscard(ctx).ReportLoading(report.StateError, formattedErr, "", nil)
Expand Down Expand Up @@ -313,7 +301,7 @@ func toConfigMap(configs []config.Config) ConfigsPerTypePerEnvironments {

// loadConfigsOfProject returns the (partial if errors) loaded configs and the errors
func loadConfigsOfProject(ctx context.Context, fs afero.Fs, loadingContext ProjectLoaderContext, projectDefinition manifest.ProjectDefinition,
environments []manifest.EnvironmentDefinition) ([]config.Config, []error) {
environments manifest.Environments) ([]config.Config, []error) {

configFiles, err := files.FindYamlFiles(fs, projectDefinition.Path)
if err != nil {
Expand All @@ -323,13 +311,7 @@ func loadConfigsOfProject(ctx context.Context, fs afero.Fs, loadingContext Proje
var configs []config.Config
var errs []error

loaderContext := &loader.LoaderContext{
ProjectId: projectDefinition.Name,
Environments: environments,
Path: projectDefinition.Path,
KnownApis: loadingContext.KnownApis,
ParametersSerDe: loadingContext.ParametersSerde,
}
loaderContext := newLoaderContext(loadingContext, projectDefinition, environments)

for _, file := range configFiles {
log.WithFields(field.F("file", file)).Debug("Loading configuration file %s", file)
Expand All @@ -341,6 +323,18 @@ func loadConfigsOfProject(ctx context.Context, fs afero.Fs, loadingContext Proje
return configs, errs
}

func newLoaderContext(loadingContext ProjectLoaderContext, projectDefinition manifest.ProjectDefinition,
environments manifest.Environments) *loader.LoaderContext {

return &loader.LoaderContext{
ProjectId: projectDefinition.Name,
Environments: environments,
Path: projectDefinition.Path,
KnownApis: loadingContext.KnownApis,
ParametersSerDe: loadingContext.ParametersSerde,
}
}

func findDuplicatedConfigIdentifiers(ctx context.Context, configs []config.Config, configErrorMap map[coordinate.Coordinate]struct{}) []error {
var errs []error
coordinates := make(map[string]struct{})
Expand Down
133 changes: 131 additions & 2 deletions pkg/project/project_loader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package project

import (
"bytes"
"fmt"
"io/fs"
"reflect"
Expand All @@ -27,6 +28,7 @@ import (
"github.com/stretchr/testify/require"

"github.com/dynatrace/dynatrace-configuration-as-code/v2/internal/errutils"
"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/config"
"github.com/dynatrace/dynatrace-configuration-as-code/v2/pkg/config/coordinate"
Expand Down Expand Up @@ -688,7 +690,7 @@ func Test_loadProject_returnsErrorIfProjectPathDoesNotExist(t *testing.T) {
Path: "this/does/not/exist",
}

_, gotErrs := loadProject(t.Context(), fs, loaderContext, definition, []manifest.EnvironmentDefinition{})
_, gotErrs := loadProject(t.Context(), fs, loaderContext, definition, manifest.Environments{})
assert.Len(t, gotErrs, 1)
assert.ErrorContains(t, gotErrs[0], "filepath `this/does/not/exist` does not exist")
}
Expand Down Expand Up @@ -716,7 +718,14 @@ func Test_loadProject_returnsErrorIfScopeForWebKUAhasWrongTypeOfParameter(t *tes
name: key-user-actions-web
scope: APPLICATION-3F2C9E73509D15B6`), 0644))
require.NoError(t, afero.WriteFile(testFs, "project/kua-web/kua-web.json", []byte("{}"), 0644))
_, gotErrs := loadProject(t.Context(), testFs, loaderContext, definition, []manifest.EnvironmentDefinition{{Name: "env"}})
_, gotErrs := loadProject(t.Context(), testFs, loaderContext, definition, manifest.Environments{
SelectedEnvironments: manifest.EnvironmentDefinitionsByName{
"testEnv": {Name: "testEnv"},
},
AllEnvironmentNames: map[string]struct{}{
"testenv": {},
},
})
assert.Len(t, gotErrs, 1)
assert.ErrorContains(t, gotErrs[0], "scope parameter of config of type 'key-user-actions-web' with ID 'kua-web-1' needs to be a reference parameter to another web-application config")
}
Expand Down Expand Up @@ -1522,6 +1531,126 @@ func TestLoadProjects_NetworkZonesContainsParameterToSetting(t *testing.T) {
assert.Contains(t, networkZone2.Parameters, "__MONACO_NZONE_ENABLED__")
}

// TestLoadProjects_EnvironmentOverrideWithUndefinedEnvironmentProducesWarning tests that referencing an undefined environment in an environment override produces a warning.
func TestLoadProjects_EnvironmentOverrideWithUndefinedEnvironmentProducesWarning(t *testing.T) {
managementZoneConfig := []byte(`configs:
- id: mz
config:
template: mz.json
type:
settings:
schema: builtin:management-zones
scope: environment
environmentOverrides:
- environment: prod
override:
skip: true
`)

managementZoneJSON := []byte(`{ "name": "", "rules": [] }`)

testFs := testutils.TempFs(t)
logSpy := bytes.Buffer{}
log.PrepareLogging(t.Context(), afero.NewMemMapFs(), false, &logSpy, false, false)

require.NoError(t, testFs.MkdirAll("a/builtinmanagement-zones", testDirectoryFileMode))
require.NoError(t, afero.WriteFile(testFs, "a/builtinmanagement-zones/config.yaml", managementZoneConfig, testFileFileMode))
require.NoError(t, afero.WriteFile(testFs, "a/builtinmanagement-zones/mz.json", managementZoneJSON, testFileFileMode))

testContext := ProjectLoaderContext{
KnownApis: map[string]struct{}{"builtin:management-zones": {}},
WorkingDir: ".",
Manifest: manifest.Manifest{
Projects: manifest.ProjectDefinitionByProjectID{
"a": {
Name: "a",
Path: "a/",
},
},
Environments: manifest.Environments{
SelectedEnvironments: manifest.EnvironmentDefinitionsByName{
"dev": {
Name: "dev",
Auth: manifest.Auth{Token: &manifest.AuthSecret{Name: "ENV_VAR"}},
},
},
AllEnvironmentNames: map[string]struct{}{
"dev": {},
},
},
},
ParametersSerde: config.DefaultParameterParsers,
}

gotProjects, gotErrs := LoadProjects(t.Context(), testFs, testContext, nil)
assert.Len(t, gotErrs, 0, "Expected no errors loading dependent projects ")
assert.Len(t, gotProjects, 1)

assert.Contains(t, logSpy.String(), "environment override references unknown environment 'prod'")
}

// TestLoadProjects_GroupOverrideWithUndefinedGroupProducesWarning tests that referencing an undefined environment group in a group override produces a warning.
func TestLoadProjects_GroupOverrideWithUndefinedGroupProducesWarning(t *testing.T) {
managementZoneConfig := []byte(`configs:
- id: mz
config:
template: mz.json
type:
settings:
schema: builtin:management-zones
scope: environment
groupOverrides:
- group: prod
override:
skip: true
`)

managementZoneJSON := []byte(`{ "name": "", "rules": [] }`)

testFs := testutils.TempFs(t)

logSpy := bytes.Buffer{}
log.PrepareLogging(t.Context(), afero.NewMemMapFs(), false, &logSpy, false, false)

require.NoError(t, testFs.MkdirAll("a/builtinmanagement-zones", testDirectoryFileMode))
require.NoError(t, afero.WriteFile(testFs, "a/builtinmanagement-zones/config.yaml", managementZoneConfig, testFileFileMode))
require.NoError(t, afero.WriteFile(testFs, "a/builtinmanagement-zones/mz.json", managementZoneJSON, testFileFileMode))
testContext := ProjectLoaderContext{
KnownApis: map[string]struct{}{"builtin:management-zones": {}},
WorkingDir: ".",
Manifest: manifest.Manifest{
Projects: manifest.ProjectDefinitionByProjectID{
"a": {
Name: "a",
Path: "a/",
},
},
Environments: manifest.Environments{
SelectedEnvironments: manifest.EnvironmentDefinitionsByName{
"dev": {
Name: "dev",
Group: "dev",
Auth: manifest.Auth{Token: &manifest.AuthSecret{Name: "ENV_VAR"}},
},
},
AllEnvironmentNames: map[string]struct{}{
"dev": {},
},
AllGroupNames: map[string]struct{}{
"dev": {},
},
},
},
ParametersSerde: config.DefaultParameterParsers,
}

gotProjects, gotErrs := LoadProjects(t.Context(), testFs, testContext, nil)
assert.Len(t, gotErrs, 0, "Expected no errors loading dependent projects ")
assert.Len(t, gotProjects, 1)

assert.Contains(t, logSpy.String(), "group override references unknown group 'prod'")
}

type propResolver func(coordinate.Coordinate, string) (any, bool)

func (p propResolver) GetResolvedProperty(coordinate coordinate.Coordinate, propertyName string) (any, bool) {
Expand Down