From 4ad9db07cb0b866956443f89fec4340dded85f76 Mon Sep 17 00:00:00 2001 From: Justin Abrahms Date: Mon, 20 Jan 2025 22:57:46 -0800 Subject: [PATCH] Support enforcement that the transitive dependency chain is there. Signed-off-by: Justin Abrahms --- cmd/apply.go | 1 + cmd/diff.go | 1 + cmd/sync.go | 1 + cmd/template.go | 1 + pkg/app/app.go | 17 ++++++++++- pkg/app/config.go | 1 + pkg/config/apply.go | 7 +++++ pkg/config/diff.go | 7 +++++ pkg/config/lint.go | 7 +++++ pkg/config/sync.go | 7 +++++ pkg/config/template.go | 7 +++++ pkg/state/state.go | 28 +++++++++++++++++++ .../missing-needs/input/empty/Chart.yaml | 6 ++++ .../input/empty/templates/serviceaccount.yaml | 4 +++ .../missing-needs/input/empty/values.yaml | 0 .../missing-needs/input/helmfile.yaml | 8 ++++++ 16 files changed, 102 insertions(+), 1 deletion(-) create mode 100644 test/integration/test-cases/missing-needs/input/empty/Chart.yaml create mode 100644 test/integration/test-cases/missing-needs/input/empty/templates/serviceaccount.yaml create mode 100644 test/integration/test-cases/missing-needs/input/empty/values.yaml create mode 100644 test/integration/test-cases/missing-needs/input/helmfile.yaml diff --git a/cmd/apply.go b/cmd/apply.go index 9e6cfa47..856d8f46 100644 --- a/cmd/apply.go +++ b/cmd/apply.go @@ -54,6 +54,7 @@ func NewApplyCmd(globalCfg *config.GlobalImpl) *cobra.Command { f.BoolVar(&applyOptions.SkipCRDs, "skip-crds", false, "if set, no CRDs will be installed on sync. By default, CRDs are installed if not already present") f.BoolVar(&applyOptions.SkipNeeds, "skip-needs", true, `do not automatically include releases from the target release's "needs" when --selector/-l flag is provided. Does nothing when --selector/-l flag is not provided. Defaults to true when --include-needs or --include-transitive-needs is not provided`) f.BoolVar(&applyOptions.IncludeNeeds, "include-needs", false, `automatically include releases from the target release's "needs" when --selector/-l flag is provided. Does nothing when --selector/-l flag is not provided`) + f.BoolVar(&applyOptions.EnforceNeedsAreInstalled, "enforce-needs-are-installed", false, "enforce that all 'needs' dependencies are installable before applying changes") f.BoolVar(&applyOptions.IncludeTransitiveNeeds, "include-transitive-needs", false, `like --include-needs, but also includes transitive needs (needs of needs). Does nothing when --selector/-l flag is not provided. Overrides exclusions of other selectors and conditions.`) f.BoolVar(&applyOptions.SkipDiffOnInstall, "skip-diff-on-install", false, "Skips running helm-diff on releases being newly installed on this apply. Useful when the release manifests are too huge to be reviewed, or it's too time-consuming to diff at all") f.BoolVar(&applyOptions.IncludeTests, "include-tests", false, "enable the diffing of the helm test hooks") diff --git a/cmd/diff.go b/cmd/diff.go index 020bc6f1..0c2257ca 100644 --- a/cmd/diff.go +++ b/cmd/diff.go @@ -41,6 +41,7 @@ func NewDiffCmd(globalCfg *config.GlobalImpl) *cobra.Command { f.BoolVar(&diffOptions.IncludeTests, "include-tests", false, "enable the diffing of the helm test hooks") f.BoolVar(&diffOptions.IncludeNeeds, "include-needs", false, `automatically include releases from the target release's "needs" when --selector/-l flag is provided. Does nothing when --selector/-l flag is not provided`) f.BoolVar(&diffOptions.IncludeTransitiveNeeds, "include-transitive-needs", false, `like --include-needs, but also includes transitive needs (needs of needs). Does nothing when --selector/-l flag is not provided. Overrides exclusions of other selectors and conditions.`) + f.BoolVar(&diffOptions.EnforceNeedsAreInstalled, "enforce-needs-are-installed", false, "enforce that all 'needs' dependencies are installable before applying changes") f.BoolVar(&diffOptions.SkipDiffOnInstall, "skip-diff-on-install", false, "Skips running helm-diff on releases being newly installed on this apply. Useful when the release manifests are too huge to be reviewed, or it's too time-consuming to diff at all") f.BoolVar(&diffOptions.ShowSecrets, "show-secrets", false, "do not redact secret values in the output. should be used for debug purpose only") f.BoolVar(&diffOptions.NoHooks, "no-hooks", false, "do not diff changes made by hooks.") diff --git a/cmd/sync.go b/cmd/sync.go index 418bf32c..fbd6cda6 100644 --- a/cmd/sync.go +++ b/cmd/sync.go @@ -41,6 +41,7 @@ func NewSyncCmd(globalCfg *config.GlobalImpl) *cobra.Command { f.BoolVar(&syncOptions.SkipCRDs, "skip-crds", false, "if set, no CRDs will be installed on sync. By default, CRDs are installed if not already present") f.BoolVar(&syncOptions.IncludeNeeds, "include-needs", false, `automatically include releases from the target release's "needs" when --selector/-l flag is provided. Does nothing when --selector/-l flag is not provided`) f.BoolVar(&syncOptions.IncludeTransitiveNeeds, "include-transitive-needs", false, `like --include-needs, but also includes transitive needs (needs of needs). Does nothing when --selector/-l flag is not provided. Overrides exclusions of other selectors and conditions.`) + f.BoolVar(&syncOptions.EnforceNeedsAreInstalled, "enforce-needs-are-installed", false, "enforce that all 'needs' dependencies are installable before applying changes") f.BoolVar(&syncOptions.HideNotes, "hide-notes", false, "add --hide-notes flag to helm") f.BoolVar(&syncOptions.TakeOwnership, "take-ownership", false, `add --take-ownership flag to helm`) f.BoolVar(&syncOptions.Wait, "wait", false, `Override helmDefaults.wait setting "helm upgrade --install --wait"`) diff --git a/cmd/template.go b/cmd/template.go index 6cb81fea..8713f5b3 100644 --- a/cmd/template.go +++ b/cmd/template.go @@ -43,6 +43,7 @@ func NewTemplateCmd(globalCfg *config.GlobalImpl) *cobra.Command { f.BoolVar(&templateOptions.SkipNeeds, "skip-needs", true, `do not automatically include releases from the target release's "needs" when --selector/-l flag is provided. Does nothing when --selector/-l flag is not provided. Defaults to true when --include-needs or --include-transitive-needs is not provided`) f.BoolVar(&templateOptions.IncludeNeeds, "include-needs", false, `automatically include releases from the target release's "needs" when --selector/-l flag is provided. Does nothing when --selector/-l flag is not provided`) f.BoolVar(&templateOptions.IncludeTransitiveNeeds, "include-transitive-needs", false, `like --include-needs, but also includes transitive needs (needs of needs). Does nothing when --selector/-l flag is not provided. Overrides exclusions of other selectors and conditions.`) + f.BoolVar(&templateOptions.EnforceNeedsAreInstalled, "enforce-needs-are-installed", false, "enforce that all 'needs' dependencies are installable before applying changes") f.BoolVar(&templateOptions.SkipCleanup, "skip-cleanup", false, "Stop cleaning up temporary values generated by helmfile and helm-secrets. Useful for debugging. Don't use in production for security") f.BoolVar(&templateOptions.NoHooks, "no-hooks", false, "do not template files made by hooks.") f.StringVar(&templateOptions.PostRenderer, "post-renderer", "", `pass --post-renderer to "helm template" or "helm upgrade --install"`) diff --git a/pkg/app/app.go b/pkg/app/app.go index 91beba57..f07d5353 100644 --- a/pkg/app/app.go +++ b/pkg/app/app.go @@ -2015,7 +2015,13 @@ func (a *App) withNeeds(r *Run, c DAGConfig, includeDisabled bool, f func(*state includeNeeds = true } - batches, err := st.PlanReleases(state.PlanOptions{Reverse: false, SelectedReleases: selectedReleases, IncludeNeeds: includeNeeds, IncludeTransitiveNeeds: c.IncludeTransitiveNeeds(), SkipNeeds: c.SkipNeeds()}) + batches, err := st.PlanReleases(state.PlanOptions{ + Reverse: false, + SelectedReleases: selectedReleases, + IncludeNeeds: includeNeeds, + IncludeTransitiveNeeds: c.IncludeTransitiveNeeds(), + SkipNeeds: c.SkipNeeds(), + }) if err != nil { return false, []error{err} } @@ -2041,6 +2047,15 @@ func (a *App) withNeeds(r *Run, c DAGConfig, includeDisabled bool, f func(*state } } + if c.EnforceNeedsAreInstalled() { + for _, r := range toRender { + visited := make(map[string]bool) + if depErr := st.HasTransitiveDependencyWithInstalledFalse(r, visited); depErr != nil { + return false, []error{fmt.Errorf("Release %s has a transitive dependency %s marked as install=false", depErr.Release.Name, depErr.Dependency.Name)} + } + } + } + var rels []state.ReleaseSpec if len(toRender) > 0 { diff --git a/pkg/app/config.go b/pkg/app/config.go index b6e3a064..d0dddacb 100644 --- a/pkg/app/config.go +++ b/pkg/app/config.go @@ -261,6 +261,7 @@ type DAGConfig interface { SkipNeeds() bool IncludeNeeds() bool IncludeTransitiveNeeds() bool + EnforceNeedsAreInstalled() bool } type WriteValuesConfigProvider interface { diff --git a/pkg/config/apply.go b/pkg/config/apply.go index 4d67aa61..64f60294 100644 --- a/pkg/config/apply.go +++ b/pkg/config/apply.go @@ -32,6 +32,8 @@ type ApplyOptions struct { IncludeNeeds bool // IncludeTransitiveNeeds is true if the transitive needs should be included IncludeTransitiveNeeds bool + // EnforceNeedsAreInstalled is true we should error if/when there are unmeetable dependencies + EnforceNeedsAreInstalled bool // SkipDiffOnInstall is true if the diff should be skipped on install SkipDiffOnInstall bool // DiffArgs is the list of arguments to pass to the helm-diff. @@ -139,6 +141,11 @@ func (a *ApplyImpl) IncludeTransitiveNeeds() bool { return a.ApplyOptions.IncludeTransitiveNeeds } +// EnforceNeedsAreInstalled is true we should error if/when there are unmeetable dependencies +func (a *ApplyImpl) EnforceNeedsAreInstalled() bool { + return a.ApplyOptions.EnforceNeedsAreInstalled +} + // TODO: Remove this function once Helmfile v0.x // RetainValuesFiles returns the retain values files. func (a *ApplyImpl) RetainValuesFiles() bool { diff --git a/pkg/config/diff.go b/pkg/config/diff.go index c2717b89..3c6f1943 100644 --- a/pkg/config/diff.go +++ b/pkg/config/diff.go @@ -18,6 +18,8 @@ type DiffOptions struct { IncludeNeeds bool // IncludeTransitiveNeeds is the include transitive needs flag IncludeTransitiveNeeds bool + // EnforceNeedsAreInstalled is the enforce needs are installed flag + EnforceNeedsAreInstalled bool // SkipDiffOnInstall is the skip diff on install flag SkipDiffOnInstall bool // ShowSecrets is the show secrets flag @@ -85,6 +87,11 @@ func (t *DiffImpl) IncludeTransitiveNeeds() bool { return t.DiffOptions.IncludeTransitiveNeeds } +// EnforceNeedsAreInstalled errors if the transitive dependencies are not installable +func (t *DiffImpl) EnforceNeedsAreInstalled() bool { + return t.DiffOptions.EnforceNeedsAreInstalled +} + // Set returns the Set func (t *DiffImpl) Set() []string { return t.DiffOptions.Set diff --git a/pkg/config/lint.go b/pkg/config/lint.go index 46cf02f8..02bc8c3c 100644 --- a/pkg/config/lint.go +++ b/pkg/config/lint.go @@ -14,6 +14,8 @@ type LintOptions struct { IncludeNeeds bool // IncludeTransitiveNeeds is the include transitive needs flag IncludeTransitiveNeeds bool + // EnforceNeedsAreInstalled is the enforce needs are installed flag + EnforceNeedsAreInstalled bool // SkipDeps is the skip deps flag } @@ -74,3 +76,8 @@ func (l *LintImpl) SkipNeeds() bool { return false } + +// EnforceNeedsAreInstalled errors if the transitive dependencies are not installable +func (l *LintImpl) EnforceNeedsAreInstalled() bool { + return l.LintOptions.EnforceNeedsAreInstalled +} diff --git a/pkg/config/sync.go b/pkg/config/sync.go index c8927b72..f7d9b3ed 100644 --- a/pkg/config/sync.go +++ b/pkg/config/sync.go @@ -16,6 +16,8 @@ type SyncOptions struct { IncludeNeeds bool // IncludeTransitiveNeeds is the include transitive needs flag IncludeTransitiveNeeds bool + // EnforceNeedsAreInstalled is true we should error if/when there are unmeetable dependencies + EnforceNeedsAreInstalled bool // SkipCrds is the skip crds flag SkipCRDs bool // Wait is the wait flag @@ -160,3 +162,8 @@ func (t *SyncImpl) HideNotes() bool { func (t *SyncImpl) TakeOwnership() bool { return t.SyncOptions.TakeOwnership } + +// EnforceNeedsAreInstalled errors if the transitive dependencies are not installable +func (t *SyncImpl) EnforceNeedsAreInstalled() bool { + return t.SyncOptions.EnforceNeedsAreInstalled +} diff --git a/pkg/config/template.go b/pkg/config/template.go index 134cc6aa..2e8b74ea 100644 --- a/pkg/config/template.go +++ b/pkg/config/template.go @@ -30,6 +30,8 @@ type TemplateOptions struct { IncludeNeeds bool // IncludeTransitiveNeeds is the include transitive needs flag IncludeTransitiveNeeds bool + // EnforceNeedsAreInstalled is true we should error if/when there are unmeetable dependencies + EnforceNeedsAreInstalled bool // No-Hooks is the no hooks flag NoHooks bool // SkipCleanup is the skip cleanup flag @@ -158,3 +160,8 @@ func (t *TemplateImpl) KubeVersion() string { func (t *TemplateImpl) ShowOnly() []string { return t.TemplateOptions.ShowOnly } + +// EnforceNeedsAreInstalled errors if the transitive dependencies are not installable +func (t *TemplateImpl) EnforceNeedsAreInstalled() bool { + return t.TemplateOptions.EnforceNeedsAreInstalled +} diff --git a/pkg/state/state.go b/pkg/state/state.go index a28c05d9..ae3a441c 100644 --- a/pkg/state/state.go +++ b/pkg/state/state.go @@ -2557,6 +2557,34 @@ func (st *HelmState) UpdateDeps(helm helmexec.Interface, includeTransitiveNeeds return nil } +// DependencyError holds information about a release and its dependency that is not installed. +type DependencyError struct { + Release ReleaseSpec + Dependency ReleaseSpec +} + +// hasTransitiveDependencyWithInstallFalse checks if a release has a transitive dependency with Install set to false. +func (st *HelmState) HasTransitiveDependencyWithInstalledFalse(release ReleaseSpec, visited map[string]bool) *DependencyError { + if visited[release.Name] { + return nil + } + visited[release.Name] = true + + for _, dep := range release.Needs { + for _, r := range st.Releases { + if r.Name == dep { + if r.Installed != nil && !*r.Installed { + return &DependencyError{Release: release, Dependency: r} + } + if depErr := st.HasTransitiveDependencyWithInstalledFalse(r, visited); depErr != nil { + return depErr + } + } + } + } + return nil +} + // find "Chart.yaml" func findChartDirectory(topLevelDir string) (string, error) { var files []string diff --git a/test/integration/test-cases/missing-needs/input/empty/Chart.yaml b/test/integration/test-cases/missing-needs/input/empty/Chart.yaml new file mode 100644 index 00000000..8ec53206 --- /dev/null +++ b/test/integration/test-cases/missing-needs/input/empty/Chart.yaml @@ -0,0 +1,6 @@ +apiVersion: v2 +name: empty +description: A Helm chart for Kubernetes +type: application +version: 0.1.0 +appVersion: "1.16.0" diff --git a/test/integration/test-cases/missing-needs/input/empty/templates/serviceaccount.yaml b/test/integration/test-cases/missing-needs/input/empty/templates/serviceaccount.yaml new file mode 100644 index 00000000..1598b542 --- /dev/null +++ b/test/integration/test-cases/missing-needs/input/empty/templates/serviceaccount.yaml @@ -0,0 +1,4 @@ +apiVersion: v1 +kind: ServiceAccount +metadata: + name: test diff --git a/test/integration/test-cases/missing-needs/input/empty/values.yaml b/test/integration/test-cases/missing-needs/input/empty/values.yaml new file mode 100644 index 00000000..e69de29b diff --git a/test/integration/test-cases/missing-needs/input/helmfile.yaml b/test/integration/test-cases/missing-needs/input/helmfile.yaml new file mode 100644 index 00000000..0af46667 --- /dev/null +++ b/test/integration/test-cases/missing-needs/input/helmfile.yaml @@ -0,0 +1,8 @@ +releases: + - name: empty1 + installed: false + chart: ./empty + - name: empty2 + chart: ./empty + needs: + - empty1