Skip to content

Fix parameter name and structure for radius server setting for VpnGat…#3524

Closed
ritwikbasu wants to merge 1 commit intoAzure:psSdkJson6from
ritwikbasu:psSdkJson6
Closed

Fix parameter name and structure for radius server setting for VpnGat…#3524
ritwikbasu wants to merge 1 commit intoAzure:psSdkJson6from
ritwikbasu:psSdkJson6

Conversation

@ritwikbasu
Copy link
Copy Markdown
Contributor

@ritwikbasu ritwikbasu commented Jul 28, 2017

…eways.

Fix parameter name and structure for radius server setting for
VpnGateways.

Description


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

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 the commitid of your swagger spec or link to the swagger spec, used to generate the code.
  • The *.csproj and AssemblyInfo.cs files have been updated with the new version of the SDK.

The Rest-API-Spec PR for this change is here:
Azure/azure-rest-api-specs#1484

…eways.

Fix parameter name and structure for radius server setting for
VpnGateways.
Copy link
Copy Markdown
Contributor

@shahabhijeet shahabhijeet left a comment

Choose a reason for hiding this comment

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

@ritwikbasu if this is a breaking change.
Bump up the major version.
Also where are the tests for this change?

/// 'Deleting', and 'Failed'.</param>
/// <param name="etag">Gets a unique read-only string that changes
/// whenever the resource is updated.</param>
public VirtualNetworkGateway(string id = default(string), string name = default(string), string type = default(string), string location = default(string), IDictionary<string, string> tags = default(IDictionary<string, string>), IList<VirtualNetworkGatewayIPConfiguration> ipConfigurations = default(IList<VirtualNetworkGatewayIPConfiguration>), string gatewayType = default(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 radiusServer = default(string), string radiusSecret = default(string), string resourceGuid = default(string), string provisioningState = default(string), string etag = default(string))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@ritwikbasu so anyone who use to provide radiusServer info will be broken during constructing VirtualNetworkGateway?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This feature is not exposed to customers. It has been deployed in NRP but is protected by a feature flag. This change ensures that the contract matches NRP and the documentation that will go out to the customer once the feature is released publicly.

The correct behavior is that radius settings should go as part of VpnClientConfiguration object when the feature is released. That is also the behavior in ARM/NRP now. This is a bugfix on the previous iteration which exposed radius address.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is not a breaking change - this is a bugfix.

The tests are compiled as part of the powershell release. This specific change requires pre-setup that is already available in powershell.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants