Adding VirtualWan Swagger #3204

priyanag · 2018-06-08T23:28:22Z

This checklist is used to make sure that common issues in a pull request are addressed. This will expedite the process of getting your pull request merged and avoid extra work on your part to fix issues discovered during the review process.

PR information

The title of the PR is clear and informative.
There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For information on cleaning up the commits in your pull request, see this page.
Except for special cases involving multiple contributors, the PR is started from a fork of the main repository, not a branch.
If applicable, the PR references the bug/issue that it fixes.
Swagger files are correctly named (e.g. the api-version in the path should match the api-version in the spec).

Quality of Swagger

I have read the contribution guidelines.
My spec meets the review criteria:
- The spec conforms to the Swagger 2.0 specification.
- The spec follows the guidelines described in the Swagger checklist document.
- Validation tools were run on swagger spec(s) and have all been fixed in this PR.

AutorestCI · 2018-06-08T23:39:46Z

Automation for azure-libraries-for-java

A PR has been created for you:
AutorestCI/azure-libraries-for-java#237

AutorestCI · 2018-06-08T23:46:54Z

Automation for azure-sdk-for-node

Encountered a Subprocess error: (azure-sdk-for-node)

Command: ['/usr/local/bin/autorest', '/tmp/tmpwzd1cpo0/rest/specification/network/resource-manager/readme.md', '--license-header=MICROSOFT_MIT_NO_VERSION', '--node-sdks-folder=/tmp/tmpwzd1cpo0/sdk', '--nodejs', '--use=@microsoft.azure/autorest.nodejs@^2.1.43']
Finished with return code 1
and output:

AutoRest code generation utility [version: 2.0.4262; node: v7.10.1]
(C) 2018 Microsoft Corporation.
https://aka.ms/autorest

There is a new version of AutoRest available (2.0.4280).
 > You can install the newer version with with npm install -g autorest@latest

   Loading AutoRest core      '/root/.autorest/@microsoft.azure_autorest-core@2.0.4278/node_modules/@microsoft.azure/autorest-core/dist' (2.0.4278)
   Loading AutoRest extension '@microsoft.azure/autorest.nodejs' (^2.1.43->2.1.59)
   Loading AutoRest extension '@microsoft.azure/autorest.modeler' (2.3.44->2.3.44)
Shutting Down
Shutting Down
FATAL: nodejs/generate - FAILED
FATAL: Error: [Exception] AutoRest extension '@microsoft.azure/autorest.nodejs' terminated.
Process() cancelled due to exception : [Exception] AutoRest extension '@microsoft.azure/autorest.nodejs' terminated.

AutorestCI · 2018-06-08T23:55:29Z

Automation for azure-sdk-for-python

A PR has been created for you:
Azure/azure-sdk-for-python#2756

AutorestCI · 2018-06-08T23:56:22Z

Automation for azure-sdk-for-ruby

Nothing to generate for azure-sdk-for-ruby

priyanag · 2018-06-09T00:10:38Z

This is the original PR that was signed off by ARM team and the Azure-Rest-APi-Specs reviewer: https://github.com/Azure/azure-rest-api-specs-pr/pull/372

AutorestCI · 2018-06-09T03:02:31Z

Automation for azure-sdk-for-go

A PR has been created for you:
Azure/azure-sdk-for-go#2091

MikhailTryakhov · 2018-06-09T05:07:09Z

@priyanag please review error in CI

MikhailTryakhov · 2018-06-12T05:38:54Z

@priyanag there are still issues in CI. please ping me when they all would pass.
Please note that expected code freeze date is June 13

ravbhatnagar

@priyanag - This has already been reviewed long back, correct? I am assuming this is just putting the APis in public repo? I took a quick glance and it looked ok. Please let me know if there is some delta between what was last reviewed or any specific areas you would like me to focus on. Otherwise looks fine. Just one minor note.

ravbhatnagar · 2018-06-12T19:09:19Z

specification/network/resource-manager/Microsoft.Network/stable/2018-06-01/virtualWan.json

+        }
+      }
+    },
+    "/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Network/virtualWans/{virtualWANName}/vpnConfiguration": {


action name not indicative of an action. Should it be something like listvpnSiteConfiguration etc.?

Yes, this has already been reviewed and signed-off by ARM few months, now just making this public. Here is the PR that was signed off by ARM: https://github.com/Azure/azure-rest-api-specs-pr/pull/372

By action-name are you referring to the operationId?

ravbhatnagar · 2018-06-12T19:09:58Z

specification/network/resource-manager/Microsoft.Network/stable/2018-06-01/virtualWan.json

+        }
+      }
+    },
+    "GetVpnSitesConfigurationRequest": {


great, and now there will be linked access check on the vpnSites passed in the body?

jianghaolu

virtualwan.json is an invalid JSON - please fix it first so that I can wrong further validations.

jianghaolu · 2018-06-12T19:23:47Z

specification/network/resource-manager/Microsoft.Network/stable/2018-06-01/virtualWan.json

+              "$ref": "#/definitions/VpnConnection"
+            },
+            "description": "Parameters supplied to create or Update a VPN Connection."
+          },


Remove the extra ','

jianghaolu · 2018-06-12T21:38:46Z

.../network/resource-manager/Microsoft.Network/stable/2018-06-01/examples/VpnConnectionPut.json

+            "sharedKey": "key"
+          }
+        },
+        "202": {


"202" is not defined as a valid response for this operation.

Also sample response for 201 is missing.

Yes, we don't want 202 response. Only 200, 201 and default response.

jianghaolu · 2018-06-12T21:40:06Z

.../network/resource-manager/Microsoft.Network/stable/2018-06-01/examples/VpnConnectionPut.json

@@ -0,0 +1,42 @@
+{
+    "parameters": {


Missing required parameter "connectionName".

jianghaolu · 2018-06-12T21:44:47Z

...twork/resource-manager/Microsoft.Network/stable/2018-06-01/examples/VpnConnectionDelete.json

@@ -0,0 +1,14 @@
+{
+    "parameters": {


Required parameter "connectionName" missing.

EvgenyAgafonchikov · 2018-06-13T08:54:54Z

specification/network/resource-manager/readme.md

@@ -60,6 +60,7 @@ input-file:
 - Microsoft.Network/stable/2018-06-01/virtualNetworkGateway.json
 - Microsoft.Network/stable/2018-06-01/vmssNetworkInterface.json
 - Microsoft.Network/stable/2018-06-01/vmssPublicIpAddress.json
+- Microsoft.Network/stable/2018-06-01/virtualWan.json


should we sort this alphabetically? @jianghaolu, please confirm if so.

I don't think it's necessary though if it seems more organized to service team then it would be nice to have.

EvgenyAgafonchikov · 2018-06-13T09:59:21Z

Suppression in readme.md need to be updated to suppress "RequiredPropertiesMissingInResourceModel" errors in the added file.
Also, please fix warnings of kind "model/property lacks 'description' and 'title' property. Consider adding a 'description'/'title' element"
You may use azure-rest-api-specs\specification\network\resource-manager>autorest --azure-validator --openapi-type=arm to check the above.

Please also run oav validate-example Microsoft.Network\stable\2018-06-01\virtualWan.json and fix listed examples issues.

EvgenyAgafonchikov · 2018-06-13T10:01:19Z

specification/network/resource-manager/Microsoft.Network/stable/2018-06-01/virtualWan.json

+    },
+    "/subscriptions/{subscriptionId}/providers/Microsoft.Network/virtualWans": {
+      "get": {
+        "operationId": "VirtualWANs_ListBySubscriptionId",


@jianghaolu, is it common naming for now? As I remember previously it was ListByResourceGroup+List pair. Was this changed recently?

No this should be `VirtualWANs_List". Good catch!

EvgenyAgafonchikov · 2018-06-13T10:16:53Z

specification/network/resource-manager/Microsoft.Network/stable/2018-06-01/virtualWan.json

+        },
+        "nextLink": {
+          "type": "string",
+          "des


sorry, unable to figure out where?

provisisoning -> provisioning

EvgenyAgafonchikov · 2018-06-15T08:45:50Z

specification/network/resource-manager/Microsoft.Network/stable/2018-06-01/virtualWan.json

+    },
+    "/subscriptions/{subscriptionId}/providers/Microsoft.Network/vpnSites": {
+      "get": {
+        "operationId": "VpnSites_ListBySubscriptionId",


Please rename to VpnSites_List

EvgenyAgafonchikov · 2018-06-15T08:46:33Z

specification/network/resource-manager/Microsoft.Network/stable/2018-06-01/virtualWan.json

+    },
+    "/subscriptions/{subscriptionId}/providers/Microsoft.Network/virtualHubs": {
+      "get": {
+        "operationId": "VirtualHubs_ListBySubscriptionId",


same as above

EvgenyAgafonchikov · 2018-06-15T08:46:48Z

specification/network/resource-manager/Microsoft.Network/stable/2018-06-01/virtualWan.json

+    },
+    "/subscriptions/{subscriptionId}/providers/Microsoft.Network/vpnGateways": {
+      "get": {
+        "operationId": "VpnGateways_ListBySubscriptionId",


same as above

…nding examples

jianghaolu · 2018-06-15T23:15:55Z

.../network/resource-manager/Microsoft.Network/stable/2018-06-01/examples/VpnConnectionPut.json

+    "resourceGroupName": "rg1",
+    "api-version": "2018-06-01",
+    "subscriptionId": "subid",
+    "VpnConnectionParameters": {


Should be "VpnConnectionParameters" because of https://github.com/Azure/azure-rest-api-specs/pull/3204/files#diff-87bfafdce3f0047871a27733d64bf461R1430

Here: "vpnConnectionParameters"

jianghaolu · 2018-06-15T23:16:26Z

...twork/resource-manager/Microsoft.Network/stable/2018-06-01/examples/VpnConnectionUpdate.json

@@ -0,0 +1,68 @@
+{


I don't see this example being referenced anywhere.

jianghaolu · 2018-06-15T23:18:00Z

specification/network/resource-manager/Microsoft.Network/stable/2018-06-01/virtualWan.json

+            "$ref": "#/parameters/ApiVersionParameter"
+          },
+          {
+            "name": "VpnSiteParameters",


Some of your parameter names are camel cased, some are pascal cased. We recommend having the parameter names camel cased.

Leaving them as is.

Sure - but in the case of VpnConnections_CreateOrUpdate it's mismatching - the parameter name is vpnConnectionParameters but VpnConnectionParameters in the example

Sure- fixing that. Also, as per the suggestion by @MikhailTryakhov changing the Api version to 2018-04-01 for this swagger and hence moving everything to 2018-04-01 folder.

AutorestCI · 2018-06-20T18:49:54Z

Automation for azure-sdk-for-java

Nothing to generate for azure-sdk-for-java

priyanag · 2018-06-20T19:42:33Z

@MikhailTryakhov @ravbhatnagar @jianghaolu - I am targeting to close this PR today. Kindly help with the review. :)

ravbhatnagar · 2018-06-20T20:43:41Z

Looks good.

MikhailTryakhov

Please provide autorest validator result, correct PR checklist "Validation tools were run...." and sign off that results are OK. Looks like CI doesn't contains this script anymore...

jianghaolu · 2018-06-20T22:37:07Z

@MikhailTryakhov I'm not sure I understand what exactly you are looking for. Do you have an example?

MikhailTryakhov · 2018-06-20T22:39:21Z

@jianghaolu example how to run autorest validator? it's:
autorest <proj directory>\azure-rest-api-specs\specification\network\resource-manager\readme.md --azure-validator=true --validation --message-format=json

* Adding VirtualWan Swagger * Adding the CRUD Apis for VpnConnections * Updating VpnGateway examples * Fixing build errors * Addressing comments * Addressing comments and editing VpnConnection definition and corresponding examples * Addressing comments * Removing enableRateLimiting property * Addressing comments * Changing the version to 2018-04-01 version

…1) (#3261) * Added Network 2018-06-01; VM ref fix (#3118) * Init 2018-06-01 (copy from 2018-05-01) * Make VM reference in NIC readonly * Updated version in readme * New VPN Client Protocol (#3133) * Added new VPN protocol * indents * Add examples for Express Route Circuit APIs (#3182) * Add examples for Express Route Circuit APIs * Fix errors in expressRouteCircuit.json validations * Fix errors in validate examples for ExR Circuit * appgwv2 changes (#3192) * AppGW FIPS mode (#3202) * Release of Azure Networking Network-2018-06-01 branch (api-version 2018-04-01) * Adding VirtualWan Swagger (#3204) * Adding VirtualWan Swagger * Adding the CRUD Apis for VpnConnections * Updating VpnGateway examples * Fixing build errors * Addressing comments * Addressing comments and editing VpnConnection definition and corresponding examples * Addressing comments * Removing enableRateLimiting property * Addressing comments * Changing the version to 2018-04-01 version * Suppress ConnectionSharedKey model issue * Reverted extra-changes in readme

Adding VirtualWan Swagger

72d5eb2

priyanag requested review from DeepakRajendranMsft and MikhailTryakhov as code owners June 8, 2018 23:28

azuresdkci assigned jianghaolu Jun 8, 2018

azuresdkci requested a review from jianghaolu June 8, 2018 23:29

AutorestCI mentioned this pull request Jun 8, 2018

[AutoPR network/resource-manager] Adding VirtualWan Swagger AutorestCI/azure-libraries-for-java#237

Open

AutorestCI mentioned this pull request Jun 8, 2018

[AutoPR network/resource-manager] Adding VirtualWan Swagger Azure/azure-sdk-for-node#2965

Closed

Adding the CRUD Apis for VpnConnections

b2d39fd

Updating VpnGateway examples

2aa26f1

jianghaolu added the WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required label Jun 12, 2018

ravbhatnagar reviewed Jun 12, 2018

View reviewed changes

jianghaolu suggested changes Jun 12, 2018

View reviewed changes

Fixing build errors

09bf1be

jianghaolu suggested changes Jun 12, 2018

View reviewed changes

EvgenyAgafonchikov reviewed Jun 13, 2018

View reviewed changes

Addressing comments

e94e026

AutorestCI mentioned this pull request Jun 14, 2018

[AutoPR network/resource-manager] Adding VirtualWan Swagger Azure/azure-sdk-for-python#2756

Merged

EvgenyAgafonchikov reviewed Jun 15, 2018

View reviewed changes

Addressing comments and editing VpnConnection definition and correspo…

62deb8e

…nding examples

jianghaolu reviewed Jun 15, 2018

View reviewed changes

priyanag added 2 commits June 20, 2018 11:15

Addressing comments

3276bc2

Removing enableRateLimiting property

998bf05

ravbhatnagar added ARMSignedOff <valid label in PR review process>add this label when ARM approve updates after review and removed WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required labels Jun 20, 2018

priyanag added 2 commits June 20, 2018 14:32

Addressing comments

78518b2

Changing the version to 2018-04-01 version

0dc30ed

MikhailTryakhov approved these changes Jun 20, 2018

View reviewed changes

jianghaolu approved these changes Jun 20, 2018

View reviewed changes

MikhailTryakhov merged commit 2c50de6 into Azure:Network-2018-06-01 Jun 20, 2018

MikhailTryakhov mentioned this pull request Jun 20, 2018

Release of Azure Networking Network-2018-06-01 (api-version 2018-04-01) #3261

Merged

10 tasks

AutorestCI mentioned this pull request Jun 21, 2018

[AutoPR network/resource-manager] Adding VirtualWan Swagger Azure/azure-sdk-for-go#2091

Merged

Adding VirtualWan Swagger #3204

Adding VirtualWan Swagger #3204

Conversation

priyanag commented Jun 8, 2018 • edited Loading

PR information

Quality of Swagger

AutorestCI commented Jun 8, 2018 • edited Loading

Automation for azure-libraries-for-java

AutorestCI commented Jun 8, 2018 • edited Loading

Automation for azure-sdk-for-node

AutorestCI commented Jun 8, 2018 • edited Loading

Automation for azure-sdk-for-python

AutorestCI commented Jun 8, 2018 • edited Loading

Automation for azure-sdk-for-ruby

priyanag commented Jun 9, 2018

AutorestCI commented Jun 9, 2018 • edited Loading

Automation for azure-sdk-for-go

MikhailTryakhov commented Jun 9, 2018

MikhailTryakhov commented Jun 12, 2018 • edited Loading

ravbhatnagar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

priyanag Jun 14, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jianghaolu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

EvgenyAgafonchikov commented Jun 13, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AutorestCI commented Jun 20, 2018 • edited Loading

Automation for azure-sdk-for-java

priyanag commented Jun 20, 2018

ravbhatnagar commented Jun 20, 2018

MikhailTryakhov left a comment

Choose a reason for hiding this comment

jianghaolu commented Jun 20, 2018

MikhailTryakhov commented Jun 20, 2018

priyanag commented Jun 8, 2018 •

edited

Loading

AutorestCI commented Jun 8, 2018 •

edited

Loading

AutorestCI commented Jun 8, 2018 •

edited

Loading

AutorestCI commented Jun 8, 2018 •

edited

Loading

AutorestCI commented Jun 8, 2018 •

edited

Loading

AutorestCI commented Jun 9, 2018 •

edited

Loading

MikhailTryakhov commented Jun 12, 2018 •

edited

Loading

priyanag Jun 14, 2018 •

edited

Loading

AutorestCI commented Jun 20, 2018 •

edited

Loading