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

Swagger API for Azure CDN WebApplicationFirewallPolicy resource #5885

Merged
merged 119 commits into from
Jun 12, 2019
Merged

Swagger API for Azure CDN WebApplicationFirewallPolicy resource #5885

merged 119 commits into from
Jun 12, 2019

Conversation

sushantsingh
Copy link
Contributor

@sushantsingh sushantsingh commented May 7, 2019

This PR includes the following changes

Adds

  1. Added the swagger api spec for CdnWebApplicationFirewallPolicy which is a new (public preview) arm resource under the Microsoft.Cdn provider
  2. Added the relevant examples

Edits

  1. To integrate this with Azure CDN, a new property "webApplicationFirewallPolicyLink" is added to the "Endpoint" sub resource in cdn.json arm resource. This new property is the means by which a "CdnWebApplicationFirewallPolicy" can be references in a cdn "Endpoint" resource.
  2. All the examples related to the "Endpoint" resource in cdn.json were edited to include this new property
  3. Readme.md, a new package was added for this new change

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.

@sushantsingh
Copy link
Contributor Author

Hi @devigned do you have any other pending concerns?

@devigned
Copy link
Member

devigned commented Jun 7, 2019

@sushantsingh please resolve the build issue. I know it is from a previous example, but it's not a good reason to keep propagating incorrect examples. Thank you!

@sushantsingh
Copy link
Contributor Author

sushantsingh commented Jun 10, 2019

@devigned Sure I can try to fix the build issue but I have no idea what it means (I did try my best to reduce the error count due to existing issues by fixing everything I could understand).
Can you please help me understand what "message: Cannot read property 'readUInt8' of null" means?

Validating "examples" and "x-ms-examples" in specification/cdn/resource-manager/Microsoft.Cdn/preview/2019-06-15-preview/cdn.json:

error : ‌
operationId: CustomDomains_EnableCustomHttps
scenario: CustomDomains_EnableCustomHttpsUsingYourOwnCertificate
source: request
responseCode: ALL
severity: 0
code: INTERNAL_ERROR
details:
message: Cannot read property 'readUInt8' of null
code: INTERNAL_ERROR

…06-15-preview/cdnwebapplicationfirewall.json

Co-Authored-By: Nick Schonning <nschonni@gmail.com>
…06-15-preview/cdnwebapplicationfirewall.json

Co-Authored-By: Nick Schonning <nschonni@gmail.com>
@devigned devigned self-requested a review June 11, 2019 21:20
Copy link
Member

@devigned devigned left a comment

Choose a reason for hiding this comment

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

Approved with the understanding that subscription level get will be added and cacheDuration will be converted to a ISO duration rather than a string before a stable release.

@devigned
Copy link
Member

devigned commented Jun 11, 2019

@KrisBash are you good for preview? @sushantsingh and I chatted and we have commitment to add a subscription level get, improved documentation for models / operations, and to start a conversation about migrating cache duration to an ISO duration rather than string for the stable release.

@sushantsingh
Copy link
Contributor Author

sushantsingh commented Jun 12, 2019

Hi folks, this needs to be merged soon or we risk not meeting the preview release date of Jun 15th. Please let me know if anything is needed from our end to expedite things.
@KrisBash @amarzavery @devigned

@amarzavery amarzavery requested a review from veronicagg June 12, 2019 21:43
Copy link
Contributor

@KrisBash KrisBash left a comment

Choose a reason for hiding this comment

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

Signing off with understanding that subscription level GET will be added for stable API version

@KrisBash KrisBash 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 Jun 12, 2019
@amarzavery amarzavery merged commit a7485ff into Azure:master Jun 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ARMSignedOff <valid label in PR review process>add this label when ARM approve updates after review
Projects
None yet
Development

Successfully merging this pull request may close these issues.