From 4871a92b8ca7ce042787d1cfcb31360b40b294a3 Mon Sep 17 00:00:00 2001 From: Seonghoi Lee Date: Tue, 2 Jan 2024 09:56:07 +0900 Subject: [PATCH] fix: prevent preparing chart for disabled releases (#1210) * fix: prevent preparing chart for disabled releases Previously, PrepareCharts does not filter any releases whose condition is disabled with no selectors. Prevent following things from - using any unnecessary computing resources for disabled charts - throwing any error from wrong configurations for disabled charts Signed-off-by: Seonghoi lee * fix: working for integration test about list and build Some tests require that PrepareCharts without any selector may not add any labels on the release. make markExcludedReleases do not add any label without any selectors Signed-off-by: Seonghoi lee * fix: prevent resolved chart version loss State loss the resolved chart version info from st.Releases when st.GetSelectedReleases() be called update st.Releases after st.GetSelectedReleases() in prepareCharts Signed-off-by: Seonghoi lee * fix: preserve resolved version from resolveDeps In PrepareCharts, the version, resolved from resolveDeps, is removed after invoking GetSelectedReleases. Do updateDeps at the first before GetSelectedReleases call Signed-off-by: Seonghoi lee --------- Signed-off-by: Seonghoi lee --- pkg/state/state.go | 46 ++++++++----------- .../happypath/input/environment.values.yaml | 2 + .../test-cases/happypath/input/happypath.yaml | 17 +++++++ 3 files changed, 38 insertions(+), 27 deletions(-) diff --git a/pkg/state/state.go b/pkg/state/state.go index adb5eabd..16f228c9 100644 --- a/pkg/state/state.go +++ b/pkg/state/state.go @@ -1123,21 +1123,6 @@ type PrepareChartKey struct { // // If exists, it will also patch resources by json patches, strategic-merge patches, and injectors. func (st *HelmState) PrepareCharts(helm helmexec.Interface, dir string, concurrency int, helmfileCommand string, opts ChartPrepareOptions) (map[PrepareChartKey]string, []error) { - var selected []ReleaseSpec - - if len(st.Selectors) > 0 { - var err error - - // This and releasesNeedCharts ensures that we run operations like helm-dep-build and prepare-hook calls only on - // releases that are (1) selected by the selectors and (2) to be installed. - selected, err = st.GetSelectedReleases(opts.IncludeTransitiveNeeds) - if err != nil { - return nil, []error{err} - } - } else { - selected = st.Releases - } - if !opts.SkipResolve { updated, err := st.ResolveDeps() if err != nil { @@ -1145,6 +1130,10 @@ func (st *HelmState) PrepareCharts(helm helmexec.Interface, dir string, concurre } *st = *updated } + selected, err := st.GetSelectedReleases(opts.IncludeTransitiveNeeds) + if err != nil { + return nil, []error{err} + } releases := releasesNeedCharts(selected) @@ -2204,18 +2193,21 @@ func markExcludedReleases(releases []ReleaseSpec, selectors []string, commonLabe filters = append(filters, f) } for _, r := range releases { - if r.Labels == nil { - r.Labels = map[string]string{} - } - // Let the release name, namespace, and chart be used as a tag - r.Labels["name"] = r.Name - r.Labels["namespace"] = r.Namespace - // Strip off just the last portion for the name stable/newrelic would give newrelic - chartSplit := strings.Split(r.Chart, "/") - r.Labels["chart"] = chartSplit[len(chartSplit)-1] - // Merge CommonLabels into release labels - for k, v := range commonLabels { - r.Labels[k] = v + // Do not add any label without any filter, see #276 + if len(filters) > 0 { + if r.Labels == nil { + r.Labels = map[string]string{} + } + // Let the release name, namespace, and chart be used as a tag + r.Labels["name"] = r.Name + r.Labels["namespace"] = r.Namespace + // Strip off just the last portion for the name stable/newrelic would give newrelic + chartSplit := strings.Split(r.Chart, "/") + r.Labels["chart"] = chartSplit[len(chartSplit)-1] + // Merge CommonLabels into release labels + for k, v := range commonLabels { + r.Labels[k] = v + } } var filterMatch bool for _, f := range filters { diff --git a/test/integration/test-cases/happypath/input/environment.values.yaml b/test/integration/test-cases/happypath/input/environment.values.yaml index 405e41b4..c65bc7d6 100644 --- a/test/integration/test-cases/happypath/input/environment.values.yaml +++ b/test/integration/test-cases/happypath/input/environment.values.yaml @@ -1,3 +1,5 @@ mysecret: MYSECRET raw2: enabled: false +release-disabled: + enabled: false diff --git a/test/integration/test-cases/happypath/input/happypath.yaml b/test/integration/test-cases/happypath/input/happypath.yaml index 06f88c48..9e475026 100644 --- a/test/integration/test-cases/happypath/input/happypath.yaml +++ b/test/integration/test-cases/happypath/input/happypath.yaml @@ -54,3 +54,20 @@ releases: values: - mysecret: {{ .Environment.Values.mysecret }} - values.yaml + + - name: release-disabled + chart: ../../../charts/helmx + namespace: release-disabled + condition: release-disabled.enabled + values: + - values-not-found.yaml + jsonPatches: + - target: + version: v1 + kind: ConfigMap + name: release-name + patch: + - op: add + path: /metadata/annotations + value: + foo: bar