microsoft / microsoft/aspire

Should IAzureComputeEnvironmentResource.ContainerRegistry be obsoleted in favor of ContainerRegistryReferenceAnnotation?

Open
#18,551 2 comments 0 reactions 0 assignees View on GitHub
area-integrations triage:bot-seen
Dominant language
C#
Stars
6.3k
Forks
991
Avg merge
2d 12h
Merged PRs (30d)
201

Description

## Question

Should `Aspire.Hosting.Azure.IAzureComputeEnvironmentResource.ContainerRegistry` be marked `[Obsolete]` (or removed from the interface and replaced with an annotation-based public helper)?

Today the interface declares a default-implementation property:

```csharp
public interface IAzureComputeEnvironmentResource : IComputeEnvironmentResource, IResource
{
IAzureContainerRegistryResource? ContainerRegistry => null;
}
```

In practice, the runtime resolution path that every compute environment uses to find its container registry is the annotation-based one — `TryGetLastAnnotation` on the environment resource. The same pattern appears in:

- `AzureContainerAppEnvironmentResource.GetContainerRegistry`
- `AzureAppServiceEnvironmentExtensions`
- `AzureContainerAppExtensions`
- `KubernetesEnvironmentResource`
- `DockerComposeEnvironmentResource`
- `Aspire.Hosting.Foundry/Project/ProjectResource`

And `AzureContainerRegistryExtensions.AddAzureContainerRegistry` stamps a `RegistryTargetAnnotation` on every resource via `OnBeforeStart`. So the annotation is the canonical source of truth at runtime.

The only public consumer that reads the interface property directly is `AzureContainerRegistryExtensions.GetAzureContainerRegistry()`:

```csharp
public static IResourceBuilder GetAzureContainerRegistry(
this IResourceBuilder builder)
where T : IResource, IAzureComputeEnvironmentResource
{
var containerRegistry = builder.Resource.ContainerRegistry
?? throw new InvalidOperationException(...);
...
}
```

That call works for `AzureContainerAppEnvironmentResource` (which overrides `ContainerRegistry`) but throws for `AzureKubernetesEnvironmentResource`, because AKS doesn't override the interface property — it only sets the annotation. The AKS deploy path itself doesn't care, because it goes through the annotation; but `GetAzureContainerRegistry()` and any downstream consumer (e.g. [devdiv-microsoft/aspire-1p#238](https://github.com/devdiv-microsoft/aspire-1p/issues/238)) that reads the property directly is left in a different state from the annotation.

That parallelism — a public property and an annotation that are supposed to mean the same thing, but only sometimes agree — is what's prompting the question.

## Options

A few directions worth discussing:

1. **Annotation-first, keep the property.** Update `GetAzureContainerRegistry()` to read `ContainerRegistryReferenceAnnotation` first, then fall back to `IAzureComputeEnvironmentResource.ContainerRegistry`. Leave the interface property as-is; document that the annotation is canonical.

2. **Mark the interface property `[Obsolete]`.** Same behavior change to `GetAzureContainerRegistry()` as in (1), plus a soft-deprecation on `IAzureComputeEnvironmentResource.ContainerRegistry` pointing folks at the extension/annotation. The default-interface implementation can keep returning `null` indefinitely.

3. **Remove the property from the interface in a future major version.** After (2), once consumers have migrated, drop the property entirely and have `GetAzureContainerRegistry()` be the only public way to ask "what registry does this compute environment publish to?".

4. **Status quo + override on AKS.** Override `ContainerRegistry` on `AzureKubernetesEnvironmentResource` to return `DefaultContainerRegistry ?? `, so the interface property and the annotation always agree. This keeps the interface property as the canonical answer but doesn't address the broader "two sources of truth" smell.

I don't have a strong opinion on which is right; mainly raising this so the team can decide whether the property should stay first-class or be obsoleted in favor of the annotation-based pattern that the runtime already uses everywhere.

## Repro / related

[devdiv-microsoft/aspire-1p#238](https://github.com/devdiv-microsoft/aspire-1p/issues/238) hits the property-vs-annotation mismatch on AKS today.

Contributor guide

Open the contributing guide

Research direction

Start with AzureContainerRegistryExtensions.GetAzureContainerRegistry(), IAzureComputeEnvironmentResource.ContainerRegistry, and the ContainerRegistryReferenceAnnotation lookup. Compare the behavior across AzureContainerAppEnvironmentResource, AzureKubernetesEnvironmentResource, and the other listed environment resources, including the linked AKS mismatch. Done means the team has selected and implemented one consistent public API direction with aligned documentation and tests.

Written by the indexing model from the issue text.

Assessment

Tech stack
azure, csharp
Domain
backend-api-design, cloud
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Needs clarification
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.