Skip to content

Commit

Permalink
Akhilailla/fix false alarm for resource name restriction. (#645)
Browse files Browse the repository at this point in the history
* Fix ResourceNameRestriction rule to exclude validation for system-defined parameters

* Add additional changelog file for latestVersionOfCommonTypesMustBeUsed rule

---------

Co-authored-by: akhilailla <[email protected]>
  • Loading branch information
AkhilaIlla and akhilailla authored Jan 16, 2024
1 parent 18b20eb commit 2873e67
Show file tree
Hide file tree
Showing 7 changed files with 71 additions and 10 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"changes": [
{
"packageName": "@microsoft.azure/openapi-validator-rulesets",
"comment": "Fix ResourceNameRestriction rule to exclude validation for system-defined parameters",
"type": "patch"
}
],
"packageName": "@microsoft.azure/openapi-validator-rulesets"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"changes": [
{
"packageName": "@microsoft.azure/openapi-validator-rulesets",
"comment": "Update latestVersionOfCommonTypesMustBeUsed rule to check for latest version(V5) of all common types",
"type": "patch"
}
],
"packageName": "@microsoft.azure/openapi-validator-rulesets"
}
12 changes: 7 additions & 5 deletions packages/rulesets/generated/spectral/az-arm.js
Original file line number Diff line number Diff line change
Expand Up @@ -175,9 +175,10 @@ const mutabilityWithReadOnly = (prop, _opts, ctx) => {
const LATEST_VERSION_BY_COMMON_TYPES_FILENAME = new Map([
["types.json", "v5"],
["managedidentity.json", "v5"],
["privatelinks.json", "v4"],
["customermanagedkeys.json", "v4"],
["managedidentitywithdelegation.json", "v4"],
["privatelinks.json", "v5"],
["customermanagedkeys.json", "v5"],
["managedidentitywithdelegation.json", "v5"],
["mobo.json", "v5"],
]);
function isLatestCommonTypesVersionForFile(version, fileName) {
return LATEST_VERSION_BY_COMMON_TYPES_FILENAME.get(fileName) === version.toLowerCase();
Expand Down Expand Up @@ -1832,7 +1833,7 @@ const noDuplicatePathsForScopeParameter = (path, _opts, ctx) => {
if (path === null || typeof path !== "string" || path.length === 0 || swagger === null) {
return [];
}
const pathRegEx = new RegExp(path.replace(scopeParameter, ".*"));
const pathRegEx = new RegExp(path.replace(scopeParameter, ".*").concat("$"));
const otherPaths = Object.keys(swagger.paths).filter((p) => p !== path);
const matches = otherPaths.filter((p) => pathRegEx.test(p));
const errors = matches.map((match) => {
Expand Down Expand Up @@ -2557,6 +2558,7 @@ const reservedResourceNamesModelAsEnum = (pathItem, _opts, ctx) => {
return errors;
};

const EXCEPTION_LIST = ["resourceGroupName", "privateEndpointConnectionName", "managementGroupName"];
const resourceNameRestriction = (paths, _opts, ctx) => {
if (paths === null || typeof paths !== "object") {
return [];
Expand All @@ -2581,7 +2583,7 @@ const resourceNameRestriction = (paths, _opts, ctx) => {
var _a;
if (v.includes("}")) {
const param = (_a = v.match(/[^{}]+(?=})/)) === null || _a === void 0 ? void 0 : _a[0];
if ((param === null || param === void 0 ? void 0 : param.match(/^\w+Name+$/)) && param !== "resourceGroupName") {
if ((param === null || param === void 0 ? void 0 : param.match(/^\w+Name+$/)) && !EXCEPTION_LIST.includes(param)) {
const paramDefinition = getPathParameter(paths[pathKey], param);
if (paramDefinition && !paramDefinition.pattern) {
errors.push({
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
// Check conformance to Azure parameter naming conventions:

//system-defined parameters => needs to be excluded from validation
const EXCEPTION_LIST = ["resourceGroupName", "privateEndpointConnectionName", "managementGroupName"]
export const resourceNameRestriction = (paths: any, _opts: any, ctx: any) => {
if (paths === null || typeof paths !== "object") {
return []
Expand Down Expand Up @@ -27,7 +29,7 @@ export const resourceNameRestriction = (paths: any, _opts: any, ctx: any) => {
if (v.includes("}")) {
const param = v.match(/[^{}]+(?=})/)?.[0]
// Get the preceding path segment
if (param?.match(/^\w+Name+$/) && param !== "resourceGroupName") {
if (param?.match(/^\w+Name+$/) && !EXCEPTION_LIST.includes(param)) {
const paramDefinition = getPathParameter(paths[pathKey], param)
if (paramDefinition && !paramDefinition.pattern) {
errors.push({
Expand Down
7 changes: 4 additions & 3 deletions packages/rulesets/src/spectral/functions/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,10 @@ import _ from "lodash"
export const LATEST_VERSION_BY_COMMON_TYPES_FILENAME = new Map([
["types.json", "v5"],
["managedidentity.json", "v5"],
["privatelinks.json", "v4"],
["customermanagedkeys.json", "v4"],
["managedidentitywithdelegation.json", "v4"],
["privatelinks.json", "v5"],
["customermanagedkeys.json", "v5"],
["managedidentitywithdelegation.json", "v5"],
["mobo.json", "v5"],
])

export function isLatestCommonTypesVersionForFile(version: string, fileName: string) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ test("LatestVersionOfCommonTypesMustBeUsed should find errors for obsolete versi
return nonResolvingLinter.run(myOpenApiDocument).then((results) => {
expect(results.length).toBe(3)
expect(results[0].path.join(".")).toBe("paths./foo.get.parameters.0.$ref")
expect(results[0].message).toContain("Use the latest version v4 of customermanagedkeys.json.")
expect(results[0].message).toContain("Use the latest version v5 of customermanagedkeys.json.")
expect(results[1].path.join(".")).toBe("paths./foo.get.parameters.1.$ref")
expect(results[1].message).toContain("Use the latest version v5 of managedidentity.json.")
expect(results[2].path.join(".")).toBe("paths./foo.get.responses.200.$ref")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,3 +108,39 @@ test("ResourceNameRestriction should find no errors", () => {
expect(results.length).toBe(0)
})
})

test("ResourceNameRestriction should find no errors for system-defined variables", () => {
const oasDoc = {
swagger: "2.0",
paths: {
"/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/My.NS/foo/{fooName}/privateEndpointConnections/{privateEndpointConnectionName}":
{
parameters: [
{
name: "fooName",
in: "path",
required: true,
type: "string",
pattern: "[a-zA-Z_0-9]+",
"x-ms-parameter-location": "method",
},
{
name: "privateEndpointConnectionName",
in: "path",
required: true,
type: "string",
description: "The name of the private endpoint connection associated with the Azure resource.",
"x-ms-parameter-location": "method",
},
],
get: {
parameters: [],
responses: {},
},
},
},
}
return linter.run(oasDoc).then((results) => {
expect(results.length).toBe(0)
})
})

0 comments on commit 2873e67

Please sign in to comment.