akuity / kargo-render

Tool and library for managing rendered, environment-specific branches
Apache License 2.0
41 stars 17 forks source link

Error rendering kube-prometheus stack with includeCRDs #294

Open hoopty opened 2 months ago

hoopty commented 2 months ago

When using the below with kustomize + helm, I get the error:

time="2024-06-26T02:18:44Z" level=info msg="Starting Kargo Render Action" commit=2248e73dfa633d768d0ac528c8da16d153cfb6f6 version=v0.1.0-rc.40
time="2024-06-26T02:19:03Z" level=fatal msg="error writing manifests for app \"prometheus\" to \"/tmp/repo-293497742/repo/manifests/prometheus\": error unmarshaling resource: error converting YAML to JSON: yaml: line 5: mapping values are not allowed in this context"

As a workaround, if I disable the includeCRDs: true then kargo-render will at least run (but then it won't include up-to-date CRD's for ArgoCD). I have tried with no values.yaml overrides from chart defaults to verify that it is not my settings that are an issue.

Here's my helmChart.yaml:

kind: HelmChartInflationGenerator
metadata:
  name: prometheus
releaseName: prometheus
namespace: prometheus
repo: https://prometheus-community.github.io/helm-charts
name: kube-prometheus-stack
includeCRDs: true
valuesFile: ../../../addons/prometheus/base/values.yaml
additionalValuesFiles:
  - ./values.yaml
version: 60.4.0

Here's the relevant kargo-render.yaml snippet:

      prometheus:
        configManagement:
          path: clusters/${1}/prometheus
          kustomize:
            buildOptions: --enable-helm --load-restrictor LoadRestrictionsNone
        outputPath: manifests/prometheus
joaoestrela commented 1 month ago

Similar issue happens when using kyverno chart with versions >= 3.2.1

From my troubleshooting the difference on Kyverno chart is the usage of |- instead of a multiline string with quotes

Edit: Might also be related https://github.com/akuity/kargo-render/issues/278 since the changes introduce the ---\n

dan1el-k commented 1 month ago

Have the same experience with rendering certmanager chart as part of our Umbrella chart.

As a workaround just add combineManifests: true param. That solves the issue for me. With that all manifest are kept (no remove of CRDs required). The drawback is however, that you end up with one big file in the stage branch.

hoopty commented 3 weeks ago

Have the same experience with rendering certmanager chart as part of our Umbrella chart.

As a workaround just add combineManifests: true param. That solves the issue for me. With that all manifest are kept (no remove of CRDs required). The drawback is however, that you end up with one big file in the stage branch.

@dan1el-k kudos to you for finding that workaround, it works for me as well!