Azure / Azure/apiops

[FEATURE REQ] Store subscription ownerId relative to the service, the way scope already is

Open
#877 2 comments 0 reactions 0 assignees View on GitHub
Dominant language
C#
Stars
448
Forks
247
PR merge metrics
No merged PRs in 30d

Description

v7 relativizes cross-resource references so an artifact can be published to any instance, which is a great improvement. Subscription `scope` is covered, but `ownerId` is not, so subscription artifacts remain pinned to the instance they were extracted from.

### What the extractor writes today

Extracting a subscription produces:

```json
{
"properties": {
"displayName": "Unlimited",
"scope": "/products/unlimited",
"ownerId": "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/rg-dev/providers/Microsoft.ApiManagement/service/apim-dev/users/1",
"state": "active"
}
}
```

`scope` is relative and publishes anywhere. `ownerId` still carries the source instance's subscription id, resource group and service name, so publishing this artifact to another instance requires a configuration override for every subscription:

```yaml
subscriptions:
- name: unlimited
properties:
ownerId: "/subscriptions/{#SUB#}/resourceGroups/{#RG#}/providers/Microsoft.ApiManagement/service/{#SVC#}/users/1"
```

That override is pure boilerplate. It says nothing except "the same user id, on whichever instance this is", which is exactly what a relative id already expresses.

### The relative form is the documented contract

This is not a request to invent a new format. Microsoft documents `ownerId` as relative:

> **ownerId** — The user resource identifier of the subscription owner. The value is a valid relative URL in the format of `/users/{userId}` where `{userId}` is a user identifier.

[SubscriptionContract.ownerId](https://learn.microsoft.com/javascript/api/@azure/arm-apimanagement/subscriptioncontract)

APIM returns the expanded absolute form on GET, the same as it does for `scope`, but the documented canonical format is relative. So the extractor is persisting a shape that is less portable than the contract allows.

### Why the existing mechanism cannot simply be extended

The obvious change is to add `ownerId` to `OptionalReferencedResourceDtoProperties` in `SubscriptionResource`, next to the two `Scope` entries. That does not work, for two reasons:

1. That dictionary is keyed by `IResource`, and there is no `UserResource`. Introducing one would mean APIOps starts modelling users as an extractable and publishable resource type, which is a much larger change than this problem warrants.

2. The same dictionaries feed the publisher's relationship graph in `Relationships.cs` (`getReferences`). A `UserResource` key would create a subscription-to-user dependency edge, so the publisher would expect the user in source and `STRICT_VALIDATION=true` would report a missing predecessor. Users are typically not managed as artifacts, so that would be a false failure.

The two concerns are currently coupled: "this property holds an absolute id that should be stored relative" and "this property references a resource we manage". `ownerId` is the first case without the second.

### Proposed change

Separate the two by adding a relativization-only declaration.

**`common/Resource.cs`**, on `IResourceWithReference`:

```csharp
///
/// DTO properties holding an absolute resource id that should be persisted relative to the
/// service, but whose target is not a resource APIOps manages. These take part in id
/// relativization only; they never contribute to the relationship graph.
///
ImmutableHashSet RelativeIdDtoProperties => [];
```

**`common/Resource.FileSystem.cs`**, include them when formatting the information file:

```csharp
private static JsonObject FormatInformationFileDto(this IResourceWithReference resource, JsonObject dto) =>
resource.MandatoryReferencedResourceDtoProperties
.Union(resource.OptionalReferencedResourceDtoProperties)
.Select(kvp => kvp.Value)
.Union(resource.RelativeIdDtoProperties)
.Aggregate(dto, SetAbsoluteToRelativeId);
```

`Union` on the projected property names also removes the need for the current `DistinctBy(kvp => kvp.Value)`, which exists because `Scope` is declared twice.

**`common/Subscription.cs`** and **`common/WorkspaceSubscription.cs`**:

```csharp
public ImmutableHashSet RelativeIdDtoProperties { get; } =
[nameof(SubscriptionDto.Properties.OwnerId)];
```

The interface member defaults to empty, so nothing else changes and no existing resource is affected. `nameof` follows the convention already used for `Scope` and `LoggerId`, which works because DTOs are parsed with `JsonSerializerOptions.Web`.

### Scope

- `SubscriptionResource.OwnerId`
- `WorkspaceSubscriptionResource.OwnerId`

I looked for other properties in the same category and did not find an obvious one, but the new member gives a place for them if they turn up.

### Effect

Subscription artifacts become fully instance-agnostic, and the per-subscription `ownerId` override disappears from every environment configuration file. In our own repo that is 21 override lines in one operator repository and 3 in each of two environment files in another.

### Happy to implement

I am glad to send the PR with tests if you agree with the shape. The main open question is naming and whether you would rather see this as a separate member as proposed, or handled some other way, for example a marker on the DTO property or widening the existing dictionaries to accept a null resource key.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.