network/2020-05-01: adding the missing web application firewall files #10384
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tombuildsstuff
requested review from
MikhailTryakhov and
v-shhatt
as code owners
|
You don't have permission to trigger SDK Automation. |
[Staging] Swagger Validation Report
️✔️ |
| Rule | Message |
|---|---|
|
Resource type naming must follow camel case. Path: '/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Network/ApplicationGatewayWebApplicationFirewallPolicies' New: Microsoft.Network/stable/2020-05-01/webapplicationfirewall.json#L37 |
|
Resource type naming must follow camel case. Path: '/subscriptions/{subscriptionId}/providers/Microsoft.Network/ApplicationGatewayWebApplicationFirewallPolicies' New: Microsoft.Network/stable/2020-05-01/webapplicationfirewall.json#L83 |
|
Resource type naming must follow camel case. Path: '/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Network/ApplicationGatewayWebApplicationFirewallPolicies/{policyName}' New: Microsoft.Network/stable/2020-05-01/webapplicationfirewall.json#L122 |
|
The integer type does not have a format, please add it. New: Microsoft.Network/stable/2020-05-01/webapplicationfirewall.json#L493 |
️️✔️Avocado [Detail]
️✔️Validation passes for Avocado.
️️✔️ModelValidation [Detail]
️✔️Validation passes for ModelValidation.
️️✔️SemanticValidation [Detail]
️✔️Validation passes for SemanticValidation.
|
@ArcturusZhang out of interest, shouldn't this breaking change to the Networking API be detected by the breaking change tooling? |
|
Can one of the admins verify this patch? |
|
@tombuildsstuff, there are a couple of linting errors, could you take a look? |
lirenhe
added
Reviewed-ChangesRequested
|
hey, better to use common az networking rules to prevent such cases... sync up with me related to it |
|
Hi @tombuildsstuff since this PR is adding some new APIs to network rather than replacing - therefore the breaking change CI will detect no breaking changes. |
To clarify - I meant in the PR which added API version 2020-05, where presumably the breaking change detector should have compared this with 2020-04 and detected this functionality is being removed in the new API version? |
|
Hi @tombuildsstuff unfortunately this is not the way that our breaking changes CI works. Our breaking change CI only check the breaking changes in the same api-version, it would not check the same thing between different api-versions. As for the LintDiff error, it is detected because one of the paths is not in camel cases. And the service team has decided that they would fix this issue in a future api-version - therefore we could temporarily ignore the LintDiff error. This is to say, they will change this path to in a new api-version. A potential impact is that after this change lands, the ID of the WAF resource will change (and as well as the |
...etwork/resource-manager/Microsoft.Network/stable/2020-05-01/examples/WafListAllPolicies.json
Outdated
Show resolved
Hide resolved
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
azure-sdk-for-js - Release
|
Azure CLI Extension Generation - Release
No readme.md specification configuration files were found that are associated with the files modified in this pull request, or swagger_to_sdk section in readme.md is not configured
|
azure-sdk-for-java - Release
|
azure-sdk-for-go - Release
|
azure-sdk-for-net - Release
|
azure-sdk-for-python - Release
- Breaking Change detected in SDK
|
azure-resource-manager-schemas - Release
|
azure-sdk-for-python-track2 - Release
- Breaking Change detected in SDK
|
Trenton Generation - Release
No readme.md specification configuration files were found that are associated with the files modified in this pull request, or swagger_to_sdk section in readme.md is not configured
|
This was referenced Aug 13, 2020
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
lirenhe
removed
WaitForARMFeedback
lirenhe
approved these changes
Aug 14, 2020
venkatsvpr
reviewed
Sep 2, 2020
| } | ||
| } | ||
| }, | ||
| "/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Network/applicationGatewayWebApplicationFirewallPolicies/{policyName}": { |
There was a problem hiding this comment.
Why is this in camelCasing since the api-name before and after are in PascalCasing?
There was a problem hiding this comment.
because the linter requires it to be so - I can update this so it's PascalCased, but it seems weird that this API doesn't comply with the linter everything else is using?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There's a large number of files in this Resource Provider - it appears this one was missed when copying over from 2020-04-01