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

backend/azurerm: exclusively using Microsoft Graph/MSAL and removing Azure Active Directory Graph/ADAL #31070

Merged
merged 1 commit into from
Jul 12, 2022

Conversation

tombuildsstuff
Copy link
Contributor

This PR finishes the migration from ADAL to MSAL for the AzureRM Backend - meaning that Microsoft Graph (and MSAL) are now used by default and Azure Active Directory Graph (and ADAL) are no longer available.

Whilst this is technically a breaking change to the config, this has been called out in the v1.1 and v1.2 releases (and is marked as deprecated in v1.2) - we believe that this shouldn't be impactful to users - and ADAL is scheduled for removal in the near future: https://docs.microsoft.com/graph/migrate-azure-ad-graph-faq

$ TF_ACC=1 go test -v ./internal/backend/remote-state/azure/ -timeout=60m
=== RUN   TestBackend_impl
--- PASS: TestBackend_impl (0.00s)
=== RUN   TestBackendConfig
--- PASS: TestBackendConfig (0.00s)
=== RUN   TestAccBackendAccessKeyBasic
--- PASS: TestAccBackendAccessKeyBasic (103.50s)
=== RUN   TestAccBackendSASTokenBasic
--- PASS: TestAccBackendSASTokenBasic (102.84s)
=== RUN   TestAccBackendOIDCBasic
--- PASS: TestAccBackendOIDCBasic (110.92s)
=== RUN   TestAccBackendMSALAzureADAuthBasic
--- PASS: TestAccBackendMSALAzureADAuthBasic (105.68s)
=== RUN   TestAccBackendMSALManagedServiceIdentityBasic
   helpers_test.go:38: Skipping test since not running in Azure
--- SKIP: TestAccBackendMSALManagedServiceIdentityBasic (0.00s)
=== RUN   TestAccBackendMSALServicePrincipalClientCertificateBasic
   backend_test.go:188: Skipping since `ARM_CLIENT_CERTIFICATE_PATH` is not specified!
--- SKIP: TestAccBackendMSALServicePrincipalClientCertificateBasic (0.00s)
=== RUN   TestAccBackendMSALServicePrincipalClientSecretBasic
--- PASS: TestAccBackendMSALServicePrincipalClientSecretBasic (111.77s)
=== RUN   TestAccBackendMSALServicePrincipalClientSecretCustomEndpoint
   backend_test.go:254: Skipping as ARM_ENDPOINT isn't configured
--- SKIP: TestAccBackendMSALServicePrincipalClientSecretCustomEndpoint (0.00s)
=== RUN   TestAccBackendAccessKeyLocked
--- PASS: TestAccBackendAccessKeyLocked (104.27s)
=== RUN   TestAccBackendServicePrincipalLocked
--- PASS: TestAccBackendServicePrincipalLocked (104.48s)
=== RUN   TestRemoteClient_impl
--- PASS: TestRemoteClient_impl (0.00s)
=== RUN   TestRemoteClientAccessKeyBasic
--- PASS: TestRemoteClientAccessKeyBasic (97.54s)
=== RUN   TestRemoteClientManagedServiceIdentityBasic
   helpers_test.go:38: Skipping test since not running in Azure
--- SKIP: TestRemoteClientManagedServiceIdentityBasic (0.00s)
=== RUN   TestRemoteClientSasTokenBasic
--- PASS: TestRemoteClientSasTokenBasic (112.18s)
=== RUN   TestRemoteClientServicePrincipalBasic
--- PASS: TestRemoteClientServicePrincipalBasic (102.07s)
=== RUN   TestRemoteClientAccessKeyLocks
--- PASS: TestRemoteClientAccessKeyLocks (100.13s)
=== RUN   TestRemoteClientServicePrincipalLocks
--- PASS: TestRemoteClientServicePrincipalLocks (103.39s)
=== RUN   TestPutMaintainsMetaData
--- PASS: TestPutMaintainsMetaData (100.11s)
PASS
ok  	github.com/hashicorp/terraform/internal/backend/remote-state/azure	1359.403s

Copy link
Member

@jackofallops jackofallops left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@korinne korinne added this to the v. milestone May 20, 2022
Copy link
Contributor

@manicminer manicminer left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

@apparentlymart
Copy link
Contributor

apparentlymart commented Jun 27, 2022

I don't have sufficient context to comment on the code changes here but just wanted to be explicit that you can merge this when ready since the main branch is already tracking what will become Terraform v1.3, and so we'll only be cutting v1.3 prereleases from this branch until we reach v1.3.0 final. (1.2 releases come from the v1.2 branch.)

@manicminer
Copy link
Contributor

Thanks @apparentlymart, this is ready to go so merging now.

@manicminer manicminer merged commit dc1f5bc into main Jul 12, 2022
@manicminer manicminer deleted the f/removing-adal-auth branch July 12, 2022 11:18
@github-actions
Copy link
Contributor

Reminder for the merging maintainer: if this is a user-visible change, please update the changelog on the appropriate release branch.

@github-actions
Copy link
Contributor

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.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 12, 2022
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.

6 participants