azurerm_batch_pool - support for new block security_profile#28069
Merged
catriona-m merged 5 commits intohashicorp:mainfrom Dec 17, 2024
Merged
azurerm_batch_pool - support for new block security_profile#28069catriona-m merged 5 commits intohashicorp:mainfrom
azurerm_batch_pool - support for new block security_profile#28069catriona-m merged 5 commits intohashicorp:mainfrom
Conversation
github-actions
bot
added
enhancement
documentation
service/batch
size/M
and removed
enhancement
labels
liuwuliuyun
changed the title
azurerm_batch_pool - support new block security_profileazurerm_batch_pool - support for new block security_profile
catriona-m
requested changes
Dec 9, 2024
Member
catriona-m
left a comment
catriona-m
left a comment
There was a problem hiding this comment.
Thanks for this @liuwuliuyun - I've had a look through and left some suggestions inline but once those are addressed I can take another look. Thanks!
| }) | ||
| } | ||
|
|
||
| func TestAccBatchPool_securityProfileWithUEFISettings(t *testing.T) { |
Member
There was a problem hiding this comment.
could we add additional steps to this test to test updating the properties in the block and to test adding/removing the block?
Collaborator
Author
There was a problem hiding this comment.
Oh I have just recalled that the security_profile can only be specified during creation and does not support updates. I will ensure this information is added to the documentation.
Collaborator
Author
There was a problem hiding this comment.
Plus I will mark this block as forceNew
Comment on lines
+1283
to
+1296
| if config.SecurityProfile != nil { | ||
| securityProfile := make([]interface{}, 0) | ||
| securityConfig := make(map[string]interface{}) | ||
| securityConfig["host_encryption_enabled"] = pointer.ToBool(config.SecurityProfile.EncryptionAtHost) | ||
| if config.SecurityProfile.SecurityType != nil { | ||
| securityConfig["security_type"] = string(*config.SecurityProfile.SecurityType) | ||
| } | ||
| if config.SecurityProfile.UefiSettings != nil { | ||
| securityConfig["secure_boot_enabled"] = pointer.ToBool(config.SecurityProfile.UefiSettings.SecureBootEnabled) | ||
| securityConfig["vtpm_enabled"] = pointer.ToBool(config.SecurityProfile.UefiSettings.VTpmEnabled) | ||
| } | ||
| securityProfile = append(securityProfile, securityConfig) | ||
| d.Set("security_profile", securityProfile) | ||
| } |
Member
There was a problem hiding this comment.
should we move this into a flatten func?
Co-authored-by: catriona-m <[email protected]>
Collaborator
Author
|
New testing evidence: |
catriona-m
requested changes
Dec 11, 2024
Member
catriona-m
left a comment
catriona-m
left a comment
There was a problem hiding this comment.
Thanks for updating this @liuwuliuyun - I left a couple more suggestions inline but can take another look after those are addressed. Thanks!
Comment on lines
+418
to
+420
| if configProfile.EncryptionAtHost != nil { | ||
| securityConfig["host_encryption_enabled"] = *configProfile.EncryptionAtHost | ||
| } |
Member
There was a problem hiding this comment.
Suggested change
| if configProfile.EncryptionAtHost != nil { | |
| securityConfig["host_encryption_enabled"] = *configProfile.EncryptionAtHost | |
| } | |
| securityConfig["host_encryption_enabled"] = pointer.From(configProfile.EncryptionAtHost) |
Comment on lines
+422
to
+424
| if configProfile.SecurityType != nil { | ||
| securityConfig["security_type"] = string(*configProfile.SecurityType) | ||
| } |
Member
There was a problem hiding this comment.
Suggested change
| if configProfile.SecurityType != nil { | |
| securityConfig["security_type"] = string(*configProfile.SecurityType) | |
| } | |
| securityConfig["security_type"] = pointer.From(*configProfile.SecurityType) |
Comment on lines
+427
to
+429
| if configProfile.UefiSettings.SecureBootEnabled != nil { | ||
| securityConfig["secure_boot_enabled"] = pointer.ToBool(configProfile.UefiSettings.SecureBootEnabled) | ||
| } |
Member
There was a problem hiding this comment.
Suggested change
| if configProfile.UefiSettings.SecureBootEnabled != nil { | |
| securityConfig["secure_boot_enabled"] = pointer.ToBool(configProfile.UefiSettings.SecureBootEnabled) | |
| } | |
| securityConfig["secure_boot_enabled"] = pointer.From(configProfile.UefiSettings.SecureBootEnabled) |
Comment on lines
+430
to
+432
| if configProfile.UefiSettings.VTpmEnabled != nil { | ||
| securityConfig["vtpm_enabled"] = pointer.ToBool(configProfile.UefiSettings.VTpmEnabled) | ||
| } |
Member
There was a problem hiding this comment.
Suggested change
| if configProfile.UefiSettings.VTpmEnabled != nil { | |
| securityConfig["vtpm_enabled"] = pointer.ToBool(configProfile.UefiSettings.VTpmEnabled) | |
| } | |
| securityConfig["vtpm_enabled"] = pointer.From(configProfile.UefiSettings.VTpmEnabled) |
| } | ||
|
|
||
| if v, ok := item["host_encryption_enabled"]; ok { | ||
| securityProfile.EncryptionAtHost = pointer.FromBool(v.(bool)) |
Member
There was a problem hiding this comment.
Suggested change
| securityProfile.EncryptionAtHost = pointer.FromBool(v.(bool)) | |
| securityProfile.EncryptionAtHost = pointer.To(v.(bool)) |
| } | ||
|
|
||
| if v, ok := item["secure_boot_enabled"]; ok { | ||
| securityProfile.UefiSettings.SecureBootEnabled = pointer.FromBool(v.(bool)) |
Member
There was a problem hiding this comment.
Suggested change
| securityProfile.UefiSettings.SecureBootEnabled = pointer.FromBool(v.(bool)) | |
| securityProfile.UefiSettings.SecureBootEnabled = pointer.To(v.(bool)) |
| } | ||
|
|
||
| if v, ok := item["vtpm_enabled"]; ok { | ||
| securityProfile.UefiSettings.VTpmEnabled = pointer.FromBool(v.(bool)) |
Member
There was a problem hiding this comment.
Suggested change
| securityProfile.UefiSettings.VTpmEnabled = pointer.FromBool(v.(bool)) | |
| securityProfile.UefiSettings.VTpmEnabled = pointer.To(v.(bool)) |
|
|
||
| * `os_disk_placement` - (Optional) Specifies the ephemeral disk placement for operating system disk for all VMs in the pool. This property can be used by user in the request to choose which location the operating system should be in. e.g., cache disk space for Ephemeral OS disk provisioning. For more information on Ephemeral OS disk size requirements, please refer to Ephemeral OS disk size requirements for Windows VMs at <https://docs.microsoft.com/en-us/azure/virtual-machines/windows/ephemeral-os-disks#size-requirements> and Linux VMs at <https://docs.microsoft.com/en-us/azure/virtual-machines/linux/ephemeral-os-disks#size-requirements>. The only possible value is `CacheDisk`. | ||
|
|
||
| * `security_profile` - (Optional) A `security_profile` block that describes the security settings for the Batch pool as defined below. |
Member
There was a problem hiding this comment.
can we add here that Changing this forces a new resource to be created.?
Comment on lines
+510
to
+516
| * `host_encryption_enabled` - (Optional) Whether to enable host encryption for the Virtual Machine or Virtual Machine Scale Set. This will enable the encryption for all the disks including Resource/Temp disk at host itself. Possible values are `true` and `false`. Changing this forces a new resource to be created. | ||
|
|
||
| * `security_type` - (Optional) The security type of the Virtual Machine. Possible values are `confidentialVM` and `trustedLaunch`. Changing this forces a new resource to be created. | ||
|
|
||
| * `secure_boot_enabled` - (Optional) Whether to enable secure boot for the Virtual Machine or Virtual Machine Scale Set. Possible values are `true` and `false`. Changing this forces a new resource to be created. | ||
|
|
||
| * `vtpm_enabled` - (Optional) Whether to enable virtual trusted platform module (vTPM) for the Virtual Machine or Virtual Machine Scale Set. Possible values are `true` and `false`. Changing this forces a new resource to be created. |
Member
There was a problem hiding this comment.
if the properties within the block cannot be updated once it's been created as well, we should add ForceNew to these properties in the schema too
catriona-m
added a commit
that referenced
this pull request
Dec 17, 2024
jackofallops
pushed a commit
that referenced
this pull request
Jan 10, 2025
jackofallops
added a commit
that referenced
this pull request
Jan 10, 2025
* Update CHANGELOG.md for #28233 * Update for #28215 * Update CHANGELOG.md for #28279 * Update CHANGELOG.md #28269 * Update CHANGELOG.md #27876 * Update CHANGELOG.md #28069 * Update CHANGELOG.md for #28312 * Update CHANGELOG.md for #28278 * Update CHANGELOG.md #28311 * Update CHANGELOG.md undo 28311 * Update CHANGELOG.md #27874 * Update CHANGELOG.md * Update CHANGELOG for #28352 * Update CHANGELOG.md for #28390 * Update CHANGELOG.md for #28398 * Update CHANGELOG.md for #28425 * Update CHANGELOG.md #28427 * Update CHANGELOG.md #28280 * Update CHANGELOG.md for #28319 * Update CHANGELOG.md #24801 * Update for #28360 #28216 #27830 #28404 #27401 #27122 #27931 #28442 * Update for #28379 * Update CHANGELOG.md for #28281 * Update for #28380 * Update for #27375 * Update for #25695 * Update CHANGELOG.md #27985 * Update CHANGELOG.md - update release date manually until can be scripted * Update CHANGELOG.md revert date change as script available * pre-release script updates --------- Co-authored-by: stephybun <[email protected]> Co-authored-by: catriona-m <[email protected]> Co-authored-by: Wyatt Fry <[email protected]> Co-authored-by: sreallymatt <[email protected]> Co-authored-by: Matthew Frahry <[email protected]> Co-authored-by: kt <[email protected]>
Contributor
|
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Community Note
Description
For resource
azurerm_batch_poolsecurity_profilesecurity_profilePR Checklist
For example: “
resource_name_here- description of change e.g. adding propertynew_property_name_here”Changes to existing Resource / Data Source
Testing
Change Log
Below please provide what should go into the changelog (if anything) conforming to the Changelog Format documented here.
azurerm_batch_pool- support for new blocksecurity_profile[azurerm_batch_pool- support for new blocksecurity_profile#28069]This is a (please select all that apply):
Related Issue(s)
Fixes #27952