terraform-provider-helm icon indicating copy to clipboard operation
terraform-provider-helm copied to clipboard

Helm manifest diffs aren't generated for OCI charts

Open mmckeen opened this issue 5 months ago • 6 comments

Terraform, Provider, Kubernetes and Helm Versions

Terraform version: v1.3.4
Provider version: 2.12.1
Kubernetes version: 1.26.13

Affected Resource(s)

  • helm_release

Terraform Configuration Files

provider "helm" {
  burst_limit = 3000
  kubernetes {
    host                   = local.host
    cluster_ca_certificate = local.cluster_ca_certificate
    insecure               = var.kube_insecure
    client_certificate     = local.client_certificate
    client_key             = local.client_key
    token                  = local.token
  }
  experiments {
    manifest = true
  }
  registry {
    url      = var.oci_url
    username = var.user
    password = var.password
  }
}

resource "helm_release" "foo" {
  name      = "foo"
  namespace = "bar"
  chart     = "bar"
  version   = "baz"
  repository          = "${var.oci_url}/foo"

  ...
}

Debug Output

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

Steps to Reproduce

Update the version of the released chart, run a Terraform plan.

Expected Behavior

Expect the manifest diff to show up in the plan.

Actual Behavior

No diff shows up, we've forked the provider and debugged the issue down to the following:

2024-02-06T22:40:50.491Z [INFO]  provider.terraform-provider-helm_2.12.1: 2024/02/06 22:40:50 [DEBUG] [resourceDiff: postgres-operator] Failed to get chart: unable to lookup chart "oci://container-registry.secretcdn.net/fastly/helm-charts/postgres-operator", missing registry client: timestamp=2024-02-06T22:40:50.491Z

this comes from a log line added to https://github.com/hashicorp/terraform-provider-helm/blob/8da862177cb9dd1ad8cdd36b309a4ad59752f691/helm/resource_release.go#L867 and https://github.com/helm/helm/blob/v3.14.0/pkg/action/install.go#L726 where it seems like the registry client doesn't get properly initialized from provider configuration prior to trying to fetch the chart from the OCI registry.

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

mmckeen avatar Feb 06 '24 23:02 mmckeen

I also notice in other code paths, we create a cpo object via https://github.com/hashicorp/terraform-provider-helm/blob/8da862177cb9dd1ad8cdd36b309a4ad59752f691/helm/resource_release.go#L555 (actions.NewUpgrade/NewInstall) which is responsible for initializing the repository client. That doesn't seem to happen within the resourceDiff codepath and instead the cpo object is initialized empty https://github.com/hashicorp/terraform-provider-helm/blob/8da862177cb9dd1ad8cdd36b309a4ad59752f691/helm/resource_release.go#L860 and thus a repository client is never present for the getChart call.

mmckeen avatar Feb 06 '24 23:02 mmckeen

I'd also ask that we make https://github.com/hashicorp/terraform-provider-helm/blob/8da862177cb9dd1ad8cdd36b309a4ad59752f691/helm/resource_release.go#L868 return the error rather than silently failing.

mmckeen avatar Feb 06 '24 23:02 mmckeen

Currently working on a draft PR to fix this.

mmckeen avatar Feb 07 '24 00:02 mmckeen

Hello @mmckeen, can you provide a terraform config that reproduces the output that you're experiencing? Your provided config is lacking information in order to be reproducible.

BBBmau avatar Feb 13 '24 21:02 BBBmau

Hello @mmckeen, can you provide a terraform config that reproduces the output that you're experiencing? Your provided config is lacking information in order to be reproducible.

Hey @BBBmau 👋, the issue will show up whenever the stored manifest for the chart changes as part of the plan, I'm not able to fully produce a config with the issue since we're using private charts but any change to the resource that changes the applied manifest should be able to reproduce the issue.

mmckeen avatar Feb 14 '24 19:02 mmckeen

We are having the same issue.

I tested using the same chart, with identical config, on two resources. One is coming from a public github repo, the other is from our private ECR using OCI. We pushed the chart as-is in our private ECR. The only one providing a manifest diff is the public one.

resource "helm_release" "prometheus_operator" {
  name                = "prometheus-operator"
  chart               = "oci://###.dkr.ecr.us-east-1.amazonaws.com/helmchart/kube-prometheus-stack"
  repository_username = var.private_ecr_creds.username
  repository_password = var.private_ecr_creds.password
  namespace           = var.namespace
  version             = "54.2.2"

  values = sensitive([local.prometheus_values])
}
resource "helm_release" "prometheus_operator" {
  name       = "prometheus-operator"
  repository = "https://prometheus-community.github.io/helm-charts"
  chart      = "kube-prometheus-stack"
  namespace  = var.namespace
  version    = "54.2.2"

  values = sensitive([local.prometheus_values])
}

In these examples, local.prometheus_values is a templated yaml values file using templatefile. Its set sensitive just because we don't want to see the huge values diff as it has low value for us. Manifests, though are high value in the diff

Hope this helps with reproduction

sbeginCoveo avatar Feb 19 '24 14:02 sbeginCoveo