terraform-provider-azurerm icon indicating copy to clipboard operation
terraform-provider-azurerm copied to clipboard

Default value For premium_messaging_partitions in azurerm_servicebus_namespace is destructive

Open ImIOImI opened this issue 1 year ago • 6 comments

Is there an existing issue for this?

  • [X] I have searched the existing issues

Community Note

  • Please vote on this issue by adding a :thumbsup: reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment and review the contribution guide to help.

Terraform Version

1.6.1

AzureRM Provider Version

3.93.0

Affected Resource(s)/Data Source(s)

azurerm_servicebus_namespace

Terraform Configuration Files

resource "azurerm_servicebus_namespace" "this" {
  name                         = "name"
  capacity                     = 1
  minimum_tls_version          = "1.0"
  location                     = "us-eastus"
  resource_group_name          = "myResourceGroup"
  sku                          = "Premium"
}

Debug Output/Panic Output

# azurerm_servicebus_namespace.this must be replaced
-/+ resource "azurerm_servicebus_namespace" "this" {
      ~ default_primary_connection_string   = (sensitive value)
      ~ default_primary_key                 = (sensitive value)
      ~ default_secondary_connection_string = (sensitive value)
      ~ default_secondary_key               = (sensitive value)
      ~ endpoint                            = "https://name.servicebus.windows.net:443/" -> (known after apply)
      ~ id                                  = "/subscriptions/..." -> (known after apply)
        name                                = "name"
      ~ premium_messaging_partitions        = 1 -> 0 # forces replacement       
        # (8 unchanged attributes hidden)

      - network_rule_set {
          - default_action                = "Deny" -> null
          - ip_rules                      = [] -> null
          - public_network_access_enabled = true -> null
          - trusted_services_allowed      = false -> null

          - network_rules {
              - ignore_missing_vnet_service_endpoint = false -> null
              - subnet_id                            = "/subscriptions/..." -> null
            }
        }
    }

Expected Behaviour

I would expect when upgrading Azurerm versions the provider to prioritize non-destructive changes. Here we have the added functionality of being able to set premium partitions set to 0 as default. Since changing the partitions (which were at 1) to 0 causes a redeploy of the namespace, any existing namespace will be destroyed. If Terraform attempts to recreate the namespace, it will fail because premium sku partitions cannot be 0.

I would expect Terraform to either

  1. throw an error when premium_messaging_partitions is set on a non-premium sku and default the number to 1
  2. ignore the attribute premium_messaging_partitions for non-premium skus and default the number to 1

Actual Behaviour

When updating the Azurerm provider this causes the namespace to be deleted and terraform throws an error when attempting to rebuild the namespace.

Steps to Reproduce

  1. Have an existing namespace using 3.92.0 or lower
  2. Upgrade to 3.93.0
  3. terraform init
  4. terraform apply

Important Factoids

No response

References

No response

ImIOImI avatar Feb 23 '24 22:02 ImIOImI

Thanks @ImIOImI for raising this issue. I'd like to explain about the reason of assiging a default value to this property. If there is no default value, the user will also get plan diff like 1->null, like below: image

So the case of assigning the default value to 1 or 0 is almost the same, either standard namespace or premium namespace will receive the plan diff.

let me check the issue, prior to that, can you try either adding ignore_changes block to this property, or explicitly assigning a >0 value to the premium namespace?

Example:

resource "azurerm_servicebus_namespace" "premium" {
  name                = "xiaxinpremium"
  location            = azurerm_resource_group.primary.location
  resource_group_name = azurerm_resource_group.primary.name
  sku                 = "Premium"
  zone_redundant      = true
  capacity            = 8

  lifecycle {
    ignore_changes = ["premium_messaging_partitions"]
  }
}

xiaxyi avatar Feb 26 '24 05:02 xiaxyi

@xiaxyi I totally understand why it was defaulted that way, and yes setting premium_messaging_partitions = 1 stops terraform from replacing it. However, setting premium_messaging_partitions = 1 as default will break Basic and Standard skus

the plan will look like:

# azurerm_servicebus_namespace.basic must be replaced
-/+ resource "azurerm_servicebus_namespace" "basic" {
      ~ default_primary_connection_string   = (sensitive value)
      ~ default_primary_key                 = (sensitive value)
      ~ default_secondary_connection_string = (sensitive value)
      ~ default_secondary_key               = (sensitive value)
      ~ endpoint                            = "https://basic-skilled-anteater.servicebus.windows.net:443/" -> (known after apply)
      ~ id                                  = "/subscriptions/.../resourceGroups/staging/providers/Microsoft.ServiceBus/namespaces/basic-skilled-anteater" -> (known after apply)
      ~ minimum_tls_version                 = "1.2" -> (known after apply)
        name                                = "basic-skilled-anteater"
      ~ premium_messaging_partitions        = 0 -> 1 # forces replacement
      - tags                                = {} -> null
      - zone_redundant                      = false -> null
        # (6 unchanged attributes hidden)

      - network_rule_set {
          - default_action                = "Allow" -> null
          - ip_rules                      = [] -> null
          - public_network_access_enabled = true -> null
          - trusted_services_allowed      = false -> null
        }
    }

  # azurerm_servicebus_namespace.standard must be replaced
-/+ resource "azurerm_servicebus_namespace" "standard" {
      ~ default_primary_connection_string   = (sensitive value)
      ~ default_primary_key                 = (sensitive value)
      ~ default_secondary_connection_string = (sensitive value)
      ~ default_secondary_key               = (sensitive value)
      ~ endpoint                            = "https://standard-skilled-anteater.servicebus.windows.net:443/" -> (known after apply)
      ~ id                                  = "/subscriptions/.../resourceGroups/staging/providers/Microsoft.ServiceBus/namespaces/standard-skilled-anteater" -> (known after apply)
      ~ minimum_tls_version                 = "1.2" -> (known after apply)
        name                                = "standard-skilled-anteater"
      ~ premium_messaging_partitions        = 0 -> 1 # forces replacement
      - tags                                = {} -> null
      - zone_redundant                      = false -> null
        # (6 unchanged attributes hidden)

      - network_rule_set {
          - default_action                = "Allow" -> null
          - ip_rules                      = [] -> null
          - public_network_access_enabled = true -> null
          - trusted_services_allowed      = false -> null
        }
    }

and the error looks like:

│ Error: Premium messaging partition is not supported by service Bus SKU "Standard" and it can only be set to 0
│
│   with azurerm_servicebus_namespace.standard,
│   on main.tf line 1, in resource "azurerm_servicebus_namespace" "standard":
│    1: resource "azurerm_servicebus_namespace" "standard" {
│
╵
╷
│ Error: Premium messaging partition is not supported by service Bus SKU "Basic" and it can only be set to 0
│
│   with azurerm_servicebus_namespace.basic,
│   on main.tf line 9, in resource "azurerm_servicebus_namespace" "basic":
│    9: resource "azurerm_servicebus_namespace" "basic" {
│

I'm just wondering if there's a way to make it to where upgrading won't break any existing skus. I imagine the most common use case will be to not have premium_messaging_partitions defined at all, so as long as it works in that case I'm happy with whatever solution.

ImIOImI avatar Feb 26 '24 17:02 ImIOImI

Thanks for the response @ImIOImI , for premium namespace, would you mind explicitly setting the premium_messaging_partition to a >1 value, or adding the ignore_changes in the lifecycles block? I understand that you may look for an answer which doesn't include any modifications in the config, unfortunately, I don't have a perfect answers now as the default value diffs from SKU, either premium / standard would be impacted... Appreciate if you can share your idea with me if you have a better solution. Thanks!

xiaxyi avatar Feb 27 '24 01:02 xiaxyi

@xiaxyi I totally understand why it was defaulted that way, and yes setting premium_messaging_partitions = 1 stops terraform from replacing it

@xiaxyi I apologize if the above wasn't clear enough, but it indeed stops Terraform from replacing the namespace. This is how I've set it now. I ran it that way before I even opened up this issue and it was fine.

I'm just reporting this and asking for a better solution. One that doesn't replace the namespace by default. Personally, I think it makes no sense to set premium_messaging_partitions on basic and standard skus. Any attempt to do so should throw an error. This is a consistent behavior with many of the other Azurerm APIs, where setting non-sensical values results in an error. On the backend the value for premium_messaging_partitions should be set to null and not passed to the Azure API. For premium skus it would make sense to set the default to 1.

ImIOImI avatar Feb 27 '24 02:02 ImIOImI

Thanks @ImIOImI for the follow ups. The property is always returned by api, even for standard namespace, the value is set to 0, so there will be a diff as well..., but I'll check if there is a better way to not set the property for the standard namespace.

xiaxyi avatar Feb 27 '24 05:02 xiaxyi

Any update on this? Just ran into the issue and we're having both Standard & Premium service bus.

tomaaron avatar Jun 28 '24 11:06 tomaaron