New-AzVM bug: when image id is provided, fails to find version (Azure…

…#23897) * dev code * test trying * test manually works but cannot record bc gallery image not found * manual test * changelog * cleanup * cleanup
azure-sdk · Jan 29, 2024 · 858a3c9 · 858a3c9
1 parent 130f102
commit 858a3c9
Show file tree

Hide file tree

Showing 4 changed files with 260 additions and 46 deletions.
diff --git a/src/Compute/Compute.Test/ScenarioTests/VirtualMachineTests.cs b/src/Compute/Compute.Test/ScenarioTests/VirtualMachineTests.cs
@@ -626,5 +626,12 @@ public void TestVMDefaultsToTrustedLaunchWithNullEncryptionAtHost()
         {
             TestRunner.RunTestScript("Test-VMDefaultsToTrustedLaunchWithNullEncryptionAtHost");
         }
+
+        [Fact]
+        [Trait(Category.AcceptanceType, Category.LiveOnly)]
+        public void TestVMTLWithGallerySourceImage()
+        {
+            TestRunner.RunTestScript("Test-VMTLWithGallerySourceImage");
+        }
     }
 }
diff --git a/src/Compute/Compute.Test/ScenarioTests/VirtualMachineTests.ps1 b/src/Compute/Compute.Test/ScenarioTests/VirtualMachineTests.ps1
@@ -1,4 +1,4 @@
-# ----------------------------------------------------------------------------------
+# ----------------------------------------------------------------------------------
 #
 # Copyright Microsoft Corporation
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -7266,3 +7266,178 @@ function Test-VMDefaultsToTrustedLaunchWithNullEncryptionAtHost
         Clean-ResourceGroup $rgname;
     }
 }
+
+<#
+.SYNOPSIS
+Tests that a VM that is created from a Gallery Image source
+does not error out due to TL defaulting code that looks for the image  version 
+assuming it has a different format. 
+#>
+function Test-VMTLWithGallerySourceImage
+{
+    # Setup
+    $rgname = Get-ComputeTestResourceName;
+    $loc = Get-ComputeVMLocation;
+
+    try
+    {
+        $location = $loc;
+        New-AzResourceGroup -Name $rgname -Location $loc -Force;
+        # create credential 
+        $password = Get-PasswordForVM;
+        $securePassword = $password | ConvertTo-SecureString -AsPlainText -Force;  
+        $user = Get-ComputeTestResourceName;
+        $cred = New-Object System.Management.Automation.PSCredential ($user, $securePassword);
+
+        # Add one VM from creation 
+        $vmname = 'vm' + $rgname;
+        $domainNameLabel = "d1" + $rgname;
+        $securityType_TL = "TrustedLaunch";
+        $PublisherName = "MicrosoftWindowsServer";
+        $Offer = "WindowsServer";
+        $SKU = "2022-datacenter-azure-edition";
+        $version = "latest";
+        $disable = $false;
+        $enable = $true;
+        $galleryName = "g" + $rgname;
+        $vnetname = "vn" + $rgname;
+        $vnetAddress = "10.0.0.0/16";
+        $subnetname = "slb" + $rgname;
+        $subnetAddress = "10.0.2.0/24";
+        $pubipname = "p" + $rgname;
+        $OSDiskName = $vmname + "-osdisk";
+        $NICName = $vmname+ "-nic";
+        $NSGName = $vmname + "-NSG";
+        $nsgrulename = "nsr" + $rgname;
+        $OSDiskSizeinGB = 128;
+        $VMSize = "Standard_DS2_v2";
+        $vmname2 = "2" + $vmname;
+
+        # Gallery
+        $resourceGroup = $rgname
+        $galleryName = 'gl' + $rgname
+        $definitionName = 'def' + $rgname
+        $skuDetails = @{
+            Publisher = 'test'
+            Offer = 'test'
+            Sku = 'test'
+        }
+        $osType = 'Windows'
+        $osState = 'Specialized'
+        [bool]$trustedLaunch = $false
+        $storageAccountSku = 'Standard_LRS'
+        $hyperVGeneration = 'v1'
+
+        # create new VM
+        $paramNewAzVm = @{
+            ResourceGroupName = $resourceGroup
+            Name = $vmName
+            Credential = $cred
+            Location = $location
+            ErrorAction = 'Stop'
+        }
+        if ($trustedLaunch -eq $false) {
+            $paramNewAzVm.Add('SecurityType', 'Standard')
+        }
+        $vm = New-AzVM @paramNewAzVm
+
+        # Setup Image Gallery
+        New-AzGallery -ResourceGroupName $resourceGroup -Name $galleryName -location $location -ErrorAction 'Stop' | Out-Null
+
+        # Setup Image Definition
+        $paramNewAzImageDef = @{
+            ResourceGroupName = $resourceGroup
+            GalleryName       = $galleryName
+            Name              = $definitionName
+            Publisher         = $skuDetails.Publisher
+            Offer             = $skuDetails.Offer
+            Sku               = $skuDetails.Sku
+            Location          = $location
+            OSState           = $osState
+            OsType            = $osType
+            HyperVGeneration  = $hyperVGeneration
+            ErrorAction       = 'Stop'
+        }
+
+        New-AzGalleryImageDefinition @paramNewAzImageDef;
+
+        # Setup Image Version
+        $imageVersionName = "1.0.0";
+        $paramNewAzImageVer = @{
+            ResourceGroupName   = $resourceGroup
+            GalleryName         = $galleryName
+            GalleryImageDefinitionName  = $definitionName
+            Name                = $imageVersionName
+            Location            = $location
+            SourceImageId       = $vm.Id
+            ErrorAction         = 'Stop'
+            StorageAccountType  = $storageAccountSku
+            AsJob               = $true
+        }
+        New-AzGalleryImageVersion @paramNewAzImageVer | Out-Null;
+
+        $imageDefinition = Get-AzGalleryImageDefinition -ResourceGroupName $rgname -GalleryName $galleryName -Name $definitionName;
+
+        # Check image version status  
+        # Looping to wait for the provisioningState to go to Succeeded never ends despite 
+        # the status changing in portal. Image Definition is never seen by this PS script 
+        # despite it being there, so making this a manual test. 
+        Start-Sleep -Seconds 1000 ; 
+
+        # Vm
+        $vmSize = "Standard_D2s_v5";
+        # Network pieces
+        $subnetConfig = New-AzVirtualNetworkSubnetConfig `
+            -Name $subnetname `
+            -AddressPrefix $subnetAddress;
+        $vnet = New-AzVirtualNetwork `
+            -ResourceGroupName $rgName -Location $location -Name $vnetname -AddressPrefix $vnetAddress `
+            -Subnet $subnetConfig;
+        $pip = New-AzPublicIpAddress `
+            -ResourceGroupName $rgName `
+            -Location $location `
+            -Name $pubipname `
+            -AllocationMethod Static `
+            -IdleTimeoutInMinutes 4;
+        $nsgRuleRDP = New-AzNetworkSecurityRuleConfig `
+            -Name $nsgrulename  `
+            -Protocol Tcp `
+            -Direction Inbound `
+            -Priority 1000 `
+            -SourceAddressPrefix * `
+            -SourcePortRange * `
+            -DestinationAddressPrefix * `
+            -DestinationPortRange 3389 `
+            -Access Deny;
+        $nsg = New-AzNetworkSecurityGroup `
+            -ResourceGroupName $rgName `
+            -Location $location `
+            -Name $NSGName `
+            -SecurityRules $nsgRuleRDP;
+        $nic = New-AzNetworkInterface `
+            -Name $NICName `
+            -ResourceGroupName $rgName `
+            -Location $location `
+            -SubnetId $vnet.Subnets[0].Id `
+            -PublicIpAddressId $pip.Id `
+            -NetworkSecurityGroupId $nsg.Id;
+        $vm = New-AzVMConfig -vmName $vmname2  -vmSize $vmSize | `
+        Set-AzVMSourceImage -Id $imageDefinition.Id | `
+        Add-AzVMNetworkInterface -Id $nic.Id;
+
+        New-AzVM `
+            -ResourceGroupName $rgName `
+            -Location $location `
+            -VM $vm;
+
+        $vm = Get-AzVM -ResourceGroupName $rgname -Name $vmname2;
+
+        # Validate
+        Assert-Null $vm.SecurityProfile;
+    }
+    finally 
+    {
+        # Cleanup
+        Clean-ResourceGroup $rgname;
+    }
+}
diff --git a/src/Compute/Compute/ChangeLog.md b/src/Compute/Compute/ChangeLog.md
@@ -25,6 +25,7 @@
 * Fixed `New-AzVmss` to correctly work when using `-EdgeZone` by creating the Load Balancer in the correct edge zone.
 * Removed references to image aliases in `New-AzVM` and `New-AzVmss` to images that were removed.
 * Az.Compute is updated to use the 2023-09-01 ComputeRP REST API calls. 
+* Fixed `New-AzVM` when a source image is specified to avoid an error on the `Version` value. 
 
 ## Version 7.1.0
 * Added new parameter `-ElasticSanResourceId` to `New-AzSnapshotConfig` cmdlet.

diff --git a/src/Compute/Compute/VirtualMachine/Operation/NewAzureVMCommand.cs b/src/Compute/Compute/VirtualMachine/Operation/NewAzureVMCommand.cs
@@ -54,6 +54,8 @@
 using System.Security.AccessControl;
 using System.Security.Principal;
 using Microsoft.Azure.Commands.Common.Strategies.Compute;
+using System.Security.Policy;
+using System.Text.RegularExpressions;
 
 namespace Microsoft.Azure.Commands.Compute
 {
@@ -936,14 +938,12 @@ public void DefaultExecuteCmdlet()
              && this.VM.StorageProfile?.ImageReference?.SharedGalleryImageId == null) //had to add this
             {
                 defaultTrustedLaunchAndUefi();
-
                 setTrustedLaunchImage();
             }
-
             // Disk attached scenario for TL defaulting
             // Determines if the disk has SecurityType enabled.
             // If so, turns on TrustedLaunch for this VM.
-            if (this.VM.SecurityProfile?.SecurityType == null
+            else if (this.VM.SecurityProfile?.SecurityType == null
                 && this.VM.StorageProfile?.OsDisk?.ManagedDisk?.Id != null)
             {
                 var mDiskId = this.VM.StorageProfile?.OsDisk?.ManagedDisk.Id.ToString();
@@ -957,48 +957,65 @@ public void DefaultExecuteCmdlet()
                     defaultTrustedLaunchAndUefi();
                 }
             }
-
-            // Guest Attestation extension defaulting scenario check.
-            // And SecureBootEnabled and VtpmEnabled defaulting scenario.
-            if (this.VM.SecurityProfile?.SecurityType != null
-                && (this.VM.SecurityProfile?.SecurityType?.ToLower() == ConstantValues.TrustedLaunchSecurityType 
-                || this.VM.SecurityProfile?.SecurityType?.ToLower() == ConstantValues.ConfidentialVMSecurityType))
-            {
-                if (this.VM?.SecurityProfile?.UefiSettings != null)
-                {
-                    this.VM.SecurityProfile.UefiSettings.SecureBootEnabled = this.VM.SecurityProfile.UefiSettings.SecureBootEnabled ?? true;
-                    this.VM.SecurityProfile.UefiSettings.VTpmEnabled = this.VM.SecurityProfile.UefiSettings.VTpmEnabled ?? true;
-                }
-                else
-                {
-                    this.VM.SecurityProfile.UefiSettings = new UefiSettings(true, true);
-                }
-            }
-
-
             // ImageReference provided, TL defaulting occurs if image is Gen2. 
-            if (this.VM.SecurityProfile?.SecurityType == null
+            // This will handle when the Id is provided in a URI format and 
+            // when the image segments are provided individually.
+            else if (this.VM.SecurityProfile?.SecurityType == null
                 && this.VM.StorageProfile?.ImageReference != null)
             {
-                if (this.VM.StorageProfile?.ImageReference?.Id != null)//This code should never happen apparently
+                if (this.VM.StorageProfile?.ImageReference?.Id != null)
                 {
                     string imageRefString = this.VM.StorageProfile.ImageReference.Id.ToString();
 
-                    var parts = imageRefString.Split(new char[] { '/' }, StringSplitOptions.RemoveEmptyEntries);
-
-                    string imagePublisher = parts[Array.IndexOf(parts, "Publishers") + 1];
-                    string imageOffer = parts[Array.IndexOf(parts, "Offers") + 1];
-                    string imageSku = parts[Array.IndexOf(parts, "Skus") + 1];
-                    string imageVersion = parts[Array.IndexOf(parts, "Versions") + 1];
-                    //location is required when config object provided. 
-                    var imgResponse = ComputeClient.ComputeManagementClient.VirtualMachineImages.GetWithHttpMessagesAsync(
-                            this.Location.Canonicalize(),
-                            imagePublisher,
-                            imageOffer,
-                            imageSku,
-                            version: imageVersion).GetAwaiter().GetResult();
-
-                    setHyperVGenForImageCheckAndTLDefaulting(imgResponse);
+                    string galleryImgIdPattern = @"/subscriptions/(?<subscriptionId>[^/]+)/resourceGroups/(?<resourceGroup>[^/]+)/providers/Microsoft.Compute/galleries/(?<gallery>[^/]+)/images/(?<image>[^/]+)";
+                    string managedImageIdPattern = @"/subscriptions/(?<subscriptionId>[^/]+)/resourceGroups/(?<resourceGroup>[^/]+)/providers/Microsoft.Compute/images/(?<image>[^/]+)";
+                    string defaultExistingImagePattern = @"/Subscriptions/(?<subscriptionId>[^/]+)/Providers/Microsoft.Compute/Locations/(?<location>[^/]+)/Publishers/(?<publisher>[^/]+)/ArtifactTypes/VMImage/Offers/(?<offer>[^/]+)/Skus/(?<sku>[^/]+)/Versions/(?<version>[^/]+)";
+
+                    //Gallery Id
+                    Regex galleryRgx = new Regex(galleryImgIdPattern, RegexOptions.IgnoreCase);
+                    Match galleryMatch = galleryRgx.Match(imageRefString);
+                    // Managed Image Id
+                    Regex managedImageRgx = new Regex(managedImageIdPattern, RegexOptions.IgnoreCase);
+                    Match managedImageMatch = managedImageRgx.Match(imageRefString);
+                    // Default Image Id
+                    Regex defaultImageRgx = new Regex(defaultExistingImagePattern, RegexOptions.IgnoreCase);
+                    Match defaultImageMatch = defaultImageRgx.Match(imageRefString);
+
+                    if (defaultImageMatch.Success)
+                    {
+                        var parts = imageRefString.Split(new char[] { '/' }, StringSplitOptions.RemoveEmptyEntries);
+                        // It's a default existing image  
+                        string imagePublisher = parts[Array.IndexOf(parts, "Publishers") + 1];
+                        string imageOffer = parts[Array.IndexOf(parts, "Offers") + 1];
+                        string imageSku = parts[Array.IndexOf(parts, "Skus") + 1];
+                        string imageVersion = parts[Array.IndexOf(parts, "Versions") + 1];
+                        //location is required when config object provided. 
+                        var imgResponse = ComputeClient.ComputeManagementClient.VirtualMachineImages.GetWithHttpMessagesAsync(
+                                this.Location.Canonicalize(),
+                                imagePublisher,
+                                imageOffer,
+                                imageSku,
+                                version: imageVersion).GetAwaiter().GetResult();
+
+                        setHyperVGenForImageCheckAndTLDefaulting(imgResponse);
+                    }
+                    // This scenario might have additional logic added later, so making its own if check fo now.
+                    else if (galleryMatch.Success || managedImageMatch.Success)
+                    {
+                        // do nothing, send message to use TL.
+                        if (this.AsJobPresent() == false) // to avoid a failure when it is a job. Seems to fail when it is a job.
+                        {
+                            WriteInformation(HelpMessages.TrustedLaunchUpgradeMessage, new string[] { "PSHOST" });
+                        }
+                    }
+                    else
+                    {
+                        // Default behavior is to remind customer to use TrustedLaunch.
+                        if (this.AsJobPresent() == false) // to avoid a failure when it is a job. Seems to fail when it is a job.
+                        {
+                            WriteInformation(HelpMessages.TrustedLaunchUpgradeMessage, new string[] { "PSHOST" });
+                        }
+                    }
                 }
                 else
                 {
@@ -1007,17 +1024,31 @@ public void DefaultExecuteCmdlet()
                     setHyperVGenForImageCheckAndTLDefaulting(specificImageRespone);
                 }
             }
-
-            if (this.VM.SecurityProfile?.SecurityType == ConstantValues.TrustedLaunchSecurityType
-             && this.VM.StorageProfile?.ImageReference == null
-             && this.VM.StorageProfile?.OsDisk?.ManagedDisk?.Id == null //had to add this
-             && this.VM.StorageProfile?.ImageReference?.SharedGalleryImageId == null) 
+            else if (this.VM.SecurityProfile?.SecurityType == ConstantValues.TrustedLaunchSecurityType
+                && this.VM.StorageProfile?.ImageReference == null
+                && this.VM.StorageProfile?.OsDisk?.ManagedDisk?.Id == null //had to add this
+                && this.VM.StorageProfile?.ImageReference?.SharedGalleryImageId == null)
             {
                 defaultTrustedLaunchAndUefi();
-
                 setTrustedLaunchImage();
             }
 
+            // SecureBootEnabled and VtpmEnabled defaulting scenario.
+            if (this.VM.SecurityProfile?.SecurityType != null
+                && (this.VM.SecurityProfile?.SecurityType?.ToLower() == ConstantValues.TrustedLaunchSecurityType 
+                || this.VM.SecurityProfile?.SecurityType?.ToLower() == ConstantValues.ConfidentialVMSecurityType))
+            {
+                if (this.VM?.SecurityProfile?.UefiSettings != null)
+                {
+                    this.VM.SecurityProfile.UefiSettings.SecureBootEnabled = this.VM.SecurityProfile.UefiSettings.SecureBootEnabled ?? true;
+                    this.VM.SecurityProfile.UefiSettings.VTpmEnabled = this.VM.SecurityProfile.UefiSettings.VTpmEnabled ?? true;
+                }
+                else
+                {
+                    this.VM.SecurityProfile.UefiSettings = new UefiSettings(true, true);
+                }
+            }
+
             // 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)