Document x-ms-arm-id-details extension

[matthchr]

No description provided.

[matthchr]

See Azure/k8s-infra#442 for the original discussion around this topic.

@johanste @m-nash @markcowl - would be great if you could take a look at this.

In terms of validating that Swagger authors actually set this:

1. I will send a PR to azure-openapi-validator adding a linter that uses a heuristic to detect things that look like references. Basically it'll find fields that are id or *Id (and possibly a few more variants) and look at the documentation to see if they are likely ARM IDs.
2. I'll work with Arthur Ning's team to use their https://aka.ms/LiveValidation pipeline to perform validation against live requests.

I think between these two things we should be able to catch 90-100% of all references.

The linter will also perform some basic assertions such as that this extension is only used in valid locations (although I'm not sure such an assertion is actually performed for other extensions)

[matthchr]

I originally wanted to use the same format that ARM specifies in ARM templates (Microsoft.Resources/resourceGroups or Microsoft.Compute/virtualMachines, etc), but that format doesn't actually convey if the resource roots up to subscription, resourceGroup, or something else, which is required to validate resource ID validity (or generate an ID that mathces) so I think we need a more verbose format such as this one.

[PhoenixHe-NV]

One id property MAY want to point to specified resource type but don't want to specify scope. For example, some property may want to point to any authorization resource. How do you think about this?

[matthchr]

@PhoenixHe-msft - what does an authorization resource URL look like?

[johanste]

I assume that the general question is what referring to an extension resource looks like... In an ARM template, this would be done by providing the scope for the resource.

I'm not sure how deep down the rabbit hole we'd like to run with specifying that an ARM ID can only refer to extension resource type A but only if it is scoped to resource type B. I believe that could be a recursive definition that would make the hole very deep indeed. But I could see a scope: * or similar added to indicate that this is an extension provider.

[PhoenixHe-NV]

Exactly. Suggest to referencing the design of ARM template's scope.

[matthchr]

• allowedTypes -> allowedResources: I'm happy to make this change.
• add scopes: Agreed
• resource -> type: The other document calls it paths (or path for singular here), and it uses the full URL like /subscriptions/{subscriptionId}/resourceGroups/{rg}/providers/Microsoft.Compute/virtualMachines/{vm} whereas your example us just using Microsoft.Compute/virtualMachines. Just wanted to highlight that difference to the proposal that @majastrz made. I do think what you're suggesting here makes a lot of sense though as it doesn't make sense to put the full path here if it can possibly apply to many scopes (since the full path would be different for each scope).

I'll go ahead and make these changes because they seem good to me.

[matthchr]

Oh one more thing, I think type probably should support * too. That way you can do something like:

// This can be any ARM ID  
"id": {
  "type": "string",
  "format": "arm-id",
}

// This can be any ARM ID as long as it's RG scoped
"id": {
  "type": "string",
  "format": "arm-id",
  "x-ms-arm-id-details": {
    "allowedResources": [
      {
        "scopes": ["ResourceGroup"],
        "type": "*"
      },
    ]
}

I don't actually know if we have any resources that need this functionality today but I don't know that we don't so we might as well support it?

[matthchr]

One other thought... I think we should drop * and replace it with Extension.

*'s meaning is ambiguous right now. Is "scope": "*" == "scope": ["Tenant", "Subscription", "ResourceGroup", "ManagementGroup"]? Not really, because * also supports extension resources. It's confusing if "scope": "*" just means "this resource is an extension resource" though - if we want it to mean that we should call it "scope": "Extension" or "scope": "AnyResource" instead.

* meaning "All scopes" doesn't seem that useful because scopes is already a collection (as proposed by @leni-msft), and it seems unlikely to me that if Azure comes up with a new scope that all the old resources that said "scope": "*" with actually jump to immediately supporting references at the new scope. It seems better to just be explicit and make resources that refer to something like Microsoft.Resources/deployments say:

"deploymentId": {
  "type": "string",
  "format": "arm-id",
  "x-ms-arm-id-details": {
    "allowedResources": [
      {
        "scopes": ["Tenant", "Subscription", "ResourceGroup", "ManagementGroup"],
        "type": "Microsoft.Resources/deployments"
      },
    ]
}

Thoughts?

[leni-msft]

Thanks @matthchr for the quick updates.

• resource -> type: The other document calls it paths (or path for singular here) ...

No problem, the change has also been agreed by @majastrz in the doc comment.

I think type probably should support * too

Agree. One further step, the type should support Microsoft.Compute/* - to limit the resource providers but allow any resource types under it.

I think we should drop * and replace it with Extension

Agree to add Extension as a valid scope value. But I suggest to keep *(any valid scope) just for easier writing - it's unlikely that Azure will have a new scope.

[matthchr]

Agree. One further step, the type should support Microsoft.Compute/* - to limit the resource providers but allow any resource types under it.

Done.

Agree to add Extension as a valid scope value. But I suggest to keep *(any valid scope) just for easier writing - it's unlikely that Azure will have a new scope.

Ok, as long as it's not likely for Azure to be adding new scopes I'm fine with *. Have added it back and documented it.

[matthchr]

@akning-ms as well

[jianyexi]

what's the common type 'ArmId' for ?

[matthchr]

@johanste mentioned that he thought it would be useful to have a shared type in the common defs (like we do for Resource and a few other types today).

[akning-ms]

• @leni-msft to comment

[johanste]

@markcowl may also have some valuable feedback on this proposal. Explicitly mentioning him (and adding him as a reviewer) probably gets his attention!

[timotheeguerin]

Is there anybody else that should still look at this PR or should we merge it?

[matthchr]

@timotheeguerin - @leni-msft linked me to a work document from the ARM Bicep team that has a related proposal and we wanted to ensure we were using a similar scope for each proposal. I haven't finished doing that yet though (I've been busy on other stuff). I'll try to get that done later this week or probably more likely early next week.

It would also be great if @markcowl gets a chance to take a look probably? I am not sure what other stakeholders there are and if we've gotten their approval...

[ghost]

Hi @matthchr. Thank you for your interest in helping to improve the Azure SDK experience and for your contribution. We've noticed that there hasn't been recent engagement on this pull request. If this is still an active work stream, please let us know by pushing some changes or leaving a comment. Otherwise, we'll close this out in 7 days.

[matthchr]

I wanted to unify the experience with the document being driven by @majastrz, but it looks like there hasn't been movement there. I would still like to get this merged but maybe the thing to do is just take it basically as it is, I can work with @leni-msft and team to help add detectors to find Swaggers that are missing (via the Swagger completeness workstream that flows to S360 probably?)

@leni-msft is there a wiki or something on how to add rules to detect things for stuff like this? What's the process there. I'm basically happy to merge this assuming people are still OK with it but a merged doc doesn't do much to move the needle on actually getting teams to adopt this. I can actually go annotate some major services Swaggers myself and send PRs for that and it would be great to get some linting as well if I can get help on how to do it...

[leni-msft]

@matthchr To add new rules, please submit form via https://aka.ms/lintrulecandidate , and our team will review and discuss how to implement the rule in PR static validation and live validation.

[markcowl]

What are the thoughts on how scopes should be interpreted? Does this apply a restriction to the same tenant / subscription / resource group / management group as the referring resource, or something else?

I worry about making this too deployment-specific. It would be nice to apply this in general to encode actual 'locality' restrictions that could also be used both for smart tools to validate an input reference or for for smart tools to deploy the resource. One such restriction would be 'location', for resources that must be in the same region as the referring resource.

[matthchr]

What are the thoughts on how scopes should be interpreted? Does this apply a restriction to the same tenant / subscription / resource group / management group as the referring resource, or something else?

I hadn't thought about "the referenced resource must be in the same region/tenant/subscription as the current resource". That's an interesting angle and we could definitely consider expanding this proposal to cover that case as well. Scope as proposed is about the ability to restrict what deployment locations are allowed for the given resource type.

So you would read {"scopes": ["ResourceGroup"], "type": "Microsoft.Network/virtualNetworks"} as "This property must refer to an ARM ID pointing to a virtual network rooted in a resource group". There's no requirement of what subscription that RG is in. If you were to validate this ARM ID with a regex it would look something like /subscriptions/[a-zA-Z0-0-]+/resourceGroups/[a-zA-Z0-0-]+/Microsoft.Network/virtualNetworks/[a-zA-Z0-0-]+. This reference explicitly disallows a Microsoft.Network/virtualNetworks deployed elsewhere, say in a management group (That's probably not allowed today but some resources are dual-homed I believe).

Services that don't care can use ``{"scopes": ["*"], "type": "Microsoft.Network/virtualNetworks"}. This is the same concept of scope` that is proposed/discussed in @majastrz's document as well. It's really just a shorthand for `/subscriptions/{}/resourceGroups/{}` (and the various other scope prefixes).

[markcowl]

So, management groups typically sit 'above' subscriptions, so, unless I am wildly mistaken, you can only deploy a virtual network to a resource group. Management Groups are a good deployment location for extension resources, but don't apply to resources that are Tracked Resources or Subscription resources, or resource group resources.

I guess here, you want to be able to deploy the resource without having to know anything about the properties of the resource type, is that correct?

[matthchr]

I guess here, you want to be able to deploy the resource without having to know anything about the properties of the resource type, is that correct?

Yes, exactly. Maybe I need to do some documentation improvements (or we need to pick a different name than Scope?). As discussed over teams the issue is that this is a reference to a say Microsoft.Network/virtualnetworks, and at the reference site there's no knowledge of what that thing looks like. The objective for this annotation would be that client tools could give better hints/warnings about ARM references, so ideally given this annotation we can look at an ARM ID and confirm if it's valid. Without a scope section though we couldn't confirm that:
/subscriptions/<id>/Microsoft.Network/virtualNetworks/myVnet was an invalid reference, because it's not at the right scope.

[markcowl]

I would say, this is good for the first version, and perhaps we can augment this later with some restrictions around the resource (for example, that it has to be in the same region,