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

Make TrustedLaunch default in for VM and VMSS creation #22974

Merged
merged 45 commits into from
Nov 7, 2023

Conversation

Sandido
Copy link
Member

@Sandido Sandido commented Oct 13, 2023

Description

I still have a lot of cleaning up to do, and the new default value requires me to re-record many PS tests, but the functionality is all there.
When no SecurityType is provided, TrustedLaunch and its requirements will be defaulted in.
If no Image is provided, the 2022AzureEdition image, compatible with TL, will be defaulted in.
Resources made from a SharedGalleryImage are not considered for this feature.
Resources made from an Image that is Gen1 (HyperVGeneration V1) are not considered for this feature.
PM for this feature is [email protected].

I have had issues testing the managed disk scenario (3) as the request always times out (VMAgentStatus and os health is never good), but it seems to probably be working.

This feature consists of 4 different feature requests:

  1. https://github.com/Azure/azure-powershell-cmdlet-review-pr/issues/1240
    For Vm and Vmss creation, with minimal inputs, the default will be SecurityType = TrustedLaunch, with UEFI settings also set to true for TL if the user had not manually set the securitytype.

  2. https://github.com/Azure/azure-powershell-cmdlet-review-pr/issues/1248
    For a new vm disk created from an Image, I query for that image and determine if it is HyperVGeneration of V2. If yes, then default to trusted launch turned on if the user had not manually set the securitytype.

  3. https://github.com/Azure/azure-powershell-cmdlet-review-pr/issues/1243
    For VMs created from a managed disk for its os disk, it checks to see if the disk supports trusted launch, and if yes it will default the VM to trusted launch if the related values were not manually set by the user.

  4. https://github.com/Azure/azure-powershell-cmdlet-review-pr/issues/1241
    This impacts the Config parameter sets for VM and Vmss creation because it requires the user to set the ImageReference values, specifically not the Id as marketplace images like 2022-datacenter-azure-edition do not have an Id. This defaulting only occurs if the user did not manually set the TL related values.

As this feature changes the default behavior for VM and Vmss creation when SecurityType is null (which is typical) then I had to update many tests to set SecurityType to "Standard". This is seen in the client logic and prevents TrustedLaunch defaulting logic, but since the API does not support Standard it is nulled out before the .Net SDK call is made to create the VM or VMSS.

I ended up re-recording many tests without this Standard value to ensure previous behavior with this new defaulting is not broken, hence the many new json files.

Checklist

  • SHOULD select appropriate branch. Cmdlets from Autorest.PowerShell should go to generation branch.
  • SHOULD make the title of PR clear and informative, and in the present imperative tense.
  • SHOULD update ChangeLog.md file(s) appropriately
    • 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 in the past tense. Add changelog in description section if PR goes into generation branch.
    • Should not change ChangeLog.md if no new release is required, such as fixing test case only.
  • SHOULD have approved design review for the changes in this repository (Microsoft internal only) with following situations
    • Create new module from scratch
    • Create new resource types which are not easy to conform to Azure PowerShell Design Guidelines
    • Create new resource type which name doesn't use module name as prefix
    • Have design question before implementation
  • SHOULD regenerate markdown help files if there is cmdlet API change. Instruction
  • SHOULD have proper test coverage for changes in pull request.
  • SHOULD NOT introduce breaking changes in Az minor release except preview version.
  • SHOULD NOT adjust version of module manually in pull request

@azure-client-tools-bot-prd
Copy link

azure-client-tools-bot-prd bot commented Oct 13, 2023

️✔️Az.Accounts
️✔️Build
️✔️PowerShell Core - Windows
️✔️Windows PowerShell - Windows
️✔️Az.ApplicationInsights
️✔️Build
️✔️PowerShell Core - Windows
️✔️Windows PowerShell - Windows
⚠️Az.Compute
️✔️Build
️✔️PowerShell Core - Windows
️✔️Windows PowerShell - Windows
️✔️Breaking Change Check
️✔️PowerShell Core - Windows
️✔️Windows PowerShell - Windows
️✔️Signature Check
️✔️PowerShell Core - Windows
️✔️Windows PowerShell - Windows
⚠️Help Example Check
⚠️PowerShell Core - Windows
Type Cmdlet Example Line RuleName Description Extent Remediation
⚠️ New-AzVmssConfig 1 1 Unassigned_Variable New-AzVmssConfig -Location $Loc is a null-valued parameter value. -Location Assign value for $Loc.
⚠️ New-AzVmssConfig 1 1 Unassigned_Variable New-AzVmssConfig -NetworkInterfaceConfiguration $NetCfg is a null-valued parameter value. -NetworkInterfaceConfiguration Assign value for $NetCfg.
⚠️ New-AzVmssConfig 1 2 Unassigned_Variable Add-AzVmssNetworkInterfaceConfiguration -IPConfiguration $IPCfg is a null-valued parameter value. -IPConfiguration Assign value for $IPCfg.
⚠️ New-AzVmssConfig 1 3 Unassigned_Variable Set-AzVmssOsProfile -AdminUsername $adminUsername is a null-valued parameter value. -AdminUsername Assign value for $adminUsername.
⚠️ New-AzVmssConfig 1 3 Unassigned_Variable Set-AzVmssOsProfile -AdminPassword $AdminPassword is a null-valued parameter value. -AdminPassword Assign value for $AdminPassword.
⚠️ New-AzVmssConfig 1 5 Unassigned_Variable Set-AzVmssStorageProfile -ImageReferenceOffer $ImgRef.Offer is a null-valued parameter value. -ImageReferenceOffer Assign value for $ImgRef.Offer.
⚠️ New-AzVmssConfig 1 5 Unassigned_Variable Set-AzVmssStorageProfile -ImageReferenceSku $ImgRef.Skus is a null-valued parameter value. -ImageReferenceSku Assign value for $ImgRef.Skus.
⚠️ New-AzVmssConfig 1 5 Unassigned_Variable Set-AzVmssStorageProfile -ImageReferenceVersion $ImgRef.Version is a null-valued parameter value. -ImageReferenceVersion Assign value for $ImgRef.Version.
⚠️ New-AzVmssConfig 1 6 Unassigned_Variable Set-AzVmssStorageProfile -ImageReferencePublisher $ImgRef.PublisherName is a null-valued parameter value. -ImageReferencePublisher Assign value for $ImgRef.PublisherName.
⚠️ New-AzVmssConfig 1 6 Unassigned_Variable Set-AzVmssStorageProfile -VhdContainer $VHDContainer is a null-valued parameter value. -VhdContainer Assign value for $VHDContainer.
⚠️ New-AzVmssConfig 1 7 Unassigned_Variable Add-AzVmssAdditionalUnattendContent -ComponentName $AUCComponentName is a null-valued parameter value. -ComponentName Assign value for $AUCComponentName.
⚠️ New-AzVmssConfig 1 7 Unassigned_Variable Add-AzVmssAdditionalUnattendContent -Content $AUCContent is a null-valued parameter value. -Content Assign value for $AUCContent.
⚠️ New-AzVmssConfig 1 7 Unassigned_Variable Add-AzVmssAdditionalUnattendContent -PassName $AUCPassName is a null-valued parameter value. -PassName Assign value for $AUCPassName.
⚠️ New-AzVmssConfig 1 7 Unassigned_Variable Add-AzVmssAdditionalUnattendContent -SettingName $AUCSetting is a null-valued parameter value. -SettingName Assign value for $AUCSetting.
⚠️ New-AzVmssConfig 1 9 Unassigned_Variable New-AzVmss -ResourceGroupName $RGName is a null-valued parameter value. -ResourceGroupName Assign value for $RGName.
⚠️ New-AzVmssConfig 1 9 Unassigned_Variable New-AzVmss -Name $VMSSName is a null-valued parameter value. -Name Assign value for $VMSSName.
⚠️ New-AzVmssConfig 4 19 Unassigned_Variable New-AzStorageAccount -ResourceGroupName $rgname is a null-valued parameter value. -ResourceGroupName Assign value for $rgname.
⚠️ New-AzVmssConfig 4 19 Unassigned_Variable New-AzStorageAccount -Location $loc is a null-valued parameter value. -Location Assign value for $loc.
⚠️ New-AzVmssConfig 4 20 Unassigned_Variable Get-AzStorageAccount -ResourceGroupName $rgname is a null-valued parameter value. -ResourceGroupName Assign value for $rgname.
⚠️ New-AzVmssConfig 4 24 Unassigned_Variable New-AzVirtualNetwork -ResourceGroupName $rgname is a null-valued parameter value. -ResourceGroupName Assign value for $rgname.
⚠️ New-AzVmssConfig 4 24 Unassigned_Variable New-AzVirtualNetwork -Location $loc is a null-valued parameter value. -Location Assign value for $loc.
⚠️ New-AzVmssConfig 4 25 Unassigned_Variable Get-AzVirtualNetwork -ResourceGroupName $rgname is a null-valued parameter value. -ResourceGroupName Assign value for $rgname.
⚠️ New-AzVmssConfig 4 31 Unassigned_Variable New-AzVmssConfig -Location $loc is a null-valued parameter value. -Location Assign value for $loc.
⚠️ New-AzVmssConfig 4 38 Unassigned_Variable New-AzVmss -ResourceGroupName $rgname is a null-valued parameter value. -ResourceGroupName Assign value for $rgname.
⚠️ New-AzVmssConfig 4 40 Unassigned_Variable Get-AzVmss -ResourceGroupName $rgname is a null-valued parameter value. -ResourceGroupName Assign value for $rgname.
⚠️ New-AzVmssConfig 5 36 Mismatched_Parameter_Value_Type Set-AzVmssOsProfile -AdminPassword $adminPassword is not an expected parameter value type. -AdminPassword Use correct parameter value type. Expected Type is string. Now the type is securestring.(Command).
⚠️ Set-AzDiskSecurityProfile 2 11 Unassigned_Variable New-AzKeyVault -ResourceGroupName $ResourceGroupName is a null-valued parameter value. -ResourceGroupName Assign value for $ResourceGroupName.
⚠️ Set-AzDiskSecurityProfile 2 17 Unassigned_Variable Get-AzKeyVault -ResourceGroupName $ResourceGroupName is a null-valued parameter value. -ResourceGroupName Assign value for $ResourceGroupName.
⚠️ Set-AzDiskSecurityProfile 2 23 Unassigned_Variable New-AzDiskEncryptionSet -ResourceGroupName $ResourceGroupName is a null-valued parameter value. -ResourceGroupName Assign value for $ResourceGroupName.
⚠️ Set-AzDiskSecurityProfile 2 24 Unassigned_Variable Get-AzDiskEncryptionSet -ResourceGroupName $ResourceGroupName is a null-valued parameter value. -ResourceGroupName Assign value for $ResourceGroupName.
⚠️ Set-AzDiskSecurityProfile 2 27 Unassigned_Variable Get-AzDiskEncryptionSet -ResourceGroupName $ResourceGroupName is a null-valued parameter value. -ResourceGroupName Assign value for $ResourceGroupName.
⚠️ Set-AzDiskSecurityProfile 2 28 Unassigned_Variable Set-AzKeyVaultAccessPolicy -ResourceGroupName $ResourceGroupName is a null-valued parameter value. -ResourceGroupName Assign value for $ResourceGroupName.
⚠️ Set-AzDiskSecurityProfile 2 35 Unassigned_Variable New-AzDisk -ResourceGroupName $ResourceGroupName is a null-valued parameter value. -ResourceGroupName Assign value for $ResourceGroupName.
⚠️ Set-AzDiskSecurityProfile 2 36 Unassigned_Variable Get-AzDisk -ResourceGroupName $ResourceGroupName is a null-valued parameter value. -ResourceGroupName Assign value for $ResourceGroupName.
⚠️Windows PowerShell - Windows
Type Cmdlet Example Line RuleName Description Extent Remediation
⚠️ New-AzVmssConfig 1 1 Unassigned_Variable New-AzVmssConfig -Location $Loc is a null-valued parameter value. -Location Assign value for $Loc.
⚠️ New-AzVmssConfig 1 1 Unassigned_Variable New-AzVmssConfig -NetworkInterfaceConfiguration $NetCfg is a null-valued parameter value. -NetworkInterfaceConfiguration Assign value for $NetCfg.
⚠️ New-AzVmssConfig 1 2 Unassigned_Variable Add-AzVmssNetworkInterfaceConfiguration -IPConfiguration $IPCfg is a null-valued parameter value. -IPConfiguration Assign value for $IPCfg.
⚠️ New-AzVmssConfig 1 3 Unassigned_Variable Set-AzVmssOsProfile -AdminUsername $adminUsername is a null-valued parameter value. -AdminUsername Assign value for $adminUsername.
⚠️ New-AzVmssConfig 1 3 Unassigned_Variable Set-AzVmssOsProfile -AdminPassword $AdminPassword is a null-valued parameter value. -AdminPassword Assign value for $AdminPassword.
⚠️ New-AzVmssConfig 1 5 Unassigned_Variable Set-AzVmssStorageProfile -ImageReferenceOffer $ImgRef.Offer is a null-valued parameter value. -ImageReferenceOffer Assign value for $ImgRef.Offer.
⚠️ New-AzVmssConfig 1 5 Unassigned_Variable Set-AzVmssStorageProfile -ImageReferenceSku $ImgRef.Skus is a null-valued parameter value. -ImageReferenceSku Assign value for $ImgRef.Skus.
⚠️ New-AzVmssConfig 1 5 Unassigned_Variable Set-AzVmssStorageProfile -ImageReferenceVersion $ImgRef.Version is a null-valued parameter value. -ImageReferenceVersion Assign value for $ImgRef.Version.
⚠️ New-AzVmssConfig 1 6 Unassigned_Variable Set-AzVmssStorageProfile -ImageReferencePublisher $ImgRef.PublisherName is a null-valued parameter value. -ImageReferencePublisher Assign value for $ImgRef.PublisherName.
⚠️ New-AzVmssConfig 1 6 Unassigned_Variable Set-AzVmssStorageProfile -VhdContainer $VHDContainer is a null-valued parameter value. -VhdContainer Assign value for $VHDContainer.
⚠️ New-AzVmssConfig 1 7 Unassigned_Variable Add-AzVmssAdditionalUnattendContent -ComponentName $AUCComponentName is a null-valued parameter value. -ComponentName Assign value for $AUCComponentName.
⚠️ New-AzVmssConfig 1 7 Unassigned_Variable Add-AzVmssAdditionalUnattendContent -Content $AUCContent is a null-valued parameter value. -Content Assign value for $AUCContent.
⚠️ New-AzVmssConfig 1 7 Unassigned_Variable Add-AzVmssAdditionalUnattendContent -PassName $AUCPassName is a null-valued parameter value. -PassName Assign value for $AUCPassName.
⚠️ New-AzVmssConfig 1 7 Unassigned_Variable Add-AzVmssAdditionalUnattendContent -SettingName $AUCSetting is a null-valued parameter value. -SettingName Assign value for $AUCSetting.
⚠️ New-AzVmssConfig 1 9 Unassigned_Variable New-AzVmss -ResourceGroupName $RGName is a null-valued parameter value. -ResourceGroupName Assign value for $RGName.
⚠️ New-AzVmssConfig 1 9 Unassigned_Variable New-AzVmss -Name $VMSSName is a null-valued parameter value. -Name Assign value for $VMSSName.
⚠️ New-AzVmssConfig 4 19 Unassigned_Variable New-AzStorageAccount -ResourceGroupName $rgname is a null-valued parameter value. -ResourceGroupName Assign value for $rgname.
⚠️ New-AzVmssConfig 4 19 Unassigned_Variable New-AzStorageAccount -Location $loc is a null-valued parameter value. -Location Assign value for $loc.
⚠️ New-AzVmssConfig 4 20 Unassigned_Variable Get-AzStorageAccount -ResourceGroupName $rgname is a null-valued parameter value. -ResourceGroupName Assign value for $rgname.
⚠️ New-AzVmssConfig 4 24 Unassigned_Variable New-AzVirtualNetwork -ResourceGroupName $rgname is a null-valued parameter value. -ResourceGroupName Assign value for $rgname.
⚠️ New-AzVmssConfig 4 24 Unassigned_Variable New-AzVirtualNetwork -Location $loc is a null-valued parameter value. -Location Assign value for $loc.
⚠️ New-AzVmssConfig 4 25 Unassigned_Variable Get-AzVirtualNetwork -ResourceGroupName $rgname is a null-valued parameter value. -ResourceGroupName Assign value for $rgname.
⚠️ New-AzVmssConfig 4 31 Unassigned_Variable New-AzVmssConfig -Location $loc is a null-valued parameter value. -Location Assign value for $loc.
⚠️ New-AzVmssConfig 4 38 Unassigned_Variable New-AzVmss -ResourceGroupName $rgname is a null-valued parameter value. -ResourceGroupName Assign value for $rgname.
⚠️ New-AzVmssConfig 4 40 Unassigned_Variable Get-AzVmss -ResourceGroupName $rgname is a null-valued parameter value. -ResourceGroupName Assign value for $rgname.
⚠️ New-AzVmssConfig 5 36 Mismatched_Parameter_Value_Type Set-AzVmssOsProfile -AdminPassword $adminPassword is not an expected parameter value type. -AdminPassword Use correct parameter value type. Expected Type is string. Now the type is securestring.(Command).
⚠️ Set-AzDiskSecurityProfile 2 11 Unassigned_Variable New-AzKeyVault -ResourceGroupName $ResourceGroupName is a null-valued parameter value. -ResourceGroupName Assign value for $ResourceGroupName.
⚠️ Set-AzDiskSecurityProfile 2 17 Unassigned_Variable Get-AzKeyVault -ResourceGroupName $ResourceGroupName is a null-valued parameter value. -ResourceGroupName Assign value for $ResourceGroupName.
⚠️ Set-AzDiskSecurityProfile 2 23 Unassigned_Variable New-AzDiskEncryptionSet -ResourceGroupName $ResourceGroupName is a null-valued parameter value. -ResourceGroupName Assign value for $ResourceGroupName.
⚠️ Set-AzDiskSecurityProfile 2 24 Unassigned_Variable Get-AzDiskEncryptionSet -ResourceGroupName $ResourceGroupName is a null-valued parameter value. -ResourceGroupName Assign value for $ResourceGroupName.
⚠️ Set-AzDiskSecurityProfile 2 27 Unassigned_Variable Get-AzDiskEncryptionSet -ResourceGroupName $ResourceGroupName is a null-valued parameter value. -ResourceGroupName Assign value for $ResourceGroupName.
⚠️ Set-AzDiskSecurityProfile 2 28 Unassigned_Variable Set-AzKeyVaultAccessPolicy -ResourceGroupName $ResourceGroupName is a null-valued parameter value. -ResourceGroupName Assign value for $ResourceGroupName.
⚠️ Set-AzDiskSecurityProfile 2 35 Unassigned_Variable New-AzDisk -ResourceGroupName $ResourceGroupName is a null-valued parameter value. -ResourceGroupName Assign value for $ResourceGroupName.
⚠️ Set-AzDiskSecurityProfile 2 36 Unassigned_Variable Get-AzDisk -ResourceGroupName $ResourceGroupName is a null-valued parameter value. -ResourceGroupName Assign value for $ResourceGroupName.
️✔️Help File Existence Check
️✔️PowerShell Core - Windows
️✔️Windows PowerShell - Windows
️✔️File Change Check
️✔️PowerShell Core - Windows
️✔️Windows PowerShell - Windows
️✔️UX Metadata Check
️✔️PowerShell Core - Windows
️✔️Windows PowerShell - Windows
️✔️Test
️✔️PowerShell Core - Linux
️✔️PowerShell Core - MacOS
️✔️PowerShell Core - Windows
️✔️Windows PowerShell - Windows
️✔️Az.EventHub
️✔️Build
️✔️PowerShell Core - Windows
️✔️Windows PowerShell - Windows
️✔️Az.KeyVault
️✔️Build
️✔️PowerShell Core - Windows
️✔️Windows PowerShell - Windows
️✔️Az.ManagedServiceIdentity
️✔️Build
️✔️PowerShell Core - Windows
️✔️Windows PowerShell - Windows
️✔️Az.Monitor
️✔️Build
️✔️PowerShell Core - Windows
️✔️Windows PowerShell - Windows
️✔️Az.Network
️✔️Build
️✔️PowerShell Core - Windows
️✔️Windows PowerShell - Windows
️✔️Test
️✔️PowerShell Core - Linux
️✔️PowerShell Core - MacOS
️✔️PowerShell Core - Windows
️✔️Windows PowerShell - Windows
️✔️Az.OperationalInsights
️✔️Build
️✔️PowerShell Core - Windows
️✔️Windows PowerShell - Windows
️✔️Az.PrivateDns
️✔️Build
️✔️PowerShell Core - Windows
️✔️Windows PowerShell - Windows
️✔️Az.RecoveryServices
️✔️Build
️✔️PowerShell Core - Windows
️✔️Windows PowerShell - Windows
️✔️Test
️✔️PowerShell Core - Linux
️✔️PowerShell Core - MacOS
️✔️PowerShell Core - Windows
️✔️Windows PowerShell - Windows
️✔️Az.Security
️✔️Build
️✔️PowerShell Core - Windows
️✔️Windows PowerShell - Windows
️✔️Test
️✔️PowerShell Core - Linux
️✔️PowerShell Core - MacOS
️✔️PowerShell Core - Windows
️✔️Windows PowerShell - Windows
️✔️Az.Sql
️✔️Build
️✔️PowerShell Core - Windows
️✔️Windows PowerShell - Windows
️✔️Az.Ssh
️✔️Build
️✔️PowerShell Core - Windows
️✔️Windows PowerShell - Windows
️✔️Test
️✔️PowerShell Core - Linux
️✔️PowerShell Core - MacOS
️✔️PowerShell Core - Windows
️✔️Windows PowerShell - Windows
️✔️Az.Storage
️✔️Build
️✔️PowerShell Core - Windows
️✔️Windows PowerShell - Windows

@Sandido
Copy link
Member Author

Sandido commented Nov 5, 2023

@VeryEarly , heads up, this will be out for review out of default for tonight.

@Sandido
Copy link
Member Author

Sandido commented Nov 6, 2023

all checks and tests currently pass. More commits incoming.

vVirtualMachineProfile.SecurityProfile.UefiSettings.VTpmEnabled = vVirtualMachineProfile.SecurityProfile.UefiSettings.VTpmEnabled == null ? true : this.EnableVtpm;
vVirtualMachineProfile.SecurityProfile.UefiSettings.SecureBootEnabled = vVirtualMachineProfile.SecurityProfile.UefiSettings.SecureBootEnabled == null ? true : this.EnableSecureBoot;
}
}

if (this.IsParameterBound(c => c.SecurityType) && this.SecurityType?.ToLower() == ConstantValues.StandardSecurityType)
Copy link
Member

Choose a reason for hiding this comment

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

what is this.IsParameterBound(c => c.SecurityType) but not this.SecurityType?.ToLower() == ConstantValues.StandardSecurityType ?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think you're right that this can be removed and just use the above code but again, remove the 2nd conditional from the above if.

@@ -72,6 +72,19 @@ private void Run()
}
this.VirtualMachineScaleSet.VirtualMachineProfile.SecurityProfile.SecurityType = this.SecurityType;
}
else if (this.IsParameterBound(c => c.SecurityType) && this.SecurityType?.ToLower() == ConstantValues.StandardSecurityType)
Copy link
Member

Choose a reason for hiding this comment

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

can we just take out the second conditional in line 62 instead?

vm.SecurityProfile.UefiSettings.VTpmEnabled = vm.SecurityProfile.UefiSettings.VTpmEnabled == null ? true : this.EnableVtpm;
vm.SecurityProfile.UefiSettings.SecureBootEnabled = vm.SecurityProfile.UefiSettings.SecureBootEnabled == null ? true : this.EnableSecureBoot;
}
}

if (this.SecurityType != null && this.SecurityType?.ToLower() == ConstantValues.StandardSecurityType)
Copy link
Member

Choose a reason for hiding this comment

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

what if this.SecurityType != null but not this.SecurityType?.ToLower() == ConstantValues.StandardSecurityType

Copy link
Member Author

Choose a reason for hiding this comment

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

Then the above code block would be executed. I think this could just be an Else If of the above.

if (!this.IsParameterBound(c => c.SecurityType)
&& !this.IsParameterBound(c => c.ImageName)
&& !this.IsParameterBound(c => c.ImageReferenceId)
&& !this.IsParameterBound(c => c.SharedGalleryImageId))
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

my changes are in VirtualMachineScaleSetStrategy.cs

grizzlytheodore
grizzlytheodore previously approved these changes Nov 6, 2023
Copy link
Member

@grizzlytheodore grizzlytheodore left a comment

Choose a reason for hiding this comment

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

some design comments, other than that LGTM.
However, a very complicated PR. Would want to rely heavily on tests that new features work but also unrelated parts remain working

@@ -44,6 +44,12 @@
}
},
"Windows": {
"Win2022AzureEdition": {
Copy link
Member Author

Choose a reason for hiding this comment

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

unsure if I need this anymore

@VeryEarly VeryEarly merged commit 8abfd3c into main Nov 7, 2023
12 checks passed
@VeryEarly VeryEarly deleted the adsandor/tldefault branch November 7, 2023 05:08
@Sandido Sandido restored the adsandor/tldefault branch November 7, 2023 18:45
@isra-fel isra-fel added the Breaking Change Release This PR contains breaking change label Nov 8, 2023
Copy link

github-actions bot commented Nov 8, 2023

To the author of the pull request,
This PR was labeled "Breaking Change Release" because it contains breaking changes.
According to our policy, breaking changes can only take place during major release and they must be preannounced.
Please follow our guide on the detailed steps.

@Sandido Sandido mentioned this pull request Nov 9, 2023
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Breaking Change Release This PR contains breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants