Skip to content
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_management_group validation should allow spaces #6757

Closed
tom-dudley opened this issue May 4, 2020 · 6 comments · Fixed by #6845
Closed

azurerm_management_group validation should allow spaces #6757

tom-dudley opened this issue May 4, 2020 · 6 comments · Fixed by #6845

Comments

@tom-dudley
Copy link
Contributor

tom-dudley commented May 4, 2020

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

Terraform (and AzureRM Provider) Version

Terraform version: 0.12.8
AzureRM version: 2.0.0

Affected Resource(s)

  • azurerm_management_group
  • data.azurerm_management_group

Debug Output

can only consist of ASCII letters, digits, -, _, (, ), . , and cannot exceed the maximum length of 90

Output as shown here: https://github.com/terraform-providers/terraform-provider-azurerm/blob/master/azurerm/internal/services/managementgroup/validate/management_group.go#L17-L20

Expected Behavior

Successful terraform plan

Actual Behavior

Validation fails due to regex not allowing for a space in management group names

Steps to Reproduce

  1. Config:
data azurerm_management_group "foo" {
    name = "group with spaces"
}
  1. terraform plan

References

While the code does contain a comment about validation being shown in the portal, I couldn't find it. Management groups can still be created with spaces. Existing documentation suggests this is also the case too. E.g.: https://docs.microsoft.com/en-us/azure/governance/management-groups/create#create-in-portal

@tom-dudley
Copy link
Contributor Author

There is a related powershell issue (found by a google of the error message in the code comment) here: Azure/azure-powershell#11718

@ArcturusZhang
Copy link
Contributor

Hi @tom-dudley thanks for this issue!
The thing is that in terraform, we call the ID in portal as group_id(deprecated) or name, and the Name in portal as display_name. We have some reasons:

  1. In the rest api specs, this parameter is called group_id
  2. A management group has a request URL/ID structured as
    /providers/Microsoft.Management/managementGroups/{groupId}
    Comparing with other resource IDs, the parameter that has the same position as groupId is usually called something name, therefore we also named this as name.
  3. To call this parameter id or group_id may cause some confusion since we also have a thing /providers/Microsoft.Management/managementGroups/something is also called id. When some other resource want to reference a management group (such like the policy definition), they will have an attribute of management_group_id, but this name is very confusing between group_id or id (actually we have to add a note in the doc to specify that this attribute has to be a group_id rather than id)
  4. There is a DisplayName field in the REST API response of management group.
  5. To align the same naming pattern of azurerm_policy_definition and azurerm_policy_set_definition

As a summary of all the relative reasons above, we name the ID in portal as name and deprecated group_id, and name in portal as display_name.

Back to the question you are asking in this issue, you are trying to get the data source of a management group by using its display_name. Unfortunately this is not supported yet, currently we can only use the name or group_id (same thing as the ID in portal) to retrieve a data source of management group. And I will also take some time to implement this for management group, same functionalities have been implemented for policy definition and policy set definition (you can retrieve data source of policy definition and policy set definition by both its name and display name)

@tom-dudley
Copy link
Contributor Author

@ArcturusZhang Thanks very much for the detailed response. I fully appreciate that you're just going off the current request/response properties available in the REST API.

You have helped highlight that it was indeed the display_name argument I was after, and that this isn't the same as the name, so thanks for clearing that up.

Regarding the not-yet-existing display_name argument, I was wondering if the 'iterate through the management groups and find one with the correct name' approach is at all valid and whether it's a pattern used anywhere else in the azurerm provider. I can definitely appreciate in certain cases it may be an unacceptable number of web requests made though.

@ArcturusZhang
Copy link
Contributor

Yes, the iterate through the management groups and find one with the correct name approach is valid and I am working on that.
For the record, the data source of policy definition and policy set definition supports filtering by display_names.

@ghost
Copy link

ghost commented Jun 19, 2020

This has been released in version 2.15.0 of the provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. As an example:

provider "azurerm" {
    version = "~> 2.15.0"
}
# ... other configuration ...

@ghost
Copy link

ghost commented Jul 18, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks!

@ghost ghost locked and limited conversation to collaborators Jul 18, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.