Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Akhilailla/fix false alarm for resource name restriction. #645

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
AkhilaIlla marked this conversation as resolved.
Show resolved Hide resolved
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)
})
})