Skip to content

Commit 3d6774a

Browse files
AkhilaIllaakhilaillaCopilotmikeharder
authored
Fix TopLevelResourcesListBySubscription to exclude tenant level API's… (#808)
* Fix TopLevelResourcesListBySubscription to exclude tenant level API's from validation * Update test name * Remove the guard * Update package.json and CHANGELOG.md for TopLevelResourcesListBySubscription fix (#815) * Initial plan * Update package.json and CHANGELOG.md for version 2.2.1 Co-authored-by: mikeharder <9459391+mikeharder@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: mikeharder <9459391+mikeharder@users.noreply.github.com> * Apply suggestion from @mikeharder * Increment patch version to 2.2.2 (#818) * Initial plan * Increment patch version from 2.2.1 to 2.2.2 Co-authored-by: mikeharder <9459391+mikeharder@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: mikeharder <9459391+mikeharder@users.noreply.github.com> --------- Co-authored-by: akhilailla <akhilailla@microsoft.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: mikeharder <9459391+mikeharder@users.noreply.github.com> Co-authored-by: Mike Harder <mharder@microsoft.com>
1 parent 4fce231 commit 3d6774a

7 files changed

Lines changed: 407 additions & 23 deletions

‎packages/rulesets/CHANGELOG.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,11 @@
11
# Change Log - @microsoft.azure/openapi-validator-rulesets
22

3+
## 2.2.5
4+
5+
### Patches
6+
7+
- [TopLevelResourcesListBySubscription] Exclude tenant-level APIs from validation
8+
39
## 2.2.4
410

511
### Patches

‎packages/rulesets/package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@microsoft.azure/openapi-validator-rulesets",
3-
"version": "2.2.4",
3+
"version": "2.2.5",
44
"description": "Azure OpenAPI Validator",
55
"main": "dist/index.js",
66
"files": [

‎packages/rulesets/src/native/legacyRules/TopLevelResourcesListBySubscription.ts‎

Lines changed: 29 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -15,22 +15,41 @@ rules.push({
1515
*run(doc, node, path, ctx) {
1616
const msg = 'The top-level resource "{0}" does not have list by subscription operation, please add it.'
1717
const utils = new ArmHelper(doc, ctx?.specPath!, ctx?.inventory!)
18-
const topLevelResources = utils.getTopLevelResourceNames()
18+
// Top-level resources are calculated by ArmHelper.getTopLevelResources():
19+
// 1. Scans all operations (GET/PUT/POST/PATCH/DELETE) in paths and x-ms-paths
20+
// 2. For each operation, checks the 200/201 response schema's $ref to find resource definitions
21+
// 3. A definition is a "resource" if it has x-ms-azure-resource:true or allOfs Resource/TrackedResource/ProxyResource
22+
// 4. A resource is "top-level" if its path has exactly ONE resource-type segment after /providers/
23+
// e.g., /subscriptions/{sub}/providers/Microsoft.Contoso/widgets/{name} → "widgets" is top-level
24+
// vs. /.../widgets/{name}/parts/{partName} → "parts" is nested (2 segments)
25+
const topLevelResources = utils.getTopLevelResources()
1926
const allCollectionApis = utils.getCollectionApiInfo()
20-
for (const path in node.paths) {
21-
const hasMatchedTenantLevel = !path.includes("/subscriptions/")
22-
if (hasMatchedTenantLevel) {
23-
return
24-
}
25-
}
27+
28+
// The same model can be discovered multiple times (different operations/files),
29+
// but we only want to report at most once per definition name.
30+
const validatedModels = new Set<string>()
2631
for (const resource of topLevelResources) {
32+
const resourceName = resource.modelName
33+
// Skip already validated resources
34+
if (validatedModels.has(resourceName)) {
35+
continue
36+
}
37+
38+
// Tenant-scoped resources do not need list-by-subscription, so skip them.
39+
const hasSubscriptionScopedOperation = resource.operations?.some((op: { apiPath: string }) => utils.isPathBySubscription(op.apiPath))
40+
if (!hasSubscriptionScopedOperation) {
41+
continue
42+
}
43+
44+
validatedModels.add(resourceName)
2745
const hasMatched = allCollectionApis.some(
28-
(collection) => resource === collection.childModelName && collection.collectionGetPath.some((p) => utils.isPathBySubscription(p)),
46+
(collection) =>
47+
resourceName === collection.childModelName && collection.collectionGetPath.some((p) => utils.isPathBySubscription(p)),
2948
)
3049
if (!hasMatched) {
3150
yield {
32-
message: msg.replace("{0}", resource),
33-
location: ["$", "definitions", resource] as JsonPath,
51+
message: msg.replace("{0}", resourceName),
52+
location: ["$", "definitions", resourceName] as JsonPath,
3453
}
3554
}
3655
}

‎packages/rulesets/src/native/tests/composite-azure-tests.ts‎

Lines changed: 42 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ describe("CompositeAzureTests", () => {
5959
const messages: LintResultMessage[] = await collectTestMessagesFromValidator(
6060
fileName,
6161
OpenApiTypes.arm,
62-
NestedResourcesMustHaveListOperation
62+
NestedResourcesMustHaveListOperation,
6363
)
6464
assertValidationRuleCount(messages, NestedResourcesMustHaveListOperation, 1)
6565
})
@@ -69,17 +69,17 @@ describe("CompositeAzureTests", () => {
6969
const messages: LintResultMessage[] = await collectTestMessagesFromValidator(
7070
fileName,
7171
OpenApiTypes.arm,
72-
TopLevelResourcesListByResourceGroup
72+
TopLevelResourcesListByResourceGroup,
7373
)
7474
assertValidationRuleCount(messages, TopLevelResourcesListByResourceGroup, 1)
7575
})
7676

77-
test("extension top level resources should report no errors", async () => {
77+
test("extension top level resources should report no errors for list-by-subscription", async () => {
7878
const fileName = ["armResource/extensionTrackedResourceNoListBySubscription.json", "armResource/trackedResourceCommon.json"]
7979
const messages: LintResultMessage[] = await collectTestMessagesFromValidator(
8080
fileName,
8181
OpenApiTypes.arm,
82-
TopLevelResourcesListBySubscription
82+
TopLevelResourcesListBySubscription,
8383
)
8484
assertValidationRuleCount(messages, TopLevelResourcesListBySubscription, 0)
8585
})
@@ -89,12 +89,42 @@ describe("CompositeAzureTests", () => {
8989
const messages: LintResultMessage[] = await collectTestMessagesFromValidator(
9090
fileName,
9191
OpenApiTypes.arm,
92-
TopLevelResourcesListBySubscription
92+
TopLevelResourcesListBySubscription,
9393
)
94-
// There is tenant level api in compute.json file
94+
// This spec includes tenant-level APIs; validation should still run for subscription-scoped resources.
9595
assertValidationRuleCount(messages, TopLevelResourcesListBySubscription, 0)
9696
})
9797

98+
test("tenant-only specs should skip list-by-subscription validation", async () => {
99+
const fileName = "armResource/topLevelListBySubscription_tenantOnlyResource.json"
100+
const messages: LintResultMessage[] = await collectTestMessagesFromValidator(
101+
fileName,
102+
OpenApiTypes.arm,
103+
TopLevelResourcesListBySubscription,
104+
)
105+
assertValidationRuleCount(messages, TopLevelResourcesListBySubscription, 0)
106+
})
107+
108+
test("mixed tenant and subscription specs should still validate list-by-subscription", async () => {
109+
const fileName = "armResource/topLevelListBySubscription_mixedTenantAndSubscription_missingCollection.json"
110+
const messages: LintResultMessage[] = await collectTestMessagesFromValidator(
111+
fileName,
112+
OpenApiTypes.arm,
113+
TopLevelResourcesListBySubscription,
114+
)
115+
assertValidationRuleCount(messages, TopLevelResourcesListBySubscription, 1)
116+
})
117+
118+
test("subscription paths under x-ms-paths should still validate list-by-subscription", async () => {
119+
const fileName = "armResource/topLevelListBySubscription_subscriptionInXmsPaths_missingCollection.json"
120+
const messages: LintResultMessage[] = await collectTestMessagesFromValidator(
121+
fileName,
122+
OpenApiTypes.arm,
123+
TopLevelResourcesListBySubscription,
124+
)
125+
assertValidationRuleCount(messages, TopLevelResourcesListBySubscription, 1)
126+
})
127+
98128
test("get collection response schema should match the ARM specification", async () => {
99129
const fileName = "armResource/cdn.json"
100130
const messages: LintResultMessage[] = await collectTestMessagesFromValidator(fileName, OpenApiTypes.arm, GetCollectionResponseSchema)
@@ -106,7 +136,7 @@ describe("CompositeAzureTests", () => {
106136
const messages: LintResultMessage[] = await collectTestMessagesFromValidator(
107137
fileName,
108138
OpenApiTypes.arm,
109-
AllResourcesMustHaveGetOperation
139+
AllResourcesMustHaveGetOperation,
110140
)
111141
assertValidationRuleCount(messages, AllResourcesMustHaveGetOperation, 1)
112142
})
@@ -116,7 +146,7 @@ describe("CompositeAzureTests", () => {
116146
const messages: LintResultMessage[] = await collectTestMessagesFromValidator(
117147
fileName,
118148
OpenApiTypes.arm,
119-
AllResourcesMustHaveGetOperation
149+
AllResourcesMustHaveGetOperation,
120150
)
121151
assertValidationRuleCount(messages, AllResourcesMustHaveGetOperation, 0)
122152
})
@@ -126,7 +156,7 @@ describe("CompositeAzureTests", () => {
126156
const messages: LintResultMessage[] = await collectTestMessagesFromValidator(
127157
fileName,
128158
OpenApiTypes.arm,
129-
AllResourcesMustHaveGetOperation
159+
AllResourcesMustHaveGetOperation,
130160
)
131161
assertValidationRuleCount(messages, AllResourcesMustHaveGetOperation, 0)
132162
})
@@ -135,7 +165,7 @@ describe("CompositeAzureTests", () => {
135165
const messages: LintResultMessage[] = await collectTestMessagesFromValidator(
136166
fileName,
137167
OpenApiTypes.arm,
138-
AllResourcesMustHaveGetOperation
168+
AllResourcesMustHaveGetOperation,
139169
)
140170
assertValidationRuleCount(messages, AllResourcesMustHaveGetOperation, 0)
141171
})
@@ -145,7 +175,7 @@ describe("CompositeAzureTests", () => {
145175
const messages: LintResultMessage[] = await collectTestMessagesFromValidator(
146176
fileName,
147177
OpenApiTypes.arm,
148-
AllResourcesMustHaveGetOperation
178+
AllResourcesMustHaveGetOperation,
149179
)
150180
assertValidationRuleCount(messages, AllResourcesMustHaveGetOperation, 0)
151181
})
@@ -161,7 +191,7 @@ describe("CompositeAzureTests", () => {
161191
const messages: LintResultMessage[] = await collectTestMessagesFromValidator(
162192
fileName,
163193
OpenApiTypes.arm,
164-
PrivateEndpointResourceSchemaValidation
194+
PrivateEndpointResourceSchemaValidation,
165195
)
166196
assertValidationRuleCount(messages, PrivateEndpointResourceSchemaValidation, 2)
167197
})

‎packages/rulesets/src/native/tests/resources/armResource/topLevelListBySubscription_mixedTenantAndSubscription_missingCollection.json‎

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
{
2+
"swagger": "2.0",
3+
"info": {
4+
"title": "MixedTenantAndSubscriptionRP",
5+
"version": "2025-01-01"
6+
},
7+
"host": "management.azure.com",
8+
"schemes": [
9+
"https"
10+
],
11+
"produces": [
12+
"application/json"
13+
],
14+
"consumes": [
15+
"application/json"
16+
],
17+
"paths": {
18+
"/providers/Microsoft.Contoso/operations": {
19+
"get": {
20+
"operationId": "Operations_List",
21+
"responses": {
22+
"200": {
23+
"description": "OK",
24+
"schema": {
25+
"$ref": "#/definitions/OperationListResult"
26+
}
27+
}
28+
}
29+
}
30+
},
31+
"/subscriptions/{subscriptionId}/providers/Microsoft.Contoso/widgets/{widgetName}": {
32+
"get": {
33+
"operationId": "Widgets_Get",
34+
"parameters": [
35+
{
36+
"name": "subscriptionId",
37+
"in": "path",
38+
"required": true,
39+
"type": "string"
40+
},
41+
{
42+
"name": "widgetName",
43+
"in": "path",
44+
"required": true,
45+
"type": "string"
46+
}
47+
],
48+
"responses": {
49+
"200": {
50+
"description": "OK",
51+
"schema": {
52+
"$ref": "#/definitions/Widget"
53+
}
54+
}
55+
}
56+
}
57+
}
58+
},
59+
"definitions": {
60+
"Resource": {
61+
"type": "object",
62+
"properties": {
63+
"id": {
64+
"type": "string"
65+
},
66+
"name": {
67+
"type": "string"
68+
},
69+
"type": {
70+
"type": "string"
71+
}
72+
}
73+
},
74+
"Widget": {
75+
"allOf": [
76+
{
77+
"$ref": "#/definitions/Resource"
78+
}
79+
],
80+
"properties": {
81+
"properties": {
82+
"type": "object",
83+
"properties": {
84+
"size": {
85+
"type": "string"
86+
}
87+
}
88+
}
89+
}
90+
},
91+
"OperationListResult": {
92+
"type": "object",
93+
"properties": {
94+
"value": {
95+
"type": "array",
96+
"items": {
97+
"$ref": "#/definitions/Operation"
98+
}
99+
}
100+
}
101+
},
102+
"Operation": {
103+
"type": "object",
104+
"properties": {
105+
"name": {
106+
"type": "string"
107+
}
108+
}
109+
}
110+
}
111+
}

0 commit comments

Comments
 (0)
⚡