-
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
Assessment metadata API (Azure Security Center) #7622
Conversation
azure-sdk-for-python - Release⌛ Pending...
|
Can one of the admins verify this patch? |
Automation for azure-sdk-for-goA PR has been created for you based on this PR content. Once this PR will be merged, content will be added to your service PR: |
...-01-01-preview/examples/AssessmentsMetadata/GetAssessmentsMetadata_subscription_example.json
Outdated
Show resolved
Hide resolved
.../preview/2019-01-01-preview/examples/AssessmentsMetadata/GetAssessmentsMetadata_example.json
Outdated
Show resolved
Hide resolved
...-01-preview/examples/AssessmentsMetadata/CreateAssessmentsMetadata_subscription_example.json
Outdated
Show resolved
Hide resolved
...-01-preview/examples/AssessmentsMetadata/CreateAssessmentsMetadata_subscription_example.json
Outdated
Show resolved
Hide resolved
...-01-preview/examples/AssessmentsMetadata/CreateAssessmentsMetadata_subscription_example.json
Outdated
Show resolved
Hide resolved
...-01-preview/examples/AssessmentsMetadata/CreateAssessmentsMetadata_subscription_example.json
Outdated
Show resolved
Hide resolved
...preview/2019-01-01-preview/examples/AssessmentsMetadata/ListAssessmentsMetadata_example.json
Show resolved
Hide resolved
...urity/resource-manager/Microsoft.Security/preview/2019-01-01-preview/assessmentMetadata.json
Outdated
Show resolved
Hide resolved
Automation for azure-sdk-for-pythonA PR has been created for you based on this PR content. Once this PR will be merged, content will be added to your service PR: |
@yungezz I've tidied the PR - please review |
@yungezz can you please review? |
...urity/resource-manager/Microsoft.Security/preview/2019-01-01-preview/assessmentMetadata.json
Outdated
Show resolved
Hide resolved
} | ||
}, | ||
"paths": { | ||
"/providers/Microsoft.Security/assessmentMetadata": { |
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 are you handling RBAC of your tenant level API?
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 tenant level API will be available for all to get the built in assessment metadata (similar to roleDefinitions or policyDefinitions)
} | ||
}, | ||
"/providers/Microsoft.Security/assessmentMetadata/{assessmentMetadataName}": { | ||
"get": { |
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.
Can these be created or only retrieved?
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.
tenant level can only be retrieved (built ins)
...urity/resource-manager/Microsoft.Security/preview/2019-01-01-preview/assessmentMetadata.json
Show resolved
Hide resolved
...urity/resource-manager/Microsoft.Security/preview/2019-01-01-preview/assessmentMetadata.json
Show resolved
Hide resolved
...-01-preview/examples/AssessmentsMetadata/CreateAssessmentsMetadata_subscription_example.json
Show resolved
Hide resolved
} | ||
}, | ||
"paths": { | ||
"/providers/Microsoft.Security/assessmentMetadata": { |
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.
assessmentMetadata doesn't really allude to what this is, as the term metadata doesn't really have any specific meaning. If I try to make sense of both your PRs, this is really the assessmentDefinitions?
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.
I guess that it might be a better name, but since this API was already reviewed for most of its properties by ARM, approved with that name and publicly exposed in the manifest it will be a little problematic to change the name now :/
WDYT?
9f724b4
to
00eab4b
Compare
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.
ARM feedback appears to be addressed. Signing off
@yungezz |
hi @erich-wang could you pls review the PR since ARM signedofff? |
azure-sdk-for-go - Release⌛ Pending...
|
azure-sdk-for-js - Release⌛ Pending...
|
azure-sdk-for-net - Release⌛ Pending...
|
azure-sdk-for-java - Release⌛ Pending...
|
* create assessmentMetadata.json * prettier fix * review fixes * add userImpact, implementationEffort, threat fields to assessmentMetadata * cleanup readme.md * Fix property name * prettier fixes * Property casing fix
Review summary:
Picked up from this PR:
Latest improvements:
MSFT employees can try out our new experience at OpenAPI Hub - one location for using our validation tools and finding your workflow.
Contribution checklist:
ARM API Review Checklist
Failure to comply may result in delays for manifest application. Note this does not apply to data plane APIs.
Please follow the link to find more details on API review process.