fix: change mTLS associated hostnames to a set

[BSFishy]

The API doesn't order mTLS associated hostnames in any meaningful way. This can cause drift as seen in #3436. We can mitigate this by ignoring associated hostname changes, when the only thing that has changed is the ordering.

I tried a number of other approaches including sorting the list that is returned from the API and sorting the incoming associated hostname list, but none of that type of stuff worked.

fixes #3436

[github-actions]

changelog detected ✅

[ajholland]

Looks good to me!

[jacobbednarz]

the changes you're making here are basically what TypeSet is built for. i'd suggest we swap to that instead and remove the additional schema functions here.

[BSFishy]

the changes you're making here are basically what TypeSet is built for. i'd suggest we swap to that instead and remove the additional schema functions here.

I thought what I was doing felt wrong! Thanks for pointing me in the right direction. Updated and ready for re-review :)

[bporter816]

Hi @jacobbednarz, can we get this reviewed for the next release?

[jacobbednarz]

the acceptance tests are failing here (and on master) so we'll need to work out what needs to change to support it.

TF_ACC=1 go test ./internal/sdkv2provider -v -run "^TestAccCloudflareAccessMutualTLS" -count 1 -timeout 120m -parallel 1
=== RUN   TestAccCloudflareAccessMutualTLSBasic
    resource_cloudflare_access_mutual_tls_certificate_test.go:75: Step 1/2 error: Error running apply: exit status 1
        
        Error: error creating Access Mutual TLS Certificate for accounts "f037e56e89293a057740de681ac9abbe": error from makeRequest: access.api.error.certificate_required (12032)
        
          with cloudflare_access_mutual_tls_certificate.jndisxqick,
          on terraform_plugin_test.tf line 12, in resource "cloudflare_access_mutual_tls_certificate" "jndisxqick":
          12: resource "cloudflare_access_mutual_tls_certificate" "jndisxqick" {
        
--- FAIL: TestAccCloudflareAccessMutualTLSBasic (0.86s)
=== RUN   TestAccCloudflareAccessMutualTLSBasicWithZoneID
    resource_cloudflare_access_mutual_tls_certificate_test.go:118: Step 1/2 error: Error running apply: exit status 1
        
        Error: error creating Access Mutual TLS Certificate for zones "0da42c8d2132a9ddaf714f9e7c920711": error from makeRequest: access.api.error.certificate_required (12032)
        
          with cloudflare_access_mutual_tls_certificate.sjkcssabds,
          on terraform_plugin_test.tf line 12, in resource "cloudflare_access_mutual_tls_certificate" "sjkcssabds":
          12: resource "cloudflare_access_mutual_tls_certificate" "sjkcssabds" {
        
--- FAIL: TestAccCloudflareAccessMutualTLSBasicWithZoneID (0.72s)
FAIL
FAIL	github.com/cloudflare/terraform-provider-cloudflare/internal/sdkv2provider	2.365s
FAIL
make: *** [testacc] Error 1

[bporter816]

From the error message, is it possible that CLOUDFLARE_MUTUAL_TLS_CERTIFICATE isn't being set? In terms of the changes needed for this PR, seems like there needs to be several changes in this file, like using TestCheckResourceAddrSet instead of TestCheckResourceAttr, toset([]) instead of [], etc.

[BSFishy]

I think I agree with @bporter816 that it looks like the CLOUDFLARE_MUTUAL_TLS_CERTIFICATE variable isn't being set. Apart from that, the file mentioned seems to be fine to me. To my understanding, TestCheckResourceAttrSet checks if an attribute is set, rather than if it contains a specific value (which for the certificate is fine I think), and lists should automatically be coerced to sets where necissary, according to these docs. Let me know if any of this sounds wrong and I can take another look

[bporter816]

Ah yes, I think I misread the docs on that function. Another thing I wasn't sure about is if things like this indexing still work:

terraform-provider-cloudflare/internal/sdkv2provider/resource_cloudflare_access_mutual_tls_certificate_test.go

Line 131 in 4708b62

resource.TestCheckResourceAttr(name, "associated_hostnames.0", domain),

[jacobbednarz]

nice find. after fixing that (and adding a check in the tests for future runners), this is all now passing

TF_ACC=1 go test ./internal/sdkv2provider -v -run "^TestAccCloudflareAccessMutualTLS" -count 1 -timeout 120m -parallel 1
=== RUN   TestAccCloudflareAccessMutualTLSBasic
--- PASS: TestAccCloudflareAccessMutualTLSBasic (9.57s)
=== RUN   TestAccCloudflareAccessMutualTLSBasicWithZoneID
--- PASS: TestAccCloudflareAccessMutualTLSBasicWithZoneID (9.22s)
PASS
ok  	github.com/cloudflare/terraform-provider-cloudflare/internal/sdkv2provider	19.298s

[github-actions]

This functionality has been released in v4.41.0 of the Terraform Cloudflare 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!