-
Notifications
You must be signed in to change notification settings - Fork 4.5k
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
azurerm_app_configuration_feature - fix feature flags are not valid #26506
base: main
Are you sure you want to change the base?
Conversation
Fixes error while unmarshalling underlying key's value: unknown type ""
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.
Thanks for this @ksmets. It should be possible to add an acceptance test that follows the reproduction steps you've outlined in the PR description to validate this change. Would you mind adding that?
@@ -480,6 +502,27 @@ resource "azurerm_app_configuration_feature" "test" { | |||
`, t.template(data), data.RandomInteger) | |||
} | |||
|
|||
func (t AppConfigurationFeatureResource) basicTargetingFilter(data acceptance.TestData) string { |
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.
Thanks for adding the test, looks like a required argument is missing here though:
------- Stdout: -------
=== RUN TestAccAppConfigurationFeature_basicAddTargetingFilter
=== PAUSE TestAccAppConfigurationFeature_basicAddTargetingFilter
=== CONT TestAccAppConfigurationFeature_basicAddTargetingFilter
testcase.go:113: Step 3/4 error: Error running pre-apply refresh: exit status 1
Error: Missing required argument
on terraform_plugin_test.tf line 59, in resource "azurerm_app_configuration_feature" "test":
59: targeting_filter {
The argument "default_rollout_percentage" is required, but no definition was
found.
…t_percentage to targeting filter
Community Note
Description
Adding a targeting filter, after first creating a feature flag without one, results in a feature flag that is not valid.
To reproduce. First create a feature flag without targeting filter, e.g.
Then, re-apply with a targeting filter, e.g.
This results in an invalid feature flag as indicated in the feature manager:
The fix is to easy: to rule out whether the default
percentageFilter := PercentageFeatureFilter{}
is assigned when iteratingfv.Conditions.ClientFilters.Filters
, we change theelse if
-clause to checkpercentageFilter.Name != ""
.PR Checklist
For example: “
resource_name_here
- description of change e.g. adding propertynew_property_name_here
”Changes to existing Resource / Data Source
Testing
Change Log
Below please provide what should go into the changelog (if anything) conforming to the Changelog Format documented here.
azurerm_app_configuration_feature
- fix feature flags are not validThis is a (please select all that apply):
Related Issue(s)
Note
If this PR changes meaningfully during the course of review please update the title and description as required.