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

azurerm_batch_pool - support for new block security_profile #28069

Merged
merged 5 commits into from
Dec 17, 2024
Merged
Show file tree
Hide file tree
Changes from 2 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
33 changes: 33 additions & 0 deletions internal/services/batch/batch_pool.go
Original file line number Diff line number Diff line change
Expand Up @@ -791,13 +791,46 @@ func expandBatchPoolVirtualMachineConfig(d *pluginsdk.ResourceData) (*pool.Virtu
result.OsDisk = expandBatchPoolOSDisk(v)
}

if v, ok := d.GetOk("security_profile"); ok {
result.SecurityProfile = expandBatchPoolSecurityProfile(v.([]interface{}))
}

if v, ok := d.GetOk("windows"); ok {
result.WindowsConfiguration = expandBatchPoolWindowsConfiguration(v.([]interface{}))
}

return &result, nil
}

func expandBatchPoolSecurityProfile(profile []interface{}) *pool.SecurityProfile {
if len(profile) == 0 {
return nil
}

item := profile[0].(map[string]interface{})
securityProfile := &pool.SecurityProfile{
UefiSettings: &pool.UefiSettings{},
}

if v, ok := item["host_encryption_enabled"]; ok {
securityProfile.EncryptionAtHost = pointer.FromBool(v.(bool))
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
securityProfile.EncryptionAtHost = pointer.FromBool(v.(bool))
securityProfile.EncryptionAtHost = pointer.To(v.(bool))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

}

if v, ok := item["security_type"]; ok {
securityProfile.SecurityType = pointer.To(pool.SecurityTypes(v.(string)))
}

if v, ok := item["secure_boot_enabled"]; ok {
securityProfile.UefiSettings.SecureBootEnabled = pointer.FromBool(v.(bool))
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
securityProfile.UefiSettings.SecureBootEnabled = pointer.FromBool(v.(bool))
securityProfile.UefiSettings.SecureBootEnabled = pointer.To(v.(bool))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

}

if v, ok := item["vtpm_enabled"]; ok {
securityProfile.UefiSettings.VTpmEnabled = pointer.FromBool(v.(bool))
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
securityProfile.UefiSettings.VTpmEnabled = pointer.FromBool(v.(bool))
securityProfile.UefiSettings.VTpmEnabled = pointer.To(v.(bool))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

}

return securityProfile
}

func expandBatchPoolOSDisk(ref interface{}) *pool.OSDisk {
if ref == nil {
return nil
Expand Down
43 changes: 43 additions & 0 deletions internal/services/batch/batch_pool_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -724,6 +724,35 @@ func resourceBatchPool() *pluginsdk.Resource {
}, false),
},

"security_profile": {
Type: pluginsdk.TypeList,
Optional: true,
MaxItems: 1,
Elem: &pluginsdk.Resource{
Schema: map[string]*pluginsdk.Schema{
"host_encryption_enabled": {
Type: pluginsdk.TypeBool,
Optional: true,
},
"security_type": {
Type: pluginsdk.TypeString,
Optional: true,
ValidateFunc: validation.StringInSlice(pool.PossibleValuesForSecurityTypes(), false),
},
"secure_boot_enabled": {
Type: pluginsdk.TypeBool,
Optional: true,
RequiredWith: []string{"security_profile.0.security_type"},
},
"vtpm_enabled": {
Type: pluginsdk.TypeBool,
Optional: true,
RequiredWith: []string{"security_profile.0.security_type"},
},
},
},
},

"target_node_communication_mode": {
Type: pluginsdk.TypeString,
Optional: true,
Expand Down Expand Up @@ -1251,6 +1280,20 @@ func resourceBatchPoolRead(d *pluginsdk.ResourceData, meta interface{}) error {
osDiskPlacement = string(*config.OsDisk.EphemeralOSDiskSettings.Placement)
}
d.Set("os_disk_placement", osDiskPlacement)
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)
}
Copy link
Member

Choose a reason for hiding this comment

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

should we move this into a flatten func?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, will do

if config.WindowsConfiguration != nil {
windowsConfig := []interface{}{
map[string]interface{}{
Expand Down
52 changes: 52 additions & 0 deletions internal/services/batch/batch_pool_resource_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -740,6 +740,24 @@ func TestAccBatchPool_interNodeCommunicationWithTaskSchedulingPolicy(t *testing.
})
}

func TestAccBatchPool_securityProfileWithUEFISettings(t *testing.T) {
Copy link
Member

Choose a reason for hiding this comment

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

could we add additional steps to this test to test updating the properties in the block and to test adding/removing the block?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

@liuwuliuyun liuwuliuyun Dec 10, 2024

Choose a reason for hiding this comment

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

Plus I will mark this block as forceNew

data := acceptance.BuildTestData(t, "azurerm_batch_pool", "test")
r := BatchPoolResource{}
data.ResourceTest(t, r, []acceptance.TestStep{
{
Config: r.securityProfileWithUEFISettings(data),
Check: acceptance.ComposeTestCheckFunc(
check.That(data.ResourceName).ExistsInAzure(r),
check.That(data.ResourceName).Key("security_profile.0.host_encryption_enabled").HasValue("false"),
check.That(data.ResourceName).Key("security_profile.0.security_type").HasValue("trustedLaunch"),
check.That(data.ResourceName).Key("security_profile.0.secure_boot_enabled").HasValue("true"),
check.That(data.ResourceName).Key("security_profile.0.vtpm_enabled").HasValue("false"),
),
},
data.ImportStep("stop_pending_resize_operation"),
})
}

func TestAccBatchPool_linuxUserAccounts(t *testing.T) {
data := acceptance.BuildTestData(t, "azurerm_batch_pool", "test")
r := BatchPoolResource{}
Expand Down Expand Up @@ -2653,3 +2671,37 @@ resource "azurerm_subnet_network_security_group_association" "test" {
}
`, data.RandomInteger, data.Locations.Primary, data.RandomString, data.RandomString, data.RandomString)
}

func (BatchPoolResource) securityProfileWithUEFISettings(data acceptance.TestData) string {
template := BatchPoolResource{}.template(data)
return fmt.Sprintf(`
%s
resource "azurerm_batch_account" "test" {
name = "testaccbatch%s"
liuwuliuyun marked this conversation as resolved.
Show resolved Hide resolved
resource_group_name = azurerm_resource_group.test.name
location = azurerm_resource_group.test.location
}
resource "azurerm_batch_pool" "test" {
name = "testaccpool%s"
liuwuliuyun marked this conversation as resolved.
Show resolved Hide resolved
resource_group_name = azurerm_resource_group.test.name
account_name = azurerm_batch_account.test.name
node_agent_sku_id = "batch.node.ubuntu 22.04"
vm_size = "Standard_A1"
fixed_scale {
target_dedicated_nodes = 1
}
security_profile {
host_encryption_enabled = false
security_type = "trustedLaunch"
secure_boot_enabled = true
vtpm_enabled = false
}
storage_image_reference {
publisher = "Canonical"
offer = "0001-com-ubuntu-server-jammy"
sku = "22_04-lts"
version = "latest"
}
}
`, template, data.RandomString, data.RandomString)
}
15 changes: 15 additions & 0 deletions website/docs/r/batch_pool.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,8 @@ The following arguments are supported:

* `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.
Copy link
Member

Choose a reason for hiding this comment

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

can we add here that Changing this forces a new resource to be created.?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure


* `target_node_communication_mode` - (Optional) The desired node communication mode for the pool. Possible values are `Classic`, `Default` and `Simplified`.

* `task_scheduling_policy` - (Optional) A `task_scheduling_policy` block that describes how tasks are distributed across compute nodes in a pool as defined below. If not specified, the default is spread as defined below.
Expand Down Expand Up @@ -502,6 +504,19 @@ A `task_scheduling_policy` block supports the following:

* `node_fill_type` - (Optional) Supported values are "Pack" and "Spread". "Pack" means as many tasks as possible (taskSlotsPerNode) should be assigned to each node in the pool before any tasks are assigned to the next node in the pool. "Spread" means that tasks should be assigned evenly across all nodes in the pool.

---
A `security_profile` block supports the following:

* `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`.
liuwuliuyun marked this conversation as resolved.
Show resolved Hide resolved

* `security_type` - (Optional) The security type of the virtual machine. Possible values are `confidentialVM` and `trustedLaunch`.
liuwuliuyun marked this conversation as resolved.
Show resolved Hide resolved

* `secure_boot_enabled` - (Optional) Whether to enable secure boot for the virtual machine or virtual machine scale set. Possible values are `true` and `false`.
liuwuliuyun marked this conversation as resolved.
Show resolved Hide resolved

* `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`.
liuwuliuyun marked this conversation as resolved.
Show resolved Hide resolved

~> **NOTE:** `security_type` must be specified to set UEFI related properties including `secure_boot_enabled` and `vtpm_enabled`.

---

A `user_accounts` block supports the following:
Expand Down
Loading