`azurerm_servicebus_namespace` supports `customer_managed_key` #15601

ms-henglu · 2022-02-25T07:59:02Z

katbyte

Have a build error on the merge branch:

ms-henglu · 2022-03-07T02:07:30Z

Hi @katbyte , I've updated the PR, would you please take another look? Thanks!

katbyte

Thanks @ms-henglu - have some test failures now:

<img width="1609" alt="image" src="https://user-images.githubusercontent.com/1638467/157171081-820c2318-8639-4f7b-addb-5c44c3015e5d.png">

ms-henglu · 2022-03-09T05:21:49Z

Hi, @katbyte , I've rebased this PR, the tests are passed in my local.

katbyte · 2022-03-10T20:23:57Z

they are still failing in teamcity..

------- Stdout: -------
=== RUN   TestAccServiceBusNamespaceCustomerManagedKey_basic
=== PAUSE TestAccServiceBusNamespaceCustomerManagedKey_basic
=== CONT  TestAccServiceBusNamespaceCustomerManagedKey_basic
    testing_new.go:70: Error running post-test destroy, there may be dangling resources: exit status 1
        
        Error: cannot purge key "acctestkvkey4j4xa" because vault "Vault: (Name \"acctestkv4j4xa\" / Resource Group \"acctestRG-servicebus-220310185644268784\")" has purge protection enabled
        
--- FAIL: TestAccServiceBusNamespaceCustomerManagedKey_basic (1313.26s)
FAIL

tombuildsstuff

hi @ms-henglu

Thanks for this PR - I've highlighted a major blocker for this PR which'll need to be addressed for us to continue with this. Unfortunately deleting and recreating all of the contents of the ServiceBus Namespace during the deletion of the Key will lead to the loss of all user data with no confirmation - so this needs to be addressed.

Thanks!

tombuildsstuff · 2022-03-11T15:35:32Z

internal/services/servicebus/servicebus_namespace_customer_managed_key_resource.go

+	// Since this isn't a real object and it cannot be disabled once Customer Managed Key at rest has been enabled
+	// And it must keep at least one key once Customer Managed Key is enabled
+	// So for the delete operation, it has to recreate the EventHub Namespace with disabled Customer Managed Key
+	deleteFuture, err := client.Delete(ctx, id.ResourceGroup, id.Name)
+	if err != nil {
+		return fmt.Errorf("deleting %s: %+v", *id, err)
+	}
+	if err = deleteFuture.WaitForCompletionRef(ctx, client.Client); err != nil {
+		if !response.WasNotFound(deleteFuture.Response()) {
+			return fmt.Errorf("failed to wait for removal of %q: %+v", id, err)
+		}
+	}
+
+	namespace := resp
+	namespace.Encryption = nil
+
+	future, err := client.CreateOrUpdate(ctx, id.ResourceGroup, id.Name, namespace)
+	if err != nil {
+		return fmt.Errorf("creating/updating %s: %+v", id, err)
+	}
+
+	if err = future.WaitForCompletionRef(ctx, client.Client); err != nil {
+		return fmt.Errorf("waiting for create/update of %s: %+v", id, err)
+	}


Unfortunately this isn't something we can ship - this deletes all messages and other configuration for the ServiceBus Namespace with no confirmation.

Can you reach out to the Service Team about this?

Unfortunately this feature can't be disabled,

As suggested above, I'd recommend reaching out to the Service Team here.

Whilst we unintentionally shipped support for this in EventHub Namespace - in 3.0 this resources delete becomes a noop - which isn't ideal and is the only possible alternative instead of removing this resource entirely.

In order to ship this, we'll need a means of disabling this - so I'd suggest chatting with the Service Team here, as deleting all user data and configuration silently within the ServiceBus Namespace isn't something we can ship unfortunately.

Hi @tombuildsstuff ,

I've confirmed with service team, that CMK can't be disabled and won't support disabling CMK in the future. I've added a a commit to make delete no-op. Please take another look, thanks!

@ms-henglu since this can't be disabled we'll need to instead inline this within the ServiceBus Namespace resource - whilst that's not ideal (as it means this can't work for System Assigned Identities), it's the only way to make this work given the service limitations here

ms-henglu · 2022-03-14T02:26:44Z

Hi @tombuildsstuff ,

Thank you for taking time to review the codes.

Unfortunately this feature can not be disabled, maybe adding some explanation to the docs can unblock merging this PR?

ms-henglu · 2022-03-23T03:19:26Z

Hi @tombuildsstuff ,

I've implemented the CMK as a nested block in service bus namespace resource. Would you please take another look? Thanks!

ms-henglu · 2022-03-23T05:58:21Z

teamcity test results, thouth there's a failed case which is not related.

katbyte · 2022-03-29T02:43:52Z

website/docs/r/servicebus_namespace.html.markdown

@@ -64,6 +66,19 @@ A `identity` block supports the following:

 * `identity_ids` - (Optional) A list of User Managed Identity ID's which should be assigned to the ServiceBus Namespace.

+---
+
+-> **Note:** Once customer-managed key encryption has been enabled, it cannot be disabled.


should we have a custom diff that forces a new resource if the key is removed?

what happens if we change the key?

Good idea! I've added the custom diff.

And it allows user to change the key and identity, but can't toggle infrastructure_encryption_enabled, so I added ForceNew to it.

Please take another look, thanks!

katbyte

Thanks @ms-henglu - LGTM now 🚜

github-actions · 2022-04-08T07:18:00Z

This functionality has been released in v3.1.0 of the Terraform Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

github-actions · 2022-05-09T02:30:42Z

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.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

github-actions bot added documentation service/service-bus size/XL labels Feb 25, 2022

ms-henglu force-pushed the ticket-13229868-servicebus-namespace-supports-cmk branch from 4f0cd66 to cfeeb74 Compare February 28, 2022 03:09

katbyte requested changes Mar 4, 2022

View reviewed changes

ms-henglu force-pushed the ticket-13229868-servicebus-namespace-supports-cmk branch from cfeeb74 to 9b7ab3c Compare March 4, 2022 08:03

ms-henglu requested a review from katbyte March 7, 2022 02:07

katbyte requested changes Mar 8, 2022

View reviewed changes

ms-henglu force-pushed the ticket-13229868-servicebus-namespace-supports-cmk branch from 9b7ab3c to 3129b9a Compare March 9, 2022 05:19

tombuildsstuff requested changes Mar 11, 2022

View reviewed changes

tombuildsstuff added this to the Blocked milestone Mar 11, 2022

tombuildsstuff added the waiting-response label Mar 22, 2022

ms-henglu force-pushed the ticket-13229868-servicebus-namespace-supports-cmk branch from 41a4301 to 251420e Compare March 23, 2022 03:18

github-actions bot added size/L and removed size/XL labels Mar 23, 2022

github-actions bot removed the waiting-response label Mar 23, 2022

ms-henglu changed the title ~~new resource azurerm_servicebus_namespace_customer_managed_key~~ azurerm_servicebus_namespace supports customer_managed_key Mar 23, 2022

ms-henglu requested a review from tombuildsstuff March 24, 2022 08:46

katbyte reviewed Mar 29, 2022

View reviewed changes

ms-henglu force-pushed the ticket-13229868-servicebus-namespace-supports-cmk branch from 251420e to 90c2748 Compare March 29, 2022 07:25

ms-henglu added 2 commits March 31, 2022 09:54

servicebus_namepsace supports customer_managed_key

3102463

add customizeDiff to force new when change customer_managed_key

10020c2

ms-henglu force-pushed the ticket-13229868-servicebus-namespace-supports-cmk branch from 90c2748 to 10020c2 Compare March 31, 2022 01:54

ms-henglu requested a review from katbyte March 31, 2022 06:36

katbyte approved these changes Apr 6, 2022

View reviewed changes

katbyte modified the milestones: Blocked, v3.1.0 Apr 6, 2022

katbyte merged commit ce99c26 into hashicorp:main Apr 6, 2022

katbyte added a commit that referenced this pull request Apr 6, 2022

CHANGELOG.md for #15601

67fb4c6

This was referenced Apr 6, 2022

azurerm_servicebus_namespace is missing infrastructure encryption #15630

Closed

Support for Service Bus Premium Customer-Managed Key Encryption #7116

Closed

github-actions bot locked as resolved and limited conversation to collaborators May 9, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`azurerm_servicebus_namespace` supports `customer_managed_key` #15601

`azurerm_servicebus_namespace` supports `customer_managed_key` #15601

ms-henglu commented Feb 25, 2022 •

edited

katbyte left a comment

ms-henglu commented Mar 7, 2022

katbyte left a comment

ms-henglu commented Mar 9, 2022

katbyte commented Mar 10, 2022

tombuildsstuff left a comment

tombuildsstuff Mar 11, 2022

ms-henglu Mar 14, 2022

tombuildsstuff Mar 14, 2022

ms-henglu Mar 15, 2022

tombuildsstuff Mar 22, 2022

ms-henglu commented Mar 14, 2022

ms-henglu commented Mar 23, 2022

ms-henglu commented Mar 23, 2022

katbyte Mar 29, 2022

ms-henglu Mar 29, 2022

katbyte left a comment

github-actions bot commented Apr 8, 2022

github-actions bot commented May 9, 2022

azurerm_servicebus_namespace supports customer_managed_key #15601

azurerm_servicebus_namespace supports customer_managed_key #15601

Conversation

ms-henglu commented Feb 25, 2022 • edited

katbyte left a comment

Choose a reason for hiding this comment

ms-henglu commented Mar 7, 2022

katbyte left a comment

Choose a reason for hiding this comment

ms-henglu commented Mar 9, 2022

katbyte commented Mar 10, 2022

tombuildsstuff left a comment

Choose a reason for hiding this comment

tombuildsstuff Mar 11, 2022

Choose a reason for hiding this comment

ms-henglu Mar 14, 2022

Choose a reason for hiding this comment

tombuildsstuff Mar 14, 2022

Choose a reason for hiding this comment

ms-henglu Mar 15, 2022

Choose a reason for hiding this comment

tombuildsstuff Mar 22, 2022

Choose a reason for hiding this comment

ms-henglu commented Mar 14, 2022

ms-henglu commented Mar 23, 2022

ms-henglu commented Mar 23, 2022

katbyte Mar 29, 2022

Choose a reason for hiding this comment

ms-henglu Mar 29, 2022

Choose a reason for hiding this comment

katbyte left a comment

Choose a reason for hiding this comment

github-actions bot commented Apr 8, 2022

github-actions bot commented May 9, 2022

`azurerm_servicebus_namespace` supports `customer_managed_key` #15601

`azurerm_servicebus_namespace` supports `customer_managed_key` #15601

ms-henglu commented Feb 25, 2022 •

edited