Ipsec Policies for Virtual Network Gateway Connection by henry416 · Pull Request #2943 · Azure/azure-sdk-for-net

henry416 · 2017-03-16T18:23:32Z

Description

REST spec here: Azure/azure-rest-api-specs#956

Added IPsec Policies support to virtual network gateway connection model
Added passing tests for IPsec Policies connections
new api-version of Network swagger added (all changes but in 2017-03-01 version, copied over from previous rest spec version)

This checklist is used to make sure that common guidelines for a pull request are followed.

I have read the contribution guidelines.
The pull request does not introduce breaking changes.

General Guidelines

Title of the pull request 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 more information on cleaning up the commits in your PR, see this page.

Testing Guidelines

Pull request includes test coverage for the included changes.

SDK Generation Guidelines

If an SDK is being regenerated based on a new swagger spec, a link to the pull request containing these swagger spec changes has been included above.
The generate.cmd file for the SDK has been updated with the version of AutoRest, as well as a link to the swagger spec, used to generate the code.
The project.json and AssemblyInfo.cs files have been updated with the new version of the SDK.

Added virtual network gateway connection tests for ipsec policies Added VirtualNetworkGatewayConnection Ipsec Policies Test record Test Cases updated to use field names Connection Ipsec Policies Test session data Ipsec policies Test added for virtual network gateway connections

…icies added compared to previous swagger)

azuresdkci · 2017-03-16T18:23:34Z

Can one of the admins verify this patch?

msftclas · 2017-03-16T18:23:36Z

@henry416,
Thanks for your contribution as a Microsoft full-time employee or intern. You do not need to sign a CLA.
Thanks,
Microsoft Pull Request Bot

henry416 · 2017-03-17T17:52:37Z

@cormacpayne Hi, is this waiting on the spec to be merged for the CR to proceed?

cormacpayne · 2017-03-17T17:57:37Z

@henry416 Hey Henry, we are waiting for the spec to be merged in order to continue with the review. This ensures that all changes with the generated code are finalized, and we don't have to keep updating the review based on changes to the rest-api-specs PR

cormacpayne · 2017-03-17T17:57:53Z

@azuresdkci add to whitelist

henry416 · 2017-03-17T18:06:21Z

@cormacpayne OK. However can you clarify if what I have submitted for test records is sufficient?

cormacpayne · 2017-03-17T18:16:41Z

@henry416 test coverage looks good to me 👍

cormacpayne · 2017-03-17T20:52:54Z

@azuresdkci test this please

henry416 · 2017-03-17T23:39:24Z

@cormacpayne Is the current checked-in build stable?

henry416 · 2017-03-20T17:55:17Z

@cormacpayne rest specs have been merged, this PR can proceed

cormacpayne

@henry416 Hey Henry, just a few comments that need clarification

cormacpayne · 2017-03-20T18:56:59Z


-[assembly: AssemblyVersion("9.1.0.0")]
-[assembly: AssemblyFileVersion("9.1.0.0")]
+[assembly: AssemblyVersion("9.2.0.0")]


@henry416 you can set AssemblyVersion to 9.0.0 and keep AssemblyFileVersion at 9.2.0.

AssemblyFileVersion should always match the version found in project.json, and AssemblyVersion shares only the major version of the AssemblyFileVersion

cormacpayne · 2017-03-20T18:58:15Z

-        /// <param name="routeFilter">The reference of the RouteFilter
-        /// resource.</param>
-        public ExpressRouteCircuitPeering(string id = default(string), string peeringType = default(string), string state = default(string), int? azureASN = default(int?), int? peerASN = default(int?), string primaryPeerAddressPrefix = default(string), string secondaryPeerAddressPrefix = default(string), string primaryAzurePort = default(string), string secondaryAzurePort = default(string), string sharedKey = default(string), int? vlanId = default(int?), ExpressRouteCircuitPeeringConfig microsoftPeeringConfig = default(ExpressRouteCircuitPeeringConfig), ExpressRouteCircuitStats stats = default(ExpressRouteCircuitStats), string provisioningState = default(string), string gatewayManagerEtag = default(string), string lastModifiedBy = default(string), string name = default(string), string etag = default(string), RouteFilter routeFilter = default(RouteFilter)) 
+        public ExpressRouteCircuitPeering(string id = default(string), string peeringType = default(string), string state = default(string), int? azureASN = default(int?), int? peerASN = default(int?), string primaryPeerAddressPrefix = default(string), string secondaryPeerAddressPrefix = default(string), string primaryAzurePort = default(string), string secondaryAzurePort = default(string), string sharedKey = default(string), int? vlanId = default(int?), ExpressRouteCircuitPeeringConfig microsoftPeeringConfig = default(ExpressRouteCircuitPeeringConfig), ExpressRouteCircuitStats stats = default(ExpressRouteCircuitStats), string provisioningState = default(string), string gatewayManagerEtag = default(string), string lastModifiedBy = default(string), RouteFilter routeFilter = default(RouteFilter), string name = default(string), string etag = default(string))


@henry416 I thought the position of RouteFilter in the constructor was fixed in a previous swagger spec PR (so that it would be the last parameter). Do you know if this change was reverted?

I never touched the expressroute specs - I will investigate this

cormacpayne · 2017-03-20T18:58:39Z

        /// <param name="etag">Gets a unique read-only string that changes
        /// whenever the resource is updated.</param>
-        public VirtualNetworkGateway(IList<VirtualNetworkGatewayIPConfiguration> ipConfigurations, string gatewayType, string vpnType, string id = default(string), string name = default(string), string type = default(string), string location = default(string), IDictionary<string, string> tags = default(IDictionary<string, string>), bool? enableBgp = default(bool?), bool? activeActive = default(bool?), SubResource gatewayDefaultSite = default(SubResource), VirtualNetworkGatewaySku sku = default(VirtualNetworkGatewaySku), VpnClientConfiguration vpnClientConfiguration = default(VpnClientConfiguration), BgpSettings bgpSettings = default(BgpSettings), string resourceGuid = default(string), string provisioningState = default(string), string etag = default(string))
+        public VirtualNetworkGateway(IList<VirtualNetworkGatewayIPConfiguration> ipConfigurations, string gatewayType, string id = default(string), string name = default(string), string type = default(string), string location = default(string), IDictionary<string, string> tags = default(IDictionary<string, string>), string vpnType = default(string), bool? enableBgp = default(bool?), bool? activeActive = default(bool?), SubResource gatewayDefaultSite = default(SubResource), VirtualNetworkGatewaySku sku = default(VirtualNetworkGatewaySku), VpnClientConfiguration vpnClientConfiguration = default(VpnClientConfiguration), BgpSettings bgpSettings = default(BgpSettings), string resourceGuid = default(string), string provisioningState = default(string), string etag = default(string))


@henry416 similar comment to that of RouteFilter. Was this position changed?

@Nilambari has a change in the parameter waiting for PR

@cormacpayne Confirmed that this is intended, see Nilambari's PR: https://github.com/Nilambari/azure-sdk-for-net/blob/4a1d0de7713aaf848ecc5a2f9b8b5b0aeea94d9c/src/ResourceManagement/Network/Microsoft.Azure.Management.Network/Generated/Models/VirtualNetworkGateway.cs

henry416 · 2017-03-21T17:40:00Z

@cormacpayne I have addressed the issues with the help of the teams that own those components + fixed the tests

henry416 · 2017-03-21T22:33:58Z

@cormacpayne Is there any other outstanding issue with this PR?

cormacpayne · 2017-03-21T23:00:33Z

@henry416 did you get the email I sent? There are multiple Network PRs that have been opened in this repository and they need to be coordinated so we are not publishing multiple packages in a short time frame.

henry416 · 2017-03-22T05:53:17Z

@cormacpayne can the merge proceed?

henry416 · 2017-03-22T19:01:36Z

@cormacpayne Is this still in review? Once this is done we can do a double merge (my changes into branch and then branch into AutoRest master)

jobatzil · 2017-03-22T22:33:06Z

@henry416 Hey henry, could you maybe remove the files from your PR with no changes but a different EOF?

henry416 · 2017-03-22T22:56:44Z

@jobatzil EOF removed and merge conflict with network-changes branch resolved

henry416 · 2017-03-23T16:15:30Z

@cormacpayne I have resolved the merge issues, can this go through now?

henry416 added 3 commits March 16, 2017 10:59

Generated code for network new 2017-03-01 api-version (only Ipsec Pol…

af97a84

…icies added compared to previous swagger)

package version incremented

3e92d91

msftclas added the cla-not-required label Mar 16, 2017

henry416 changed the title ~~Ipsec Policies for~~ Ipsec Policies for Virtual Network Gateway Connection Mar 16, 2017

cormacpayne mentioned this pull request Mar 16, 2017

Arm-Network swagger: added support for IPSec parameters for connection Azure/azure-rest-api-specs#956

Merged

10 tasks

cormacpayne added the spec-not-merged label Mar 16, 2017

Merge remote-tracking branch 'upstream/AutoRest' into AutoRest

c95c199

cormacpayne removed the spec-not-merged label Mar 20, 2017

cormacpayne suggested changes Mar 20, 2017

View reviewed changes

cormacpayne added the needs-revision label Mar 20, 2017

Using proper rest api spec for 2017-03-01 generation

a245f83

jobatzil mentioned this pull request Mar 21, 2017

#WAF Added the changes of web application firewall for application gateway #2958

Merged

8 tasks

henry416 added 2 commits March 21, 2017 09:11

Update Network test versions to 2017-03-01

86f6cd3

corrected Assembly version for 2017-03-01

9cac289

cormacpayne changed the base branch from AutoRest to network-changes March 22, 2017 00:45

Merge remote-tracking branch 'upstream/network-changes' into AutoRest

ccbc38e

AssemblyInfo revert AssemblyVersion to original as requested

05e52a9

This was referenced Mar 22, 2017

Ipsec Policy for Virtual Network Gateway Connection Azure/azure-powershell#3666

Merged

#INIT to 2017-03-01 #2976

Merged

henry416 added 2 commits March 22, 2017 15:48

merge with network-changes branch

8af95e9

removed EOF-only changes

2108694

cormacpayne approved these changes Mar 23, 2017

View reviewed changes

cormacpayne merged commit 97c5852 into Azure:network-changes Mar 23, 2017

Conversation

henry416 commented Mar 16, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Uh oh!

azuresdkci commented Mar 16, 2017

Uh oh!

msftclas commented Mar 16, 2017

Uh oh!

henry416 commented Mar 17, 2017

Uh oh!

cormacpayne commented Mar 17, 2017

Uh oh!

cormacpayne commented Mar 17, 2017

Uh oh!

henry416 commented Mar 17, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cormacpayne commented Mar 17, 2017

Uh oh!

cormacpayne commented Mar 17, 2017

Uh oh!

henry416 commented Mar 17, 2017

Uh oh!

henry416 commented Mar 20, 2017

Uh oh!

cormacpayne 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!

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!

henry416 commented Mar 21, 2017

Uh oh!

henry416 commented Mar 21, 2017

Uh oh!

cormacpayne commented Mar 21, 2017

Uh oh!

henry416 commented Mar 22, 2017

Uh oh!

henry416 commented Mar 22, 2017

Uh oh!

jobatzil commented Mar 22, 2017

Uh oh!

henry416 commented Mar 22, 2017

Uh oh!

henry416 commented Mar 23, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

henry416 commented Mar 16, 2017 •

edited

Loading

henry416 commented Mar 17, 2017 •

edited

Loading