Virtual Wan point to site feature redesign:- #7238

Nilambari · 2019-09-17T23:10:32Z

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.

AutorestCI · 2019-09-17T23:12:33Z

Automation for azure-sdk-for-python

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

Command: ['/usr/local/bin/autorest', '/tmp/tmpuefj4e8u/rest/specification/network/resource-manager/readme.md', '--keep-version-file', '--multiapi', '--no-async', '--python', '--python-mode=update', '--python-sdks-folder=/tmp/tmpuefj4e8u/sdk/sdk', '[email protected]/[email protected]', '--version=preview']
Finished with return code -6
and output:

AutoRest code generation utility [version: 2.0.4283; node: v10.15.3]
(C) 2018 Microsoft Corporation.
https://aka.ms/autorest
   Loading AutoRest core      '/root/.autorest/@[email protected]/node_modules/@microsoft.azure/autorest-core/dist' (2.0.4399)
   Loading AutoRest extension '@microsoft.azure/autorest.python' (4.0.70->4.0.70)
   Loading AutoRest extension '@microsoft.azure/autorest.modeler' (2.3.44->2.3.44)
Processing batch task - {"tag":"package-2019-06"} .

<--- Last few GCs --->

[28686:0x3cf1cc0]    94677 ms: Mark-sweep 1382.0 (1427.8) -> 1381.9 (1427.8) MB, 2355.1 / 0.0 ms  (average mu = 0.144, current mu = 0.001) allocation failure GC in old space requested
[28686:0x3cf1cc0]    97039 ms: Mark-sweep 1382.3 (1427.8) -> 1382.2 (1428.3) MB, 2360.5 / 0.0 ms  (average mu = 0.078, current mu = 0.001) allocation failure GC in old space requested


<--- JS stacktrace --->

==== JS stack trace =========================================

    0: ExitFrame [pc: 0x178786edbe1d]
Security context: 0x12e4ce21e6e9 <JSObject>
    1: newScalar [0x5690a1b4461] [/node_modules/yaml-ast-parser/dist/src/yamlAST.js:~38] [pc=0x1787874a1e36](this=0x1ba5e75b0eb9 <Object map = 0x2e6e01aa2339>,v=0x3a1d46b826f1 <undefined>)
    2: arguments adaptor frame: 0->1
    3: readDoubleQuotedScalar(aka readDoubleQuotedScalar) [0x5690a1b4341] [/node_modules/yaml-ast-parser/dist/src/loader.js:~530] [pc=...

FATAL ERROR: Ineffective mark-compacts near heap limit Allocation failed - JavaScript heap out of memory
 1: 0x8dc510 node::Abort() [node]
 2: 0x8dc55c  [node]
 3: 0xad9b5e v8::Utils::ReportOOMFailure(v8::internal::Isolate*, char const*, bool) [node]
 4: 0xad9d94 v8::internal::V8::FatalProcessOutOfMemory(v8::internal::Isolate*, char const*, bool) [node]
 5: 0xec7bf2  [node]
 6: 0xec7cf8 v8::internal::Heap::CheckIneffectiveMarkCompact(unsigned long, double) [node]
 7: 0xed3dd2 v8::internal::Heap::PerformGarbageCollection(v8::internal::GarbageCollector, v8::GCCallbackFlags) [node]
 8: 0xed4704 v8::internal::Heap::CollectGarbage(v8::internal::AllocationSpace, v8::internal::GarbageCollectionReason, v8::GCCallbackFlags) [node]
 9: 0xed7371 v8::internal::Heap::AllocateRawWithRetryOrFail(int, v8::internal::AllocationSpace, v8::internal::AllocationAlignment) [node]
10: 0xea07f4 v8::internal::Factory::NewFillerObject(int, bool, v8::internal::AllocationSpace) [node]
11: 0x11402db v8::internal::Runtime_AllocateInTargetSpace(int, v8::internal::Object**, v8::internal::Isolate*) [node]
12: 0x178786edbe1d

Nilambari · 2019-09-17T23:12:42Z

VpnServerConfiguration resource APIs were already reviewed and signed off as a part of past PR:- #6644

azuresdkci · 2019-09-17T23:18:28Z

Can one of the admins verify this patch?

AutorestCI · 2019-09-17T23:46:48Z

Automation for azure-sdk-for-go

Nothing to generate for azure-sdk-for-go

anton-evseev

API version in the following examples is wrong

P2SVpnGatewayGetConnectionHealthDetailed.json
VpnServerConfigurationDelete.json
VpnServerConfigurationGet.json
VpnServerConfigurationList.json
VpnServerConfigurationListByResourceGroup.json
VpnServerConfigurationPut.json
VpnServerConfigurationUpdateTags.json

anton-evseev · 2019-09-18T17:18:15Z

...r/Microsoft.Network/stable/2019-08-01/examples/P2SVpnGatewayGetConnectionHealthDetailed.json

+        } 
+    }, 
+    "responses": { 
+        "200": {}, 


200 should return P2SVpnConnectionHealth according to specs

anton-evseev · 2019-09-18T17:20:09Z

...nager/Microsoft.Network/stable/2019-08-01/examples/GetVirtualWanVpnServerConfigurations.json

+    "api-version": "2019-08-01",
+    "virtualWANName": "wan1"
+  },
+  "responses": {


Specs also define 202 response, please add it to example (it should be empty though)

anton-evseev · 2019-09-18T17:32:56Z

This looks like breaking change.
cc: @MikhailTryakhov

anton-evseev · 2019-09-18T20:14:59Z

@majastrz please review this from ARM's side

Nilambari · 2019-09-18T21:15:02Z

API version in the following examples is wrong
P2SVpnGatewayGetConnectionHealthDetailed.json
VpnServerConfigurationDelete.json
VpnServerConfigurationGet.json
VpnServerConfigurationList.json
VpnServerConfigurationListByResourceGroup.json
VpnServerConfigurationPut.json
VpnServerConfigurationUpdateTags.json

Done.

…azure-rest-api-specs into network-september-release2

majastrz · 2019-09-18T23:30:33Z

specification/network/resource-manager/Microsoft.Network/stable/2019-08-01/virtualWan.json

+            }
+          }
+        },
+        "x-ms-long-running-operation": true


"x-ms-long-running-operation": true [](start = 8, length = 35)

Is it actually asynchronous? You're just setting new tags right? If it is intentionally marked as long running, you should add x-ms-long-running-operation-options #Closed

Done. #Closed

majastrz · 2019-09-18T23:31:08Z

specification/network/resource-manager/Microsoft.Network/stable/2019-08-01/virtualWan.json

+        },
+        "x-ms-long-running-operation": true,
+        "x-ms-long-running-operation-options": {
+          "final-state-via": "location"


"final-state-via": "location" [](start = 10, length = 29)

So PUT is using azure-async-operation and DELETE is using location. Should these be consistent? #Closed

Yes it was intentional and its similar to other networking resources. #Closed

majastrz · 2019-09-18T23:34:37Z

specification/network/resource-manager/Microsoft.Network/stable/2019-08-01/virtualWan.json

        }
      }
    },
-    "/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Network/virtualWans/{virtualWanName}/p2sVpnServerConfigurations/{p2SVpnServerConfigurationName}": {


p2sVpnServerConfigurations/{p2SVpnServerConfigurationName} [](start = 129, length = 58)

What's the plan for migrating existing resources when you delete this? Are you following the applicable steps documented at https://armwiki.azurewebsites.net/api_contracts/APIDeprecationPolicy.html?q=deprecation?

You will need to pay attention to the policy part of that document because there exist policy aliases for this resource type.

In reply to: 325936168 [](ancestors = 325936168)

Actually, we have already contacted existing customers (3 months back) to delete these resources and whoever didn't delete, we have already done the cleanup on their behalf as per the deadline communicated. So, there are no existing P2SVpnServerConfiguration resources owned by customers on Production. Now, new design changes are already released and so customer can no more create P2SVpnServerConfiguration resource. However, from next time, we will follow the steps documented at link.

@Nilambari, see my message in the email thread. Please look at the ARM deprecation guidelines (link in email). They include steps for contacting the Azure Policy team to determine the impact to customer policies, and to have us remove aliases as the properties are removed.

majastrz · 2019-09-18T23:38:22Z

@mentat9 is added to the review. #Closed

majastrz · 2019-09-18T23:40:14Z

specification/network/resource-manager/Microsoft.Network/stable/2019-08-01/virtualWan.json

+        "parameters": [
+          {
+            "$ref": "./network.json#/parameters/SubscriptionIdParameter"
+          },		


[](start = 12, length = 2)

nit - extra tabs #Closed

Done. #Closed

majastrz · 2019-09-18T23:40:55Z

specification/network/resource-manager/Microsoft.Network/stable/2019-08-01/virtualWan.json

+            "$ref": "./network.json#/parameters/SubscriptionIdParameter"
+          }
+        ],
+        "responses": {


responses [](start = 9, length = 9)

You should also have a default response on all operations to capture errors. I see this on new changes and old.

majastrz · 2019-09-18T23:45:40Z

specification/network/resource-manager/Microsoft.Network/stable/2019-08-01/virtualWan.json

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


vpnServerConfigurations [](start = 129, length = 23)

The name vpnServerConfigurations suggests that this is a nested resource type but this is an action on the wan. Can you rename it to something more imperative?

No as per new design. VpnServerConfiguration is a new Top level resource. It contains the VPN configurations which resides on server (P2SVpnGateway) when attached to it. Actually, we have finalized this resource name after through discussion.

majastrz

I added some comments. Please take a look.

Nilambari · 2019-09-19T23:14:55Z

@majastrz Incorporated review comments, Please take a look. Thanks! #Resolved

majastrz · 2019-09-19T23:43:43Z

specification/network/resource-manager/Microsoft.Network/stable/2019-08-01/virtualWan.json

+          "default": {
+            "description": "Error.",
+            "schema": {
+              "$ref": "./network.json#/definitions/Error"


Error [](start = 51, length = 5)

The ./network.json#/definitions/Error definition is not the right one. The standard error response documented at https://github.com/Azure/azure-resource-manager-rpc/blob/master/v1.0/common-api-details.md#common-api-response-details looks like this:

{ "error": { "code": "...", "message": "...", .... } }

But the one you're referencing looks like this.

{ "code": "...", "message": "...", .... } ``` #Resolved

(This applies to the others as well.)

In reply to: 326424772 [](ancestors = 326424772)

majastrz

There's an issue with the default error response schema. I added a comment.

Nilambari · 2019-09-20T00:57:10Z

There's an issue with the default error response schema. I added a comment.

@majastrz Fixed it. #Resolved

majastrz · 2019-09-20T00:59:25Z

Looks good now.

In reply to: 533361590 [](ancestors = 533361590)

majastrz

Signed off from ARM side.

anton-evseev · 2019-09-20T16:57:21Z

specification/network/resource-manager/Microsoft.Network/stable/2019-08-01/virtualWan.json

+      ],
+      "description": "P2SConnectionConfiguration Resource."
+    },
+    "DefaultError": {


@Nilambari @majastrz this is a fourth (!) definition of the same cloud error in Network specs. We're planning to fix that soon by re-using ARM's common definitions but for now please reference CloudError from ddosCustomPolicy.json

Sure. I will refer to CloudError from ddosCustomPolicy.json everywhere in this swagger spec.

…azure-rest-api-specs into network-september-release2

Nilambari · 2019-09-20T20:51:43Z

@number213 Build is passed. Can you help merge the changes?

anton-evseev · 2019-09-20T20:52:54Z

@tjprescott could you please merge this PR?

* Adds base for updating Microsoft.Network from version stable/2019-07-01 to version 2019-08-01 * Updates readme * Updates API version in new specs and examples * Add reference to Connection Monitor specs in 2019-08-01 (#7218) * Support AH protocol (#7231) * Virtual Network BGP Communities (#7248) * Virtual Wan point to site feature redesign:- (#7238) * Virtual Wan point to site feature redesign * Fix examples * Fix validation errors * Fix validation error * Fix validation errors * Incorporate review comments * Fix default error in standard error response * Fix validation error * Refer to default error from ddos swagger spec * Fix build error * Firewall Manager Feature Swagger Updates for Public Preview (#7266) * Adding the structure of urlConfiguration in rewriteRuleActionSet (#7275) * Revert "Adding the structure of urlConfiguration in rewriteRuleActionSet (#7275)" (#7333) This reverts commit e89f35d. * Virtual network gateway changes for dns forwarding (#7359) * Virtuan network gateway changes for dns forwarding * Fixed example response * Fixed examples * Fixed 201 example * Top-level WAF (#7222) * Move new props to the end to avoid breaking changes in SDK (#7387)

Virtual Wan point to site feature redesign

eef8aef

Nilambari requested review from MikhailTryakhov and anton-evseev as code owners September 17, 2019 23:10

azuresdkci assigned tjprescott Sep 17, 2019

azuresdkci requested a review from tjprescott September 17, 2019 23:11

anton-evseev added the WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required label Sep 17, 2019

anton-evseev suggested changes Sep 18, 2019

View reviewed changes

Nilambari added 2 commits September 18, 2019 14:34

Fix examples

324999d

Merge branch 'network-september-release' of https://github.com/Azure/…

403fd35

…azure-rest-api-specs into network-september-release2

Nilambari changed the title ~~Virtual Wan point to site feature redesign~~ Virtual Wan point to site feature redesign:- Sep 18, 2019

Nilambari added 3 commits September 18, 2019 15:06

Fix validation errors

2fdd40e

Fix validation error

7a7870f

Fix validation errors

a6f02ba

majastrz reviewed Sep 18, 2019

View reviewed changes

majastrz added the ARMChangesRequested label Sep 18, 2019

majastrz requested changes Sep 18, 2019

View reviewed changes

Incorporate review comments

2a5418a

majastrz reviewed Sep 19, 2019

View reviewed changes

majastrz requested changes Sep 19, 2019

View reviewed changes

Fix default error in standard error response

a8a6e10

majastrz added ARMSignedOff <valid label in PR review process>add this label when ARM approve updates after review and removed ARMChangesRequested WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required labels Sep 20, 2019

majastrz approved these changes Sep 20, 2019

View reviewed changes

Fix validation error

37b0975

tjprescott approved these changes Sep 20, 2019

View reviewed changes

anton-evseev suggested changes Sep 20, 2019

View reviewed changes

Nilambari added 3 commits September 20, 2019 11:08

Refer to default error from ddos swagger spec

46d6bae

Merge branch 'network-september-release' of https://github.com/Azure/…

2b91292

…azure-rest-api-specs into network-september-release2

Fix build error

7771c35

anton-evseev approved these changes Sep 20, 2019

View reviewed changes

tjprescott merged commit 64bcd0e into Azure:network-september-release Sep 20, 2019

Nilambari mentioned this pull request Oct 15, 2019

Fix Virtual Wan Point to site feature resources property names from swagger. #7498

Merged

5 tasks

Nilambari mentioned this pull request Apr 15, 2020

CLI for Virtual WAN Point to site feature. Azure/azure-cli#13020

Closed

Virtual Wan point to site feature redesign:- #7238

Virtual Wan point to site feature redesign:- #7238

Uh oh!

Conversation

Nilambari commented Sep 17, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Latest improvements:

Contribution checklist:

ARM API Review Checklist

Uh oh!

AutorestCI commented Sep 17, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Automation for azure-sdk-for-python

Uh oh!

Nilambari commented Sep 17, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

azuresdkci commented Sep 17, 2019

Uh oh!

AutorestCI commented Sep 17, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Automation for azure-sdk-for-go

Uh oh!

anton-evseev left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

anton-evseev commented Sep 18, 2019

Uh oh!

anton-evseev commented Sep 18, 2019

Uh oh!

Nilambari commented Sep 18, 2019

Uh oh!

majastrz Sep 18, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Nilambari Sep 19, 2019 • edited by majastrz Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

majastrz Sep 18, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Nilambari Sep 19, 2019 • edited by majastrz Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Nilambari Sep 19, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

majastrz commented Sep 18, 2019

Uh oh!

majastrz Sep 18, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Nilambari Sep 19, 2019 • edited by majastrz Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Nilambari commented Sep 17, 2019 •

edited

Loading

AutorestCI commented Sep 17, 2019 •

edited

Loading

Nilambari commented Sep 17, 2019 •

edited

Loading

AutorestCI commented Sep 17, 2019 •

edited

Loading

majastrz Sep 18, 2019 •

edited

Loading

Nilambari Sep 19, 2019 •

edited by majastrz

Loading

majastrz Sep 18, 2019 •

edited

Loading

Nilambari Sep 19, 2019 •

edited by majastrz

Loading

Nilambari Sep 19, 2019 •

edited

Loading

majastrz Sep 18, 2019 •

edited

Loading

Nilambari Sep 19, 2019 •

edited by majastrz

Loading

Nilambari Sep 19, 2019 •

edited

Loading

Nilambari commented Sep 19, 2019 •

edited by majastrz

Loading

majastrz Sep 19, 2019 •

edited

Loading

Nilambari commented Sep 20, 2019 •

edited by majastrz

Loading