-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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_resources - Add resource group to output of individual resources. #21676
azurerm_resources - Add resource group to output of individual resources. #21676
Conversation
re := regexp.MustCompile(`.+/resourceGroups/([^/]+).?`) | ||
resResourceGroupName = string(re.FindSubmatch([]byte(*res.ID))[1]) |
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.
Rather than doing this, can we use this function to parse the Resource Group from the ID? That'd handle the resourceGroups
segment differing in casing as required
(note: this does say legacy
in the name as we're working to remove this parser, but we'll be replacing this with something else, so using the older method is fine here)
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.
Hi @tombuildsstuff,
Thanks for the feedback. I'll take a look at ParseAzureResourceID and post here once I've wrapped my head around it and updated the PR.
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.
Hi @tombuildsstuff , I've updated the pull request to use ParseAzureResourceID as requested
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.
hey @marcfor
Thanks for this PR - taking a look through here on the whole this is looking pretty good, but can we update the method being used to parse the Resource Group from the resourceids.ParseAzureResourceID
function, rather than doing this regex - since not all resources returned from the API use the casing resourceGroups
(although that's what's defined in the API Specification) - as such using this function will alleviate that.
Thanks!
instead ofwith regex
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 @marcfor ! 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. |
The azurerm_resources data source is useful for discovering resources in a subscription. However, in order to be able to use specific data sources for a resource type, we need the name of the resource group the resource exists in. Adding a resource_group_name attribute to each resource in the resources list makes it easy to instantiate a data source for the resulting resources.
Example: