hashicorp / terraform-plugin-go

A low-level Go binding for the Terraform protocol for integrations to be built on top of.
Mozilla Public License 2.0
128 stars 33 forks source link

using `(tftypes.Types).Equal` to check for Type change doesn't work as expected. #392

Open BBBmau opened 7 months ago

BBBmau commented 7 months ago

This occurs when checking for a Type change between two configs that share have different values but still share the same type. Despite this it still returns the wrong bool.

["test"] -> ["test", "test2"] causes Equal() to return false despite the Type being the same

Using Diff instead results in the expected output of no type being present.

terraform-plugin-go version

v0.20.0

Relevant provider source code

// resulted in type change despite the types staying the same
typeChanged := !(wasCfg.(tftypes.Value).Type().Equal(nowCfg.(tftypes.Value).Type()))

// produced diff == nil since no type change was present (same tfconfig used)
_, diff := wasCfg.(tftypes.Value).Diff(nowCfg.(tftypes.Value))

Terraform Configuration Files

resource "kubernetes_manifest" "raw" {
  field_manager {
    name            = "tf-argo-app"
    # force_conflicts = true
  }

  timeouts {
    create = "10m"
    update = "10m"
    delete = "10m"
  }

  manifest = {
    apiVersion = "argoproj.io/v1alpha1"
    kind       = "Application"

    metadata = {
      name        = "unknown-test"
      namespace   = "argocd"
    }

    spec = {
      project = ""
      destination = {
        server    = "testcluster"
        namespace = "argocd"
      }
      revisionHistoryLimit = 1
//
      sources = [{
        repoURL        = "https://helm.releases.hashicorp.com"
        path           = "hashicorp/terraform"
        chart          = "terraform-helm"
        targetRevision = 10
        helm = {
          releaseName = "test"
          values = "test"
          valuesObject = {
            duration    = 2
            maxDuration = 5
            factor      = "2"
          }
        }
      }
      ,
      {
        repoURL        = "https://helm.releases.hashicorp.com"
        chart          = "terraform-helm"
        targetRevision = 1
        helm = {
          releaseName = "test"
          values = "test"
          valuesObject = {
            "mau": ["test"] // make changes here such as add or removing elements from tuple
            }
        }
      }]
//
      syncPolicy = {
        retry = {
          limit = 10
          backoff = {
            duration    = "2m"
            maxDuration = "5m"
            factor      = 2
          }
        }
      }
    }
  }
}

Expected Behavior

Should have returned that no type change was present

Actual Behavior

Type change was present when adding/removing from the tuple.

Steps to Reproduce

Please list the full steps required to reproduce the issue, for example:

  1. terraform init
  2. terraform apply
  3. add or remove from tuple that's commented in config

The change will be reproducible once this PR is merged. https://github.com/hashicorp/terraform-provider-kubernetes/pull/2437

A simpler config can be used, the provided config was the one that was first brought to attention the different behavior of Equal() and Diff()

austinvalle commented 7 months ago

Hey there @BBBmau 👋🏻 ,

Tuples are structural types where the type is made up of each exact element type, so I believe your example of adding a new element to the tuple should result in a difference between types.

For example, these two tuple outputs are different types:

output "tuple_one" {
  # tuple[string]
  value = ["test"]
}

output "tuple_two" {
  # tuple[string, string]
  value = ["test", "test_two"]
}

State output:

{
  "version": 4,
  "terraform_version": "1.8.0",
  "serial": 28,
  "lineage": "87f4a8d4-57e7-2f4e-d081-aef9e7e6b675",
  "outputs": {
    "tuple_one": {
      "value": [
        "test"
      ],
      "type": [
        "tuple",
        [
          "string"
        ]
      ]
    },
    "tuple_two": {
      "value": [
        "test",
        "test_two"
      ],
      "type": [
        "tuple",
        [
          "string",
          "string"
        ]
      ]
    }
  },
  "resources": [],
  "check_results": null
}

If they were a collection type, like a list which has a single element type, then they would be considered an equal type (which I know this kubernetes_manifest resource is dynamic, so it unfortunately can't force that scenario on the practitioner):

output "tuple_one" {
  # list(string)
  value = tolist(["test"])
}

output "tuple_two" {
  # list(string)
  value = tolist(["test", "test_two"])
}

State output:

{
  "version": 4,
  "terraform_version": "1.8.0",
  "serial": 29,
  "lineage": "87f4a8d4-57e7-2f4e-d081-aef9e7e6b675",
  "outputs": {
    "tuple_one": {
      "value": [
        "test"
      ],
      "type": [
        "list",
        "string"
      ]
    },
    "tuple_two": {
      "value": [
        "test",
        "test_two"
      ],
      "type": [
        "list",
        "string"
      ]
    }
  },
  "resources": [],
  "check_results": null
}
austinvalle commented 7 months ago

I'd need to dig into it deeper, but I think Diff may have a bug that is allowing tuples and lists to be considered equivalent types when comparing, which shouldn't be the expected behavior (based on how Terraform defines the tuple type)