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 properties type object no definition false negatives #718

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
8 changes: 4 additions & 4 deletions packages/rulesets/generated/spectral/az-arm.js
Original file line number Diff line number Diff line change
Expand Up @@ -2351,15 +2351,14 @@ const errorMessageObject = "Properties with type:object that don't reference a m
const errorMessageNull = "Properties with 'type' NULL are not allowed, please specify the 'type' as 'Primitive' or 'Object' referring a model.";
const propertiesTypeObjectNoDefinition = (definitionObject, opts, ctx) => {
const path = ctx.path || [];
if ((definitionObject === null || definitionObject === void 0 ? void 0 : definitionObject.type) === "") {
return [{ message: errorMessageNull, path }];
}
if (definitionObject === null || definitionObject === void 0 ? void 0 : definitionObject.properties) {
if (definitionObject.properties === null) {
return [{ message: errorMessageNull, path }];
}
}
const values = Object.values(definitionObject);
if ((definitionObject === null || definitionObject === void 0 ? void 0 : definitionObject.type) === "") {
return [{ message: errorMessageNull, path }];
}
if (typeof definitionObject === "object") {
if (definitionObject.allOf) {
return;
Expand All @@ -2370,6 +2369,7 @@ const propertiesTypeObjectNoDefinition = (definitionObject, opts, ctx) => {
}
}
}
const values = Object.values(definitionObject);
for (const val of values) {
if (typeof val === "object") {
if (val === null) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,23 +1,23 @@
// If a property is defined with `'type': 'object'`, it must also define an object body.
// RPC Code: RPC-Policy-V1-03
const errorMessageObject =
"Properties with type:object that don't reference a model definition are not allowed. ARM doesn't allow generic type definitions as this leads to bad customer experience."
const errorMessageNull =
"Properties with 'type' NULL are not allowed, please specify the 'type' as 'Primitive' or 'Object' referring a model."

const errorMessageObject = "Properties with type:object that don't reference a model definition are not allowed. ARM doesn't allow generic type definitions as this leads to bad customer experience."
const errorMessageNull = "Properties with 'type' NULL are not allowed, please specify the 'type' as 'Primitive' or 'Object' referring a model."

export const propertiesTypeObjectNoDefinition: any = (definitionObject: any, opts: any, ctx: any) => {
const path = ctx.path || []

// return errorMessageNull if type or properties are null
AkhilaIlla marked this conversation as resolved.
Show resolved Hide resolved
if (definitionObject?.type === "") {
return [{ message: errorMessageNull, path }]
}

if (definitionObject?.properties) {
if (definitionObject.properties === null) {
return [{ message: errorMessageNull, path }]
}
}

const values = Object.values(definitionObject)
if (definitionObject?.type === "") {
return [{ message: errorMessageNull, path }]
}

if (typeof definitionObject === "object") {
if (definitionObject.allOf) {
return
Expand All @@ -29,6 +29,7 @@ export const propertiesTypeObjectNoDefinition: any = (definitionObject: any, opt
}
}

const values = Object.values(definitionObject)
for (const val of values) {
if (typeof val === "object") {
if (val === null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -487,3 +487,69 @@ test("PropertiesTypeObjectNoDefinition should find no errors for type allOf", ()
expect(results.length).toBe(0)
})
})

test("PropertiesTypeObjectNoDefinition should find errors when type:object is not defined", () => {
const oasDoc = {
swagger: "2.0",
info: {
version: "4.0",
title: "Common types",
},
paths: {},
definitions: {
VirtualMachineScaleSetExtension: {
properties: {
name: {
type: "string",
description: "The name of the extension.",
},
type: {
readOnly: true,
type: "string",
description: "Resource type",
},
properties: {
"x-ms-client-flatten": true,
$ref: "#/definitions/VirtualMachineScaleSetExtensionProperties",
},
},
description: "Describes a Virtual Machine Scale Set Extension.",
},
VirtualMachineScaleSetExtensionListResult: {
properties: {
value: {
type: "array",
items: {
$ref: "#/definitions/VirtualMachineScaleSetExtension",
},
description: "The list of VM scale set extensions.",
},
nextLink: {
type: "string",
description:
"The uri to fetch the next page of VM scale set extensions. Call ListNext() with this to fetch the next page of VM scale set extensions.",
},
},
required: ["value"],
description: "The List VM scale set extension operation response.",
},
VirtualMachineScaleSetExtensionProperties: {
properties: {
settings: {
type: "object",
description: "Json formatted public settings for the extension.",
},
},
},
},
}
return linter.run(oasDoc).then((results) => {
expect(results.length).toBe(3)
expect(results[0].path.join(".")).toBe("definitions.VirtualMachineScaleSetExtension.properties.properties.properties.settings"),
expect(results[0].message).toBe(errorMessageObject),
expect(results[1].path.join(".")).toBe("definitions.VirtualMachineScaleSetExtensionListResult.properties.value.items.properties.properties.properties.settings"),
expect(results[1].message).toBe(errorMessageObject),
expect(results[2].path.join(".")).toBe("definitions.VirtualMachineScaleSetExtensionProperties.properties.settings"),
expect(results[2].message).toBe(errorMessageObject)
})
})