[KeyVault] Handle Nullable Parameters for Certificate Auto-Renewal in…

… Set-AzKeyVaultCertificatePolicy (#25844) * Added manual validation for piped attributes * Updated ChangeLog * Validate using a helper function * Added back ValueFromPipeline Testing * Removed ValueFromPipelineProperty * Added Suggested Changes * Dummy Commit * Added Pester Testing * Added CI error suppression * Updated Help Docs for cmdlet --------- Co-authored-by: Yash Patil <[email protected]>
Azure · Sep 9, 2024 · 1219f92 · 1219f92
1 parent 7fe801e
commit 1219f92
Show file tree

Hide file tree

Showing 7 changed files with 130 additions and 9 deletions.
diff --git a/src/KeyVault/KeyVault.Test/PesterTests/CertificatePolicy.Tests.ps1 b/src/KeyVault/KeyVault.Test/PesterTests/CertificatePolicy.Tests.ps1
@@ -0,0 +1,58 @@
+# CertificatePolicy.Tests.ps1
+
+BeforeAll {
+    $vaultName = 'yash-kv'
+    . "$PSScriptRoot\..\Scripts\Common.ps1" # Common setup script
+
+    $psd1Path = Join-Path $PSScriptRoot "../../../../artifacts/Debug/" -Resolve
+    $keyVaultPsd1 = Join-Path $psd1Path "./Az.KeyVault/Az.KeyVault.psd1" -Resolve
+    Import-Module $keyVaultPsd1 -Force
+}
+
+Describe "Set-AzKeyVaultCertificatePolicy Null Handling" {
+    Context "When setting null for RenewAtNumberOfDaysBeforeExpiry and RenewAtPercentageLifetime" {
+
+        It "Should not throw an error when setting null values" {
+
+            # Arrange: Generate a random certificate name
+            $certName = Get-CertificateName -suffix (Get-Random)
+
+            # Retrieve Key Vault & Certificate
+            $KV = Get-AzKeyVault -VaultName $vaultName
+            $cert = $KV | Get-AzKeyVaultCertificate -Name $certName
+
+            if ($cert -eq $null) {
+                # Create a certificate if it doesn't exist
+                $policy = New-AzKeyVaultCertificatePolicy `
+                    -SubjectName "CN=$certName" `
+                    -IssuerName "Self" `
+                    -ValidityInMonths 12
+
+                $cert = Add-AzKeyVaultCertificate -VaultName $vaultName -Name $certName -CertificatePolicy $policy
+            }
+
+            # Retrieve Cert & Certificate Policy
+            $cert = $KV | Get-AzKeyVaultCertificate -Name $certName
+            $policy = $cert | Get-AzKeyVaultCertificatePolicy
+
+            # Act: Set null for RenewAtPercentageLifetime and some value for RenewAtNumberOfDaysBeforeExpiry 
+            $policy.RenewAtNumberOfDaysBeforeExpiry = 25
+            $policy.RenewAtPercentageLifetime = $null
+
+            # Apply policy and verify no errors
+            $policy | Set-AzKeyVaultCertificatePolicy -VaultName $vaultName -Name $certName
+
+            # Retrieve updated policy
+            $updatedCert = $KV | Get-AzKeyVaultCertificate -Name $certName
+            $updatedPolicy = $updatedCert | Get-AzKeyVaultCertificatePolicy
+
+            # Clean up the created resources
+            Remove-AzKeyVaultCertificate -VaultName $vaultName -Name $certName -Force
+
+            # Assert: Check if the properties have been set to null
+            $updatedPolicy.RenewAtNumberOfDaysBeforeExpiry | Should -Be 25
+            $updatedPolicy.RenewAtPercentageLifetime | Should -Be $null
+        }
+    }
+}
+
diff --git a/src/KeyVault/KeyVault/ChangeLog.md b/src/KeyVault/KeyVault/ChangeLog.md
@@ -18,6 +18,7 @@
         - Additional information about change #1
 -->
 ## Upcoming Release
+* Fixed a parameter validation issue in Set-AzureKeyVaultCertificatePolicy. [#25649]
 
 ## Version 6.1.0
 * Fixed secrets exposure in example documentation.

diff --git a/src/KeyVault/KeyVault/Commands/Certificate/Policy/SetAzureKeyVaultCertificatePolicy.cs b/src/KeyVault/KeyVault/Commands/Certificate/Policy/SetAzureKeyVaultCertificatePolicy.cs
@@ -20,6 +20,7 @@
 using System.Management.Automation;
 using System.Security.Cryptography;
 using System.Security.Cryptography.X509Certificates;
+using Microsoft.Azure.Commands.Common.Exceptions;
 using PSKeyVaultProperties = Microsoft.Azure.Commands.KeyVault.Properties;
 
 namespace Microsoft.Azure.Commands.KeyVault
@@ -81,7 +82,6 @@ public class SetAzureKeyVaultCertificatePolicy : KeyVaultCmdletBase
         [Parameter(Mandatory = true,
             ParameterSetName = ExpandedRenewNumberParameterSet,
             HelpMessage = "Specifies the number of days before expiration when automatic renewal should start.")]
-        [ValidateRange(1, int.MaxValue)]
         public int? RenewAtNumberOfDaysBeforeExpiry { get; set; }
 
         /// <summary>
@@ -90,7 +90,6 @@ public class SetAzureKeyVaultCertificatePolicy : KeyVaultCmdletBase
         [Parameter(Mandatory = false,
             ParameterSetName = ExpandedRenewPercentageParameterSet,
             HelpMessage = "Specifies the percentage of the lifetime after which the automatic process for the certificate renewal begins.")]
-        [ValidateRange(0, 99)]
         public int? RenewAtPercentageLifetime { get; set; }
 
         /// <summary>
@@ -231,7 +230,6 @@ public class SetAzureKeyVaultCertificatePolicy : KeyVaultCmdletBase
         /// Key size
         /// </summary>
         [Parameter(Mandatory = false,
-                   ValueFromPipelineByPropertyName = true,
                    HelpMessage = "Specifies the key size of the certificate. Default is 2048.")]
         [ValidateSet("2048", "3072", "4096", "256", "384", "521")]
         public int KeySize { get; set; }
@@ -247,6 +245,9 @@ public class SetAzureKeyVaultCertificatePolicy : KeyVaultCmdletBase
             HelpMessage = "Specifies whether the key is not exportable.")]
         public SwitchParameter KeyNotExportable { get; set; }
 
+        /// <summary>
+        /// CertificateTransparency
+        /// </summary>
         [Parameter(ValueFromPipelineByPropertyName = false,
             HelpMessage = "Indicates whether certificate transparency is enabled for this certificate/issuer; if not specified, the default is 'true'")]
         public bool? CertificateTransparency { get; set; }
@@ -261,7 +262,6 @@ public class SetAzureKeyVaultCertificatePolicy : KeyVaultCmdletBase
         /// Elliptic Curve Name of the key
         /// </summary>
         [Parameter(Mandatory = false,
-                   ValueFromPipelineByPropertyName = true,
                    HelpMessage = "Specifies the elliptic curve name of the key of the ECC certificate.")]
         [ValidateSet(Constants.P256, Constants.P384, Constants.P521, Constants.P256K, Constants.SECP256K1)]
         public string Curve { get; set; }
@@ -271,6 +271,8 @@ public override void ExecuteCmdlet()
         {
             if (ShouldProcess(Name, Properties.Resources.SetCertificatePolicy))
             {
+
+                ValidateArguments();
                 PSKeyVaultCertificatePolicy policy = new PSKeyVaultCertificatePolicy();
 
                 switch (ParameterSetName)
@@ -313,5 +315,21 @@ public override void ExecuteCmdlet()
                 }
             }
         }
+
+
+        private void ValidateArguments()
+        {
+            // Manually Validate `RenewAtNumberOfDaysBeforeExpiry` and `RenewAtPercentageLifetime`
+            if (RenewAtNumberOfDaysBeforeExpiry.HasValue && (RenewAtNumberOfDaysBeforeExpiry < 1 || RenewAtNumberOfDaysBeforeExpiry > int.MaxValue))
+            {
+                throw new AzPSArgumentOutOfRangeException(Properties.Resources.InvalidRangeDaysBeforeExpiry, nameof(RenewAtNumberOfDaysBeforeExpiry));
+            }
+
+            if (RenewAtPercentageLifetime.HasValue && (RenewAtPercentageLifetime < 0 || RenewAtPercentageLifetime > 99))
+            {
+                throw new AzPSArgumentOutOfRangeException(Properties.Resources.InvalidRangePercentageLifetime, nameof(RenewAtPercentageLifetime));
+            }
+        }
     }
+
 }
diff --git a/src/KeyVault/KeyVault/Properties/Resources.Designer.cs b/src/KeyVault/KeyVault/Properties/Resources.Designer.cs
diff --git a/src/KeyVault/KeyVault/Properties/Resources.resx b/src/KeyVault/KeyVault/Properties/Resources.resx
@@ -630,4 +630,10 @@ You can find the object ID using Azure Active Directory Module for Windows Power
   <data name="ProcessingCertError" xml:space="preserve">
     <value>Error happens when processing certificate '{0}'. See detailed error: {1}</value>
   </data>
+  <data name="InvalidRangeDaysBeforeExpiry" xml:space="preserve">
+    <value>Value must be between 1 and int.MaxValue.</value>
+  </data>
+  <data name="InvalidRangePercentageLifetime" xml:space="preserve">
+    <value>"Value must be between 0 and 99."</value>
+  </data>
 </root>
diff --git a/src/KeyVault/KeyVault/help/Set-AzKeyVaultCertificatePolicy.md b/src/KeyVault/KeyVault/help/Set-AzKeyVaultCertificatePolicy.md
@@ -22,7 +22,7 @@ Set-AzKeyVaultCertificatePolicy [-VaultName] <String> [-Name] <String> [-RenewAt
  [-Ekus <System.Collections.Generic.List`1[System.String]>] [-ValidityInMonths <Int32>] [-IssuerName <String>]
  [-CertificateType <String>] [-EmailAtNumberOfDaysBeforeExpiry <Int32>] [-EmailAtPercentageLifetime <Int32>]
  [-KeyType <String>] [-KeySize <Int32>] [-KeyNotExportable] [-CertificateTransparency <Boolean>] [-PassThru]
- [-Curve <String>] [-DefaultProfile <IAzureContextContainer>] [-WhatIf]
+ [-Curve <String>] [-DefaultProfile <IAzureContextContainer>] [-ProgressAction <ActionPreference>] [-WhatIf]
  [-Confirm] [<CommonParameters>]
 ```
 
@@ -32,7 +32,7 @@ Set-AzKeyVaultCertificatePolicy [-VaultName] <String> [-Name] <String>
  [-InputObject] <PSKeyVaultCertificatePolicy> [-EmailAtNumberOfDaysBeforeExpiry <Int32>]
  [-EmailAtPercentageLifetime <Int32>] [-KeyType <String>] [-KeySize <Int32>]
  [-CertificateTransparency <Boolean>] [-PassThru] [-Curve <String>] [-DefaultProfile <IAzureContextContainer>]
- [-WhatIf] [-Confirm] [<CommonParameters>]
+ [-ProgressAction <ActionPreference>] [-WhatIf] [-Confirm] [<CommonParameters>]
 ```
 
 ### ExpandedRenewNumber
@@ -44,7 +44,7 @@ Set-AzKeyVaultCertificatePolicy [-VaultName] <String> [-Name] <String> -RenewAtN
  [-Ekus <System.Collections.Generic.List`1[System.String]>] [-ValidityInMonths <Int32>] [-IssuerName <String>]
  [-CertificateType <String>] [-EmailAtNumberOfDaysBeforeExpiry <Int32>] [-EmailAtPercentageLifetime <Int32>]
  [-KeyType <String>] [-KeySize <Int32>] [-KeyNotExportable] [-CertificateTransparency <Boolean>] [-PassThru]
- [-Curve <String>] [-DefaultProfile <IAzureContextContainer>] [-WhatIf]
+ [-Curve <String>] [-DefaultProfile <IAzureContextContainer>] [-ProgressAction <ActionPreference>] [-WhatIf]
  [-Confirm] [<CommonParameters>]
 ```
 
@@ -135,7 +135,7 @@ Accepted values: P-256, P-384, P-521, P-256K, SECP256K1
 Required: False
 Position: Named
 Default value: None
-Accept pipeline input: True (ByPropertyName)
+Accept pipeline input: False
 Accept wildcard characters: False
 ```
 
@@ -293,7 +293,7 @@ Accepted values: 2048, 3072, 4096, 256, 384, 521
 Required: False
 Position: Named
 Default value: 2048
-Accept pipeline input: True (ByPropertyName)
+Accept pipeline input: False
 Accept wildcard characters: False
 ```
 
@@ -365,6 +365,21 @@ Accept pipeline input: False
 Accept wildcard characters: False
 ```
 
+### -ProgressAction
+{{ Fill ProgressAction Description }}
+
+```yaml
+Type: System.Management.Automation.ActionPreference
+Parameter Sets: (All)
+Aliases: proga
+
+Required: False
+Position: Named
+Default value: None
+Accept pipeline input: False
+Accept wildcard characters: False
+```
+
 ### -RenewAtNumberOfDaysBeforeExpiry
 Specifies the number of days before expiry after which the automatic process for certificate renewal begins.
 

diff --git a/tools/StaticAnalysis/Exceptions/Az.KeyVault/BreakingChangeIssues.csv b/tools/StaticAnalysis/Exceptions/Az.KeyVault/BreakingChangeIssues.csv
@@ -0,0 +1,5 @@
+"Module","ClassName","Target","Severity","ProblemId","Description","Remediation"
+"Az.KeyVault","Microsoft.Azure.Commands.KeyVault.SetAzureKeyVaultCertificatePolicy","Set-AzKeyVaultCertificatePolicy","0","1050","The parameter set '__AllParameterSets' for cmdlet 'Set-AzKeyVaultCertificatePolicy' has been removed.","Add parameter set '__AllParameterSets' back to cmdlet 'Set-AzKeyVaultCertificatePolicy'."
+"Az.KeyVault","Microsoft.Azure.Commands.KeyVault.SetAzureKeyVaultCertificatePolicy","Set-AzKeyVaultCertificatePolicy","0","1050","The parameter set 'ByValue' for cmdlet 'Set-AzKeyVaultCertificatePolicy' has been removed.","Add parameter set 'ByValue' back to cmdlet 'Set-AzKeyVaultCertificatePolicy'."
+"Az.KeyVault","Microsoft.Azure.Commands.KeyVault.SetAzureKeyVaultCertificatePolicy","Set-AzKeyVaultCertificatePolicy","0","1050","The parameter set 'ExpandedRenewNumber' for cmdlet 'Set-AzKeyVaultCertificatePolicy' has been removed.","Add parameter set 'ExpandedRenewNumber' back to cmdlet 'Set-AzKeyVaultCertificatePolicy'."
+"Az.KeyVault","Microsoft.Azure.Commands.KeyVault.SetAzureKeyVaultCertificatePolicy","Set-AzKeyVaultCertificatePolicy","0","1050","The parameter set 'ExpandedRenewPercentage' for cmdlet 'Set-AzKeyVaultCertificatePolicy' has been removed.","Add parameter set 'ExpandedRenewPercentage' back to cmdlet 'Set-AzKeyVaultCertificatePolicy'."