Fix TopLevelResourcesListBySubscription to exclude tenant level API's… #808
Conversation
AkhilaIlla
requested review from
mikeharder,
raosuhas,
tejaswiMinnu and
weshaggard
as code owners
There was a problem hiding this comment.
Pull request overview
This PR fixes a regression in the TopLevelResourcesListBySubscription ARM rule that caused validation to stop as soon as any tenant-level API path was encountered, and adds tests to ensure correct behavior for tenant-only and mixed-scope specs.
Changes:
- Updated
TopLevelResourcesListBySubscriptionto:- Use full
ResourceInfoobjects instead of just names. - Skip validation when a spec has no subscription-scoped paths at all (including
x-ms-paths). - Only validate top-level resources that actually have subscription-scoped operations.
- De-duplicate validation per model name and ignore purely tenant-scoped resources.
- Use full
- Extended
composite-azure-testswith new test cases for:- Tenant-only specs.
- Mixed tenant and subscription specs missing list-by-subscription.
- Subscription-scoped paths defined under
x-ms-paths.
- Added three focused Swagger fixtures to model tenant-only, mixed tenant/subscription, and
x-ms-pathssubscription scenarios.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
packages/rulesets/src/native/legacyRules/TopLevelResourcesListBySubscription.ts |
Refines rule logic to correctly distinguish tenant-only specs, require list-by-subscription only for resources with subscription-scoped operations, and avoid duplicate reports per model. |
packages/rulesets/src/native/tests/composite-azure-tests.ts |
Adds targeted tests for tenant-only, mixed-scope, and x-ms-paths subscription scenarios and clarifies the existing compute test comment. |
packages/rulesets/src/native/tests/resources/armResource/topLevelListBySubscription_tenantOnlyResource.json |
New tenant-only Swagger fixture to verify that list-by-subscription validation is skipped when no subscription-scoped paths exist. |
packages/rulesets/src/native/tests/resources/armResource/topLevelListBySubscription_subscriptionInXmsPaths_missingCollection.json |
New Swagger fixture to ensure subscription-scoped paths under x-ms-paths still trigger list-by-subscription validation. |
packages/rulesets/src/native/tests/resources/armResource/topLevelListBySubscription_mixedTenantAndSubscription_missingCollection.json |
New Swagger fixture to confirm that mixed tenant and subscription specs still enforce list-by-subscription for subscription-scoped top-level resources. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return | ||
| } | ||
|
|
||
| // Skip validation if there is no subscription-scoped paths in the spec |
There was a problem hiding this comment.
Somehow this block causes the rule to terminate entirely when a spec contains ANY tenant-level API,
even if the spec also has subscription-scoped resources that need validation.
Subscription-scoped resources should be validated for list operations per resources that you are already handling on lines 39 - 42
There was a problem hiding this comment.
I dont understand.
line 23 checks for all the sub level paths in the spec.
line 24 is for early termination if there arent any sub paths.
I dont think this actually skips validating the sub paths., right?
AkhilaIlla
added
test-TopLevelResourcesListBySubscription
and removed
test-TopLevelResourcesListBySubscription
labels
The fix in the PR - Fix TopLevelResourcesListBySubscription to author tenant-level APIs by tejaswiMinnu · Pull Request … introduced a regression which makes the rule terminate even if a single tenant level API is encountered/present in the spec file.
The spec in this Typespec for Postgres Migration RP by nikhilhope3 · Pull Request #25493 · Azure/azure-rest-api-specs-pr has-
"/providers/Microsoft.DataMigration/operations"
making the rule to terminate and not validate further.
This PR addresses-https://msazure.visualstudio.com/One/_workitems/edit/36410442 and fixes the regression and also addresses the scenario to skip tenant level API's from validation.