Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changing Variable to a JSON value in the UI makes next Terraform run fail #254

Closed
felixpelletier opened this issue Sep 5, 2024 · 4 comments
Assignees

Comments

@felixpelletier
Copy link
Contributor

felixpelletier commented Sep 5, 2024

Community Note

  • Please vote on this issue by adding a 👍 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.

Terraform Version

terraform 0.15.3 (I know it's old, but I don't believe the issue is there)
provider v2.2.2

Affected Resource(s)

  • prefect_variable

Terraform Configuration Files

resource "prefect_variable" "some_variable" {
  name = "some_variable_name"
  value = jsonencode([])
}

Expected Behavior

After creating the variable, I expected the variable's value to be a JSON.

After modifying the value from "[]" to ["some_value"] (notice the absence of surrounding double-quotes) in the UI and trying to re-apply the terraform plan: I would expect it to convert the JSON to a String so I can decode it and use it.

Actual Behavior

After creating the variable, the value is a string "[]"

Applying the plan results in an error:
Could not get Variable, unexpected error: failed to decode response: json: cannot unmarshal array into Go struct field Variable.value of type string

Even when adding an ignore_changes lifecycle rule, the problem persists. (Which makes sense. It's still a data source).

Steps to Reproduce

  1. terraform apply
  2. In the Cloud UI, change the value from "[]" to ["some_value"]
  3. terraform apply

Important Factoids

N/A

References

N/A

@felixpelletier felixpelletier added the bug Something isn't working label Sep 5, 2024
@parkedwards
Copy link
Contributor

thanks for raising @felixpelletier - let us take a look at this

@mitchnielsen
Copy link
Contributor

mitchnielsen commented Oct 4, 2024

Alrighty, looks like I was able to replicate this:

First, I created a variable in the Prefect UI and gave it a value of ["some_value"].

Then I created the Terraform config to read that variable as a datasource:

data "prefect_variable" "some_variable" {
  name = "some_variable_name"
}

output "some_variable" {
  value = data.prefect_variable.some_variable.value
}

When I run terraform apply, I get:

Could not get Variable, unexpected error: failed to decode response: json: cannot unmarshal array into Go struct field Variable.value of type string

This makes sense, because what I've stored in the UI is an array. If I modify that array to be a string, I can get this to work:

["some_value"]
"[\"some_value\"]"

This gets me:

Changes to Outputs:
  ~ some_variable = jsonencode(
      ~ [
          + "some_value",
        ]
    )

You can apply this plan to save these new output values to the Terraform state, without changing
any real infrastructure.

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


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

Outputs:

some_variable = "[\"some_value\"]"

@mitchnielsen
Copy link
Contributor

So what's the fix? We'll want to change the underlying Terraform Type of Variable values from string to JSON. This is due to changes made fairly recently to Variables in the API to make them JSON objects instead of just strings (example: PrefectHQ/prefect#13543).

We'll go ahead and do that in the provider and publish that change as soon as we can. In the meantime, you should be able to work with plain strings just fine. Using JSON objects (arrays, maps, etc) will be supported once we push that update.

Thanks again @felixpelletier!

mitchnielsen added a commit that referenced this issue Oct 4, 2024
Changes the Variable 'value' type from string to JSON.

In PrefectHQ/prefect#13543 and associated
changes around May 2024, Variables were updated from simple strings to
JSON objects. The Terraform provider has still been treating them as
strings, so when folks tried to put JSON-compatible values in them,
Terraform would fail to work with them as found in
#254

Related to https://linear.app/prefect/issue/PLA-247/changing-variable-to-a-json-value-in-the-ui-makes-next-terraform-run

Related to #254
mitchnielsen added a commit that referenced this issue Oct 25, 2024
* fix(variable): change 'value' type to JSON

Changes the Variable 'value' type from string to JSON.

In PrefectHQ/prefect#13543 and associated
changes around May 2024, Variables were updated from simple strings to
JSON objects. The Terraform provider has still been treating them as
strings, so when folks tried to put JSON-compatible values in them,
Terraform would fail to work with them as found in
#254

Related to https://linear.app/prefect/issue/PLA-247/changing-variable-to-a-json-value-in-the-ui-makes-next-terraform-run

Related to #254

* Datasource: use Dynamic attribute

* Change variable attribute type to dynamic

The JSON custom type won't work for variables, because they can actually
be almost any type - including standalone numbers and strings.

* Update test to change value types

Goes from a string to a bool in the test to confirm variables can store
multiple types.

* Correctly test the name change

Before, when trying to test a variable resource name change, we were
actually creating an entirely new resource because we use the same
(randomized) value for the resource name and the attribute 'name'.

To ensure we're updating the same resource, this hard-codes the resource
name to 'test'. This shouldn't cause any conflicts with other tests
because the resource is created in an ephemeral workspace.

* Support and test bool, number, and object

* Tidy up

* Support tuple, correctly test tuple and object

* Generate Terraform Docs

* Document supported value types

* Datasource: break value type checking into func

It's big enough that it helps to have it separated out.

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
@mitchnielsen
Copy link
Contributor

Hi @felixpelletier, we've released v2.5.0 which includes a change to the variable value type to a dynamic Terraform type, which supports more types than just a string. The specific types supported are listed in the docs, and we're definitely open to supporting more types in the future depending on use cases.

Thanks again for reporting this 👍🏼

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants