[service_fabric] Don't pass the reverse proxy port if not set

[hbuckle]

This prevents the reverse proxy port being passed to the API when not specified. Previously if you didn't specify it then a value of 0 was passed, which breaks the cluster provisioning.

I tried adding a test for this using TestCheckNoResourceAttr but I always get Check failed: Check 6/12 error: azurerm_service_fabric_cluster.test: Attribute 'node_type.0.reverse_proxy_endpoint_port' found when not expected so not sure if I am using it correctly?

func TestAccAzureRMServiceFabricCluster_reverseProxyNotSet(t *testing.T) {
	resourceName := "azurerm_service_fabric_cluster.test"
	ri := tf.AccRandTimeInt()
	location := testLocation()
	config := testAccAzureRMServiceFabricCluster_basic(ri, location, 3)

	resource.ParallelTest(t, resource.TestCase{
		PreCheck:     func() { testAccPreCheck(t) },
		Providers:    testAccProviders,
		CheckDestroy: testCheckAzureRMServiceFabricClusterDestroy,
		Steps: []resource.TestStep{
			{
				Config: config,
				Check: resource.ComposeTestCheckFunc(
					testCheckAzureRMServiceFabricClusterExists(resourceName),
					resource.TestCheckResourceAttr(resourceName, "management_endpoint", "http://example:80"),
					resource.TestCheckResourceAttr(resourceName, "add_on_features.#", "0"),
					resource.TestCheckResourceAttr(resourceName, "certificate.#", "0"),
					resource.TestCheckResourceAttr(resourceName, "reverse_proxy_certificate.#", "0"),
					resource.TestCheckNoResourceAttr(resourceName, "node_type.0.reverse_proxy_endpoint_port"),
					resource.TestCheckResourceAttr(resourceName, "client_certificate_thumbprint.#", "0"),
					resource.TestCheckResourceAttr(resourceName, "azure_active_directory.#", "0"),
					resource.TestCheckResourceAttr(resourceName, "diagnostics_config.#", "0"),
					resource.TestCheckResourceAttr(resourceName, "node_type.#", "1"),
					resource.TestCheckResourceAttr(resourceName, "node_type.0.instance_count", "3"),
					resource.TestCheckResourceAttr(resourceName, "tags.%", "0"),
				),
			},
			{
				ResourceName:      resourceName,
				ImportState:       true,
				ImportStateVerify: true,
			},
		},
	})
}

[katbyte]

Hi @hbuckle,

Thanks for the PR. I'm not sure why TestCheckNoResourceAttr isn't working, by all accounts it should 🤔 could you push that so i can take a closer look?

In addition could we get a test for updating the port, something like empty -> 7777 -> empty ?

[hbuckle]

@katbyte I've pushed the test and I'll look at adding the additional update one as well

[hbuckle]

@katbyte It appears from this comment that an unset optional value is set to 0 by Terraform so I've updated the test checks to reflect that. I've also added the update test.

[katbyte]

Thanks for the fix @hbuckle, LGTM 👍

[ghost]

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!