Output CRDs separate from manifests

[phillebaba]

Description

Currently the helm_template datasource will pass the rendered manifests through the two outputs manifest and manifests. The difference being that one is a multipart yaml while the other is a map containing each individual file. With the optional variable include_crds the contents of the CRD directory in the helm chart will be added to the outputs, this variable is false by default.

Helm has made an active choice to not manage CRD upgrades if the CRD already exists in the cluster. Instead it is up to the end user to manage this, which is not optimal with large amounts of clusters. Instead this should be possible to automate, preferably with existing maintained providers.

One option is that the helm_template datasource adds a new output called crds which is a map with all of the CRDs only. This would make it possible to make use of other provider such as terraform-provider-kubectl to apply the manifests before the Helm chart is installed.

Potential Terraform Configuration

data "helm_template" "opa_gatekeeper" {
  repository = "https://open-policy-agent.github.io/gatekeeper/charts"
  chart      = "gatekeeper"
  version    = "3.6.0"
  name       = "gatekeeper"
}

resource "kubectl_manifest" "opa_gatekeeper_crd" {
  for_each = data.helm_template.opa_gatekeeper.crds
  yaml_body  = each.value
}

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

[phillebaba]

I can implement this, but my guess is that it really is not a big feature to implement.

[jrhouston]

@phillebaba You are probably right that we could make the interface for the manifests attribute more convenient. I expect in future we will change it to a more structured object and bucket things by API group and kind rather than just being a flat map using the template path as a key.

Your proposal to have a separate attribute for the rendered CRDs does make sense as include_crds is a separate attribute that needs to be explicitly enabled, so I would be open to adding that to the data source. We would just need to add the new attribute and some additional logic to pick out the CRDs and add them to it here:

terraform-provider-helm/helm/data_template.go

Lines 541 to 550 in 82fe05d

	for _, manifestKey := range manifestsToRender {
	manifest := splitManifests[manifestKey]
	manifestName := manifestNamesByKey[manifestKey]
	// Manifests
	computedManifests[manifestName] = fmt.Sprintf("%s---\n%s\n", computedManifests[manifestName], manifest)
	// Manifest bundle
	fmt.Fprintf(computedManifest, "---\n%s\n", manifest)
	}

I think we'll need to actually parse the YAML to pick out which ones are CRDs, though.

In the mean time I figured out how to work around this using a super gnarlyfor_each expression that will pick out only the CRDs from the manifests attribute.

resource "null_resource" "test" {
  for_each = toset(flatten([
    for k, v in data.helm_template.opa_gatekeeper.manifests :
      [for vvv in [for vv in split("---\n", v) : vv if vv != ""] : vvv
         if yamldecode(vvv).kind == "CustomResourceDefinition"]]))

  provisioner "local-exec" {
    command = "echo \"${each.value}\""
  }
}

[phillebaba]

@jrhouston so I have done some work and I think I can solve two issues in one PR. I stumbled across #782 and realized I could fix that at the same time. I read through most of the Helm code that does the manifest rendering and came to the conclusion that it treats templates and CRDs very differently. My solution was to basically skip having the client include the CRDs and instead two it myself, this is possible as the Helm Chart resource includes the list of the CRDs which can be easily be rendered in the same format as a Helm template output. I just need to get some tests done and I will be able to create the PR.

I agree with you that updating the key would have its benefits. Especially because changing the filename but not the content would still result in a state change in Terraform. Doing this with API group and kind however can also be tricky as they may also change, while the actual content does not. Resources may go from a v1beta1 to a v1beta2 or move to a different API group without changing the spec, which could force a new resource. We have had some issues with this in the Flux Terraform provider, and I know that the kubectl provider has some issues with this as it includes the api version in the key wen parsing a multidoc yaml file.

Creating a standard Kubernetes resource identifier for Terraform that does not have these problems would actually be a pretty helpful thing for a lot of Kubernetes related providers out there.

[MatthiasWinzeler]

@phillebaba by accident I stumbled upon this thread as I searched for ways to render a helm template with the helm provider and applying it with kubectl_manifest. Did you get the configuration suggested in the issue description to work?

I'm even struggling with applying the whole manifest, and I think applying only the CRDs might have the same issue:

resource "kubectl_manifest" "karpenter" {
  for_each = data.helm_template.karpenter.manifests
  yaml_body  = each.value
}

yields:

╷
│ Error: Invalid for_each argument
│
│   on ../../../eks-kubernetes/karpenter.tf line 77, in resource "kubectl_manifest" "helm_karpenter":
│   77:   for_each  = data.helm_template.karpenter.manifests
│     ├────────────────
│     │ data.helm_template.karpenter.manifests is a map of string, known only after apply
│
│ The "for_each" value depends on resource attributes that cannot be determined until apply, so Terraform cannot predict how many instances will be created. To work around this,
│ use the -target argument to first apply only the resources that the for_each depends on.
╵

[dhoeld]

I guess this is fixed by the PR #1050 which has been released yesterday in release v2.9.0.

I tested it using the example code below:

data "helm_template" "kube_prometheus_stack" {
  name       = "kube-prometheus-stack"
  namespace  = "kube-prometheus-stack"
  repository = "https://prometheus-community.github.io/helm-charts"

  chart   = "kube-prometheus-stack"
  version = "45.1.0"

  include_crds = true
}

locals {
  kube_prometheus_stack_crds_name_to_manifest_map = {
    for crd in data.helm_template.kube_prometheus_stack.crds :
    yamldecode(crd).metadata.name => crd
  }
}

resource "kubectl_manifest" "crds" {
  for_each  = local.kube_prometheus_stack_crds_name_to_manifest_map
  yaml_body = each.value

  force_conflicts = true
  # Fix 'metadata.annotations: Too long: must have at most 262144 bytes'
  server_side_apply = true
}

[github-actions]

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!