-
Notifications
You must be signed in to change notification settings - Fork 5.1k
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
Add On Premise Sql Resource Details #10041
Changes from all commits
286f25e
d9cb46a
d90e611
aac399d
c61f867
b7597a2
82437d5
ae0a37f
826ef09
5e0323c
d5fb80c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -169,7 +169,8 @@ | |
"description": "The platform where the assessed resource resides", | ||
"enum": [ | ||
"Azure", | ||
"OnPremise" | ||
"OnPremise", | ||
"OnPremiseSql" | ||
], | ||
"x-ms-enum": { | ||
"name": "source", | ||
|
@@ -182,6 +183,10 @@ | |
{ | ||
"value": "OnPremise", | ||
"description": "Resource in an on premise machine connected to Azure cloud" | ||
}, | ||
{ | ||
"value": "OnPremiseSql", | ||
"description": "SQL Resource in an on premise machine connected to Azure cloud" | ||
} | ||
] | ||
} | ||
|
@@ -242,6 +247,30 @@ | |
"machineName" | ||
] | ||
}, | ||
"OnPremiseSqlResourceDetails": { | ||
"type": "object", | ||
"description": "Details of the On Premise Sql resource that was assessed", | ||
"x-ms-discriminator-value": "OnPremiseSql", | ||
"allOf": [ | ||
{ | ||
"$ref": "#/definitions/OnPremiseResourceDetails" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Make sure this inherits correctly, and doesn't just defaults to base type There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I implemented it this way after seeing similar examples, for example : The SDK that generated from the swagger - |
||
} | ||
], | ||
"properties": { | ||
"serverName": { | ||
"type": "string", | ||
"description": "The Sql server name installed on the machine" | ||
}, | ||
"databaseName": { | ||
"type": "string", | ||
"description": "The Sql database name installed on the machine" | ||
} | ||
}, | ||
"required": [ | ||
"serverName", | ||
"databaseName" | ||
] | ||
}, | ||
"AzureResourceLinks": { | ||
"description": "array of azure resource IDs", | ||
"readOnly": true, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PR appears to be modifying some definitions shared across security swaggers. Which API versions will this affect?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Microsoft.Security/subassessments 2019-01-01-preview and 2020-01-01
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your change would effectively alter the behavior of 2 API versions. As per https://github.com/microsoft/api-guidelines/blob/vNext/azure/Guidelines.md, this is not allowed. You should bump the API version and only allow the new enum values in that API version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My change doesn’t affect the API behavior – I didn’t change any of the current fields (not rename any field and not removed any field) I only added a new option for a new value that field can get, this field already exists.