fix: Add DiffSuppressFunc on argocd_cluster.server

[mkilchhofer]

While working on this PR here:

• fix(argocd_application): don't flatten spec.info if list is empty #349

, I saw that the build is already broken on master branch:

It seems that the test is failing due to inconsistent handling of a trailing /.

=== RUN   TestAccArgoCDCluster_invalidSameServer
    resource_argocd_cluster_test.go:275: Step 3/3, expected an error with pattern, no match on: After applying this test step, the plan was not empty.
        stdout:
        
        
        Terraform used the selected providers to generate the following execution
        plan. Resource actions are indicated with the following symbols:
          ~ update in-place
        
        Terraform will perform the following actions:
        
          # argocd_cluster.cluster_with_trailing_slash will be updated in-place
          ~ resource "argocd_cluster" "cluster_with_trailing_slash" {
                id     = "https://kubernetes.default.svc.cluster.local/server"
                name   = "server"
              ~ server = "https://kubernetes.default.svc.cluster.local/" -> "https://kubernetes.default.svc.cluster.local/"
                # (1 unchanged attribute hidden)
        
                # (1 unchanged block hidden)
            }
        
        Plan: 0 to add, 1 to change, 0 to destroy.
    testing_new.go:82: Error running post-test destroy, there may be dangling resources: exit status 1
        
        Error: failed to delete cluster https://kubernetes.default.svc.cluster.local/server
        
        rpc error: code = PermissionDenied desc = permission denied

I am not familiar with the provider internals here but it seems that putting a diff-ignore function could help here. It is already present on the cluster create function:
https://github.com/oboukili/terraform-provider-argocd/blob/b42b92267a212b108537caf32a462c133f21813d/argocd/resource_argocd_cluster.go#L55

FYI @onematchfox

[onematchfox]

Thanks for opening this up. That test is notoriously flaky and has been a bother for quite some time. There is, however, already some handling for this case here. That said, this approach may be the better option. Unfortunately, I don't really have the bandwidth to give it the consideration it requires at this point. So, I hope you don't mind this PR languishing for a little while until I can do so. Have merged your other PR and will ensure that that fix is released ASAP.

[mkilchhofer]

That test is notoriously flaky and has been a bother for quite some time. There is, however, already some handling for this case here. That said, this approach may be the better option.

What I tested is this here manually with the provider version 6.0.3:

$ terraform apply

Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:
  + create

Terraform will perform the following actions:

  # argocd_cluster.cluster_with_trailing_slash will be created
  + resource "argocd_cluster" "cluster_with_trailing_slash" {
      + id     = (known after apply)
      + info   = (known after apply)
      + name   = "server"
      + server = "https://kubernetes.default.svc.cluster.local/"

      + config {
          + bearer_token = (sensitive value)

          + tls_client_config {
              + ca_data  = <<-EOT
                    -----BEGIN CERTIFICATE-----
                    REDACTED
                    -----END CERTIFICATE-----
                EOT
              + insecure = false
            }
        }
    }

Plan: 1 to add, 0 to change, 0 to destroy.

Do you want to perform these actions?
  Terraform will perform the actions described above.
  Only 'yes' will be accepted to approve.

  Enter a value: yes

argocd_cluster.cluster_with_trailing_slash: Creating...
argocd_cluster.cluster_with_trailing_slash: Creation complete after 2s [id=https://kubernetes.default.svc.cluster.local/server]

Apply complete! Resources: 1 added, 0 changed, 0 destroyed.

and plan again:

$ terraform apply
argocd_cluster.cluster_with_trailing_slash: Refreshing state... [id=https://kubernetes.default.svc.cluster.local/server]

Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:
  ~ update in-place

Terraform will perform the following actions:

  # argocd_cluster.cluster_with_trailing_slash will be updated in-place
  ~ resource "argocd_cluster" "cluster_with_trailing_slash" {
        id         = "https://kubernetes.default.svc.cluster.local/server"
        name       = "server"
      ~ server     = "https://kubernetes.default.svc.cluster.local" -> "https://kubernetes.default.svc.cluster.local/"
        # (2 unchanged attributes hidden)

        # (1 unchanged block hidden)
    }

Plan: 0 to add, 1 to change, 0 to destroy.

Do you want to perform these actions?
  Terraform will perform the actions described above.
  Only 'yes' will be accepted to approve.

  Enter a value:

Since the remove of trailing / is handled inside the Create function only, the diffing doesn't work properly.
I think can also be handled inside the read function but it is probably easier to only handle it like my proposal and maybe we can also remove the special handling you mentioned :)

[github-actions]

This pr is stale because it has been open 14 days with no activity. Remove stale label or comment or this will be closed in 10 days.

[github-actions]

This pr is stale because it has been open 14 days with no activity. Remove stale label or comment or this will be closed in 10 days.

[mkilchhofer]

not stale. ping @onematchfox

[github-actions]

This pr is stale because it has been open 14 days with no activity. Remove stale label or comment or this will be closed in 10 days.

[onematchfox]

Not stale... Been away. Hoping to get around to this soon.

[github-actions]

This pr is stale because it has been open 14 days with no activity. Remove stale label or comment or this will be closed in 10 days.

[mkilchhofer]

not stale

[onematchfox]

Sorry for the delay in getting to this. Such a small change. I hate that we need to do this but I do think it will do the job until such time as we look at migrating this resource to the plugin framework where we can look at custom types (e.g. URL) which provide a mechanism for checking semantic equality.