fix: Add Azure sovereign cloud support (GCC High, China)#2192
Open
fix: Add Azure sovereign cloud support (GCC High, China)#2192
Conversation
Add cloud_environment setting to Azure auth providers for sovereign cloud support. Replace 12 hardcoded Azure endpoints across 6 subsystems with a centralized CloudEnvironment registry supporting public, usgovernment, and china clouds. Fix !terraform.state YAML function for GCC High by reading the azurerm backend environment field for blob storage URL resolution. Closes #2006 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Dependency Review✅ No vulnerabilities or license issues found.Scanned FilesNone |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds Azure sovereign-cloud support: a CloudEnvironment registry (public/usgovernment/china) and propagation of environment-specific endpoints, scopes, portal URLs, and blob storage suffixes through OIDC/device-code/CLI providers, MSAL/profile updates, Terraform azurerm backend, tests, and docs. Changes
Sequence Diagram(s)sequenceDiagram
participant Config as Configuration
participant Provider as Provider (OIDC / Device Code / CLI)
participant CloudEnv as CloudEnvironment Registry
participant TokenSvc as Azure AD
participant MSAL as MSAL Cache/Profile
participant Backend as Terraform Backend
Config->>Provider: Instantiate with cloud_environment
Provider->>CloudEnv: GetCloudEnvironment(name)
CloudEnv-->>Provider: CloudEnvironment(LoginEndpoint,Scopes,BlobSuffix,PortalURL)
Provider->>TokenSvc: Request token (authority = https://<LoginEndpoint>/<tenant>)
TokenSvc-->>Provider: Access token
Provider->>MSAL: Populate cache/profile using env-specific scopes and env name
Provider->>Backend: Use BlobSuffix for service URL / cache key
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
pkg/auth/cloud/azure/setup.go (1)
201-225:⚠️ Potential issue | 🟠 MajorThe new cloud parameter still defaults to
publicon the identity path shown.
pkg/auth/identities/azure/subscription.gois still callingUpdateAzureCLIFiles(..., ""), so any sovereign-cloud identity that goes through this helper will keep writing public-cloud MSAL entries. Please thread the resolved provider environment through that path instead of defaulting here.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/auth/cloud/azure/setup.go` around lines 201 - 225, The UpdateAzureCLIFiles call is being passed an empty string so sovereign-cloud identities still get the public cloud; update the caller in pkg/auth/identities/azure/subscription.go (where UpdateAzureCLIFiles(..., "") is invoked) to pass the resolved provider environment string instead of "" (e.g., the same cloudEnvName you resolved earlier), and ensure that the path uses that value through to UpdateAzureCLIFiles (which calls GetCloudEnvironment(cloudEnvName)) so MSAL entries are written for the correct cloud environment.pkg/auth/providers/azure/device_code.go (1)
346-349:⚠️ Potential issue | 🔴 CriticalUse the selected cloud's management scope for the interactive request.
Line 348 still requests
https://management.azure.com/.default. Forusgovernmentandchina, the first device-code hop will ask for a public-cloud ARM token even though the rest of the provider now keys offp.cloudEnv, so sovereign-cloud logins will fail or cache the wrong audience.Patch
- accessToken, expiresOn, err := p.acquireTokenByDeviceCode(ctx, client, - []string{"https://management.azure.com/.default"}) + accessToken, expiresOn, err := p.acquireTokenByDeviceCode(ctx, client, + []string{p.cloudEnv.ManagementScope})🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/auth/providers/azure/device_code.go` around lines 346 - 349, The device-code flow is using a hard-coded public cloud scope ("https://management.azure.com/.default"); change the call to acquireTokenByDeviceCode in this code path to build the scope from the provider's selected cloud (use p.cloudEnv's management/resource manager endpoint + "/.default") so the initial interactive request uses the correct sovereign-cloud audience (update the call to p.acquireTokenByDeviceCode(...) that currently passes "https://management.azure.com/.default" to instead pass the dynamically constructed scope derived from p.cloudEnv, e.g., p.cloudEnv.ResourceManagerEndpoint + "/.default" or the equivalent management endpoint property).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/fixes/2026-03-13-azure-gcc-high-sovereign-cloud-support.md`:
- Around line 266-271: The identity example uses a top-level provider field
("provider: azure-gov") under the gov-sub identity but the rest of the docs
expect the provider to be nested under via.provider; update the gov-sub identity
(identifier "gov-sub" and its "kind: azure/subscription") to remove the
top-level provider key and instead place the provider value under a via.provider
mapping so the identity shape matches the documented auth schema.
- Around line 108-162: The docs reference the wrong package path; update the
documentation to point to the actual package that contains cloud_environments.go
and its tests (where the CloudEnvironment type and GetCloudEnvironment function
are implemented) instead of the incorrect pkg/auth/providers/azure path, and
make the same correction for the corresponding _test.go references mentioned
later in the note.
In `@internal/terraform_backend/terraform_backend_azurerm_test.go`:
- Around line 981-1075: The tests are tautological because they bypass the
production client/cache-key construction; change
TestReadTerraformBackendAzurermInternal_SovereignCloud to NOT inject a prebuilt
mockAzureBlobClient but instead let ReadTerraformBackendAzurermInternal (the
real client-building path) construct the blob client from backend["environment"]
so the environment handling is exercised; similarly, modify
TestCacheKeySovereignCloud to call the production routine that builds the cache
key (rather than recomputing blobSuffix inline) — e.g., replace the inline
concatenation with the code path/function the production code uses to produce
the cache key (the same routine that currently uses resolveAzureBlobSuffix) so
the test validates behavior rather than duplicating implementation.
In `@pkg/auth/cloud/azure/console.go`:
- Around line 20-22: Resolve the sovereign-cloud bug where ResolveDestination
and resolveDestinationWithDefault always use AzurePortalURL (public cloud):
update the design so the cloud environment is available to these functions —
either add a CloudEnvironment (or cloud name) field to AzureCredentials and
thread it through ResolveDestination/resolveDestinationWithDefault, or add a
CloudEnvironment/cloudName parameter to those function signatures; then replace
uses of AzurePortalURL with GetCloudEnvironment(cloudName).PortalURL (or use the
PortalURL from the AzureCredentials field) inside ResolveDestination,
resolveDestinationWithDefault, and any callers to ensure sovereign clouds
produce correct portal URLs.
In `@pkg/auth/cloud/azure/setup.go`:
- Around line 223-230: The azureProfile update currently hardcodes
"environmentName":"AzureCloud"; pass the resolved cloud environment name from
GetCloudEnvironment (cloudEnv or cloudEnvName) through updateAzureProfile into
UpdateSubscriptionsInProfile and use that value instead of the literal
"AzureCloud" when writing both existing subscriptions and the new subscription
block so GCC High and China profiles get "AzureUSGovernment" and
"AzureChinaCloud" respectively; update function signatures (updateAzureProfile,
UpdateSubscriptionsInProfile) to accept the cloud environment name (or struct)
and set environmentName from that parameter in both code paths.
In `@pkg/auth/identities/azure/subscription.go`:
- Line 201: The call to azureCloud.UpdateAzureCLIFiles currently passes an empty
string for the cloud environment (which defaults to PublicCloud); change the
identity parameter plumbing so the Azure provider's configured cloud environment
is propagated into the identity params and passed to UpdateAzureCLIFiles instead
of "". Specifically, add or populate a cloud/environment field on the identity
params object created where provider config is available (see
pkg/auth/providers/azure/oidc.go), ensure that field is carried into the
identities code path that calls UpdateAzureCLIFiles, and replace the literal ""
in the call to azureCloud.UpdateAzureCLIFiles(params.Credentials,
azureCreds.TenantID, i.subscriptionID, "") with the propagated cloud environment
value.
In `@pkg/auth/providers/azure/oidc.go`:
- Around line 62-68: Environment() and PrepareEnvironment() currently emit
TenantID/ClientID/SubscriptionID/Location but ignore the new CloudEnvironment
field, so update both functions to include CloudEnvironment when set by adding
it to the returned env map (do not add when empty) under the standard keys
"AZURE_ENVIRONMENT" and also "ARM_ENVIRONMENT" for Terraform/ARM consumers;
reference the struct fields TenantID, ClientID, SubscriptionID, Location,
Audience, TokenFilePath, CloudEnvironment and update Environment() and
PrepareEnvironment() to propagate CloudEnvironment into the subprocess
environment maps.
- Around line 105-107: The cloud_environment assignment in
pkg/auth/providers/azure/oidc.go must validate input before storing: if
spec["cloud_environment"] is an empty string leave config.CloudEnvironment as
the default (public), but if non-empty check it against
azureCloud.KnownCloudEnvironments(); on no match return ErrInvalidProviderConfig
with a message listing valid options (same style used for required fields like
tenant_id and client_id in the tenant/client validation block). Apply the same
validation logic at the other occurrence noted (around the similar assignment at
the second location referenced). Use the function/variable names
config.CloudEnvironment, spec["cloud_environment"],
azureCloud.KnownCloudEnvironments(), and ErrInvalidProviderConfig to locate and
implement the change.
In `@website/docs/tutorials/azure-authentication.mdx`:
- Around line 450-573: The "Missing Token Scopes" troubleshooting checklist only
shows commercial scopes and must be made cloud-aware: update the "Missing Token
Scopes" section to detect/mention the selected cloud_environment and list the
correct scope/resource hostnames for each supported cloud (public, usgovernment,
china); include concrete example scopes for management and storage per cloud
(e.g., management/.default and storage/.default using the environment-specific
hostnames) and demonstrate the corresponding checks for providers like
azure-gov, azure-china and azure-gov-oidc so GCC High and China users see the
correct expected scopes.
---
Outside diff comments:
In `@pkg/auth/cloud/azure/setup.go`:
- Around line 201-225: The UpdateAzureCLIFiles call is being passed an empty
string so sovereign-cloud identities still get the public cloud; update the
caller in pkg/auth/identities/azure/subscription.go (where
UpdateAzureCLIFiles(..., "") is invoked) to pass the resolved provider
environment string instead of "" (e.g., the same cloudEnvName you resolved
earlier), and ensure that the path uses that value through to
UpdateAzureCLIFiles (which calls GetCloudEnvironment(cloudEnvName)) so MSAL
entries are written for the correct cloud environment.
In `@pkg/auth/providers/azure/device_code.go`:
- Around line 346-349: The device-code flow is using a hard-coded public cloud
scope ("https://management.azure.com/.default"); change the call to
acquireTokenByDeviceCode in this code path to build the scope from the
provider's selected cloud (use p.cloudEnv's management/resource manager endpoint
+ "/.default") so the initial interactive request uses the correct
sovereign-cloud audience (update the call to p.acquireTokenByDeviceCode(...)
that currently passes "https://management.azure.com/.default" to instead pass
the dynamically constructed scope derived from p.cloudEnv, e.g.,
p.cloudEnv.ResourceManagerEndpoint + "/.default" or the equivalent management
endpoint property).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1672b805-e2b5-4224-b424-ff231c85f83d
📒 Files selected for processing (17)
docs/fixes/2026-03-13-azure-gcc-high-sovereign-cloud-support.mdinternal/terraform_backend/terraform_backend_azurerm.gointernal/terraform_backend/terraform_backend_azurerm_test.gopkg/auth/cloud/azure/cloud_environments.gopkg/auth/cloud/azure/cloud_environments_test.gopkg/auth/cloud/azure/console.gopkg/auth/cloud/azure/setup.gopkg/auth/identities/azure/subscription.gopkg/auth/providers/azure/device_code.gopkg/auth/providers/azure/device_code_cache.gopkg/auth/providers/azure/device_code_cache_test.gopkg/auth/providers/azure/device_code_test.gopkg/auth/providers/azure/oidc.gopkg/auth/providers/azure/oidc_test.gowebsite/docs/cli/configuration/auth/providers.mdxwebsite/docs/stacks/auth.mdxwebsite/docs/tutorials/azure-authentication.mdx
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2192 +/- ##
==========================================
+ Coverage 77.29% 77.36% +0.06%
==========================================
Files 960 961 +1
Lines 91088 91169 +81
==========================================
+ Hits 70410 70533 +123
+ Misses 16593 16558 -35
+ Partials 4085 4078 -7
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
- Wrap ValidateCloudEnvironment error with ErrInvalidAuthConfig sentinel - Introduce ProfileUpdateParams struct to reduce updateAzureProfile and UpdateSubscriptionsInProfile from 6 args to 2 (revive argument-limit) - Update all callers in setup.go, device_code_cache.go, and tests Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pkg/auth/cloud/azure/setup_test.go (1)
743-872:⚠️ Potential issue | 🟡 MinorAdd the China profile-environment case to this table.
This suite only validates
AzureCloudandAzureUSGovernment, so a brokenAzureChinaCloudmapping inupdateAzureProfilewould still slip through even thoughchinais part of the supported matrix.As per coding guidelines, "Every new feature must include comprehensive unit tests targeting >80% code coverage for all packages."♻️ Suggested test case
{ name: "sovereign cloud sets correct environment name", setupProfile: func(home string) { // Don't create profile file. }, username: "admin@gov.onmicrosoft.us", tenantID: "gov-tenant-789", subscriptionID: "gov-sub-456", azureProfileEnvName: "AzureUSGovernment", expectError: false, checkProfile: func(t *testing.T, profile map[string]interface{}) { subs, ok := profile["subscriptions"].([]interface{}) require.True(t, ok) require.Len(t, subs, 1) sub := subs[0].(map[string]interface{}) assert.Equal(t, "gov-sub-456", sub["id"]) assert.Equal(t, "gov-tenant-789", sub["tenantId"]) assert.True(t, sub["isDefault"].(bool)) assert.Equal(t, "AzureUSGovernment", sub["environmentName"]) }, }, + { + name: "china cloud sets correct environment name", + setupProfile: func(home string) { + // Don't create profile file. + }, + username: "admin@china.partner.onmschina.cn", + tenantID: "china-tenant-789", + subscriptionID: "china-sub-456", + azureProfileEnvName: "AzureChinaCloud", + expectError: false, + checkProfile: func(t *testing.T, profile map[string]interface{}) { + subs, ok := profile["subscriptions"].([]interface{}) + require.True(t, ok) + require.Len(t, subs, 1) + + sub := subs[0].(map[string]interface{}) + assert.Equal(t, "china-sub-456", sub["id"]) + assert.Equal(t, "china-tenant-789", sub["tenantId"]) + assert.True(t, sub["isDefault"].(bool)) + assert.Equal(t, "AzureChinaCloud", sub["environmentName"]) + }, + },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/auth/cloud/azure/setup_test.go` around lines 743 - 872, Test suite is missing a case for the China sovereign mapping; add a new entry in the tests slice that mirrors the other cases but sets AzureProfileEnvName to "AzureChinaCloud" (e.g., username "user@china.onmicrosoft.com", tenantID "china-tenant-999", subscriptionID "china-sub-999"), call updateAzureProfile with ProfileUpdateParams as done in other tests, and include a checkProfile function that asserts the created/updated subscription has id "china-sub-999", tenantId "china-tenant-999", isDefault true, and environmentName "AzureChinaCloud" so updateAzureProfile's China mapping is exercised.
🧹 Nitpick comments (1)
pkg/auth/cloud/azure/console.go (1)
175-195: Add perf tracking toResolveDestination.This public helper now owns environment-aware URL resolution but still skips the repo-standard
defer perf.Track(...)()hook.As per coding guidelines, "All public Go functions must include `defer perf.Track(atmosConfig, "pkg.FuncName")()` performance tracking with a blank line after."♻️ Suggested change
func ResolveDestination(dest string, azureCreds *types.AzureCredentials) (string, error) { + defer perf.Track(nil, "azure.ResolveDestination")() + // Validate credentials. if err := validateDestinationCredentials(azureCreds); err != nil { return "", err }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/auth/cloud/azure/console.go` around lines 175 - 195, Add the standard perf tracking hook at the top of the public function ResolveDestination: immediately inside ResolveDestination add a line defer perf.Track(atmosConfig, "pkg/auth/cloud/azure.ResolveDestination")() and ensure there is a blank line after that defer; keep the rest of the function body unchanged. If perf or atmosConfig aren't already available/imported in this file, add the necessary import/variable references consistent with other files in the package.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/fixes/2026-03-13-azure-gcc-high-sovereign-cloud-support.md`:
- Around line 28-31: The fenced code block containing the Azure error message
starting with "failed to get blob from Azure Blob Storage: Get
\"https://redacted.blob.core.windows.net/redacted/tfstate\"" should include a
language specifier to satisfy markdownlint MD040; update the triple-backtick
opener to use ```text (or ```plaintext) so the block becomes ```text followed by
the error lines and closing ```, ensuring the content remains unchanged.
In `@website/docs/tutorials/azure-authentication.mdx`:
- Around line 335-359: The device-code "How it works" list earlier hardcodes
Public scopes; update that list to either reference this "Azure Commercial
(public) / Azure Government (GCC High) / Azure China (Mooncake)" scope table or
replace the hardcoded scopes with a cross-reference sentence pointing readers to
this section (the tables under the headings "Azure Commercial (public):", "Azure
Government (GCC High):", and "Azure China (Mooncake):") so GCC High and China
users see the correct scopes; ensure the device-code walkthrough uses the
variable/cloud-aware language (e.g., mention cloud_environment selection) rather
than fixed public scopes to keep CLI and website docs in sync.
---
Outside diff comments:
In `@pkg/auth/cloud/azure/setup_test.go`:
- Around line 743-872: Test suite is missing a case for the China sovereign
mapping; add a new entry in the tests slice that mirrors the other cases but
sets AzureProfileEnvName to "AzureChinaCloud" (e.g., username
"user@china.onmicrosoft.com", tenantID "china-tenant-999", subscriptionID
"china-sub-999"), call updateAzureProfile with ProfileUpdateParams as done in
other tests, and include a checkProfile function that asserts the
created/updated subscription has id "china-sub-999", tenantId
"china-tenant-999", isDefault true, and environmentName "AzureChinaCloud" so
updateAzureProfile's China mapping is exercised.
---
Nitpick comments:
In `@pkg/auth/cloud/azure/console.go`:
- Around line 175-195: Add the standard perf tracking hook at the top of the
public function ResolveDestination: immediately inside ResolveDestination add a
line defer perf.Track(atmosConfig, "pkg/auth/cloud/azure.ResolveDestination")()
and ensure there is a blank line after that defer; keep the rest of the function
body unchanged. If perf or atmosConfig aren't already available/imported in this
file, add the necessary import/variable references consistent with other files
in the package.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3996cb32-5702-4602-8e69-f4758c9b518a
📒 Files selected for processing (23)
docs/fixes/2026-03-13-azure-gcc-high-sovereign-cloud-support.mdinternal/terraform_backend/terraform_backend_azurerm_test.gopkg/auth/cloud/azure/cloud_environments.gopkg/auth/cloud/azure/cloud_environments_test.gopkg/auth/cloud/azure/console.gopkg/auth/cloud/azure/console_test.gopkg/auth/cloud/azure/env.gopkg/auth/cloud/azure/env_test.gopkg/auth/cloud/azure/setup.gopkg/auth/cloud/azure/setup_test.gopkg/auth/identities/azure/subscription.gopkg/auth/identities/azure/subscription_test.gopkg/auth/providers/azure/cli.gopkg/auth/providers/azure/cli_test.gopkg/auth/providers/azure/device_code.gopkg/auth/providers/azure/device_code_cache.gopkg/auth/providers/azure/device_code_cache_test.gopkg/auth/providers/azure/device_code_test.gopkg/auth/providers/azure/oidc.gopkg/auth/providers/azure/oidc_test.gopkg/auth/types/azure_credentials.gopkg/schema/schema.gowebsite/docs/tutorials/azure-authentication.mdx
🚧 Files skipped from review as they are similar to previous changes (1)
- pkg/auth/providers/azure/device_code_cache_test.go
docs/fixes/2026-03-13-azure-gcc-high-sovereign-cloud-support.md
Outdated
Show resolved
Hide resolved
…d scope note - Add `text` language specifier to error message code block (MD040) - Add cross-link to cloud-specific scopes in device-code "How it works" section Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
website/docs/tutorials/azure-authentication.mdx (2)
519-546: Make the GCC High OIDC example self-contained.This section defines only the provider, and the workflow below never shows how that provider becomes an identity or which identity the command should use. Adding the
auth.identitieswiring here, or explicitly noting that it is defined elsewhere, would make the sovereign example copy/pasteable on its own. As per coding guidelines, "Keep CLI documentation and website documentation in sync and document new features on the website with examples and use cases."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@website/docs/tutorials/azure-authentication.mdx` around lines 519 - 546, The example only defines the azure-gov-oidc provider (auth.providers.azure-gov-oidc) but omits wiring it to an identity, so make the snippet self-contained by adding an auth.identities entry that references the provider (e.g., auth.identities.<name>.provider: azure-gov-oidc) and sets the identity name used by the workflow; update the workflow comment or env to show the identity name being used (so atmos terraform apply mycomponent -s prod is clearly tied to that identity) or add a short note that auth.identities is defined elsewhere.
548-565: Surface the Terraform environment variable this feature drives.
pkg/schema/schema.go:800-805documentscloud_environmentas the value used to setARM_ENVIRONMENT, but this backend section never tells readers what to verify after enablingusgovernmentorchina. Even a short note that Atmos exportsARM_ENVIRONMENThere would make sovereign-cloud Terraform issues much easier to debug. As per coding guidelines, "Keep CLI documentation and website documentation in sync and document new features on the website with examples and use cases."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@website/docs/tutorials/azure-authentication.mdx` around lines 548 - 565, Update the docs section that describes the azurerm backend `environment` value (the YAML example under "Terraform Backend with Sovereign Clouds") to explicitly state that Atmos maps that `environment`/`cloud_environment` setting to the ARM_ENVIRONMENT environment variable; mention the exact symbol names `cloud_environment` (pkg/schema/schema.go lines ~800-805) and `ARM_ENVIRONMENT`, and add a short verification note telling users to check that ARM_ENVIRONMENT is exported with the same value (e.g., `usgovernment` or `china`) after deployment to diagnose sovereign-cloud blob endpoint issues.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/fixes/2026-03-13-azure-gcc-high-sovereign-cloud-support.md`:
- Line 289: Update the incorrect fallback description: replace the sentence
stating "Unknown `cloud_environment` values fall back to `\"public\"` with a
warning" with a statement that unknown non-empty `cloud_environment` values
cause provider initialization to fail with a validation error (because
`ValidateCloudEnvironment` returns an error), and clarify that only an empty
string is silently defaulted to `\"public\"` during provider initialization.
In `@website/docs/tutorials/azure-authentication.mdx`:
- Around line 453-454: The docs overstate support by saying “all Azure clouds”;
update the wording in website/docs/tutorials/azure-authentication.mdx to
explicitly list the supported cloud environments and note fallback behavior:
mention that Atmos supports the registered values public, usgovernment, and
china via the cloud_environment provider setting and that unknown or typoed
values will fall back to public (see pkg/auth/cloud/azure/cloud_environments.go
and the cloud_environment enum/handling). Make the sentence concise and factual
so it no longer implies support for additional Azure clouds.
---
Nitpick comments:
In `@website/docs/tutorials/azure-authentication.mdx`:
- Around line 519-546: The example only defines the azure-gov-oidc provider
(auth.providers.azure-gov-oidc) but omits wiring it to an identity, so make the
snippet self-contained by adding an auth.identities entry that references the
provider (e.g., auth.identities.<name>.provider: azure-gov-oidc) and sets the
identity name used by the workflow; update the workflow comment or env to show
the identity name being used (so atmos terraform apply mycomponent -s prod is
clearly tied to that identity) or add a short note that auth.identities is
defined elsewhere.
- Around line 548-565: Update the docs section that describes the azurerm
backend `environment` value (the YAML example under "Terraform Backend with
Sovereign Clouds") to explicitly state that Atmos maps that
`environment`/`cloud_environment` setting to the ARM_ENVIRONMENT environment
variable; mention the exact symbol names `cloud_environment`
(pkg/schema/schema.go lines ~800-805) and `ARM_ENVIRONMENT`, and add a short
verification note telling users to check that ARM_ENVIRONMENT is exported with
the same value (e.g., `usgovernment` or `china`) after deployment to diagnose
sovereign-cloud blob endpoint issues.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6bc956e8-668d-4038-bf52-ba21385d7f3d
📒 Files selected for processing (2)
docs/fixes/2026-03-13-azure-gcc-high-sovereign-cloud-support.mdwebsite/docs/tutorials/azure-authentication.mdx
docs/fixes/2026-03-13-azure-gcc-high-sovereign-cloud-support.md
Outdated
Show resolved
Hide resolved
…ements - Tighten "all Azure clouds" to list three supported clouds explicitly - Add identity wiring to GCC High OIDC CI/CD example (self-contained) - Surface ARM_ENVIRONMENT export in Terraform backend docs section - Fix incorrect fallback claim: unknown cloud_environment fails, not warns - Add TestResolveUsername (4 cases: valid JWT, SP fallback, user fallback) - Add sovereign cloud Environment() tests for CLI and OIDC providers - All Environment() functions now at 100% coverage Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
pkg/auth/providers/azure/oidc_test.go (1)
1349-1388: Refactor sovereign endpoint scenarios into a table-driven test.This function checks three scenarios but is implemented as repeated subtests. Converting to table-driven format keeps it consistent with repo standards and easier to extend.
♻️ Proposed refactor
func TestOIDCProvider_SovereignCloudEndpoints(t *testing.T) { - t.Run("US government token endpoint", func(t *testing.T) { - provider := &oidcProvider{ - name: "test-oidc", - tenantID: "gov-tenant-123", - clientID: "client-456", - cloudEnv: azureCloud.GetCloudEnvironment("usgovernment"), - tokenEndpoint: "", - } - - endpoint := provider.getTokenEndpoint() - assert.Equal(t, "https://login.microsoftonline.us/gov-tenant-123/oauth2/v2.0/token", endpoint) - }) - - t.Run("China cloud token endpoint", func(t *testing.T) { - provider := &oidcProvider{ - name: "test-oidc", - tenantID: "cn-tenant-456", - clientID: "client-789", - cloudEnv: azureCloud.GetCloudEnvironment("china"), - tokenEndpoint: "", - } - - endpoint := provider.getTokenEndpoint() - assert.Equal(t, "https://login.chinacloudapi.cn/cn-tenant-456/oauth2/v2.0/token", endpoint) - }) - - t.Run("custom endpoint overrides cloud environment", func(t *testing.T) { - provider := &oidcProvider{ - name: "test-oidc", - tenantID: "tenant-123", - clientID: "client-456", - cloudEnv: azureCloud.GetCloudEnvironment("usgovernment"), - tokenEndpoint: "https://custom.endpoint.example.com/token", - } - - endpoint := provider.getTokenEndpoint() - assert.Equal(t, "https://custom.endpoint.example.com/token", endpoint) - }) + tests := []struct { + name string + tenantID string + clientID string + cloudEnvName string + tokenEndpoint string + expected string + }{ + { + name: "US government token endpoint", + tenantID: "gov-tenant-123", + clientID: "client-456", + cloudEnvName: "usgovernment", + expected: "https://login.microsoftonline.us/gov-tenant-123/oauth2/v2.0/token", + }, + { + name: "China cloud token endpoint", + tenantID: "cn-tenant-456", + clientID: "client-789", + cloudEnvName: "china", + expected: "https://login.chinacloudapi.cn/cn-tenant-456/oauth2/v2.0/token", + }, + { + name: "custom endpoint overrides cloud environment", + tenantID: "tenant-123", + clientID: "client-456", + cloudEnvName: "usgovernment", + tokenEndpoint: "https://custom.endpoint.example.com/token", + expected: "https://custom.endpoint.example.com/token", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + provider := &oidcProvider{ + name: "test-oidc", + tenantID: tt.tenantID, + clientID: tt.clientID, + cloudEnv: azureCloud.GetCloudEnvironment(tt.cloudEnvName), + tokenEndpoint: tt.tokenEndpoint, + } + + assert.Equal(t, tt.expected, provider.getTokenEndpoint()) + }) + } }This aligns with the coding guideline for
**/*_test.go: Use table-driven tests for testing multiple scenarios in Go.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/auth/providers/azure/oidc_test.go` around lines 1349 - 1388, Refactor TestOIDCProvider_SovereignCloudEndpoints into a table-driven test: create a slice of test cases each with name, tenantID, cloudEnv (use azureCloud.GetCloudEnvironment with "usgovernment" and "china"), tokenEndpoint input (empty or custom) and expected endpoint, then iterate and run subtests calling the same oidcProvider{...}.getTokenEndpoint() and assert the expected string; keep the three scenarios (US government, China, and custom endpoint override) and reuse the existing oidcProvider field names to construct each case.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/fixes/2026-03-13-azure-gcc-high-sovereign-cloud-support.md`:
- Around line 113-122: The CloudEnvironment struct example is missing the
AzureProfileEnvName field; update the example for the CloudEnvironment type to
include the AzureProfileEnvName string field (used for the azureProfile.json
environment name) so the doc matches the implementation and consumers can see
the full struct definition; locate the CloudEnvironment declaration in the diff
and add a AzureProfileEnvName string comment consistent with the other fields.
- Around line 154-161: Update the comment on GetCloudEnvironment to accurately
reflect runtime behavior: state that only an empty name will fall back to the
"public" environment because unknown non-empty names are rejected earlier by
ValidateCloudEnvironment during provider initialization; note that the function
still defensively returns cloudEnvironments["public"] if validation is bypassed,
but clarify this is not reached in normal provider flow. Reference
GetCloudEnvironment, cloudEnvironments, and ValidateCloudEnvironment in the
comment to make the relationship explicit.
In `@pkg/auth/providers/azure/cli_test.go`:
- Around line 557-614: The test TestCLIProvider_Environment_SovereignCloud
should tighten negative assertions and add an unknown fallback case: update the
test cases for "public" and empty cloudEnvName to assert that both
"ARM_ENVIRONMENT" and "AZURE_ENVIRONMENT" are not present in the returned map
from cliProvider.Environment(), and add a new test case with cloudEnvName
"unknown" (or any non-mapped value) that verifies Environment() falls back to
public by asserting neither "ARM_ENVIRONMENT" nor "AZURE_ENVIRONMENT" exist;
locate the checks around p := &cliProvider{...} and the env, err :=
p.Environment() block to modify the assertions and add the new test case.
In `@website/docs/tutorials/azure-authentication.mdx`:
- Around line 576-583: Update the "Supported Cloud Environments" section to
explicitly state that the same cloud_environment values (`public`,
`usgovernment`, `china`) also apply to `kind: azure/cli`; add a brief note or
parenthetical line directly beneath the table referencing `cloud_environment`
and `kind: azure/cli` and, if possible, include a one-line example or pointer
showing `kind: azure/cli` using `cloud_environment: usgovernment` to mirror the
sovereign-cloud walkthrough examples.
- Around line 559-575: Update the doc to explicitly state that
backend.azurerm.environment (which controls !terraform.state blob endpoint
resolution) must match spec.cloud_environment (which controls auth endpoints and
exported ARM_ENVIRONMENT/AZURE_ENVIRONMENT), and instruct users to set both
values consistently for sovereign clouds to avoid using commercial blob suffixes
with sovereign auth; reference the YAML keys backend.azurerm.environment,
spec.cloud_environment, the !terraform.state function, and the exported
ARM_ENVIRONMENT/AZURE_ENVIRONMENT environment variables so readers know which
settings must align.
---
Nitpick comments:
In `@pkg/auth/providers/azure/oidc_test.go`:
- Around line 1349-1388: Refactor TestOIDCProvider_SovereignCloudEndpoints into
a table-driven test: create a slice of test cases each with name, tenantID,
cloudEnv (use azureCloud.GetCloudEnvironment with "usgovernment" and "china"),
tokenEndpoint input (empty or custom) and expected endpoint, then iterate and
run subtests calling the same oidcProvider{...}.getTokenEndpoint() and assert
the expected string; keep the three scenarios (US government, China, and custom
endpoint override) and reuse the existing oidcProvider field names to construct
each case.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 897e784a-648b-4d34-9459-7f07b424c664
📒 Files selected for processing (5)
docs/fixes/2026-03-13-azure-gcc-high-sovereign-cloud-support.mdpkg/auth/cloud/azure/setup_test.gopkg/auth/providers/azure/cli_test.gopkg/auth/providers/azure/oidc_test.gowebsite/docs/tutorials/azure-authentication.mdx
- Add AzureProfileEnvName to doc struct example - Clarify GetCloudEnvironment comment (validation rejects unknowns) - Refactor OIDC sovereign endpoint test to table-driven format - Tighten CLI Environment() negative assertions (both env vars, unknown case) - Add azure/cli note to supported cloud environments table - Add alignment guidance for spec.cloud_environment and backend.environment - Add TestUpdateAzureCLIFiles (5 cases: non-Azure, invalid token, valid, sovereign, SP) - Add TestCLIProvider_SetRealm and TestCLIProvider_Paths - UpdateAzureCLIFiles: 0% -> 80%, overall cloud/azure: 83% -> 87% Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/fixes/2026-03-13-azure-gcc-high-sovereign-cloud-support.md`:
- Around line 126-152: The registry example omits the AzureProfileEnvName field
for each environment entry (symbols "public", "usgovernment", "china") even
though the struct includes AzureProfileEnvName; update each environment object
to include an appropriate AzureProfileEnvName value (e.g. the environment's
canonical profile env var name) so the example matches the struct and is
internally consistent.
In `@pkg/auth/cloud/azure/cloud_environments.go`:
- Around line 89-99: The exported function ValidateCloudEnvironment is missing
the required performance instrumentation; add a deferred perf.Track call at the
top of the function: insert defer perf.Track(atmosConfig,
"pkg.ValidateCloudEnvironment")() as the first statement inside
ValidateCloudEnvironment (followed by a blank line), ensuring the perf package
and atmosConfig symbol are available in the scope and imported if necessary;
keep the existing logic unchanged.
In `@pkg/auth/cloud/azure/setup_test.go`:
- Around line 1497-1577: The tests in TestUpdateAzureCLIFiles write to the real
user home (~/.azure); update each subtest to create a temporary directory (via
t.TempDir()) and set HOME using t.Setenv("HOME", tmpDir) before calling
UpdateAzureCLIFiles so file creation is isolated and auto-cleaned; ensure
assertions (msalPath/profilePath checks) use os.UserHomeDir() after t.Setenv or
construct paths from the tmpDir to verify files, and replace any
os.Setenv/os.Unsetenv usage with testing.T.Setenv for automatic teardown.
In `@pkg/auth/providers/azure/oidc_test.go`:
- Around line 1456-1509: In TestOIDCProvider_Environment_SovereignCloud tighten
the "public" case by also asserting AZURE_ENVIRONMENT is not present: inside the
table-driven loop for the test name "public does not set ARM_ENVIRONMENT"
(TestOIDCProvider_Environment_SovereignCloud) add a check alongside the existing
ARM_ENVIRONMENT negative assertion to verify env does not contain
"AZURE_ENVIRONMENT" (use the same pattern that checks tt.unexpectedEnvVar — or
add a second unexpected check when cloudEnvName == "public") after calling
oidcProvider.Environment(); this ensures both ARM_ENVIRONMENT and
AZURE_ENVIRONMENT are absent for the public cloud case returned by
azureCloud.GetCloudEnvironment and evaluated by p.Environment().
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2fb9e716-11b4-4cbc-b62b-d6a4c72905e8
📒 Files selected for processing (6)
docs/fixes/2026-03-13-azure-gcc-high-sovereign-cloud-support.mdpkg/auth/cloud/azure/cloud_environments.gopkg/auth/cloud/azure/setup_test.gopkg/auth/providers/azure/cli_test.gopkg/auth/providers/azure/oidc_test.gowebsite/docs/tutorials/azure-authentication.mdx
- Add AzureProfileEnvName to doc registry entries for consistency - Isolate TestUpdateAzureCLIFiles from real ~/.azure using t.Setenv - Tighten OIDC Environment() negative assertions (both env vars) - Add cloud_environment test cases to TestNewCLIProvider (valid + invalid) - NewCLIProvider: 90.9% -> 100%, all testable CLI functions at 100% Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
what
cloud_environmentsetting to all Azure auth providers (azure/device-code,azure/oidc,azure/cli) for sovereign cloud supportCloudEnvironmentregistry (pkg/auth/cloud/azure/cloud_environments.go) with endpoint definitions for Azure Commercial, US Government (GCC High), and China (Mooncake)!terraform.stateYAML function for GCC High by reading the Terraform azurerm backendenvironmentfield to resolve the correct blob storage URL suffixwhy
!terraform.statefails because Atmos hardcodesblob.core.windows.netas the blob storage suffix, which doesn't resolve in sovereign clouds (should beblob.core.usgovcloudapi.net)login.microsoftonline.comand public cloud API scopes, preventing sovereign cloud users from authenticatingcloud_environmentdefaults topublic, so existing Azure Commercial users are unaffectedreferences
docs/fixes/2026-03-13-azure-gcc-high-sovereign-cloud-support.mdSummary by CodeRabbit
New Features
Documentation
Tests