Skip to content

Adding cmdltets to support crud operations for new Route Table child resource of VirtualHub #10317

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 24 commits into from
Oct 23, 2019

Conversation

nagula-ritvika
Copy link
Contributor

@nagula-ritvika nagula-ritvika commented Oct 17, 2019

Description

  1. Added new cmdlets to support creating route tables and adding them to hub, removing route tables of a hub.
  2. Exposed enableInternetSecurity property for HubVnetConnection, VpnConnection and ExpressRouteConnection and added update cmdlets needed.
  3. Exposed new hub sku property and VirtualWANtype property and modified existing corresponding cmdlets as needed.
  1. Design Review PR - Azure/azure-powershell-cmdlet-review-pr#397
  2. The changes regarding support for EnableInternetSecurity for VirtualHubVnetConnection and VpnConnection were committed to a private-preview version in this PR - Adding support for security provider integration in cortex cmdlets. #7261. I am just picking the changes we need here.

Checklist

  • I have read the Submitting Changes section of CONTRIBUTING.md
  • The title of the PR is clear and informative
  • The appropriate ChangeLog.md file(s) has been updated:
    • For any service, the ChangeLog.md file can be found at src/{{SERVICE}}/{{SERVICE}}/ChangeLog.md
    • A snippet outlining the change(s) made in the PR should be written under the ## Upcoming Release header -- no new version header should be added
  • The PR does not introduce breaking changes
  • If applicable, the changes made in the PR have proper test coverage
  • For public API changes to cmdlets:
    • a cmdlet design review was approved for the changes in this repository (Microsoft internal only)
    • the markdown help files have been regenerated using the commands listed here

@adxsdkps
Copy link
Collaborator

Can one of the admins verify this patch?

@nagula-ritvika
Copy link
Contributor Author

@nagula-ritvika
Copy link
Contributor Author

The changes regarding support for EnableInternetSecurity for VirtualHubVnetConnection and VpnConnection were committed to a private-preview version in this PR - #7261.

I am just picking the changes we need here.

@nagula-ritvika
Copy link
Contributor Author

I have created the PR by building the sdk locally, will revert the SDK changes when needed,

@nagula-ritvika nagula-ritvika changed the base branch from master to network-october October 17, 2019 17:52
Copy link
Contributor

@anton-evseev anton-evseev left a comment

Choose a reason for hiding this comment

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

Please add test(s) for these changes

@nagula-ritvika
Copy link
Contributor Author

Please add test(s) for these changes

I am working on adding the tests locally and will update the PR by tomorrow. Meanwhile it would be great if you could review the other code changes if possible.

Copy link
Contributor

@anton-evseev anton-evseev left a comment

Choose a reason for hiding this comment

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

There's a bit of inconsistency with EnableInternetSecurity: in some cmdlets it's a switch, in some - nullable bool. I get that it's done so in Set the value wouldn't be reset, but I think having two different types for one parameter would be confusing
Please also re-generate help files

@anton-evseev
Copy link
Contributor

We've checked in test recordings with API version 2019-09-01 into network-october, please merge/rebase to unblock your PR

@anton-evseev
Copy link
Contributor

/azp run

@azure-pipelines
Copy link
Contributor

Azure Pipelines successfully started running 3 pipeline(s).

@anton-evseev
Copy link
Contributor

/azp run

@azure-pipelines
Copy link
Contributor

Azure Pipelines successfully started running 3 pipeline(s).

@VeryEarly VeryEarly merged commit c39c2e4 into Azure:network-october Oct 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants