Skip to content
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

Portal UX - Fix VWAN Address Space & Remove AzFW Subnet For VWAN #718

Merged
merged 16 commits into from
Aug 13, 2021
Merged

Portal UX - Fix VWAN Address Space & Remove AzFW Subnet For VWAN #718

merged 16 commits into from
Aug 13, 2021

Conversation

jtracey93
Copy link
Collaborator

Overview/Summary

This PR fixes #714 which has re-occurred in the new eslzArm experience. It also removes the AzFw subnet address space from being specified in VWAN deployments, as this is not required.

This PR fixes/adds/changes/removes

  1. Azure vWAN Bootstrapping suggests /16 hub address range - Reappeared #714
  2. Removes Azure Firewall Subnet from having to be specified when VWAN is selected for network topology, as this is not used or required.

Breaking Changes

None

Testing Evidence

Hub & Spoke with AzFW

image

image

image

image

image

VWAN

image

image

image

image

image

Other

Can also be tested via this URL: https://portal.azure.com/#blade/Microsoft_Azure_CreateUIDef/CustomDeploymentBlade/uri/https%3A%2F%2Fraw.githubusercontent.com%2Fjtracey93%2FEnterprise-Scale%2Ffix-714%2FeslzArm%2FeslzArm.json/uiFormDefinitionUri/https%3A%2F%2Fraw.githubusercontent.com%2Fjtracey93%2FEnterprise-Scale%2Ffix-714%2FeslzArm%2Feslz-portal.json

As part of this Pull Request I have

  • Checked for duplicate Pull Requests
  • Associated it with relevant issues, for tracking and closure.
  • Ensured my code/branch is up-to-date with the latest changes in the main branch
  • Performed testing and provided evidence.
  • Updated relevant and associated documentation.
  • Updated the "What's New?" wiki page (located: /docs/wiki/whats-new.md) - not required

@jtracey93 jtracey93 added bug Something isn't working enhancement New feature or request labels Jul 29, 2021
@jtracey93 jtracey93 requested a review from krnese as a code owner July 29, 2021 23:31
@krnese
Copy link
Contributor

krnese commented Aug 2, 2021

Thanks Jack for submitting this PR. Instead of adding additional parameters to the ARM template(s), let’s solve this in the portal UX extension directly (client side) to default to the address space recommended for VWAN when VWAN option is selected. We can discuss further when back. Should be a minor update :-)

@krnese krnese requested a review from a team as a code owner August 11, 2021 17:50
hjscherer
hjscherer previously approved these changes Aug 12, 2021
Copy link
Contributor

@hjscherer hjscherer left a comment

Choose a reason for hiding this comment

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

tested and LGTM

@jtracey93 jtracey93 marked this pull request as draft August 12, 2021 15:49
@jtracey93
Copy link
Collaborator Author

Thanks @hjscherer. After speaking with Kristian yesterday. There may be a slicker way to do this and keep the code DRY'er. I'll convert this to draft whilst I make the changes and test before submitting later 👍

@jtracey93 jtracey93 marked this pull request as ready for review August 12, 2021 18:11
@jtracey93
Copy link
Collaborator Author

@krnese @hjscherer This should now be good to go 👍

The addressPrefix output now uses a coalesce, I have tested this in the UI definition Sandbox and outputs are as desired when the relevant options are selected 👍

"addressPrefix": "[coalesce(steps('esConnectivityGoalState').esAddressHubVWAN, steps('esConnectivityGoalState').esAddressHubHS, '')]",

@krnese krnese merged commit 297e154 into Azure:main Aug 13, 2021
@jtracey93 jtracey93 deleted the fix-714 branch August 13, 2021 09:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Azure vWAN Bootstrapping suggests /16 hub address range - Reappeared
3 participants