-
Notifications
You must be signed in to change notification settings - Fork 5.1k
Adding vulnerability Assessemtn clinet code #4327
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
Conversation
|
@jaredmoo - please review too... |
jaredmoo
left a comment
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.
Overall looking pretty good :)
| /// Gets or sets the recurring scans settings | ||
| /// </summary> | ||
| [JsonProperty(PropertyName = "properties.recurringScans")] | ||
| public VulnerabilityAssessmentRecurringScansProperties RecurringScans { get; set; } |
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.
Do you not want to flatten these properties up into this class? Either way is fine, but I want to check it is a conscious decision :)
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.
It is a conscious decision.
I'll flatten these properties up in powershell
| /// parameter. | ||
| /// </summary> | ||
| [JsonProperty(PropertyName = "properties.storageContainerSasKey")] | ||
| public string StorageContainerSasKey { get; set; } |
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.
Are any of these properties required?
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.
Yep, will fix
|
|
||
| sqlClient.DatabaseVulnerabilityAssessmentRuleBaselines.CreateOrUpdate(resourceGroup.Name, server.Name, dbName, testRuleId, ruleBaselineToSet); | ||
|
|
||
| // Get basleine |
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.
typo here
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.
Same typo is copied a few times elsewhere
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.
Thanks, fixed.
| string scanId1 = string.Format("scantest2_{0}", DateTime.UtcNow.Ticks); | ||
| sqlClient.DatabaseVulnerabilityAssessmentScans.Execute(resourceGroup.Name, server.Name, dbName, scanId1); | ||
|
|
||
| // Veroify get scan and list scans |
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.
typo: Verify
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.
Thanks, fixed.
| await SetPolicy(context, sqlClient, resourceGroup, server, dbName); | ||
|
|
||
| // Run some scans | ||
| string scanId = string.Format("scantest1_{0}", DateTime.UtcNow.Ticks); |
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.
How is this going to work with test playback? UtcNow is different every time you run the test, so the scan uri is not reproducable
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.
Thanks, fixed.
|
|
||
| // Run some scans | ||
| string scanId = string.Format("scantest1_{0}", DateTime.UtcNow.Ticks); | ||
| sqlClient.DatabaseVulnerabilityAssessmentScans.Execute(resourceGroup.Name, server.Name, dbName, scanId); |
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.
Wasn't this changed to InitiateScan?
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.
yep, it was.
will change the client code and update the swagger in azure-rest-api-specs repo
|
@yaakoviyun Please fix failing test cases |
dsgouda
left a comment
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.
These look like additive changes. Please bump the minor version in csproj file along with PackageReleaseNotes
Added needed changes to Swagger and exampels Added needed changes to Swagger Azure/azure-sdk-for-net#4327 (review)
Added needed changes to Swagger and exampels Added needed changes to Swagger Azure/azure-sdk-for-net#4327 (review)
Added needed changes to Swagger and exampels Added needed changes to Swagger Azure/azure-sdk-for-net#4327 (review)
Added needed changes to Swagger and exampels Added needed changes to Swagger Azure/azure-sdk-for-net#4327 (review)
Turn ON database threat detection as a preprequite to use VA
|
@dsgouda - Jenkins build failure below seems like not our fault |
|
@dsgouda , can you merge? |
|
This PR was merged into #4351 and can be closed, |
* Adding Vulnerability Assessment APIs on managed instance Adding Vulnerability Assessment APIs on managed instance * fixed scan operation id * character mismatch fix * Added needed changes to Swagger and exampels Added needed changes to Swagger Azure/azure-sdk-for-net#4327 (review) * Delete managedDatabaseVulnerabilityAssessmentBaselines.json * Delete managedDatabaseVulnerabilityAssessmentScans.json * Delete managedDatabaseVulnerabilityAssessments.json * Delete ManagedDatabaseVulnerabilityAssessmentCreateMax.json * Delete ManagedDatabaseVulnerabilityAssessmentCreateMin.json * Delete ManagedDatabaseVulnerabilityAssessmentDelete.json * Delete ManagedDatabaseVulnerabilityAssessmentGet.json * Delete ManagedDatabaseVulnerabilityAssessmentRuleBaselineCreate.json * Delete ManagedDatabaseVulnerabilityAssessmentRuleBaselineDelete.json * Delete ManagedDatabaseVulnerabilityAssessmentRuleBaselineGet.json * Delete ManagedDatabaseVulnerabilityAssessmentScanExport.json * Delete ManagedDatabaseVulnerabilityAssessmentScanRecordsGet.json * Delete ManagedDatabaseVulnerabilityAssessmentScanRecordsListByDatabase.json * Delete ManagedDatabaseVulnerabilityAssessmentScansExecute.json * Update readme.md
Added needed changes to Swagger and exampels Added needed changes to Swagger Azure/azure-sdk-for-net#4327 (review)
Added needed changes to Swagger and exampels Added needed changes to Swagger Azure/azure-sdk-for-net#4327 (review)
Adding vulnerability Assessemtn clinet code