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.

Sign up for GitHub

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

Jump to bottom

New security Regulatory compliance API #5183

Merged
merged 16 commits into from
Mar 25, 2019
Merged

New security Regulatory compliance API #5183

merged 16 commits into from
Mar 25, 2019

Conversation

himarkov
Copy link
Contributor

@himarkov himarkov commented Feb 10, 2019 •
edited
Loading

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:

  • I have reviewed the documentation for the workflow.
  • Validation tools were run on swagger spec(s) and have all been fixed in this PR.
  • The OpenAPI Hub was used for checking validation status and next steps.

ARM API Review Checklist

  • Service team MUST add the "WaitForARMFeedback" label if the management plane API changes fall into one of the below categories.
  • adding/removing APIs.
  • adding/removing properties.
  • adding/removing API-version.
  • adding a new service in Azure.

Failure to comply may result in delays for manifest application. Note this does not apply to data plane APIs.

  • If you are blocked on ARM review and want to get the PR merged urgently, please get the ARM oncall for reviews (RP Manifest Approvers team under Azure Resource Manager service) from IcM and reach out to them.
    Please follow the link to find more details on API review process.

@msftclas
Copy link

msftclas commented Feb 10, 2019 •
edited
Loading

CLA assistant check
All CLA requirements met.

@AutorestCI
Copy link

AutorestCI commented Feb 10, 2019 •
edited
Loading

Automation for azure-sdk-for-ruby

Nothing to generate for azure-sdk-for-ruby

@AutorestCI
Copy link

AutorestCI commented Feb 10, 2019 •
edited
Loading

Automation for azure-sdk-for-python

The initial PR has been merged into your service PR:
Azure/azure-sdk-for-python#3782

@AutorestCI
Copy link

AutorestCI commented Feb 10, 2019 •
edited
Loading

Automation for azure-sdk-for-js

A 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:
Azure/azure-sdk-for-js#1765

@AutorestCI
Copy link

AutorestCI commented Feb 10, 2019 •
edited
Loading

Automation for azure-sdk-for-java

Nothing to generate for azure-sdk-for-java

@AutorestCI
Copy link

AutorestCI commented Feb 10, 2019 •
edited
Loading

Automation for azure-sdk-for-go

The initial PR has been merged into your service PR:
Azure/azure-sdk-for-go#4374

@AutorestCI
Copy link

AutorestCI commented Feb 10, 2019 •
edited
Loading

Automation for azure-sdk-for-node

A 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:
Azure/azure-sdk-for-node#5044

@chlahav chlahav self-requested a review February 10, 2019 17:08
@chlahav chlahav added the WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required label Feb 10, 2019
@azuresdkci
Copy link
Contributor

Can one of the admins verify this patch?

jhendrixMSFT
jhendrixMSFT reviewed Feb 11, 2019
View reviewed changes
Copy link
Member

@jhendrixMSFT jhendrixMSFT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A lot of the descriptions for the new operation IDs look to be copy/pasted. Please consider updating all of the descriptions with accurate content as they are used in doc generation.

jhendrixMSFT
jhendrixMSFT requested changes Feb 11, 2019
View reviewed changes
Copy link
Member

@jhendrixMSFT jhendrixMSFT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your additions are malformed JSON which is causing the failure here.

nschonni
nschonni reviewed Feb 12, 2019
View reviewed changes
specification/security/resource-manager/Microsoft.Security/stable/2019-01-01/security.json Outdated Show resolved Hide resolved
specification/security/resource-manager/Microsoft.Security/stable/2019-01-01/security.json Outdated Show resolved Hide resolved
specification/security/resource-manager/Microsoft.Security/stable/2019-01-01/security.json Outdated Show resolved Hide resolved
specification/security/resource-manager/Microsoft.Security/stable/2019-01-01/security.json Outdated Show resolved Hide resolved
specification/security/resource-manager/Microsoft.Security/stable/2019-01-01/security.json Outdated Show resolved Hide resolved
specification/security/resource-manager/Microsoft.Security/stable/2019-01-01/security.json Outdated Show resolved Hide resolved
specification/security/resource-manager/Microsoft.Security/stable/2019-01-01/security.json Outdated Show resolved Hide resolved
specification/security/resource-manager/Microsoft.Security/stable/2019-01-01/security.json Outdated Show resolved Hide resolved
specification/security/resource-manager/Microsoft.Security/stable/2019-01-01/security.json Outdated Show resolved Hide resolved
@himarkov
Copy link
Contributor Author

@jhendrixMSFT requested changes were handled.
Thanks.

ravbhatnagar
ravbhatnagar reviewed Feb 19, 2019
View reviewed changes
Copy link
Contributor

@ravbhatnagar ravbhatnagar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

few comments below

.../2019-01-01/examples/RegulatoryCompliance/getRegulatoryComplianceAssessmentList_example.json Outdated Show resolved Hide resolved
specification/security/resource-manager/Microsoft.Security/stable/2019-01-01/security.json Outdated Show resolved Hide resolved
specification/security/resource-manager/Microsoft.Security/stable/2019-01-01/security.json Outdated Show resolved Hide resolved
specification/security/resource-manager/Microsoft.Security/stable/2019-01-01/security.json Outdated Show resolved Hide resolved
@ravbhatnagar ravbhatnagar added the ARMChangesRequested <valid label in PR review process>add this label when require changes after ARM review label Feb 19, 2019
@AutorestCI AutorestCI mentioned this pull request Feb 19, 2019
Closed
@ravbhatnagar
Copy link
Contributor

Looks good, except that just add the description for the filter. Signing off from ARM side.

@ravbhatnagar ravbhatnagar added ARMSignedOff <valid label in PR review process>add this label when ARM approve updates after review and removed ARMChangesRequested <valid label in PR review process>add this label when require changes after ARM review WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required labels Feb 20, 2019
@chlahav chlahav added the DoNotMerge <valid label in PR review process> use to hold merge after approval label Feb 20, 2019
jhendrixMSFT
jhendrixMSFT requested changes Feb 20, 2019
View reviewed changes
Copy link
Member

@jhendrixMSFT jhendrixMSFT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also fix the formatting of your additions as some of the whitespacing is incorrect.
In addition, I think your List APIs should have the x-ms-pageable attribute.

specification/security/resource-manager/Microsoft.Security/stable/2019-01-01/security.json Outdated Show resolved Hide resolved
specification/security/resource-manager/Microsoft.Security/stable/2019-01-01/security.json Outdated Show resolved Hide resolved
specification/security/resource-manager/Microsoft.Security/stable/2019-01-01/security.json Outdated Show resolved Hide resolved
@AutorestCI AutorestCI mentioned this pull request Feb 24, 2019
Closed
@jhendrixMSFT
Copy link
Member

@himarkov can you please resolve the merge conflict?

@jhendrixMSFT
Copy link
Member

@himarkov hello any update?

@himarkov
Copy link
Contributor Author

@jhendrixMSFT Hi, Sorry for my delay!
We're indeed waiting for the feature to be deployed and for everything to be ready in Prod before merging,.
Regarding the readme.md, i've now updated it as well.
Thank you!

This was referenced Mar 20, 2019
Closed
Closed
adxsdknet added a commit to adxsdknet/azure-sdk-for-net that referenced this pull request Mar 20, 2019
@adxsdknet
.NET SDK Resource Provider:'SecurityCenter'
320d167 
REST Spec PR 'Azure/azure-rest-api-specs#5183'
REST Spec PR Author 'himarkov'
REST Spec PR Last commit
@AutorestCI AutorestCI mentioned this pull request Mar 20, 2019
Merged
adxsdknet added a commit to adxsdknet/azure-sdk-for-net that referenced this pull request Mar 20, 2019
@adxsdknet
.NET SDK Resource Provider:'SecurityCenter'
735b073 
REST Spec PR 'Azure/azure-rest-api-specs#5183'
REST Spec PR Author 'himarkov'
REST Spec PR Last commit
@Azure Azure deleted a comment from adxsdknet Mar 20, 2019
@Azure Azure deleted a comment from adxsdknet Mar 20, 2019
@Azure Azure deleted a comment from adxsdknet Mar 20, 2019
@Azure Azure deleted a comment from adxsdknet Mar 20, 2019
@Azure Azure deleted a comment from adxsdknet Mar 20, 2019
@Azure Azure deleted a comment from adxsdknet Mar 20, 2019
@chlahav chlahav removed the DoNotMerge <valid label in PR review process> use to hold merge after approval label Mar 24, 2019
@himarkov
Copy link
Contributor Author

@jhendrixMSFT Hi,
We are ready for merge now :)
Thanks!

@jhendrixMSFT jhendrixMSFT merged commit e6dd183 into Azure:master Mar 25, 2019
This was referenced Mar 25, 2019
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ravbhatnagar ravbhatnagar ravbhatnagar left review comments

@nschonni nschonni nschonni left review comments

@jhendrixMSFT jhendrixMSFT jhendrixMSFT approved these changes

@chlahav chlahav Awaiting requested review from chlahav

Assignees

@jhendrixMSFT jhendrixMSFT

Labels
ARMSignedOff <valid label in PR review process>add this label when ARM approve updates after review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@himarkov @msftclas @AutorestCI @azuresdkci @ravbhatnagar @jhendrixMSFT @nschonni @chlahav