-
Notifications
You must be signed in to change notification settings - Fork 517
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
Added several user defined types, ability for custom resources names in vwanConnectivity and mgDiagSettings #656
Conversation
Thanks @johnlokerse, dont forget to update the parameter input files also 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@johnlokerse we've got some test failures, could you review and resolve?
Also could you review the other modules to ensure they all allow a custom name for resources, if desired and don't have any hardcoded patterns like this one did?
Added succeeded deployment screenshots |
How do we want to present the parameter file? The new properties are fully optional so there is no need to add them to the parameter file. It is only required when you want to use a custom name.
Will do! :) |
infra-as-code/bicep/modules/mgDiagSettings/mgDiagSettings.bicep
Outdated
Show resolved
Hide resolved
Fixed the broken Action regarding listing the used resource types. The underlying ARM template is different when using User-Defined Types. The Action was looking for the resource type I have tested this locally and got it working like it was before: In this test I had two JSON files (hubNetworking.json and policyAssignedManagementGroup.json) with User-Defined Types and one JSON file (publicIp.json) without types. Can you rerun the Action for me @jtracey93 / @oZakari? |
FYI. I am testing something so that I do not have to use the |
Is there any need for
|
Yes, that was my change to reduce complexity/add readability :-) Thanks for the suggestion @picccard! Tested the solution through deployments: |
Thank you much @johnlokerse! I will plan on going through this PR later this week. |
infra-as-code/bicep/modules/vwanConnectivity/vwanConnectivity.bicep
Outdated
Show resolved
Hide resolved
Already has been resolved by John
Hey @johnlokerse, for the VWAN user defined type, I removed the parUseCustomNamingScheme switch references as it looks like you had removed that from the conditions in a previous commit. I also adjusted the properties for the user defined type in that module as well. Does what I changed it to make sense to you? Everything else looks good for the PR, let me know your thoughts. Thanks again for all of your work with this :) |
Looks good to me! Thanks for updating and let me know if I can help with something else 👍🏼💪🏼 |
/azp run validateazcloud |
Azure Pipelines successfully started running 1 pipeline(s). |
Overview/Summary
The changes in this PR allow the user to set custom names for Azure Firewall, Virtual WAN Hubs, ExpressRoute Gateway, and VPN Gateway. Also, for readability, I added the
virtualWanOptionsType
so the array of objects is easier to understand, read and work with.Issue: #625
AB#30600
This PR fixes/adds/changes/removes
virtualWanOptionsType
parVpnGatewayCustomName
,parExpressRouteGatewayCustomName
,parAzFirewallCustomName
, andparVirtualWanHubCustomName
paramaters to specify custom resource names.mgDiagSettings.bicep
subnetOptionsType
nonComplianceMessageType
Breaking Changes
Not breaking due to the conditional values for VWAN module changes as well as new parameters using previously hard coded value
Testing Evidence
Succeeded deployment without custom names (as is situation):
Succeeded deployment with custom names:
As part of this Pull Request I have
.bicep
file/s I am adding/editing are using the latest API version possiblemain
branch