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_eventhub_namespace_customer_managed_key - support for user_assigned_identity_id #23635

Merged
merged 31 commits into from
Oct 27, 2023
Merged
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
7e8dc72
working implementation, still need to finish UAC tests
bruceharrison1984 Oct 19, 2023
038f8f9
Support Users Assigned IDs with BYOK
jake-scott Aug 8, 2023
e793543
add docs
bruceharrison1984 Oct 19, 2023
1f75f99
add heading to docs
bruceharrison1984 Oct 19, 2023
64a2c5a
add note to property
bruceharrison1984 Oct 19, 2023
32dcc63
use existing identity schema
bruceharrison1984 Oct 20, 2023
a7d1fbe
fix typo
bruceharrison1984 Oct 20, 2023
4463ffc
move method
bruceharrison1984 Oct 20, 2023
aefa032
linting
bruceharrison1984 Oct 20, 2023
d6832ae
fix test to use identity block
bruceharrison1984 Oct 20, 2023
3df4e1a
update docs
bruceharrison1984 Oct 20, 2023
a8644d1
rename test
bruceharrison1984 Oct 20, 2023
741cab4
linting
bruceharrison1984 Oct 20, 2023
791a6cc
unnest for loop
bruceharrison1984 Oct 20, 2023
e43ffe8
remove redunant string check
bruceharrison1984 Oct 20, 2023
32a7bf3
rename method
bruceharrison1984 Oct 20, 2023
1b03cfc
cleanup if checks
bruceharrison1984 Oct 20, 2023
7186e6a
fix faulty if statement
bruceharrison1984 Oct 23, 2023
d0f6ce8
update docs
bruceharrison1984 Oct 23, 2023
9bfe4ff
populate identity property on read
bruceharrison1984 Oct 23, 2023
475d11f
fix error in docs
bruceharrison1984 Oct 23, 2023
c1b10be
code review cleanup, fix null ref error
bruceharrison1984 Oct 24, 2023
99d5d51
remove dead code
bruceharrison1984 Oct 24, 2023
d6022dd
add additional docs note
bruceharrison1984 Oct 24, 2023
b4c2f6e
Update internal/services/eventhub/eventhub_namespace_customer_managed…
katbyte Oct 24, 2023
72ca0af
Update internal/services/eventhub/eventhub_namespace_customer_managed…
katbyte Oct 24, 2023
1252b0a
refactor to use a string Id instead of an identity block
bruceharrison1984 Oct 26, 2023
259eb27
add comments
bruceharrison1984 Oct 26, 2023
d979e2a
move if check up
bruceharrison1984 Oct 26, 2023
5a89a56
move method body up
bruceharrison1984 Oct 26, 2023
87e9ae7
Update eventhub_namespace_customer_managed_key_resource.go
katbyte Oct 27, 2023
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
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import (
"time"

"github.com/hashicorp/go-azure-helpers/lang/response"
"github.com/hashicorp/go-azure-helpers/resourcemanager/commonschema"
"github.com/hashicorp/go-azure-helpers/resourcemanager/identity"
"github.com/hashicorp/go-azure-sdk/resource-manager/eventhub/2022-01-01-preview/namespaces"
"github.com/hashicorp/terraform-provider-azurerm/helpers/tf"
"github.com/hashicorp/terraform-provider-azurerm/internal/clients"
Expand Down Expand Up @@ -84,6 +86,8 @@ func resourceEventHubNamespaceCustomerManagedKey() *pluginsdk.Resource {
Default: false,
ForceNew: true,
},

"identity": commonschema.SystemOrUserAssignedIdentityOptional(),
katbyte marked this conversation as resolved.
Show resolved Hide resolved
},
}
}
Expand Down Expand Up @@ -116,7 +120,6 @@ func resourceEventHubNamespaceCustomerManagedKeyCreateUpdate(d *pluginsdk.Resour
}

namespace := resp.Model

keySource := namespaces.KeySourceMicrosoftPointKeyVault
namespace.Properties.Encryption = &namespaces.Encryption{
KeySource: &keySource,
Expand All @@ -126,6 +129,48 @@ func resourceEventHubNamespaceCustomerManagedKeyCreateUpdate(d *pluginsdk.Resour
if err != nil {
return err
}

assignedIdentities, err := identity.ExpandSystemOrUserAssignedMap(d.Get("identity").([]interface{}))
if err != nil {
return fmt.Errorf("expanding `identity`: %+v", err)
}

if assignedIdentities.Type == identity.TypeUserAssigned {
isIdentityAssignedToParent := false
userAssignedIdentity := ""

// only a single user assigned ID can be used (azure's error message is very unclear so we are doing these checks to make it clear the requirements to the user)
if len(assignedIdentities.IdentityIds) != 1 {
return fmt.Errorf("exactly one identity_ids is required if identity type is UserAssigned")
}

if namespace.Identity == nil {
return fmt.Errorf("user assigned identity must also be assigned to the parent event hub - currently no user assigned identities are assigned to the parent event hub")
}

for k := range assignedIdentities.IdentityIds {
userAssignedIdentity = k
}

for item := range namespace.Identity.IdentityIds {
if item == userAssignedIdentity {
isIdentityAssignedToParent = true
}
}

if !isIdentityAssignedToParent {
return fmt.Errorf("user assigned identity '%s' must also be assigned to the parent event hub", userAssignedIdentity)
}

// multiple keys can be assigned to an event hub, but only a single user managed ID can be utilized for all of them
// we just use the same user assigned ID for all keys assigned to the event hub
for i := 0; i < len(*keyVaultProps); i++ {
katbyte marked this conversation as resolved.
Show resolved Hide resolved
(*keyVaultProps)[i].Identity = &namespaces.UserAssignedIdentityProperties{
UserAssignedIdentity: &userAssignedIdentity,
}
}
}

namespace.Properties.Encryption.KeyVaultProperties = keyVaultProps
namespace.Properties.Encryption.RequireInfrastructureEncryption = utils.Bool(d.Get("infrastructure_encryption_enabled").(bool))

Expand Down Expand Up @@ -174,14 +219,17 @@ func resourceEventHubNamespaceCustomerManagedKeyRead(d *pluginsdk.ResourceData,

d.Set("key_vault_key_ids", keyVaultKeyIds)
d.Set("infrastructure_encryption_enabled", props.Encryption.RequireInfrastructureEncryption)

if err := d.Set("identity", getUserManagedIdentity(props.Encryption)); err != nil {
return fmt.Errorf("setting `identity`: %+v", err)
}
}

return nil
}

func resourceEventHubNamespaceCustomerManagedKeyDelete(d *pluginsdk.ResourceData, meta interface{}) error {
log.Printf(`[INFO] Customer Managed Keys cannot be removed from EventHub Namespaces once added. To remove the Customer Managed Key delete and recreate the parent EventHub Namespace")
`)
log.Printf(`[INFO] Customer Managed Keys cannot be removed from EventHub Namespaces once added. To remove the Customer Managed Key delete and recreate the parent EventHub Namespace`)
return nil
}

Expand All @@ -208,8 +256,31 @@ func expandEventHubNamespaceKeyVaultKeyIds(input []interface{}) (*[]namespaces.K
return &results, nil
}

func flattenEventHubNamespaceKeyVaultKeyIds(input *namespaces.Encryption) ([]interface{}, error) {
results := make([]interface{}, 0)
// Get the user managed id for the custom keys
func getUserManagedIdentity(input *namespaces.Encryption) *[]interface{} {
if input == nil || input.KeyVaultProperties == nil {
return nil
}

// we can only have a single user managed id for N number of keys, azure portal only allows setting a single one and then applies it to each key
for _, item := range *input.KeyVaultProperties {
if item.Identity != nil {
return &[]interface{}{
map[string]interface{}{
"type": "UserAssigned",
"identity_ids": []string{*item.Identity.UserAssignedIdentity},
"principal_id": "",
"tenant_id": "",
},
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

we've got consistent Expand and Flatten functions we use across the Provider for this - we're using the Expand above, so can we update this to use the Flatten method here so that this behaviour is normalised across the provider?

Copy link
Contributor Author

@bruceharrison1984 bruceharrison1984 Oct 25, 2023

Choose a reason for hiding this comment

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

So i dug into this a bit, and the reason I didn't use the existing Flatten method is because the data type that is returned for the encryption.keyVaultProperties.identity doesn't match the existing SystemOrUserAssignedList type. The identity block declared in this resource was used since it already existed, but the Identity data that is returned is just the ID of the User Managed ID that is associated with the encryption, alongside the Key Vault details.

We could do a one-off transform so it fits with FlattenSystemAssignedOrUserAssignedList, but it seems like then we accomplish the same thing with more cycles, and we still are left with a one-off transformation.

Comparison of payloads:

{
    "sku": {
        "name": "Premium",
        "tier": "Premium",
        "capacity": 1
    },
    "id": "/subscriptions/c6d2f82d-4baf-4b80-8d69-1fc6fbf92ad8/resourceGroups/bharrison-Tz6Ium/providers/Microsoft.EventHub/namespaces/bharrison-namespace-Tz6Ium",
    "name": "bharrison-namespace-Tz6Ium",
    "type": "Microsoft.EventHub/Namespaces",
    "location": "East US",
    "tags": {},
    "identity": {  <-- Parent Event Hub Identity Block (standard shape)
        "type": "UserAssigned",
        "userAssignedIdentities": {
            "/subscriptions/c6d2f82d-4baf-4b80-8d69-1fc6fbf92ad8/resourceGroups/bharrison-Tz6Ium/providers/Microsoft.ManagedIdentity/userAssignedIdentities/test": {
                "clientId": "5d7f143c-610c-4342-a574-941237de2d13",
                "principalId": "5fe787f4-e013-4202-8ef7-70e03c79b574"
            }
        }
    },
    "properties": {
        "minimumTlsVersion": "1.2",
        "publicNetworkAccess": "Enabled",
        "disableLocalAuth": false,
        "zoneRedundant": true,
        "isAutoInflateEnabled": false,
        "maximumThroughputUnits": 0,
        "encryption": {
            "keySource": "Microsoft.KeyVault",
            "keyVaultProperties": [
                {
                    "keyName": "bharrisonkey",
                    "keyVaultUri": "https://bharrisonkvtz6ium.vault.azure.net",
                    "keyVersion": "c90e6fcef73041d69801250f8bb60a86",
                    "identity": { <-- Encryption Identity block
                        "userAssignedIdentity": "/subscriptions/c6d2f82d-4baf-4b80-8d69-1fc6fbf92ad8/resourceGroups/bharrison-Tz6Ium/providers/Microsoft.ManagedIdentity/userAssignedIdentities/test"
                    }
                }
            ],
            "requireInfrastructureEncryption": false
        },
        "kafkaEnabled": true,
        "provisioningState": "Succeeded",
        "metricId": "c6d2f82d-4baf-4b80-8d69-1fc6fbf92ad8:bharrison-namespace-tz6ium",
        "createdAt": "2023-10-25T16:27:16.77Z",
        "updatedAt": "2023-10-25T16:34:43.68Z",
        "serviceBusEndpoint": "https://bharrison-namespace-Tz6Ium.servicebus.windows.net:443/",
        "status": "Active"
    }
}

Copy link
Contributor

Choose a reason for hiding this comment

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

In which case we're modelling this elsewhere in the provider as a single (string) field (user_assigned_identity_id) - so I think we should be able to take that approach here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So ditch the ID block and just go with a string?

I've got no issue with that, but I'll ask @katbyte to weigh in since she originally steered me towards the ID block.

I've got no preference, so either is fine with me.


return nil
}

func flattenEventHubNamespaceKeyVaultKeyIds(input *namespaces.Encryption) ([]string, error) {
results := make([]string, 0)
if input == nil || input.KeyVaultProperties == nil {
return results, nil
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,21 @@ func TestAccEventHubNamespaceCustomerManagedKey_basic(t *testing.T) {
})
}

func TestAccEventHubNamespaceCustomerManagedKey_withUserAssignedIdentity(t *testing.T) {
data := acceptance.BuildTestData(t, "azurerm_eventhub_namespace_customer_managed_key", "test")
r := EventHubNamespaceCustomerManagedKeyResource{}

data.ResourceTest(t, r, []acceptance.TestStep{
{
Config: r.withUserAssignedIdentity(data),
Check: acceptance.ComposeTestCheckFunc(
check.That(data.ResourceName).ExistsInAzure(r),
),
},
data.ImportStep(),
})
}

func TestAccEventHubNamespaceCustomerManagedKey_requiresImport(t *testing.T) {
data := acceptance.BuildTestData(t, "azurerm_eventhub_namespace_customer_managed_key", "test")
r := EventHubNamespaceCustomerManagedKeyResource{}
Expand Down Expand Up @@ -129,6 +144,22 @@ resource "azurerm_eventhub_namespace_customer_managed_key" "test" {
`, r.template(data))
}

func (r EventHubNamespaceCustomerManagedKeyResource) withUserAssignedIdentity(data acceptance.TestData) string {
return fmt.Sprintf(`
%s

resource "azurerm_eventhub_namespace_customer_managed_key" "test" {
eventhub_namespace_id = azurerm_eventhub_namespace.test.id
key_vault_key_ids = [azurerm_key_vault_key.test.id]

identity {
type = "UserAssigned"
identity_ids = [azurerm_user_assigned_identity.test.id]
}
}
`, r.templateWithUserAssignedIdentity(data))
}

func (r EventHubNamespaceCustomerManagedKeyResource) update(data acceptance.TestData) string {
return fmt.Sprintf(`
%s
Expand Down Expand Up @@ -262,3 +293,95 @@ resource "azurerm_key_vault_key" "test" {
}
`, data.RandomInteger, data.Locations.Primary, data.RandomInteger, data.RandomInteger, data.RandomString, data.RandomString)
}

func (r EventHubNamespaceCustomerManagedKeyResource) templateWithUserAssignedIdentity(data acceptance.TestData) string {
return fmt.Sprintf(`
provider "azurerm" {
features {
key_vault {
purge_soft_delete_on_destroy = false
purge_soft_deleted_keys_on_destroy = false
}
}
}

resource "azurerm_resource_group" "test" {
name = "acctestRG-namespacecmk-%d"
location = "%s"
}

resource "azurerm_eventhub_cluster" "test" {
name = "acctest-cluster-%d"
resource_group_name = azurerm_resource_group.test.name
location = azurerm_resource_group.test.location
sku_name = "Dedicated_1"
}

resource "azurerm_user_assigned_identity" "test" {
location = azurerm_resource_group.test.location
name = "acctest-identity-%s"
resource_group_name = azurerm_resource_group.test.name
}

resource "azurerm_eventhub_namespace" "test" {
name = "acctest-namespace-%d"
location = azurerm_resource_group.test.location
resource_group_name = azurerm_resource_group.test.name
sku = "Standard"
dedicated_cluster_id = azurerm_eventhub_cluster.test.id

identity {
type = "UserAssigned"
identity_ids = [azurerm_user_assigned_identity.test.id]
}
}

data "azurerm_client_config" "current" {}

resource "azurerm_key_vault" "test" {
name = "acctestkv%s"
location = azurerm_resource_group.test.location
resource_group_name = azurerm_resource_group.test.name
tenant_id = data.azurerm_client_config.current.tenant_id
sku_name = "standard"
purge_protection_enabled = true
}

resource "azurerm_key_vault_access_policy" "test" {
key_vault_id = azurerm_key_vault.test.id
tenant_id = azurerm_user_assigned_identity.test.tenant_id
object_id = azurerm_user_assigned_identity.test.principal_id

key_permissions = ["Get", "UnwrapKey", "WrapKey", "GetRotationPolicy"]
}

resource "azurerm_key_vault_access_policy" "test2" {
key_vault_id = azurerm_key_vault.test.id
tenant_id = data.azurerm_client_config.current.tenant_id
object_id = data.azurerm_client_config.current.object_id

key_permissions = [
"Create",
"Delete",
"Get",
"List",
"Purge",
"Recover",
"GetRotationPolicy"
]
}

resource "azurerm_key_vault_key" "test" {
name = "acctestkvkey%s"
key_vault_id = azurerm_key_vault.test.id
key_type = "RSA"
key_size = 2048
key_opts = ["decrypt", "encrypt", "sign", "unwrapKey", "verify", "wrapKey"]

depends_on = [
azurerm_key_vault_access_policy.test,
azurerm_key_vault_access_policy.test2,
]
}
`, data.RandomInteger, data.Locations.Primary, data.RandomInteger, data.RandomString, data.RandomInteger, data.RandomString, data.RandomString)
}
Loading