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

Trusted Launch for VM/VMSS #27260

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 15 additions & 2 deletions src/Compute/Compute.Test/ScenarioTests/VirtualMachineTests.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -6820,7 +6820,7 @@ function Test-VirtualMachineSecurityTypeWithoutConfig
{
# Setup
$rgname = Get-ComputeTestResourceName;
$loc = Get-ComputeVMLocation;
$loc = "eastus2euap";
try
{
New-AzResourceGroup -Name $rgname -Location $loc -Force;
Expand Down Expand Up @@ -6864,6 +6864,16 @@ function Test-VirtualMachineSecurityTypeWithoutConfig

Assert-AreEqual $updated_vm.SecurityProfile.UefiSettings.VTpmEnabled $true;

# Update SecurityType to Standard. Errors - Changing property 'securityProfile.securityType' is not allowed.
Stop-AzVM -ResourceGroupName $rgname -Name $vmname2 -Force
Update-AzVm -ResourceGroupName $rgname -VM $res -SecurityType "Standard"
Start-AzVM -ResourceGroupName $rgname -Name $vmname2
$updated_vm = Get-AzVM -ResourceGroupName $rgname -Name $vmname2;

Assert-Null $updated_vm.SecurityProfile.SecurityType;
Copy link
Member

Choose a reason for hiding this comment

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

shouldn't this be Standard now?

Copy link
Member Author

Choose a reason for hiding this comment

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

part of Trusted launch headache - they want Standard to be passed in for security type, because it is a valid input now and SecurityType:null will default to TL now.
But passing in SecurityType:Standard will return with SecurityProfile: null

Assert-Null $updated_vm.SecurityProfile.UefiSettings;
Assert-Null $updated_vm.SecurityProfile.SecurityType;

# validate GA extension
# We removed this logic as per request fro the feature team.
# Keeping this code here as this may be added back in the future.
Expand Down Expand Up @@ -6895,7 +6905,7 @@ function Test-VirtualMachineSecurityTypeStandard
{
Copy link
Member

Choose a reason for hiding this comment

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

also re-record
Test-VirtualMachineSecurityType
Test-VMDefaultsToTrustedLaunch
Test-VMDefaultsToTrustedLaunchWithManagedDisk
Test-VMDefaultsToTrustedLaunchWithNullEncryptionAtHost

Copy link
Member Author

Choose a reason for hiding this comment

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

VirtualMachineScaleSetDefaultImgWhenStandard
VirtualMachineScaleSetConfidentialVMSSSecurityType

Copy link
Member Author

Choose a reason for hiding this comment

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

Looks like Test-VMDefaultsToTrustedLaunchWithManagedDisk was never a working test and there is no recording for it. I am not sure how it exists without causing CI failure

Copy link
Member Author

Choose a reason for hiding this comment

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

Fails with " ErrorMessage: VM 'vmcrptestps4346' has not reported status for VM agent or extensions. Verify that the OS is up and healthy, the VM has a running VM agent, and that it can establish outbound connections to Azure storage. Please refer to https://aka.ms/vmextensionwindowstroubleshoot for additional VM agent troubleshooting information.
"

Copy link
Member Author

Choose a reason for hiding this comment

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

reproducable

# Setup
$rgname = Get-ComputeTestResourceName;
$loc = Get-ComputeVMLocation;
$loc = "Westus2"
try
{
New-AzResourceGroup -Name $rgname -Location $loc -Force;
Expand All @@ -6918,7 +6928,10 @@ function Test-VirtualMachineSecurityTypeStandard
New-AzVM -ResourceGroupName $rgname -Location $loc -Name $vmname1 -Credential $cred -Size $vmsize -Image $imageName -DomainNameLabel $domainNameLabel1 -SecurityType $securityTypeStnd;
# Verify security value
$vm1 = Get-AzVM -ResourceGroupName $rgname -Name $vmname1;

# VM Gets created with SecurityType: Standard but response has securityProfile null
Assert-Null $vm1.SecurityProfile;
#Assert-AreEqual $vm1.SecurityProfile.SecurityType "Standard";

# validate GA extension is not installed by default.
$extDefaultName = "GuestAttestation";
Expand Down

Large diffs are not rendered by default.

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions src/Compute/Compute/ChangeLog.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

-->
## Upcoming Release
* Updated `New-AzVM`, `New-AzVmss`, `Update-AzVM`, and `Update-AzVmss` to pass `Standard` as an input of `-SecurityType` parameter.
Copy link
Member

Choose a reason for hiding this comment

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

Where are the vmss tests? Those are needed.

* Added breaking change message for `Get-AzVMSize`.

## Version 9.1.0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2203,6 +2203,10 @@ private void BuildPutObject()
this.VirtualMachineScaleSet.VirtualMachineProfile.SecurityProfile.UefiSettings.VTpmEnabled = this.VirtualMachineScaleSet.VirtualMachineProfile.SecurityProfile.UefiSettings.VTpmEnabled == null ? true : this.EnableVtpm;
this.VirtualMachineScaleSet.VirtualMachineProfile.SecurityProfile.UefiSettings.SecureBootEnabled = this.VirtualMachineScaleSet.VirtualMachineProfile.SecurityProfile.UefiSettings.SecureBootEnabled == null ? true : this.EnableSecureBoot;
}
else
{
this.VirtualMachineScaleSet.VirtualMachineProfile.SecurityProfile.UefiSettings = null;
}
}
// Only used for SecurityType == TrustedLaunch
if (this.IsParameterBound(c => c.EnableVtpm))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -752,7 +752,7 @@ async Task SimpleParameterSetExecuteCmdlet(IAsyncCmdlet asyncCmdlet)
if (this.IsParameterBound(c => c.SecurityType)
&& this.SecurityType?.ToLower() == ConstantValues.StandardSecurityType)
{
this.SecurityType = null;
this.SecurityType = "Standard";
}

//TrustedLaunch value defaulting for UEFI values.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ internal static ResourceConfig<VirtualMachineScaleSet> CreateVirtualMachineScale
PlatformFaultDomainCount = platformFaultDomainCount,
VirtualMachineProfile = new VirtualMachineScaleSetVMProfile
{
SecurityProfile = ((encryptionAtHost == true || enableVtpm != null || enableSecureBoot != null || securityType != null) && (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.

so now Standard can have a security profile?

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah. Standard HAS to have security profile to pass SecurityProfile.SecurityType as Standard

SecurityProfile = (encryptionAtHost == true || enableVtpm != null || enableSecureBoot != null || securityType != null)
? new SecurityProfile
{
EncryptionAtHost = encryptionAtHost,
Expand Down Expand Up @@ -278,7 +278,7 @@ internal static ResourceConfig<VirtualMachineScaleSet> CreateVirtualMachineScale
PlatformFaultDomainCount = platformFaultDomainCount,
VirtualMachineProfile = new VirtualMachineScaleSetVMProfile
{
SecurityProfile = ((encryptionAtHost == true || enableVtpm != null || enableSecureBoot != null || securityType != null) && (securityType?.ToLower() != ConstantValues.StandardSecurityType))
SecurityProfile = (encryptionAtHost == true || enableVtpm != null || enableSecureBoot != null || securityType != null)
? new SecurityProfile
{
EncryptionAtHost = encryptionAtHost,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ public static ResourceConfig<VirtualMachine> CreateVirtualMachineConfig(
Priority = priority,
EvictionPolicy = evictionPolicy,
BillingProfile = (maxPrice == null) ? null : new BillingProfile(maxPrice),
SecurityProfile = ((encryptionAtHostPresent == true || enableVtpm != null || enableSecureBoot != null || securityType != null) && (securityType?.ToLower() != ConstantValues.StandardSecurityType))
SecurityProfile = (encryptionAtHostPresent == true || enableVtpm != null || enableSecureBoot != null || securityType != null)
? new SecurityProfile
{
EncryptionAtHost = encryptionAtHostPresent,
Expand Down Expand Up @@ -257,7 +257,7 @@ public static ResourceConfig<VirtualMachine> CreateVirtualMachineConfig(
Priority = priority,
EvictionPolicy = evictionPolicy,
BillingProfile = (maxPrice == null) ? null : new BillingProfile(maxPrice),
SecurityProfile = ((encryptionAtHostPresent == true || enableVtpm != null || enableSecureBoot != null || securityType!= null) && (securityType?.ToLower() != ConstantValues.StandardSecurityType))
SecurityProfile = (encryptionAtHostPresent == true || enableVtpm != null || enableSecureBoot != null || securityType!= null)
? new SecurityProfile
{
EncryptionAtHost = encryptionAtHostPresent,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -606,7 +606,7 @@ public async Task<ResourceConfig<VirtualMachine>> CreateConfigAsync()
&& _cmdlet.SecurityType != null
&& _cmdlet.SecurityType.ToString().ToLower() == ConstantValues.StandardSecurityType)
{
_cmdlet.SecurityType = null;
_cmdlet.SecurityType = "Standard";
}

var resourceGroup = ResourceGroupStrategy.CreateResourceGroupConfig(_cmdlet.ResourceGroupName);
Expand Down Expand Up @@ -1098,22 +1098,6 @@ public void DefaultExecuteCmdlet()
}
}

// Standard security type removing value since API does not support it yet.
if (this.VM.SecurityProfile?.SecurityType != null
&& this.VM.SecurityProfile?.SecurityType?.ToString().ToLower() == ConstantValues.StandardSecurityType)
{
if (this.VM.SecurityProfile.UefiSettings?.SecureBootEnabled == null
&& this.VM.SecurityProfile.UefiSettings?.VTpmEnabled == null
&& this.VM.SecurityProfile.EncryptionAtHost == null)
{
this.VM.SecurityProfile = null;
}
else
{
this.VM.SecurityProfile.SecurityType = null;
}
}

if (ShouldProcess(this.VM.Name, VerbsCommon.New))
{
ExecuteClientAction(() =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ public class UpdateAzureVMCommand : VirtualMachineBaseCmdlet
ValueFromPipelineByPropertyName = true,
Mandatory = false)]
[ValidateNotNullOrEmpty]
[PSArgumentCompleter("TrustedLaunch", "ConfidentialVM")]
[PSArgumentCompleter("TrustedLaunch", "ConfidentialVM", "Standard")]
public string SecurityType { get; set; }

[Parameter(
Expand Down Expand Up @@ -346,16 +346,21 @@ public override void ExecuteCmdlet()
{
parameters.SecurityProfile = new SecurityProfile();
}
if (parameters.SecurityProfile.UefiSettings == null)
{
parameters.SecurityProfile.UefiSettings = new UefiSettings();
}
parameters.SecurityProfile.SecurityType = this.SecurityType;

if (parameters.SecurityProfile.SecurityType == "TrustedLaunch" || parameters.SecurityProfile.SecurityType == "ConfidentialVM")
{
if (parameters.SecurityProfile.UefiSettings == null)
{
parameters.SecurityProfile.UefiSettings = new UefiSettings();
}
parameters.SecurityProfile.UefiSettings.VTpmEnabled = parameters.SecurityProfile.UefiSettings.VTpmEnabled == null ? true : this.EnableVtpm;
parameters.SecurityProfile.UefiSettings.SecureBootEnabled = parameters.SecurityProfile.UefiSettings.SecureBootEnabled == null ? true : this.EnableSecureBoot;
}
else
{
parameters.SecurityProfile.UefiSettings = null;
}
}

if (this.IsParameterBound(c => c.EnableVtpm))
Expand Down