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

v3.0.0 - Use dynamic type for Helm chart values #1562

Open
stevehipwell opened this issue Jan 17, 2025 · 1 comment
Open

v3.0.0 - Use dynamic type for Helm chart values #1562

stevehipwell opened this issue Jan 17, 2025 · 1 comment

Comments

@stevehipwell
Copy link

Description

Given that this provider can now support dynamic types and the fact that any object type is valid for the Helm values, it would make sense to make use of dynamic types when specifying chart values. This should be relevant to the set inputs, but would be even more powerful for the values input. The primary driver here is using the provider like Helm and passing in a block of YAML (or HCL) and getting a better diff on the changes.

Our current pattern is as follows and it results in a poor diff experience as the whole values string is shown for any change.

locals {
  chart_values = {
    myValue = "a"

    myOtherValue = {
      nestedValue = [1, 2, 3]
    }
  }
}

resource "helm_release" "test" { 
  values = [yamlencode(local.chart_values)]
}

If the values type was made dynamic it could support list of strings, a single string, list of object, or object; and the diff would only show the actual changes instead of the whole string.

Potential Terraform Configuration

The example from above could then be represented by the following code.

locals {
  chart_values = {
    myValue = "a"

    myOtherValue = {
      nestedValue = [1, 2, 3]
    }
  }
}

resource "helm_release" "test" { 
  values = local.chart_values
}

References

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment
@stevehipwell
Copy link
Author

I've just implemented this pattern in another provider and have first hand experience of how much better this pattern is for representing YAML. It would make this provider significantly better as changes to the values would show the actual changes instead of the whole YAML string. Given that there are already breaking changes for v3, I think this should be a priority. I'm happy to open a PR.

The code changes are minimal, and depending on the exact implementation it could significantly reduce the provider complexity.

Values                   types.Dynamic    `tfsdk:"values"`
			"values": schema.DynamicAttribute{
				Optional:    true,
				Description: "Values to pass in to Helm",
			},

The above code would support creating a list of object to be merged, but for the sake of simplicity it would be better to constrain this to force a single object.

Further to the above comment this would also allow for a more idiomatic pattern by removing set, set_list & set_sensitive as values would cover all of the use cases due to the way Terraform operates (the HCL values would be composed before being passed to the resource).

CC @JaylonmcShan03 @jrhouston @BBBmau

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

2 participants