-
Notifications
You must be signed in to change notification settings - Fork 98
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
Conversation
e234621
to
8e0024b
Compare
E2E Test Results 3 files ± 0 263 suites ±0 25m 30s ⏱️ + 2m 57s For more details on these failures, see this check. Results for commit 92c2c3b. ± Comparison against base commit 784755a. This pull request removes 26 and adds 3 tests. Note that renamed tests count towards both.
♻️ This comment has been updated with latest results. |
8505894
to
b19958e
Compare
8e0024b
to
11c3d15
Compare
@@ -55,7 +56,19 @@ func parseConfigEntry( | |||
return nil, []error{newDefinitionParserError(configId, singleConfigContext, err.Error())} | |||
} | |||
|
|||
for _, group := range definition.GroupOverrides { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would move each check into a separate function to decrease the complexity and have an opportunity to comment on what is checked under which assumption.
My opinion you decide if it makes sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have also improved the actual log message.
11c3d15
to
c66b269
Compare
c66b269
to
70134f1
Compare
70134f1
to
92c2c3b
Compare
92c2c3b
to
8317a0b
Compare
8317a0b
to
0b08984
Compare
|
Why this PR?
When copying and pasting configs between projects, it is easy to forget to update any overrides. Currently, this is silent: the overrides simply have no effect, which can have undesirable consequences.
What has changed?
With this PR, a warning is emitted when an override references an environment or environment group that is not defined in the manifest.
A decision was made not to emit an error, at least for now, as this would be a breaking change.
How does it do it?
This PR builds on #1897, which adds the names of all environments and groups to the loaded manifest, regardless of whether they're selected to be deployed or not.
In this PR, when a config entry is loaded, the environment or group name of each override is checked against those in the manifest.
How is it tested?
TestLoadProjects_GroupOverrideWithUndefinedGroupProducesWarning
TestLoadProjects_EnvironmentOverrideWithUndefinedEnvironmentProducesWarning
How does it affect users?
Users will now receive a warning when an override references an environment or environment group that is not defined in the manifest.