Use Standard IP SKU by default for edge case with private_virtual_network_with_public_ip field
#469
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
JenGoldstrich
commented
Feb 21, 2025
| @@ -302,7 +302,18 @@ func GetVirtualMachineTemplateBuilder(config *Config) (*template.TemplateBuilder | |||
| } | |||
| } | |||
|
|
|||
| if config.VirtualNetworkName != "" && DefaultPrivateVirtualNetworkWithPublicIp != config.PrivateVirtualNetworkWithPublicIp { | |||
| // Standard is the default, basic will no longer be supported in the future | |||
| if config.PublicIpSKU != "Basic" { | |||
There was a problem hiding this comment.
We no longer need to check the vnet or private IP status here, because we simply moved the public IP sku block up above the check that removes the public IP. So if the public IP shouldn't be here we remove it when we call builder.SetVirtualNetwork
galapas1
approved these changes
Feb 21, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR fixes an issue raised where the
private_virtual_network_with_public_ipfield that allows users to bring their own vnet, and have the plugin create a public IP, subnet.This is a breaking change, as the default behavior for this for this field will change, but on March 31st 2025 https://azure.microsoft.com/en-us/updates?id=upgrade-to-standard-sku-public-ip-addresses-in-azure-by-30-september-2025-basic-sku-will-be-retired Azure is removing the ability to create these IPs, so we don't have much of a choice but to make a breaking change, users can roll back by setting the
public_ip_skufield tobasic, if they want to use the newest plugin version, but still create basic IP skus.Following up we should address this issue #468, which basically requests having the plugin creates the Network Security Group (NSG). But I didn't wanna delay rolling out this fix in the mean time.
The requestor had a good suggestion in the other linked issue to assign it at the NIC level, doing some reading I've learned that this can have some unexpected side effects as the NIC NSG can conflict with the Subnet NSG. The default behavior should be to encourage users to create their own NSGs in this situation, since they're managing their own vnet, but we should add support for creating an NSG at the NIC level, for users who do not want to create this security group (or those that do not know their build machine's IP ahead of time, and want to explicitly only allow that build VM)
Closes #458