-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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_container_group
- Supports CMK with user assigned identity
#23332
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -553,6 +553,12 @@ func resourceContainerGroup() *pluginsdk.Resource { | |||||
ForceNew: true, | ||||||
ValidateFunc: keyVaultValidate.NestedItemId, | ||||||
}, | ||||||
|
||||||
"key_vault_user_identity_id": { | ||||||
Type: pluginsdk.TypeString, | ||||||
Optional: true, | ||||||
ValidateFunc: commonids.ValidateUserAssignedIdentityID, | ||||||
}, | ||||||
}, | ||||||
} | ||||||
} | ||||||
|
@@ -777,6 +783,10 @@ func resourceContainerGroupCreate(d *pluginsdk.ResourceData, meta interface{}) e | |||||
KeyName: keyId.Name, | ||||||
KeyVersion: keyId.Version, | ||||||
} | ||||||
|
||||||
if keyVaultUAI := d.Get("key_vault_user_identity_id").(string); keyVaultUAI != "" { | ||||||
containerGroup.Properties.EncryptionProperties.Identity = &keyVaultUAI | ||||||
} | ||||||
} | ||||||
|
||||||
// Avoid parallel provisioning if "subnet_ids" are given. | ||||||
|
@@ -942,6 +952,12 @@ func resourceContainerGroupRead(d *pluginsdk.ResourceData, meta interface{}) err | |||||
return err | ||||||
} | ||||||
d.Set("key_vault_key_id", keyId.ID()) | ||||||
|
||||||
var uai string | ||||||
if kvProps.Identity != nil { | ||||||
uai = *kvProps.Identity | ||||||
} | ||||||
d.Set("key_vault_user_identity_id", uai) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This can become
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure, I'll change it. |
||||||
} | ||||||
} | ||||||
|
||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -99,6 +99,8 @@ The following arguments are supported: | |
|
||
* `key_vault_key_id` - (Optional) The Key Vault key URI for CMK encryption. Changing this forces a new resource to be created. | ||
|
||
* `key_vault_user_identity_id` - (Optional) The user assigned identity that has access to the Key Vault Key. If not specified, the RP principal named "Azure Container Instance Service" will be used instead. Make sure the identity has the proper `key_permissions` set, at least with `Get`, `UnwrapKey`, `WrapKey` and `GetRotationPolicy`. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The ordering of the sentences here is a little confusing, are the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It should be both identities. |
||
|
||
* `subnet_ids` - (Optional) The subnet resource IDs for a container group. Changing this forces a new resource to be created. | ||
|
||
* `image_registry_credential` - (Optional) An `image_registry_credential` block as documented below. Changing this forces a new resource to be created. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently the identity properties for CMK are inconsistent across the provider, some are called
identity_id
,primary_user_assigned_identity_id
oruser_assigned_identity_id
. We might want to consider standardising these for the next major release and having acommonschema
for them like we do for identities.All this to say, I think for the time being this might benefit from being called
key_vault_user_assigned_identity_id
since it's clear at first glance what type of ID it is and is also closer to what we currently have in the provider.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Renamed.