azurerm_app_configuration_feature - Add support for custom_filter#30160
azurerm_app_configuration_feature - Add support for custom_filter#30160mbfrahry merged 11 commits intohashicorp:mainfrom
azurerm_app_configuration_feature - Add support for custom_filter#30160Conversation
…s in FeatureResource
|
I'm working to run the tests and post the acceptance screenshots here, will provide when possible. Hopefully tomorrow I'll do that, I had some challenges on running because it was trying to create a resource_group. I'm working into that. |
|
Tests added to the PR! |
mbfrahry
left a comment
There was a problem hiding this comment.
Hey @panoramix360! Thanks for taking the time to open this issue. Unfortunately, it doesn't seem possible to remove a custom filter with the way it's been written and I've left a comment for how I found that out. I've also left a couple formatting comments as well
| { | ||
| Config: r.basicNoFilters(data), | ||
| Check: acceptance.ComposeTestCheckFunc( | ||
| check.That(data.ResourceName).ExistsInAzure(r), | ||
| ), | ||
| }, | ||
| data.ImportStep(), | ||
| { | ||
| Config: r.basicCustomFilter(data), | ||
| Check: acceptance.ComposeTestCheckFunc( | ||
| check.That(data.ResourceName).ExistsInAzure(r), | ||
| ), | ||
| }, | ||
| data.ImportStep(), |
There was a problem hiding this comment.
Sometimes I like to check to make sure any new properties we add can also be removed by adding a basic test config to the end of the test. I did that here:
| { | |
| Config: r.basicNoFilters(data), | |
| Check: acceptance.ComposeTestCheckFunc( | |
| check.That(data.ResourceName).ExistsInAzure(r), | |
| ), | |
| }, | |
| data.ImportStep(), | |
| { | |
| Config: r.basicCustomFilter(data), | |
| Check: acceptance.ComposeTestCheckFunc( | |
| check.That(data.ResourceName).ExistsInAzure(r), | |
| ), | |
| }, | |
| data.ImportStep(), | |
| { | |
| Config: r.basicNoFilters(data), | |
| Check: acceptance.ComposeTestCheckFunc( | |
| check.That(data.ResourceName).ExistsInAzure(r), | |
| ), | |
| }, | |
| data.ImportStep(), | |
| { | |
| Config: r.basicCustomFilter(data), | |
| Check: acceptance.ComposeTestCheckFunc( | |
| check.That(data.ResourceName).ExistsInAzure(r), | |
| ), | |
| }, | |
| data.ImportStep(), | |
| { | |
| Config: r.basicNoFilters(data), | |
| Check: acceptance.ComposeTestCheckFunc( | |
| check.That(data.ResourceName).ExistsInAzure(r), | |
| ), | |
| }, | |
| data.ImportStep(), |
And it looks like it's not being removed as I get this error back
Terraform will perform the following actions:
# azurerm_app_configuration_feature.test will be updated in-place
~ resource "azurerm_app_configuration_feature" "test" {
...
- custom_filter {
- name = "custom-filter" -> null
- parameters = {
- "param1" = "123"
- "param2" = "321"
} -> null
}
}
I don't see how your implementation of custom_filter is any different than the others filters so this might be an api issue. Can you dig a little further to see if you can remove a custom filter?
There was a problem hiding this comment.
Added what you mention above.
Tested the terraform plan to remove the filters, and it seems that it's not removing the custom filter entirely, it's just transforming the fields to null, right?
# azurerm_app_configuration_feature.platform["TestFilters/TestLabel"] will be updated in-place
~ resource "azurerm_app_configuration_feature" "platform" {
id = ""
name = "TestFilters"
tags = {
"Environment" = "qa"
"Owner" = ""
"Project" = ""
"Source" = "terraform"
}
# (7 unchanged attributes hidden)
- custom_filter {
- name = "Filter1" -> null
- parameters = {
- "param1" = "value1"
} -> null
}
- custom_filter {
- name = "Filter2" -> null
- parameters = {} -> null
}
}
I will dig a little further and try to remove it in my project, I hope it's not a implementation issue by Azure hehe
There was a problem hiding this comment.
hey @mbfrahry
I tested a bit on my project and it seems the custom filters are being removed correctly.
The plans for the other filters seems to be the same as well.
# azurerm_app_configuration_feature.test will be destroyed
- resource "azurerm_app_configuration_feature" "test" {
- configuration_store_id = "<configuration_store_id>" -> null
- description = "Test description" -> null
- enabled = true -> null
- id = "<resource_id>" -> null
- label = "TestLabel" -> null
- locked = false -> null
- name = "TestFilter" -> null
- percentage_filter_value = 0 -> null
- tags = {
- "environment" = "test"
} -> null
}
I tested removing the custom_filter on my Azure project that I used to test it, and it removed the custom_filter as well, in the example that you sent.
Let me know your thoughts!
…esource_test.go Co-authored-by: Matthew Frahry <mbfrahry@gmail.com>
|
Thank you for the review @mbfrahry! I'll dig a little bit more to see if we can implement the delete feature. Maybe I'll need to debug it a little further, since the struct of the Custom Filter is different to the others. |
mbfrahry
left a comment
There was a problem hiding this comment.
That's weird about that test issue I saw initially but is now good! Looks like we'll be good to once the linter is happy. Thanks for the fixes @panoramix360
|
Thank you @mbfrahry I fixed the lint. I tried running the Make commands locally to check the lint errors, but it seems to be throwing a lot of other lint errors that are not related to any changes that I made. Is there something specific to be done to run the Thanks! |
|
Hi, any updates? |
|
hey @mbfrahry Any new requests for this PR? I can probably solve the merge conflicts this week. |
|
Hi, any updates? |
azurerm_app_configuration_feature: Adds new property custom_filters/client_filtersazurerm_app_configuration_feature - Add support for custom_filter
Community Note
Description
This PR adds the possibility to create custom filters with the resource
azurerm_app_configuration_featureaka Feature Flags on Azure, this was not supported before and didn't worked, now it adds the possibility to create them there.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- support for thecustom_filterpropertyThis is a (please select all that apply):
Related Issue(s)
Closes #23497
Rollback Plan
If a change needs to be reverted, we will publish an updated version of the provider.
Note
If this PR changes meaningfully during the course of review please update the title and description as required.
Special thanks to @phsmith for all your help in running and testing these changes