Skip to content

Commit 4ebb88d

Browse files
authored
Fix mgmt generator: correctly detect extension resources with specific parent types (Azure#57722)
1 parent 53334f8 commit 4ebb88d

24 files changed

+784
-939
lines changed

eng/packages/http-client-csharp-mgmt/emitter/src/resolve-arm-resources-converter.ts

Lines changed: 12 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -564,35 +564,27 @@ function convertScopeToResourceScope(
564564
export function getOperationScopeFromPath(path: string): ResourceScope {
565565
// Match any path starting with a variable segment followed by /providers/
566566
// This covers scope-based operations like /{resourceUri}/providers/..., /{scope}/providers/..., /{resourceId}/providers/..., etc.
567-
if (/^\/\{[^}]+\}\/providers\//.test(path)) {
568-
return ResourceScope.Extension;
569-
} else if (
570-
/^\/subscriptions\/\{[^}]+\}\/resourceGroups\/\{[^}]+\}\//.test(path)
567+
const lastProviderIndex = path.lastIndexOf("/providers/");
568+
const scopePath = path.substring(0, lastProviderIndex);
569+
if (!scopePath) {
570+
return ResourceScope.Tenant;
571+
}
572+
else if (
573+
/^\/subscriptions\/\{[^}]+\}\/resourceGroups\/\{[^}]+\}$/.test(scopePath)
571574
) {
572575
return ResourceScope.ResourceGroup;
573-
} else if (/^\/subscriptions\/\{[^}]+\}\//.test(path)) {
576+
} else if (/^\/subscriptions\/\{[^}]+\}$/.test(scopePath)) {
574577
return ResourceScope.Subscription;
575578
} else if (
576-
/^\/providers\/Microsoft\.Management\/managementGroups\/\{[^}]+\}\//.test(
577-
path
579+
/^\/providers\/Microsoft\.Management\/managementGroups\/\{[^}]+\}$/.test(
580+
scopePath
578581
)
579582
) {
580583
return ResourceScope.ManagementGroup;
581-
} else if (hasMultipleProviderSegments(path)) {
582-
// Paths with multiple /providers/ segments indicate extension resources
583-
// e.g., /providers/Microsoft.Management/serviceGroups/{name}/providers/Microsoft.Edge/sites/{siteName}
584+
}
585+
else {
584586
return ResourceScope.Extension;
585587
}
586-
return ResourceScope.Tenant; // all the templates work as if there is a tenant decorator when there is no such decorator
587-
}
588-
589-
/**
590-
* Check if a path has multiple /providers/ segments, indicating an extension resource
591-
* that extends another ARM resource.
592-
*/
593-
function hasMultipleProviderSegments(path: string): boolean {
594-
const providerMatches = path.match(/\/providers\//gi);
595-
return providerMatches !== null && providerMatches.length > 1;
596588
}
597589

598590
/**

eng/packages/http-client-csharp-mgmt/emitter/src/resource-detection.ts

Lines changed: 9 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -48,10 +48,7 @@ import {
4848
builtInResourceOperationName,
4949
parentResourceName,
5050
readsResourceName,
51-
resourceGroupResource,
5251
singleton,
53-
subscriptionResource,
54-
tenantResource
5552
} from "./sdk-context-options.js";
5653
import {
5754
DecoratorApplication,
@@ -422,12 +419,8 @@ export function buildArmProviderSchema(
422419

423420
// Update the model's resourceScope based on resource scope decorator if it exists or based on the Read method's scope.
424421
// This is specific to legacy resource detection
425-
for (const [metadataKey, metadata] of resourcePathToMetadataMap) {
426-
const modelId = metadataKey.split("|")[0];
427-
const model = resourceModelMap.get(modelId);
428-
if (model) {
429-
metadata.resourceScope = getResourceScope(model, metadata.methods);
430-
}
422+
for (const metadata of resourcePathToMetadataMap.values()) {
423+
metadata.resourceScope = getResourceScope(metadata.methods);
431424
}
432425

433426
// Create parent lookup context for legacy resource detection
@@ -1039,31 +1032,25 @@ function getSingletonResource(
10391032
return singletonResource ?? "default";
10401033
}
10411034
function getResourceScope(
1042-
model: InputModelType,
10431035
methods?: ResourceMethod[]
10441036
): ResourceScope {
1045-
// First, check for explicit scope decorators
1046-
const decorators = model.decorators;
1047-
if (decorators?.some((d) => d.name == tenantResource)) {
1048-
return ResourceScope.Tenant;
1049-
} else if (decorators?.some((d) => d.name == subscriptionResource)) {
1050-
return ResourceScope.Subscription;
1051-
} else if (decorators?.some((d) => d.name == resourceGroupResource)) {
1052-
return ResourceScope.ResourceGroup;
1053-
}
1054-
1055-
// Fall back to Read method's scope only if no scope decorators are found
1037+
// Determine scope from the Read method's operation path, which is the source of truth.
1038+
// Scope decorators (@resourceGroupResource, etc.) can be inherited implicitly from base
1039+
// model types like ProxyResource and may not reflect the actual scope for extension
1040+
// resources that use Legacy.ExtensionOperations with specific parent types.
10561041
if (methods) {
10571042
const getMethod = methods.find(
10581043
(m) => m.kind === ResourceOperationKind.Read
10591044
);
1045+
// We have logic to filter out resources without get/read operations later in post-processing,
1046+
// so it's possible to have a resource with no Read method. In that case, we skip scope detection since the resource will be filtered out anyway.
10601047
if (getMethod) {
10611048
return getMethod.operationScope;
10621049
}
10631050
}
10641051

10651052
// Final fallback to ResourceGroup
1066-
return ResourceScope.ResourceGroup; // all the templates work as if there is a resource group decorator when there is no such decorator
1053+
return ResourceScope.ResourceGroup;
10671054
}
10681055

10691056
/**

eng/packages/http-client-csharp-mgmt/emitter/src/resource-metadata.ts

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,9 @@ export interface ResourceMetadata {
8080
parentResourceModelId?: string;
8181
singletonResourceName?: string;
8282
resourceName: string;
83+
/** The expected parent resource type for extension resources with specific parent types (e.g., "Microsoft.Compute/virtualMachines") */
84+
// TODO: consider to calculate this in generator directly within RequestPathPattern instead of carrying it through emitter and post-processing
85+
parentResourceType?: string;
8386
/** The name constraints for the resource, from TypeSpec decorators */
8487
nameConstraints: NameConstraints;
8588
/** The API versions that this resource is available in */
@@ -286,6 +289,7 @@ export function convertArmProviderSchemaToArguments(
286289
})),
287290
resourceScope: r.metadata.resourceScope,
288291
parentResourceId: r.metadata.parentResourceId,
292+
parentResourceType: r.metadata.parentResourceType,
289293
singletonResourceName: r.metadata.singletonResourceName,
290294
resourceName: r.metadata.resourceName,
291295
nameConstraints: r.metadata.nameConstraints,
@@ -525,6 +529,15 @@ export function postProcessArmResources(
525529
sortResourceMethods(resource.metadata.methods);
526530
}
527531

532+
// Step 8: Compute parentResourceType for extension resources with specific parent types
533+
for (const resource of filteredResources) {
534+
if (countProviderSegments(resource.metadata.resourceIdPattern) > 1) {
535+
resource.metadata.parentResourceType = getExpectedParentResourceType(
536+
resource.metadata.resourceIdPattern
537+
);
538+
}
539+
}
540+
528541
return filteredResources;
529542
}
530543

@@ -767,3 +780,57 @@ function relocateCrossResourceListActions(
767780
targetResource.metadata.methods.push(method);
768781
}
769782
}
783+
784+
/**
785+
* Extracts the expected parent resource type from a resource ID pattern that is
786+
* already known to have multiple /providers/ segments. Returns undefined if the
787+
* parent segment is not a simple `<namespace>/<type>/{name}` pattern (e.g., for
788+
* complex paths with nested types or mixed scopes).
789+
*
790+
* For example, for a pattern like:
791+
* /subscriptions/{sub}/resourceGroups/{rg}/providers/Microsoft.Compute/virtualMachines/{vm}/providers/MyService/resources/{name}
792+
* This returns "Microsoft.Compute/virtualMachines".
793+
*/
794+
function getExpectedParentResourceType(
795+
resourceIdPattern: string
796+
): string | undefined {
797+
// Find the last /providers/ segment (the extension resource's own provider)
798+
const lastProvidersIndex = resourceIdPattern.lastIndexOf("/providers/");
799+
800+
// Find the second-to-last /providers/ segment (the parent resource's provider)
801+
const parentProvidersIndex = resourceIdPattern.lastIndexOf(
802+
"/providers/",
803+
lastProvidersIndex - 1
804+
);
805+
806+
if (parentProvidersIndex === -1) {
807+
return undefined;
808+
}
809+
810+
// Extract the parent segment between the two /providers/ markers
811+
const parentSegment = resourceIdPattern.substring(
812+
parentProvidersIndex + "/providers/".length,
813+
lastProvidersIndex
814+
);
815+
816+
// Only accept simple parent segments: "<namespace>/<type>/{name}"
817+
// Reject complex paths (nested types, mixed scopes) to avoid false positives
818+
const segments = parentSegment.split("/");
819+
if (segments.length !== 3) {
820+
return undefined;
821+
}
822+
823+
const [providerNamespace, resourceType, nameSegment] = segments;
824+
825+
// All three segments must be well-formed: constants for namespace/type, variable for name
826+
if (
827+
providerNamespace.includes("{") ||
828+
resourceType.includes("{") ||
829+
!nameSegment.startsWith("{") ||
830+
!nameSegment.endsWith("}")
831+
) {
832+
return undefined;
833+
}
834+
835+
return `${providerNamespace}/${resourceType}`;
836+
}

eng/packages/http-client-csharp-mgmt/emitter/test/resource-type.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -138,11 +138,11 @@ describe("Operation Scope Detection", () => {
138138
strictEqual(scope, ResourceScope.Extension);
139139
});
140140

141-
it("resource group scope takes priority over nested extension resources", async () => {
141+
it("extension scope for resources extending a specific ARM resource within a resource group", async () => {
142142
const path =
143143
"/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Something/parentResource/{parentName}/providers/Microsoft.Edge/sites/{siteName}";
144144
const scope = getOperationScopeFromPath(path);
145-
strictEqual(scope, ResourceScope.ResourceGroup);
145+
strictEqual(scope, ResourceScope.Extension);
146146
});
147147
});
148148

eng/packages/http-client-csharp-mgmt/generator/Azure.Generator.Management/src/Models/ResourceSchemas/ArmResourceMetadata.cs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ namespace Azure.Generator.Management.Models;
1717
/// <param name="Methods"> The list of methods associated with the resource. </param>
1818
/// <param name="SingletonResourceName"> The singleton resource name, if applicable. </param>
1919
/// <param name="ParentResourceId"> The parent resource ID pattern, if applicable. </param>
20+
/// <param name="ParentResourceType"> The expected parent resource type for extension resources with specific parent types (e.g., "Microsoft.Compute/virtualMachines"), if applicable. </param>
2021
/// <param name="ChildResourceIds"> The list of child resource ID patterns. </param>
2122
/// <param name="NameConstraints"> The name constraints for the resource. </param>
2223
/// <param name="ApiVersions"> The API versions that this resource is available in. </param>
@@ -30,6 +31,7 @@ public record ArmResourceMetadata(
3031
IReadOnlyList<ResourceMethod> Methods,
3132
string? SingletonResourceName,
3233
string? ParentResourceId,
34+
string? ParentResourceType,
3335
IReadOnlyList<string> ChildResourceIds,
3436
ArmResourceNameConstraints NameConstraints,
3537
IReadOnlyList<string> ApiVersions,
@@ -83,6 +85,11 @@ internal static ArmResourceMetadata DeserializeResourceMetadata(JsonElement elem
8385
{
8486
parentResource = parentResourceElement.GetString();
8587
}
88+
string? parentResourceType = null;
89+
if (element.TryGetProperty("parentResourceType", out var parentResourceTypeElement))
90+
{
91+
parentResourceType = parentResourceTypeElement.GetString();
92+
}
8693
if (element.TryGetProperty("resourceName", out var resourceNameElement))
8794
{
8895
resourceName = resourceNameElement.GetString();
@@ -125,6 +132,7 @@ internal static ArmResourceMetadata DeserializeResourceMetadata(JsonElement elem
125132
methods,
126133
singletonResourceName,
127134
parentResource,
135+
parentResourceType,
128136
childResourceIds,
129137
nameConstraints,
130138
apiVersions,

eng/packages/http-client-csharp-mgmt/generator/Azure.Generator.Management/src/Providers/ResourceClientProvider.cs

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,8 @@ private ResourceClientProvider(string resourceName, InputModelType model, IReadO
8383

8484
internal ResourceScope ResourceScope => _resourceMetadata.ResourceScope;
8585
internal string? ParentResourceIdPattern => _resourceMetadata.ParentResourceId;
86+
internal string ResourceIdPattern => _resourceMetadata.ResourceIdPattern;
87+
internal string? ParentResourceType => _resourceMetadata.ParentResourceType;
8688

8789
internal bool IsExtensionResource => ResourceScope == ResourceScope.Extension;
8890

@@ -130,16 +132,18 @@ private IReadOnlyList<ResourceClientProvider> BuildChildResources()
130132

131133
private CSharpType BuildTypeOfParentResource()
132134
{
133-
if (_resourceMetadata.ResourceScope == ResourceScope.Extension)
134-
{
135-
return typeof(ArmResource);
136-
}
137-
138135
// if the resource has a parent resource id, we can find it in the output library
139136
if (_resourceMetadata.ParentResourceId is not null)
140137
{
141138
return ManagementClientGenerator.Instance.OutputLibrary.GetResourceById(_resourceMetadata.ParentResourceId).Type;
142139
}
140+
141+
// if not and this is an extension resource, we return ArmResource as the parent type as fallback
142+
if (_resourceMetadata.ResourceScope == ResourceScope.Extension)
143+
{
144+
return typeof(ArmResource);
145+
}
146+
143147
// if it does not, this resource's parent must be one of the MockableResource
144148
return ManagementClientGenerator.Instance.OutputLibrary.GetMockableResourceByScope(_resourceMetadata.ResourceScope).ArmCoreType;
145149
}

eng/packages/http-client-csharp-mgmt/generator/Azure.Generator.Management/src/Providers/ResourceCollectionClientProvider.cs

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -126,8 +126,6 @@ private static RequestPathPattern GetContextualPath(ArmResourceMetadata resource
126126
// For example, for path /providers/Microsoft.Management/serviceGroups/{servicegroupName}/providers/Microsoft.Edge/sites/{siteName}
127127
// the contextual path is /providers/Microsoft.Management/serviceGroups/{servicegroupName}
128128
// This ensures that servicegroupName is derived from the scope Id (id.Name) rather than being an extra constructor parameter.
129-
// For specific extension resources (those targeting a known parent type like Microsoft.Compute/virtualMachines),
130-
// the ParentResourceType property contains the explicit parent type for method name disambiguation.
131129
return new RequestPathPattern(resourceMetadata.ResourceIdPattern).GetParent();
132130
}
133131

@@ -199,7 +197,7 @@ private static void InitializeMethods(
199197

200198
protected override FormattableString BuildDescription()
201199
{
202-
var parentResourceType = GetParentResourceType(_resourceMetadata, _resource);
200+
var parentResourceType = GetParentResourceType(_resourceMetadata, _resource) ?? typeof(ArmResource); // TODO: will update this with actual external resource type
203201
return $"A class representing a collection of {_resource.Type:C} and their operations.\nEach {_resource.Type:C} in the collection will belong to the same instance of {parentResourceType:C}.\nTo get a {Type:C} instance call the Get{ResourceName.Pluralize()} method from an instance of {parentResourceType:C}.";
204202
}
205203

@@ -292,16 +290,16 @@ private ConstructorProvider BuildResourceIdentifierConstructor()
292290
bodyStatements.Add(clientInfo.RestClientField.Assign(New.Instance(clientInfo.RestClientProvider.Type, clientInfo.DiagnosticsField, thisCollection.Pipeline(), thisCollection.Endpoint(), effectiveApiVersion)).Terminate());
293291
}
294292

295-
// Do not call ValidateResourceId for Extension resources
296-
if (_resourceMetadata.ResourceScope != ResourceScope.Extension)
293+
// skip resource Id validation for extension resource without parent resource type, since we don't have enough information to validate the resource Id. For example, for an extension resource with resource scope of extension and no parent resource type specified, the resource Id pattern could be something like /{scope}/providers/Microsoft.ABC/def/{defName}, in this case we don't know what the {scope} is, it could be subscription, resource group, or even a management group, so we can't validate the resource Id.
294+
if (_resourceMetadata.ResourceScope != ResourceScope.Extension || !string.IsNullOrEmpty(_resourceMetadata.ParentResourceType))
297295
{
298296
bodyStatements.Add(Static(Type).As<ArmCollection>().ValidateResourceId(idParameter).Terminate());
299297
}
300298

301299
return new ConstructorProvider(signature, bodyStatements, this);
302300
}
303301

304-
private static CSharpType GetParentResourceType(ArmResourceMetadata resourceMetadata, ResourceClientProvider resource)
302+
private static CSharpType? GetParentResourceType(ArmResourceMetadata resourceMetadata, ResourceClientProvider resource)
305303
{
306304
// First check if the resource has a parent resource
307305
if (resourceMetadata.ParentResourceId is not null)
@@ -318,10 +316,12 @@ private static CSharpType GetParentResourceType(ArmResourceMetadata resourceMeta
318316
return typeof(SubscriptionResource);
319317
case ResourceScope.Tenant:
320318
return typeof(TenantResource);
321-
case ResourceScope.Extension:
322-
return typeof(ArmResource);
323319
case ResourceScope.ManagementGroup:
324320
return typeof(ManagementGroupResource);
321+
case ResourceScope.Extension:
322+
// Generic-scope extension resources (e.g., /{scope}/providers/...) have no
323+
// specific parent type. Return null so callers fall back to ArmResource.
324+
return null;
325325
default:
326326
// TODO -- this is incorrect, but we put it here as a placeholder.
327327
return resource.Type;
@@ -331,11 +331,16 @@ private static CSharpType GetParentResourceType(ArmResourceMetadata resourceMeta
331331
protected override MethodProvider[] BuildMethods()
332332
{
333333
var methods = new List<MethodProvider>();
334-
335-
// Do not build ValidateResourceIdMethod for Extension resources
334+
var parentResourceCsharpType = GetParentResourceType(_resourceMetadata, _resource);
336335
if (_resourceMetadata.ResourceScope != ResourceScope.Extension)
337336
{
338-
methods.Add(ResourceMethodSnippets.BuildValidateResourceIdMethod(this, Static(GetParentResourceType(_resourceMetadata, _resource)).As<ArmResource>().ResourceType()));
337+
methods.Add(ResourceMethodSnippets.BuildValidateResourceIdMethod(this,Static(parentResourceCsharpType!).As<ArmResource>().ResourceType()));
338+
}
339+
// For extension resource with parent resource type specified, we can also generate a ValidateResourceId method with parent resource type, which will be used in the Get{ResourceName} method in the parent resource's collection client.
340+
// This is needed to ensure the resource Id passed in is valid and belongs to the correct parent resource type.
341+
else if (!string.IsNullOrEmpty(_resourceMetadata.ParentResourceType))
342+
{
343+
methods.Add(ResourceMethodSnippets.BuildValidateResourceIdMethod(this, Literal(_resourceMetadata.ParentResourceType!)));
339344
}
340345

341346
methods.AddRange(BuildCreateOrUpdateMethods());

0 commit comments

Comments
 (0)