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

Conditional loading of the Subscription ID / Tenant ID / Environment #574

Merged
merged 3 commits into from
Dec 6, 2017

Conversation

tombuildsstuff
Copy link
Contributor

Needs some tests / validation - but seems promising in some initial testing

Fixes #562

@pmarques
Copy link

pmarques commented Nov 18, 2017

@tombuildsstuff I just read your changes but it seems to be error-prone when someone has multiple subscriptions and multiple tenants (multiple accounts).
If I try to use subscription-A and my default subscription is subscription-B for a different tenant this seems to pick the wrong tenant.

@tombuildsstuff
Copy link
Contributor Author

@pmarques thanks for the feedback, I wasn't sure that scenario was possible - so I'll update this to take that into account - thanks!

…vironment from the SubscriptionID

Splitting out the authentication logic into a helpers folder
Also adding unit tests for these - which pass:

```
$ go test . -v
=== RUN   TestAzureFindValidAccessTokenForTenant_InvalidDate
--- PASS: TestAzureFindValidAccessTokenForTenant_InvalidDate (0.00s)
=== RUN   TestAzureFindValidAccessTokenForTenant_Expired
2017/11/30 15:02:01 [DEBUG] Token "7cabcf30-8dca-43f9-91e6-fd56dfb8632f" has expired
--- PASS: TestAzureFindValidAccessTokenForTenant_Expired (0.00s)
=== RUN   TestAzureFindValidAccessTokenForTenant_ExpiringIn
--- PASS: TestAzureFindValidAccessTokenForTenant_ExpiringIn (0.00s)
=== RUN   TestAzureFindValidAccessTokenForTenant_InvalidManagementDomain
2017/11/30 15:02:01 [DEBUG] Resource "https://portal.azure.com/" isn't a management domain
--- PASS: TestAzureFindValidAccessTokenForTenant_InvalidManagementDomain (0.00s)
=== RUN   TestAzureFindValidAccessTokenForTenant_DifferentTenant
2017/11/30 15:02:01 [DEBUG] Resource "https://management.core.windows.net/" isn't for the correct Tenant
--- PASS: TestAzureFindValidAccessTokenForTenant_DifferentTenant (0.00s)
=== RUN   TestAzureFindValidAccessTokenForTenant_ValidFromCloudShell
--- PASS: TestAzureFindValidAccessTokenForTenant_ValidFromCloudShell (0.00s)
=== RUN   TestAzureFindValidAccessTokenForTenant_ValidFromAzureCLI
--- PASS: TestAzureFindValidAccessTokenForTenant_ValidFromAzureCLI (0.00s)
=== RUN   TestAzureFindValidAccessTokenForTenant_NoTokens
--- PASS: TestAzureFindValidAccessTokenForTenant_NoTokens (0.00s)
=== RUN   TestAzureCLIProfileFindDefaultSubscription
--- PASS: TestAzureCLIProfileFindDefaultSubscription (0.00s)
=== RUN   TestAzureCLIProfileFindSubscription
--- PASS: TestAzureCLIProfileFindSubscription (0.00s)
=== RUN   TestAzurePopulateSubscriptionFromCLIProfile_Missing
--- PASS: TestAzurePopulateSubscriptionFromCLIProfile_Missing (0.00s)
=== RUN   TestAzurePopulateSubscriptionFromCLIProfile_NoDefault
--- PASS: TestAzurePopulateSubscriptionFromCLIProfile_NoDefault (0.00s)
=== RUN   TestAzurePopulateSubscriptionFromCLIProfile_Default
--- PASS: TestAzurePopulateSubscriptionFromCLIProfile_Default (0.00s)
=== RUN   TestAzurePopulateTenantAndEnvironmentFromCLIProfile_Empty
--- PASS: TestAzurePopulateTenantAndEnvironmentFromCLIProfile_Empty (0.00s)
=== RUN   TestAzurePopulateTenantAndEnvironmentFromCLIProfile_MissingSubscription
--- PASS: TestAzurePopulateTenantAndEnvironmentFromCLIProfile_MissingSubscription (0.00s)
=== RUN   TestAzurePopulateTenantAndEnvironmentFromCLIProfile_PopulateEnvironment
--- PASS: TestAzurePopulateTenantAndEnvironmentFromCLIProfile_PopulateEnvironment (0.00s)
=== RUN   TestAzurePopulateTenantAndEnvironmentFromCLIProfile_NormaliseAndPopulateEnvironment
--- PASS: TestAzurePopulateTenantAndEnvironmentFromCLIProfile_NormaliseAndPopulateEnvironment (0.00s)
=== RUN   TestAzurePopulateTenantAndEnvironmentFromCLIProfile_PopulateTenantId
--- PASS: TestAzurePopulateTenantAndEnvironmentFromCLIProfile_PopulateTenantId (0.00s)
=== RUN   TestAzurePopulateTenantAndEnvironmentFromCLIProfile_Complete
--- PASS: TestAzurePopulateTenantAndEnvironmentFromCLIProfile_Complete (0.00s)
=== RUN   TestAzurePopulateFromAccessToken_Missing
--- PASS: TestAzurePopulateFromAccessToken_Missing (0.00s)
=== RUN   TestAzurePopulateFromAccessToken_Exists
--- PASS: TestAzurePopulateFromAccessToken_Exists (0.00s)
=== RUN   TestAzureEnvironmentNames
--- PASS: TestAzureEnvironmentNames (0.00s)
=== RUN   TestAzureValidateBearerAuth
--- PASS: TestAzureValidateBearerAuth (0.00s)
=== RUN   TestAzureValidateServicePrincipal
--- PASS: TestAzureValidateServicePrincipal (0.00s)
PASS
ok      github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/authentication    0.012s
```
@tombuildsstuff tombuildsstuff changed the title [WIP] Conditional loading of the Subscription ID / Tenant ID / Environment Conditional loading of the Subscription ID / Tenant ID / Environment Nov 30, 2017
@tombuildsstuff
Copy link
Contributor Author

@pmarques thanks for the testing suggestions :)

I've gone through and verified these scenarios:

  • Specifying a Service Principal through Environment Variables
  • Logging into CloudShell with a Single Subscription in a Single Tenant
  • Logging into CloudShell with Multiple Subscriptions in a Single Tenant (as shown below)
  • Logging into the Azure CLI with a Single Subscription in a Single Tenant
  • Logging into the Azure CLI with multiple Subscriptions in the same Tenant - as shown below:
provider "azurerm" {
  alias           = "First"
  subscription_id = "00000000-0000-0000-0000-000000000000"
}

resource "azurerm_resource_group" "first" {
  name     = "tharvey-dev1"
  location = "West Europe"
  provider = "azurerm.First"
}

provider "azurerm" {
  alias = "Second"
}

resource "azurerm_resource_group" "second" {
  name     = "tharvey-dev2"
  location = "West Europe"
  provider = "azurerm.Second"
}

provider "azurerm" {
  alias           = "Third"
  subscription_id = "00000000-0000-0000-0000-000000000000"
  tenant_id       = "00000000-0000-0000-0000-000000000000"
}

resource "azurerm_resource_group" "third" {
  name     = "tharvey-dev3"
  location = "West Europe"
  provider = "azurerm.Third"
}

However I'm unable to test the scenario of using Multiple Subscriptions in different Tenants (since we don't have an account with that configuration).

If you've got access to an account with this configuration and feel comfortable - I'd really appreciate it if you could test this works - the best way to do that would be to check out this branch and build it locally as outlined here. That said - I'm relatively confident that this should work.

Thanks!

Copy link
Member

@mbfrahry mbfrahry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@tombuildsstuff tombuildsstuff merged commit 1005762 into master Dec 6, 2017
@tombuildsstuff tombuildsstuff deleted the provider-block branch December 6, 2017 09:35
tombuildsstuff added a commit that referenced this pull request Dec 6, 2017
sebastus added a commit to sebastus/terraform-provider-azurerm that referenced this pull request Dec 8, 2017
* Provision sample for ASP.NET on azure_rm_app_service

* Added vnet datasource

* add identity property to vm

* refactor, tests and docs

* added vnet_peering

* changing to TypeMap

* Updating the Provider block

* Variable consistency and removing unused variables

* Changed to azure_virtual_network, added crash control and added documentation.

* vmss: Support for updating the customData field

Fixes hashicorp#61
Fixes hashicorp#490

* Updating to include hashicorp#559

* Support for Auto Inflating

```
$ acctests azurerm TestAccAzureRMEventHubNamespace_maximumThroughputUnits
=== RUN   TestAccAzureRMEventHubNamespace_maximumThroughputUnits
--- PASS: TestAccAzureRMEventHubNamespace_maximumThroughputUnits (202.41s)
PASS
ok      github.com/terraform-providers/terraform-provider-azurerm/azurerm    202.432s
```

* New Resource: `azurerm_network_watcher`

```
$ acctests azurerm TestAccAzureRMNetworkWatcher_
=== RUN   TestAccAzureRMNetworkWatcher_importBasic
--- PASS: TestAccAzureRMNetworkWatcher_importBasic (75.79s)
=== RUN   TestAccAzureRMNetworkWatcher_importComplete
--- PASS: TestAccAzureRMNetworkWatcher_importComplete (69.85s)
=== RUN   TestAccAzureRMNetworkWatcher_basic
--- PASS: TestAccAzureRMNetworkWatcher_basic (69.62s)
=== RUN   TestAccAzureRMNetworkWatcher_complete
--- PASS: TestAccAzureRMNetworkWatcher_complete (72.16s)
=== RUN   TestAccAzureRMNetworkWatcher_update
--- PASS: TestAccAzureRMNetworkWatcher_update (81.75s)
=== RUN   TestAccAzureRMNetworkWatcher_disappears
--- PASS: TestAccAzureRMNetworkWatcher_disappears (94.38s)
PASS
ok
```

* Updating to include hashicorp#569

* Hotfix: upgrade packages under go-autorest to be v9.4.1.

Intergrate with latest version of go-autorest to read access tokens through new way
customized through environment variable. The old behavior on local shell will be kept.

Notice: for Azure Cloud Shell user, please make sure that they're using latest patched
provider.

* Vendoring the Locks SDK

* New Resource: `azurerm_management_lock`

Note: As the Subscription specific Locks will break other tests; these tests need to be run individually.
As such I've introduced the `TF_ACC_SUBSCRIPTION_PARALLEL_LOCK`  environment variable for this purpose.

Tests pass:

```
$ TF_ACC_SUBSCRIPTION_PARALLEL_LOCK=1 acctests azurerm TestAccAzureRMManagementLock_
=== RUN   TestAccAzureRMManagementLock_importResourceGroupReadOnlyBasic
--- PASS: TestAccAzureRMManagementLock_importResourceGroupReadOnlyBasic (61.52s)
=== RUN   TestAccAzureRMManagementLock_importResourceGroupReadOnlyComplete
--- PASS: TestAccAzureRMManagementLock_importResourceGroupReadOnlyComplete (58.75s)
=== RUN   TestAccAzureRMManagementLock_importResourceGroupCanNotDeleteBasic
--- PASS: TestAccAzureRMManagementLock_importResourceGroupCanNotDeleteBasic (53.38s)
=== RUN   TestAccAzureRMManagementLock_importResourceGroupCanNotDeleteComplete
--- PASS: TestAccAzureRMManagementLock_importResourceGroupCanNotDeleteComplete (46.87s)
=== RUN   TestAccAzureRMManagementLock_importPublicIPCanNotDeleteBasic
--- PASS: TestAccAzureRMManagementLock_importPublicIPCanNotDeleteBasic (80.46s)
=== RUN   TestAccAzureRMManagementLock_importPublicIPReadOnlyBasic
--- PASS: TestAccAzureRMManagementLock_importPublicIPReadOnlyBasic (68.53s)
=== RUN   TestAccAzureRMManagementLock_resourceGroupReadOnlyBasic
--- PASS: TestAccAzureRMManagementLock_resourceGroupReadOnlyBasic (61.24s)
=== RUN   TestAccAzureRMManagementLock_resourceGroupReadOnlyComplete
--- PASS: TestAccAzureRMManagementLock_resourceGroupReadOnlyComplete (64.10s)
=== RUN   TestAccAzureRMManagementLock_resourceGroupCanNotDeleteBasic
--- PASS: TestAccAzureRMManagementLock_resourceGroupCanNotDeleteBasic (72.49s)
=== RUN   TestAccAzureRMManagementLock_resourceGroupCanNotDeleteComplete
--- PASS: TestAccAzureRMManagementLock_resourceGroupCanNotDeleteComplete (113.71s)
=== RUN   TestAccAzureRMManagementLock_publicIPReadOnlyBasic
--- PASS: TestAccAzureRMManagementLock_publicIPReadOnlyBasic (64.05s)
=== RUN   TestAccAzureRMManagementLock_publicIPCanNotDeleteBasic
--- PASS: TestAccAzureRMManagementLock_publicIPCanNotDeleteBasic (94.53s)
=== RUN   TestAccAzureRMManagementLock_subscriptionReadOnlyBasic
--- PASS: TestAccAzureRMManagementLock_subscriptionReadOnlyBasic (17.98s)
=== RUN   TestAccAzureRMManagementLock_subscriptionCanNotDeleteBasic
--- PASS: TestAccAzureRMManagementLock_subscriptionCanNotDeleteBasic (15.20s)
PASS
ok      github.com/terraform-providers/terraform-provider-azurerm/azurerm    872.839s
```

Fixes hashicorp#23

* Updating to include hashicorp#573

* Updating to include hashicorp#571

* Adding validation for the locks name

Tests:
```
$ acctests azurerm TestValidateManagementLockName
=== RUN   TestValidateManagementLockName
--- PASS: TestValidateManagementLockName (0.00s)
PASS
ok      github.com/terraform-providers/terraform-provider-azurerm/azurerm    0.020s
```

* Linting

* Updating to include hashicorp#575

* Updating the changelog for consistency

* removed tabs, used spaces

* add test for issue hashicorp#450

* Updated the way user agent string gets assigned.

* Changed code to make it more readable.

* pr tweaks

* Avoid out of index errors when flattening image data disks.

* Updating to include hashicorp#587

* Updating to include hashicorp#589

* Conditional loading of the Subscription ID / Tenant ID / Environment

* Refactoring the provider block to support determining the TenantID/Environment from the SubscriptionID

Splitting out the authentication logic into a helpers folder
Also adding unit tests for these - which pass:

```
$ go test . -v
=== RUN   TestAzureFindValidAccessTokenForTenant_InvalidDate
--- PASS: TestAzureFindValidAccessTokenForTenant_InvalidDate (0.00s)
=== RUN   TestAzureFindValidAccessTokenForTenant_Expired
2017/11/30 15:02:01 [DEBUG] Token "7cabcf30-8dca-43f9-91e6-fd56dfb8632f" has expired
--- PASS: TestAzureFindValidAccessTokenForTenant_Expired (0.00s)
=== RUN   TestAzureFindValidAccessTokenForTenant_ExpiringIn
--- PASS: TestAzureFindValidAccessTokenForTenant_ExpiringIn (0.00s)
=== RUN   TestAzureFindValidAccessTokenForTenant_InvalidManagementDomain
2017/11/30 15:02:01 [DEBUG] Resource "https://portal.azure.com/" isn't a management domain
--- PASS: TestAzureFindValidAccessTokenForTenant_InvalidManagementDomain (0.00s)
=== RUN   TestAzureFindValidAccessTokenForTenant_DifferentTenant
2017/11/30 15:02:01 [DEBUG] Resource "https://management.core.windows.net/" isn't for the correct Tenant
--- PASS: TestAzureFindValidAccessTokenForTenant_DifferentTenant (0.00s)
=== RUN   TestAzureFindValidAccessTokenForTenant_ValidFromCloudShell
--- PASS: TestAzureFindValidAccessTokenForTenant_ValidFromCloudShell (0.00s)
=== RUN   TestAzureFindValidAccessTokenForTenant_ValidFromAzureCLI
--- PASS: TestAzureFindValidAccessTokenForTenant_ValidFromAzureCLI (0.00s)
=== RUN   TestAzureFindValidAccessTokenForTenant_NoTokens
--- PASS: TestAzureFindValidAccessTokenForTenant_NoTokens (0.00s)
=== RUN   TestAzureCLIProfileFindDefaultSubscription
--- PASS: TestAzureCLIProfileFindDefaultSubscription (0.00s)
=== RUN   TestAzureCLIProfileFindSubscription
--- PASS: TestAzureCLIProfileFindSubscription (0.00s)
=== RUN   TestAzurePopulateSubscriptionFromCLIProfile_Missing
--- PASS: TestAzurePopulateSubscriptionFromCLIProfile_Missing (0.00s)
=== RUN   TestAzurePopulateSubscriptionFromCLIProfile_NoDefault
--- PASS: TestAzurePopulateSubscriptionFromCLIProfile_NoDefault (0.00s)
=== RUN   TestAzurePopulateSubscriptionFromCLIProfile_Default
--- PASS: TestAzurePopulateSubscriptionFromCLIProfile_Default (0.00s)
=== RUN   TestAzurePopulateTenantAndEnvironmentFromCLIProfile_Empty
--- PASS: TestAzurePopulateTenantAndEnvironmentFromCLIProfile_Empty (0.00s)
=== RUN   TestAzurePopulateTenantAndEnvironmentFromCLIProfile_MissingSubscription
--- PASS: TestAzurePopulateTenantAndEnvironmentFromCLIProfile_MissingSubscription (0.00s)
=== RUN   TestAzurePopulateTenantAndEnvironmentFromCLIProfile_PopulateEnvironment
--- PASS: TestAzurePopulateTenantAndEnvironmentFromCLIProfile_PopulateEnvironment (0.00s)
=== RUN   TestAzurePopulateTenantAndEnvironmentFromCLIProfile_NormaliseAndPopulateEnvironment
--- PASS: TestAzurePopulateTenantAndEnvironmentFromCLIProfile_NormaliseAndPopulateEnvironment (0.00s)
=== RUN   TestAzurePopulateTenantAndEnvironmentFromCLIProfile_PopulateTenantId
--- PASS: TestAzurePopulateTenantAndEnvironmentFromCLIProfile_PopulateTenantId (0.00s)
=== RUN   TestAzurePopulateTenantAndEnvironmentFromCLIProfile_Complete
--- PASS: TestAzurePopulateTenantAndEnvironmentFromCLIProfile_Complete (0.00s)
=== RUN   TestAzurePopulateFromAccessToken_Missing
--- PASS: TestAzurePopulateFromAccessToken_Missing (0.00s)
=== RUN   TestAzurePopulateFromAccessToken_Exists
--- PASS: TestAzurePopulateFromAccessToken_Exists (0.00s)
=== RUN   TestAzureEnvironmentNames
--- PASS: TestAzureEnvironmentNames (0.00s)
=== RUN   TestAzureValidateBearerAuth
--- PASS: TestAzureValidateBearerAuth (0.00s)
=== RUN   TestAzureValidateServicePrincipal
--- PASS: TestAzureValidateServicePrincipal (0.00s)
PASS
ok      github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/authentication    0.012s
```

* Fixing the build

* Remove the field marked as "Removed" according to hashicorp#572.

* Upgrading to v11.2.2-beta of the Azure SDK for Go

* Updating to include hashicorp#593

* Fixing the Management Lock validation

* Adding a default value for the identity field

* Updating to include hashicorp#482

* Updating to include hashicorp#574

* Adding settings to the hash

Test passes:

```
$ acctests azurerm TestAccAzureRMVirtualMachineScaleSet_extensionUpdate
=== RUN   TestAccAzureRMVirtualMachineScaleSet_extensionUpdate
--- PASS: TestAccAzureRMVirtualMachineScaleSet_extensionUpdate (593.13s)
PASS
ok      github.com/terraform-providers/terraform-provider-azurerm/azurerm    593.153s
```

* Updating to include hashicorp#609

* Local Network Gateways: support for BGP Settings

```
$ acctests azurerm TestAccAzureRMLocalNetworkGateway_
=== RUN   TestAccAzureRMLocalNetworkGateway_importBasic
--- PASS: TestAccAzureRMLocalNetworkGateway_importBasic (82.23s)
=== RUN   TestAccAzureRMLocalNetworkGateway_basic
--- PASS: TestAccAzureRMLocalNetworkGateway_basic (81.29s)
=== RUN   TestAccAzureRMLocalNetworkGateway_disappears
--- PASS: TestAccAzureRMLocalNetworkGateway_disappears (79.17s)
=== RUN   TestAccAzureRMLocalNetworkGateway_bgpSettings
--- PASS: TestAccAzureRMLocalNetworkGateway_bgpSettings (78.70s)
=== RUN   TestAccAzureRMLocalNetworkGateway_bgpSettingsDisable
--- PASS: TestAccAzureRMLocalNetworkGateway_bgpSettingsDisable (96.18s)
=== RUN   TestAccAzureRMLocalNetworkGateway_bgpSettingsEnable
--- PASS: TestAccAzureRMLocalNetworkGateway_bgpSettingsEnable (97.39s)
=== RUN   TestAccAzureRMLocalNetworkGateway_bgpSettingsComplete
--- PASS: TestAccAzureRMLocalNetworkGateway_bgpSettingsComplete (79.68s)
PASS
ok      github.com/terraform-providers/terraform-provider-azurerm/azurerm    594.680s
```

* Refactoring

* Adding an import test for BGP Settings:

```
$ acctests azurerm TestAccAzureRMLocalNetworkGateway_importBGPSettingsComplete
=== RUN   TestAccAzureRMLocalNetworkGateway_importBGPSettingsComplete
--- PASS: TestAccAzureRMLocalNetworkGateway_importBGPSettingsComplete (80.96s)
PASS
ok      github.com/terraform-providers/terraform-provider-azurerm/azurerm    80.987s
```

* Splitting the data source out into it's own step

* Minor refactoring

* Updating to include hashicorp#533

* Exporting the Default Hostname field

* Updating the App Service example to be complete

This removes support for Publishing, since the SCM URL's aren't consistent across Sovereign Clouds (China/Germany/Govt etc)
Switches to using the new `default_site_hostname` field introduced in hashicorp#612 rather than assuming what it is

* Updating to include hashicorp#594

* Updating to include hashicorp#611

* Updating to include hashicorp#612

* Remove leading line break from key_vault_key docs

Leading line break causes page metadata to be ignored.
@bnygld
Copy link

bnygld commented Jan 31, 2018

I'm still seeing this problem when I specify the Subscription ID in the provider using
Terraform v0.11.2 provider.azurerm v1.1.0

My account has multiple subscriptions and one tenant.

@pmarques
Copy link

pmarques commented Feb 18, 2018

@benny-gold this is currently working for me, do you still have issues? Was using v0.11.2 and now I'm using v0.11.3, so far so good.

@bnygld
Copy link

bnygld commented Feb 20, 2018

all good with v.0.11.3 @pmarques thanks!

@ghost
Copy link

ghost commented Mar 31, 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 Mar 31, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

v0.3.3 azurerm provider fails to read cli auth if subscription_id is set
4 participants