Closed ashpr closed 1 year ago
@fujiwara I'm sorry to bother you but could I get a review on this?
@ashpr Thank you! I am currently occupied a little. I will review your pull request by this weekend.
@ashpr
This PR seems only works when {blob_name} does not include '/'.
For example, If
The two URLs below should work.
However, the current code is unable to correctly detect elements within the URL path.
For compatibility, I would prefer not to include the {subscription_id} in the URL path.
I'm not familiar with Azure, how do you think about this URL format?
azurerm://{subscription_id}@{resource_group_name}/{storage_account_name}/{container_name}/{blob_name}
This format can be parsed by net/url.Parse()
easily, and {subscription_id} is parsed as url.User
https://pkg.go.dev/net/url#URL
Thanks @fujiwara ! I genuinely appreciate the feedback and testing.
I think part of the headache here is I tried to retain the URL format that Azure uses. All Azure resources use the format /subscriptions/{subscription_id}/resourceGroup/{resource_group/resource/{resourceId}
so I was trying to mimic that.
I do like your suggestion to use the user identifier. It should work long-term as the subscription is the highest possible level you can go.
I'll attempt that approach and update this PR accordingly and make sure the test cases described function properly
Thanks again @fujiwara .. Turns out that was a much easier solution! I wish I had done that from the start. Great idea.
I've tested the following scenarios:
I've rebased my commits into a single commit for a cleaner history.
Proof supplied below with multiple subscriptions as I appreciate you're not familiar with Azure:
@ashpr Thank you! Great!!
I've confirmed works well for two patterns of URL that {subscription_id} is included or not.
When using the library programmatically, it may be required to talk to a terraform state stored in a different subscription. A common use case is larger orgs which use DevTest subscriptions and whatnot.
I know that this functionality already exists when using environment variables via
AZURE_SUBSCRIPTION_ID
, however this can be problematic if the user is accessing multiple states in different subscriptions in the same execution pipeline which is our use case when using tfstate-lookup via ArgoCD.Tested on my local machine against our own subscriptions covering both scenarios where a user does and doesn't provide the subscription id.
This is my first punt at Go so feedback is very much welcomed :)