Skip to content

Commit

Permalink
Fix chartify regression of missing chart dependencies (#1869)
Browse files Browse the repository at this point in the history 
* Fix chartify regression of missing chart dependencies

Fixes #1867

* Add integration test cases for issues #1857 and #1867
  • Loading branch information
@mumoshu
mumoshu authored Jun 8, 2021
1 parent 0d4adfe commit 72e7160
Show file tree
Hide file tree
Showing 5 changed files with 60 additions and 1 deletion.
2 changes: 1 addition & 1 deletion go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ require (
github.com/spf13/cobra v1.1.1
github.com/tatsushid/go-prettytable v0.0.0-20141013043238-ed2d14c29939
github.com/urfave/cli v1.22.5
github.com/variantdev/chartify v0.8.6
github.com/variantdev/chartify v0.8.9
github.com/variantdev/dag v1.0.0
github.com/variantdev/vals v0.14.0
go.uber.org/multierr v1.6.0
Expand Down
4 changes: 4 additions & 0 deletions go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -630,6 +630,10 @@ github.com/variantdev/chartify v0.8.5 h1:yAw2+IxftPDR5A2/vr97Y6DR1U/2lJCHxfp40DB
github.com/variantdev/chartify v0.8.5/go.mod h1:qF4XzQlkfH/6k2jAi1hLas+lK4zSCa8kY+r5JdmLA68=
github.com/variantdev/chartify v0.8.6 h1:8WIJ/79qHg4cobFhZsA7VDyvLp0+W3O9SI31LlDnyOM=
github.com/variantdev/chartify v0.8.6/go.mod h1:qF4XzQlkfH/6k2jAi1hLas+lK4zSCa8kY+r5JdmLA68=
github.com/variantdev/chartify v0.8.8 h1:dsQyEfoSexCOPmGTSHIWYUuGkArN53B+43qTMfE7xU0=
github.com/variantdev/chartify v0.8.8/go.mod h1:qF4XzQlkfH/6k2jAi1hLas+lK4zSCa8kY+r5JdmLA68=
github.com/variantdev/chartify v0.8.9 h1:kECyWario6UOShilDThKfuvk+FhXWjlFod/Cg/wTmVs=
github.com/variantdev/chartify v0.8.9/go.mod h1:qF4XzQlkfH/6k2jAi1hLas+lK4zSCa8kY+r5JdmLA68=
github.com/variantdev/dag v0.0.0-20191028002400-bb0b3c785363 h1:KrfQBEUn+wEOQ/6UIfoqRDvn+Q/wZridQ7t0G1vQqKE=
github.com/variantdev/dag v0.0.0-20191028002400-bb0b3c785363/go.mod h1:pH1TQsNSLj2uxMo9NNl9zdGy01Wtn+/2MT96BrKmVyE=
github.com/variantdev/dag v1.0.0 h1:7SFjATxHtrYV20P3tx53yNDBMegz6RT4jv8JPHqAHdM=
Expand Down
22 changes: 22 additions & 0 deletions test/integration/issue.1857.yaml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
repositories:
- name: prometheus-community
url: https://prometheus-community.github.io/helm-charts

releases:
- name: prometheus-operator-k8s-test
chart: prometheus-community/kube-prometheus-stack
namespace: monitoring
forceNamespace: monitoring
version: 15.4.6
labels:
component: test-upg
values:
- nameOverride: k8s
fullnameOverride: prometheus-k8s
global:
imagePullSecrets:
- name: regsecret
alertmanager:
enabled: false
grafana:
enabled: {{ .Values.grafanaEnabled }}
18 changes: 18 additions & 0 deletions test/integration/issue.1867.yaml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
repositories:
- name: bitnami
url: https://charts.bitnami.com/bitnami

releases:
- name: elasticsearch
chart: bitnami/elasticsearch
version: 15.2.2
jsonPatches:
- target:
version: v1
kind: StatefulSet
name: elasticsearch-master
patch:
- op: add
path: /spec/template/spec/containers/0/volumeMounts/0/subPathExpr
value: elasticsearch/$(MY_POD_NAME)

15 changes: 15 additions & 0 deletions test/integration/run.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,21 @@ info "Ensuring \"helmfile template\" output does contain only YAML docs"

test_pass "happypath"

test_start "regression tests"

if [[ helm_major_version -eq 3 ]]; then
info "https://github.com/roboll/helmfile/issues/1857"
(${helmfile} -f ${dir}/issue.1857.yaml --state-values-set grafanaEnabled=true template | grep grafana 1>/dev/null) || fail "\"helmfile template\" shouldn't include grafana"
! (${helmfile} -f ${dir}/issue.1857.yaml --state-values-set grafanaEnabled=false template | grep grafana) || fail "\"helmfile template\" shouldn't include grafana"

info "https://github.com/roboll/helmfile/issues/1867"
(${helmfile} -f ${dir}/issue.1867.yaml template 1>/dev/null) || fail "\"helmfile template\" shouldn't fail"
else
info "There are no regression tests for helm 2 because all the target charts have dropped helm 2 support."
fi

test_pass "regression tests"

if [[ helm_major_version -eq 3 ]]; then
export VAULT_ADDR=http://127.0.0.1:8200
export VAULT_TOKEN=toor
Expand Down

1 comment on commit 72e7160

@danksim
Copy link

@danksim danksim commented on 72e7160 Mar 6, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @mumoshu,

$ helmfile version
helmfile version v0.139.8

I am experiencing a similar problem to #1857.

I have a custom helm chart that uses bitnami/thanos helm chart as a dependency defined in its requirements.yaml file.

And the bitnami/thanos helm chart uses the bitnami/minio helm chart as its dependency defined in its Chart.yaml file (as seen here):

dependencies:
  - condition: minio.enabled
    name: minio
    repository: https://charts.bitnami.com/bitnami
    version: 7.x.x # note: this is defined in its Chart.lock file

Now my releases:

releases:
  - name: thanos
    chart: some-company/some-company-thanos
    version: {{ .Values.some_company.thanos.chart_version }}
    namespace: monitoring
    values:
      - ./values/some-company-thanos.yaml.gotmpl

And my ./values/some-company-thanos.yaml.gotmpl:

thanos:
  minio:
    enabled: false # hardcoded for sake of testing

The problem is that I can see that minio is set to false but I am still seeing minio resources when I run helmfile template|diff:

$ helmfile --debug --allow-no-matching-release --file monitoring.yaml -e dev diff --concurrency=4
...
envvals_loader: loaded ./dev.yaml.gotmpl:map[...minio:map[enabled:false]...]
...
monitoring, thanos-minio, Deployment (apps) has been added:
- 
+ # Source: some-company-thanos/charts/thanos/charts/minio/templates/standalone/deployment.yaml
...

Any help would be greatly appreciated. Thanks!

Edit:
Ah. It looks like requirements.yaml has been deprecated in favor of Chart.yaml -> dependencies. I will try that right now since that may interfere with value propagation.

Please sign in to comment.