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

Dynamic Resource IDs / a replacement for azure.ValidateResourceID #189

Open
tombuildsstuff opened this issue Nov 3, 2023 · 4 comments
Open
Assignees
Labels
enhancement New feature or request

Comments

@tombuildsstuff
Copy link
Contributor

tombuildsstuff commented Nov 3, 2023

The new Resource ID parsing logic defines Go Structs for each Resource ID Type, which allow parsing a Resource ID based on a known set of Resource ID segments, allowing us to provide differentiation based on the Resource ID Segment Type in the future.

However some Azure Resources support specifying any Azure Resource ID as an argument, meaning that today we have a validation function in the form of azure.ValidateResourceID which is used to ensure that the argument looks vaguely like an Azure Resource ID, but only ensures that we have a matching number of Key/Value Pairs.

Resource ID Scopes follow a similar pattern, with the Scope being an Azure Resource ID of some description - which (at this point in time) gets parsed out as a literal string value - but isn't validated. The behaviour of both of these differs from the rest of the new Resource ID parsing logic, which implies a known set of Resource ID Segments for a given Resource ID - since these can be any Azure Resource ID.

This means that we're currently unable to use the ResourceIDReference Common Schema types and that we're unable to offer dynamic recasing to workaround issues where the Azure API returns a different casing than is defined in the Terraform configuration.


This commit introduces a proof-of-concept dynamic recaser, which means that we can dynamically recase a Resource ID, for example:

recaser.ReCase("/subscriptions/11111/resourcegroups/bobby/providers/Microsoft.Compute/availabilitySets/HeYO")

gets output as:

/subscriptions/11111/resourceGroups/bobby/providers/Microsoft.Compute/availabilitySets/HeYO

This is achieved by having an init() in each Resource ID type register itself with the recaser, meaning that as Resource ID Types are imported, these get registered, meaning those Resource IDs can be used in this recasing function - which can be used on scopes, fixing issues such as hashicorp/terraform-provider-azurerm#23644.

This logic should also be reusable for an updated ValidateGenericAzureResourceID function too - which presumably requires parsing the Resource ID twice, once case-insensitively (to check that the Resource ID type is valid, where an error can mean it's the wrong type) - and a second time to ensure the casing matches what we expect (which can be surfaced as an error).

In both cases this allows for best-effort validation and recasing of Resource IDs, without knowing the specific types being used - meaning that in the event we know the Resource ID type we can ensure the casing matches what we're expecting - and in the event we don't we can fallback to validating only a common set of keys (subscriptions, resourceGroups` etc).

This obviously requires additional investigation, however in early testing seems promising - however is dependent on #190 which introduces a new FromParseResult method that this functionality uses - but in time will allow us to roll out the commonschema.ResourceIDReference to better hand Resource ID casing.

@tombuildsstuff
Copy link
Contributor Author

For the sake of completeness, there is an alternative approach for the azure.ValidateResourceID replacement which was considered - which involves parsing the URIs within Azure/azure-rest-api-specs to pull out a unique set of values for the Key Segments for the Resource ID Segments - however that approach would mean we're not able to output the rich error messages when the specified value doesn't match the Resource ID type - which is an additional reason for using the approach above.

@ngdangdat
Copy link

I have encountered this while working with azurerm provider in Terraform. I'm using a managed certificate for containerapp but it seems azurerm doesn't support yet. When I create it manually and paste certificate ID for the custom domain, it throws the pattern failed error.

Does this fall under this issue as well? I can help fix the issue although I'm not familiar with the source code.

Error: parsing "/subscriptions/***/resourceGroups/staging/providers/Microsoft.App/managedEnvironments/advanzo-ace/managedCertificates/***": parsing segment "staticCertificates": parsing the Certificate ID: the segment at position 8 didn't match

│ Expected a Certificate ID that matched:

│ > /subscriptions/12345678-1234-9876-4563-123456789012/resourceGroups/example-resource-group/providers/Microsoft.App/managedEnvironments/managedEnvironmentValue/certificates/certificateValue

│ However this value was provided:

│ > /subscriptions/***/resourceGroups/staging/providers/Microsoft.App/managedEnvironments/advanzo-ace/managedCertificates/***

@tombuildsstuff
Copy link
Contributor Author

@ngdangdat

Does this fall under this issue as well?

Not in this case - it appears that the incorrect Resource ID is being specified for that field, which is expecting the ID of a Container App Certificate rather than the ID of a Container App Managed Certificate, which are different Resource types. Without digging into the API there, I'd guess that the first is an uploaded certificate and the other is a certificate purchased/managed by Container Apps - but since they're different Resource Types within the API they're going to be both different from a payload/behavioural perspective.

It could well be that the resource supports both types of IDs for that field, which would also need digging into - but that's either going to be a configuration issue (i.e. that the wrong Resource ID/Type is being provided, as the error message is suggesting above) or that an enhancement to the resource is required to support both types of certificates (i.e. which'd be a feature request to the AzureRM Provider).

Hope that helps clarify that :)

@ngdangdat
Copy link

@tombuildsstuff
Thanks for the information. Before posting the comment, I also think that it's not appropriate to post it here but I'm not sure which
repository/component to raise the issue.

it appears that the incorrect Resource ID is being specified for that field

It's actually a correct resource ID inputted using the Azure portal (Azure's client side). However, managed certificate for container app seems not being supported by Azure CLI (az containerapp env certificate list doesn't show managed certs) and SDK (Terraform provider doesn't have).

After being updated manually via portal, the next time I applied Terraform, I had to ignore the field in Terraform so that it wouldn't be updated by the old self-managed certificate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants