-
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_servicebus_subscription - support status property #7852
azurerm_servicebus_subscription - support status property #7852
Conversation
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.
Hey @marc-sensenich, these changes look good but we can aim for a simpler one liner over the helper functions. We should be good to merge once that is taken care of!
@@ -222,6 +236,7 @@ func resourceArmServiceBusSubscriptionRead(d *schema.ResourceData, meta interfac | |||
d.Set("requires_session", props.RequiresSession) | |||
d.Set("forward_to", props.ForwardTo) | |||
d.Set("forward_dead_lettered_messages_to", props.ForwardDeadLetteredMessagesTo) | |||
d.Set("status", helper.FlattenEntityStatus(props.Status)) |
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.
Similar to the above comment, we should aim for simplicity here.
d.Set("status", helper.FlattenEntityStatus(props.Status)) | |
d.Set("status", string(props.Status)) |
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.
Changed this to a one-liner in 87b1115
@@ -145,6 +158,7 @@ func resourceArmServiceBusSubscriptionCreateUpdate(d *schema.ResourceData, meta | |||
EnableBatchedOperations: &enableBatchedOps, | |||
MaxDeliveryCount: &maxDeliveryCount, | |||
RequiresSession: &requiresSession, | |||
Status: helper.ExpandEntityStatus(d.Get("status")), |
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.
I appreciate all the work you put into making the helper function to get the status but we should aim for simplicity. We know that status
can only be servicebus.EntityStatus
because of what we allow in the ValidateFunc
so we can be confident that we can get away with a single line versus the helper function + tests that you wrote
Status: helper.ExpandEntityStatus(d.Get("status")), | |
Status: servicebus.EntityStatus(d.Get("status").(string)), |
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.
Changed this to a one-liner in 87b1115
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.
Thanks @marc-sensenich - this LGTM 👍
This has been released in version 2.21.0 of the provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. As an example: provider "azurerm" {
version = "~> 2.21.0"
}
# ... other configuration ... |
## Description 🗣️ This pull requests is easier to read commit by commit `SBQueueProperties` struct contains a handful of read-write attributes, with a few of them not being exposed to the terraform schema representing it. This includes `enable_batched_operations`, `forward_to`, `forward_dead_lettered_messages_to` ( fixes #6472 ) and `status`. The spec itself is described in <https://docs.microsoft.com/en-us/azure/templates/microsoft.servicebus/2017-04-01/namespaces/queues>. This pulls requests aims to align the current terraform schema with the `2017-01-01` spec for Azure ServiceBus Queues, with the following three components : 1. Add the missing attributes 2. Reorder the attributes to match the official doc 3. Update the doc accordingly ## Motivation - [x] Fix #6472 - [x] Backport d331091 ( `enable_batched_operations` ) - [x] Backport #861 ( `forward_to` ) - [x] Backport #4789 ( `forward_dead_lettered_messages_to` ) - [x] Backport #7852 ( `status` ) ## Testing ### Initialization <details><summary>This section is merely a small doc for myself</summary> <p> See details in <https://medium.com/@gmusumeci/getting-started-with-terraform-and-microsoft-azure-a2fcb690eb67> ```bash $ az login $ az ad sp create-for-rbac --name terraform-provider-azurerm-acceptance-tests --role Contributor --scopes /subscriptions/00000000-0000-0000-0000-000000000000 Changing "terraform-provider-azurerm-acceptance-tests" to a valid URI of "http://ayaz-tf-acceptance-tests", which is the required format used for service principal names Creating a role assignment under the scope of "/subscriptions/00000000-0000-0000-0000-000000000000" Retrying role assignment creation: 1/36 Retrying role assignment creation: 2/36 { "appId": "<client-id>", "displayName": "terraform-provider-azurerm-acceptance-tests", "name": "http://terraform-provider-azurerm-acceptance-tests", "password": "<client-secret>", "tenant": "<tenant-id>" } ``` Once the credentials have been retrieved, save them as environment variables.⚠️ Never run these commands as is, otherwise they will be written to disk within the shell history file. ```bash $ export ARM_CLIENT_ID='<client-id>' $ export ARM_CLIENT_SECRET='<client-secret>' $ export ARM_SUBSCRIPTION_ID='00000000-0000-0000-0000-000000000000' $ export ARM_TENANT_ID='<tenant-id>' $ export ARM_TEST_LOCATION='West US 2' $ export ARM_TEST_LOCATION_ALT='West US 2' $ export ARM_TEST_LOCATION_ALT2='West US 2' ``` And now we're good to go testing ! </p> </details> ### Run Run tests with the following commands : ```bash $ make acctests SERVICE="servicebus" TESTARGS="-run=TestAccAzureRMServiceBusQueue" TESTTIMEOUT="6h" ; terminal-notifier -title "terraform-provider-azurerm" -message "Acceptance tests just finished \!" Wed Aug 5 21:43:16 CEST 2020 ==> Checking that code complies with gofmt requirements... ==> Checking that Custom Timeouts are used... ==> Checking that acceptance test packages are used... TF_ACC=1 go test -v ./azurerm/internal/services/servicebus/tests/ -run=TestAccAzureRMServiceBusQueue -timeout 6h -ldflags="-X=github.com/terraform-providers/terraform-provider-azurerm/version.ProviderVersion=acc" === RUN TestAccAzureRMServiceBusQueueAuthorizationRule_listen === PAUSE TestAccAzureRMServiceBusQueueAuthorizationRule_listen === RUN TestAccAzureRMServiceBusQueueAuthorizationRule_send === PAUSE TestAccAzureRMServiceBusQueueAuthorizationRule_send === RUN TestAccAzureRMServiceBusQueueAuthorizationRule_listensend === PAUSE TestAccAzureRMServiceBusQueueAuthorizationRule_listensend === RUN TestAccAzureRMServiceBusQueueAuthorizationRule_manage === PAUSE TestAccAzureRMServiceBusQueueAuthorizationRule_manage === RUN TestAccAzureRMServiceBusQueueAuthorizationRule_rightsUpdate === PAUSE TestAccAzureRMServiceBusQueueAuthorizationRule_rightsUpdate === RUN TestAccAzureRMServiceBusQueueAuthorizationRule_requiresImport === PAUSE TestAccAzureRMServiceBusQueueAuthorizationRule_requiresImport === RUN TestAccAzureRMServiceBusQueue_basic === PAUSE TestAccAzureRMServiceBusQueue_basic === RUN TestAccAzureRMServiceBusQueue_requiresImport === PAUSE TestAccAzureRMServiceBusQueue_requiresImport === RUN TestAccAzureRMServiceBusQueue_update === PAUSE TestAccAzureRMServiceBusQueue_update === RUN TestAccAzureRMServiceBusQueue_enablePartitioningStandard === PAUSE TestAccAzureRMServiceBusQueue_enablePartitioningStandard === RUN TestAccAzureRMServiceBusQueue_defaultEnablePartitioningPremium === PAUSE TestAccAzureRMServiceBusQueue_defaultEnablePartitioningPremium === RUN TestAccAzureRMServiceBusQueue_enableDuplicateDetection === PAUSE TestAccAzureRMServiceBusQueue_enableDuplicateDetection === RUN TestAccAzureRMServiceBusQueue_enableRequiresSession === PAUSE TestAccAzureRMServiceBusQueue_enableRequiresSession === RUN TestAccAzureRMServiceBusQueue_enableDeadLetteringOnMessageExpiration === PAUSE TestAccAzureRMServiceBusQueue_enableDeadLetteringOnMessageExpiration === RUN TestAccAzureRMServiceBusQueue_lockDuration === PAUSE TestAccAzureRMServiceBusQueue_lockDuration === RUN TestAccAzureRMServiceBusQueue_isoTimeSpanAttributes === PAUSE TestAccAzureRMServiceBusQueue_isoTimeSpanAttributes === RUN TestAccAzureRMServiceBusQueue_maxDeliveryCount === PAUSE TestAccAzureRMServiceBusQueue_maxDeliveryCount === RUN TestAccAzureRMServiceBusQueue_forwardTo === PAUSE TestAccAzureRMServiceBusQueue_forwardTo === RUN TestAccAzureRMServiceBusQueue_forwardDeadLetteredMessagesTo === PAUSE TestAccAzureRMServiceBusQueue_forwardDeadLetteredMessagesTo === RUN TestAccAzureRMServiceBusQueue_status === PAUSE TestAccAzureRMServiceBusQueue_status === CONT TestAccAzureRMServiceBusQueueAuthorizationRule_listen === CONT TestAccAzureRMServiceBusQueue_enableDuplicateDetection === CONT TestAccAzureRMServiceBusQueue_maxDeliveryCount === CONT TestAccAzureRMServiceBusQueue_lockDuration --- PASS: TestAccAzureRMServiceBusQueueAuthorizationRule_listen (227.06s) === CONT TestAccAzureRMServiceBusQueue_defaultEnablePartitioningPremium --- PASS: TestAccAzureRMServiceBusQueue_enableDuplicateDetection (271.59s) === CONT TestAccAzureRMServiceBusQueue_enablePartitioningStandard --- PASS: TestAccAzureRMServiceBusQueue_lockDuration (286.99s) === CONT TestAccAzureRMServiceBusQueue_update --- PASS: TestAccAzureRMServiceBusQueue_maxDeliveryCount (286.99s) === CONT TestAccAzureRMServiceBusQueue_requiresImport --- PASS: TestAccAzureRMServiceBusQueue_requiresImport (247.19s) === CONT TestAccAzureRMServiceBusQueue_basic --- PASS: TestAccAzureRMServiceBusQueue_enablePartitioningStandard (344.15s) === CONT TestAccAzureRMServiceBusQueueAuthorizationRule_requiresImport --- PASS: TestAccAzureRMServiceBusQueue_defaultEnablePartitioningPremium (407.24s) === CONT TestAccAzureRMServiceBusQueueAuthorizationRule_rightsUpdate --- PASS: TestAccAzureRMServiceBusQueue_basic (197.08s) === CONT TestAccAzureRMServiceBusQueueAuthorizationRule_manage --- PASS: TestAccAzureRMServiceBusQueueAuthorizationRule_rightsUpdate (247.67s) === CONT TestAccAzureRMServiceBusQueueAuthorizationRule_listensend --- PASS: TestAccAzureRMServiceBusQueue_update (614.87s) === CONT TestAccAzureRMServiceBusQueueAuthorizationRule_send --- PASS: TestAccAzureRMServiceBusQueueAuthorizationRule_manage (207.24s) === CONT TestAccAzureRMServiceBusQueue_isoTimeSpanAttributes --- PASS: TestAccAzureRMServiceBusQueueAuthorizationRule_requiresImport (395.49s) === CONT TestAccAzureRMServiceBusQueue_status --- PASS: TestAccAzureRMServiceBusQueueAuthorizationRule_listensend (260.17s) === CONT TestAccAzureRMServiceBusQueue_forwardDeadLetteredMessagesTo --- PASS: TestAccAzureRMServiceBusQueue_isoTimeSpanAttributes (212.45s) === CONT TestAccAzureRMServiceBusQueue_enableDeadLetteringOnMessageExpiration --- PASS: TestAccAzureRMServiceBusQueue_forwardDeadLetteredMessagesTo (230.64s) === CONT TestAccAzureRMServiceBusQueue_forwardTo --- PASS: TestAccAzureRMServiceBusQueueAuthorizationRule_send (492.21s) === CONT TestAccAzureRMServiceBusQueue_enableRequiresSession --- PASS: TestAccAzureRMServiceBusQueue_enableDeadLetteringOnMessageExpiration (291.38s) --- PASS: TestAccAzureRMServiceBusQueue_status (571.67s) --- PASS: TestAccAzureRMServiceBusQueue_forwardTo (228.06s) --- PASS: TestAccAzureRMServiceBusQueue_enableRequiresSession (233.17s) PASS ok github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/servicebus/tests 1628.884s ``` ### Document <details><summary>Do not use this section, the diff is huge</summary> <p> ```bash $ make scaffold-website BRAND_NAME="ServiceBus Queue" RESOURCE_NAME="azurerm_servicebus_queue" RESOURCE_TYPE="resource" RESOURCE_ID="/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/mygroup1/providers/microsoft.servicebus/namespaces/sbns1/queues/snqueue1" ./scripts/scaffold-website.sh ==> Verifying required variables are set... ==> Validated. ==> Scaffolding Documentation... ==> Done. ``` </p> </details> ### Cleanup <details><summary>This section is merely a small doc for myself</summary> <p> In this code block, `$ARM_CLIENT_ID` is the content of the `appId` retrieved in the initialization section. ```bash $ az ad sp delete --id $ARM_CLIENT_ID Removing role assignments ``` </p> </details>
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks! |
Allows the status to be set for an azurerm_servicebus_subscription resource
New Acceptance Tests
Existing Acceptance Tests