policy assignment - validate policy assignment name length#30179
Conversation
sreallymatt
left a comment
There was a problem hiding this comment.
Thanks @ezhong-msft, left one comment inline
| ValidateFunc: validation.All( | ||
| validation.StringIsNotWhiteSpace, | ||
| validation.StringDoesNotContainAny("/"), | ||
| validation.StringLenBetween(3, 24), |
There was a problem hiding this comment.
Quickly testing this, it seems the API does accept names of only 1 character like the other *_policy_assignment resources, so to avoid breaking existing configurations that may be using that we should change this to 1
| validation.StringLenBetween(3, 24), | |
| validation.StringLenBetween(1, 24), |
E.g. this worked:
resource "azurerm_management_group_policy_assignment" "test" {
name = "a"
management_group_id = azurerm_management_group.test.id
policy_definition_id = data.azurerm_policy_definition.test.id
parameters = jsonencode({
"listOfAllowedLocations" = {
"value" = [azurerm_resource_group.test.location]
}
})
}Could you update the documentation as well?
There was a problem hiding this comment.
And MS doc also states it accept 1-24 characters.
https://learn.microsoft.com/en-us/azure/azure-resource-manager/management/resource-name-rules
Can we have the provider document updated accordingly as well? Thanks.
There was a problem hiding this comment.
Ah thanks for the docs, was not aware this existed. Updated the length and added the additional validations too.
|
Thanks @ezhong-msft , I'll leave this to @sreallymatt for review as he is the owner of this component. The code looks good to me in general, however wondering whether we should make it super clear in the doc as about what characters are not allowed, but I am not too fussy about this. Thanks again for the work. |
sreallymatt
left a comment
There was a problem hiding this comment.
Thanks @ezhong-msft LGTM ✅
|
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. |
Community Note
Description
Docs were added previously but the validation wasn't being enforced
https://github.com/hashicorp/terraform-provider-azurerm/pull/21549/files
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_resource- support for thething1property [GH-00000]This is a (please select all that apply):
Related Issue(s)
Fixes #30081
Rollback Plan
If a change needs to be reverted, we will publish an updated version of the provider.
Changes to Security Controls
Are there any changes to security controls (access controls, encryption, logging) in this pull request? If so, explain.
Note
If this PR changes meaningfully during the course of review please update the title and description as required.