[Breaking Change] Default New-AzGalleryImageDefinition to HyperV Gen2 and Trusted Launch #24869
Conversation
grizzlytheodore
requested review from
bilaakpan-ms,
Sandido and
haagha
as code owners
️✔️Az.Accounts
️✔️Az.Compute
️✔️Az.KeyVault
️✔️Az.ManagedServiceIdentity
️✔️Az.Monitor
️✔️Az.Network
️✔️Az.OperationalInsights
️✔️Az.PrivateDns
|
| Type | Title | Current Coverage | Last Coverage | Description |
|---|---|---|---|---|
| Test Coverage Less Than 80% | 62.50 % | 66.67% | Test coverage cannot be lower than the number of the last release. |
⚠️ - MacOS
| Type | Title | Current Coverage | Last Coverage | Description |
|---|---|---|---|---|
| Test Coverage Less Than 80% | 62.50% | 66.67% | Test coverage cannot be lower than the number of the last release. |
⚠️ PowerShell Core - Windows
| Type | Title | Current Coverage | Last Coverage | Description |
|---|---|---|---|---|
| Test Coverage Less Than 80% | 62.50% | 66.67% | Test coverage cannot be lower than the number of the last release. |
⚠️ Windows PowerShell - Windows
| Type | Title | Current Coverage | Last Coverage | Description |
|---|---|---|---|---|
| Test Coverage Less Than 80% | 62.50% | 66.67% | Test coverage cannot be lower than the number of the last release. |
️✔️Az.Security
️✔️Build
️✔️PowerShell Core - Windows
️✔️Windows PowerShell - Windows
️✔️Test
️✔️ - Linux
️✔️ - 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
️✔️ - Linux
️✔️ - MacOS
️✔️PowerShell Core - Windows
️✔️Windows PowerShell - Windows
️✔️Az.Storage
️✔️Build
️✔️PowerShell Core - Windows
️✔️Windows PowerShell - Windows
grizzlytheodore
changed the title
| @@ -185,7 +188,19 @@ public override void ExecuteCmdlet() | |||
|
|
|||
| if (this.IsParameterBound(c => c.Feature)) | |||
| { | |||
| galleryImage.Features = this.Feature; | |||
There was a problem hiding this comment.
changing from direct assign, to make it easier to add another feature when needed below.
Direct assigned made it immutable list
src/Compute/Compute/Generated/GalleryImage/GalleryImageCreateOrUpdateMethod.cs
Outdated
Show resolved
Hide resolved
|
To the author of the pull request, |
|
Hi @grizzlytheodore , please resove the conflicted file: src/Compute/Compute/ChangeLog.md |
|
@YanaXu done ! |
| galleryImage.Features.Add(this.Feature[i]); | ||
| } | ||
| } | ||
| if (SecurityTypeNotFoundInFeatures(galleryImage.Features) && galleryImage.HyperVGeneration == "V2")// if SecurityType is not in Features, default to TrustedLaunch |
There was a problem hiding this comment.
Suggestion: Can you collapse the separate function into 1 single line check?
if (galleryImage.Features?.All(f => f.Name.ToLower() != "securitytype") == true && galleryImage.HyperVGeneration == "V2")
There was a problem hiding this comment.
thank you for the cool notation. Yes, one line code is better than a function.
I had to add one more conditional for when Feature is not passed in at all, but this notation works well
…com/Azure/azure-powershell into feature/cplat-GalleryBreakingChange
I will refrain from changing filename at this point. I don't want to create more complexity this late before code merge deadline |
|
Hi @grizzlytheodore, please remember to fix the CI build error from this file "src\Compute\Compute\Generated\GalleryImage\GalleryImageCreateOrUpdateMethod.cs". |
|
Hi @grizzlytheodore & @Sandido & @haagha , I've fixed the CI error. Could you please review it ASAP? The code freeze date is today. Thanks! |
|
Providing "None" SecurityType creates the resource in standard securityType |
Description
CLI request equivalent: Azure/azure-cli#28557
New-AzGalleryImageDefinition will default to -HyperVGeneration V2 and -Feature @{name='SecurityType', value='TrustedLaunchSupport'} unless the parameters are provided explicitly
Mandatory Checklist
Please choose the target release of Azure PowerShell. (⚠️ Target release is a different concept from API readiness. Please click below links for details.)
Check this box to confirm: I have read the Submitting Changes section of
CONTRIBUTING.mdand reviewed the following information:ChangeLog.mdfile(s) appropriatelysrc/{{SERVICE}}/{{SERVICE}}/ChangeLog.md.## Upcoming Releaseheader in the past tense.ChangeLog.mdif no new release is required, such as fixing test case only.