-
Notifications
You must be signed in to change notification settings - Fork 268
Support cross-tenant authentication for remote environment state in Azure Blob Storage #6441
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
base: main
Are you sure you want to change the base?
Conversation
…nant authentication Co-authored-by: vhvb1989 <24213737+vhvb1989@users.noreply.github.com>
Co-authored-by: vhvb1989 <24213737+vhvb1989@users.noreply.github.com>
…mock Co-authored-by: vhvb1989 <24213737+vhvb1989@users.noreply.github.com>
Co-authored-by: vhvb1989 <24213737+vhvb1989@users.noreply.github.com>
|
/azp run azure-dev - cli |
|
Azure Pipelines successfully started running 1 pipeline(s). |
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.
Pull request overview
This PR adds support for cross-tenant authentication when accessing Azure Blob Storage for remote environment state. It addresses an issue where users received InvalidAuthenticationInfo errors when their storage account resided in a different tenant than their home tenant. The solution introduces an optional subscriptionId field to the remote state configuration, which enables tenant resolution for cross-tenant scenarios.
Key Changes:
- Added
SubscriptionIdfield toAccountConfigstruct for optional subscription-based tenant resolution - Enhanced
NewBlobSdkClientto accept aSubscriptionTenantResolverand resolve the tenant from subscription ID when provided - Added comprehensive unit tests covering tenant resolution scenarios (home tenant fallback, successful resolution, and error handling)
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
cli/azd/pkg/azsdk/storage/storage_blob_client.go |
Added SubscriptionTenantResolver interface and SubscriptionId field to AccountConfig; implemented tenant resolution logic in NewBlobSdkClient |
cli/azd/pkg/azsdk/storage/storage_blob_client_test.go |
Added three unit tests verifying tenant resolution behavior for different scenarios |
cli/azd/test/functional/remote_state_test.go |
Updated test helper to provide mock SubscriptionTenantResolver to NewBlobSdkClient |
cli/azd/pkg/environment/manager_test.go |
Added mock SubscriptionTenantResolver registration in test container setup |
cli/azd/go.mod |
Moved pkg/errors from indirect to direct dependencies; removed unused packages |
cli/azd/go.sum |
Updated checksums reflecting dependency changes |
| "github.com/Azure/azure-sdk-for-go/sdk/azcore" | ||
| "github.com/Azure/azure-sdk-for-go/sdk/azcore/policy" | ||
| "github.com/azure/azure-dev/cli/azd/pkg/cloud" | ||
| "github.com/pkg/errors" |
Copilot
AI
Jan 3, 2026
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.
The pkg/errors import is only used once in this test file (line 138) and introduces an unnecessary third-party dependency. Use the standard library errors package instead for new code, which has been sufficient since Go 1.13.
| "github.com/pkg/errors" | |
| "errors" |
cli/azd/go.mod
Outdated
| github.com/moby/patternmatcher v0.6.0 | ||
| github.com/nathan-fiscaletti/consolesize-go v0.0.0-20220204101620-317176b6684d | ||
| github.com/otiai10/copy v1.14.1 | ||
| github.com/pkg/errors v0.9.1 |
Copilot
AI
Jan 3, 2026
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.
The github.com/pkg/errors package should not be added as a direct dependency. It's only used in a test file (storage_blob_client_test.go:138) and goes against modern Go best practices. Since Go 1.13, the standard library provides equivalent error handling capabilities. Remove this from the direct dependencies or better yet, update the test to use the standard errors package.
| github.com/pkg/errors v0.9.1 |
| // SubscriptionTenantResolver allows resolving the correct tenant ID | ||
| // that allows the current account access to a given subscription. | ||
| type SubscriptionTenantResolver interface { | ||
| // Resolve the tenant ID required by the current account to access the given subscription. | ||
| LookupTenant(ctx context.Context, subscriptionId string) (tenantId string, err error) | ||
| } |
Copilot
AI
Jan 3, 2026
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.
This interface is a duplicate of the existing account.SubscriptionTenantResolver interface defined in cli/azd/pkg/account/subscriptions_manager.go. Creating duplicate interfaces reduces maintainability and creates confusion. Instead, import and use account.SubscriptionTenantResolver directly. The IoC container already has a registration for this interface at cli/azd/cmd/container.go:651-653 that converts *account.SubscriptionsManager to account.SubscriptionTenantResolver.
| mockTenantResolver.On("LookupTenant", mock.Anything, testSubscriptionId). | ||
| Return("", errors.New("subscription not found")) |
Copilot
AI
Jan 3, 2026
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.
Using the third-party github.com/pkg/errors package for new code goes against modern Go best practices. Since Go 1.13, the standard library's errors package and fmt.Errorf with %w provide equivalent functionality. For consistency and to avoid unnecessary dependencies, use errors.New("subscription not found") instead, and import the standard errors package.
|
@copilot implemente changes from #6441 (review) |
… and standard errors package Co-authored-by: vhvb1989 <24213737+vhvb1989@users.noreply.github.com>
Changes implemented in commit f961b76:
|
Fix remote environment state authentication for cross-tenant storage accounts
This PR fixes an issue where remote environment state stored in Azure Blob Storage fails with
InvalidAuthenticationInfoerror when the storage account resides in a different tenant than the user's home tenant.Changes Made
SubscriptionIdfield toAccountConfig: Users can now specify a subscription ID in their remote state configurationNewBlobSdkClient: Now acceptsaccount.SubscriptionTenantResolver(existing interface) to resolve the correct tenant for a subscriptionaccount.SubscriptionTenantResolverinterfaceUsage
Users can now specify a subscription ID in their
azure.yamlconfiguration to access storage accounts in different tenants:When
subscriptionIdis not provided, the system uses the home tenant (backward compatible).Code Review Feedback Addressed
SubscriptionTenantResolverinterface and use existingaccount.SubscriptionTenantResolverinsteadgithub.com/pkg/errorswith standard libraryerrorspackagego mod tidyPlan
AccountConfigto supportsubscriptionIdfieldNewBlobSdkClientto accept SubscriptionTenantResolverSecurity Summary
CodeQL security scan completed with 0 alerts - no security vulnerabilities introduced.
Original prompt
This section details on the original issue you should resolve
<issue_title>[Issue] Remote end -
InvalidAuthenticationInfo</issue_title><issue_description>- [x] Make sure you've installed the latest version using instructions in the wiki
Output from
azd versionazd version 1.8.1
Describe the bug
Configured remote env state like this (also tried to define the
containerName)And following error is returned:
To Reproduce
Using terraform-starter template
Expected behavior
This should work.
I'm Owner of the Subscription, also have RBAC roles
Storage Blob Data Ownerand tried to addStorage Blob Data Contributorjust to be on the safe sideEnvironment
Information on your environment:
* IaC Only
* VS Code with freshly build GH Codespaces (Ubuntu:jammy)
Additional Context
I found this issue related to failing test > #3669
I assume the issue was not the test, but the AZD code and the test failed due to issue i'm facing now
</issue_description>
<agent_instructions>Look and find the implementation for remote env and check if it is using a credential using the same tenant-id as the user's home tenant, causing the issue where the account is not in the user's home tenant. Fix it by supporting
subscriptionIdnext to the accountName config for the remote settings. When no subscriptionId is provided, use the user's home tenant id, otherwise, if the subscriptionId is defined, use it to resolve the tenant-id where the subscription is and when asking for auth-tokens with the credential</agent_instructions>Comments on the Issue (you are @copilot in this section)
@rajeshkamal5050 @wbreza can you take a look? @wbreza @petr-stupka Based on your configuration it looks like you are attempting to use the AZD remote state configuration also as your remote storage configuration for terraform.There isn't any reason you could not use the same Azure storage account for both Terraform remote state and AZD remote environment state but right now this would require a bit more configuration.
Take a look at the Terraform configuration feature docs which outlines the required environment variables that you will need to set.
If you also want to use AZD remote environments take a look at the Remote environments feature docs.
@wbreza @petr-stupka You shouldn't need to use the legacy `useAzCliAuth`. As long as your storage account configuration set in your `azure.yaml` and have [Storage Blob Data Contributor](https://learn.microsoft.com/en-us/azure/role-based-access-control/built-in-roles/storage#storage-blob-data-contributor) role on the storage account you should be good to go. Let me know if you are seeing otherwise. @HadwaAbdelhalem still hitting this error when trying to configure azd remote env. @HadwaAbdelhalem For me, IT was due to enforced policies on my test subscription. Once I updated that It worked as expected @rajeshkamal5050 > @petr-stupka You shouldn't need to use the legacy `useAzCliAuth`. As long as your storage account configuration set in your `azure.yaml` and have [Storage Blob Data Contributor](https://learn.microsoft.com/en-us/azure/role-based-access-control/...InvalidAuthenticationInfo#3808✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.