hashicorp / terraform-provider-helm

Terraform Helm provider
https://www.terraform.io/docs/providers/helm/
Mozilla Public License 2.0
1k stars 371 forks source link

Prevent setting variables with a `null` value #753

Open nitrocode opened 3 years ago

nitrocode commented 3 years ago

Terraform, Provider, Kubernetes and Helm Versions

Terraform version: 0.15.x
Provider version: 2.1.2
Kubernetes version: 1.20

Affected Resource(s)

Terraform Configuration Files

variable "disable_polling" {
  default = null
}

resource "helm_release" "this" {
  name = "external-secrets"

  # ... etc

  #### This will not work since DISABLE_POLLING will be enabled with a non null value
  #
  # set {
  #   name  = "env.DISABLE_POLLING"
  #   value = var.disable_polling
  # }

  # This is the workaround
  dynamic "set" {
    for_each = {
      for k, v in local.helm_values :
      k => v
      if v != null
    }
    content {
      name  = set.key
      value = set.value
    }
  }
}

locals {
  helm_values = {
    "nameOverride"        = "external-secrets"
    "env.DISABLE_POLLING" = var.disable_polling
  }
}

Debug Output

NOTE: In addition to Terraform debugging, please set HELM_DEBUG=1 to enable debugging info from helm.

Panic Output

Steps to Reproduce

  1. terraform apply

Expected Behavior

The helm_release resource should prevent any sets if the value is null

Actual Behavior

Using null caused a terraform error. The workaround of using dynamic eliminates the error and does what's expected.

Important Factoids

N/A

References

N/A

Community Note

jnmcfly commented 2 years ago

same goes for values = [ ]. When I try this:

values = [
    file("settings/values.yaml"),
    fileexists("settings/helm/${var.environment}/values.yaml") ? file("settings/helm/${var.environment}/values.yaml") : null,
  ]

I got Null values are not allowed for this attribute value. if the second file not exist

anapsix commented 1 year ago

Imho, I'd expect Helm provider to set a value as passed to mimic the --set env.DISABLE_POLLING=null behavior. Alas, it's not possible with set {} directive.

nitrocode commented 1 year ago

@jnmcfly instead of null, an empty string can be used and surrounded by compact()

  values = compact([
    file("${path.module}/settings/values.yaml"),
    fileexists("${path.module}/settings/helm/${var.environment}/values.yaml") ? file(
      "settings/helm/${var.environment}/values.yaml"
    ) : "",
  ])

@anapsix agreed! The helm provider should mimic the --set directive

github-actions[bot] commented 11 months ago

Marking this issue as stale due to inactivity. If this issue receives no comments in the next 30 days it will automatically be closed. If this issue was automatically closed and you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. This helps our maintainers find and focus on the active issues. Maintainers may also remove the stale label at their discretion. Thank you!