Repository navigation
feat(auth): add Azure AKS/ACR integrations mirroring EKS/ECR - #2790
Merged
Merged
Conversation
Extends the `auth.integrations` system to Azure: `atmos azure aks token`, `atmos azure aks update-kubeconfig`, and `atmos azure acr login`, so `atmos auth login` can provision kubectl and Docker credentials for Azure the same way it already does for AWS EKS/ECR — no `az` CLI or `kubelogin` binary required. Generalizes `pkg/auth/cloud/kube.KubeconfigManager` from AWS-specific to a cloud-agnostic writer shared by both clouds, and widens the existing `Cluster`/`Registry` schema structs (renamed from `EKSCluster`/`ECRRegistry`) so `spec.cluster`/`spec.registry` are reused verbatim across `aws/eks`+`azure/aks` and `aws/ecr`+`azure/acr`. Since Azure AAD tokens are scope-bound at issuance (unlike AWS SigV4), all three Azure identity providers now acquire an AKS-scoped token at login time alongside their existing Graph/KeyVault tokens. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Bumps four vulnerable dependencies to their patched versions, all within the semver-major bump policy allowed by .github/dependabot.yml: - google.golang.org/grpc v1.81.1 -> v1.82.1 (GHSA-hrxh-6v49-42gf): xDS RBAC authorization fail-open, HTTP/2 Rapid Reset mitigation bypass, and an RBAC-engine panic. - fast-uri (website, transitive) -> 3.1.4 (GHSA-v2hh-gcrm-f6hx): host confusion via literal backslash authority delimiter. - svgo (website, transitive) -> 3.3.4 (GHSA-2p49-hgcm-8545): removeScripts plugin left some executable scripts intact. - dompurify (website, transitive) -> 3.4.12 (GHSA-c2j3-45gr-mqc4): CUSTOM_ELEMENT_HANDLING bypassed afterSanitizeElements for allowed custom elements. Regenerated NOTICE via scripts/generate-notice.sh. No CodeQL alerts were open at remediation time. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Erik Osterman (Cloud Posse) (osterman)
requested a review
from a team
as a code owner
July 23, 2026 18:30
Contributor
|
Tip Atmos Pro
No affected stacks workflow was detected for this pull request. 
|
Erik Osterman (Cloud Posse) (osterman)
added
the
minor
Dependency ReviewThe following issues were found:
License Issuesgo.mod
Scanned Files
|
Condenses the ECR/ACR and EKS/AKS integration sections into one combined section so the file stays under agent-skills' 500-line limit. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds native Azure authentication for AKS and ACR. The change introduces shared AWS/Azure schemas, Azure providers and integrations, CLI commands, AKS token and kubeconfig flows, ACR Docker authentication, registration wiring, tests, documentation, and the Azure SDK dependency. ChangesAzure authentication and CLI flows
Estimated code review effort: 5 (Critical) | ~120 minutes Mergeability Score: ⚪ Minimal · up to The PR adds Azure AKS/ACR authentication integrations and related dependency updates; no actionable merge-blocking risk remains beyond normal checks and review. Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Resource Changes Found for
|
![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: 6
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/kube/config.go (2)
232-251: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winCorrect the generalized ID contract.
ListClusterIDsnow returns AWS ARNs or Azure ARM resource IDs, but its exported comment and local variable still call every value an ARN.Suggested fix
-// ListClusterIDs returns all cluster ARN keys from the kubeconfig file. +// ListClusterIDs returns all cluster ID keys from the kubeconfig file. ... - arns := make([]string, 0, len(existing.Clusters)) + ids := make([]string, 0, len(existing.Clusters)) for k := range existing.Clusters { - arns = append(arns, k) + ids = append(ids, k) } - return arns, nil + return ids, nil🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/auth/cloud/kube/config.go` around lines 232 - 251, Update the exported comment for KubeconfigManager.ListClusterIDs and rename the local arns variable to a provider-neutral name, such as clusterIDs, so the method clearly represents both AWS ARNs and Azure ARM resource IDs without changing behavior.
266-298: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftPrevent AKS auth-info collisions across subscriptions.
Azure passes the resource group as
Region, but resource groups are unique only within a subscription. Two AKS clusters with the same name and resource group in different subscriptions therefore get separate cluster/context entries but the sameAuthInfokey; merging the second can overwrite the exec configuration used by the first."error"mode also misses this collision.
pkg/auth/cloud/kube/config.go#L266-L298: carry a caller-provided unique auth-info name (or a subscription/resource-ID suffix), preserve the AWS legacy name, and reject an existingAuthInfocollision in"error"mode.docs/prd/azure-aks-acr-integrations.md#L165-L169: remove the claim that resource group alone provides username uniqueness and document the subscription dimension.Add a regression test for two same-name/same-resource-group AKS clusters in different subscriptions.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/auth/cloud/kube/config.go` around lines 266 - 298, The kubeconfig construction around userName and config.AuthInfos must use a caller-provided unique auth-info name or subscription/resource-ID suffix for Azure, while preserving the existing AWS legacy naming; in “error” mode, reject an already-existing AuthInfo collision instead of overwriting it. Update docs/prd/azure-aks-acr-integrations.md lines 165-169 to remove resource-group-only uniqueness and document the subscription dimension. Add a regression test covering same-name, same-resource-group AKS clusters from different subscriptions.
🧹 Nitpick comments (2)
cmd/azure/aks/token.go (1)
239-245: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
flags.NewStandardParser()for token command flags.
tokenCmd's flags are registered directly on thepflag.FlagSetinstead of going throughflags.NewStandardParser(), unlike the siblingupdate_kubeconfig.goin the same package. As per coding guidelines, "CLI commands must useflags.NewStandardParser()for command-specific flags and must not callviper.BindEnv()orviper.BindPFlag()directly."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/azure/aks/token.go` around lines 239 - 245, Update init and the tokenCmd flag registration to use flags.NewStandardParser() for the command-specific flags, matching the sibling update_kubeconfig implementation. Register cluster-name, resource-group, subscription-id, and identity through the standard parser rather than directly on tokenCmd.Flags(), while preserving their names, defaults, descriptions, and shorthand.Source: Coding guidelines
pkg/auth/providers/azure/cli.go (1)
154-165: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winSequential second
azcall doubles CLI-provider auth latency for everyone.The AKS-scoped token fetch spawns a second
azprocess serially after the primary token fetch, on everyAuthenticate()call — even for identities that never touch AKS. Since it's best-effort and independent of the primary token, consider running it concurrently (goroutine/errgroup) so non-AKS users don't pay the extraazprocess latency.⚡ Sketch: run the AKS-scoped fetch concurrently
- // Acquire an AKS-scoped token, for `atmos azure aks token` (best-effort, - // non-fatal — az CLI-backed identities without AKS access simply won't - // have an AKSToken populated). - if aksResp, err := p.executeAzCommand(ctx, azureCloud.AKSServerAppID); err != nil { - log.Debug("Failed to acquire AKS token via az CLI, atmos azure aks token may not work", "error", err) - } else if aksExpiresOn, err := parseAzureCLITime(aksResp.ExpiresOn); err != nil { - log.Debug("Failed to parse AKS token expiration via az CLI, atmos azure aks token may not work", "error", err) - } else { - creds.AKSToken = aksResp.AccessToken - creds.AKSTokenExpiration = aksExpiresOn.Format(time.RFC3339) - log.Debug("Acquired AKS token via az CLI", "expiresOn", creds.AKSTokenExpiration) - } + // Acquire an AKS-scoped token concurrently with the rest of Authenticate's + // bookkeeping, for `atmos azure aks token` (best-effort, non-fatal). + aksDone := make(chan struct{}) + go func() { + defer close(aksDone) + if aksResp, err := p.executeAzCommand(ctx, azureCloud.AKSServerAppID); err != nil { + log.Debug("Failed to acquire AKS token via az CLI, atmos azure aks token may not work", "error", err) + } else if aksExpiresOn, err := parseAzureCLITime(aksResp.ExpiresOn); err != nil { + log.Debug("Failed to parse AKS token expiration via az CLI, atmos azure aks token may not work", "error", err) + } else { + creds.AKSToken = aksResp.AccessToken + creds.AKSTokenExpiration = aksExpiresOn.Format(time.RFC3339) + log.Debug("Acquired AKS token via az CLI", "expiresOn", creds.AKSTokenExpiration) + } + }() + <-aksDone // wait here, or move the wait past unrelated bookkeeping to overlap work🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/auth/providers/azure/cli.go` around lines 154 - 165, Update the Authenticate flow around the primary token fetch and AKS token block to run the AKS-scoped executeAzCommand call concurrently rather than serially after the primary request. Preserve its best-effort behavior, existing parseAzureCLITime handling, credential population, and debug logs, and ensure Authenticate waits for the concurrent work before returning any resulting credentials.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmd/azure/acr/login.go`:
- Around line 208-223: Replace the direct identity and registry flag
registrations in init with the command-local standard parser via
flags.NewStandardParser. Preserve the existing flag names, shorthands, defaults,
descriptions, and optional --identity behavior using the parser’s NoOptDefVal
support, then attach the parser to loginCmd before adding it to AcrCmd.
- Around line 77-91: Update the explicit-registry branch in the login command
before calling executeExplicitRegistries to reject any supplied identityName or
integrationName, preserving the mutual-exclusivity error behavior used by
executeWithAuthManager. Only execute explicit registry login when neither
auth-manager argument is provided.
In `@pkg/auth/cloud/azure/aks.go`:
- Around line 92-117: Update the AKS authentication flow so the ServerID parsed
by DescribeCluster and stored in AKSClusterInfo is propagated by
BuildKubeClusterInfo to GetToken. Have GetToken request or refresh the token
using info.ServerID/.default instead of the credential-scoped AKSServerScope,
ensuring kubectl receives a cluster-scoped audience for custom or legacy AKS
server app IDs.
In `@pkg/auth/integrations/azure/aks.go`:
- Around line 278-296: findClusterID currently matches AKS resource IDs by
cluster name alone; include a.cluster.ResourceGroup in the full Azure
managed-cluster suffix so Cleanup selects the correct kubeconfig entry. In
pkg/auth/integrations/azure/aks.go lines 278-296, update findClusterID’s
matching suffix; in pkg/auth/integrations/azure/aks_test.go lines 476-504, add
coverage with same-named clusters in different resource groups and assert the
configured resource group’s ID is returned.
In `@website/docs/cli/commands/azure/acr-login.mdx`:
- Around line 9-13: Add the required static CastPlayer terminal demonstration
after the Intro in each affected Azure documentation page:
website/docs/cli/commands/azure/acr-login.mdx (ACR login cast),
website/docs/cli/commands/azure/aks/aks.mdx (AKS command-group cast),
website/docs/cli/commands/azure/aks/update-kubeconfig.mdx (kubeconfig workflow
cast), website/docs/cli/commands/azure/azure-aks-token.mdx (exec-credential
token cast), and website/docs/cli/commands/azure/usage.mdx (Azure command-group
cast).
In `@website/docs/cli/commands/azure/aks/update-kubeconfig.mdx`:
- Around line 14-16: Add a “## Usage” heading immediately before the command
synopsis in the AKS update-kubeconfig documentation, preserving the existing
command and surrounding structure.
---
Outside diff comments:
In `@pkg/auth/cloud/kube/config.go`:
- Around line 232-251: Update the exported comment for
KubeconfigManager.ListClusterIDs and rename the local arns variable to a
provider-neutral name, such as clusterIDs, so the method clearly represents both
AWS ARNs and Azure ARM resource IDs without changing behavior.
- Around line 266-298: The kubeconfig construction around userName and
config.AuthInfos must use a caller-provided unique auth-info name or
subscription/resource-ID suffix for Azure, while preserving the existing AWS
legacy naming; in “error” mode, reject an already-existing AuthInfo collision
instead of overwriting it. Update docs/prd/azure-aks-acr-integrations.md lines
165-169 to remove resource-group-only uniqueness and document the subscription
dimension. Add a regression test covering same-name, same-resource-group AKS
clusters from different subscriptions.
---
Nitpick comments:
In `@cmd/azure/aks/token.go`:
- Around line 239-245: Update init and the tokenCmd flag registration to use
flags.NewStandardParser() for the command-specific flags, matching the sibling
update_kubeconfig implementation. Register cluster-name, resource-group,
subscription-id, and identity through the standard parser rather than directly
on tokenCmd.Flags(), while preserving their names, defaults, descriptions, and
shorthand.
In `@pkg/auth/providers/azure/cli.go`:
- Around line 154-165: Update the Authenticate flow around the primary token
fetch and AKS token block to run the AKS-scoped executeAzCommand call
concurrently rather than serially after the primary request. Preserve its
best-effort behavior, existing parseAzureCLITime handling, credential
population, and debug logs, and ensure Authenticate waits for the concurrent
work before returning any resulting credentials.
🪄 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 Plus
Run ID: f07a89cc-33a3-4c4b-98be-790e315dc071
⛔ Files ignored due to path filters (2)
go.sumis excluded by!**/*.sumwebsite/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (66)
.claude/skills/atmos-azure-acr.claude/skills/atmos-azure-aksNOTICEagent-skills/skills/atmos-auth/SKILL.mdagent-skills/skills/atmos-azure-acr/SKILL.mdagent-skills/skills/atmos-azure-aks/SKILL.mdcmd/aws/eks/update_kubeconfig_sdk.gocmd/azure/acr/acr.gocmd/azure/acr/login.gocmd/azure/acr/login_test.gocmd/azure/aks/aks.gocmd/azure/aks/token.gocmd/azure/aks/token_test.gocmd/azure/aks/update_kubeconfig.gocmd/azure/aks/update_kubeconfig_sdk.gocmd/azure/aks/update_kubeconfig_test.gocmd/azure/azure.gocmd/root.godocs/prd/azure-aks-acr-integrations.mderrors/errors.gogo.modpkg/auth/cloud/aws/eks.gopkg/auth/cloud/aws/eks_test.gopkg/auth/cloud/azure/acr.gopkg/auth/cloud/azure/acr_test.gopkg/auth/cloud/azure/aks.gopkg/auth/cloud/azure/aks_test.gopkg/auth/cloud/azure/config.gopkg/auth/cloud/azure/config_test.gopkg/auth/cloud/azure/constants.gopkg/auth/cloud/azure/mock_aks_client_test.gopkg/auth/cloud/kube/config.gopkg/auth/cloud/kube/config_diff_test.gopkg/auth/cloud/kube/config_test.gopkg/auth/cloud/kube/multi_cluster_test.gopkg/auth/identities/azure/subscription.gopkg/auth/identities/azure/subscription_test.gopkg/auth/integrations/aws/ecr.gopkg/auth/integrations/aws/ecr_public_test.gopkg/auth/integrations/aws/ecr_test.gopkg/auth/integrations/aws/eks.gopkg/auth/integrations/aws/eks_test.gopkg/auth/integrations/azure/acr.gopkg/auth/integrations/azure/acr_test.gopkg/auth/integrations/azure/aks.gopkg/auth/integrations/azure/aks_test.gopkg/auth/integrations/registry_test.gopkg/auth/integrations/types.gopkg/auth/manager_integrations_test.gopkg/auth/providers/azure/cli.gopkg/auth/providers/azure/device_code.gopkg/auth/providers/azure/oidc.gopkg/auth/providers/azure/oidc_test.gopkg/auth/types/azure_credentials.gopkg/datafetcher/schema/atmos/config/1.0.jsonpkg/schema/schema_auth.gowebsite/blog/2026-07-23-azure-aks-acr-authentication.mdxwebsite/docs/cli/commands/azure/_category_.jsonwebsite/docs/cli/commands/azure/acr-login.mdxwebsite/docs/cli/commands/azure/aks/_category_.jsonwebsite/docs/cli/commands/azure/aks/aks.mdxwebsite/docs/cli/commands/azure/aks/update-kubeconfig.mdxwebsite/docs/cli/commands/azure/azure-aks-token.mdxwebsite/docs/cli/commands/azure/usage.mdxwebsite/package.jsonwebsite/src/data/roadmap.js
Numbered-list continuation lines used 3-space indentation instead of a multiple of 2, and the architecture diagram had a matching off-by-one; both tripped the repo's editorconfig CI check.
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/prd/azure-aks-acr-integrations.md (1)
88-101: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winNarrow the token-isolation claim for non-default server applications.
The architecture and security sections imply every AKS token is scoped to the cluster’s server application, but the future-enhancement section says non-default
--server-idsupport is not implemented and currently only warns. Document those clusters as unsupported/fail-closed, or limit the guarantee to the well-known default server application.Also applies to: 335-337, 353-356
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/prd/azure-aks-acr-integrations.md` around lines 88 - 101, Update the AKS token-isolation claims in the architecture, security, and referenced future-enhancement sections to apply only to the well-known default server application, or explicitly state that non-default --server-id clusters are unsupported and fail closed. Align the kubectl token flow and all related guarantees with the existing behavior that only warns for unsupported non-default server IDs.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@docs/prd/azure-aks-acr-integrations.md`:
- Around line 88-101: Update the AKS token-isolation claims in the architecture,
security, and referenced future-enhancement sections to apply only to the
well-known default server application, or explicitly state that non-default
--server-id clusters are unsupported and fail closed. Align the kubectl token
flow and all related guarantees with the existing behavior that only warns for
unsupported non-default server IDs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: e2381aa6-ea96-433e-973c-71f8d8e3ee61
📒 Files selected for processing (1)
docs/prd/azure-aks-acr-integrations.md
The new atmos azure command group added a row to the top-level help output that the CLI acceptance-test snapshots didn't account for.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2790 +/- ##
==========================================
+ Coverage 82.98% 82.99% +0.01%
==========================================
Files 1881 1891 +10
Lines 183067 183860 +793
==========================================
+ Hits 151925 152602 +677
- Misses 23317 23401 +84
- Partials 7825 7857 +32
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/auth/integrations/azure/aks_test.go`:
- Around line 514-521: Update the cluster fixture loop in WriteClusterConfig so
the nonmatching alias uses Region "other-rg" alongside wrongID, while the target
alias retains "target-rg" with wantID. Keep the fixture metadata internally
consistent and preserve the existing disambiguation assertions.
In `@pkg/auth/providers/azure/cli_test.go`:
- Around line 19-32: Replace the shell-based az fixture in
TestCLIProvider_Authenticate_UsesClusterServerID with a platform-independent
Go-native fake, using the existing helper-process fixture or package-level
command hook if available. Preserve the current argument matching and JSON
responses for the custom server ID and management-token paths, while removing
the executable script, shell syntax, and PATH manipulation.
🪄 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 Plus
Run ID: 1d415aa5-a7ed-4438-add6-e2bb22fdfe27
📒 Files selected for processing (23)
cmd/azure/acr/login.gocmd/azure/acr/login_test.gocmd/azure/aks/token.gocmd/azure/aks/token_test.godemo/casts/atmos.d/screengrabs/cli.yamlpkg/auth/cloud/azure/aks.gopkg/auth/cloud/azure/aks_test.gopkg/auth/integrations/azure/aks.gopkg/auth/integrations/azure/aks_test.gopkg/auth/providers/azure/cli.gopkg/auth/providers/azure/cli_test.gopkg/auth/providers/azure/device_code.gopkg/auth/providers/azure/oidc.gowebsite/docs/cli/commands/azure/acr-login.mdxwebsite/docs/cli/commands/azure/aks/aks.mdxwebsite/docs/cli/commands/azure/aks/update-kubeconfig.mdxwebsite/docs/cli/commands/azure/azure-aks-token.mdxwebsite/docs/cli/commands/azure/usage.mdxwebsite/static/casts/screengrabs/atmos-azure--help.castwebsite/static/casts/screengrabs/atmos-azure-acr-login--help.castwebsite/static/casts/screengrabs/atmos-azure-aks--help.castwebsite/static/casts/screengrabs/atmos-azure-aks-token--help.castwebsite/static/casts/screengrabs/atmos-azure-aks-update-kubeconfig--help.cast
🚧 Files skipped from review as they are similar to previous changes (12)
- website/docs/cli/commands/azure/usage.mdx
- website/docs/cli/commands/azure/aks/aks.mdx
- website/docs/cli/commands/azure/aks/update-kubeconfig.mdx
- pkg/auth/providers/azure/oidc.go
- pkg/auth/providers/azure/cli.go
- pkg/auth/cloud/azure/aks_test.go
- pkg/auth/integrations/azure/aks.go
- website/docs/cli/commands/azure/azure-aks-token.mdx
- cmd/azure/aks/token_test.go
- cmd/azure/acr/login.go
- website/docs/cli/commands/azure/acr-login.mdx
- cmd/azure/aks/token.go
Erik Osterman (Cloud Posse) (osterman)
temporarily deployed
to
screengrabs
July 24, 2026 01:17 — with 
GitHub Actions
Inactive
GitHub Actions
Inactive
…kills/atmos-auth'
Andriy Knysh (aknysh)
temporarily deployed
to
screengrabs
August 11, 2026 22:49 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
cmd/azure/aks/update_kubeconfig_sdk_test.go (1)
106-129: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the wrapped error cause.
The table accepts any error for configuration, manager, and authentication failures. Set an optional expected cause such as
errBoom, then userequire.ErrorIs. This prevents unrelated errors or removed wrapping from passing the test.As per coding guidelines, “Prefer behavior-focused, table-driven unit tests with mocks; avoid tautological, stub, always-skipped, or coverage-only tests and target at least 85% coverage.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/azure/aks/update_kubeconfig_sdk_test.go` around lines 106 - 129, Update the table-driven tests around executeAKSUpdateKubeconfigDirect to include an expected error cause for configuration, manager, and authentication failures, such as errBoom. Replace the broad require.Error assertion with require.ErrorIs using that expected cause, while retaining the existing nil-credentials coverage and ensuring each failure case verifies the wrapped underlying error.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@agent-skills/skills/atmos-auth/references/azure-acr-integration.md`:
- Around line 73-74: Update the ACR access-model statement to note that
registries are private by default but Standard and Premium registries may enable
anonymous pulls. Clarify that when ABAC repository permissions are enabled,
AcrPull and AcrPush do not apply, and document the applicable repository-scoped
roles instead.
In `@agent-skills/skills/atmos-auth/references/azure-aks-integration.md`:
- Around line 73-74: Update the kubeconfig documentation in the referenced
integration guide to specify Linux/macOS and Windows default paths, state that
ATMOS_XDG_CONFIG_HOME overrides XDG_CONFIG_HOME, and document the AKS override
precedence/options for --kubeconfig, ATMOS_KUBECONFIG, and KUBECONFIG.
In `@cmd/azure/aks/update_kubeconfig_sdk_test.go`:
- Around line 97-130: Update executeAKSUpdateKubeconfigDirect to treat a nil
whoami result the same as nil credentials before dereferencing
whoami.Credentials. Extend TestExecuteAKSUpdateKubeconfigDirect_Errors with a
"nil whoami" case that supplies a nil authentication result and expects an
error.
---
Nitpick comments:
In `@cmd/azure/aks/update_kubeconfig_sdk_test.go`:
- Around line 106-129: Update the table-driven tests around
executeAKSUpdateKubeconfigDirect to include an expected error cause for
configuration, manager, and authentication failures, such as errBoom. Replace
the broad require.Error assertion with require.ErrorIs using that expected
cause, while retaining the existing nil-credentials coverage and ensuring each
failure case verifies the wrapped underlying error.
🪄 Autofix
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 Plus
Run ID: f8232e9c-e146-4197-9c04-3f48055ddc7c
📒 Files selected for processing (9)
agent-skills/skills/atmos-auth/SKILL.mdagent-skills/skills/atmos-auth/references/azure-acr-integration.mdagent-skills/skills/atmos-auth/references/azure-aks-integration.mdcmd/azure/acr/login_more_test.gocmd/azure/aks/update_kubeconfig_dispatch_test.gocmd/azure/aks/update_kubeconfig_sdk.gocmd/azure/aks/update_kubeconfig_sdk_test.gocmd/azure/azure_test.gopkg/auth/cloud/azure/coverage_extra_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- cmd/azure/aks/update_kubeconfig_sdk.go
Andriy Knysh (aknysh)
temporarily deployed
to
screengrabs
August 11, 2026 23:25 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
Andriy Knysh (aknysh)
approved these changes
Aug 12, 2026
Andriy Knysh (aknysh)
enabled auto-merge
August 12, 2026 00:58
Andriy Knysh (aknysh)
temporarily deployed
to
screengrabs
August 12, 2026 23:59 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
Erik Osterman (Cloud Posse) (osterman)
temporarily deployed
to
screengrabs
August 12, 2026 23:59 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
Andriy Knysh (aknysh)
disabled auto-merge
August 13, 2026 00:26
Andriy Knysh (aknysh)
temporarily deployed
to
screengrabs
August 13, 2026 04:36 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
Andriy Knysh (aknysh)
force-pushed
the
osterman/aks-acr-support
branch
from
August 13, 2026 17:38
acdff40 to
21cadd2
Compare
Andriy Knysh (aknysh)
temporarily deployed
to
screengrabs
August 13, 2026 17:38 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
Contributor
|
Tip Atmos Pro
No affected stacks workflow was detected for this pull request.
|
Contributor
|
Tip Atmos Pro
No affected stacks workflow was detected for this pull request.
|
|
These changes were released in v1.226.0-rc.7. |
This branch was successfully deployed
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
2 participants
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
atmos azure aks token,atmos azure aks update-kubeconfig, andatmos azure acr login, mirroring the existingatmos aws eks/atmos aws ecrintegrations.pkg/auth/cloud/kube.KubeconfigManagerfrom AWS-specific to a cloud-agnostic writer shared by EKS and AKS, with a regression suite locking in byte-identical AWS output.IntegrationSpec.Cluster/.Registryschema structs (renamed fromEKSCluster/ECRRegistrytoCluster/Registry) sospec.cluster/spec.registryare reused verbatim acrossaws/eks+azure/aksandaws/ecr+azure/acr— no new per-cloud config keys.website/docs/cli/commands/azure/), a changelog post, a roadmap update, two new agent skills (atmos-azure-aks,atmos-azure-acr), and a PRD documenting the design (docs/prd/azure-aks-acr-integrations.md).google.golang.org/grpc(xDS RBAC auth bypass / HTTP2 rapid-reset bypass), and three transitive website npm packages (fast-uri,svgo,dompurify).why
kubectland Docker credentials in one step viaatmos auth login. Azure had the same auth foundation (providers, identities) but no equivalent for AKS/ACR, so Azure users still needed theazCLI — and for AAD-enabled clusters, the separatekubeloginbinary — outside of Atmos entirely.atmos azure aks tokeninstead ofkubelogin; ACR login is a plain OAuth2 token exchange, matching whataz acr logindoes under the hood.references
docs/prd/azure-aks-acr-integrations.mddocs/prd/eks-kubeconfig.md), ECR authentication PRD (docs/prd/ecr-authentication.md)manual testing
Exercised end-to-end against a live AAD-enabled AKS cluster (Azure CNI Overlay + Cilium, AAD + Azure RBAC, local accounts disabled) — the live path that PRD Success Metric #2 had previously left to unit tests only. This surfaced, and fixed, a registration gap.
Bug found + fixed.
atmos azure aks update-kubeconfig --integration <name>failed withunknown integration kind: azure/aks. Thepkg/auth/integrations/azurepackage self-registersazure/aksandazure/acrin itsinit(), but nothing blank-imported that package inpkg/auth/manager.go(unlike theawsandgithubintegration packages), soinit()never ran and the kinds never registered. The unit suites import theazurepackage directly, which registered the kinds incidentally and masked the missing production import. Fixed by adding the blank import alongsideaws/github.Integration mode — describe the cluster and write kubeconfig via the Go SDK (no
az, nokubelogin):The kubeconfig Atmos wrote drives its exec plugin through
atmos azure aks token(notkubelogin), with--server-iddiscovered from the cluster (here the well-known AKS AAD server app):auth execmode — Atmos injectsKUBECONFIGinto the child process from the integration'sEnvironment()(works even withauto_provision: false, which only suppresses the auto-write on login, not the env composition), so no manualexportis needed:Both paths mint bearer tokens through
atmos azure aks tokenagainst the Atmos-managed identity — noazCLI and nokubeloginbinary. (ACR login against a live registry remains unit-test-only.)