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

Support for Subscription ID in AzureRM URL #115

Merged
merged 1 commit into from
May 19, 2023

Conversation

ashpr
Copy link
Contributor

@ashpr ashpr commented May 12, 2023

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 :)

@ashpr
Copy link
Contributor Author

ashpr commented May 16, 2023

@fujiwara I'm sorry to bother you but could I get a review on this?

@fujiwara
Copy link
Owner

@ashpr Thank you! I am currently occupied a little. I will review your pull request by this weekend.

@fujiwara
Copy link
Owner

fujiwara commented May 18, 2023

@ashpr

This PR seems only works when {blob_name} does not include '/'.

For example, If

  • {subscription_id}: xxxx
  • {resource_group_name}: foo
  • {storage_account_name}: bar
  • {container_name}: baz
  • {blob_name} = subdir/terraform.tfstate

The two URLs below should work.

  • azurerm://foo/bar/baz/subdir/terraform.tfstate
  • azurerm://xxxx/foo/bar/baz/subdir/terraform.tfstate

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

@ashpr
Copy link
Contributor Author

ashpr commented May 18, 2023

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

@ashpr ashpr closed this May 18, 2023
@ashpr ashpr reopened this May 18, 2023
@ashpr
Copy link
Contributor Author

ashpr commented May 18, 2023

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:

  • Lookup against a tfstate in a storage account in a different subscription without setting the subscription id, fails (as expected)
  • Lookup against a tfstate in a storage account in a different subscription with setting the subscription id, succeeds
  • Lookup against a tfstate in a storage account in a different subscription, with a subpath, without setting the subscription id, fails (as expected)
  • Lookup against a tfstate in a storage account in a different subscription, with a subpath, with setting the subscription id, succeeds
  • Lookup against a tfstate in a storage account in the accounts subscription, succeeds.
  • Lookup against a tfstate in a storage account in the accounts subscription, with a subpath, succeeds.

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:

image
image
image
image

@fujiwara fujiwara merged commit 2c1f5e9 into fujiwara:main May 19, 2023
@fujiwara
Copy link
Owner

@ashpr Thank you! Great!!

I've confirmed works well for two patterns of URL that {subscription_id} is included or not.

@ashpr ashpr deleted the add-subscription-id-url branch May 19, 2023 11:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants