New Resource: `azurerm_servicebus_namespace_customer_managed_key` #28888

catriona-m · 2025-02-26T14:32:09Z

Community Note

Please vote on this PR by adding a 👍 reaction to the original PR to help the community and maintainers prioritize for review
Please do not leave comments along the lines of "+1", "me too" or "any updates", they generate extra noise for PR followers and do not help prioritize for review

Description

This PR adds support for the new resource azurerm_servicebus_namespace_customer_managed_key

PR Checklist

I have followed the guidelines in our Contributing Documentation.
I have checked to ensure there aren't other open Pull Requests for the same update/change.
I have checked if my changes close any open issues. If so please include appropriate closing keywords below.
I have updated/added Documentation as required written in a helpful and kind way to assist users that may be unfamiliar with the resource / data source.
I have used a meaningful PR title to help maintainers and other users understand this change and help prevent duplicate work.
For example: “resource_name_here - description of change e.g. adding property new_property_name_here”

Testing

My submission includes Test coverage as described in the Contribution Guide and the tests pass. (if this is not possible for any reason, please include details of why you did or could not add test coverage)

=== RUN   TestAccServicebusNamespaceCustomerManagedKey_basic
=== PAUSE TestAccServicebusNamespaceCustomerManagedKey_basic
=== CONT  TestAccServicebusNamespaceCustomerManagedKey_basic
--- PASS: TestAccServicebusNamespaceCustomerManagedKey_basic (581.52s)
=== RUN   TestAccServicebusNamespaceCustomerManagedKey_complete
=== PAUSE TestAccServicebusNamespaceCustomerManagedKey_complete
=== CONT  TestAccServicebusNamespaceCustomerManagedKey_complete
--- PASS: TestAccServicebusNamespaceCustomerManagedKey_complete (584.67s)
=== RUN   TestAccServicebusNamespaceCustomerManagedKey_requiresImport
=== PAUSE TestAccServicebusNamespaceCustomerManagedKey_requiresImport
=== CONT  TestAccServicebusNamespaceCustomerManagedKey_requiresImport
--- PASS: TestAccServicebusNamespaceCustomerManagedKey_requiresImport (529.35s)
=== RUN   TestAccServicebusNamespaceCustomerManagedKey_updated
=== PAUSE TestAccServicebusNamespaceCustomerManagedKey_updated
=== CONT  TestAccServicebusNamespaceCustomerManagedKey_updated
--- PASS: TestAccServicebusNamespaceCustomerManagedKey_updated (780.80s)
=== RUN   TestAccServicebusNamespaceCustomerManagedKey_userAssignedIdentityUpdated
=== PAUSE TestAccServicebusNamespaceCustomerManagedKey_userAssignedIdentityUpdated
=== CONT  TestAccServicebusNamespaceCustomerManagedKey_userAssignedIdentityUpdated
--- PASS: TestAccServicebusNamespaceCustomerManagedKey_userAssignedIdentityUpdated (496.32s)
PASS

Change Log

Below please provide what should go into the changelog (if anything) conforming to the Changelog Format documented here.

New Resource: azurerm_servicebus_namespace_customer_managed_key [GH-00000]

This is a (please select all that apply):

Bug Fix
New Feature (ie adding a service, resource, or data source)
Enhancement
Breaking Change

Related Issue(s)

Fixes #21313

Note

If this PR changes meaningfully during the course of review please update the title and description as required.

stephybun · 2025-02-27T09:10:50Z

internal/services/servicebus/servicebus_namespace_customer_managed_key_resource.go

+				identityId, err := commonids.ParseUserAssignedIdentityID(cmk.IdentityID)
+				if err != nil {
+					return err
+				}
+				(*payload.Properties.Encryption.KeyVaultProperties)[0].Identity = &namespaces.UserAssignedIdentityProperties{
+					UserAssignedIdentity: pointer.To(identityId.ID()),


The validation function on the property will ensure that the user input is a user assigned identity ID with correct casing in the static segments, so there isn't strictly a need to parse that here if we're just passing the string value of it to the API

Suggested change

identityId, err := commonids.ParseUserAssignedIdentityID(cmk.IdentityID)

if err != nil {

return err

}

(*payload.Properties.Encryption.KeyVaultProperties)[0].Identity = &namespaces.UserAssignedIdentityProperties{

UserAssignedIdentity: pointer.To(identityId.ID()),

(*payload.Properties.Encryption.KeyVaultProperties)[0].Identity = &namespaces.UserAssignedIdentityProperties{

UserAssignedIdentity: pointer.To(cmk.IdentityID),

stephybun · 2025-02-27T09:13:28Z

internal/services/servicebus/servicebus_namespace_customer_managed_key_resource.go

+			var state ServiceBusNamespaceCustomerManagedKeyModel
+			state.NamespaceID = id.ID()
+
+			if props := resp.Model.Properties; props != nil && props.Encryption != nil {


We should also nil check model here:

Suggested change

if props := resp.Model.Properties; props != nil && props.Encryption != nil {

if model := resp.Model; model != nil {

if props := model.Properties; props != nil && props.Encryption != nil {

...

}

stephybun · 2025-02-27T09:14:48Z

internal/services/servicebus/servicebus_namespace_customer_managed_key_resource.go

+				identityId, err := commonids.ParseUserAssignedIdentityID(cmk.IdentityID)
+				if err != nil {
+					return err
+				}
+				(*payload.Properties.Encryption.KeyVaultProperties)[0].Identity = &namespaces.UserAssignedIdentityProperties{
+					UserAssignedIdentity: pointer.To(identityId.ID()),
+				}


Suggested change

identityId, err := commonids.ParseUserAssignedIdentityID(cmk.IdentityID)

if err != nil {

return err

}

(*payload.Properties.Encryption.KeyVaultProperties)[0].Identity = &namespaces.UserAssignedIdentityProperties{

UserAssignedIdentity: pointer.To(identityId.ID()),

}

(*payload.Properties.Encryption.KeyVaultProperties)[0].Identity = &namespaces.UserAssignedIdentityProperties{

UserAssignedIdentity: pointer.To(cmk.IdentityID),

}

stephybun · 2025-02-27T09:17:12Z

internal/services/servicebus/servicebus_namespace_customer_managed_key_resource_test.go

+func (r ServicebusNamespaceCustomerManagedKeyResource) userAssigned(data acceptance.TestData) string {
+	return fmt.Sprintf(`
+%s
+
+resource "azurerm_servicebus_namespace_customer_managed_key" "test" {
+  namespace_id                      = azurerm_servicebus_namespace.test.id
+  key_vault_key_id                  = azurerm_key_vault_key.test.id
+  identity_id                       = azurerm_user_assigned_identity.test.id
+  infrastructure_encryption_enabled = true
+}
+`, r.templateUserAssigned(data))
+}
+
+func (r ServicebusNamespaceCustomerManagedKeyResource) userAssignedUpdated(data acceptance.TestData) string {
+	return fmt.Sprintf(`
+%s
+
+resource "azurerm_servicebus_namespace_customer_managed_key" "test" {
+  namespace_id                      = azurerm_servicebus_namespace.test.id
+  key_vault_key_id                  = azurerm_key_vault_key.test.id
+  identity_id                       = azurerm_user_assigned_identity.test.id
+  infrastructure_encryption_enabled = true
+}
+`, r.templateUserAssigned(data))


I can't tell the difference between these two configs, are they the same?

stephybun · 2025-02-27T09:18:26Z

internal/services/servicebus/servicebus_namespace_customer_managed_key_resource_test.go

+
+	data.ResourceTest(t, r, []acceptance.TestStep{
+		{
+			Config: r.complete(data),


We usually use the basic config for the requires import check

Suggested change

Config: r.complete(data),

Config: r.basic(data),

stephybun · 2025-02-27T09:20:47Z

website/docs/r/servicebus_namespace_customer_managed_key.html.markdown

+layout: "azurerm"
+page_title: "Azure Resource Manager: azurerm_servicebus_namespace"
+description: |-
+  Manages a ServiceBus Namespace Customer Managed Key.


I realise ServiceBus is written together as one word in a lot of our documentation, but I think this should actually be split into two words. Could you split it in this doc and we can update the others at a later point?

stephybun · 2025-02-27T09:31:07Z

website/docs/r/servicebus_namespace_customer_managed_key.html.markdown

+Manages a ServiceBus Namespace Customer Managed Key.
+
+!> **Note:** It is not possible to remove the Customer Managed Key from the ServiceBus Namespace once it's been added. To remove the Customer Managed Key, the parent ServiceBus Namespace must be deleted and recreated.
+


I think it would be good to warn users not to try and manage the CMK with both this resource and the block on the resource. We have this for other resources where something that can be configured in-line is also available as a virtual resource

Suggested change

~> **Note:** It's possible to define a Customer Managed Key both within [the `azurerm_servicebus_namespace` resource](servicebus_namespace.html) via the `customer_managed_key` block and by using this resource. It is not possible to use both methods to manage a Customer Managed Key for a Service Bus Namespace since it will result in a conflict.

A note on the service bus namespace resource under the customer_managed_key block informing users that they need to add this to ignore_changes if they're using this resource to manage CMK might also be a good addition too.

add new resource azurerm_servicebus_namespace_customer_managed_key

5c9f009

catriona-m requested a review from a team as a code owner February 26, 2025 14:32

github-actions bot added enhancement documentation service/service-bus size/XL labels Feb 26, 2025

stephybun requested changes Feb 27, 2025

View reviewed changes

github-actions bot assigned stephybun Feb 27, 2025

github-actions bot added the waiting-response label Feb 27, 2025

stephybun mentioned this pull request Feb 27, 2025

azurerm_log_analytics_cluster_customer_managed_key - fix create/update/delete functions #28862

Merged

14 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New Resource: `azurerm_servicebus_namespace_customer_managed_key` #28888

New Resource: `azurerm_servicebus_namespace_customer_managed_key` #28888

catriona-m commented Feb 26, 2025 •

edited

Loading

stephybun Feb 27, 2025

stephybun Feb 27, 2025

stephybun Feb 27, 2025

stephybun Feb 27, 2025

stephybun Feb 27, 2025

stephybun Feb 27, 2025

stephybun Feb 27, 2025

-			if props := resp.Model.Properties; props != nil && props.Encryption != nil {
+			if model := resp.Model; model != nil {
+				if props := model.Properties; props != nil && props.Encryption != nil {
+				...
+			}

		Manages a ServiceBus Namespace Customer Managed Key.

		!> Note: It is not possible to remove the Customer Managed Key from the ServiceBus Namespace once it's been added. To remove the Customer Managed Key, the parent ServiceBus Namespace must be deleted and recreated.



	~> Note: It's possible to define a Customer Managed Key both within [the `azurerm_servicebus_namespace` resource](servicebus_namespace.html) via the `customer_managed_key` block and by using this resource. It is not possible to use both methods to manage a Customer Managed Key for a Service Bus Namespace since it will result in a conflict.

New Resource: azurerm_servicebus_namespace_customer_managed_key #28888

Are you sure you want to change the base?

New Resource: azurerm_servicebus_namespace_customer_managed_key #28888

Conversation

catriona-m commented Feb 26, 2025 • edited Loading

Community Note

Description

PR Checklist

Testing

Change Log

Related Issue(s)

stephybun Feb 27, 2025

Choose a reason for hiding this comment

stephybun Feb 27, 2025

Choose a reason for hiding this comment

stephybun Feb 27, 2025

Choose a reason for hiding this comment

stephybun Feb 27, 2025

Choose a reason for hiding this comment

stephybun Feb 27, 2025

Choose a reason for hiding this comment

stephybun Feb 27, 2025

Choose a reason for hiding this comment

stephybun Feb 27, 2025

Choose a reason for hiding this comment

New Resource: `azurerm_servicebus_namespace_customer_managed_key` #28888

New Resource: `azurerm_servicebus_namespace_customer_managed_key` #28888

catriona-m commented Feb 26, 2025 •

edited

Loading