Share Azure scope resolution between Aspire.Hosting.Azure and Aspire.Hosting.Azure.Kubernetes
- Dominant language
- C#
- Stars
- 6.3k
- Forks
- 991
- Avg merge
- 2d 15h
- Merged PRs (30d)
- 196
Description
## Background
`Aspire.Hosting.Azure.Kubernetes` currently maintains its own copy of Azure scope-resolution logic, discovered during review of #19219 ([review comment](https://github.com/microsoft/aspire/pull/19219#discussion_r3762737885)).
The AKS credential pipeline needs to know which subscription and resource group a cluster actually lives in, honoring `AsExistingInResourceGroup(...)` and an explicitly assigned `Scope`. That answer already exists in `Aspire.Hosting.Azure`, but is not reachable from the Kubernetes assembly, so `AzureKubernetesEnvironmentResource.AksPipeline.cs` reimplements it:
| Local copy | Mirrors |
|---|---|
| `GetExplicitScopeValues` | `BicepUtilities.GetExistingResourceScope` + `AzureBicepResourceScope.FromExistingResourceAnnotation` |
| `ResolveScopeValueAsync` | `BicepProvisioner.ResolveScopeValueAsync` |
This leaves two independent answers to "what scope did we actually deploy to?" in two assemblies. Drift between the provisioner's view of scope and the credential step's view is the same failure class that produced #19216, and a future change to the provisioner's precedence rules will not propagate to the Kubernetes copy.
## Why it wasn't fixed in #19219
The obvious remedy — granting `InternalsVisibleTo` to `Aspire.Hosting.Azure.Kubernetes` — currently fails to compile with roughly 19 `CS0436` errors.
`Aspire.Hosting.Azure` and `Aspire.Hosting.Azure.Kubernetes` both compile the same five source files:
- `ProcessResult.cs`
- `ProcessSpec.cs`
- `ProcessOutputCapture.cs`
- `ProcessUtil.cs`
- `PathLookupHelper.cs`
Granting IVT makes those types ambiguous between the two assemblies.
This is worth contrasting with the existing IVT grantees, `Aspire.Hosting.Azure.ContainerRegistry` and `Aspire.Hosting.Foundry`. Both link shared sources of their own, but neither overlaps with `Aspire.Hosting.Azure` on any compiled file, which is why their IVT is harmless. The Kubernetes project is the only one with genuine overlap.
Separately, `BicepProvisioner.ResolveScopeValueAsync` is `private static`, so IVT alone would not make it reusable; it would need extracting first.
## Proposed work
1. Stop double-compiling the five shared sources in `Aspire.Hosting.Azure.Kubernetes`, consuming the `Aspire.Hosting.Azure` copies instead.
2. Add `` to `Aspire.Hosting.Azure.csproj`.
3. Extract `ResolveScopeValueAsync` out of `BicepProvisioner` so it can be shared.
4. Replace `GetExplicitScopeValues` / `ResolveScopeValueAsync` in `AzureKubernetesEnvironmentResource.AksPipeline.cs` with calls to the shared helpers.
5. Revert `AzureBicepResourceScope.HasResourceGroup` from `public` back to `internal` if the shared helpers make it unnecessary. Note it is defensible as public regardless, since `ResourceGroup` is a public property that throws with no public guard.
## Related: converge the empty-string semantics
The two implementations currently diverge deliberately. The Kubernetes copy rejects an empty scope value; `BicepProvisioner.ResolveScopeValueAsync` rejects only null.
Empty is worth rejecting in both. Nothing upstream guards against it (`AsExistingInResourceGroup` and the `AzureBicepResourceScope` constructors only do `ArgumentNullException.ThrowIfNull`), so an empty value flows through `string.IsNullOrEmpty` checks downstream and is silently treated as unpinned, falling back to the global deployment scope. That is the same silent wrong-scope failure the null check exists to prevent.
Suggest converging on the stricter behavior by tightening the provisioner, rather than relaxing the Kubernetes copy.
## Acceptance criteria
- [ ] Only one implementation of Azure scope resolution remains.
- [ ] The AKS credential pipeline and the Bicep provisioner cannot disagree about a resource's effective scope.
- [ ] Empty scope values are rejected consistently.
- [ ] Existing AKS scope coverage in `AzureKubernetesInfrastructureTests` continues to pass.
Contributor guide
Research direction
Start with AzureKubernetesEnvironmentResource.AksPipeline.cs, BicepProvisioner, and the two project files; compare the duplicated scope helpers and the five shared source inclusions. Run the existing AzureKubernetesInfrastructureTests while tracing scope precedence and empty values. Done means one shared implementation remains, both callers agree, empty scopes are rejected consistently, and the coverage still passes.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- azure, csharp, kubernetes
- Domain
- cloud, devops
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 55/100