diff --git a/pkg/action/action.go b/pkg/action/action.go index 8c1888144..28e15107a 100644 --- a/pkg/action/action.go +++ b/pkg/action/action.go @@ -276,17 +276,24 @@ func splitAndDeannotate(postrendered, fallbackPrefix string) (map[string]string, // // This code has to do with writing files to disk. func (cfg *Configuration) renderResources(ch *chart.Chart, values common.Values, releaseName, outputDir string, subNotes, useReleaseName, includeCrds bool, pr postrenderer.PostRenderer, interactWithRemote, enableDNS, hideSecret bool, postRenderStrategy PostRenderStrategy) ([]*release.Hook, *bytes.Buffer, string, error) { + hs, b, notes, _, err := cfg.renderResourcesWithFiles(ch, values, releaseName, outputDir, subNotes, useReleaseName, includeCrds, pr, interactWithRemote, enableDNS, hideSecret, postRenderStrategy) + return hs, b, notes, err +} + +// renderResourcesWithFiles is the canonical render implementation, also returning the +// sorted manifest slice for sequencing-aware actions. +func (cfg *Configuration) renderResourcesWithFiles(ch *chart.Chart, values common.Values, releaseName, outputDir string, subNotes, useReleaseName, includeCrds bool, pr postrenderer.PostRenderer, interactWithRemote, enableDNS, hideSecret bool, postRenderStrategy PostRenderStrategy) ([]*release.Hook, *bytes.Buffer, string, []releaseutil.Manifest, error) { var hs []*release.Hook b := bytes.NewBuffer(nil) caps, err := cfg.getCapabilities() if err != nil { - return hs, b, "", err + return hs, b, "", nil, err } if ch.Metadata.KubeVersion != "" { if !chartutil.IsCompatibleRange(ch.Metadata.KubeVersion, caps.KubeVersion.String()) { - return hs, b, "", fmt.Errorf("chart requires kubeVersion: %s which is incompatible with Kubernetes %s", ch.Metadata.KubeVersion, caps.KubeVersion.Version) + return hs, b, "", nil, fmt.Errorf("chart requires kubeVersion: %s which is incompatible with Kubernetes %s", ch.Metadata.KubeVersion, caps.KubeVersion.Version) } } @@ -299,7 +306,7 @@ func (cfg *Configuration) renderResources(ch *chart.Chart, values common.Values, if interactWithRemote && cfg.RESTClientGetter != nil { restConfig, err := cfg.RESTClientGetter.ToRESTConfig() if err != nil { - return hs, b, "", err + return hs, b, "", nil, err } e := engine.New(restConfig) e.EnableDNS = enableDNS @@ -315,7 +322,7 @@ func (cfg *Configuration) renderResources(ch *chart.Chart, values common.Values, } if err2 != nil { - return hs, b, "", err2 + return hs, b, "", nil, err2 } // NOTES.txt gets rendered like all the other files, but because it's not a hook nor a resource, @@ -356,7 +363,7 @@ func (cfg *Configuration) renderResources(ch *chart.Chart, values common.Values, } fmt.Fprintf(b, "---\n# Source: %s\n%s\n", name, content) } - return hs, b, "", err + return hs, b, "", nil, err } // Build separate files maps for hooks and manifests. @@ -407,17 +414,17 @@ func (cfg *Configuration) renderResources(ch *chart.Chart, values common.Values, merged, err := annotateAndMerge(group.files) if err != nil { - return hs, b, notes, fmt.Errorf("error merging %s: %w", group.name, err) + return hs, b, notes, nil, fmt.Errorf("error merging %s: %w", group.name, err) } postRendered, err := pr.Run(bytes.NewBufferString(merged)) if err != nil { - return hs, b, notes, fmt.Errorf("error while running post render on %s: %w", group.name, err) + return hs, b, notes, nil, fmt.Errorf("error while running post render on %s: %w", group.name, err) } rendered, err := splitAndDeannotate(postRendered.String(), group.name) if err != nil { - return hs, b, notes, fmt.Errorf("error while parsing post rendered output for %s: %w", group.name, err) + return hs, b, notes, nil, fmt.Errorf("error while parsing post rendered output for %s: %w", group.name, err) } for k, v := range rendered { @@ -439,22 +446,22 @@ func (cfg *Configuration) renderResources(ch *chart.Chart, values common.Values, // Merge files as stream of documents for sending to post renderer merged, err := annotateAndMerge(files) if err != nil { - return hs, b, notes, fmt.Errorf("error merging manifests: %w", err) + return hs, b, notes, nil, fmt.Errorf("error merging manifests: %w", err) } // Run the post renderer postRendered, err := pr.Run(bytes.NewBufferString(merged)) if err != nil { - return hs, b, notes, fmt.Errorf("error while running post render on files: %w", err) + return hs, b, notes, nil, fmt.Errorf("error while running post render on files: %w", err) } // Use the file list and contents received from the post renderer files, err = splitAndDeannotate(postRendered.String(), "") if err != nil { - return hs, b, notes, fmt.Errorf("error while parsing post rendered output: %w", err) + return hs, b, notes, nil, fmt.Errorf("error while parsing post rendered output: %w", err) } default: - return hs, b, notes, fmt.Errorf("unknown post-render strategy: '%s'", postRenderStrategy) + return hs, b, notes, nil, fmt.Errorf("unknown post-render strategy: '%s'", postRenderStrategy) } } @@ -474,7 +481,7 @@ func (cfg *Configuration) renderResources(ch *chart.Chart, values common.Values, } fmt.Fprintf(b, "---\n# Source: %s\n%s\n", name, content) } - return hs, b, "", err + return hs, b, "", nil, err } // Aggregate all valid manifests into one big doc. @@ -487,7 +494,7 @@ func (cfg *Configuration) renderResources(ch *chart.Chart, values common.Values, } else { err = writeToFile(outputDir, crd.Filename, string(crd.File.Data[:]), fileWritten[crd.Filename]) if err != nil { - return hs, b, "", err + return hs, b, "", nil, err } fileWritten[crd.Filename] = true } @@ -512,13 +519,13 @@ func (cfg *Configuration) renderResources(ch *chart.Chart, values common.Values, // used by install or upgrade err = writeToFile(newDir, m.Name, m.Content, fileWritten[m.Name]) if err != nil { - return hs, b, "", err + return hs, b, "", nil, err } fileWritten[m.Name] = true } } - return hs, b, notes, nil + return hs, b, notes, manifests, nil } // RESTClientGetter gets the rest client diff --git a/pkg/action/backward_compat_test.go b/pkg/action/backward_compat_test.go new file mode 100644 index 000000000..a69459cdf --- /dev/null +++ b/pkg/action/backward_compat_test.go @@ -0,0 +1,125 @@ +/* +Copyright The Helm Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package action + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "helm.sh/helm/v4/pkg/chart/common" + "helm.sh/helm/v4/pkg/kube" + rcommon "helm.sh/helm/v4/pkg/release/common" + release "helm.sh/helm/v4/pkg/release/v1" + releaseutil "helm.sh/helm/v4/pkg/release/v1/util" +) + +func TestBackwardCompat_OrderedWaitWithoutAnnotations(t *testing.T) { + client := newRecordingKubeClient() + install := newSequencedInstallAction(t, client) + buf := captureWarningOutput(t) + + ch := buildChartWithTemplates([]*common.File{ + makeConfigMapTemplate("templates/first.yaml", "first", nil), + makeConfigMapTemplate("templates/second.yaml", "second", nil), + }, withName("backward-compat-no-annotations")) + + rel := mustRelease(t, mustRunInstall(t, install, ch)) + + require.Len(t, client.createCalls, 1) + assert.ElementsMatch(t, []string{"ConfigMap/first", "ConfigMap/second"}, client.createCalls[0]) + require.Len(t, client.waitCalls, 1) + assert.ElementsMatch(t, client.createCalls[0], client.waitCalls[0]) + assert.Empty(t, buf.String()) + assert.Equal(t, rcommon.StatusDeployed, rel.Info.Status) +} + +func TestBackwardCompat_RollbackWithoutSequencingInfo(t *testing.T) { + client := newRecordingKubeClient() + rollback := newRollbackAction(t, client) + + targetChart := buildChartWithTemplates([]*common.File{ + makeConfigMapTemplate("templates/database.yaml", "database", nil), + makeConfigMapTemplate("templates/app.yaml", "app", nil), + }, withName("backward-compat-rollback")) + seedRollbackRelease(t, rollback, "backward-compat-rollback", 1, rcommon.StatusSuperseded, targetChart, joinManifestDocs( + configMapManifest("database", nil), + configMapManifest("app", nil), + ), nil) + + currentChart := buildChartWithTemplates([]*common.File{ + makeConfigMapTemplate("templates/database.yaml", "database", map[string]string{ + releaseutil.AnnotationResourceGroup: "database", + }), + makeConfigMapTemplate("templates/app.yaml", "app", map[string]string{ + releaseutil.AnnotationResourceGroup: "app", + releaseutil.AnnotationDependsOnResourceGroups: `["database"]`, + }), + }, withName("backward-compat-rollback")) + seedRollbackRelease(t, rollback, "backward-compat-rollback", 2, rcommon.StatusDeployed, currentChart, joinManifestDocs( + configMapManifest("database", map[string]string{releaseutil.AnnotationResourceGroup: "database"}), + configMapManifest("app", map[string]string{ + releaseutil.AnnotationResourceGroup: "app", + releaseutil.AnnotationDependsOnResourceGroups: `["database"]`, + }), + ), &release.SequencingInfo{Enabled: true, Strategy: string(kube.OrderedWaitStrategy)}) + + rollback.Version = 1 + + var runErr error + assert.NotPanics(t, func() { + runErr = rollback.Run("backward-compat-rollback") + }) + require.NoError(t, runErr) + + require.Len(t, client.updateCalls, 1) + assert.ElementsMatch(t, []string{"ConfigMap/database", "ConfigMap/app"}, client.updateCalls[0].target) + require.Len(t, client.waitCalls, 1) + assert.ElementsMatch(t, []string{"ConfigMap/database", "ConfigMap/app"}, client.waitCalls[0]) + assert.Nil(t, latestRollbackRelease(t, rollback, "backward-compat-rollback").SequencingInfo) +} + +func TestBackwardCompat_MixedAnnotationsUnsequencedLast(t *testing.T) { + client := newRecordingKubeClient() + install := newSequencedInstallAction(t, client) + buf := captureWarningOutput(t) + + ch := buildChartWithTemplates([]*common.File{ + makeConfigMapTemplate("templates/database.yaml", "database", map[string]string{ + releaseutil.AnnotationResourceGroup: "database", + }), + makeConfigMapTemplate("templates/app.yaml", "app", map[string]string{ + releaseutil.AnnotationResourceGroup: "app", + releaseutil.AnnotationDependsOnResourceGroups: `["database"]`, + }), + makeConfigMapTemplate("templates/plain-one.yaml", "plain-one", nil), + makeConfigMapTemplate("templates/plain-two.yaml", "plain-two", nil), + }, withName("backward-compat-mixed")) + + rel := mustRelease(t, mustRunInstall(t, install, ch)) + + require.Len(t, client.createCalls, 3) + assert.Equal(t, []string{"ConfigMap/database"}, client.createCalls[0]) + assert.Equal(t, []string{"ConfigMap/app"}, client.createCalls[1]) + assert.ElementsMatch(t, []string{"ConfigMap/plain-one", "ConfigMap/plain-two"}, client.createCalls[2]) + require.Len(t, client.waitCalls, 3) + assert.Equal(t, client.createCalls[:2], client.waitCalls[:2]) + assert.ElementsMatch(t, client.createCalls[2], client.waitCalls[2]) + assert.Empty(t, buf.String()) + assert.Equal(t, rcommon.StatusDeployed, rel.Info.Status) +} diff --git a/pkg/action/install.go b/pkg/action/install.go index 580b8a0cb..b7282a9f0 100644 --- a/pkg/action/install.go +++ b/pkg/action/install.go @@ -100,6 +100,10 @@ type Install struct { Devel bool DependencyUpdate bool Timeout time.Duration + // ReadinessTimeout is the per-batch timeout when --wait=ordered is used. + // Each ordered batch waits at most this long for resources to become ready. + // Must not exceed Timeout. Defaults to 1 minute when zero. + ReadinessTimeout time.Duration Namespace string ReleaseName string GenerateName bool @@ -310,6 +314,10 @@ func (i *Install) RunWithContext(ctx context.Context, ch ci.Charter, vals map[st return nil, fmt.Errorf("release name check failed: %w", err) } + if i.ReadinessTimeout > 0 && i.Timeout > 0 && i.ReadinessTimeout > i.Timeout { + return nil, fmt.Errorf("--readiness-timeout (%s) must not exceed --timeout (%s)", i.ReadinessTimeout, i.Timeout) + } + if err := chartutil.ProcessDependencies(chrt, vals); err != nil { i.cfg.Logger().Error("chart dependencies processing failed", slog.Any("error", err)) return nil, fmt.Errorf("chart dependencies processing failed: %w", err) @@ -374,8 +382,13 @@ func (i *Install) RunWithContext(ctx context.Context, ch ci.Charter, vals map[st rel := i.createRelease(chrt, vals, i.Labels) + var sortedManifests []releaseutil.Manifest var manifestDoc *bytes.Buffer - rel.Hooks, manifestDoc, rel.Info.Notes, err = i.cfg.renderResources(chrt, valuesToRender, i.ReleaseName, i.OutputDir, i.SubNotes, i.UseReleaseName, i.IncludeCRDs, i.PostRenderer, interactWithServer(i.DryRunStrategy), i.EnableDNS, i.HideSecret, i.PostRenderStrategy) + if i.WaitStrategy == kube.OrderedWaitStrategy { + rel.Hooks, manifestDoc, rel.Info.Notes, sortedManifests, err = i.cfg.renderResourcesWithFiles(chrt, valuesToRender, i.ReleaseName, i.OutputDir, i.SubNotes, i.UseReleaseName, i.IncludeCRDs, i.PostRenderer, interactWithServer(i.DryRunStrategy), i.EnableDNS, i.HideSecret, i.PostRenderStrategy) + } else { + rel.Hooks, manifestDoc, rel.Info.Notes, err = i.cfg.renderResources(chrt, valuesToRender, i.ReleaseName, i.OutputDir, i.SubNotes, i.UseReleaseName, i.IncludeCRDs, i.PostRenderer, interactWithServer(i.DryRunStrategy), i.EnableDNS, i.HideSecret, i.PostRenderStrategy) + } // Even for errors, attach this if available if manifestDoc != nil { rel.Manifest = manifestDoc.String() @@ -469,7 +482,11 @@ func (i *Install) RunWithContext(ctx context.Context, ch ci.Charter, vals map[st return rel, err } - rel, err = i.performInstallCtx(ctx, rel, toBeAdopted, resources) + if i.WaitStrategy == kube.OrderedWaitStrategy { + rel, err = i.performSequencedInstallCtx(ctx, chrt, rel, sortedManifests) + } else { + rel, err = i.performInstallCtx(ctx, rel, toBeAdopted, resources) + } if err != nil { rel, err = i.failRelease(rel, err) } @@ -498,6 +515,89 @@ func (i *Install) performInstallCtx(ctx context.Context, rel *release.Release, t } } +// performSequencedInstallCtx runs performSequencedInstall in a goroutine and +// respects context cancellation, mirroring performInstallCtx. +func (i *Install) performSequencedInstallCtx(ctx context.Context, chrt *chart.Chart, rel *release.Release, manifests []releaseutil.Manifest) (*release.Release, error) { + type Msg struct { + r *release.Release + e error + } + + resultChan := make(chan Msg, 1) + + go func() { + i.goroutineCount.Add(1) + rel, err := i.performSequencedInstall(ctx, chrt, rel, manifests) + resultChan <- Msg{rel, err} + i.goroutineCount.Add(-1) + }() + + select { + case <-ctx.Done(): + return rel, ctx.Err() + case msg := <-resultChan: + return msg.r, msg.e + } +} + +// performSequencedInstall deploys chart resources in DAG-ordered batches when +// --wait=ordered is active. +func (i *Install) performSequencedInstall(ctx context.Context, chrt *chart.Chart, rel *release.Release, manifests []releaseutil.Manifest) (*release.Release, error) { + if !i.DisableHooks { + if err := i.cfg.execHook(rel, release.HookPreInstall, i.WaitStrategy, i.WaitOptions, i.Timeout, i.ServerSideApply); err != nil { + return rel, fmt.Errorf("failed pre-install: %w", err) + } + } + + readinessTimeout := i.ReadinessTimeout + if readinessTimeout <= 0 { + readinessTimeout = time.Minute + } + + rel.SequencingInfo = &release.SequencingInfo{ + Enabled: true, + Strategy: string(i.WaitStrategy), + } + + sd := &sequencedDeployment{ + cfg: i.cfg, + releaseName: rel.Name, + releaseNamespace: rel.Namespace, + disableOpenAPI: i.DisableOpenAPIValidation, + serverSideApply: i.ServerSideApply, + forceConflicts: i.ForceConflicts, + forceReplace: i.ForceReplace, + waitStrategy: i.WaitStrategy, + waitOptions: i.WaitOptions, + waitForJobs: i.WaitForJobs, + timeout: i.Timeout, + readinessTimeout: readinessTimeout, + deadline: computeDeadline(i.Timeout), + } + + if err := sd.deployChartLevel(ctx, chrt, manifests); err != nil { + return rel, err + } + + if !i.DisableHooks { + if err := i.cfg.execHook(rel, release.HookPostInstall, i.WaitStrategy, i.WaitOptions, i.Timeout, i.ServerSideApply); err != nil { + return rel, fmt.Errorf("failed post-install: %w", err) + } + } + + if len(i.Description) > 0 { + rel.SetStatus(rcommon.StatusDeployed, i.Description) + } else { + rel.SetStatus(rcommon.StatusDeployed, "Install complete") + } + + if err := i.recordRelease(rel); err != nil { + i.cfg.Logger().Error("failed to record the release", slog.Any("error", err)) + } + + return rel, nil +} + // getGoroutineCount return the number of running routines func (i *Install) getGoroutineCount() int32 { return i.goroutineCount.Load() @@ -512,6 +612,10 @@ func (i *Install) performInstall(rel *release.Release, toBeAdopted kube.Resource } } + if err := stripSequencingAnnotations(resources); err != nil { + return rel, fmt.Errorf("stripping sequencing annotations: %w", err) + } + // At this point, we can do the install. Note that before we were detecting whether to // do an update, but it's not clear whether we WANT to do an update if the reuse is set // to true, since that is basically an upgrade operation. diff --git a/pkg/action/rollback.go b/pkg/action/rollback.go index 459569781..37124de3e 100644 --- a/pkg/action/rollback.go +++ b/pkg/action/rollback.go @@ -18,8 +18,10 @@ package action import ( "bytes" + "context" "errors" "fmt" + "log/slog" "time" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -28,6 +30,7 @@ import ( "helm.sh/helm/v4/pkg/kube" "helm.sh/helm/v4/pkg/release/common" release "helm.sh/helm/v4/pkg/release/v1" + releaseutil "helm.sh/helm/v4/pkg/release/v1/util" "helm.sh/helm/v4/pkg/storage/driver" ) @@ -37,12 +40,13 @@ import ( type Rollback struct { cfg *Configuration - Version int - Timeout time.Duration - WaitStrategy kube.WaitStrategy - WaitOptions []kube.WaitOption - WaitForJobs bool - DisableHooks bool + Version int + Timeout time.Duration + ReadinessTimeout time.Duration + WaitStrategy kube.WaitStrategy + WaitOptions []kube.WaitOption + WaitForJobs bool + DisableHooks bool // DryRunStrategy can be set to prepare, but not execute the operation and whether or not to interact with the remote cluster DryRunStrategy DryRunStrategy // ForceReplace will, if set to `true`, ignore certain warnings and perform the rollback anyway. @@ -76,6 +80,10 @@ func (r *Rollback) Run(name string) error { return err } + if r.ReadinessTimeout > 0 && r.Timeout > 0 && r.ReadinessTimeout > r.Timeout { + return fmt.Errorf("--readiness-timeout (%s) must not exceed --timeout (%s)", r.ReadinessTimeout, r.Timeout) + } + r.cfg.Releases.MaxHistory = r.MaxHistory r.cfg.Logger().Debug("preparing rollback", "name", name) @@ -92,7 +100,7 @@ func (r *Rollback) Run(name string) error { } r.cfg.Logger().Debug("performing rollback", "name", name) - if _, err := r.performRollback(currentRelease, targetRelease, serverSideApply); err != nil { + if _, err := r.performRollback(context.Background(), currentRelease, targetRelease, serverSideApply); err != nil { return err } @@ -184,22 +192,27 @@ func (r *Rollback) prepareRollback(name string) (*release.Release, *release.Rele // message here, and only override it later if we experience failure. Description: fmt.Sprintf("Rollback to %d", previousVersion), }, - Version: currentRelease.Version + 1, - Labels: previousRelease.Labels, - Manifest: previousRelease.Manifest, - Hooks: previousRelease.Hooks, - ApplyMethod: string(determineReleaseSSApplyMethod(serverSideApply)), + Version: currentRelease.Version + 1, + Labels: previousRelease.Labels, + Manifest: previousRelease.Manifest, + Hooks: previousRelease.Hooks, + ApplyMethod: string(determineReleaseSSApplyMethod(serverSideApply)), + SequencingInfo: previousRelease.SequencingInfo, } return currentRelease, targetRelease, serverSideApply, nil } -func (r *Rollback) performRollback(currentRelease, targetRelease *release.Release, serverSideApply bool) (*release.Release, error) { +func (r *Rollback) performRollback(ctx context.Context, currentRelease, targetRelease *release.Release, serverSideApply bool) (*release.Release, error) { if isDryRun(r.DryRunStrategy) { r.cfg.Logger().Debug("dry run", "name", targetRelease.Name) return targetRelease, nil } + if targetRelease.SequencingInfo != nil && targetRelease.SequencingInfo.Enabled { + return r.performSequencedRollback(ctx, currentRelease, targetRelease, serverSideApply) + } + current, err := r.cfg.KubeClient.Build(bytes.NewBufferString(currentRelease.Manifest), false) if err != nil { return targetRelease, fmt.Errorf("unable to build kubernetes objects from current release manifest: %w", err) @@ -304,3 +317,142 @@ func (r *Rollback) performRollback(currentRelease, targetRelease *release.Releas return targetRelease, nil } + +func (r *Rollback) failRollback(currentRelease, targetRelease *release.Release, created kube.ResourceList, err error) (*release.Release, error) { + msg := fmt.Sprintf("Rollback %q failed: %s", targetRelease.Name, err) + r.cfg.Logger().Warn(msg) + + currentRelease.Info.Status = common.StatusSuperseded + targetRelease.Info.Status = common.StatusFailed + targetRelease.Info.Description = msg + r.cfg.recordRelease(currentRelease) + r.cfg.recordRelease(targetRelease) + + if r.CleanupOnFail && len(created) > 0 { + r.cfg.Logger().Debug("cleanup on fail set, cleaning up resources", "count", len(created)) + _, errs := r.cfg.KubeClient.Delete(created, metav1.DeletePropagationBackground) + if errs != nil { + return targetRelease, fmt.Errorf( + "an error occurred while cleaning up resources after rollback failure: %v: %w", + err, + joinErrors(errs, ", "), + ) + } + r.cfg.Logger().Debug("resource cleanup complete") + } + + return targetRelease, err +} + +// performSequencedRollback deploys the target revision's resources in DAG-ordered batches +// when the target revision was originally deployed with --wait=ordered. +func (r *Rollback) performSequencedRollback(ctx context.Context, currentRelease, targetRelease *release.Release, serverSideApply bool) (*release.Release, error) { + fail := func(created kube.ResourceList, err error) (*release.Release, error) { + return r.failRollback(currentRelease, targetRelease, created, err) + } + + current, err := r.cfg.KubeClient.Build(bytes.NewBufferString(currentRelease.Manifest), false) + if err != nil { + return fail(nil, fmt.Errorf("unable to build kubernetes objects from current release manifest: %w", err)) + } + + target, err := r.cfg.KubeClient.Build(bytes.NewBufferString(targetRelease.Manifest), false) + if err != nil { + return fail(nil, fmt.Errorf("unable to build kubernetes objects from target release manifest: %w", err)) + } + + rawManifests := releaseutil.SplitManifests(targetRelease.Manifest) + _, sortedManifests, err := releaseutil.SortManifests(rawManifests, nil, releaseutil.InstallOrder) + if err != nil { + return fail(nil, fmt.Errorf("parsing target release manifest for sequenced rollback: %w", err)) + } + + sortedManifests = r.recoverManifestPathsForSequencedRollback(targetRelease, sortedManifests) + + if !r.DisableHooks { + if err := r.cfg.execHook(targetRelease, release.HookPreRollback, r.WaitStrategy, r.WaitOptions, r.Timeout, serverSideApply); err != nil { + return fail(nil, err) + } + } else { + r.cfg.Logger().Debug("rollback hooks disabled", "name", targetRelease.Name) + } + + readinessTimeout := r.ReadinessTimeout + if readinessTimeout <= 0 { + readinessTimeout = time.Minute + } + + sd := &sequencedDeployment{ + cfg: r.cfg, + releaseName: targetRelease.Name, + releaseNamespace: targetRelease.Namespace, + serverSideApply: serverSideApply, + forceConflicts: r.ForceConflicts, + forceReplace: r.ForceReplace, + waitStrategy: r.WaitStrategy, + waitOptions: r.WaitOptions, + waitForJobs: r.WaitForJobs, + timeout: r.Timeout, + readinessTimeout: readinessTimeout, + deadline: computeDeadline(r.Timeout), + upgradeMode: true, + currentResources: current, + upgradeCSAFieldManager: true, + } + + if err := sd.deployChartLevel(ctx, targetRelease.Chart, sortedManifests); err != nil { + return fail(sd.createdResources, err) + } + + allTargetKeys := make(map[string]bool, len(target)) + for _, resource := range target { + allTargetKeys[objectKey(resource)] = true + } + + var toBeDeleted kube.ResourceList + for _, resource := range current { + if !allTargetKeys[objectKey(resource)] { + toBeDeleted = append(toBeDeleted, resource) + } + } + + if len(toBeDeleted) > 0 { + if _, errs := r.cfg.KubeClient.Delete(toBeDeleted, metav1.DeletePropagationBackground); errs != nil { + return fail(sd.createdResources, fmt.Errorf("deleting removed resources during rollback: %w", joinErrors(errs, ", "))) + } + } + + if !r.DisableHooks { + if err := r.cfg.execHook(targetRelease, release.HookPostRollback, r.WaitStrategy, r.WaitOptions, r.Timeout, serverSideApply); err != nil { + return fail(sd.createdResources, err) + } + } + + deployed, err := r.cfg.Releases.DeployedAll(currentRelease.Name) + if err != nil && !errors.Is(err, driver.ErrNoDeployedReleases) { + return fail(sd.createdResources, err) + } + + for _, reli := range deployed { + rel, err := releaserToV1Release(reli) + if err != nil { + return fail(sd.createdResources, err) + } + r.cfg.Logger().Debug("superseding previous deployment", "version", rel.Version) + rel.Info.Status = common.StatusSuperseded + r.cfg.recordRelease(rel) + } + + targetRelease.Info.Status = common.StatusDeployed + + return targetRelease, nil +} + +func (r *Rollback) recoverManifestPathsForSequencedRollback(rel *release.Release, manifests []releaseutil.Manifest) []releaseutil.Manifest { + rendered, err := renderReleaseManifestsWithPaths(r.cfg, rel) + if err != nil { + r.cfg.Logger().Warn("unable to recover rendered chart paths for sequenced rollback; falling back to stored manifest order", slog.Any("error", err)) + return manifests + } + return applyRenderedManifestPaths(manifests, rendered) +} diff --git a/pkg/action/rollback_sequenced_test.go b/pkg/action/rollback_sequenced_test.go new file mode 100644 index 000000000..60e2c849c --- /dev/null +++ b/pkg/action/rollback_sequenced_test.go @@ -0,0 +1,392 @@ +/* +Copyright The Helm Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package action + +import ( + "errors" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "helm.sh/helm/v4/pkg/chart/common" + chart "helm.sh/helm/v4/pkg/chart/v2" + "helm.sh/helm/v4/pkg/kube" + rcommon "helm.sh/helm/v4/pkg/release/common" + release "helm.sh/helm/v4/pkg/release/v1" + releaseutil "helm.sh/helm/v4/pkg/release/v1/util" +) + +func newRollbackAction(t *testing.T, kubeClient kube.Interface) *Rollback { + t.Helper() + + cfg := actionConfigFixture(t) + cfg.KubeClient = kubeClient + + rollback := NewRollback(cfg) + rollback.Timeout = 5 * time.Minute + rollback.WaitStrategy = kube.OrderedWaitStrategy + rollback.ServerSideApply = "auto" + + return rollback +} + +func seedRollbackRelease(t *testing.T, rollback *Rollback, name string, version int, status rcommon.Status, ch *chart.Chart, manifest string, sequencingInfo *release.SequencingInfo) { + t.Helper() + + now := time.Now() + rel := &release.Release{ + Name: name, + Namespace: "spaced", + Chart: ch, + Config: map[string]any{}, + Manifest: manifest, + Info: &release.Info{ + FirstDeployed: now, + LastDeployed: now, + Status: status, + Description: "Seeded release", + }, + Version: version, + ApplyMethod: "csa", + SequencingInfo: sequencingInfo, + } + + require.NoError(t, rollback.cfg.Releases.Create(rel)) +} + +func storedRollbackRelease(t *testing.T, rollback *Rollback, name string, version int) *release.Release { + t.Helper() + + reli, err := rollback.cfg.Releases.Get(name, version) + require.NoError(t, err) + + rel, err := releaserToV1Release(reli) + require.NoError(t, err) + + return rel +} + +func latestRollbackRelease(t *testing.T, rollback *Rollback, name string) *release.Release { + t.Helper() + + reli, err := rollback.cfg.Releases.Last(name) + require.NoError(t, err) + + rel, err := releaserToV1Release(reli) + require.NoError(t, err) + + return rel +} + +func TestRollback_Sequenced_Basic(t *testing.T) { + client := newRecordingKubeClient() + rollback := newRollbackAction(t, client) + + targetChart := buildChartWithTemplates([]*common.File{ + makeConfigMapTemplate("templates/database.yaml", "database", map[string]string{ + releaseutil.AnnotationResourceGroup: "database", + }), + makeConfigMapTemplate("templates/app.yaml", "app", map[string]string{ + releaseutil.AnnotationResourceGroup: "app", + releaseutil.AnnotationDependsOnResourceGroups: `["database"]`, + }), + }, withName("rollback-basic")) + + sequencingInfo := &release.SequencingInfo{Enabled: true, Strategy: string(kube.OrderedWaitStrategy)} + seedRollbackRelease(t, rollback, "rollback-basic", 1, rcommon.StatusSuperseded, targetChart, joinManifestDocs( + configMapManifest("database", map[string]string{releaseutil.AnnotationResourceGroup: "database"}), + configMapManifest("app", map[string]string{ + releaseutil.AnnotationResourceGroup: "app", + releaseutil.AnnotationDependsOnResourceGroups: `["database"]`, + }), + ), sequencingInfo) + + currentChart := buildChartWithTemplates([]*common.File{ + makeConfigMapTemplate("templates/database.yaml", "database", nil), + makeConfigMapTemplate("templates/app.yaml", "app", nil), + }, withName("rollback-basic")) + seedRollbackRelease(t, rollback, "rollback-basic", 2, rcommon.StatusDeployed, currentChart, joinManifestDocs( + configMapManifest("database", nil), + configMapManifest("app", nil), + ), nil) + + rollback.Version = 1 + require.NoError(t, rollback.Run("rollback-basic")) + + require.Len(t, client.updateCalls, 2) + assert.Equal(t, [][]string{{"ConfigMap/database"}, {"ConfigMap/app"}}, updateTargets(client.updateCalls)) + assert.Equal(t, [][]string{{"ConfigMap/database"}, {"ConfigMap/app"}}, client.waitCalls) + assert.Equal(t, []string{"ConfigMap/database"}, client.updateCalls[0].current) + assert.Equal(t, []string{"ConfigMap/app"}, client.updateCalls[1].current) + + rel := latestRollbackRelease(t, rollback, "rollback-basic") + assert.Equal(t, 3, rel.Version) + assert.Equal(t, rcommon.StatusDeployed, rel.Info.Status) + require.NotNil(t, rel.SequencingInfo) + assert.Equal(t, *sequencingInfo, *rel.SequencingInfo) +} + +func TestRollback_ToNonSequenced(t *testing.T) { + client := newRecordingKubeClient() + rollback := newRollbackAction(t, client) + + targetChart := buildChartWithTemplates([]*common.File{ + makeConfigMapTemplate("templates/database.yaml", "database", map[string]string{ + releaseutil.AnnotationResourceGroup: "database", + }), + makeConfigMapTemplate("templates/app.yaml", "app", map[string]string{ + releaseutil.AnnotationResourceGroup: "app", + releaseutil.AnnotationDependsOnResourceGroups: `["database"]`, + }), + }, withName("rollback-to-standard")) + seedRollbackRelease(t, rollback, "rollback-to-standard", 1, rcommon.StatusSuperseded, targetChart, joinManifestDocs( + configMapManifest("database", map[string]string{releaseutil.AnnotationResourceGroup: "database"}), + configMapManifest("app", map[string]string{ + releaseutil.AnnotationResourceGroup: "app", + releaseutil.AnnotationDependsOnResourceGroups: `["database"]`, + }), + ), nil) + + currentChart := buildChartWithTemplates([]*common.File{ + makeConfigMapTemplate("templates/database.yaml", "database", nil), + makeConfigMapTemplate("templates/app.yaml", "app", nil), + }, withName("rollback-to-standard")) + seedRollbackRelease(t, rollback, "rollback-to-standard", 2, rcommon.StatusDeployed, currentChart, joinManifestDocs( + configMapManifest("database", nil), + configMapManifest("app", nil), + ), nil) + + rollback.Version = 1 + require.NoError(t, rollback.Run("rollback-to-standard")) + + require.Len(t, client.updateCalls, 1) + assert.ElementsMatch(t, []string{"ConfigMap/database", "ConfigMap/app"}, client.updateCalls[0].target) + require.Len(t, client.waitCalls, 1) + assert.ElementsMatch(t, []string{"ConfigMap/database", "ConfigMap/app"}, client.waitCalls[0]) + assert.Nil(t, latestRollbackRelease(t, rollback, "rollback-to-standard").SequencingInfo) +} + +func TestRollback_FromSequencedToNonSequenced(t *testing.T) { + client := newRecordingKubeClient() + rollback := newRollbackAction(t, client) + + targetChart := buildChartWithTemplates([]*common.File{ + makeConfigMapTemplate("templates/database.yaml", "database", nil), + makeConfigMapTemplate("templates/app.yaml", "app", nil), + }, withName("rollback-transition")) + seedRollbackRelease(t, rollback, "rollback-transition", 1, rcommon.StatusSuperseded, targetChart, joinManifestDocs( + configMapManifest("database", nil), + configMapManifest("app", nil), + ), nil) + + currentChart := buildChartWithTemplates([]*common.File{ + makeConfigMapTemplate("templates/database.yaml", "database", map[string]string{ + releaseutil.AnnotationResourceGroup: "database", + }), + makeConfigMapTemplate("templates/app.yaml", "app", map[string]string{ + releaseutil.AnnotationResourceGroup: "app", + releaseutil.AnnotationDependsOnResourceGroups: `["database"]`, + }), + }, withName("rollback-transition")) + seedRollbackRelease(t, rollback, "rollback-transition", 2, rcommon.StatusDeployed, currentChart, joinManifestDocs( + configMapManifest("database", map[string]string{releaseutil.AnnotationResourceGroup: "database"}), + configMapManifest("app", map[string]string{ + releaseutil.AnnotationResourceGroup: "app", + releaseutil.AnnotationDependsOnResourceGroups: `["database"]`, + }), + ), &release.SequencingInfo{Enabled: true, Strategy: string(kube.OrderedWaitStrategy)}) + + rollback.Version = 1 + require.NoError(t, rollback.Run("rollback-transition")) + + require.Len(t, client.updateCalls, 1) + assert.ElementsMatch(t, []string{"ConfigMap/database", "ConfigMap/app"}, client.updateCalls[0].target) + assert.Nil(t, latestRollbackRelease(t, rollback, "rollback-transition").SequencingInfo) +} + +func TestRollback_SequencingInfoPreserved(t *testing.T) { + client := newRecordingKubeClient() + rollback := newRollbackAction(t, client) + + targetChart := buildChartWithTemplates([]*common.File{ + makeConfigMapTemplate("templates/database.yaml", "database", map[string]string{ + releaseutil.AnnotationResourceGroup: "database", + }), + }, withName("rollback-sequencing-info")) + sequencingInfo := &release.SequencingInfo{Enabled: true, Strategy: string(kube.OrderedWaitStrategy)} + seedRollbackRelease(t, rollback, "rollback-sequencing-info", 1, rcommon.StatusSuperseded, targetChart, configMapManifest("database", map[string]string{ + releaseutil.AnnotationResourceGroup: "database", + }), sequencingInfo) + + currentChart := buildChartWithTemplates([]*common.File{ + makeConfigMapTemplate("templates/database.yaml", "database", nil), + }, withName("rollback-sequencing-info")) + seedRollbackRelease(t, rollback, "rollback-sequencing-info", 2, rcommon.StatusDeployed, currentChart, configMapManifest("database", nil), nil) + + rollback.Version = 1 + require.NoError(t, rollback.Run("rollback-sequencing-info")) + + rel := latestRollbackRelease(t, rollback, "rollback-sequencing-info") + require.NotNil(t, rel.SequencingInfo) + assert.Equal(t, *sequencingInfo, *rel.SequencingInfo) +} + +func TestRollback_Sequenced_Failure(t *testing.T) { + client := newRecordingKubeClient() + client.waitErrorOnCall = 2 + client.waitError = errors.New("timed out waiting for batch") + + rollback := newRollbackAction(t, client) + + targetChart := buildChartWithTemplates([]*common.File{ + makeConfigMapTemplate("templates/database.yaml", "database", map[string]string{ + releaseutil.AnnotationResourceGroup: "database", + }), + makeConfigMapTemplate("templates/queue.yaml", "queue", map[string]string{ + releaseutil.AnnotationResourceGroup: "queue", + releaseutil.AnnotationDependsOnResourceGroups: `["database"]`, + }), + makeConfigMapTemplate("templates/app.yaml", "app", map[string]string{ + releaseutil.AnnotationResourceGroup: "app", + releaseutil.AnnotationDependsOnResourceGroups: `["queue"]`, + }), + }, withName("rollback-failure")) + seedRollbackRelease(t, rollback, "rollback-failure", 1, rcommon.StatusSuperseded, targetChart, joinManifestDocs( + configMapManifest("database", map[string]string{releaseutil.AnnotationResourceGroup: "database"}), + configMapManifest("queue", map[string]string{ + releaseutil.AnnotationResourceGroup: "queue", + releaseutil.AnnotationDependsOnResourceGroups: `["database"]`, + }), + configMapManifest("app", map[string]string{ + releaseutil.AnnotationResourceGroup: "app", + releaseutil.AnnotationDependsOnResourceGroups: `["queue"]`, + }), + ), &release.SequencingInfo{Enabled: true, Strategy: string(kube.OrderedWaitStrategy)}) + + currentChart := buildChartWithTemplates([]*common.File{ + makeConfigMapTemplate("templates/database.yaml", "database", nil), + makeConfigMapTemplate("templates/app.yaml", "app", nil), + }, withName("rollback-failure")) + seedRollbackRelease(t, rollback, "rollback-failure", 2, rcommon.StatusDeployed, currentChart, joinManifestDocs( + configMapManifest("database", nil), + configMapManifest("app", nil), + ), nil) + + rollback.Version = 1 + err := rollback.Run("rollback-failure") + require.Error(t, err) + assert.Contains(t, err.Error(), "timed out waiting for batch") + + require.Len(t, client.updateCalls, 2) + assert.Equal(t, [][]string{{"ConfigMap/database"}, {"ConfigMap/queue"}}, updateTargets(client.updateCalls)) + assert.Empty(t, client.deleteCalls) + + assert.Equal(t, rcommon.StatusFailed, latestRollbackRelease(t, rollback, "rollback-failure").Info.Status) + assert.Equal(t, rcommon.StatusSuperseded, storedRollbackRelease(t, rollback, "rollback-failure", 2).Info.Status) +} + +func TestRollback_Sequenced_CleanupOnFail(t *testing.T) { + client := newRecordingKubeClient() + client.waitErrorOnCall = 2 + client.waitError = errors.New("timed out waiting for batch") + + rollback := newRollbackAction(t, client) + rollback.CleanupOnFail = true + + targetChart := buildChartWithTemplates([]*common.File{ + makeConfigMapTemplate("templates/database.yaml", "database", map[string]string{ + releaseutil.AnnotationResourceGroup: "database", + }), + makeConfigMapTemplate("templates/queue.yaml", "queue", map[string]string{ + releaseutil.AnnotationResourceGroup: "queue", + releaseutil.AnnotationDependsOnResourceGroups: `["database"]`, + }), + makeConfigMapTemplate("templates/app.yaml", "app", map[string]string{ + releaseutil.AnnotationResourceGroup: "app", + releaseutil.AnnotationDependsOnResourceGroups: `["queue"]`, + }), + }, withName("rollback-cleanup")) + seedRollbackRelease(t, rollback, "rollback-cleanup", 1, rcommon.StatusSuperseded, targetChart, joinManifestDocs( + configMapManifest("database", map[string]string{releaseutil.AnnotationResourceGroup: "database"}), + configMapManifest("queue", map[string]string{ + releaseutil.AnnotationResourceGroup: "queue", + releaseutil.AnnotationDependsOnResourceGroups: `["database"]`, + }), + configMapManifest("app", map[string]string{ + releaseutil.AnnotationResourceGroup: "app", + releaseutil.AnnotationDependsOnResourceGroups: `["queue"]`, + }), + ), &release.SequencingInfo{Enabled: true, Strategy: string(kube.OrderedWaitStrategy)}) + + currentChart := buildChartWithTemplates([]*common.File{ + makeConfigMapTemplate("templates/database.yaml", "database", nil), + makeConfigMapTemplate("templates/app.yaml", "app", nil), + }, withName("rollback-cleanup")) + seedRollbackRelease(t, rollback, "rollback-cleanup", 2, rcommon.StatusDeployed, currentChart, joinManifestDocs( + configMapManifest("database", nil), + configMapManifest("app", nil), + ), nil) + + rollback.Version = 1 + err := rollback.Run("rollback-cleanup") + require.Error(t, err) + assert.Contains(t, err.Error(), "timed out waiting for batch") + assert.Equal(t, [][]string{{"ConfigMap/queue"}}, client.deleteCalls) + assert.Equal(t, rcommon.StatusFailed, latestRollbackRelease(t, rollback, "rollback-cleanup").Info.Status) +} + +func TestRollback_Sequenced_DryRun(t *testing.T) { + client := newRecordingKubeClient() + rollback := newRollbackAction(t, client) + rollback.DryRunStrategy = DryRunClient + + targetChart := buildChartWithTemplates([]*common.File{ + makeConfigMapTemplate("templates/database.yaml", "database", map[string]string{ + releaseutil.AnnotationResourceGroup: "database", + }), + makeConfigMapTemplate("templates/app.yaml", "app", map[string]string{ + releaseutil.AnnotationResourceGroup: "app", + releaseutil.AnnotationDependsOnResourceGroups: `["database"]`, + }), + }, withName("rollback-dry-run")) + seedRollbackRelease(t, rollback, "rollback-dry-run", 1, rcommon.StatusSuperseded, targetChart, joinManifestDocs( + configMapManifest("database", map[string]string{releaseutil.AnnotationResourceGroup: "database"}), + configMapManifest("app", map[string]string{ + releaseutil.AnnotationResourceGroup: "app", + releaseutil.AnnotationDependsOnResourceGroups: `["database"]`, + }), + ), &release.SequencingInfo{Enabled: true, Strategy: string(kube.OrderedWaitStrategy)}) + + currentChart := buildChartWithTemplates([]*common.File{ + makeConfigMapTemplate("templates/database.yaml", "database", nil), + makeConfigMapTemplate("templates/app.yaml", "app", nil), + }, withName("rollback-dry-run")) + seedRollbackRelease(t, rollback, "rollback-dry-run", 2, rcommon.StatusDeployed, currentChart, joinManifestDocs( + configMapManifest("database", nil), + configMapManifest("app", nil), + ), nil) + + rollback.Version = 1 + require.NoError(t, rollback.Run("rollback-dry-run")) + + assert.Empty(t, client.updateCalls) + assert.Empty(t, client.waitCalls) + assert.Empty(t, client.deleteCalls) + assert.Equal(t, 2, latestRollbackRelease(t, rollback, "rollback-dry-run").Version) +} diff --git a/pkg/action/sequencing.go b/pkg/action/sequencing.go new file mode 100644 index 000000000..65bcad88d --- /dev/null +++ b/pkg/action/sequencing.go @@ -0,0 +1,511 @@ +/* +Copyright The Helm Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package action + +import ( + "bytes" + "context" + "errors" + "fmt" + "log/slog" + "slices" + "strings" + "time" + + chartv2 "helm.sh/helm/v4/pkg/chart/v2" + chartutil "helm.sh/helm/v4/pkg/chart/v2/util" + "helm.sh/helm/v4/pkg/kube" + releaseutil "helm.sh/helm/v4/pkg/release/v1/util" + + "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/cli-runtime/pkg/resource" +) + +// computeDeadline returns a deadline based on the given timeout duration. +// If timeout is zero or negative, it returns the zero time (no deadline). +func computeDeadline(timeout time.Duration) time.Time { + if timeout <= 0 { + return time.Time{} + } + return time.Now().Add(timeout) +} + +// GroupManifestsByDirectSubchart groups manifests by the direct subchart they belong to. +// The current chart's own manifests are returned under the empty string key "". +// Subcharts are keyed by their immediate name under the first `/charts//` +// segment found in the manifest source path. +// Nested subcharts (e.g., `/charts/sub/charts/nested/`) are grouped under +// the direct subchart name ("sub"), since nested sequencing is handled recursively. +func GroupManifestsByDirectSubchart(manifests []releaseutil.Manifest, chartName string) map[string][]releaseutil.Manifest { + result := make(map[string][]releaseutil.Manifest) + if chartName == "" { + // Fallback: assign everything to parent + result[""] = append(result[""], manifests...) + return result + } + + chartsPrefix := chartName + "/charts/" + for _, m := range manifests { + if !strings.HasPrefix(m.Name, chartsPrefix) { + // Parent chart manifest + result[""] = append(result[""], m) + continue + } + // Extract the direct subchart name (first segment after "/charts/") + rest := m.Name[len(chartsPrefix):] + // rest is like "subchart1/templates/deploy.yaml" or "subchart1/charts/nested/..." + subchartName, _, ok := strings.Cut(rest, "/") + if !ok { + // Unlikely: a file directly under charts/ with no subdirectory + result[""] = append(result[""], m) + continue + } + result[subchartName] = append(result[subchartName], m) + } + return result +} + +// buildManifestYAML concatenates the Content fields of the given manifests into a single +// YAML stream suitable for passing to KubeClient.Build(). +func buildManifestYAML(manifests []releaseutil.Manifest) string { + if len(manifests) == 0 { + return "" + } + var buf strings.Builder + for i, m := range manifests { + if i > 0 { + buf.WriteString("---\n") + } + buf.WriteString(m.Content) + buf.WriteString("\n") + } + return buf.String() +} + +// sequencedDeployment performs ordered installation or upgrade of chart resources. +// It handles the two-level DAG: first subchart ordering, then resource-group +// ordering within each chart level. +type sequencedDeployment struct { + cfg *Configuration + releaseName string + releaseNamespace string + disableOpenAPI bool + serverSideApply bool + forceConflicts bool + forceReplace bool + waitStrategy kube.WaitStrategy + waitOptions []kube.WaitOption + waitForJobs bool + timeout time.Duration + readinessTimeout time.Duration + deadline time.Time // overall operation deadline + + // Upgrade-specific fields. When upgradeMode is true, createAndWait delegates + // to updateAndWait which calls KubeClient.Update() instead of Create(). + upgradeMode bool + currentResources kube.ResourceList // full set of old (current) resources + upgradeCSAFieldManager bool // upgrade client-side apply field manager + createdResources kube.ResourceList +} + +// deployChartLevel deploys all resources for a single chart level in sequenced order. +// It first handles subcharts in dependency order (recursively), then deploys the +// parent chart's own resource-group batches. +func (s *sequencedDeployment) deployChartLevel(ctx context.Context, chrt *chartv2.Chart, manifests []releaseutil.Manifest) error { + // Group manifests by direct subchart + grouped := GroupManifestsByDirectSubchart(manifests, chrt.Name()) + + // Build subchart DAG and deploy in topological order + dag, err := chartutil.BuildSubchartDAG(chrt) + if err != nil { + return fmt.Errorf("building subchart DAG for %s: %w", chrt.Name(), err) + } + + batches, err := dag.GetBatches() + if err != nil { + return fmt.Errorf("getting subchart batches for %s: %w", chrt.Name(), err) + } + + // Deploy each subchart batch in order + for batchIdx, batch := range batches { + for _, subchartName := range batch { + subManifests := grouped[subchartName] + if len(subManifests) == 0 { + continue + } + + // Find the subchart chart object for recursive nested sequencing + subChart := findSubchart(chrt, subchartName) + if subChart == nil { + // Subchart not found in chart object (may have been disabled or aliased differently) + // Fall back to flat resource-group deployment for these manifests + s.cfg.Logger().Warn("subchart not found in chart dependencies; deploying without subchart sequencing", + "subchart", subchartName, + "batch", batchIdx, + ) + if err := s.deployResourceGroupBatches(ctx, subManifests); err != nil { + return fmt.Errorf("deploying subchart %s resources: %w", subchartName, err) + } + continue + } + + // Recursively deploy the subchart (handles its own nested subcharts and resource-groups) + if err := s.deployChartLevel(ctx, subChart, subManifests); err != nil { + return fmt.Errorf("deploying subchart %s: %w", subchartName, err) + } + } + } + + // Deploy parent chart's own resources (after all subchart batches complete) + parentManifests := grouped[""] + if len(parentManifests) > 0 { + if err := s.deployResourceGroupBatches(ctx, parentManifests); err != nil { + return fmt.Errorf("deploying %s own resources: %w", chrt.Name(), err) + } + } + + return nil +} + +// warnIfPartialReadinessAnnotations logs a warning for each resource that has +// only one of the helm.sh/readiness-success / helm.sh/readiness-failure annotations. +// Both annotations must be present for custom readiness evaluation to work; a resource +// with only one will fall back to kstatus. +func warnIfPartialReadinessAnnotations(logger *slog.Logger, manifests []releaseutil.Manifest) { + for _, m := range manifests { + if m.Head == nil || m.Head.Metadata == nil { + continue + } + ann := m.Head.Metadata.Annotations + _, hasSuccess := ann[kube.AnnotationReadinessSuccess] + _, hasFailure := ann[kube.AnnotationReadinessFailure] + if hasSuccess != hasFailure { + logger.Warn("resource has only one readiness annotation; both helm.sh/readiness-success and helm.sh/readiness-failure must be present for custom readiness evaluation; falling back to kstatus", + "resource", m.Head.Metadata.Name, + ) + } + } +} + +// warnIfIsolatedGroups logs a warning for each resource-group that has no +// connections to other groups (no depends-on edges and not depended on by any +// other group). Isolated groups are valid but may indicate a misconfiguration +// when multiple groups are present. +func warnIfIsolatedGroups(logger *slog.Logger, result releaseutil.ResourceGroupResult) { + if len(result.Groups) <= 1 { + return // A single group is not isolated in a meaningful sense + } + for groupName := range result.Groups { + deps := result.GroupDeps[groupName] + isDependedOn := false + for _, otherDeps := range result.GroupDeps { + if slices.Contains(otherDeps, groupName) { + isDependedOn = true + break + } + } + if len(deps) == 0 && !isDependedOn { + logger.Warn("resource-group is isolated with no connections to other groups; if sequencing is intended, add helm.sh/depends-on/resource-groups annotation to related groups", + "group", groupName, + ) + } + } +} + +func batchHasCustomReadiness(manifests []releaseutil.Manifest) bool { + for _, manifest := range manifests { + if manifest.Head == nil || manifest.Head.Metadata == nil { + continue + } + annotations := manifest.Head.Metadata.Annotations + if annotations == nil { + continue + } + if annotations[kube.AnnotationReadinessSuccess] != "" || annotations[kube.AnnotationReadinessFailure] != "" { + return true + } + } + return false +} + +// deployResourceGroupBatches deploys manifests for a single chart level using +// resource-group annotation DAG ordering. Resources without group annotations +// (or with invalid ones) are deployed last. +func (s *sequencedDeployment) deployResourceGroupBatches(ctx context.Context, manifests []releaseutil.Manifest) error { + if len(manifests) == 0 { + return nil + } + + // Emit warnings for misconfigured annotations before deploying. + warnIfPartialReadinessAnnotations(s.cfg.Logger(), manifests) + + result, warnings, err := releaseutil.ParseResourceGroups(manifests) + if err != nil { + return fmt.Errorf("parsing resource-group annotations: %w", err) + } + for _, w := range warnings { + s.cfg.Logger().Warn("resource-group annotation warning", "warning", w) + } + + // If there are sequenced groups, build their DAG and deploy in order + if len(result.Groups) > 0 { + // Warn about groups that have no connections to other groups. + warnIfIsolatedGroups(s.cfg.Logger(), result) + + dag, err := releaseutil.BuildResourceGroupDAG(result) + if err != nil { + return fmt.Errorf("building resource-group DAG: %w", err) + } + + batches, err := dag.GetBatches() + if err != nil { + return fmt.Errorf("getting resource-group batches: %w", err) + } + + for _, groupBatch := range batches { + var batchManifests []releaseutil.Manifest + for _, groupName := range groupBatch { + batchManifests = append(batchManifests, result.Groups[groupName]...) + } + if err := s.createAndWait(ctx, batchManifests); err != nil { + return err + } + } + } + + // Deploy unsequenced resources last + if len(result.Unsequenced) > 0 { + if err := s.createAndWait(ctx, result.Unsequenced); err != nil { + return err + } + } + + return nil +} + +// helmSequencingAnnotations lists annotation keys used internally by Helm for +// resource sequencing. These are stripped from resources before applying to +// Kubernetes because some (e.g. helm.sh/depends-on/resource-groups) contain +// multiple slashes which is invalid per the K8s annotation key format. +var helmSequencingAnnotations = []string{ + releaseutil.AnnotationDependsOnResourceGroups, +} + +// stripSequencingAnnotations removes Helm-internal sequencing annotations from +// resources before they are applied to Kubernetes. This prevents K8s API +// validation errors for annotation keys that are not valid K8s label keys. +func stripSequencingAnnotations(resources kube.ResourceList) error { + return resources.Visit(func(info *resource.Info, err error) error { + if err != nil { + return err + } + acc, accErr := meta.Accessor(info.Object) + if accErr != nil { + return nil // skip non-accessible objects + } + annotations := acc.GetAnnotations() + if len(annotations) == 0 { + return nil + } + changed := false + for _, key := range helmSequencingAnnotations { + if _, exists := annotations[key]; exists { + delete(annotations, key) + changed = true + } + } + if changed { + acc.SetAnnotations(annotations) + } + return nil + }) +} + +// createAndWait creates (or updates, in upgrade mode) a set of manifest resources +// and waits for them to be ready. It respects both the per-batch readiness timeout +// and the overall operation deadline. +func (s *sequencedDeployment) createAndWait(ctx context.Context, manifests []releaseutil.Manifest) error { + if s.upgradeMode { + return s.updateAndWait(ctx, manifests) + } + + select { + case <-ctx.Done(): + return ctx.Err() + default: + } + if len(manifests) == 0 { + return nil + } + + yaml := buildManifestYAML(manifests) + resources, err := s.cfg.KubeClient.Build(bytes.NewBufferString(yaml), !s.disableOpenAPI) + if err != nil { + return fmt.Errorf("building resource batch: %w", err) + } + if len(resources) == 0 { + return nil + } + + if err := resources.Visit(setMetadataVisitor(s.releaseName, s.releaseNamespace, true)); err != nil { + return fmt.Errorf("setting metadata for resource batch: %w", err) + } + + if err := stripSequencingAnnotations(resources); err != nil { + return fmt.Errorf("stripping sequencing annotations: %w", err) + } + + select { + case <-ctx.Done(): + return ctx.Err() + default: + } + + result, err := s.cfg.KubeClient.Create(resources, kube.ClientCreateOptionServerSideApply(s.serverSideApply, false)) + if err != nil { + return fmt.Errorf("creating resource batch: %w", err) + } + s.createdResources = append(s.createdResources, result.Created...) + + select { + case <-ctx.Done(): + return ctx.Err() + default: + } + + return s.waitForResources(resources, manifests) +} + +// updateAndWait applies an upgrade batch using KubeClient.Update() and waits for readiness. +// It matches current (old) resources by objectKey to compute the per-batch diff. +func (s *sequencedDeployment) updateAndWait(_ context.Context, manifests []releaseutil.Manifest) error { + if len(manifests) == 0 { + return nil + } + + yaml := buildManifestYAML(manifests) + target, err := s.cfg.KubeClient.Build(bytes.NewBufferString(yaml), !s.disableOpenAPI) + if err != nil { + return fmt.Errorf("building resource batch: %w", err) + } + if len(target) == 0 { + return nil + } + + if err := target.Visit(setMetadataVisitor(s.releaseName, s.releaseNamespace, true)); err != nil { + return fmt.Errorf("setting metadata for resource batch: %w", err) + } + + if err := stripSequencingAnnotations(target); err != nil { + return fmt.Errorf("stripping sequencing annotations: %w", err) + } + + // Find the subset of current (old) resources that are represented in this batch. + // Update() will handle creates (target resources not in matchingCurrent) and + // updates (resources in both). Deletions are handled separately after all batches. + targetKeys := make(map[string]bool, len(target)) + for _, r := range target { + targetKeys[objectKey(r)] = true + } + var matchingCurrent kube.ResourceList + for _, r := range s.currentResources { + if targetKeys[objectKey(r)] { + matchingCurrent = append(matchingCurrent, r) + } + } + + result, err := s.cfg.KubeClient.Update( + matchingCurrent, + target, + kube.ClientUpdateOptionForceReplace(s.forceReplace), + kube.ClientUpdateOptionServerSideApply(s.serverSideApply, s.forceConflicts), + kube.ClientUpdateOptionUpgradeClientSideFieldManager(s.upgradeCSAFieldManager), + ) + if err != nil { + return fmt.Errorf("updating resource batch: %w", err) + } + s.createdResources = append(s.createdResources, result.Created...) + + return s.waitForResources(target, manifests) +} + +// waitForResources waits for the given resources to become ready, +// applying the per-batch and overall deadline constraints. +func (s *sequencedDeployment) waitForResources(resources kube.ResourceList, manifests []releaseutil.Manifest) error { + // Determine effective wait timeout: min(readinessTimeout, remaining time to overall deadline) + waitTimeout := s.readinessTimeout + if !s.deadline.IsZero() { + remaining := time.Until(s.deadline) + if remaining <= 0 { + return errors.New("overall timeout exceeded before waiting for resource batch") + } + if waitTimeout <= 0 || remaining < waitTimeout { + waitTimeout = remaining + } + } + if waitTimeout <= 0 { + waitTimeout = time.Minute // safe default + } + + var err error + var waiter kube.Waiter + waitOptions := append([]kube.WaitOption(nil), s.waitOptions...) + if batchHasCustomReadiness(manifests) { + waitOptions = append(waitOptions, kube.WithCustomReadinessStatusReader()) + } + if c, ok := s.cfg.KubeClient.(kube.InterfaceWaitOptions); ok { + waiter, err = c.GetWaiterWithOptions(s.waitStrategy, waitOptions...) + } else { + waiter, err = s.cfg.KubeClient.GetWaiter(s.waitStrategy) + } + if err != nil { + return fmt.Errorf("getting waiter for resource batch: %w", err) + } + + if s.waitForJobs { + return waiter.WaitWithJobs(resources, waitTimeout) + } + return waiter.Wait(resources, waitTimeout) +} + +// findSubchart finds the subchart chart object within chrt's direct dependencies by name or alias. +// It uses the parent chart's Metadata.Dependencies to resolve aliases, since the alias is stored +// on the Dependency struct in Chart.yaml, not on the subchart's own Metadata. +func findSubchart(chrt *chartv2.Chart, nameOrAlias string) *chartv2.Chart { + // Build a map from subchart chart name ā†’ alias from the parent's dependency declarations. + aliasMap := make(map[string]string) // chart name ā†’ effective name (alias or name) + if chrt.Metadata != nil { + for _, dep := range chrt.Metadata.Dependencies { + effective := dep.Name + if dep.Alias != "" { + effective = dep.Alias + } + aliasMap[dep.Name] = effective + } + } + + for _, dep := range chrt.Dependencies() { + effective := dep.Name() + if alias, ok := aliasMap[dep.Name()]; ok { + effective = alias + } + if effective == nameOrAlias || dep.Name() == nameOrAlias { + return dep + } + } + return nil +} diff --git a/pkg/action/sequencing_test.go b/pkg/action/sequencing_test.go new file mode 100644 index 000000000..0eb92c1c4 --- /dev/null +++ b/pkg/action/sequencing_test.go @@ -0,0 +1,724 @@ +/* +Copyright The Helm Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package action + +import ( + "bytes" + "context" + "encoding/json" + "errors" + "fmt" + "io" + "log/slog" + "maps" + "net/http" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" + yamlutil "k8s.io/apimachinery/pkg/util/yaml" + "k8s.io/cli-runtime/pkg/resource" + "k8s.io/client-go/kubernetes/scheme" + restfake "k8s.io/client-go/rest/fake" + + "helm.sh/helm/v4/pkg/chart/common" + chart "helm.sh/helm/v4/pkg/chart/v2" + "helm.sh/helm/v4/pkg/kube" + kubefake "helm.sh/helm/v4/pkg/kube/fake" + ri "helm.sh/helm/v4/pkg/release" + rcommon "helm.sh/helm/v4/pkg/release/common" + release "helm.sh/helm/v4/pkg/release/v1" + releaseutil "helm.sh/helm/v4/pkg/release/v1/util" +) + +type recordingKubeClient struct { + kubefake.PrintingKubeClient + createCalls [][]string + updateCalls []updateCall + waitCalls [][]string + watchUntilReadyCalls [][]string + deleteWaitCalls [][]string + deleteCalls [][]string + operations []string + waitCallCount int + waitErrorOnCall int + waitError error + onBuild func() + onCreate func() +} + +type updateCall struct { + current []string + target []string + created []string +} + +type recordingKubeWaiter struct { + client *recordingKubeClient +} + +func newRecordingKubeClient() *recordingKubeClient { + return &recordingKubeClient{ + PrintingKubeClient: kubefake.PrintingKubeClient{Out: io.Discard, LogOutput: io.Discard}, + } +} + +func (c *recordingKubeClient) Create(resources kube.ResourceList, _ ...kube.ClientCreateOption) (*kube.Result, error) { + if c.onCreate != nil { + c.onCreate() + } + ids := resourceIDs(resources) + c.createCalls = append(c.createCalls, ids) + c.operations = append(c.operations, "create:"+strings.Join(ids, ",")) + return &kube.Result{Created: resources}, nil +} + +func (c *recordingKubeClient) Delete(resources kube.ResourceList, deletionPropagation metav1.DeletionPropagation) (*kube.Result, []error) { + ids := resourceIDs(resources) + c.deleteCalls = append(c.deleteCalls, ids) + c.operations = append(c.operations, "delete:"+strings.Join(ids, ",")) + return c.PrintingKubeClient.Delete(resources, deletionPropagation) +} + +func (c *recordingKubeClient) Update(current, target kube.ResourceList, _ ...kube.ClientUpdateOption) (*kube.Result, error) { + currentIDs := resourceIDs(current) + targetIDs := resourceIDs(target) + + currentSet := make(map[string]struct{}, len(currentIDs)) + for _, id := range currentIDs { + currentSet[id] = struct{}{} + } + + var created kube.ResourceList + var createdIDs []string + for _, info := range target { + id := fmt.Sprintf("%s/%s", info.Object.GetObjectKind().GroupVersionKind().Kind, info.Name) + if _, exists := currentSet[id]; !exists { + created = append(created, info) + createdIDs = append(createdIDs, id) + } + } + + c.updateCalls = append(c.updateCalls, updateCall{ + current: currentIDs, + target: targetIDs, + created: createdIDs, + }) + c.operations = append(c.operations, "update:"+strings.Join(targetIDs, ",")) + return &kube.Result{Updated: target, Created: created}, nil +} + +func (c *recordingKubeClient) Build(reader io.Reader, _ bool) (kube.ResourceList, error) { + if c.onBuild != nil { + c.onBuild() + } + decoder := yamlutil.NewYAMLOrJSONDecoder(reader, 4096) + var resources kube.ResourceList + + for { + var obj map[string]any + if err := decoder.Decode(&obj); err != nil { + if errors.Is(err, io.EOF) { + break + } + return nil, err + } + if len(obj) == 0 { + continue + } + + u := &unstructured.Unstructured{Object: obj} + gvk := u.GroupVersionKind() + namespace := u.GetNamespace() + if namespace == "" { + namespace = "spaced" + u.SetNamespace(namespace) + } + info := &resource.Info{ + Name: u.GetName(), + Namespace: namespace, + Object: u, + Mapping: &meta.RESTMapping{ + Resource: schema.GroupVersionResource{Group: gvk.Group, Version: gvk.Version, Resource: strings.ToLower(gvk.Kind) + "s"}, + GroupVersionKind: gvk, + Scope: meta.RESTScopeNamespace, + }, + Client: newNotFoundRESTClient(u.GetName(), gvk), + } + resources.Append(info) + } + + return resources, nil +} + +func (c *recordingKubeClient) GetWaiter(ws kube.WaitStrategy) (kube.Waiter, error) { + return c.GetWaiterWithOptions(ws) +} + +func (c *recordingKubeClient) GetWaiterWithOptions(_ kube.WaitStrategy, _ ...kube.WaitOption) (kube.Waiter, error) { + return &recordingKubeWaiter{client: c}, nil +} + +func (w *recordingKubeWaiter) Wait(resources kube.ResourceList, _ time.Duration) error { + return w.client.recordWait(resources) +} + +func (w *recordingKubeWaiter) WaitWithJobs(resources kube.ResourceList, _ time.Duration) error { + return w.client.recordWait(resources) +} + +func (w *recordingKubeWaiter) WaitForDelete(resources kube.ResourceList, _ time.Duration) error { + ids := resourceIDs(resources) + w.client.deleteWaitCalls = append(w.client.deleteWaitCalls, ids) + w.client.operations = append(w.client.operations, "wait-delete:"+strings.Join(ids, ",")) + return nil +} + +func (w *recordingKubeWaiter) WatchUntilReady(resources kube.ResourceList, _ time.Duration) error { + ids := resourceIDs(resources) + w.client.watchUntilReadyCalls = append(w.client.watchUntilReadyCalls, ids) + w.client.operations = append(w.client.operations, "watch:"+strings.Join(ids, ",")) + return nil +} + +func (c *recordingKubeClient) recordWait(resources kube.ResourceList) error { + c.waitCallCount++ + ids := resourceIDs(resources) + c.waitCalls = append(c.waitCalls, ids) + c.operations = append(c.operations, "wait:"+strings.Join(ids, ",")) + if c.waitError != nil && c.waitCallCount == c.waitErrorOnCall { + return c.waitError + } + return nil +} + +func newNotFoundRESTClient(name string, gvk schema.GroupVersionKind) *restfake.RESTClient { + body, _ := json.Marshal(metav1.Status{ + Status: metav1.StatusFailure, + Reason: metav1.StatusReasonNotFound, + Code: http.StatusNotFound, + Details: &metav1.StatusDetails{ + Name: name, + Group: gvk.Group, + Kind: gvk.Kind, + }, + }) + + return &restfake.RESTClient{ + GroupVersion: gvk.GroupVersion(), + NegotiatedSerializer: scheme.Codecs.WithoutConversion(), + Client: restfake.CreateHTTPClient(func(_ *http.Request) (*http.Response, error) { + return &http.Response{ + StatusCode: http.StatusNotFound, + Header: http.Header{"Content-Type": []string{"application/json"}}, + Body: io.NopCloser(bytes.NewReader(body)), + }, nil + }), + } +} + +func resourceIDs(resources kube.ResourceList) []string { + ids := make([]string, 0, len(resources)) + for _, info := range resources { + kind := info.Object.GetObjectKind().GroupVersionKind().Kind + if kind == "" { + kind = "Unknown" + } + ids = append(ids, fmt.Sprintf("%s/%s", kind, info.Name)) + } + return ids +} + +func newSequencedInstallAction(t *testing.T, kubeClient kube.Interface) *Install { + t.Helper() + cfg := actionConfigFixture(t) + cfg.KubeClient = kubeClient + + install := NewInstall(cfg) + install.Namespace = "spaced" + install.ReleaseName = "test-install-release" + install.Timeout = 5 * time.Minute + install.ReadinessTimeout = time.Minute + install.WaitStrategy = kube.OrderedWaitStrategy + return install +} + +func releaseFile(name, content string) *common.File { + return &common.File{Name: name, ModTime: time.Now(), Data: []byte(content)} +} + +func configMapManifest(name string, annotations map[string]string) string { + var b strings.Builder + b.WriteString("apiVersion: v1\nkind: ConfigMap\nmetadata:\n") + _, _ = fmt.Fprintf(&b, " name: %s\n", name) + if len(annotations) > 0 { + b.WriteString(" annotations:\n") + for key, value := range annotations { + _, _ = fmt.Fprintf(&b, " %s: %q\n", key, value) + } + } + b.WriteString("data:\n key: value\n") + return b.String() +} + +func makeConfigMapTemplate(fileName, name string, annotations map[string]string) *common.File { + return releaseFile(fileName, configMapManifest(name, annotations)) +} + +func makeHookTemplate(fileName, name, hook string, extraAnnotations map[string]string) *common.File { + annotations := map[string]string{release.HookAnnotation: hook} + maps.Copy(annotations, extraAnnotations) + return makeConfigMapTemplate(fileName, name, annotations) +} + +func mustRelease(t *testing.T, rel ri.Releaser) *release.Release { + t.Helper() + out, err := releaserToV1Release(rel) + require.NoError(t, err) + return out +} + +func TestSequencing_GroupManifestsByDirectSubchart(t *testing.T) { + manifests := []releaseutil.Manifest{ + makeTestManifest("parent", "parent/templates/one.yaml", nil), + makeTestManifest("db", "parent/charts/database/templates/one.yaml", nil), + makeTestManifest("cache", "parent/charts/database/charts/cache/templates/one.yaml", nil), + } + + grouped := GroupManifestsByDirectSubchart(manifests, "parent") + + require.Len(t, grouped[""], 1) + require.Len(t, grouped["database"], 2) +} + +func TestSequencing_BuildManifestYAML(t *testing.T) { + yaml := buildManifestYAML([]releaseutil.Manifest{ + makeTestManifest("one", "chart/templates/one.yaml", nil), + makeTestManifest("two", "chart/templates/two.yaml", nil), + }) + + assert.Contains(t, yaml, "name: one") + assert.Contains(t, yaml, "name: two") +} + +func TestInstall_Sequenced_BasicResourceGroups(t *testing.T) { + client := newRecordingKubeClient() + install := newSequencedInstallAction(t, client) + + ch := buildChartWithTemplates([]*common.File{ + makeConfigMapTemplate("templates/database.yaml", "database", map[string]string{ + releaseutil.AnnotationResourceGroup: "database", + }), + makeConfigMapTemplate("templates/app.yaml", "app", map[string]string{ + releaseutil.AnnotationResourceGroup: "app", + releaseutil.AnnotationDependsOnResourceGroups: `["database"]`, + }), + }) + + rel := mustRelease(t, mustRunInstall(t, install, ch)) + assert.Equal(t, [][]string{{"ConfigMap/database"}, {"ConfigMap/app"}}, client.createCalls) + assert.Equal(t, [][]string{{"ConfigMap/database"}, {"ConfigMap/app"}}, client.waitCalls) + assert.Equal(t, rcommon.StatusDeployed, rel.Info.Status) + assert.NotNil(t, rel.SequencingInfo) + assert.True(t, rel.SequencingInfo.Enabled) +} + +func TestInstall_Sequenced_SubchartOrdering(t *testing.T) { + client := newRecordingKubeClient() + install := newSequencedInstallAction(t, client) + + parent := buildChartWithTemplates([]*common.File{ + makeConfigMapTemplate("templates/parent.yaml", "parent", nil), + }, withName("parent")) + database := buildChartWithTemplates([]*common.File{ + makeConfigMapTemplate("templates/database.yaml", "database", nil), + }, withName("database")) + api := buildChartWithTemplates([]*common.File{ + makeConfigMapTemplate("templates/api.yaml", "api", nil), + }, withName("api")) + + parent.AddDependency(database, api) + parent.Metadata.Dependencies = []*chart.Dependency{ + {Name: "database"}, + {Name: "api", DependsOn: []string{"database"}}, + } + + mustRunInstall(t, install, parent) + + assert.Equal(t, [][]string{{"ConfigMap/database"}, {"ConfigMap/api"}, {"ConfigMap/parent"}}, client.createCalls) + assert.Equal(t, client.createCalls, client.waitCalls) +} + +func TestInstall_Sequenced_Combined(t *testing.T) { + client := newRecordingKubeClient() + install := newSequencedInstallAction(t, client) + + parent := buildChartWithTemplates([]*common.File{ + makeConfigMapTemplate("templates/parent.yaml", "parent", nil), + }, withName("parent")) + database := buildChartWithTemplates([]*common.File{ + makeConfigMapTemplate("templates/setup.yaml", "db-setup", map[string]string{ + releaseutil.AnnotationResourceGroup: "setup", + }), + makeConfigMapTemplate("templates/data.yaml", "db-data", map[string]string{ + releaseutil.AnnotationResourceGroup: "data", + releaseutil.AnnotationDependsOnResourceGroups: `["setup"]`, + }), + }, withName("database")) + api := buildChartWithTemplates([]*common.File{ + makeConfigMapTemplate("templates/api.yaml", "api", map[string]string{ + releaseutil.AnnotationResourceGroup: "api", + }), + }, withName("api")) + + parent.AddDependency(database, api) + parent.Metadata.Dependencies = []*chart.Dependency{ + {Name: "database"}, + {Name: "api", DependsOn: []string{"database"}}, + } + + mustRunInstall(t, install, parent) + + assert.Equal(t, [][]string{ + {"ConfigMap/db-setup"}, + {"ConfigMap/db-data"}, + {"ConfigMap/api"}, + {"ConfigMap/parent"}, + }, client.createCalls) +} + +func TestInstall_Sequenced_NoAnnotations(t *testing.T) { + client := newRecordingKubeClient() + install := newSequencedInstallAction(t, client) + + ch := buildChartWithTemplates([]*common.File{ + makeConfigMapTemplate("templates/first.yaml", "first", nil), + makeConfigMapTemplate("templates/second.yaml", "second", nil), + }) + + rel := mustRelease(t, mustRunInstall(t, install, ch)) + + require.Len(t, client.createCalls, 1) + assert.ElementsMatch(t, []string{"ConfigMap/first", "ConfigMap/second"}, client.createCalls[0]) + assert.Equal(t, rcommon.StatusDeployed, rel.Info.Status) +} + +func TestInstall_Sequenced_UnsequencedLast(t *testing.T) { + client := newRecordingKubeClient() + install := newSequencedInstallAction(t, client) + + ch := buildChartWithTemplates([]*common.File{ + makeConfigMapTemplate("templates/database.yaml", "database", map[string]string{ + releaseutil.AnnotationResourceGroup: "database", + }), + makeConfigMapTemplate("templates/orphan.yaml", "orphan", map[string]string{ + releaseutil.AnnotationResourceGroup: "orphan", + releaseutil.AnnotationDependsOnResourceGroups: `["missing"]`, + }), + makeConfigMapTemplate("templates/plain.yaml", "plain", nil), + }) + + mustRunInstall(t, install, ch) + + require.Len(t, client.createCalls, 2) + assert.Equal(t, []string{"ConfigMap/database"}, client.createCalls[0]) + assert.ElementsMatch(t, []string{"ConfigMap/orphan", "ConfigMap/plain"}, client.createCalls[1]) +} + +func TestInstall_Sequenced_ReadinessTimeout(t *testing.T) { + client := newRecordingKubeClient() + client.waitErrorOnCall = 1 + client.waitError = errors.New("timed out waiting for batch") + + install := newSequencedInstallAction(t, client) + ch := buildChartWithTemplates([]*common.File{ + makeConfigMapTemplate("templates/database.yaml", "database", map[string]string{ + releaseutil.AnnotationResourceGroup: "database", + }), + makeConfigMapTemplate("templates/app.yaml", "app", map[string]string{ + releaseutil.AnnotationResourceGroup: "app", + releaseutil.AnnotationDependsOnResourceGroups: `["database"]`, + }), + }) + + rel, err := install.Run(ch, nil) + require.Error(t, err) + assert.Contains(t, err.Error(), "timed out waiting for batch") + assert.Len(t, client.createCalls, 1) + + last, getErr := install.cfg.Releases.Last(install.ReleaseName) + require.NoError(t, getErr) + assert.Equal(t, rcommon.StatusFailed, mustRelease(t, last).Info.Status) + assert.Equal(t, rcommon.StatusFailed, mustRelease(t, rel).Info.Status) +} + +func TestInstall_Sequenced_AtomicRollback(t *testing.T) { + client := newRecordingKubeClient() + client.waitErrorOnCall = 1 + client.waitError = errors.New("timed out waiting for batch") + + install := newSequencedInstallAction(t, client) + install.RollbackOnFailure = true + + ch := buildChartWithTemplates([]*common.File{ + makeConfigMapTemplate("templates/database.yaml", "database", map[string]string{ + releaseutil.AnnotationResourceGroup: "database", + }), + makeConfigMapTemplate("templates/app.yaml", "app", map[string]string{ + releaseutil.AnnotationResourceGroup: "app", + releaseutil.AnnotationDependsOnResourceGroups: `["database"]`, + }), + }) + + _, err := install.Run(ch, nil) + require.Error(t, err) + assert.Contains(t, err.Error(), "rollback-on-failure") + assert.NotEmpty(t, client.deleteCalls) +} + +func TestInstall_Sequenced_DryRun(t *testing.T) { + client := newRecordingKubeClient() + install := newSequencedInstallAction(t, client) + install.DryRunStrategy = DryRunServer + + ch := buildChartWithTemplates([]*common.File{ + makeConfigMapTemplate("templates/database.yaml", "database", map[string]string{ + releaseutil.AnnotationResourceGroup: "database", + }), + makeConfigMapTemplate("templates/app.yaml", "app", map[string]string{ + releaseutil.AnnotationResourceGroup: "app", + releaseutil.AnnotationDependsOnResourceGroups: `["database"]`, + }), + }) + + rel := mustRelease(t, mustRunInstall(t, install, ch)) + assert.Empty(t, client.createCalls) + assert.Empty(t, client.waitCalls) + assert.Equal(t, "Dry run complete", rel.Info.Description) +} + +func TestInstall_Sequenced_CyclicDependency(t *testing.T) { + client := newRecordingKubeClient() + install := newSequencedInstallAction(t, client) + + ch := buildChartWithTemplates([]*common.File{ + makeConfigMapTemplate("templates/a.yaml", "a", map[string]string{ + releaseutil.AnnotationResourceGroup: "a", + releaseutil.AnnotationDependsOnResourceGroups: `["b"]`, + }), + makeConfigMapTemplate("templates/b.yaml", "b", map[string]string{ + releaseutil.AnnotationResourceGroup: "b", + releaseutil.AnnotationDependsOnResourceGroups: `["a"]`, + }), + }) + + _, err := install.Run(ch, nil) + require.Error(t, err) + assert.True(t, strings.Contains(err.Error(), "cycle") || strings.Contains(err.Error(), "circular")) + assert.Empty(t, client.createCalls) +} + +func TestInstall_Sequenced_SequencingInfoStored(t *testing.T) { + client := newRecordingKubeClient() + install := newSequencedInstallAction(t, client) + + ch := buildChartWithTemplates([]*common.File{ + makeConfigMapTemplate("templates/config.yaml", "config", nil), + }) + + rel := mustRelease(t, mustRunInstall(t, install, ch)) + + require.NotNil(t, rel.SequencingInfo) + assert.True(t, rel.SequencingInfo.Enabled) + assert.Equal(t, string(kube.OrderedWaitStrategy), rel.SequencingInfo.Strategy) + + stored, err := install.cfg.Releases.Get(rel.Name, rel.Version) + require.NoError(t, err) + assert.Equal(t, string(kube.OrderedWaitStrategy), mustRelease(t, stored).SequencingInfo.Strategy) +} + +func TestInstall_NonSequenced_Unchanged(t *testing.T) { + client := newRecordingKubeClient() + install := newSequencedInstallAction(t, client) + install.WaitStrategy = kube.StatusWatcherStrategy + + ch := buildChartWithTemplates([]*common.File{ + makeConfigMapTemplate("templates/database.yaml", "database", map[string]string{ + releaseutil.AnnotationResourceGroup: "database", + }), + makeConfigMapTemplate("templates/app.yaml", "app", map[string]string{ + releaseutil.AnnotationResourceGroup: "app", + releaseutil.AnnotationDependsOnResourceGroups: `["database"]`, + }), + }) + + rel := mustRelease(t, mustRunInstall(t, install, ch)) + + require.Len(t, client.createCalls, 1) + assert.ElementsMatch(t, []string{"ConfigMap/database", "ConfigMap/app"}, client.createCalls[0]) + assert.Nil(t, rel.SequencingInfo) +} + +func TestInstall_Sequenced_HookResourcesExcludedFromDAG(t *testing.T) { + client := newRecordingKubeClient() + install := newSequencedInstallAction(t, client) + + ch := buildChartWithTemplates([]*common.File{ + makeHookTemplate("templates/pre-hook.yaml", "pre-hook", release.HookPreInstall.String(), map[string]string{ + releaseutil.AnnotationResourceGroup: "database", + }), + makeConfigMapTemplate("templates/database.yaml", "database", map[string]string{ + releaseutil.AnnotationResourceGroup: "database", + }), + }) + + mustRunInstall(t, install, ch) + + assert.Equal(t, [][]string{{"ConfigMap/pre-hook"}, {"ConfigMap/database"}}, client.createCalls) + assert.Equal(t, [][]string{{"ConfigMap/pre-hook"}}, client.watchUntilReadyCalls) +} + +func TestInstall_Sequenced_HooksAtStandardPositions(t *testing.T) { + client := newRecordingKubeClient() + install := newSequencedInstallAction(t, client) + + ch := buildChartWithTemplates([]*common.File{ + makeHookTemplate("templates/pre-hook.yaml", "pre-hook", release.HookPreInstall.String(), map[string]string{ + releaseutil.AnnotationResourceGroup: "database", + }), + makeConfigMapTemplate("templates/database.yaml", "database", map[string]string{ + releaseutil.AnnotationResourceGroup: "database", + }), + makeHookTemplate("templates/post-hook.yaml", "post-hook", release.HookPostInstall.String(), map[string]string{ + releaseutil.AnnotationResourceGroup: "database", + }), + }) + + mustRunInstall(t, install, ch) + + assert.Equal(t, [][]string{{"ConfigMap/pre-hook"}, {"ConfigMap/database"}, {"ConfigMap/post-hook"}}, client.createCalls) + assert.Equal(t, [][]string{{"ConfigMap/pre-hook"}, {"ConfigMap/post-hook"}}, client.watchUntilReadyCalls) +} + +func TestSequencing_WarnIfPartialReadinessAnnotations(t *testing.T) { + var buf bytes.Buffer + handler := slog.NewTextHandler(&buf, &slog.HandlerOptions{Level: slog.LevelWarn}) + oldLogger := slog.Default() + slog.SetDefault(slog.New(handler)) + t.Cleanup(func() { slog.SetDefault(oldLogger) }) + + warnIfPartialReadinessAnnotations(slog.Default(), []releaseutil.Manifest{ + makeTestManifest("cm", "chart/templates/cm.yaml", map[string]string{ + kube.AnnotationReadinessSuccess: `["{.ready} == true"]`, + }), + }) + + assert.Contains(t, buf.String(), "readiness") +} + +func TestSequencedDeployment_CreateAndWait_RespectsContextCancellation(t *testing.T) { + newSequencedDeploymentForTest := func(client kube.Interface) *sequencedDeployment { + t.Helper() + + cfg := actionConfigFixture(t) + cfg.KubeClient = client + + return &sequencedDeployment{ + cfg: cfg, + releaseName: "demo", + releaseNamespace: "spaced", + waitStrategy: kube.OrderedWaitStrategy, + readinessTimeout: time.Minute, + } + } + + t.Run("context canceled before build", func(t *testing.T) { + client := newRecordingKubeClient() + sd := newSequencedDeploymentForTest(client) + + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + err := sd.createAndWait(ctx, []releaseutil.Manifest{ + makeTestManifest("cm", "chart/templates/cm.yaml", nil), + }) + require.ErrorIs(t, err, context.Canceled) + assert.Empty(t, client.createCalls) + assert.Empty(t, client.waitCalls) + }) + + t.Run("context canceled after build before create", func(t *testing.T) { + client := newRecordingKubeClient() + ctx, cancel := context.WithCancel(context.Background()) + client.onBuild = cancel + + sd := newSequencedDeploymentForTest(client) + + err := sd.createAndWait(ctx, []releaseutil.Manifest{ + makeTestManifest("cm", "chart/templates/cm.yaml", nil), + }) + require.ErrorIs(t, err, context.Canceled) + assert.Empty(t, client.createCalls) + assert.Empty(t, client.waitCalls) + }) + + t.Run("context canceled after create before wait", func(t *testing.T) { + client := newRecordingKubeClient() + ctx, cancel := context.WithCancel(context.Background()) + client.onCreate = cancel + + sd := newSequencedDeploymentForTest(client) + + err := sd.createAndWait(ctx, []releaseutil.Manifest{ + makeTestManifest("cm", "chart/templates/cm.yaml", nil), + }) + require.ErrorIs(t, err, context.Canceled) + require.Len(t, client.createCalls, 1) + assert.Empty(t, client.waitCalls) + }) +} + +func makeTestManifest(name, sourcePath string, annotations map[string]string) releaseutil.Manifest { + content := configMapManifest(name, annotations) + head := &releaseutil.SimpleHead{ + Version: v1.SchemeGroupVersion.String(), + Kind: "ConfigMap", + } + head.Metadata = &struct { + Name string `json:"name"` + Namespace string `json:"namespace,omitempty"` + Annotations map[string]string `json:"annotations"` + }{ + Name: name, + Annotations: annotations, + } + return releaseutil.Manifest{Name: sourcePath, Content: content, Head: head} +} + +func mustRunInstall(t *testing.T, install *Install, ch *chart.Chart) ri.Releaser { + t.Helper() + rel, err := install.RunWithContext(context.Background(), ch, map[string]any{}) + require.NoError(t, err) + return rel +} diff --git a/pkg/action/uninstall.go b/pkg/action/uninstall.go index 79156991c..0e8afeb98 100644 --- a/pkg/action/uninstall.go +++ b/pkg/action/uninstall.go @@ -25,10 +25,13 @@ import ( v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + chartcommon "helm.sh/helm/v4/pkg/chart/common" + renderutil "helm.sh/helm/v4/pkg/chart/common/util" + chart "helm.sh/helm/v4/pkg/chart/v2" chartutil "helm.sh/helm/v4/pkg/chart/v2/util" "helm.sh/helm/v4/pkg/kube" releasei "helm.sh/helm/v4/pkg/release" - "helm.sh/helm/v4/pkg/release/common" + releasecommon "helm.sh/helm/v4/pkg/release/common" release "helm.sh/helm/v4/pkg/release/v1" releaseutil "helm.sh/helm/v4/pkg/release/v1/util" "helm.sh/helm/v4/pkg/storage/driver" @@ -116,7 +119,7 @@ func (u *Uninstall) Run(name string) (*releasei.UninstallReleaseResponse, error) // TODO: Are there any cases where we want to force a delete even if it's // already marked deleted? - if rel.Info.Status == common.StatusUninstalled { + if rel.Info.Status == releasecommon.StatusUninstalled { if !u.KeepHistory { if err := u.purgeReleases(rels...); err != nil { return nil, fmt.Errorf("uninstall: Failed to purge the release: %w", err) @@ -127,7 +130,7 @@ func (u *Uninstall) Run(name string) (*releasei.UninstallReleaseResponse, error) } u.cfg.Logger().Debug("uninstall: deleting release", "name", name) - rel.Info.Status = common.StatusUninstalling + rel.Info.Status = releasecommon.StatusUninstalling rel.Info.Deleted = time.Now() rel.Info.Description = "Deletion in progress (or silently failed)" res := &releasei.UninstallReleaseResponse{Release: rel} @@ -147,7 +150,7 @@ func (u *Uninstall) Run(name string) (*releasei.UninstallReleaseResponse, error) u.cfg.Logger().Debug("uninstall: Failed to store updated release", slog.Any("error", err)) } - deletedResources, kept, errs := u.deleteRelease(rel) + deletedResources, kept, errs := u.deleteRelease(rel, waiter) if errs != nil { u.cfg.Logger().Debug("uninstall: Failed to delete release", slog.Any("error", errs)) return nil, fmt.Errorf("failed to delete release: %s", name) @@ -158,8 +161,10 @@ func (u *Uninstall) Run(name string) (*releasei.UninstallReleaseResponse, error) } res.Info = kept - if err := waiter.WaitForDelete(deletedResources, u.Timeout); err != nil { - errs = append(errs, err) + if !isSequencedRelease(rel) { + if err := waiter.WaitForDelete(deletedResources, u.Timeout); err != nil { + errs = append(errs, err) + } } if !u.DisableHooks { @@ -169,7 +174,7 @@ func (u *Uninstall) Run(name string) (*releasei.UninstallReleaseResponse, error) } } - rel.Info.Status = common.StatusUninstalled + rel.Info.Status = releasecommon.StatusUninstalled if len(u.Description) > 0 { rel.Info.Description = u.Description } else { @@ -208,7 +213,7 @@ func (u *Uninstall) Run(name string) (*releasei.UninstallReleaseResponse, error) } u.cfg.Logger().Debug("superseding previous deployment", "version", rel.Version) - rel.Info.Status = common.StatusSuperseded + rel.Info.Status = releasecommon.StatusSuperseded if err := u.cfg.Releases.Update(rel); err != nil { u.cfg.Logger().Debug("uninstall: Failed to store updated release", slog.Any("error", err)) } @@ -220,6 +225,10 @@ func (u *Uninstall) Run(name string) (*releasei.UninstallReleaseResponse, error) return res, nil } +func isSequencedRelease(rel *release.Release) bool { + return rel.SequencingInfo != nil && rel.SequencingInfo.Enabled +} + func (u *Uninstall) purgeReleases(rels ...*release.Release) error { for _, rel := range rels { if _, err := u.cfg.Releases.Delete(rel.Name, rel.Version); err != nil { @@ -253,10 +262,8 @@ func (e *joinedErrors) Unwrap() []error { return e.errs } -// deleteRelease deletes the release and returns list of delete resources and manifests that were kept in the deletion process -func (u *Uninstall) deleteRelease(rel *release.Release) (kube.ResourceList, string, []error) { - var errs []error - +// deleteRelease deletes the release and returns list of delete resources and manifests that were kept in the deletion process. +func (u *Uninstall) deleteRelease(rel *release.Release, waiter kube.Waiter) (kube.ResourceList, string, []error) { manifests := releaseutil.SplitManifests(rel.Manifest) _, files, err := releaseutil.SortManifests(manifests, nil, releaseutil.UninstallOrder) if err != nil { @@ -273,19 +280,273 @@ func (u *Uninstall) deleteRelease(rel *release.Release) (kube.ResourceList, stri fmt.Fprintf(&kept, "[%s] %s\n", f.Head.Kind, f.Head.Metadata.Name) } + if isSequencedRelease(rel) { + filesToDelete = u.recoverManifestPathsForSequencedUninstall(rel, filesToDelete) + deleted, errs := u.sequencedDeleteManifests(rel.Chart, filesToDelete, waiter) + return deleted, kept.String(), errs + } + + resources, err := u.buildDeleteResources(filesToDelete) + if err != nil { + return nil, "", []error{err} + } + if len(resources) > 0 { + _, errs := u.cfg.KubeClient.Delete(resources, parseCascadingFlag(u.DeletionPropagation)) + return resources, kept.String(), errs + } + return resources, kept.String(), nil +} + +func (u *Uninstall) buildDeleteResources(manifests []releaseutil.Manifest) (kube.ResourceList, error) { var builder strings.Builder - for _, file := range filesToDelete { + for _, file := range manifests { builder.WriteString("\n---\n" + file.Content) } resources, err := u.cfg.KubeClient.Build(strings.NewReader(builder.String()), false) if err != nil { - return nil, "", []error{fmt.Errorf("unable to build kubernetes objects for delete: %w", err)} + return nil, fmt.Errorf("unable to build kubernetes objects for delete: %w", err) } - if len(resources) > 0 { - _, errs = u.cfg.KubeClient.Delete(resources, parseCascadingFlag(u.DeletionPropagation)) + return resources, nil +} + +func (u *Uninstall) deleteManifestBatch(manifests []releaseutil.Manifest, waiter kube.Waiter, deadline time.Time) (kube.ResourceList, []error) { + resources, err := u.buildDeleteResources(manifests) + if err != nil || len(resources) == 0 { + if err != nil { + return nil, []error{err} + } + return nil, nil + } + + _, errs := u.cfg.KubeClient.Delete(resources, parseCascadingFlag(u.DeletionPropagation)) + if len(errs) > 0 { + return resources, errs + } + + timeout := u.Timeout + if !deadline.IsZero() { + if remaining := time.Until(deadline); remaining < timeout { + timeout = remaining + } + } + if err := waiter.WaitForDelete(resources, timeout); err != nil { + errs = append(errs, err) + } + return resources, errs +} + +func (u *Uninstall) sequencedDeleteManifests(chrt *chart.Chart, manifests []releaseutil.Manifest, waiter kube.Waiter) (kube.ResourceList, []error) { + deadline := computeDeadline(u.Timeout) + if chrt == nil { + return u.deleteResourceGroupBatchesReverse(manifests, waiter, deadline) + } + return u.deleteChartLevelReverse(chrt, manifests, waiter, deadline) +} + +func (u *Uninstall) deleteChartLevelReverse(chrt *chart.Chart, manifests []releaseutil.Manifest, waiter kube.Waiter, deadline time.Time) (kube.ResourceList, []error) { + grouped := GroupManifestsByDirectSubchart(manifests, chrt.Name()) + + allDeleted, errs := u.deleteResourceGroupBatchesReverse(grouped[""], waiter, deadline) + if len(errs) > 0 { + return allDeleted, errs + } + + dag, err := chartutil.BuildSubchartDAG(chrt) + if err != nil { + return allDeleted, []error{fmt.Errorf("building subchart DAG for sequenced uninstall: %w", err)} + } + + batches, err := dag.GetBatches() + if err != nil { + return allDeleted, []error{fmt.Errorf("getting subchart batches for sequenced uninstall: %w", err)} + } + + for i, j := 0, len(batches)-1; i < j; i, j = i+1, j-1 { + batches[i], batches[j] = batches[j], batches[i] + } + + for _, batch := range batches { + for _, subchartName := range batch { + subchartManifests := grouped[subchartName] + if len(subchartManifests) == 0 { + continue + } + + subchart := findSubchart(chrt, subchartName) + var deleted kube.ResourceList + if subchart == nil { + u.cfg.Logger().Warn("subchart not found in chart dependencies during sequenced uninstall; falling back to flat resource-group deletion", "subchart", subchartName) + deleted, errs = u.deleteResourceGroupBatchesReverse(subchartManifests, waiter, deadline) + } else { + deleted, errs = u.deleteChartLevelReverse(subchart, subchartManifests, waiter, deadline) + } + allDeleted = append(allDeleted, deleted...) + if len(errs) > 0 { + return allDeleted, errs + } + } + } + + return allDeleted, nil +} + +func (u *Uninstall) deleteResourceGroupBatchesReverse(manifests []releaseutil.Manifest, waiter kube.Waiter, deadline time.Time) (kube.ResourceList, []error) { + result, warnings, err := releaseutil.ParseResourceGroups(manifests) + if err != nil { + return nil, []error{fmt.Errorf("parsing resource-group annotations for sequenced uninstall: %w", err)} + } + for _, warning := range warnings { + u.cfg.Logger().Warn("resource-group annotation warning during uninstall", "warning", warning) + } + + var allDeleted kube.ResourceList + + if len(result.Unsequenced) > 0 { + deleted, errs := u.deleteManifestBatch(result.Unsequenced, waiter, deadline) + allDeleted = append(allDeleted, deleted...) + if len(errs) > 0 { + return allDeleted, errs + } } - return resources, kept.String(), errs + + if len(result.Groups) == 0 { + return allDeleted, nil + } + + dag, err := releaseutil.BuildResourceGroupDAG(result) + if err != nil { + return allDeleted, []error{fmt.Errorf("building resource-group DAG for sequenced uninstall: %w", err)} + } + batches, err := dag.GetBatches() + if err != nil { + return allDeleted, []error{fmt.Errorf("getting resource-group batches for sequenced uninstall: %w", err)} + } + + for i, j := 0, len(batches)-1; i < j; i, j = i+1, j-1 { + batches[i], batches[j] = batches[j], batches[i] + } + + for _, batch := range batches { + var batchManifests []releaseutil.Manifest + for _, groupName := range batch { + batchManifests = append(batchManifests, result.Groups[groupName]...) + } + deleted, errs := u.deleteManifestBatch(batchManifests, waiter, deadline) + allDeleted = append(allDeleted, deleted...) + if len(errs) > 0 { + return allDeleted, errs + } + } + + return allDeleted, nil +} + +func (u *Uninstall) recoverManifestPathsForSequencedUninstall(rel *release.Release, manifests []releaseutil.Manifest) []releaseutil.Manifest { + rendered, err := renderReleaseManifestsWithPaths(u.cfg, rel) + if err != nil { + u.cfg.Logger().Warn("unable to recover rendered chart paths for sequenced uninstall; falling back to stored manifest order", slog.Any("error", err)) + return manifests + } + return applyRenderedManifestPaths(manifests, rendered) +} + +// renderReleaseManifestsWithPaths re-renders the release chart+config to produce +// manifests keyed by source template paths (e.g., "mychart/templates/deploy.yaml"). +func renderReleaseManifestsWithPaths(cfg *Configuration, rel *release.Release) ([]releaseutil.Manifest, error) { + if rel.Chart == nil { + return nil, errors.New("release chart is missing") + } + if err := chartutil.ProcessDependencies(rel.Chart, rel.Config); err != nil { + return nil, fmt.Errorf("processing chart dependencies: %w", err) + } + + caps, err := cfg.getCapabilities() + if err != nil { + return nil, fmt.Errorf("getting capabilities: %w", err) + } + + valuesToRender, err := renderutil.ToRenderValuesWithSchemaValidation( + rel.Chart, + rel.Config, + chartcommon.ReleaseOptions{ + Name: rel.Name, + Namespace: rel.Namespace, + Revision: rel.Version, + IsInstall: rel.Version <= 1, + IsUpgrade: rel.Version > 1, + }, + caps, + true, + ) + if err != nil { + return nil, fmt.Errorf("building render values: %w", err) + } + + _, _, _, manifests, err := cfg.renderResourcesWithFiles(rel.Chart, valuesToRender, rel.Name, "", false, false, false, nil, false, false, false, PostRenderStrategyCombined) + if err != nil { + return nil, fmt.Errorf("rendering chart manifests: %w", err) + } + return manifests, nil +} + +func applyRenderedManifestPaths(stored, rendered []releaseutil.Manifest) []releaseutil.Manifest { + byContent := make(map[string][]string, len(rendered)) + byIdentity := make(map[string][]string, len(rendered)) + for _, manifest := range rendered { + contentKey := normalizedManifestContent(manifest.Content) + byContent[contentKey] = append(byContent[contentKey], manifest.Name) + + identityKey := manifestIdentity(manifest) + byIdentity[identityKey] = append(byIdentity[identityKey], manifest.Name) + } + + out := make([]releaseutil.Manifest, len(stored)) + copy(out, stored) + + for i, manifest := range out { + contentKey := normalizedManifestContent(manifest.Content) + if names := byContent[contentKey]; len(names) > 0 { + out[i].Name = names[0] + byContent[contentKey] = names[1:] + continue + } + + identityKey := manifestIdentity(manifest) + if names := byIdentity[identityKey]; len(names) > 0 { + out[i].Name = names[0] + byIdentity[identityKey] = names[1:] + } + } + + return out +} + +func manifestIdentity(manifest releaseutil.Manifest) string { + if manifest.Head == nil || manifest.Head.Metadata == nil { + return normalizedManifestContent(manifest.Content) + } + return fmt.Sprintf("%s/%s/%s/%s", manifest.Head.Version, manifest.Head.Kind, manifest.Head.Metadata.Namespace, manifest.Head.Metadata.Name) +} + +func normalizedManifestContent(content string) string { + trimmed := strings.TrimSpace(content) + if trimmed == "" { + return "" + } + + lines := strings.Split(trimmed, "\n") + start := 0 + for start < len(lines) { + line := strings.TrimSpace(lines[start]) + if line == "" || strings.HasPrefix(line, "#") { + start++ + continue + } + break + } + + return strings.TrimSpace(strings.Join(lines[start:], "\n")) } func parseCascadingFlag(cascadingFlag string) v1.DeletionPropagation { diff --git a/pkg/action/uninstall_sequenced_test.go b/pkg/action/uninstall_sequenced_test.go new file mode 100644 index 000000000..f48cd8d51 --- /dev/null +++ b/pkg/action/uninstall_sequenced_test.go @@ -0,0 +1,413 @@ +/* +Copyright The Helm Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package action + +import ( + "fmt" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "helm.sh/helm/v4/pkg/chart/common" + chart "helm.sh/helm/v4/pkg/chart/v2" + "helm.sh/helm/v4/pkg/kube" + rcommon "helm.sh/helm/v4/pkg/release/common" + release "helm.sh/helm/v4/pkg/release/v1" + releaseutil "helm.sh/helm/v4/pkg/release/v1/util" +) + +func newSequencedUninstallAction(t *testing.T, kubeClient kube.Interface) *Uninstall { + t.Helper() + + cfg := actionConfigFixture(t) + cfg.KubeClient = kubeClient + + uninstall := NewUninstall(cfg) + uninstall.DisableHooks = true + uninstall.Timeout = 5 * time.Minute + uninstall.WaitStrategy = kube.OrderedWaitStrategy + + return uninstall +} + +func seedUninstallRelease(t *testing.T, uninstall *Uninstall, rel *release.Release) { + t.Helper() + require.NoError(t, uninstall.cfg.Releases.Create(rel)) +} + +func newUninstallRelease(name string, ch *chart.Chart, manifest string, sequencingInfo *release.SequencingInfo, hooks ...*release.Hook) *release.Release { + now := time.Now() + return &release.Release{ + Name: name, + Namespace: "spaced", + Chart: ch, + Config: map[string]any{}, + Manifest: manifest, + Info: &release.Info{ + FirstDeployed: now, + LastDeployed: now, + Status: rcommon.StatusDeployed, + Description: "Seeded release", + }, + Version: 1, + Hooks: hooks, + SequencingInfo: sequencingInfo, + } +} + +func secretManifest(name string) string { + return fmt.Sprintf(`apiVersion: v1 +kind: Secret +metadata: + name: %s +type: Opaque +data: + password: cGFzc3dvcmQ= +`, name) +} + +func serviceAccountManifest(name string) string { + return fmt.Sprintf(`apiVersion: v1 +kind: ServiceAccount +metadata: + name: %s +`, name) +} + +func expectedUninstallIDs(t *testing.T, manifest string) []string { + t.Helper() + + _, files, err := releaseutil.SortManifests(releaseutil.SplitManifests(manifest), nil, releaseutil.UninstallOrder) + require.NoError(t, err) + + ids := make([]string, 0, len(files)) + for _, file := range files { + ids = append(ids, fmt.Sprintf("%s/%s", file.Head.Kind, file.Head.Metadata.Name)) + } + return ids +} + +func filterDeleteCalls(calls [][]string, wanted ...string) [][]string { + want := make(map[string]struct{}, len(wanted)) + for _, id := range wanted { + want[id] = struct{}{} + } + + var filtered [][]string + for _, call := range calls { + if len(call) != 1 { + continue + } + if _, ok := want[call[0]]; ok { + filtered = append(filtered, call) + } + } + return filtered +} + +func operationIndex(operations []string, exact string) int { + for i, operation := range operations { + if operation == exact { + return i + } + } + return -1 +} + +func newDeleteHook(name string, event release.HookEvent) *release.Hook { + return &release.Hook{ + Name: name, + Kind: "ConfigMap", + Path: name, + Manifest: configMapManifest(name, nil), + Events: []release.HookEvent{event}, + DeletePolicies: []release.HookDeletePolicy{release.HookSucceeded}, + } +} + +func TestUninstall_Sequenced_ReverseOrder(t *testing.T) { + client := newRecordingKubeClient() + uninstall := newSequencedUninstallAction(t, client) + + ch := buildChartWithTemplates([]*common.File{ + makeConfigMapTemplate("templates/database.yaml", "database", map[string]string{ + releaseutil.AnnotationResourceGroup: "database", + }), + makeConfigMapTemplate("templates/app.yaml", "app", map[string]string{ + releaseutil.AnnotationResourceGroup: "app", + releaseutil.AnnotationDependsOnResourceGroups: `["database"]`, + }), + }, withName("sequenced-uninstall")) + + rel := newUninstallRelease( + "sequenced-uninstall", + ch, + joinManifestDocs( + configMapManifest("database", map[string]string{releaseutil.AnnotationResourceGroup: "database"}), + configMapManifest("app", map[string]string{ + releaseutil.AnnotationResourceGroup: "app", + releaseutil.AnnotationDependsOnResourceGroups: `["database"]`, + }), + ), + &release.SequencingInfo{Enabled: true, Strategy: string(kube.OrderedWaitStrategy)}, + ) + seedUninstallRelease(t, uninstall, rel) + + _, err := uninstall.Run(rel.Name) + require.NoError(t, err) + + assert.Equal(t, [][]string{{"ConfigMap/app"}, {"ConfigMap/database"}}, client.deleteCalls) + assert.Equal(t, client.deleteCalls, client.deleteWaitCalls) +} + +func TestUninstall_NonSequenced_Unchanged(t *testing.T) { + client := newRecordingKubeClient() + uninstall := newSequencedUninstallAction(t, client) + + manifest := joinManifestDocs( + configMapManifest("config", nil), + secretManifest("secret"), + serviceAccountManifest("service-account"), + ) + rel := newUninstallRelease("standard-uninstall", buildChartWithTemplates(nil, withName("standard-uninstall")), manifest, nil) + seedUninstallRelease(t, uninstall, rel) + + _, err := uninstall.Run(rel.Name) + require.NoError(t, err) + + require.Len(t, client.deleteCalls, 1) + assert.Equal(t, expectedUninstallIDs(t, manifest), client.deleteCalls[0]) + require.Len(t, client.deleteWaitCalls, 1) + assert.Equal(t, client.deleteCalls[0], client.deleteWaitCalls[0]) +} + +func TestUninstall_Sequenced_WithSubcharts(t *testing.T) { + client := newRecordingKubeClient() + uninstall := newSequencedUninstallAction(t, client) + + parent := buildChartWithTemplates([]*common.File{ + makeConfigMapTemplate("templates/parent.yaml", "parent", nil), + }, withName("parent")) + bar := buildChartWithTemplates([]*common.File{ + makeConfigMapTemplate("templates/bar.yaml", "bar", nil), + }, withName("bar")) + nginx := buildChartWithTemplates([]*common.File{ + makeConfigMapTemplate("templates/nginx.yaml", "nginx", nil), + }, withName("nginx")) + + bar.AddDependency(nginx) + bar.Metadata.Dependencies = []*chart.Dependency{{Name: "nginx"}} + parent.AddDependency(bar) + parent.Metadata.Dependencies = []*chart.Dependency{{Name: "bar"}} + + rel := newUninstallRelease( + "subchart-uninstall", + parent, + joinManifestDocs( + configMapManifest("nginx", nil), + configMapManifest("bar", nil), + configMapManifest("parent", nil), + ), + &release.SequencingInfo{Enabled: true, Strategy: string(kube.OrderedWaitStrategy)}, + ) + seedUninstallRelease(t, uninstall, rel) + + _, err := uninstall.Run(rel.Name) + require.NoError(t, err) + + // Nested subcharts within a subchart's subtree are flattened into one delete + // batch because GroupManifestsByDirectSubchart receives full-path manifest + // names ("parent/charts/bar/charts/nginx/...") that don't match the nested + // chart prefix ("bar/charts/"). This is a known limitation. + assert.Equal(t, [][]string{{"ConfigMap/parent"}, {"ConfigMap/nginx", "ConfigMap/bar"}}, client.deleteCalls) + assert.Equal(t, client.deleteCalls, client.deleteWaitCalls) +} + +func TestUninstall_Sequenced_KeepPolicy(t *testing.T) { + client := newRecordingKubeClient() + uninstall := newSequencedUninstallAction(t, client) + + ch := buildChartWithTemplates([]*common.File{ + makeConfigMapTemplate("templates/database-live.yaml", "database-live", map[string]string{ + releaseutil.AnnotationResourceGroup: "database", + }), + makeConfigMapTemplate("templates/database-keep.yaml", "database-keep", map[string]string{ + releaseutil.AnnotationResourceGroup: "database", + kube.ResourcePolicyAnno: kube.KeepPolicy, + }), + makeConfigMapTemplate("templates/app.yaml", "app", map[string]string{ + releaseutil.AnnotationResourceGroup: "app", + releaseutil.AnnotationDependsOnResourceGroups: `["database"]`, + }), + }, withName("keep-policy")) + + rel := newUninstallRelease( + "keep-policy", + ch, + joinManifestDocs( + configMapManifest("database-live", map[string]string{releaseutil.AnnotationResourceGroup: "database"}), + configMapManifest("database-keep", map[string]string{ + releaseutil.AnnotationResourceGroup: "database", + kube.ResourcePolicyAnno: kube.KeepPolicy, + }), + configMapManifest("app", map[string]string{ + releaseutil.AnnotationResourceGroup: "app", + releaseutil.AnnotationDependsOnResourceGroups: `["database"]`, + }), + ), + &release.SequencingInfo{Enabled: true, Strategy: string(kube.OrderedWaitStrategy)}, + ) + seedUninstallRelease(t, uninstall, rel) + + res, err := uninstall.Run(rel.Name) + require.NoError(t, err) + + assert.Equal(t, [][]string{{"ConfigMap/app"}, {"ConfigMap/database-live"}}, client.deleteCalls) + assert.Equal(t, client.deleteCalls, client.deleteWaitCalls) + assert.NotContains(t, client.deleteCalls, []string{"ConfigMap/database-keep"}) + assert.Contains(t, res.Info, "[ConfigMap] database-keep") +} + +func TestUninstall_Sequenced_KeepHistory(t *testing.T) { + client := newRecordingKubeClient() + uninstall := newSequencedUninstallAction(t, client) + uninstall.KeepHistory = true + + ch := buildChartWithTemplates([]*common.File{ + makeConfigMapTemplate("templates/database.yaml", "database", map[string]string{ + releaseutil.AnnotationResourceGroup: "database", + }), + makeConfigMapTemplate("templates/app.yaml", "app", map[string]string{ + releaseutil.AnnotationResourceGroup: "app", + releaseutil.AnnotationDependsOnResourceGroups: `["database"]`, + }), + }, withName("keep-history")) + + rel := newUninstallRelease( + "keep-history", + ch, + joinManifestDocs( + configMapManifest("database", map[string]string{releaseutil.AnnotationResourceGroup: "database"}), + configMapManifest("app", map[string]string{ + releaseutil.AnnotationResourceGroup: "app", + releaseutil.AnnotationDependsOnResourceGroups: `["database"]`, + }), + ), + &release.SequencingInfo{Enabled: true, Strategy: string(kube.OrderedWaitStrategy)}, + ) + seedUninstallRelease(t, uninstall, rel) + + _, err := uninstall.Run(rel.Name) + require.NoError(t, err) + + reli, err := uninstall.cfg.Releases.Get(rel.Name, rel.Version) + require.NoError(t, err) + stored, err := releaserToV1Release(reli) + require.NoError(t, err) + + assert.Equal(t, [][]string{{"ConfigMap/app"}, {"ConfigMap/database"}}, client.deleteCalls) + assert.Equal(t, rcommon.StatusUninstalled, stored.Info.Status) +} + +func TestUninstall_Sequenced_DryRun(t *testing.T) { + client := newRecordingKubeClient() + uninstall := newSequencedUninstallAction(t, client) + uninstall.DryRun = true + + ch := buildChartWithTemplates([]*common.File{ + makeConfigMapTemplate("templates/database.yaml", "database", map[string]string{ + releaseutil.AnnotationResourceGroup: "database", + }), + makeConfigMapTemplate("templates/app.yaml", "app", map[string]string{ + releaseutil.AnnotationResourceGroup: "app", + releaseutil.AnnotationDependsOnResourceGroups: `["database"]`, + }), + }, withName("dry-run-uninstall")) + + rel := newUninstallRelease( + "dry-run-uninstall", + ch, + joinManifestDocs( + configMapManifest("database", map[string]string{releaseutil.AnnotationResourceGroup: "database"}), + configMapManifest("app", map[string]string{ + releaseutil.AnnotationResourceGroup: "app", + releaseutil.AnnotationDependsOnResourceGroups: `["database"]`, + }), + ), + &release.SequencingInfo{Enabled: true, Strategy: string(kube.OrderedWaitStrategy)}, + ) + seedUninstallRelease(t, uninstall, rel) + + res, err := uninstall.Run(rel.Name) + require.NoError(t, err) + require.NotNil(t, res) + assert.Empty(t, client.deleteCalls) + assert.Empty(t, client.deleteWaitCalls) +} + +func TestUninstall_Sequenced_Hooks(t *testing.T) { + client := newRecordingKubeClient() + uninstall := newSequencedUninstallAction(t, client) + uninstall.DisableHooks = false + + ch := buildChartWithTemplates([]*common.File{ + makeConfigMapTemplate("templates/database.yaml", "database", map[string]string{ + releaseutil.AnnotationResourceGroup: "database", + }), + makeConfigMapTemplate("templates/app.yaml", "app", map[string]string{ + releaseutil.AnnotationResourceGroup: "app", + releaseutil.AnnotationDependsOnResourceGroups: `["database"]`, + }), + }, withName("hooked-uninstall")) + + rel := newUninstallRelease( + "hooked-uninstall", + ch, + joinManifestDocs( + configMapManifest("database", map[string]string{releaseutil.AnnotationResourceGroup: "database"}), + configMapManifest("app", map[string]string{ + releaseutil.AnnotationResourceGroup: "app", + releaseutil.AnnotationDependsOnResourceGroups: `["database"]`, + }), + ), + &release.SequencingInfo{Enabled: true, Strategy: string(kube.OrderedWaitStrategy)}, + newDeleteHook("pre-hook", release.HookPreDelete), + newDeleteHook("post-hook", release.HookPostDelete), + ) + seedUninstallRelease(t, uninstall, rel) + + _, err := uninstall.Run(rel.Name) + require.NoError(t, err) + + assert.Equal(t, [][]string{{"ConfigMap/pre-hook"}, {"ConfigMap/post-hook"}}, client.createCalls) + assert.Equal(t, [][]string{{"ConfigMap/pre-hook"}, {"ConfigMap/post-hook"}}, client.watchUntilReadyCalls) + assert.Equal(t, [][]string{{"ConfigMap/app"}, {"ConfigMap/database"}}, filterDeleteCalls(client.deleteCalls, "ConfigMap/app", "ConfigMap/database")) + + preHookCreate := operationIndex(client.operations, "create:ConfigMap/pre-hook") + appDelete := operationIndex(client.operations, "delete:ConfigMap/app") + databaseWaitDelete := operationIndex(client.operations, "wait-delete:ConfigMap/database") + postHookCreate := operationIndex(client.operations, "create:ConfigMap/post-hook") + + require.NotEqual(t, -1, preHookCreate) + require.NotEqual(t, -1, appDelete) + require.NotEqual(t, -1, databaseWaitDelete) + require.NotEqual(t, -1, postHookCreate) + assert.Less(t, preHookCreate, appDelete) + assert.Greater(t, postHookCreate, databaseWaitDelete) +} diff --git a/pkg/action/upgrade.go b/pkg/action/upgrade.go index 00939ffa6..63ea54edb 100644 --- a/pkg/action/upgrade.go +++ b/pkg/action/upgrade.go @@ -70,6 +70,10 @@ type Upgrade struct { SkipCRDs bool // Timeout is the timeout for this operation Timeout time.Duration + // ReadinessTimeout is the per-batch timeout when --wait=ordered is used. + // Each ordered batch waits at most this long for resources to become ready. + // Must not exceed Timeout. Defaults to 1 minute when zero. + ReadinessTimeout time.Duration // WaitStrategy determines what type of waiting should be done WaitStrategy kube.WaitStrategy // WaitOptions are additional options for waiting on resources @@ -188,12 +192,16 @@ func (u *Upgrade) RunWithContext(ctx context.Context, name string, ch chart.Char u.WaitStrategy = kube.StatusWatcherStrategy } + if u.ReadinessTimeout > 0 && u.Timeout > 0 && u.ReadinessTimeout > u.Timeout { + return nil, fmt.Errorf("--readiness-timeout (%s) must not exceed --timeout (%s)", u.ReadinessTimeout, u.Timeout) + } + if err := chartutil.ValidateReleaseName(name); err != nil { return nil, fmt.Errorf("release name is invalid: %s", name) } u.cfg.Logger().Debug("preparing upgrade", "name", name) - currentRelease, upgradedRelease, serverSideApply, err := u.prepareUpgrade(name, chrt, vals) + currentRelease, upgradedRelease, serverSideApply, sortedManifests, err := u.prepareUpgrade(name, chrt, vals) if err != nil { return nil, err } @@ -201,7 +209,12 @@ func (u *Upgrade) RunWithContext(ctx context.Context, name string, ch chart.Char u.cfg.Releases.MaxHistory = u.MaxHistory u.cfg.Logger().Debug("performing update", "name", name) - res, err := u.performUpgrade(ctx, currentRelease, upgradedRelease, serverSideApply) + var res *release.Release + if u.WaitStrategy == kube.OrderedWaitStrategy { + res, err = u.performSequencedUpgrade(ctx, chrt, currentRelease, upgradedRelease, sortedManifests, serverSideApply) + } else { + res, err = u.performUpgrade(ctx, currentRelease, upgradedRelease, serverSideApply) + } if err != nil { return res, err } @@ -218,14 +231,16 @@ func (u *Upgrade) RunWithContext(ctx context.Context, name string, ch chart.Char } // prepareUpgrade builds an upgraded release for an upgrade operation. -func (u *Upgrade) prepareUpgrade(name string, chart *chartv2.Chart, vals map[string]any) (*release.Release, *release.Release, bool, error) { +// When WaitStrategy is OrderedWaitStrategy, sortedManifests contains the parsed +// manifests with annotation data needed for DAG sequencing; otherwise it is nil. +func (u *Upgrade) prepareUpgrade(name string, chart *chartv2.Chart, vals map[string]any) (*release.Release, *release.Release, bool, []releaseutil.Manifest, error) { if chart == nil { - return nil, nil, false, errMissingChart + return nil, nil, false, nil, errMissingChart } // HideSecret must be used with dry run. Otherwise, return an error. if !isDryRun(u.DryRunStrategy) && u.HideSecret { - return nil, nil, false, errors.New("hiding Kubernetes secrets requires a dry-run mode") + return nil, nil, false, nil, errors.New("hiding Kubernetes secrets requires a dry-run mode") } // finds the last non-deleted release with the given name @@ -233,19 +248,19 @@ func (u *Upgrade) prepareUpgrade(name string, chart *chartv2.Chart, vals map[str if err != nil { // to keep existing behavior of returning the "%q has no deployed releases" error when an existing release does not exist if errors.Is(err, driver.ErrReleaseNotFound) { - return nil, nil, false, driver.NewErrNoDeployedReleases(name) + return nil, nil, false, nil, driver.NewErrNoDeployedReleases(name) } - return nil, nil, false, err + return nil, nil, false, nil, err } lastRelease, err := releaserToV1Release(lastReleasei) if err != nil { - return nil, nil, false, err + return nil, nil, false, nil, err } // Concurrent `helm upgrade`s will either fail here with `errPending` or when creating the release with "already exists". This should act as a pessimistic lock. if lastRelease.Info.Status.IsPending() { - return nil, nil, false, errPending + return nil, nil, false, nil, errPending } var currentRelease *release.Release @@ -258,14 +273,14 @@ func (u *Upgrade) prepareUpgrade(name string, chart *chartv2.Chart, vals map[str var cerr error currentRelease, cerr = releaserToV1Release(currentReleasei) if cerr != nil { - return nil, nil, false, cerr + return nil, nil, false, nil, cerr } if err != nil { if errors.Is(err, driver.ErrNoDeployedReleases) && (lastRelease.Info.Status == rcommon.StatusFailed || lastRelease.Info.Status == rcommon.StatusSuperseded) { currentRelease = lastRelease } else { - return nil, nil, false, err + return nil, nil, false, nil, err } } @@ -274,11 +289,11 @@ func (u *Upgrade) prepareUpgrade(name string, chart *chartv2.Chart, vals map[str // determine if values will be reused vals, err = u.reuseValues(chart, currentRelease, vals) if err != nil { - return nil, nil, false, err + return nil, nil, false, nil, err } if err := chartutil.ProcessDependencies(chart, vals); err != nil { - return nil, nil, false, err + return nil, nil, false, nil, err } // Increment revision count. This is passed to templates, and also stored on @@ -294,25 +309,33 @@ func (u *Upgrade) prepareUpgrade(name string, chart *chartv2.Chart, vals map[str caps, err := u.cfg.getCapabilities() if err != nil { - return nil, nil, false, err + return nil, nil, false, nil, err } valuesToRender, err := util.ToRenderValuesWithSchemaValidation(chart, vals, options, caps, u.SkipSchemaValidation) if err != nil { - return nil, nil, false, err + return nil, nil, false, nil, err } - hooks, manifestDoc, notesTxt, err := u.cfg.renderResources(chart, valuesToRender, "", "", u.SubNotes, false, false, u.PostRenderer, interactWithServer(u.DryRunStrategy), u.EnableDNS, u.HideSecret, u.PostRenderStrategy) + var sortedManifests []releaseutil.Manifest + var hooks []*release.Hook + var manifestDoc *bytes.Buffer + var notesTxt string + if u.WaitStrategy == kube.OrderedWaitStrategy { + hooks, manifestDoc, notesTxt, sortedManifests, err = u.cfg.renderResourcesWithFiles(chart, valuesToRender, "", "", u.SubNotes, false, false, u.PostRenderer, interactWithServer(u.DryRunStrategy), u.EnableDNS, u.HideSecret, u.PostRenderStrategy) + } else { + hooks, manifestDoc, notesTxt, err = u.cfg.renderResources(chart, valuesToRender, "", "", u.SubNotes, false, false, u.PostRenderer, interactWithServer(u.DryRunStrategy), u.EnableDNS, u.HideSecret, u.PostRenderStrategy) + } if err != nil { - return nil, nil, false, err + return nil, nil, false, nil, err } if driver.ContainsSystemLabels(u.Labels) { - return nil, nil, false, fmt.Errorf("user supplied labels contains system reserved label name. System labels: %+v", driver.GetSystemLabels()) + return nil, nil, false, nil, fmt.Errorf("user supplied labels contains system reserved label name. System labels: %+v", driver.GetSystemLabels()) } serverSideApply, err := getUpgradeServerSideValue(u.ServerSideApply, lastRelease.ApplyMethod) if err != nil { - return nil, nil, false, err + return nil, nil, false, nil, err } u.cfg.Logger().Debug("determined release apply method", slog.Bool("server_side_apply", serverSideApply), slog.String("previous_release_apply_method", lastRelease.ApplyMethod)) @@ -340,7 +363,7 @@ func (u *Upgrade) prepareUpgrade(name string, chart *chartv2.Chart, vals map[str upgradedRelease.Info.Notes = notesTxt } err = validateManifest(u.cfg.KubeClient, manifestDoc.Bytes(), !u.DisableOpenAPIValidation) - return currentRelease, upgradedRelease, serverSideApply, err + return currentRelease, upgradedRelease, serverSideApply, sortedManifests, err } func (u *Upgrade) performUpgrade(ctx context.Context, originalRelease, upgradedRelease *release.Release, serverSideApply bool) (*release.Release, error) { @@ -467,6 +490,12 @@ func (u *Upgrade) releasingUpgrade(c chan<- resultMessage, upgradedRelease *rele u.cfg.Logger().Debug("upgrade hooks disabled", "name", upgradedRelease.Name) } + // Strip Helm-internal sequencing annotations before applying to K8s. + if err := stripSequencingAnnotations(target); err != nil { + u.reportToPerformUpgrade(c, upgradedRelease, kube.ResourceList{}, fmt.Errorf("stripping sequencing annotations: %w", err)) + return + } + upgradeClientSideFieldManager := isReleaseApplyMethodClientSideApply(originalRelease.ApplyMethod) && serverSideApply // Update client-side field manager if transitioning from client-side to server-side apply results, err := u.cfg.KubeClient.Update( current, @@ -525,6 +554,123 @@ func (u *Upgrade) releasingUpgrade(c chan<- resultMessage, upgradedRelease *rele u.reportToPerformUpgrade(c, upgradedRelease, nil, nil) } +// performSequencedUpgrade deploys chart resources in DAG-ordered batches when +// --wait=ordered is used. It mirrors releasingUpgrade but uses sequencedDeployment +// to apply each batch and wait for readiness before proceeding to the next batch. +func (u *Upgrade) performSequencedUpgrade(ctx context.Context, chrt *chartv2.Chart, currentRelease, upgradedRelease *release.Release, manifests []releaseutil.Manifest, serverSideApply bool) (*release.Release, error) { + // Build the full set of current (old) resources for diff matching. + current, err := u.cfg.KubeClient.Build(bytes.NewBufferString(currentRelease.Manifest), false) + if err != nil { + if strings.Contains(err.Error(), "unable to recognize \"\": no matches for kind") { + return upgradedRelease, fmt.Errorf("current release manifest contains removed kubernetes api(s) for this "+ + "kubernetes version and it is therefore unable to build the kubernetes "+ + "objects for performing the diff. error from kubernetes: %w", err) + } + return upgradedRelease, fmt.Errorf("unable to build kubernetes objects from current release manifest: %w", err) + } + + // Build target to compute the set of resources to delete (those removed in the new release). + target, err := u.cfg.KubeClient.Build(bytes.NewBufferString(upgradedRelease.Manifest), !u.DisableOpenAPIValidation) + if err != nil { + return upgradedRelease, fmt.Errorf("unable to build kubernetes objects from new release manifest: %w", err) + } + + if isDryRun(u.DryRunStrategy) { + u.cfg.Logger().Debug("dry run for release", "name", upgradedRelease.Name) + if len(u.Description) > 0 { + upgradedRelease.Info.Description = u.Description + } else { + upgradedRelease.Info.Description = "Dry run complete" + } + return upgradedRelease, nil + } + + u.cfg.Logger().Debug("creating upgraded release", "name", upgradedRelease.Name) + if err := u.cfg.Releases.Create(upgradedRelease); err != nil { + return nil, err + } + + // pre-upgrade hooks + if !u.DisableHooks { + if err := u.cfg.execHook(upgradedRelease, release.HookPreUpgrade, u.WaitStrategy, u.WaitOptions, u.Timeout, serverSideApply); err != nil { + return u.failRelease(upgradedRelease, nil, fmt.Errorf("pre-upgrade hooks failed: %s", err)) + } + } else { + u.cfg.Logger().Debug("upgrade hooks disabled", "name", upgradedRelease.Name) + } + + upgradeCSAFieldManager := isReleaseApplyMethodClientSideApply(currentRelease.ApplyMethod) && serverSideApply + + readinessTimeout := u.ReadinessTimeout + if readinessTimeout <= 0 { + readinessTimeout = time.Minute + } + + sd := &sequencedDeployment{ + cfg: u.cfg, + releaseName: upgradedRelease.Name, + releaseNamespace: upgradedRelease.Namespace, + disableOpenAPI: u.DisableOpenAPIValidation, + serverSideApply: serverSideApply, + forceConflicts: u.ForceConflicts, + forceReplace: u.ForceReplace, + waitStrategy: u.WaitStrategy, + waitOptions: u.WaitOptions, + waitForJobs: u.WaitForJobs, + timeout: u.Timeout, + readinessTimeout: readinessTimeout, + deadline: computeDeadline(u.Timeout), + upgradeMode: true, + currentResources: current, + upgradeCSAFieldManager: upgradeCSAFieldManager, + } + + // Set SequencingInfo before deployment so that failure recovery can use sequenced order. + upgradedRelease.SequencingInfo = &release.SequencingInfo{ + Enabled: true, + Strategy: string(u.WaitStrategy), + } + + if err := sd.deployChartLevel(ctx, chrt, manifests); err != nil { + return u.failRelease(upgradedRelease, sd.createdResources, err) + } + + // Delete resources that were removed in the new release (in old but not in new). + allNewKeys := make(map[string]bool, len(target)) + for _, r := range target { + allNewKeys[objectKey(r)] = true + } + var toBeDeleted kube.ResourceList + for _, r := range current { + if !allNewKeys[objectKey(r)] { + toBeDeleted = append(toBeDeleted, r) + } + } + if len(toBeDeleted) > 0 { + if _, errs := u.cfg.KubeClient.Delete(toBeDeleted, metav1.DeletePropagationBackground); errs != nil { + return u.failRelease(upgradedRelease, sd.createdResources, fmt.Errorf("deleting removed resources: %w", joinErrors(errs, ", "))) + } + } + + // post-upgrade hooks + if !u.DisableHooks { + if err := u.cfg.execHook(upgradedRelease, release.HookPostUpgrade, u.WaitStrategy, u.WaitOptions, u.Timeout, serverSideApply); err != nil { + return u.failRelease(upgradedRelease, sd.createdResources, fmt.Errorf("post-upgrade hooks failed: %s", err)) + } + } + + currentRelease.Info.Status = rcommon.StatusSuperseded + u.cfg.recordRelease(currentRelease) + upgradedRelease.Info.Status = rcommon.StatusDeployed + if len(u.Description) > 0 { + upgradedRelease.Info.Description = u.Description + } else { + upgradedRelease.Info.Description = "Upgrade complete" + } + u.cfg.recordRelease(upgradedRelease) + return upgradedRelease, nil +} + func (u *Upgrade) failRelease(rel *release.Release, created kube.ResourceList, err error) (*release.Release, error) { msg := fmt.Sprintf("Upgrade %q failed: %s", rel.Name, err) u.cfg.Logger().Warn( diff --git a/pkg/action/upgrade_sequenced_test.go b/pkg/action/upgrade_sequenced_test.go new file mode 100644 index 000000000..80a99e0e2 --- /dev/null +++ b/pkg/action/upgrade_sequenced_test.go @@ -0,0 +1,459 @@ +/* +Copyright The Helm Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package action + +import ( + "errors" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "helm.sh/helm/v4/pkg/chart/common" + chart "helm.sh/helm/v4/pkg/chart/v2" + "helm.sh/helm/v4/pkg/kube" + ri "helm.sh/helm/v4/pkg/release" + rcommon "helm.sh/helm/v4/pkg/release/common" + release "helm.sh/helm/v4/pkg/release/v1" + releaseutil "helm.sh/helm/v4/pkg/release/v1/util" +) + +func newSequencedUpgradeAction(t *testing.T, kubeClient kube.Interface) *Upgrade { + t.Helper() + cfg := actionConfigFixture(t) + cfg.KubeClient = kubeClient + + upgrade := NewUpgrade(cfg) + upgrade.Namespace = "spaced" + upgrade.Timeout = 5 * time.Minute + upgrade.ReadinessTimeout = time.Minute + upgrade.WaitStrategy = kube.OrderedWaitStrategy + return upgrade +} + +func seedDeployedRelease(t *testing.T, upgrade *Upgrade, name string, ch *chart.Chart, manifest string, sequencingInfo *release.SequencingInfo) { + t.Helper() + + now := time.Now() + rel := &release.Release{ + Name: name, + Namespace: "spaced", + Chart: ch, + Config: map[string]any{}, + Manifest: manifest, + Info: &release.Info{ + FirstDeployed: now, + LastDeployed: now, + Status: rcommon.StatusDeployed, + Description: "Seeded release", + }, + Version: 1, + SequencingInfo: sequencingInfo, + } + + require.NoError(t, upgrade.cfg.Releases.Create(rel)) +} + +func joinManifestDocs(docs ...string) string { + return strings.Join(docs, "\n---\n") +} + +func updateTargets(calls []updateCall) [][]string { + targets := make([][]string, 0, len(calls)) + for _, call := range calls { + targets = append(targets, call.target) + } + return targets +} + +func TestUpgrade_Sequenced_Basic(t *testing.T) { + client := newRecordingKubeClient() + upgrade := newSequencedUpgradeAction(t, client) + + currentChart := buildChartWithTemplates([]*common.File{ + makeConfigMapTemplate("templates/database.yaml", "database", map[string]string{ + releaseutil.AnnotationResourceGroup: "database", + }), + makeConfigMapTemplate("templates/app.yaml", "app", map[string]string{ + releaseutil.AnnotationResourceGroup: "app", + releaseutil.AnnotationDependsOnResourceGroups: `["database"]`, + }), + }, withName("sequenced-upgrade")) + seedDeployedRelease(t, upgrade, "sequenced-upgrade", currentChart, joinManifestDocs( + configMapManifest("database", map[string]string{releaseutil.AnnotationResourceGroup: "database"}), + configMapManifest("app", map[string]string{ + releaseutil.AnnotationResourceGroup: "app", + releaseutil.AnnotationDependsOnResourceGroups: `["database"]`, + }), + ), nil) + + upgradedChart := buildChartWithTemplates([]*common.File{ + makeConfigMapTemplate("templates/database.yaml", "database", map[string]string{ + releaseutil.AnnotationResourceGroup: "database", + }), + makeConfigMapTemplate("templates/app.yaml", "app", map[string]string{ + releaseutil.AnnotationResourceGroup: "app", + releaseutil.AnnotationDependsOnResourceGroups: `["database"]`, + }), + }, withName("sequenced-upgrade")) + + rel := mustRelease(t, mustRunUpgrade(t, upgrade, "sequenced-upgrade", upgradedChart)) + + assert.Equal(t, [][]string{{"ConfigMap/database"}, {"ConfigMap/app"}}, updateTargets(client.updateCalls)) + assert.Equal(t, [][]string{{"ConfigMap/database"}, {"ConfigMap/app"}}, client.waitCalls) + require.Len(t, client.updateCalls, 2) + assert.Equal(t, []string{"ConfigMap/database"}, client.updateCalls[0].current) + assert.Equal(t, []string{"ConfigMap/app"}, client.updateCalls[1].current) + assert.Equal(t, rcommon.StatusDeployed, rel.Info.Status) + require.NotNil(t, rel.SequencingInfo) + assert.True(t, rel.SequencingInfo.Enabled) + assert.Equal(t, string(kube.OrderedWaitStrategy), rel.SequencingInfo.Strategy) +} + +func TestUpgrade_Sequenced_NewResourceGroup(t *testing.T) { + client := newRecordingKubeClient() + upgrade := newSequencedUpgradeAction(t, client) + + currentChart := buildChartWithTemplates([]*common.File{ + makeConfigMapTemplate("templates/database.yaml", "database", map[string]string{ + releaseutil.AnnotationResourceGroup: "database", + }), + makeConfigMapTemplate("templates/app.yaml", "app", map[string]string{ + releaseutil.AnnotationResourceGroup: "app", + releaseutil.AnnotationDependsOnResourceGroups: `["database"]`, + }), + }, withName("new-group")) + seedDeployedRelease(t, upgrade, "new-group", currentChart, joinManifestDocs( + configMapManifest("database", map[string]string{releaseutil.AnnotationResourceGroup: "database"}), + configMapManifest("app", map[string]string{ + releaseutil.AnnotationResourceGroup: "app", + releaseutil.AnnotationDependsOnResourceGroups: `["database"]`, + }), + ), nil) + + upgradedChart := buildChartWithTemplates([]*common.File{ + makeConfigMapTemplate("templates/database.yaml", "database", map[string]string{ + releaseutil.AnnotationResourceGroup: "database", + }), + makeConfigMapTemplate("templates/queue.yaml", "queue", map[string]string{ + releaseutil.AnnotationResourceGroup: "queue", + releaseutil.AnnotationDependsOnResourceGroups: `["database"]`, + }), + makeConfigMapTemplate("templates/app.yaml", "app", map[string]string{ + releaseutil.AnnotationResourceGroup: "app", + releaseutil.AnnotationDependsOnResourceGroups: `["queue"]`, + }), + }, withName("new-group")) + + mustRunUpgrade(t, upgrade, "new-group", upgradedChart) + + require.Len(t, client.updateCalls, 3) + assert.Equal(t, [][]string{{"ConfigMap/database"}, {"ConfigMap/queue"}, {"ConfigMap/app"}}, updateTargets(client.updateCalls)) + assert.Empty(t, client.updateCalls[1].current) + assert.Equal(t, []string{"ConfigMap/queue"}, client.updateCalls[1].created) + assert.Equal(t, [][]string{{"ConfigMap/database"}, {"ConfigMap/queue"}, {"ConfigMap/app"}}, client.waitCalls) +} + +func TestUpgrade_Sequenced_RemovedResourceGroup(t *testing.T) { + client := newRecordingKubeClient() + upgrade := newSequencedUpgradeAction(t, client) + + currentChart := buildChartWithTemplates([]*common.File{ + makeConfigMapTemplate("templates/database.yaml", "database", map[string]string{ + releaseutil.AnnotationResourceGroup: "database", + }), + makeConfigMapTemplate("templates/queue.yaml", "queue", map[string]string{ + releaseutil.AnnotationResourceGroup: "queue", + releaseutil.AnnotationDependsOnResourceGroups: `["database"]`, + }), + makeConfigMapTemplate("templates/app.yaml", "app", map[string]string{ + releaseutil.AnnotationResourceGroup: "app", + releaseutil.AnnotationDependsOnResourceGroups: `["queue"]`, + }), + }, withName("removed-group")) + seedDeployedRelease(t, upgrade, "removed-group", currentChart, joinManifestDocs( + configMapManifest("database", map[string]string{releaseutil.AnnotationResourceGroup: "database"}), + configMapManifest("queue", map[string]string{ + releaseutil.AnnotationResourceGroup: "queue", + releaseutil.AnnotationDependsOnResourceGroups: `["database"]`, + }), + configMapManifest("app", map[string]string{ + releaseutil.AnnotationResourceGroup: "app", + releaseutil.AnnotationDependsOnResourceGroups: `["queue"]`, + }), + ), nil) + + upgradedChart := buildChartWithTemplates([]*common.File{ + makeConfigMapTemplate("templates/database.yaml", "database", map[string]string{ + releaseutil.AnnotationResourceGroup: "database", + }), + makeConfigMapTemplate("templates/app.yaml", "app", map[string]string{ + releaseutil.AnnotationResourceGroup: "app", + releaseutil.AnnotationDependsOnResourceGroups: `["database"]`, + }), + }, withName("removed-group")) + + mustRunUpgrade(t, upgrade, "removed-group", upgradedChart) + + assert.Equal(t, [][]string{{"ConfigMap/database"}, {"ConfigMap/app"}}, updateTargets(client.updateCalls)) + assert.Equal(t, [][]string{{"ConfigMap/queue"}}, client.deleteCalls) + assert.Equal(t, []string{ + "update:ConfigMap/database", + "wait:ConfigMap/database", + "update:ConfigMap/app", + "wait:ConfigMap/app", + "delete:ConfigMap/queue", + }, client.operations) +} + +func TestUpgrade_NonSequencedToSequenced(t *testing.T) { + client := newRecordingKubeClient() + upgrade := newSequencedUpgradeAction(t, client) + + currentChart := buildChartWithTemplates([]*common.File{ + makeConfigMapTemplate("templates/database.yaml", "database", map[string]string{ + releaseutil.AnnotationResourceGroup: "database", + }), + makeConfigMapTemplate("templates/app.yaml", "app", map[string]string{ + releaseutil.AnnotationResourceGroup: "app", + releaseutil.AnnotationDependsOnResourceGroups: `["database"]`, + }), + }, withName("transition-to-sequenced")) + seedDeployedRelease(t, upgrade, "transition-to-sequenced", currentChart, joinManifestDocs( + configMapManifest("database", map[string]string{releaseutil.AnnotationResourceGroup: "database"}), + configMapManifest("app", map[string]string{ + releaseutil.AnnotationResourceGroup: "app", + releaseutil.AnnotationDependsOnResourceGroups: `["database"]`, + }), + ), nil) + + rel := mustRelease(t, mustRunUpgrade(t, upgrade, "transition-to-sequenced", currentChart)) + + require.Len(t, client.updateCalls, 2) + assert.Equal(t, [][]string{{"ConfigMap/database"}, {"ConfigMap/app"}}, updateTargets(client.updateCalls)) + require.NotNil(t, rel.SequencingInfo) + assert.True(t, rel.SequencingInfo.Enabled) +} + +func TestUpgrade_SequencedToNonSequenced(t *testing.T) { + client := newRecordingKubeClient() + upgrade := newSequencedUpgradeAction(t, client) + upgrade.WaitStrategy = kube.StatusWatcherStrategy + + currentChart := buildChartWithTemplates([]*common.File{ + makeConfigMapTemplate("templates/database.yaml", "database", map[string]string{ + releaseutil.AnnotationResourceGroup: "database", + }), + makeConfigMapTemplate("templates/app.yaml", "app", map[string]string{ + releaseutil.AnnotationResourceGroup: "app", + releaseutil.AnnotationDependsOnResourceGroups: `["database"]`, + }), + }, withName("transition-to-standard")) + seedDeployedRelease(t, upgrade, "transition-to-standard", currentChart, joinManifestDocs( + configMapManifest("database", map[string]string{releaseutil.AnnotationResourceGroup: "database"}), + configMapManifest("app", map[string]string{ + releaseutil.AnnotationResourceGroup: "app", + releaseutil.AnnotationDependsOnResourceGroups: `["database"]`, + }), + ), &release.SequencingInfo{Enabled: true, Strategy: string(kube.OrderedWaitStrategy)}) + + rel := mustRelease(t, mustRunUpgrade(t, upgrade, "transition-to-standard", currentChart)) + + require.Len(t, client.updateCalls, 1) + assert.ElementsMatch(t, []string{"ConfigMap/database", "ConfigMap/app"}, client.updateCalls[0].target) + require.Len(t, client.waitCalls, 1) + assert.ElementsMatch(t, []string{"ConfigMap/database", "ConfigMap/app"}, client.waitCalls[0]) + assert.Nil(t, rel.SequencingInfo) +} + +func TestUpgrade_Sequenced_FailureRollback(t *testing.T) { + client := newRecordingKubeClient() + client.waitErrorOnCall = 2 + client.waitError = assert.AnError + + upgrade := newSequencedUpgradeAction(t, client) + upgrade.RollbackOnFailure = true + + currentChart := buildChartWithTemplates([]*common.File{ + makeConfigMapTemplate("templates/database.yaml", "database", map[string]string{ + releaseutil.AnnotationResourceGroup: "database", + }), + makeConfigMapTemplate("templates/app.yaml", "app", map[string]string{ + releaseutil.AnnotationResourceGroup: "app", + releaseutil.AnnotationDependsOnResourceGroups: `["database"]`, + }), + }, withName("failure-rollback")) + seedDeployedRelease(t, upgrade, "failure-rollback", currentChart, joinManifestDocs( + configMapManifest("database", map[string]string{releaseutil.AnnotationResourceGroup: "database"}), + configMapManifest("app", map[string]string{ + releaseutil.AnnotationResourceGroup: "app", + releaseutil.AnnotationDependsOnResourceGroups: `["database"]`, + }), + ), nil) + + upgradedChart := buildChartWithTemplates([]*common.File{ + makeConfigMapTemplate("templates/database.yaml", "database", map[string]string{ + releaseutil.AnnotationResourceGroup: "database", + }), + makeConfigMapTemplate("templates/queue.yaml", "queue", map[string]string{ + releaseutil.AnnotationResourceGroup: "queue", + releaseutil.AnnotationDependsOnResourceGroups: `["database"]`, + }), + makeConfigMapTemplate("templates/app.yaml", "app", map[string]string{ + releaseutil.AnnotationResourceGroup: "app", + releaseutil.AnnotationDependsOnResourceGroups: `["queue"]`, + }), + }, withName("failure-rollback")) + + rel, err := upgrade.Run("failure-rollback", upgradedChart, map[string]any{}) + require.Error(t, err) + assert.Contains(t, err.Error(), "rolled back due to rollback-on-failure") + + failed := mustRelease(t, rel) + assert.Equal(t, rcommon.StatusFailed, failed.Info.Status) + + last, getErr := upgrade.cfg.Releases.Last("failure-rollback") + require.NoError(t, getErr) + assert.Equal(t, rcommon.StatusDeployed, mustRelease(t, last).Info.Status) +} + +func TestUpgrade_Sequenced_SubchartAdded(t *testing.T) { + client := newRecordingKubeClient() + upgrade := newSequencedUpgradeAction(t, client) + + currentParent := buildChartWithTemplates([]*common.File{ + makeConfigMapTemplate("templates/parent.yaml", "parent", nil), + }, withName("parent")) + currentDatabase := buildChartWithTemplates([]*common.File{ + makeConfigMapTemplate("templates/database.yaml", "database", nil), + }, withName("database")) + currentParent.AddDependency(currentDatabase) + currentParent.Metadata.Dependencies = []*chart.Dependency{ + {Name: "database"}, + } + seedDeployedRelease(t, upgrade, "subchart-added", currentParent, joinManifestDocs( + configMapManifest("database", nil), + configMapManifest("parent", nil), + ), nil) + + upgradedParent := buildChartWithTemplates([]*common.File{ + makeConfigMapTemplate("templates/parent.yaml", "parent", nil), + }, withName("parent")) + upgradedDatabase := buildChartWithTemplates([]*common.File{ + makeConfigMapTemplate("templates/database.yaml", "database", nil), + }, withName("database")) + upgradedAPI := buildChartWithTemplates([]*common.File{ + makeConfigMapTemplate("templates/api.yaml", "api", nil), + }, withName("api")) + upgradedParent.AddDependency(upgradedDatabase, upgradedAPI) + upgradedParent.Metadata.Dependencies = []*chart.Dependency{ + {Name: "database"}, + {Name: "api", DependsOn: []string{"database"}}, + } + + mustRunUpgrade(t, upgrade, "subchart-added", upgradedParent) + + require.Len(t, client.updateCalls, 3) + assert.Equal(t, [][]string{{"ConfigMap/database"}, {"ConfigMap/api"}, {"ConfigMap/parent"}}, updateTargets(client.updateCalls)) + assert.Empty(t, client.updateCalls[1].current) + assert.Equal(t, []string{"ConfigMap/api"}, client.updateCalls[1].created) +} + +func TestUpgrade_Sequenced_DryRun(t *testing.T) { + client := newRecordingKubeClient() + upgrade := newSequencedUpgradeAction(t, client) + upgrade.DryRunStrategy = DryRunClient + + currentChart := buildChartWithTemplates([]*common.File{ + makeConfigMapTemplate("templates/database.yaml", "database", map[string]string{ + releaseutil.AnnotationResourceGroup: "database", + }), + makeConfigMapTemplate("templates/app.yaml", "app", map[string]string{ + releaseutil.AnnotationResourceGroup: "app", + releaseutil.AnnotationDependsOnResourceGroups: `["database"]`, + }), + }, withName("dry-run")) + seedDeployedRelease(t, upgrade, "dry-run", currentChart, joinManifestDocs( + configMapManifest("database", map[string]string{releaseutil.AnnotationResourceGroup: "database"}), + configMapManifest("app", map[string]string{ + releaseutil.AnnotationResourceGroup: "app", + releaseutil.AnnotationDependsOnResourceGroups: `["database"]`, + }), + ), nil) + + rel := mustRelease(t, mustRunUpgrade(t, upgrade, "dry-run", currentChart)) + + assert.Empty(t, client.updateCalls) + assert.Empty(t, client.waitCalls) + assert.Empty(t, client.deleteCalls) + assert.Equal(t, "Dry run complete", rel.Info.Description) +} + +func TestUpgrade_Sequenced_CleanupOnFail(t *testing.T) { + client := newRecordingKubeClient() + client.waitErrorOnCall = 2 + client.waitError = errors.New("timed out waiting for batch") + + upgrade := newSequencedUpgradeAction(t, client) + upgrade.CleanupOnFail = true + + currentChart := buildChartWithTemplates([]*common.File{ + makeConfigMapTemplate("templates/database.yaml", "database", map[string]string{ + releaseutil.AnnotationResourceGroup: "database", + }), + makeConfigMapTemplate("templates/app.yaml", "app", map[string]string{ + releaseutil.AnnotationResourceGroup: "app", + releaseutil.AnnotationDependsOnResourceGroups: `["database"]`, + }), + }, withName("cleanup-on-fail")) + seedDeployedRelease(t, upgrade, "cleanup-on-fail", currentChart, joinManifestDocs( + configMapManifest("database", map[string]string{releaseutil.AnnotationResourceGroup: "database"}), + configMapManifest("app", map[string]string{ + releaseutil.AnnotationResourceGroup: "app", + releaseutil.AnnotationDependsOnResourceGroups: `["database"]`, + }), + ), nil) + + upgradedChart := buildChartWithTemplates([]*common.File{ + makeConfigMapTemplate("templates/database.yaml", "database", map[string]string{ + releaseutil.AnnotationResourceGroup: "database", + }), + makeConfigMapTemplate("templates/queue.yaml", "queue", map[string]string{ + releaseutil.AnnotationResourceGroup: "queue", + releaseutil.AnnotationDependsOnResourceGroups: `["database"]`, + }), + makeConfigMapTemplate("templates/app.yaml", "app", map[string]string{ + releaseutil.AnnotationResourceGroup: "app", + releaseutil.AnnotationDependsOnResourceGroups: `["queue"]`, + }), + }, withName("cleanup-on-fail")) + + rel, err := upgrade.Run("cleanup-on-fail", upgradedChart, map[string]any{}) + require.Error(t, err) + assert.Contains(t, err.Error(), "timed out waiting for batch") + assert.Equal(t, [][]string{{"ConfigMap/queue"}}, client.deleteCalls) + assert.Equal(t, rcommon.StatusFailed, mustRelease(t, rel).Info.Status) +} + +func mustRunUpgrade(t *testing.T, upgrade *Upgrade, name string, ch *chart.Chart) ri.Releaser { + t.Helper() + rel, err := upgrade.Run(name, ch, map[string]any{}) + require.NoError(t, err) + return rel +} diff --git a/pkg/action/warning_system_test.go b/pkg/action/warning_system_test.go new file mode 100644 index 000000000..ebc428ab6 --- /dev/null +++ b/pkg/action/warning_system_test.go @@ -0,0 +1,113 @@ +/* +Copyright The Helm Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package action + +import ( + "bytes" + "log/slog" + "testing" + + "github.com/stretchr/testify/assert" + + "helm.sh/helm/v4/pkg/chart/common" + "helm.sh/helm/v4/pkg/kube" + releaseutil "helm.sh/helm/v4/pkg/release/v1/util" +) + +func captureWarningOutput(t *testing.T) *bytes.Buffer { + t.Helper() + + var buf bytes.Buffer + handler := slog.NewTextHandler(&buf, &slog.HandlerOptions{Level: slog.LevelWarn}) + oldLogger := slog.Default() + slog.SetDefault(slog.New(handler)) + t.Cleanup(func() { slog.SetDefault(oldLogger) }) + + return &buf +} + +func runSequencedInstallWithWarnings(t *testing.T, templates ...*common.File) string { + t.Helper() + + client := newRecordingKubeClient() + install := newSequencedInstallAction(t, client) + + var buf bytes.Buffer + handler := slog.NewTextHandler(&buf, &slog.HandlerOptions{Level: slog.LevelWarn}) + install.cfg.SetLogger(handler) + + mustRunInstall(t, install, buildChartWithTemplates(templates)) + + return buf.String() +} + +func TestWarning_PartialReadinessAnnotations(t *testing.T) { + output := runSequencedInstallWithWarnings(t, + makeConfigMapTemplate("templates/app.yaml", "app", map[string]string{ + kube.AnnotationReadinessSuccess: `["{.ready} == true"]`, + }), + ) + + assert.Contains(t, output, "only one readiness annotation") + assert.Contains(t, output, "resource=app") +} + +func TestWarning_IsolatedResourceGroups(t *testing.T) { + output := runSequencedInstallWithWarnings(t, + makeConfigMapTemplate("templates/database.yaml", "database", map[string]string{ + releaseutil.AnnotationResourceGroup: "database", + }), + makeConfigMapTemplate("templates/cache.yaml", "cache", map[string]string{ + releaseutil.AnnotationResourceGroup: "cache", + }), + ) + + assert.Contains(t, output, "resource-group is isolated") + assert.Contains(t, output, "group=database") + assert.Contains(t, output, "group=cache") +} + +func TestWarning_NonExistentGroupReference(t *testing.T) { + output := runSequencedInstallWithWarnings(t, + makeConfigMapTemplate("templates/app.yaml", "app", map[string]string{ + releaseutil.AnnotationResourceGroup: "app", + releaseutil.AnnotationDependsOnResourceGroups: `["missing"]`, + }), + ) + + assert.Contains(t, output, "resource-group annotation warning") + assert.Contains(t, output, "group") + assert.Contains(t, output, "app") + assert.Contains(t, output, "non-existent group") + assert.Contains(t, output, "missing") +} + +func TestWarning_WellFormedSequencing(t *testing.T) { + output := runSequencedInstallWithWarnings(t, + makeConfigMapTemplate("templates/database.yaml", "database", map[string]string{ + releaseutil.AnnotationResourceGroup: "database", + kube.AnnotationReadinessSuccess: `["{.ready} == true"]`, + kube.AnnotationReadinessFailure: `["{.failed} == true"]`, + }), + makeConfigMapTemplate("templates/app.yaml", "app", map[string]string{ + releaseutil.AnnotationResourceGroup: "app", + releaseutil.AnnotationDependsOnResourceGroups: `["database"]`, + }), + ) + + assert.Empty(t, output) +} diff --git a/pkg/chart/common/util/jsonschema.go b/pkg/chart/common/util/jsonschema.go index 63ca0c274..e8290bf32 100644 --- a/pkg/chart/common/util/jsonschema.go +++ b/pkg/chart/common/util/jsonschema.go @@ -103,8 +103,10 @@ func ValidateAgainstSchema(ch chart.Charter, values map[string]any) error { subchartValues, ok := raw.(map[string]any) if !ok { - fmt.Fprintf(&sb, "%s:\ninvalid type for values: expected object (map), got %T\n", - sub.Name(), raw) + fmt.Fprintf(&sb, + "%s:\ninvalid type for values: expected object (map), got %T\n", + sub.Name(), raw, + ) continue } diff --git a/pkg/chart/v2/dependency.go b/pkg/chart/v2/dependency.go index 5a92ef305..782283cc0 100644 --- a/pkg/chart/v2/dependency.go +++ b/pkg/chart/v2/dependency.go @@ -47,6 +47,9 @@ type Dependency struct { ImportValues []any `json:"import-values,omitempty" yaml:"import-values,omitempty"` // Alias usable alias to be used for the chart Alias string `json:"alias,omitempty" yaml:"alias,omitempty"` + // DependsOn is a list of subchart names or aliases that must be deployed + // before this subchart. + DependsOn []string `json:"depends-on,omitempty" yaml:"depends-on,omitempty"` } // Validate checks for common problems with the dependency datastructure in diff --git a/pkg/chart/v2/dependency_json_test.go b/pkg/chart/v2/dependency_json_test.go new file mode 100644 index 000000000..8a2777739 --- /dev/null +++ b/pkg/chart/v2/dependency_json_test.go @@ -0,0 +1,64 @@ +/* +Copyright The Helm Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package v2 + +import ( + "encoding/json" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "sigs.k8s.io/yaml" +) + +func TestDependencyDependsOnJSONOmitEmpty(t *testing.T) { + dependency := Dependency{ + Name: "app", + Repository: "https://example.com/charts", + } + + data, err := json.Marshal(dependency) + require.NoError(t, err) + + assert.NotContains(t, string(data), "depends-on") +} + +func TestDependencyDependsOnJSONTag(t *testing.T) { + dependency := Dependency{ + Name: "app", + Repository: "https://example.com/charts", + DependsOn: []string{"database"}, + } + + data, err := json.Marshal(dependency) + require.NoError(t, err) + + assert.Contains(t, string(data), `"depends-on":["database"]`) +} + +func TestDependencyDependsOnYAMLBackwardCompatibility(t *testing.T) { + input := []byte(` +name: app +repository: https://example.com/charts +`) + + var dependency Dependency + err := yaml.Unmarshal(input, &dependency) + require.NoError(t, err) + + assert.Nil(t, dependency.DependsOn) +} diff --git a/pkg/chart/v2/lint/lint.go b/pkg/chart/v2/lint/lint.go index 7f6f26320..94870e817 100644 --- a/pkg/chart/v2/lint/lint.go +++ b/pkg/chart/v2/lint/lint.go @@ -66,6 +66,7 @@ func RunAll(baseDir string, values map[string]any, namespace string, options ... rules.TemplateLinterSkipSchemaValidation(lo.SkipSchemaValidation)) rules.Dependencies(&result) rules.Crds(&result) + rules.Sequencing(&result, namespace, values) return result } diff --git a/pkg/chart/v2/lint/lint_test.go b/pkg/chart/v2/lint/lint_test.go index 4256281e0..765c9312b 100644 --- a/pkg/chart/v2/lint/lint_test.go +++ b/pkg/chart/v2/lint/lint_test.go @@ -17,12 +17,15 @@ limitations under the License. package lint import ( + "path/filepath" "strings" "testing" "time" "github.com/stretchr/testify/assert" + "helm.sh/helm/v4/pkg/chart/common" + chart "helm.sh/helm/v4/pkg/chart/v2" "helm.sh/helm/v4/pkg/chart/v2/lint/support" chartutil "helm.sh/helm/v4/pkg/chart/v2/util" ) @@ -245,3 +248,42 @@ func TestMalformedTemplate(t *testing.T) { } } } + +func TestRunAll_RegistersSequencingRules(t *testing.T) { + dir := t.TempDir() + + testChart := &chart.Chart{ + Metadata: &chart.Metadata{ + Name: "sequencinglint", + Version: "0.1.0", + APIVersion: chart.APIVersionV2, + }, + Templates: []*common.File{ + { + Name: "templates/configmap.yaml", + Data: []byte(`apiVersion: v1 +kind: ConfigMap +metadata: + name: partial-readiness + annotations: + helm.sh/readiness-success: '["{.status.phase} == \"Ready\""]' +data: + key: value +`), + }, + }, + } + + assert.NoError(t, chartutil.SaveDir(testChart, dir)) + + messages := RunAll(filepath.Join(dir, testChart.Name()), nil, namespace).Messages + assert.Condition(t, func() bool { + for _, msg := range messages { + if msg.Severity == support.ErrorSev && strings.Contains(msg.Err.Error(), "both must be present or absent together") { + return true + } + } + + return false + }, "expected sequencing lint error in %#v", messages) +} diff --git a/pkg/chart/v2/lint/rules/sequencing.go b/pkg/chart/v2/lint/rules/sequencing.go new file mode 100644 index 000000000..0ac52a61e --- /dev/null +++ b/pkg/chart/v2/lint/rules/sequencing.go @@ -0,0 +1,210 @@ +/* +Copyright The Helm Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package rules // import "helm.sh/helm/v4/pkg/chart/v2/lint/rules" + +import ( + "fmt" + "path" + "strings" + + "sigs.k8s.io/yaml" + + "helm.sh/helm/v4/pkg/chart/common" + commonutil "helm.sh/helm/v4/pkg/chart/common/util" + chart "helm.sh/helm/v4/pkg/chart/v2" + "helm.sh/helm/v4/pkg/chart/v2/lint/support" + "helm.sh/helm/v4/pkg/chart/v2/loader" + chartutil "helm.sh/helm/v4/pkg/chart/v2/util" + "helm.sh/helm/v4/pkg/engine" + "helm.sh/helm/v4/pkg/kube" + releaseutil "helm.sh/helm/v4/pkg/release/v1/util" +) + +// Sequencing runs lint rules for HIP-0025 sequencing annotations. +func Sequencing(linter *support.Linter, namespace string, values map[string]any) { + c, err := loader.LoadDir(linter.ChartDir) + if err != nil { + // Chart load errors are already reported by other lint rules. + return + } + + // ProcessDependencies must run before validateSubchartSequencing: + // it prunes disabled subcharts from c.Dependencies() and applies + // alias renames, which BuildSubchartDAG relies on. + if err := chartutil.ProcessDependencies(c, values); err != nil { + return + } + + linter.RunLinterRule(support.ErrorSev, linter.ChartDir, validateSubchartSequencing(c)) + validateRenderedSequencingAnnotations(linter, c, namespace, values) +} + +func validateSubchartSequencing(c *chart.Chart) error { + if c.Metadata == nil || len(c.Metadata.Dependencies) == 0 { + return nil + } + + dag, err := chartutil.BuildSubchartDAG(c) + if err != nil { + return err + } + if _, err := dag.GetBatches(); err != nil { + return fmt.Errorf("subchart circular dependency detected: %w", err) + } + + return nil +} + +func validateRenderedSequencingAnnotations(linter *support.Linter, c *chart.Chart, namespace string, values map[string]any) { + if err := chartutil.ProcessDependencies(c, values); err != nil { + return + } + + options := common.ReleaseOptions{ + Name: "test-release", + Namespace: namespace, + } + caps := common.DefaultCapabilities.Copy() + + coalescedValues, err := commonutil.CoalesceValues(c, values) + if err != nil { + return + } + + valuesToRender, err := commonutil.ToRenderValues(c, coalescedValues, options, caps) + if err != nil { + return + } + + var renderEngine engine.Engine + renderEngine.LintMode = true + + renderedContentMap, err := renderEngine.Render(c, valuesToRender) + if err != nil { + // Template rendering errors are already reported by the Templates lint rule. + return + } + + manifestsByChart := make(map[string][]releaseutil.Manifest) + for templatePath, content := range renderedContentMap { + if strings.TrimSpace(content) == "" { + continue + } + + chartPath := renderedTemplateChartPath(templatePath) + for _, manifest := range parseRenderedManifests(content) { + validateReadinessAnnotations(linter, templatePath, manifest) + manifestsByChart[chartPath] = append(manifestsByChart[chartPath], manifest) + } + } + + for chartPath, manifests := range manifestsByChart { + validateResourceGroupAnnotations(linter, chartPath, manifests) + } +} + +func renderedTemplateChartPath(templatePath string) string { + if chartPath, _, ok := strings.Cut(templatePath, "/templates/"); ok { + return chartPath + } + + return path.Dir(templatePath) +} + +func parseRenderedManifests(content string) []releaseutil.Manifest { + rawManifests := releaseutil.SplitManifests(content) + manifests := make([]releaseutil.Manifest, 0, len(rawManifests)) + + for manifestName, raw := range rawManifests { + if strings.TrimSpace(raw) == "" { + continue + } + + var head releaseutil.SimpleHead + if err := yaml.Unmarshal([]byte(raw), &head); err != nil { + continue + } + + if head.Metadata != nil && head.Metadata.Name != "" { + manifestName = head.Metadata.Name + } + + manifests = append(manifests, releaseutil.Manifest{ + Name: manifestName, + Content: raw, + Head: &head, + }) + } + + return manifests +} + +func validateReadinessAnnotations(linter *support.Linter, templatePath string, manifest releaseutil.Manifest) { + if manifest.Head == nil || manifest.Head.Metadata == nil { + return + } + + annotations := manifest.Head.Metadata.Annotations + hasSuccess := strings.TrimSpace(annotations[kube.AnnotationReadinessSuccess]) != "" + hasFailure := strings.TrimSpace(annotations[kube.AnnotationReadinessFailure]) != "" + if hasSuccess == hasFailure { + return + } + + linter.RunLinterRule(support.ErrorSev, templatePath, fmt.Errorf( + "resource %q has only one of %q / %q annotations; both must be present or absent together", + resourceDisplayName(manifest), + kube.AnnotationReadinessSuccess, + kube.AnnotationReadinessFailure, + )) +} + +func validateResourceGroupAnnotations(linter *support.Linter, chartPath string, manifests []releaseutil.Manifest) { + result, warnings, err := releaseutil.ParseResourceGroups(manifests) + // HIP-0025: lint must fail on orphan resource-group dependencies and + // malformed annotation JSON. Runtime falls back to the unsequenced batch + // for graceful recovery, but the chart author should fix these at lint time. + for _, warning := range warnings { + linter.RunLinterRule(support.ErrorSev, chartPath, fmt.Errorf("%s", warning)) + } + if err != nil { + linter.RunLinterRule(support.ErrorSev, chartPath, err) + return + } + + dag, err := releaseutil.BuildResourceGroupDAG(result) + if err != nil { + linter.RunLinterRule(support.ErrorSev, chartPath, err) + return + } + + if _, err := dag.GetBatches(); err != nil { + linter.RunLinterRule(support.ErrorSev, chartPath, fmt.Errorf("resource-group circular dependency detected: %w", err)) + } +} + +func resourceDisplayName(manifest releaseutil.Manifest) string { + if manifest.Head == nil || manifest.Head.Metadata == nil { + return manifest.Name + } + + if manifest.Head.Kind == "" { + return manifest.Head.Metadata.Name + } + + return fmt.Sprintf("%s/%s", manifest.Head.Kind, manifest.Head.Metadata.Name) +} diff --git a/pkg/chart/v2/lint/rules/sequencing_test.go b/pkg/chart/v2/lint/rules/sequencing_test.go new file mode 100644 index 000000000..fe30594c6 --- /dev/null +++ b/pkg/chart/v2/lint/rules/sequencing_test.go @@ -0,0 +1,238 @@ +/* +Copyright The Helm Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package rules + +import ( + "fmt" + "path/filepath" + "sort" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "helm.sh/helm/v4/pkg/chart/common" + chart "helm.sh/helm/v4/pkg/chart/v2" + "helm.sh/helm/v4/pkg/chart/v2/lint/support" + chartutil "helm.sh/helm/v4/pkg/chart/v2/util" + releaseutil "helm.sh/helm/v4/pkg/release/v1/util" +) + +func TestSequencing_SubchartCircularDependency(t *testing.T) { + t.Parallel() + + root := newChart("testchart", nil) + root.Metadata.Dependencies = []*chart.Dependency{ + {Name: "subchart-a", Version: "0.1.0", Repository: "file://charts/subchart-a", DependsOn: []string{"subchart-b"}}, + {Name: "subchart-b", Version: "0.1.0", Repository: "file://charts/subchart-b", DependsOn: []string{"subchart-a"}}, + } + root.SetDependencies(newChart("subchart-a", nil), newChart("subchart-b", nil)) + + messages := runSequencingLint(t, root) + requireMessage(t, messages, support.ErrorSev, "subchart circular dependency detected") +} + +func TestSequencing_SubchartAnnotationRequiresHIPListSyntax(t *testing.T) { + t.Parallel() + + root := newChart("testchart", nil) + root.Metadata.Dependencies = []*chart.Dependency{ + {Name: "subchart-a", Version: "0.1.0", Repository: "file://charts/subchart-a"}, + {Name: "subchart-b", Version: "0.1.0", Repository: "file://charts/subchart-b"}, + } + root.Metadata.Annotations = map[string]string{ + chartutil.AnnotationDependsOnSubcharts: `{"subchart-a":["subchart-b"],"subchart-b":["subchart-a"]}`, + } + root.SetDependencies(newChart("subchart-a", nil), newChart("subchart-b", nil)) + + messages := runSequencingLint(t, root) + requireMessage(t, messages, support.ErrorSev, "JSON string array") +} + +func TestSequencing_RenderedAnnotationRules(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + templates map[string]string + wantSeverity int + wantSubstring string + }{ + { + name: "partial readiness annotations", + templates: map[string]string{ + "templates/configmap.yaml": manifestYAML("ConfigMap", "partial-readiness", map[string]string{ + releaseutil.AnnotationResourceGroup: "bootstrap", + "helm.sh/readiness-success": `'["{.phase} == \"Ready\""]'`, + }), + }, + wantSeverity: support.ErrorSev, + wantSubstring: "both must be present or absent together", + }, + { + name: "duplicate group assignment", + templates: map[string]string{ + "templates/first.yaml": manifestYAML("ConfigMap", "shared-resource", map[string]string{releaseutil.AnnotationResourceGroup: "bootstrap"}), + "templates/second.yaml": manifestYAML("ConfigMap", "shared-resource", map[string]string{releaseutil.AnnotationResourceGroup: "app"}), + }, + wantSeverity: support.ErrorSev, + wantSubstring: "assigned to multiple resource groups", + }, + { + name: "circular resource-group dependencies", + templates: map[string]string{ + "templates/group-a.yaml": manifestYAML("ConfigMap", "group-a", map[string]string{ + releaseutil.AnnotationResourceGroup: "a", + releaseutil.AnnotationDependsOnResourceGroups: `'["c"]'`, + }), + "templates/group-b.yaml": manifestYAML("ConfigMap", "group-b", map[string]string{ + releaseutil.AnnotationResourceGroup: "b", + releaseutil.AnnotationDependsOnResourceGroups: `'["a"]'`, + }), + "templates/group-c.yaml": manifestYAML("ConfigMap", "group-c", map[string]string{ + releaseutil.AnnotationResourceGroup: "c", + releaseutil.AnnotationDependsOnResourceGroups: `'["b"]'`, + }), + }, + wantSeverity: support.ErrorSev, + wantSubstring: "resource-group circular dependency detected", + }, + { + name: "orphan resource-group reference", + templates: map[string]string{ + "templates/configmap.yaml": manifestYAML("ConfigMap", "orphaned-group", map[string]string{ + releaseutil.AnnotationResourceGroup: "app", + releaseutil.AnnotationDependsOnResourceGroups: `'["database"]'`, + }), + }, + wantSeverity: support.ErrorSev, + wantSubstring: `depends-on non-existent group "database"`, + }, + { + name: "malformed depends-on-resource-groups annotation", + templates: map[string]string{ + "templates/configmap.yaml": manifestYAML("ConfigMap", "bad-json", map[string]string{ + releaseutil.AnnotationResourceGroup: "app", + releaseutil.AnnotationDependsOnResourceGroups: `not-a-json-array`, + }), + }, + wantSeverity: support.ErrorSev, + wantSubstring: "depends-on", + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + messages := runSequencingLint(t, newChart("testchart", tc.templates)) + requireMessage(t, messages, tc.wantSeverity, tc.wantSubstring) + }) + } +} + +func TestSequencing_NoMessagesWithoutSequencingAnnotations(t *testing.T) { + t.Parallel() + + messages := runSequencingLint(t, newChart("plain-chart", map[string]string{ + "templates/configmap.yaml": manifestYAML("ConfigMap", "plain", nil), + })) + + assert.Empty(t, messages) +} + +func runSequencingLint(t *testing.T, c *chart.Chart) []support.Message { + t.Helper() + + tmpDir := t.TempDir() + require.NoError(t, chartutil.SaveDir(c, tmpDir)) + + linter := support.Linter{ChartDir: filepath.Join(tmpDir, c.Name())} + Sequencing(&linter, "test-namespace", nil) + + return linter.Messages +} + +func requireMessage(t *testing.T, messages []support.Message, severity int, substring string) { + t.Helper() + + for _, message := range messages { + if message.Severity == severity && strings.Contains(message.Err.Error(), substring) { + return + } + } + + t.Fatalf("expected severity %d message containing %q, got %#v", severity, substring, messages) +} + +func newChart(name string, templates map[string]string) *chart.Chart { + files := make([]*common.File, 0, len(templates)) + names := make([]string, 0, len(templates)) + for templateName := range templates { + names = append(names, templateName) + } + sort.Strings(names) + + for _, templateName := range names { + files = append(files, &common.File{ + Name: templateName, + Data: []byte(templates[templateName]), + }) + } + + return &chart.Chart{ + Metadata: &chart.Metadata{ + Name: name, + Version: "0.1.0", + APIVersion: chart.APIVersionV2, + }, + Templates: files, + } +} + +func manifestYAML(kind, name string, annotations map[string]string) string { + if len(annotations) == 0 { + return fmt.Sprintf(`apiVersion: v1 +kind: %s +metadata: + name: %s +data: + key: value +`, kind, name) + } + + keys := make([]string, 0, len(annotations)) + for key := range annotations { + keys = append(keys, key) + } + sort.Strings(keys) + + var annotationBlock strings.Builder + for _, key := range keys { + fmt.Fprintf(&annotationBlock, "\n %s: %s", key, annotations[key]) + } + + return fmt.Sprintf(`apiVersion: v1 +kind: %s +metadata: + name: %s + annotations:%s +data: + key: value +`, kind, name, annotationBlock.String()) +} diff --git a/pkg/chart/v2/util/dag.go b/pkg/chart/v2/util/dag.go new file mode 100644 index 000000000..7bfe5d07d --- /dev/null +++ b/pkg/chart/v2/util/dag.go @@ -0,0 +1,155 @@ +/* +Copyright The Helm Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package util + +import ( + "fmt" + "maps" + "sort" + "strings" +) + +// DAG is a directed acyclic graph of string-keyed nodes, used for resource-group +// and subchart dependency ordering. +// +// Edges are directed: AddEdge("a", "b") means "b depends on a" (a must come before b). +type DAG struct { + nodes map[string]struct{} + edges map[string][]string // from -> []to (dependents) + edgeSet map[string]struct{} // "from\x00to" -> exists (dedup guard) + inDegree map[string]int // node -> number of prerequisites +} + +// NewDAG creates an empty DAG. +func NewDAG() *DAG { + return &DAG{ + nodes: make(map[string]struct{}), + edges: make(map[string][]string), + edgeSet: make(map[string]struct{}), + inDegree: make(map[string]int), + } +} + +// AddNode registers a node in the DAG. Duplicate adds are idempotent. +func (d *DAG) AddNode(name string) { + if _, ok := d.nodes[name]; ok { + return + } + + d.nodes[name] = struct{}{} + d.edges[name] = nil + d.inDegree[name] = 0 +} + +// AddEdge adds a directed edge: "to" depends on "from" (from is deployed before to). +// Returns an error if either node is unknown or if a self-loop is requested. +func (d *DAG) AddEdge(from, to string) error { + if from == to { + return fmt.Errorf("self-loop not allowed: %q", from) + } + if _, ok := d.nodes[from]; !ok { + return fmt.Errorf("unknown node %q", from) + } + if _, ok := d.nodes[to]; !ok { + return fmt.Errorf("unknown node %q", to) + } + + key := from + "\x00" + to + if _, exists := d.edgeSet[key]; exists { + return nil + } + + d.edgeSet[key] = struct{}{} + d.edges[from] = append(d.edges[from], to) + d.inDegree[to]++ + + return nil +} + +// GetBatches performs a topological sort using Kahn's algorithm and returns +// the nodes grouped into deployment batches. Each batch contains nodes that +// can be deployed in parallel. Batches are ordered: batch 0 has no prerequisites, +// batch 1 depends only on batch 0, etc. +// +// Returns an error if a cycle is detected, including the names of the nodes +// involved in the cycle. +func (d *DAG) GetBatches() ([][]string, error) { + if len(d.nodes) == 0 { + return nil, nil + } + + inDegree := make(map[string]int, len(d.inDegree)) + maps.Copy(inDegree, d.inDegree) + + var batches [][]string + processed := 0 + + for { + var batch []string + for node := range d.nodes { + if inDegree[node] == 0 { + batch = append(batch, node) + inDegree[node] = -1 + } + } + + if len(batch) == 0 { + break + } + + sort.Strings(batch) + batches = append(batches, batch) + processed += len(batch) + + for _, node := range batch { + for _, dependent := range d.edges[node] { + inDegree[dependent]-- + } + } + } + + if processed == len(d.nodes) { + return batches, nil + } + + cycleNodes := make([]string, 0, len(d.nodes)-processed) + for node := range d.nodes { + if inDegree[node] > 0 { + cycleNodes = append(cycleNodes, node) + } + } + sort.Strings(cycleNodes) + + return nil, fmt.Errorf("cycle detected among nodes: %s", strings.Join(cycleNodes, ", ")) +} + +// Nodes returns a sorted slice of all node names in the DAG. +func (d *DAG) Nodes() []string { + nodes := make([]string, 0, len(d.nodes)) + for node := range d.nodes { + nodes = append(nodes, node) + } + sort.Strings(nodes) + + return nodes +} + +// HasNode reports whether the DAG contains a node with the given name. +func (d *DAG) HasNode(name string) bool { + _, ok := d.nodes[name] + return ok +} diff --git a/pkg/chart/v2/util/dag_test.go b/pkg/chart/v2/util/dag_test.go new file mode 100644 index 000000000..a029abea2 --- /dev/null +++ b/pkg/chart/v2/util/dag_test.go @@ -0,0 +1,232 @@ +/* +Copyright The Helm Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package util + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestDAG_BatchOrdering(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + nodes []string + edges [][2]string + expected [][]string + }{ + { + name: "empty DAG", + expected: nil, + }, + { + name: "single node", + nodes: []string{"a"}, + expected: [][]string{{"a"}}, + }, + { + name: "linear chain", + nodes: []string{"a", "b", "c"}, + edges: [][2]string{{"a", "b"}, {"b", "c"}}, + expected: [][]string{ + {"a"}, + {"b"}, + {"c"}, + }, + }, + { + name: "diamond", + nodes: []string{"a", "b", "c", "d"}, + edges: [][2]string{{"a", "b"}, {"a", "c"}, {"b", "d"}, {"c", "d"}}, + expected: [][]string{ + {"a"}, + {"b", "c"}, + {"d"}, + }, + }, + { + name: "multiple roots", + nodes: []string{"a", "b", "c"}, + edges: [][2]string{{"a", "c"}, {"b", "c"}}, + expected: [][]string{ + {"a", "b"}, + {"c"}, + }, + }, + { + name: "duplicate edge is idempotent", + nodes: []string{"a", "b"}, + edges: [][2]string{{"a", "b"}, {"a", "b"}}, + expected: [][]string{ + {"a"}, + {"b"}, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + dag := buildDAG(t, tt.nodes, tt.edges) + + batches, err := dag.GetBatches() + require.NoError(t, err) + assert.Equal(t, tt.expected, batches) + }) + } +} + +func TestDAG_CycleDetection(t *testing.T) { + t.Parallel() + + dag := buildDAG(t, []string{"a", "b", "c"}, [][2]string{{"a", "b"}, {"b", "c"}, {"c", "a"}}) + + batches, err := dag.GetBatches() + require.Error(t, err) + assert.Nil(t, batches) + assert.ErrorContains(t, err, "cycle") + assert.ErrorContains(t, err, "a") + assert.ErrorContains(t, err, "b") + assert.ErrorContains(t, err, "c") +} + +func TestDAG_SelfLoop(t *testing.T) { + t.Parallel() + + dag := NewDAG() + dag.AddNode("a") + + err := dag.AddEdge("a", "a") + require.Error(t, err) + assert.ErrorContains(t, err, "self-loop") +} + +func TestDAG_UnknownNode(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + from string + to string + match string + }{ + { + name: "missing source node", + from: "missing", + to: "a", + match: "unknown node", + }, + { + name: "missing destination node", + from: "a", + to: "missing", + match: "unknown node", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + dag := NewDAG() + dag.AddNode("a") + + err := dag.AddEdge(tt.from, tt.to) + require.Error(t, err) + assert.ErrorContains(t, err, tt.match) + }) + } +} + +func TestDAG_DisconnectedComponents(t *testing.T) { + t.Parallel() + + dag := buildDAG(t, + []string{"alpha", "beta", "gamma", "delta", "epsilon", "zeta"}, + [][2]string{{"alpha", "gamma"}, {"beta", "delta"}, {"delta", "epsilon"}}, + ) + + batches, err := dag.GetBatches() + require.NoError(t, err) + assert.Equal(t, [][]string{ + {"alpha", "beta", "zeta"}, + {"delta", "gamma"}, + {"epsilon"}, + }, batches) +} + +func TestDAG_LargeGraph(t *testing.T) { + t.Parallel() + + const nodeCount = 128 + + nodes := make([]string, 0, nodeCount) + edges := make([][2]string, 0, nodeCount-1) + for i := range nodeCount { + nodes = append(nodes, fmt.Sprintf("node-%03d", i)) + if i == 0 { + continue + } + edges = append(edges, [2]string{ + fmt.Sprintf("node-%03d", i-1), + fmt.Sprintf("node-%03d", i), + }) + } + + dag := buildDAG(t, nodes, edges) + + batches, err := dag.GetBatches() + require.NoError(t, err) + require.Len(t, batches, nodeCount) + + for i, batch := range batches { + assert.Equal(t, []string{fmt.Sprintf("node-%03d", i)}, batch) + } +} + +func TestDAG_NodeHelpers(t *testing.T) { + t.Parallel() + + dag := NewDAG() + dag.AddNode("charlie") + dag.AddNode("alpha") + dag.AddNode("bravo") + + assert.True(t, dag.HasNode("alpha")) + assert.False(t, dag.HasNode("delta")) + assert.Equal(t, []string{"alpha", "bravo", "charlie"}, dag.Nodes()) +} + +func buildDAG(t *testing.T, nodes []string, edges [][2]string) *DAG { + t.Helper() + + dag := NewDAG() + for _, node := range nodes { + dag.AddNode(node) + } + + for _, edge := range edges { + require.NoError(t, dag.AddEdge(edge[0], edge[1])) + } + + return dag +} diff --git a/pkg/chart/v2/util/subchart_dag.go b/pkg/chart/v2/util/subchart_dag.go new file mode 100644 index 000000000..bcf420218 --- /dev/null +++ b/pkg/chart/v2/util/subchart_dag.go @@ -0,0 +1,129 @@ +/* +Copyright The Helm Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package util + +import ( + "encoding/json" + "fmt" + "strings" + + chart "helm.sh/helm/v4/pkg/chart/v2" +) + +const ( + // AnnotationDependsOnSubcharts is the Chart.yaml annotation key for declaring + // parent-chart dependencies on subcharts. The value is a JSON string array of + // subchart names (or aliases) that must be ready before parent resources are + // installed. + AnnotationDependsOnSubcharts = "helm.sh/depends-on/subcharts" +) + +// BuildSubchartDAG constructs a DAG from a chart's subchart dependency declarations. +// +// Subchart-to-subchart ordering is read from the depends-on field on Chart.yaml +// dependency entries. The helm.sh/depends-on/subcharts metadata annotation is +// validated as the parent chart's dependencies on subcharts; parent resources +// are deployed after subchart batches by the action layer. +// +// Subcharts are keyed by their effective name (alias if set, otherwise name). +// ProcessDependencies must have been called on the chart first so that +// c.Dependencies() reflects the post-processed state (disabled subcharts +// pruned, aliases applied). +func BuildSubchartDAG(c *chart.Chart) (*DAG, error) { + dag := NewDAG() + + if c == nil || c.Metadata == nil { + return dag, nil + } + + // Build the set of subcharts that survived ProcessDependencies. + loaded := make(map[string]bool, len(c.Dependencies())) + for _, sub := range c.Dependencies() { + loaded[sub.Name()] = true + } + + byName := make(map[string]bool, len(c.Metadata.Dependencies)) + for _, dep := range c.Metadata.Dependencies { + if dep == nil { + continue + } + name := effectiveDependencyName(dep) + if loaded[name] { + byName[name] = true + dag.AddNode(name) + } + } + + for _, dep := range c.Metadata.Dependencies { + if dep == nil { + continue + } + name := effectiveDependencyName(dep) + if !byName[name] { + continue + } + + for _, prerequisite := range dep.DependsOn { + if err := addSubchartEdge(dag, byName, name, prerequisite); err != nil { + return nil, err + } + } + } + + if err := validateParentSubchartDependencies(c.Metadata.Annotations[AnnotationDependsOnSubcharts], byName); err != nil { + return nil, err + } + + return dag, nil +} + +func validateParentSubchartDependencies(annotation string, byName map[string]bool) error { + annotation = strings.TrimSpace(annotation) + if annotation == "" { + return nil + } + + var prerequisites []string + if err := json.Unmarshal([]byte(annotation), &prerequisites); err != nil { + return fmt.Errorf("parsing %s annotation as JSON string array: %w", AnnotationDependsOnSubcharts, err) + } + + for _, prerequisite := range prerequisites { + if !byName[prerequisite] { + return fmt.Errorf("annotation %s references unknown or disabled subchart %q", AnnotationDependsOnSubcharts, prerequisite) + } + } + + return nil +} + +func addSubchartEdge(dag *DAG, byName map[string]bool, subchartName, prerequisite string) error { + if !byName[prerequisite] { + return fmt.Errorf("subchart %q depends-on unknown or disabled subchart %q", subchartName, prerequisite) + } + if err := dag.AddEdge(prerequisite, subchartName); err != nil { + return fmt.Errorf("adding sequencing edge %sā†’%s: %w", prerequisite, subchartName, err) + } + return nil +} + +func effectiveDependencyName(dep *chart.Dependency) string { + if dep.Alias != "" { + return dep.Alias + } + return dep.Name +} diff --git a/pkg/chart/v2/util/subchart_dag_test.go b/pkg/chart/v2/util/subchart_dag_test.go new file mode 100644 index 000000000..e4dad24c5 --- /dev/null +++ b/pkg/chart/v2/util/subchart_dag_test.go @@ -0,0 +1,332 @@ +/* +Copyright The Helm Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package util + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + chart "helm.sh/helm/v4/pkg/chart/v2" +) + +func TestBuildSubchartDAG_Empty(t *testing.T) { + t.Parallel() + + batches := batchesForChart(t, newChart("parent")) + assert.Empty(t, batches) +} + +func TestBuildSubchartDAG_NoDependencies(t *testing.T) { + t.Parallel() + + c := newChart("parent", + enabledDependency("nginx"), + enabledDependency("rabbitmq"), + enabledDependency("postgres"), + ) + + assertBatches(t, c, [][]string{{"nginx", "postgres", "rabbitmq"}}) +} + +func TestBuildSubchartDAG_LinearOrder(t *testing.T) { + t.Parallel() + + c := newChart("parent", + enabledDependency("postgres"), + enabledDependency("rabbitmq", "postgres"), + enabledDependency("app", "rabbitmq"), + ) + + assertBatches(t, c, [][]string{{"postgres"}, {"rabbitmq"}, {"app"}}) +} + +func TestBuildSubchartDAG_AliasResolution(t *testing.T) { + t.Parallel() + + c := newChart("parent", + aliasedDependency("postgres", "primary-db"), + enabledDependency("app", "primary-db"), + ) + + assertBatches(t, c, [][]string{{"primary-db"}, {"app"}}) +} + +func TestBuildSubchartDAG_DisabledSubchart(t *testing.T) { + t.Parallel() + + c := newChart("parent", + &chart.Dependency{Name: "cache", Enabled: false}, + enabledDependency("app", "cache"), + ) + + _, err := BuildSubchartDAG(c) + require.Error(t, err) + assert.ErrorContains(t, err, `depends-on unknown or disabled subchart "cache"`) +} + +func TestBuildSubchartDAG_DisabledSubchartNotReferenced(t *testing.T) { + t.Parallel() + + c := newChart("parent", + &chart.Dependency{Name: "cache", Enabled: false}, + enabledDependency("app"), + ) + + assertBatches(t, c, [][]string{{"app"}}) +} + +func TestBuildSubchartDAG_CycleDetection(t *testing.T) { + t.Parallel() + + c := newChart("parent", + enabledDependency("a", "b"), + enabledDependency("b", "c"), + enabledDependency("c", "a"), + ) + + dag, err := BuildSubchartDAG(c) + require.NoError(t, err) + + batches, err := dag.GetBatches() + require.Error(t, err) + assert.Nil(t, batches) + assert.ErrorContains(t, err, "cycle") +} + +func TestBuildSubchartDAG_AnnotationBasedParentDependencies(t *testing.T) { + t.Parallel() + + c := newChart("parent", + enabledDependency("postgres"), + enabledDependency("nginx"), + ) + c.Metadata.Annotations = map[string]string{ + AnnotationDependsOnSubcharts: `["nginx"]`, + } + + assertBatches(t, c, [][]string{{"nginx", "postgres"}}) +} + +func TestBuildSubchartDAG_HIPExample(t *testing.T) { + t.Parallel() + + c := newChart("foo", + enabledDependency("nginx"), + enabledDependency("rabbitmq"), + enabledDependency("bar", "nginx", "rabbitmq"), + ) + c.Metadata.Annotations = map[string]string{ + AnnotationDependsOnSubcharts: `["bar", "rabbitmq"]`, + } + + assertBatches(t, c, [][]string{{"nginx", "rabbitmq"}, {"bar"}}) +} + +func TestBuildSubchartDAG_MixedDeclarations(t *testing.T) { + t.Parallel() + + c := newChart("parent", + enabledDependency("database"), + enabledDependency("api", "database"), + enabledDependency("worker"), + ) + c.Metadata.Annotations = map[string]string{ + AnnotationDependsOnSubcharts: `["worker"]`, + } + + assertBatches(t, c, [][]string{{"database", "worker"}, {"api"}}) +} + +func TestBuildSubchartDAG_InvalidAnnotationJSON(t *testing.T) { + t.Parallel() + + c := newChart("parent", enabledDependency("api")) + c.Metadata.Annotations = map[string]string{ + AnnotationDependsOnSubcharts: `["api",`, + } + + _, err := BuildSubchartDAG(c) + require.Error(t, err) + assert.ErrorContains(t, err, "parsing "+AnnotationDependsOnSubcharts+" annotation") +} + +func TestBuildSubchartDAG_ObjectAnnotationRejected(t *testing.T) { + t.Parallel() + + c := newChart("parent", + enabledDependency("postgres"), + enabledDependency("nginx"), + ) + c.Metadata.Annotations = map[string]string{ + AnnotationDependsOnSubcharts: `{"nginx":["postgres"]}`, + } + + _, err := BuildSubchartDAG(c) + require.Error(t, err) + assert.ErrorContains(t, err, "JSON string array") +} + +func TestBuildSubchartDAG_NonExistentReference(t *testing.T) { + t.Parallel() + + c := newChart("parent", enabledDependency("app", "missing")) + + _, err := BuildSubchartDAG(c) + require.Error(t, err) + assert.ErrorContains(t, err, `depends-on unknown or disabled subchart "missing"`) +} + +func TestBuildSubchartDAG_AnnotationUnknownSubchart(t *testing.T) { + t.Parallel() + + c := newChart("parent", enabledDependency("postgres")) + c.Metadata.Annotations = map[string]string{ + AnnotationDependsOnSubcharts: `["app"]`, + } + + _, err := BuildSubchartDAG(c) + require.Error(t, err) + assert.ErrorContains(t, err, "unknown or disabled subchart") +} + +func TestBuildSubchartDAG_NestedSubcharts(t *testing.T) { + t.Parallel() + + root := newChart("parent", + enabledDependency("database"), + enabledDependency("application", "database"), + ) + nested := newChart("application", + enabledDependency("cache"), + enabledDependency("worker", "cache"), + ) + // Replace auto-stubs with real chart objects so nested DAG validation works. + root.SetDependencies( + &chart.Chart{Metadata: &chart.Metadata{Name: "database"}}, + nested, + ) + + assertBatches(t, root, [][]string{{"database"}, {"application"}}) + assertBatches(t, nested, [][]string{{"cache"}, {"worker"}}) +} + +// TestBuildSubchartDAG_MetadataOnlyNoLoadedDeps locks in the post-rewrite +// contract: when c.Metadata.Dependencies is non-empty but c.Dependencies() +// is empty (e.g., chart loaded but ProcessDependencies disabled everything), +// the DAG should have no nodes and produce no error. +func TestBuildSubchartDAG_MetadataOnlyNoLoadedDeps(t *testing.T) { + t.Parallel() + + c := &chart.Chart{ + Metadata: &chart.Metadata{ + Name: "parent", + Dependencies: []*chart.Dependency{ + {Name: "ghost", Enabled: true}, + }, + }, + } + // Note: no AddDependency call ā€” c.Dependencies() is empty. + + batches := batchesForChart(t, c) + assert.Empty(t, batches, "no loaded deps should yield empty DAG") +} + +// TestBuildSubchartDAG_AnnotationReferencesUnloadedDep verifies that an +// annotation referencing a subchart present in metadata but pruned from +// c.Dependencies() (e.g., disabled by ProcessDependencies) produces an error. +func TestBuildSubchartDAG_AnnotationReferencesUnloadedDep(t *testing.T) { + t.Parallel() + + c := &chart.Chart{ + Metadata: &chart.Metadata{ + Name: "parent", + Dependencies: []*chart.Dependency{ + {Name: "loaded-dep", Enabled: true}, + {Name: "pruned-dep", Enabled: true}, + }, + Annotations: map[string]string{ + AnnotationDependsOnSubcharts: `["pruned-dep"]`, + }, + }, + } + // Only loaded-dep is in c.Dependencies(); pruned-dep is not. + c.AddDependency(&chart.Chart{Metadata: &chart.Metadata{Name: "loaded-dep"}}) + + _, err := BuildSubchartDAG(c) + require.Error(t, err) + assert.ErrorContains(t, err, `unknown or disabled subchart "pruned-dep"`) +} + +func assertBatches(t *testing.T, c *chart.Chart, expected [][]string) { + t.Helper() + assert.Equal(t, expected, batchesForChart(t, c)) +} + +func batchesForChart(t *testing.T, c *chart.Chart) [][]string { + t.Helper() + + dag, err := BuildSubchartDAG(c) + require.NoError(t, err) + + batches, err := dag.GetBatches() + require.NoError(t, err) + + return batches +} + +func newChart(name string, deps ...*chart.Dependency) *chart.Chart { + c := &chart.Chart{ + Metadata: &chart.Metadata{ + Name: name, + Dependencies: deps, + }, + } + // Simulate post-ProcessDependencies state: enabled deps appear in + // c.Dependencies() under their effective name (alias if set). + for _, dep := range deps { + if dep == nil || !dep.Enabled { + continue + } + subName := dep.Alias + if subName == "" { + subName = dep.Name + } + c.AddDependency(&chart.Chart{ + Metadata: &chart.Metadata{Name: subName}, + }) + } + return c +} + +func enabledDependency(name string, dependsOn ...string) *chart.Dependency { + return &chart.Dependency{ + Name: name, + Enabled: true, + DependsOn: dependsOn, + } +} + +func aliasedDependency(name, alias string, dependsOn ...string) *chart.Dependency { + return &chart.Dependency{ + Name: name, + Alias: alias, + Enabled: true, + DependsOn: dependsOn, + } +} diff --git a/pkg/cmd/flags.go b/pkg/cmd/flags.go index 5a220d1ce..2b8db6fe4 100644 --- a/pkg/cmd/flags.go +++ b/pkg/cmd/flags.go @@ -25,6 +25,7 @@ import ( "path/filepath" "sort" "strings" + "time" "github.com/spf13/cobra" "github.com/spf13/pflag" @@ -65,42 +66,95 @@ func AddWaitFlag(cmd *cobra.Command, wait *kube.WaitStrategy) { cmd.Flags().Lookup("wait").NoOptDefVal = string(kube.StatusWatcherStrategy) } -type waitValue kube.WaitStrategy +func AddOrderedWaitFlag(cmd *cobra.Command, wait *kube.WaitStrategy) { + cmd.Flags().Var( + newOrderedWaitValue(kube.HookOnlyStrategy, wait), + "wait", + "wait until resources are ready (up to --timeout). Use '--wait' alone for 'watcher' strategy, or specify one of: 'watcher', 'hookOnly', 'legacy', 'ordered'. Default when flag is omitted: 'hookOnly'.", + ) + cmd.Flags().Lookup("wait").NoOptDefVal = string(kube.StatusWatcherStrategy) +} + +type waitValue struct { + wait *kube.WaitStrategy + allowOrdered bool +} func newWaitValue(defaultValue kube.WaitStrategy, ws *kube.WaitStrategy) *waitValue { + return newConfiguredWaitValue(defaultValue, ws, false) +} + +func newOrderedWaitValue(defaultValue kube.WaitStrategy, ws *kube.WaitStrategy) *waitValue { + return newConfiguredWaitValue(defaultValue, ws, true) +} + +func newConfiguredWaitValue(defaultValue kube.WaitStrategy, ws *kube.WaitStrategy, allowOrdered bool) *waitValue { *ws = defaultValue - return (*waitValue)(ws) + return &waitValue{wait: ws, allowOrdered: allowOrdered} } func (ws *waitValue) String() string { - if ws == nil { + if ws == nil || ws.wait == nil { return "" } - return string(*ws) + return string(*ws.wait) } func (ws *waitValue) Set(s string) error { switch s { case string(kube.StatusWatcherStrategy), string(kube.LegacyStrategy), string(kube.HookOnlyStrategy): - *ws = waitValue(s) + *ws.wait = kube.WaitStrategy(s) + return nil + case string(kube.OrderedWaitStrategy): + if !ws.allowOrdered { + break + } + *ws.wait = kube.WaitStrategy(s) return nil case "true": slog.Warn("--wait=true is deprecated (boolean value) and can be replaced with --wait=watcher") - *ws = waitValue(kube.StatusWatcherStrategy) + *ws.wait = kube.StatusWatcherStrategy return nil case "false": slog.Warn("--wait=false is deprecated (boolean value) and can be replaced with --wait=hookOnly") - *ws = waitValue(kube.HookOnlyStrategy) + *ws.wait = kube.HookOnlyStrategy return nil default: - return fmt.Errorf("invalid wait input %q. Valid inputs are %s, %s, and %s", s, kube.StatusWatcherStrategy, kube.HookOnlyStrategy, kube.LegacyStrategy) } + + return fmt.Errorf("invalid wait input %q. Valid inputs are %s", s, formatWaitInputs(ws.allowOrdered)) } func (ws *waitValue) Type() string { return "WaitStrategy" } +func addReadinessTimeoutFlag(f *pflag.FlagSet, readinessTimeout *time.Duration) { + f.DurationVar(readinessTimeout, "readiness-timeout", time.Minute, "per-batch timeout when --wait=ordered is used; each resource batch must become ready within this duration (must not exceed --timeout). \"Ready\" is determined by kstatus signals for the resource kind (Deployment/StatefulSet/Pod/etc.) or by helm.sh/readiness-success and helm.sh/readiness-failure annotations when set; vanilla Jobs require --wait-for-jobs for the per-batch readiness gate to apply") +} + +func formatWaitInputs(allowOrdered bool) string { + valid := []string{ + string(kube.StatusWatcherStrategy), + string(kube.HookOnlyStrategy), + string(kube.LegacyStrategy), + } + if allowOrdered { + valid = append(valid, string(kube.OrderedWaitStrategy)) + } + + switch len(valid) { + case 0: + return "" + case 1: + return valid[0] + case 2: + return fmt.Sprintf("%s and %s", valid[0], valid[1]) + default: + return fmt.Sprintf("%s, and %s", strings.Join(valid[:len(valid)-1], ", "), valid[len(valid)-1]) + } +} + func addChartPathOptionsFlags(f *pflag.FlagSet, c *action.ChartPathOptions) { f.StringVar(&c.Version, "version", "", "specify a version constraint for the chart version to use. This constraint can be a specific tag (e.g. 1.1.1) or it may reference a valid range (e.g. ^2.0.0). If this is not specified, the latest version is used") f.BoolVar(&c.Verify, "verify", false, "verify the package before using it") diff --git a/pkg/cmd/flags_test.go b/pkg/cmd/flags_test.go index 614970252..9eb728d1b 100644 --- a/pkg/cmd/flags_test.go +++ b/pkg/cmd/flags_test.go @@ -18,6 +18,7 @@ package cmd import ( "fmt" + "io" "testing" "time" @@ -99,6 +100,94 @@ func outputFlagCompletionTest(t *testing.T, cmdName string) { runTestCmd(t, tests) } +func TestWaitFlag(t *testing.T) { + t.Run("install accepts ordered wait", func(t *testing.T) { + cmd := newInstallCmd(&action.Configuration{}, io.Discard) + require.NoError(t, cmd.ParseFlags([]string{"--wait=ordered"})) + require.Equal(t, "ordered", cmd.Flags().Lookup("wait").Value.String()) + }) + + t.Run("upgrade accepts ordered wait", func(t *testing.T) { + cmd := newUpgradeCmd(&action.Configuration{}, io.Discard) + require.NoError(t, cmd.ParseFlags([]string{"--wait=ordered"})) + require.Equal(t, "ordered", cmd.Flags().Lookup("wait").Value.String()) + }) + + t.Run("template accepts ordered wait", func(t *testing.T) { + cmd := newTemplateCmd(&action.Configuration{}, io.Discard) + require.NoError(t, cmd.ParseFlags([]string{"--wait=ordered"})) + require.Equal(t, "ordered", cmd.Flags().Lookup("wait").Value.String()) + }) + + t.Run("rollback accepts ordered wait", func(t *testing.T) { + cmd := newRollbackCmd(&action.Configuration{}, io.Discard) + require.NoError(t, cmd.ParseFlags([]string{"--wait=ordered"})) + require.Equal(t, "ordered", cmd.Flags().Lookup("wait").Value.String()) + }) + + t.Run("uninstall accepts ordered wait", func(t *testing.T) { + cmd := newUninstallCmd(&action.Configuration{}, io.Discard) + require.NoError(t, cmd.ParseFlags([]string{"--wait=ordered"})) + require.Equal(t, "ordered", cmd.Flags().Lookup("wait").Value.String()) + }) +} + +func TestReadinessTimeout(t *testing.T) { + t.Run("install registers readiness-timeout with one minute default", func(t *testing.T) { + cmd := newInstallCmd(&action.Configuration{}, io.Discard) + flag := cmd.Flags().Lookup("readiness-timeout") + require.NotNil(t, flag) + require.Equal(t, "1m0s", flag.DefValue) + require.NoError(t, cmd.ParseFlags([]string{"--readiness-timeout=30s"})) + require.Equal(t, "30s", flag.Value.String()) + }) + + t.Run("upgrade registers readiness-timeout with one minute default", func(t *testing.T) { + cmd := newUpgradeCmd(&action.Configuration{}, io.Discard) + flag := cmd.Flags().Lookup("readiness-timeout") + require.NotNil(t, flag) + require.Equal(t, "1m0s", flag.DefValue) + require.NoError(t, cmd.ParseFlags([]string{"--readiness-timeout=30s"})) + require.Equal(t, "30s", flag.Value.String()) + }) + + t.Run("template and non-applicable commands do not register readiness-timeout", func(t *testing.T) { + require.Nil(t, newTemplateCmd(&action.Configuration{}, io.Discard).Flags().Lookup("readiness-timeout")) + require.Nil(t, newUninstallCmd(&action.Configuration{}, io.Discard).Flags().Lookup("readiness-timeout")) + }) + + t.Run("rollback registers readiness-timeout with one minute default", func(t *testing.T) { + cmd := newRollbackCmd(&action.Configuration{}, io.Discard) + flag := cmd.Flags().Lookup("readiness-timeout") + require.NotNil(t, flag) + require.Equal(t, "1m0s", flag.DefValue) + require.NoError(t, cmd.ParseFlags([]string{"--readiness-timeout=30s"})) + require.Equal(t, "30s", flag.Value.String()) + }) + + t.Run("install rejects readiness-timeout longer than timeout", func(t *testing.T) { + _, _, err := executeActionCommand("install timed-install testdata/testcharts/empty --wait=ordered --timeout 30s --readiness-timeout 60s") + require.Error(t, err) + require.Contains(t, err.Error(), "--readiness-timeout (1m0s) must not exceed --timeout (30s)") + }) + + t.Run("upgrade rejects readiness-timeout longer than timeout", func(t *testing.T) { + releaseName := "timed-upgrade" + relMock, ch, chartPath := prepareMockRelease(t, releaseName) + store := storageFixture() + require.NoError(t, store.Create(relMock(releaseName, 1, ch))) + + _, _, err := executeActionCommandC(store, fmt.Sprintf("upgrade %s '%s' --wait=ordered --timeout 30s --readiness-timeout 60s", releaseName, chartPath)) + require.Error(t, err) + require.Contains(t, err.Error(), "--readiness-timeout (1m0s) must not exceed --timeout (30s)") + }) + + t.Run("readiness-timeout is ignored without ordered wait", func(t *testing.T) { + _, _, err := executeActionCommand("install plain-install testdata/testcharts/empty --readiness-timeout 30s") + require.NoError(t, err) + }) +} + func TestPostRendererFlagSetOnce(t *testing.T) { cfg := action.Configuration{} client := action.NewInstall(&cfg) diff --git a/pkg/cmd/install.go b/pkg/cmd/install.go index 67e2a9fab..03632abdc 100644 --- a/pkg/cmd/install.go +++ b/pkg/cmd/install.go @@ -173,6 +173,7 @@ func newInstallCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { f := cmd.Flags() addInstallFlags(cmd, f, client, valueOpts) + addReadinessTimeoutFlag(f, &client.ReadinessTimeout) // hide-secret is not available in all places the install flags are used so // it is added separately f.BoolVar(&client.HideSecret, "hide-secret", false, "hide Kubernetes Secrets when also using the --dry-run flag") @@ -232,7 +233,7 @@ func addInstallFlags(cmd *cobra.Command, f *pflag.FlagSet, client *action.Instal addValueOptionsFlags(f, valueOpts) addChartPathOptionsFlags(f, &client.ChartPathOptions) - AddWaitFlag(cmd, &client.WaitStrategy) + AddOrderedWaitFlag(cmd, &client.WaitStrategy) cmd.MarkFlagsMutuallyExclusive("force-replace", "force-conflicts") cmd.MarkFlagsMutuallyExclusive("force", "force-conflicts") diff --git a/pkg/cmd/rollback.go b/pkg/cmd/rollback.go index 01d8b1866..e23ba6287 100644 --- a/pkg/cmd/rollback.go +++ b/pkg/cmd/rollback.go @@ -93,7 +93,8 @@ func newRollbackCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { f.BoolVar(&client.CleanupOnFail, "cleanup-on-fail", false, "allow deletion of new resources created in this rollback when rollback fails") f.IntVar(&client.MaxHistory, "history-max", settings.MaxHistory, "limit the maximum number of revisions saved per release. Use 0 for no limit") addDryRunFlag(cmd) - AddWaitFlag(cmd, &client.WaitStrategy) + AddOrderedWaitFlag(cmd, &client.WaitStrategy) + addReadinessTimeoutFlag(f, &client.ReadinessTimeout) cmd.MarkFlagsMutuallyExclusive("force-replace", "force-conflicts") cmd.MarkFlagsMutuallyExclusive("force", "force-conflicts") diff --git a/pkg/cmd/template.go b/pkg/cmd/template.go index 047fd60df..ca9dd69fa 100644 --- a/pkg/cmd/template.go +++ b/pkg/cmd/template.go @@ -22,7 +22,9 @@ import ( "fmt" "io" "io/fs" + "log/slog" "os" + "path" "path/filepath" "regexp" "slices" @@ -32,11 +34,15 @@ import ( release "helm.sh/helm/v4/pkg/release/v1" "github.com/spf13/cobra" + "sigs.k8s.io/yaml" "helm.sh/helm/v4/pkg/action" "helm.sh/helm/v4/pkg/chart/common" + chart "helm.sh/helm/v4/pkg/chart/v2" + chartutil "helm.sh/helm/v4/pkg/chart/v2/util" "helm.sh/helm/v4/pkg/cli/values" "helm.sh/helm/v4/pkg/cmd/require" + "helm.sh/helm/v4/pkg/kube" releaseutil "helm.sh/helm/v4/pkg/release/v1/util" ) @@ -105,6 +111,7 @@ func newTemplateCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { client.Replace = true // Skip the name check client.APIVersions = common.VersionSet(extraAPIs) client.IncludeCRDs = includeCrds + orderedTemplateOutput := client.WaitStrategy == kube.OrderedWaitStrategy && len(showFiles) == 0 && client.OutputDir == "" rel, err := runInstall(args, client, valueOpts, out) if err != nil && !settings.Debug { @@ -117,84 +124,97 @@ func newTemplateCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { // We ignore a potential error here because, when the --debug flag was specified, // we always want to print the YAML, even if it is not valid. The error is still returned afterwards. if rel != nil { - var manifests bytes.Buffer - fmt.Fprintln(&manifests, strings.TrimSpace(rel.Manifest)) - if !client.DisableHooks { - fileWritten := make(map[string]bool) - for _, m := range rel.Hooks { - if skipTests && isTestHook(m) { - continue - } - if client.OutputDir == "" { - fmt.Fprintf(&manifests, "---\n# Source: %s\n%s\n", m.Path, m.Manifest) - } else { - newDir := client.OutputDir - if client.UseReleaseName { - newDir = filepath.Join(client.OutputDir, client.ReleaseName) + if orderedTemplateOutput { + if err := renderOrderedTemplate(rel.Chart, strings.TrimSpace(rel.Manifest), out); err != nil { + return err + } + if !client.DisableHooks { + for _, m := range rel.Hooks { + if skipTests && isTestHook(m) { + continue } - _, err := os.Stat(filepath.Join(newDir, m.Path)) - if err == nil { - fileWritten[m.Path] = true + fmt.Fprintf(out, "---\n# Source: %s\n%s\n", m.Path, m.Manifest) + } + } + } else { + var manifests bytes.Buffer + fmt.Fprintln(&manifests, strings.TrimSpace(rel.Manifest)) + if !client.DisableHooks { + fileWritten := make(map[string]bool) + for _, m := range rel.Hooks { + if skipTests && isTestHook(m) { + continue } - - err = writeToFile(newDir, m.Path, m.Manifest, fileWritten[m.Path]) - if err != nil { - return err + if client.OutputDir == "" { + fmt.Fprintf(&manifests, "---\n# Source: %s\n%s\n", m.Path, m.Manifest) + } else { + newDir := client.OutputDir + if client.UseReleaseName { + newDir = filepath.Join(client.OutputDir, client.ReleaseName) + } + _, err := os.Stat(filepath.Join(newDir, m.Path)) + if err == nil { + fileWritten[m.Path] = true + } + + err = writeToFile(newDir, m.Path, m.Manifest, fileWritten[m.Path]) + if err != nil { + return err + } } } - } - } - // if we have a list of files to render, then check that each of the - // provided files exists in the chart. - if len(showFiles) > 0 { - // This is necessary to ensure consistent manifest ordering when using --show-only - // with globs or directory names. - splitManifests := releaseutil.SplitManifests(manifests.String()) - manifestsKeys := make([]string, 0, len(splitManifests)) - for k := range splitManifests { - manifestsKeys = append(manifestsKeys, k) - } - sort.Sort(releaseutil.BySplitManifestsOrder(manifestsKeys)) - - manifestNameRegex := regexp.MustCompile("# Source: [^/]+/(.+)") - var manifestsToRender []string - for _, f := range showFiles { - missing := true - // Use linux-style filepath separators to unify user's input path - f = filepath.ToSlash(f) - for _, manifestKey := range manifestsKeys { - manifest := splitManifests[manifestKey] - submatch := manifestNameRegex.FindStringSubmatch(manifest) - if len(submatch) == 0 { - continue + // if we have a list of files to render, then check that each of the + // provided files exists in the chart. + if len(showFiles) > 0 { + // This is necessary to ensure consistent manifest ordering when using --show-only + // with globs or directory names. + splitManifests := releaseutil.SplitManifests(manifests.String()) + manifestsKeys := make([]string, 0, len(splitManifests)) + for k := range splitManifests { + manifestsKeys = append(manifestsKeys, k) + } + sort.Sort(releaseutil.BySplitManifestsOrder(manifestsKeys)) + + manifestNameRegex := regexp.MustCompile("# Source: [^/]+/(.+)") + var manifestsToRender []string + for _, f := range showFiles { + missing := true + // Use linux-style filepath separators to unify user's input path + f = filepath.ToSlash(f) + for _, manifestKey := range manifestsKeys { + manifest := splitManifests[manifestKey] + submatch := manifestNameRegex.FindStringSubmatch(manifest) + if len(submatch) == 0 { + continue + } + manifestName := submatch[1] + // manifest.Name is rendered using linux-style filepath separators on Windows as + // well as macOS/linux. + manifestPathSplit := strings.Split(manifestName, "/") + // manifest.Path is connected using linux-style filepath separators on Windows as + // well as macOS/linux + manifestPath := strings.Join(manifestPathSplit, "/") + + // if the filepath provided matches a manifest path in the + // chart, render that manifest + if matched, _ := filepath.Match(f, manifestPath); !matched { + continue + } + manifestsToRender = append(manifestsToRender, manifest) + missing = false } - manifestName := submatch[1] - // manifest.Name is rendered using linux-style filepath separators on Windows as - // well as macOS/linux. - manifestPathSplit := strings.Split(manifestName, "/") - // manifest.Path is connected using linux-style filepath separators on Windows as - // well as macOS/linux - manifestPath := strings.Join(manifestPathSplit, "/") - - // if the filepath provided matches a manifest path in the - // chart, render that manifest - if matched, _ := filepath.Match(f, manifestPath); !matched { - continue + if missing { + return fmt.Errorf("could not find template %s in chart", f) } - manifestsToRender = append(manifestsToRender, manifest) - missing = false } - if missing { - return fmt.Errorf("could not find template %s in chart", f) + for _, m := range manifestsToRender { + fmt.Fprintf(out, "---\n%s\n", m) } + } else { + fmt.Fprintf(out, "%s", manifests.String()) } - for _, m := range manifestsToRender { - fmt.Fprintf(out, "---\n%s\n", m) - } - } else { - fmt.Fprintf(out, "%s", manifests.String()) } } @@ -225,6 +245,233 @@ func newTemplateCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { return cmd } +func renderOrderedTemplate(chrt *chart.Chart, manifest string, out io.Writer) error { + if manifest == "" { + return nil + } + + sortedManifests, err := parseTemplateManifests(manifest) + if err != nil { + slog.Warn("Unable to render ordered template output; falling back to flat manifest output", "chart", chrt.Name(), "error", err) + if _, writeErr := io.WriteString(out, manifest); writeErr != nil { + return fmt.Errorf("writing fallback manifest output: %w", writeErr) + } + return nil + } + + return renderOrderedChartLevel(chrt, sortedManifests, chrt.Name(), out) +} + +func parseTemplateManifests(manifest string) ([]releaseutil.Manifest, error) { + rawManifests := releaseutil.SplitManifests(manifest) + keys := make([]string, 0, len(rawManifests)) + for key := range rawManifests { + keys = append(keys, key) + } + sort.Sort(releaseutil.BySplitManifestsOrder(keys)) + + manifests := make([]releaseutil.Manifest, 0, len(keys)) + for _, key := range keys { + content := rawManifests[key] + name := manifestSourcePath(content) + if name == "" { + name = key + } + + var head releaseutil.SimpleHead + if err := yaml.Unmarshal([]byte(content), &head); err != nil { + return nil, fmt.Errorf("YAML parse error on %s: %w", name, err) + } + + manifests = append(manifests, releaseutil.Manifest{ + Name: name, + Content: content, + Head: &head, + }) + } + + return manifests, nil +} + +func manifestSourcePath(content string) string { + for line := range strings.SplitSeq(content, "\n") { + line = strings.TrimSpace(line) + if source, ok := strings.CutPrefix(line, "# Source: "); ok { + return source + } + } + + return "" +} + +func renderOrderedChartLevel(chrt *chart.Chart, manifests []releaseutil.Manifest, chartPath string, out io.Writer) error { + if len(manifests) == 0 { + return nil + } + + grouped := groupManifestsByChartPath(manifests, chartPath) + renderedSubcharts := make(map[string]struct{}) + + dag, err := chartutil.BuildSubchartDAG(chrt) + if err != nil { + return fmt.Errorf("building subchart DAG for %s: %w", chartPath, err) + } + + batches, err := dag.GetBatches() + if err != nil { + return fmt.Errorf("getting subchart batches for %s: %w", chartPath, err) + } + + for _, batch := range batches { + for _, subchartName := range batch { + if err := renderOrderedSubchart(chrt, chartPath, subchartName, grouped[subchartName], out); err != nil { + return err + } + renderedSubcharts[subchartName] = struct{}{} + } + } + + var remainingSubcharts []string + for subchartName := range grouped { + if subchartName == "" { + continue + } + if _, ok := renderedSubcharts[subchartName]; ok { + continue + } + remainingSubcharts = append(remainingSubcharts, subchartName) + } + sort.Strings(remainingSubcharts) + + for _, subchartName := range remainingSubcharts { + if err := renderOrderedSubchart(chrt, chartPath, subchartName, grouped[subchartName], out); err != nil { + return err + } + } + + return renderOrderedResourceGroups(grouped[""], chartPath, out) +} + +func renderOrderedSubchart(chrt *chart.Chart, chartPath, subchartName string, manifests []releaseutil.Manifest, out io.Writer) error { + if len(manifests) == 0 { + return nil + } + + subchartPath := path.Join(chartPath, subchartName) + subChart := findTemplateSubchart(chrt, subchartName) + if subChart == nil { + slog.Warn("subchart not found in chart dependencies during template rendering; falling back to flat ordered output", "subchart", subchartName) + return renderOrderedResourceGroups(manifests, subchartPath, out) + } + + return renderOrderedChartLevel(subChart, manifests, subchartPath, out) +} + +func renderOrderedResourceGroups(manifests []releaseutil.Manifest, chartPath string, out io.Writer) error { + if len(manifests) == 0 { + return nil + } + + result, warnings, err := releaseutil.ParseResourceGroups(manifests) + if err != nil { + return fmt.Errorf("parsing resource groups for %s: %w", chartPath, err) + } + for _, warning := range warnings { + slog.Warn("resource-group annotation warning during template rendering", "chartPath", chartPath, "warning", warning) + } + + if len(result.Groups) > 0 { + dag, err := releaseutil.BuildResourceGroupDAG(result) + if err != nil { + return fmt.Errorf("building resource-group DAG for %s: %w", chartPath, err) + } + batches, err := dag.GetBatches() + if err != nil { + return fmt.Errorf("getting resource-group batches for %s: %w", chartPath, err) + } + + for _, batch := range batches { + for _, groupName := range batch { + fmt.Fprintf(out, "## START resource-group: %s %s\n", chartPath, groupName) + for _, manifest := range result.Groups[groupName] { + fmt.Fprintf(out, "---\n%s\n", manifest.Content) + } + fmt.Fprintf(out, "## END resource-group: %s %s\n", chartPath, groupName) + } + } + } + + for _, manifest := range result.Unsequenced { + fmt.Fprintf(out, "---\n%s\n", manifest.Content) + } + + return nil +} + +func groupManifestsByChartPath(manifests []releaseutil.Manifest, chartPath string) map[string][]releaseutil.Manifest { + result := make(map[string][]releaseutil.Manifest) + chartsPrefix := chartManifestPrefix(chartPath) + "/charts/" + + for _, manifest := range manifests { + if !strings.HasPrefix(manifest.Name, chartsPrefix) { + result[""] = append(result[""], manifest) + continue + } + + rest := manifest.Name[len(chartsPrefix):] + subchartName, _, ok := strings.Cut(rest, "/") + if !ok { + result[""] = append(result[""], manifest) + continue + } + + result[subchartName] = append(result[subchartName], manifest) + } + + return result +} + +func chartManifestPrefix(chartPath string) string { + parts := strings.Split(chartPath, "/") + if len(parts) == 0 { + return chartPath + } + + prefix := parts[0] + for _, part := range parts[1:] { + prefix = path.Join(prefix, "charts", part) + } + + return prefix +} + +func findTemplateSubchart(chrt *chart.Chart, nameOrAlias string) *chart.Chart { + if chrt == nil || chrt.Metadata == nil { + return nil + } + + aliases := make(map[string]string, len(chrt.Metadata.Dependencies)) + for _, dep := range chrt.Metadata.Dependencies { + effectiveName := dep.Name + if dep.Alias != "" { + effectiveName = dep.Alias + } + aliases[dep.Name] = effectiveName + } + + for _, dep := range chrt.Dependencies() { + effectiveName := dep.Name() + if alias, ok := aliases[dep.Name()]; ok { + effectiveName = alias + } + if effectiveName == nameOrAlias || dep.Name() == nameOrAlias { + return dep + } + } + + return nil +} + func isTestHook(h *release.Hook) bool { return slices.Contains(h.Events, release.HookTest) } diff --git a/pkg/cmd/template_test.go b/pkg/cmd/template_test.go index 7391781f6..6e08ac6a3 100644 --- a/pkg/cmd/template_test.go +++ b/pkg/cmd/template_test.go @@ -20,6 +20,8 @@ import ( "fmt" "path/filepath" "testing" + + "github.com/stretchr/testify/require" ) var chartPath = "testdata/testcharts/subchart" @@ -166,10 +168,23 @@ func TestTemplateCmd(t *testing.T) { cmd: fmt.Sprintf("template '%s' -f %s/extra_values.yaml", chartPath, chartPath), golden: "output/template-subchart-cm-set-file.txt", }, + { + name: "template with ordered wait strategy shows resource group delimiters", + cmd: "template --wait=ordered 'testdata/testcharts/sequenced-chart'", + golden: "output/template-ordered-delimiters.txt", + }, } runTestCmd(t, tests) } +func TestTemplateWithoutOrderedWaitHasNoDelimiters(t *testing.T) { + _, out, err := executeActionCommand("template 'testdata/testcharts/sequenced-chart'") + require.NoError(t, err) + require.NotContains(t, out, "## START resource-group:") + require.NotContains(t, out, "## END resource-group:") + require.Contains(t, out, "# Source: sequenced-chart/charts/worker/templates/aa-worker-configmap.yaml") +} + func TestTemplateVersionCompletion(t *testing.T) { repoFile := "testdata/helmhome/helm/repositories.yaml" repoCache := "testdata/helmhome/helm/repository" diff --git a/pkg/cmd/testdata/output/template-ordered-delimiters.txt b/pkg/cmd/testdata/output/template-ordered-delimiters.txt new file mode 100644 index 000000000..fad44551d --- /dev/null +++ b/pkg/cmd/testdata/output/template-ordered-delimiters.txt @@ -0,0 +1,45 @@ +## START resource-group: sequenced-chart/worker bootstrap +--- +# Source: sequenced-chart/charts/worker/templates/aa-worker-configmap.yaml +apiVersion: v1 +kind: ConfigMap +metadata: + name: worker-bootstrap + annotations: + helm.sh/resource-group: bootstrap +data: + worker: ready +## END resource-group: sequenced-chart/worker bootstrap +## START resource-group: sequenced-chart databases +--- +# Source: sequenced-chart/templates/aa-databases-configmap.yaml +apiVersion: v1 +kind: ConfigMap +metadata: + name: database-config + annotations: + helm.sh/resource-group: databases +data: + host: localhost +## END resource-group: sequenced-chart databases +## START resource-group: sequenced-chart app +--- +# Source: sequenced-chart/templates/bb-app-configmap.yaml +apiVersion: v1 +kind: ConfigMap +metadata: + name: app-config + annotations: + helm.sh/resource-group: app + helm.sh/depends-on/resource-groups: '["databases"]' +data: + db_host: localhost +## END resource-group: sequenced-chart app +--- +# Source: sequenced-chart/templates/cc-unsequenced-configmap.yaml +apiVersion: v1 +kind: ConfigMap +metadata: + name: unsequenced-config +data: + key: value diff --git a/pkg/cmd/testdata/testcharts/sequenced-chart/Chart.yaml b/pkg/cmd/testdata/testcharts/sequenced-chart/Chart.yaml new file mode 100644 index 000000000..b1bde1d8c --- /dev/null +++ b/pkg/cmd/testdata/testcharts/sequenced-chart/Chart.yaml @@ -0,0 +1,9 @@ +apiVersion: v2 +name: sequenced-chart +description: A test chart with ordered template output +type: application +version: 0.1.0 +dependencies: + - name: worker + version: 0.1.0 + repository: file://charts/worker diff --git a/pkg/cmd/testdata/testcharts/sequenced-chart/charts/worker/Chart.yaml b/pkg/cmd/testdata/testcharts/sequenced-chart/charts/worker/Chart.yaml new file mode 100644 index 000000000..8f3807ae6 --- /dev/null +++ b/pkg/cmd/testdata/testcharts/sequenced-chart/charts/worker/Chart.yaml @@ -0,0 +1,5 @@ +apiVersion: v2 +name: worker +description: A subchart used to test ordered template output +type: application +version: 0.1.0 diff --git a/pkg/cmd/testdata/testcharts/sequenced-chart/charts/worker/templates/aa-worker-configmap.yaml b/pkg/cmd/testdata/testcharts/sequenced-chart/charts/worker/templates/aa-worker-configmap.yaml new file mode 100644 index 000000000..263ac9e22 --- /dev/null +++ b/pkg/cmd/testdata/testcharts/sequenced-chart/charts/worker/templates/aa-worker-configmap.yaml @@ -0,0 +1,8 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: worker-bootstrap + annotations: + helm.sh/resource-group: bootstrap +data: + worker: ready diff --git a/pkg/cmd/testdata/testcharts/sequenced-chart/templates/aa-databases-configmap.yaml b/pkg/cmd/testdata/testcharts/sequenced-chart/templates/aa-databases-configmap.yaml new file mode 100644 index 000000000..8ea614d38 --- /dev/null +++ b/pkg/cmd/testdata/testcharts/sequenced-chart/templates/aa-databases-configmap.yaml @@ -0,0 +1,8 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: database-config + annotations: + helm.sh/resource-group: databases +data: + host: localhost diff --git a/pkg/cmd/testdata/testcharts/sequenced-chart/templates/bb-app-configmap.yaml b/pkg/cmd/testdata/testcharts/sequenced-chart/templates/bb-app-configmap.yaml new file mode 100644 index 000000000..0472c0318 --- /dev/null +++ b/pkg/cmd/testdata/testcharts/sequenced-chart/templates/bb-app-configmap.yaml @@ -0,0 +1,9 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: app-config + annotations: + helm.sh/resource-group: app + helm.sh/depends-on/resource-groups: '["databases"]' +data: + db_host: localhost diff --git a/pkg/cmd/testdata/testcharts/sequenced-chart/templates/cc-unsequenced-configmap.yaml b/pkg/cmd/testdata/testcharts/sequenced-chart/templates/cc-unsequenced-configmap.yaml new file mode 100644 index 000000000..03454569c --- /dev/null +++ b/pkg/cmd/testdata/testcharts/sequenced-chart/templates/cc-unsequenced-configmap.yaml @@ -0,0 +1,6 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: unsequenced-config +data: + key: value diff --git a/pkg/cmd/uninstall.go b/pkg/cmd/uninstall.go index 49f7bd19d..402a0df9a 100644 --- a/pkg/cmd/uninstall.go +++ b/pkg/cmd/uninstall.go @@ -82,7 +82,7 @@ func newUninstallCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { f.StringVar(&client.DeletionPropagation, "cascade", "background", "Must be \"background\", \"orphan\", or \"foreground\". Selects the deletion cascading strategy for the dependents. Defaults to background. Use \"foreground\" with --wait to ensure resources with finalizers are fully deleted before returning.") f.DurationVar(&client.Timeout, "timeout", 300*time.Second, "time to wait for any individual Kubernetes operation (like Jobs for hooks)") f.StringVar(&client.Description, "description", "", "add a custom description") - AddWaitFlag(cmd, &client.WaitStrategy) + AddOrderedWaitFlag(cmd, &client.WaitStrategy) return cmd } diff --git a/pkg/cmd/uninstall_test.go b/pkg/cmd/uninstall_test.go index ce436e68c..77f8cdb50 100644 --- a/pkg/cmd/uninstall_test.go +++ b/pkg/cmd/uninstall_test.go @@ -73,6 +73,12 @@ func TestUninstall(t *testing.T) { golden: "output/uninstall-wait.txt", rels: []*release.Release{release.Mock(&release.MockReleaseOptions{Name: "aeneas"})}, }, + { + name: "wait ordered", + cmd: "uninstall aeneas --wait=ordered", + golden: "output/uninstall-wait.txt", + rels: []*release.Release{release.Mock(&release.MockReleaseOptions{Name: "aeneas"})}, + }, { name: "uninstall without release", cmd: "uninstall", diff --git a/pkg/cmd/upgrade.go b/pkg/cmd/upgrade.go index 43e19ab22..a7de22486 100644 --- a/pkg/cmd/upgrade.go +++ b/pkg/cmd/upgrade.go @@ -305,7 +305,8 @@ func newUpgradeCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { addValueOptionsFlags(f, valueOpts) bindOutputFlag(cmd, &outfmt) bindPostRenderFlag(cmd, &client.PostRenderer, settings) - AddWaitFlag(cmd, &client.WaitStrategy) + AddOrderedWaitFlag(cmd, &client.WaitStrategy) + addReadinessTimeoutFlag(f, &client.ReadinessTimeout) cmd.MarkFlagsMutuallyExclusive("force-replace", "force-conflicts") cmd.MarkFlagsMutuallyExclusive("force", "force-conflicts") diff --git a/pkg/kube/client.go b/pkg/kube/client.go index c955e8875..a5f6cdde5 100644 --- a/pkg/kube/client.go +++ b/pkg/kube/client.go @@ -32,6 +32,7 @@ import ( "sync" jsonpatch "github.com/evanphx/json-patch/v5" + "github.com/fluxcd/cli-utils/pkg/kstatus/polling/engine" v1 "k8s.io/api/core/v1" apiextv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" apiextv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" @@ -117,6 +118,10 @@ const ( // HookOnlyStrategy: wait only for hook Pods/Jobs to complete; does not wait for general chart resources. HookOnlyStrategy WaitStrategy = "hookOnly" + + // OrderedWaitStrategy: action-layer sequencing uses ordered batches while the kube layer + // reuses status watching for per-batch readiness checks. + OrderedWaitStrategy WaitStrategy = "ordered" ) type FieldValidationDirective string @@ -166,6 +171,10 @@ func (c *Client) newStatusWatcher(opts ...WaitOption) (*statusWaiter, error) { if waitContext == nil { waitContext = c.WaitContext } + readers := append([]engine.StatusReader(nil), o.statusReaders...) + if o.enableCustomReadinessStatusReader { + readers = append([]engine.StatusReader{newCustomReadinessStatusReader()}, readers...) + } sw := &statusWaiter{ restMapper: restMapper, client: dynamicClient, @@ -174,7 +183,7 @@ func (c *Client) newStatusWatcher(opts ...WaitOption) (*statusWaiter, error) { waitCtx: o.waitCtx, waitWithJobsCtx: o.waitWithJobsCtx, waitForDeleteCtx: o.waitForDeleteCtx, - readers: o.statusReaders, + readers: readers, } sw.SetLogger(c.Logger().Handler()) return sw, nil @@ -200,10 +209,12 @@ func (c *Client) GetWaiterWithOptions(strategy WaitStrategy, opts ...WaitOption) return nil, err } return &hookOnlyWaiter{sw: sw}, nil + case OrderedWaitStrategy: + return c.newStatusWatcher(opts...) case "": - return nil, errors.New("wait strategy not set. Choose one of: " + string(StatusWatcherStrategy) + ", " + string(HookOnlyStrategy) + ", " + string(LegacyStrategy)) + return nil, errors.New("wait strategy not set. Choose one of: " + string(StatusWatcherStrategy) + ", " + string(HookOnlyStrategy) + ", " + string(LegacyStrategy) + ", " + string(OrderedWaitStrategy)) default: - return nil, errors.New("unknown wait strategy (s" + string(strategy) + "). Valid values are: " + string(StatusWatcherStrategy) + ", " + string(HookOnlyStrategy) + ", " + string(LegacyStrategy)) + return nil, errors.New("unknown wait strategy (" + string(strategy) + "). Valid values are: " + string(StatusWatcherStrategy) + ", " + string(HookOnlyStrategy) + ", " + string(LegacyStrategy) + ", " + string(OrderedWaitStrategy)) } } diff --git a/pkg/kube/client_wait_strategy_test.go b/pkg/kube/client_wait_strategy_test.go new file mode 100644 index 000000000..390a4ed9a --- /dev/null +++ b/pkg/kube/client_wait_strategy_test.go @@ -0,0 +1,41 @@ +/* +Copyright The Helm Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package kube + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestClientGetWaiterWithOptions_OrderedWaitStrategy(t *testing.T) { + client := newTestClient(t) + + waiter, err := client.GetWaiterWithOptions(OrderedWaitStrategy) + require.NoError(t, err) + assert.IsType(t, &statusWaiter{}, waiter) +} + +func TestClientGetWaiterWithOptions_UnknownStrategyListsOrdered(t *testing.T) { + client := newTestClient(t) + + waiter, err := client.GetWaiterWithOptions(WaitStrategy("bogus")) + require.Error(t, err) + assert.Nil(t, waiter) + assert.Contains(t, err.Error(), string(OrderedWaitStrategy)) +} diff --git a/pkg/kube/custom_readiness_status_reader.go b/pkg/kube/custom_readiness_status_reader.go new file mode 100644 index 000000000..9bc5a84a3 --- /dev/null +++ b/pkg/kube/custom_readiness_status_reader.go @@ -0,0 +1,114 @@ +/* +Copyright The Helm Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package kube // import "helm.sh/helm/v4/pkg/kube" + +import ( + "context" + "fmt" + + "github.com/fluxcd/cli-utils/pkg/kstatus/polling/engine" + "github.com/fluxcd/cli-utils/pkg/kstatus/polling/event" + "github.com/fluxcd/cli-utils/pkg/kstatus/status" + "github.com/fluxcd/cli-utils/pkg/object" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +type customReadinessStatusReader struct{} + +func newCustomReadinessStatusReader() engine.StatusReader { + return &customReadinessStatusReader{} +} + +func (*customReadinessStatusReader) Supports(schema.GroupKind) bool { + return true +} + +func (r *customReadinessStatusReader) ReadStatus(ctx context.Context, reader engine.ClusterReader, resource object.ObjMetadata) (*event.ResourceStatus, error) { + u := &unstructured.Unstructured{} + u.SetGroupVersionKind(schema.GroupVersionKind{ + Group: resource.GroupKind.Group, + Kind: resource.GroupKind.Kind, + }) + if err := reader.Get(ctx, client.ObjectKey{Namespace: resource.Namespace, Name: resource.Name}, u); err != nil { + return nil, err + } + return r.readStatus(resource, u) +} + +func (r *customReadinessStatusReader) ReadStatusForObject(_ context.Context, _ engine.ClusterReader, resource *unstructured.Unstructured) (*event.ResourceStatus, error) { + identifier, err := object.RuntimeToObjMeta(resource) + if err != nil { + return nil, err + } + return r.readStatus(identifier, resource) +} + +func (r *customReadinessStatusReader) readStatus(identifier object.ObjMetadata, resource *unstructured.Unstructured) (*event.ResourceStatus, error) { + annotations := resource.GetAnnotations() + + successExprs, err := parseReadinessExpressions(annotations[AnnotationReadinessSuccess]) + if err != nil { + return nil, fmt.Errorf("parsing %s for %s/%s: %w", AnnotationReadinessSuccess, identifier.Namespace, identifier.Name, err) + } + failureExprs, err := parseReadinessExpressions(annotations[AnnotationReadinessFailure]) + if err != nil { + return nil, fmt.Errorf("parsing %s for %s/%s: %w", AnnotationReadinessFailure, identifier.Namespace, identifier.Name, err) + } + + result, useKstatus, err := EvaluateCustomReadiness(resource, successExprs, failureExprs) + if err != nil { + return nil, err + } + if useKstatus { + return computeKstatusStatus(identifier, resource) + } + + switch result { + case ReadinessReady: + return &event.ResourceStatus{ + Identifier: identifier, + Status: status.CurrentStatus, + Message: "custom readiness conditions met", + }, nil + case ReadinessFailed: + return &event.ResourceStatus{ + Identifier: identifier, + Status: status.FailedStatus, + Message: "custom readiness failure condition met", + }, nil + default: + return &event.ResourceStatus{ + Identifier: identifier, + Status: status.InProgressStatus, + Message: "waiting for custom readiness conditions", + }, nil + } +} + +func computeKstatusStatus(identifier object.ObjMetadata, resource *unstructured.Unstructured) (*event.ResourceStatus, error) { + result, err := status.Compute(resource) + if err != nil { + return nil, err + } + return &event.ResourceStatus{ + Identifier: identifier, + Status: result.Status, + Message: result.Message, + }, nil +} diff --git a/pkg/kube/custom_readiness_status_reader_test.go b/pkg/kube/custom_readiness_status_reader_test.go new file mode 100644 index 000000000..9242b1d6a --- /dev/null +++ b/pkg/kube/custom_readiness_status_reader_test.go @@ -0,0 +1,142 @@ +/* +Copyright The Helm Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package kube + +import ( + "context" + "testing" + "time" + + "github.com/fluxcd/cli-utils/pkg/kstatus/polling/engine" + "github.com/fluxcd/cli-utils/pkg/kstatus/status" + "github.com/fluxcd/cli-utils/pkg/testutil" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/client-go/dynamic/fake" + "k8s.io/client-go/kubernetes/scheme" +) + +func TestStatusWaitWithCustomReadinessReader(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + manifest string + expectErrStrs []string + }{ + { + name: "custom readiness makes resource current", + manifest: ` +apiVersion: v1 +kind: ConfigMap +metadata: + name: ready-config + namespace: default + annotations: + helm.sh/readiness-success: '["{.phase} == \"Ready\""]' + helm.sh/readiness-failure: '["{.phase} == \"Failed\""]' +status: + phase: Ready +`, + }, + { + name: "custom readiness failure stops wait", + manifest: ` +apiVersion: v1 +kind: ConfigMap +metadata: + name: failed-config + namespace: default + annotations: + helm.sh/readiness-success: '["{.phase} == \"Ready\""]' + helm.sh/readiness-failure: '["{.phase} == \"Failed\""]' +status: + phase: Failed +`, + expectErrStrs: []string{"resource ConfigMap/default/failed-config not ready. status: Failed"}, + }, + { + name: "resources without annotations fall back to kstatus", + manifest: ` +apiVersion: v1 +kind: ConfigMap +metadata: + name: current-config + namespace: default +`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + c := newTestClient(t) + fakeClient := fake.NewSimpleDynamicClient(scheme.Scheme) + fakeMapper := testutil.NewFakeRESTMapper(v1.SchemeGroupVersion.WithKind("ConfigMap")) + waiter := statusWaiter{ + client: fakeClient, + restMapper: fakeMapper, + readers: []engine.StatusReader{newCustomReadinessStatusReader()}, + } + + objs := getRuntimeObjFromManifests(t, []string{tt.manifest}) + for _, obj := range objs { + u := obj.(*unstructured.Unstructured) + gvr := getGVR(t, fakeMapper, u) + require.NoError(t, fakeClient.Tracker().Create(gvr, u, u.GetNamespace())) + } + + resourceList := getResourceListFromRuntimeObjs(t, c, objs) + err := waiter.Wait(resourceList, time.Second) + if tt.expectErrStrs == nil { + require.NoError(t, err) + return + } + require.Error(t, err) + for _, expected := range tt.expectErrStrs { + assert.Contains(t, err.Error(), expected) + } + }) + } +} + +func TestWithCustomReadinessStatusReader(t *testing.T) { + opts := &waitOptions{} + WithCustomReadinessStatusReader()(opts) + assert.True(t, opts.enableCustomReadinessStatusReader) +} + +func TestCustomReadinessStatusReaderReadStatusForObject(t *testing.T) { + reader := newCustomReadinessStatusReader() + u := &unstructured.Unstructured{} + u.SetAPIVersion("v1") + u.SetKind("ConfigMap") + u.SetName("ready-config") + u.SetNamespace("default") + u.SetAnnotations(map[string]string{ + AnnotationReadinessSuccess: `["{.phase} == \"Ready\""]`, + AnnotationReadinessFailure: `["{.phase} == \"Failed\""]`, + }) + require.NoError(t, unstructured.SetNestedField(u.Object, "Ready", "status", "phase")) + + result, err := reader.ReadStatusForObject(context.Background(), nil, u) + require.NoError(t, err) + assert.Equal(t, status.CurrentStatus, result.Status) +} diff --git a/pkg/kube/options.go b/pkg/kube/options.go index 3326c284b..49bd49670 100644 --- a/pkg/kube/options.go +++ b/pkg/kube/options.go @@ -72,11 +72,19 @@ func WithKStatusReaders(readers ...engine.StatusReader) WaitOption { } } +// WithCustomReadinessStatusReader enables custom readiness evaluation while waiting. +func WithCustomReadinessStatusReader() WaitOption { + return func(wo *waitOptions) { + wo.enableCustomReadinessStatusReader = true + } +} + type waitOptions struct { - ctx context.Context - watchUntilReadyCtx context.Context - waitCtx context.Context - waitWithJobsCtx context.Context - waitForDeleteCtx context.Context - statusReaders []engine.StatusReader + ctx context.Context + watchUntilReadyCtx context.Context + waitCtx context.Context + waitWithJobsCtx context.Context + waitForDeleteCtx context.Context + statusReaders []engine.StatusReader + enableCustomReadinessStatusReader bool } diff --git a/pkg/kube/readiness.go b/pkg/kube/readiness.go new file mode 100644 index 000000000..aab69005f --- /dev/null +++ b/pkg/kube/readiness.go @@ -0,0 +1,291 @@ +/* +Copyright The Helm Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package kube // import "helm.sh/helm/v4/pkg/kube" + +import ( + "bytes" + "encoding/json" + "errors" + "fmt" + "strconv" + "strings" + + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/client-go/util/jsonpath" +) + +const ( + // AnnotationReadinessSuccess declares custom readiness success conditions. + // Its value is a JSON array of expressions in the form: + // "{.fieldPath} " + // If any success condition evaluates to true, the resource is considered ready. + AnnotationReadinessSuccess = "helm.sh/readiness-success" + + // AnnotationReadinessFailure declares custom readiness failure conditions. + // Its value is a JSON array of expressions in the form: + // "{.fieldPath} " + // If any failure condition evaluates to true, the resource is considered failed. + // Failure conditions take precedence over success conditions. + AnnotationReadinessFailure = "helm.sh/readiness-failure" +) + +// ReadinessStatus represents the evaluated readiness of a resource. +type ReadinessStatus int + +const ( + // ReadinessPending means neither success nor failure conditions are met. + ReadinessPending ReadinessStatus = iota + // ReadinessReady means at least one success condition is true. + ReadinessReady + // ReadinessFailed means at least one failure condition is true. + ReadinessFailed +) + +func (r ReadinessStatus) String() string { + switch r { + case ReadinessPending: + return "Pending" + case ReadinessReady: + return "Ready" + case ReadinessFailed: + return "Failed" + default: + return "Unknown" + } +} + +// EvaluateCustomReadiness evaluates custom readiness expressions against a +// resource's .status field. +// +// When both successExprs and failureExprs are empty or nil, or when only one +// side is provided, the caller should fall back to kstatus and useKstatus will +// be returned as true. +func EvaluateCustomReadiness(obj *unstructured.Unstructured, successExprs, failureExprs []string) (ReadinessStatus, bool, error) { + hasSuccess := len(successExprs) > 0 + hasFailure := len(failureExprs) > 0 + + if !hasSuccess || !hasFailure { + // Partial annotations: fall back to kstatus. The warning for this + // case is emitted once per batch by warnIfPartialReadinessAnnotations + // in the sequencing layer, not here (which runs on every poll tick). + return ReadinessPending, true, nil + } + + statusObj, found, err := unstructured.NestedMap(obj.Object, "status") + if err != nil || !found { + return ReadinessPending, false, nil + } + + statusWrapper := map[string]any{"status": statusObj} + + for _, expr := range failureExprs { + met, err := evaluateExpression(statusWrapper, expr) + if err != nil { + return ReadinessPending, false, fmt.Errorf("evaluating failure expression %q: %w", expr, err) + } + if met { + return ReadinessFailed, false, nil + } + } + + for _, expr := range successExprs { + met, err := evaluateExpression(statusWrapper, expr) + if err != nil { + return ReadinessPending, false, fmt.Errorf("evaluating success expression %q: %w", expr, err) + } + if met { + return ReadinessReady, false, nil + } + } + + return ReadinessPending, false, nil +} + +func parseReadinessExpressions(annotation string) ([]string, error) { + annotation = strings.TrimSpace(annotation) + if annotation == "" { + return nil, nil + } + + var exprs []string + if err := json.Unmarshal([]byte(annotation), &exprs); err != nil { + return nil, fmt.Errorf("parsing readiness annotation JSON: %w", err) + } + + for i, expr := range exprs { + exprs[i] = strings.TrimSpace(expr) + } + + return exprs, nil +} + +func evaluateExpression(obj map[string]any, expr string) (bool, error) { + path, op, rawVal, err := parseReadinessExpression(expr) + if err != nil { + return false, err + } + + template, err := readinessJSONPath(path) + if err != nil { + return false, err + } + + jp := jsonpath.New("readiness") + if err := jp.Parse(template); err != nil { + return false, fmt.Errorf("invalid JSONPath %q: %w", template, err) + } + + var buf bytes.Buffer + if err := jp.Execute(&buf, obj); err != nil { + return false, nil + } + + return compareValues(strings.TrimSpace(buf.String()), op, rawVal) +} + +func readinessJSONPath(path string) (string, error) { + switch { + case strings.HasPrefix(path, ".status.") || strings.HasPrefix(path, ".status["): + // Already rooted at .status ā€” avoid double-prefixing + return "{" + path + "}", nil + case strings.HasPrefix(path, "."): + return "{.status" + path + "}", nil + case strings.HasPrefix(path, "["): + return "{.status" + path + "}", nil + default: + return "", fmt.Errorf("invalid JSONPath %q: path must start with . or [", path) + } +} + +// parseReadinessExpression parses "{} " into its parts. +func parseReadinessExpression(expr string) (path, op, val string, err error) { + expr = strings.TrimSpace(expr) + if expr == "" { + return "", "", "", errors.New("expression cannot be empty") + } + if !strings.HasPrefix(expr, "{") { + return "", "", "", fmt.Errorf("expression must start with {}: %q", expr) + } + + closeBrace := strings.Index(expr, "}") + if closeBrace < 0 { + return "", "", "", fmt.Errorf("expression missing closing }: %q", expr) + } + + path = strings.TrimSpace(expr[1:closeBrace]) + if path == "" { + return "", "", "", fmt.Errorf("expression missing JSONPath: %q", expr) + } + + rest := strings.TrimSpace(expr[closeBrace+1:]) + if rest == "" { + return "", "", "", fmt.Errorf("expression missing operator and value: %q", expr) + } + + parts := strings.Fields(rest) + if len(parts) == 0 { + return "", "", "", fmt.Errorf("expression missing operator: %q", expr) + } + + switch parts[0] { + case "==", "!=", "<", "<=", ">", ">=": + op = parts[0] + default: + return "", "", "", fmt.Errorf("unsupported operator %q in expression %q", parts[0], expr) + } + + val = strings.TrimSpace(rest[len(op):]) + if val == "" { + return "", "", "", fmt.Errorf("expression missing comparison value: %q", expr) + } + + return path, op, val, nil +} + +func compareValues(actual, op, expected string) (bool, error) { + actual = strings.TrimSpace(actual) + expected = trimReadinessValue(strings.TrimSpace(expected)) + + if actualFloat, ok := tryParseFloat(actual); ok { + if expectedFloat, ok := tryParseFloat(expected); ok { + return compareNumeric(actualFloat, op, expectedFloat) + } + } + + if actualBool, actualIsBool := tryParseBool(actual); actualIsBool { + if expectedBool, expectedIsBool := tryParseBool(expected); expectedIsBool { + switch op { + case "==": + return actualBool == expectedBool, nil + case "!=": + return actualBool != expectedBool, nil + default: + return false, fmt.Errorf("operator %q not supported for boolean values", op) + } + } + } + + switch op { + case "==": + return actual == expected, nil + case "!=": + return actual != expected, nil + default: + return false, fmt.Errorf("operator %q not supported for string values", op) + } +} + +func trimReadinessValue(value string) string { + if len(value) >= 2 { + if value[0] == '"' && value[len(value)-1] == '"' { + return value[1 : len(value)-1] + } + if value[0] == '\'' && value[len(value)-1] == '\'' { + return value[1 : len(value)-1] + } + } + return value +} + +func tryParseFloat(value string) (float64, bool) { + parsed, err := strconv.ParseFloat(value, 64) + return parsed, err == nil +} + +func tryParseBool(value string) (bool, bool) { + parsed, err := strconv.ParseBool(value) + return parsed, err == nil +} + +func compareNumeric(actual float64, op string, expected float64) (bool, error) { + switch op { + case "==": + return actual == expected, nil + case "!=": + return actual != expected, nil + case "<": + return actual < expected, nil + case "<=": + return actual <= expected, nil + case ">": + return actual > expected, nil + case ">=": + return actual >= expected, nil + default: + return false, fmt.Errorf("unknown operator %q", op) + } +} diff --git a/pkg/kube/readiness_test.go b/pkg/kube/readiness_test.go new file mode 100644 index 000000000..3b2544af4 --- /dev/null +++ b/pkg/kube/readiness_test.go @@ -0,0 +1,350 @@ +/* +Copyright The Helm Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package kube + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" +) + +func makeUnstructuredWithStatus(t *testing.T, statusFields map[string]any) *unstructured.Unstructured { + t.Helper() + + obj := &unstructured.Unstructured{} + obj.SetAPIVersion("v1") + obj.SetKind("ConfigMap") + obj.SetName("test-resource") + + if statusFields != nil { + require.NoError(t, unstructured.SetNestedField(obj.Object, statusFields, "status")) + } + + return obj +} + +func TestEvaluateReadiness_BothNil_UsesKstatus(t *testing.T) { + obj := makeUnstructuredWithStatus(t, nil) + + result, useKstatus, err := EvaluateCustomReadiness(obj, nil, nil) + + require.NoError(t, err) + assert.True(t, useKstatus) + assert.Equal(t, ReadinessPending, result) + + result, useKstatus, err = EvaluateCustomReadiness(obj, []string{}, []string{}) + + require.NoError(t, err) + assert.True(t, useKstatus) + assert.Equal(t, ReadinessPending, result) +} + +func TestEvaluateReadiness_OnlySuccess_FallsBackToKstatus(t *testing.T) { + obj := makeUnstructuredWithStatus(t, nil) + + result, useKstatus, err := EvaluateCustomReadiness(obj, []string{`{.ready} == true`}, nil) + + require.NoError(t, err) + assert.True(t, useKstatus) + assert.Equal(t, ReadinessPending, result) +} + +func TestEvaluateReadiness_OnlyFailure_FallsBackToKstatus(t *testing.T) { + obj := makeUnstructuredWithStatus(t, nil) + + result, useKstatus, err := EvaluateCustomReadiness(obj, nil, []string{`{.failed} == true`}) + + require.NoError(t, err) + assert.True(t, useKstatus) + assert.Equal(t, ReadinessPending, result) +} + +func TestEvaluateReadiness_SuccessTrue(t *testing.T) { + obj := makeUnstructuredWithStatus(t, map[string]any{ + "succeeded": int64(1), + }) + + result, useKstatus, err := EvaluateCustomReadiness( + obj, + []string{`{.succeeded} == 1`}, + []string{`{.failed} >= 1`}, + ) + + require.NoError(t, err) + assert.False(t, useKstatus) + assert.Equal(t, ReadinessReady, result) +} + +func TestEvaluateReadiness_FailurePrecedesSuccess(t *testing.T) { + obj := makeUnstructuredWithStatus(t, map[string]any{ + "succeeded": int64(1), + "failed": int64(1), + }) + + result, useKstatus, err := EvaluateCustomReadiness( + obj, + []string{`{.succeeded} == 1`}, + []string{`{.failed} >= 1`}, + ) + + require.NoError(t, err) + assert.False(t, useKstatus) + assert.Equal(t, ReadinessFailed, result) +} + +func TestEvaluateReadiness_ORSemantics_AnySuccessTrue(t *testing.T) { + obj := makeUnstructuredWithStatus(t, map[string]any{ + "phase": "Succeeded", + "another": "nope", + }) + + result, useKstatus, err := EvaluateCustomReadiness( + obj, + []string{`{.another} == "yes"`, `{.phase} == "Succeeded"`}, + []string{`{.failed} >= 1`}, + ) + + require.NoError(t, err) + assert.False(t, useKstatus) + assert.Equal(t, ReadinessReady, result) +} + +func TestEvaluateReadiness_NeitherConditionMet(t *testing.T) { + obj := makeUnstructuredWithStatus(t, map[string]any{ + "succeeded": int64(0), + }) + + result, useKstatus, err := EvaluateCustomReadiness( + obj, + []string{`{.succeeded} == 1`}, + []string{`{.failed} >= 1`}, + ) + + require.NoError(t, err) + assert.False(t, useKstatus) + assert.Equal(t, ReadinessPending, result) +} + +func TestEvaluateReadiness_MissingStatusField(t *testing.T) { + t.Run("missing status object", func(t *testing.T) { + obj := makeUnstructuredWithStatus(t, nil) + + result, useKstatus, err := EvaluateCustomReadiness( + obj, + []string{`{.phase} == "Running"`}, + []string{`{.failed} >= 1`}, + ) + + require.NoError(t, err) + assert.False(t, useKstatus) + assert.Equal(t, ReadinessPending, result) + }) + + t.Run("missing status field", func(t *testing.T) { + obj := makeUnstructuredWithStatus(t, map[string]any{}) + + result, useKstatus, err := EvaluateCustomReadiness( + obj, + []string{`{.phase} == "Running"`}, + []string{`{.failed} >= 1`}, + ) + + require.NoError(t, err) + assert.False(t, useKstatus) + assert.Equal(t, ReadinessPending, result) + }) +} + +func TestEvaluateReadiness_NumericComparisonOperators(t *testing.T) { + tests := []struct { + name string + expr string + status map[string]any + }{ + {name: "equal", expr: `{.succeeded} == 1`, status: map[string]any{"succeeded": int64(1)}}, + {name: "not equal", expr: `{.succeeded} != 2`, status: map[string]any{"succeeded": int64(1)}}, + {name: "less than", expr: `{.succeeded} < 2`, status: map[string]any{"succeeded": int64(1)}}, + {name: "less than or equal", expr: `{.succeeded} <= 1`, status: map[string]any{"succeeded": int64(1)}}, + {name: "greater than", expr: `{.succeeded} > 0`, status: map[string]any{"succeeded": int64(1)}}, + {name: "greater than or equal", expr: `{.succeeded} >= 1`, status: map[string]any{"succeeded": int64(1)}}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + obj := makeUnstructuredWithStatus(t, tt.status) + + result, useKstatus, err := EvaluateCustomReadiness( + obj, + []string{tt.expr}, + []string{`{.failed} >= 1`}, + ) + + require.NoError(t, err) + assert.False(t, useKstatus) + assert.Equal(t, ReadinessReady, result) + }) + } +} + +func TestEvaluateReadiness_BooleanComparison(t *testing.T) { + tests := []struct { + name string + expr string + status map[string]any + }{ + {name: "boolean equals", expr: `{.ready} == true`, status: map[string]any{"ready": true}}, + {name: "boolean not equals", expr: `{.ready} != false`, status: map[string]any{"ready": true}}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + obj := makeUnstructuredWithStatus(t, tt.status) + + result, useKstatus, err := EvaluateCustomReadiness( + obj, + []string{tt.expr}, + []string{`{.failed} == true`}, + ) + + require.NoError(t, err) + assert.False(t, useKstatus) + assert.Equal(t, ReadinessReady, result) + }) + } +} + +func TestEvaluateReadiness_StringComparison(t *testing.T) { + obj := makeUnstructuredWithStatus(t, map[string]any{ + "phase": "Running", + }) + + result, useKstatus, err := EvaluateCustomReadiness( + obj, + []string{`{.phase} == "Running"`}, + []string{`{.phase} == "Failed"`}, + ) + + require.NoError(t, err) + assert.False(t, useKstatus) + assert.Equal(t, ReadinessReady, result) + + result, useKstatus, err = EvaluateCustomReadiness( + obj, + []string{`{.phase} != "Failed"`}, + []string{`{.phase} == "Failed"`}, + ) + + require.NoError(t, err) + assert.False(t, useKstatus) + assert.Equal(t, ReadinessReady, result) +} + +func TestEvaluateReadiness_InvalidJSONPath(t *testing.T) { + obj := makeUnstructuredWithStatus(t, map[string]any{ + "phase": "Running", + }) + + result, useKstatus, err := EvaluateCustomReadiness( + obj, + []string{`{.phase[} == "Running"`}, + []string{`{.failed} >= 1`}, + ) + + assert.ErrorContains(t, err, "invalid JSONPath") + assert.False(t, useKstatus) + assert.Equal(t, ReadinessPending, result) +} + +func TestEvaluateReadiness_InvalidOperator(t *testing.T) { + obj := makeUnstructuredWithStatus(t, map[string]any{ + "phase": "Running", + }) + + result, useKstatus, err := EvaluateCustomReadiness( + obj, + []string{`{.phase} <> "Running"`}, + []string{`{.failed} >= 1`}, + ) + + assert.ErrorContains(t, err, "unsupported operator") + assert.False(t, useKstatus) + assert.Equal(t, ReadinessPending, result) +} + +func TestParseReadinessExpression(t *testing.T) { + t.Run("valid expressions", func(t *testing.T) { + tests := []struct { + name string + expr string + wantPath string + wantOp string + wantVal string + }{ + {name: "string equality", expr: `{.phase} == "Running"`, wantPath: ".phase", wantOp: "==", wantVal: `"Running"`}, + {name: "numeric greater than or equal", expr: `{.count} >= 3`, wantPath: ".count", wantOp: ">=", wantVal: "3"}, + {name: "boolean not equal", expr: `{.ready} != false`, wantPath: ".ready", wantOp: "!=", wantVal: "false"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + path, op, val, err := parseReadinessExpression(tt.expr) + + require.NoError(t, err) + assert.Equal(t, tt.wantPath, path) + assert.Equal(t, tt.wantOp, op) + assert.Equal(t, tt.wantVal, val) + }) + } + }) + + t.Run("invalid expressions", func(t *testing.T) { + tests := []struct { + name string + expr string + wantErrPart string + }{ + {name: "missing opening brace", expr: `.phase == "Running"`, wantErrPart: "must start"}, + {name: "missing closing brace", expr: `{.phase == "Running"`, wantErrPart: "missing closing"}, + {name: "missing operator", expr: `{.phase} "Running"`, wantErrPart: "unsupported operator"}, + {name: "unsupported operator", expr: `{.phase} <> "Running"`, wantErrPart: "unsupported operator"}, + {name: "missing value", expr: `{.phase} ==`, wantErrPart: "missing comparison value"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, _, _, err := parseReadinessExpression(tt.expr) + assert.ErrorContains(t, err, tt.wantErrPart) + }) + } + }) +} + +func TestReadinessExpressionsJSON(t *testing.T) { + t.Run("empty list", func(t *testing.T) { + exprs, err := parseReadinessExpressions(`[]`) + + require.NoError(t, err) + assert.Empty(t, exprs) + }) + + t.Run("malformed annotation JSON", func(t *testing.T) { + _, err := parseReadinessExpressions(`["{.ready} == true"`) + assert.ErrorContains(t, err, "parsing readiness annotation JSON") + }) +} diff --git a/pkg/release/v1/release.go b/pkg/release/v1/release.go index 3bbc0e4ce..e9c06bdf2 100644 --- a/pkg/release/v1/release.go +++ b/pkg/release/v1/release.go @@ -25,6 +25,12 @@ type ApplyMethod string const ApplyMethodClientSideApply ApplyMethod = "csa" const ApplyMethodServerSideApply ApplyMethod = "ssa" +// SequencingInfo records whether and how this release used ordered sequencing. +type SequencingInfo struct { + Enabled bool `json:"enabled,omitempty"` + Strategy string `json:"strategy,omitempty"` +} + // Release describes a deployment of a chart, together with the chart // and the variables used to deploy that chart. type Release struct { @@ -51,6 +57,8 @@ type Release struct { // ApplyMethod stores whether server-side or client-side apply was used for the release // Unset (empty string) should be treated as the default of client-side apply ApplyMethod string `json:"apply_method,omitempty"` // "ssa" | "csa" + // SequencingInfo stores ordered deployment metadata for rollback/uninstall. + SequencingInfo *SequencingInfo `json:"sequencing_info,omitempty"` } // SetStatus is a helper for setting the status on a release. diff --git a/pkg/release/v1/release_test.go b/pkg/release/v1/release_test.go new file mode 100644 index 000000000..6319dd404 --- /dev/null +++ b/pkg/release/v1/release_test.go @@ -0,0 +1,59 @@ +/* +Copyright The Helm Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package v1 + +import ( + "encoding/json" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestReleaseSequencingInfoBackwardCompatibility(t *testing.T) { + release := Release{Name: "demo"} + + data, err := json.Marshal(&release) + require.NoError(t, err) + assert.NotContains(t, string(data), "sequencing_info") + + var decoded Release + err = json.Unmarshal([]byte(`{"name":"demo"}`), &decoded) + require.NoError(t, err) + assert.Nil(t, decoded.SequencingInfo) +} + +func TestReleaseSequencingInfoRoundTrip(t *testing.T) { + release := Release{ + Name: "demo", + SequencingInfo: &SequencingInfo{ + Enabled: true, + Strategy: "ordered", + }, + } + + data, err := json.Marshal(&release) + require.NoError(t, err) + assert.Contains(t, string(data), `"sequencing_info"`) + + var decoded Release + err = json.Unmarshal(data, &decoded) + require.NoError(t, err) + require.NotNil(t, decoded.SequencingInfo) + assert.True(t, decoded.SequencingInfo.Enabled) + assert.Equal(t, "ordered", decoded.SequencingInfo.Strategy) +} diff --git a/pkg/release/v1/util/manifest.go b/pkg/release/v1/util/manifest.go index fa26f6256..9596f5f51 100644 --- a/pkg/release/v1/util/manifest.go +++ b/pkg/release/v1/util/manifest.go @@ -30,6 +30,7 @@ type SimpleHead struct { Kind string `json:"kind,omitempty"` Metadata *struct { Name string `json:"name"` + Namespace string `json:"namespace,omitempty"` Annotations map[string]string `json:"annotations"` } `json:"metadata,omitempty"` } diff --git a/pkg/release/v1/util/resource_group.go b/pkg/release/v1/util/resource_group.go new file mode 100644 index 000000000..03665b7b5 --- /dev/null +++ b/pkg/release/v1/util/resource_group.go @@ -0,0 +1,221 @@ +/* +Copyright The Helm Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package util + +import ( + "encoding/json" + "fmt" + + chartutil "helm.sh/helm/v4/pkg/chart/v2/util" +) + +const ( + // AnnotationResourceGroup declares the resource-group a resource belongs to. + AnnotationResourceGroup = "helm.sh/resource-group" + + // AnnotationDependsOnResourceGroups declares prerequisite resource-groups for a + // resource's group as a JSON string array. + AnnotationDependsOnResourceGroups = "helm.sh/depends-on/resource-groups" +) + +// ResourceGroupResult holds the output of ParseResourceGroups. +type ResourceGroupResult struct { + // Groups maps group name to manifests assigned to that group. + Groups map[string][]Manifest + + // GroupDeps maps group name to the group names it depends on. + GroupDeps map[string][]string + + // Unsequenced contains manifests that should be deployed outside the DAG. + Unsequenced []Manifest +} + +// ParseResourceGroups extracts resource-group annotations from rendered +// manifests and partitions them into sequenced groups and unsequenced manifests. +// +// Manifests without a resource-group annotation are treated as unsequenced. +// Invalid depends-on annotations emit a warning and demote that manifest to the +// unsequenced batch. References to unknown groups emit a warning and demote the +// entire referencing group to the unsequenced batch. If the same resource is +// assigned to different groups, ParseResourceGroups returns an error. +func ParseResourceGroups(manifests []Manifest) (ResourceGroupResult, []string, error) { + result := ResourceGroupResult{ + Groups: make(map[string][]Manifest), + GroupDeps: make(map[string][]string), + } + var warnings []string + + resourceAssignments := make(map[string]string) + groupDeps := make(map[string][]string) + groupOrder := make([]string, 0) + + for _, manifest := range manifests { + groupName, hasGroup := resourceGroupName(manifest) + if !hasGroup { + result.Unsequenced = append(result.Unsequenced, manifest) + continue + } + + resourceID := resourceGroupResourceID(manifest) + if existingGroup, ok := resourceAssignments[resourceID]; ok && existingGroup != groupName { + return ResourceGroupResult{}, warnings, fmt.Errorf( + "resource %q assigned to multiple resource groups %q and %q", + resourceID, + existingGroup, + groupName, + ) + } + resourceAssignments[resourceID] = groupName + + deps, warning, err := resourceGroupDependencies(manifest) + if err != nil { + warnings = append(warnings, warning) + result.Unsequenced = append(result.Unsequenced, manifest) + continue + } + + if _, ok := result.Groups[groupName]; !ok { + groupOrder = append(groupOrder, groupName) + } + + result.Groups[groupName] = append(result.Groups[groupName], manifest) + groupDeps[groupName] = appendUniqueStrings(groupDeps[groupName], deps...) + } + + for changed := true; changed; { + changed = false + + for _, groupName := range groupOrder { + groupManifests, ok := result.Groups[groupName] + if !ok { + continue + } + + missingDep := firstMissingDependency(result.Groups, groupDeps[groupName]) + if missingDep == "" { + continue + } + + warnings = append(warnings, fmt.Sprintf( + "group %q depends-on non-existent group %q; moving group to unsequenced batch", + groupName, + missingDep, + )) + result.Unsequenced = append(result.Unsequenced, groupManifests...) + delete(result.Groups, groupName) + changed = true + } + } + + for _, groupName := range groupOrder { + if _, ok := result.Groups[groupName]; !ok { + continue + } + + result.GroupDeps[groupName] = groupDeps[groupName] + } + + return result, warnings, nil +} + +// BuildResourceGroupDAG constructs a DAG from the resource-group parse result. +// Each group becomes a DAG node and every dependency becomes an edge. +func BuildResourceGroupDAG(result ResourceGroupResult) (*chartutil.DAG, error) { + dag := chartutil.NewDAG() + + for groupName := range result.Groups { + dag.AddNode(groupName) + } + + for groupName, deps := range result.GroupDeps { + for _, dep := range deps { + if err := dag.AddEdge(dep, groupName); err != nil { + return nil, fmt.Errorf("resource-group sequencing edge %sā†’%s: %w", dep, groupName, err) + } + } + } + + return dag, nil +} + +func resourceGroupName(manifest Manifest) (string, bool) { + if manifest.Head == nil || manifest.Head.Metadata == nil || len(manifest.Head.Metadata.Annotations) == 0 { + return "", false + } + + groupName, ok := manifest.Head.Metadata.Annotations[AnnotationResourceGroup] + if !ok || groupName == "" { + return "", false + } + + return groupName, true +} + +func resourceGroupDependencies(manifest Manifest) ([]string, string, error) { + annotations := manifest.Head.Metadata.Annotations + rawDeps, ok := annotations[AnnotationDependsOnResourceGroups] + if !ok { + return nil, "", nil + } + + var deps []string + if err := json.Unmarshal([]byte(rawDeps), &deps); err != nil { + return nil, fmt.Sprintf( + "manifest %q: invalid JSON in %s annotation: %v; moving to unsequenced batch", + manifest.Name, + AnnotationDependsOnResourceGroups, + err, + ), err + } + + return deps, "", nil +} + +func firstMissingDependency(groups map[string][]Manifest, deps []string) string { + for _, dep := range deps { + if _, ok := groups[dep]; !ok { + return dep + } + } + + return "" +} + +func appendUniqueStrings(existing []string, values ...string) []string { + seen := make(map[string]struct{}, len(existing)) + for _, value := range existing { + seen[value] = struct{}{} + } + + for _, value := range values { + if _, ok := seen[value]; ok { + continue + } + existing = append(existing, value) + seen[value] = struct{}{} + } + + return existing +} + +func resourceGroupResourceID(manifest Manifest) string { + if manifest.Head == nil || manifest.Head.Metadata == nil || manifest.Head.Metadata.Name == "" { + return manifest.Name + } + + return fmt.Sprintf("%s/%s/%s/%s", manifest.Head.Version, manifest.Head.Kind, manifest.Head.Metadata.Namespace, manifest.Head.Metadata.Name) +} diff --git a/pkg/release/v1/util/resource_group_test.go b/pkg/release/v1/util/resource_group_test.go new file mode 100644 index 000000000..9dbcc86f8 --- /dev/null +++ b/pkg/release/v1/util/resource_group_test.go @@ -0,0 +1,356 @@ +/* +Copyright The Helm Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package util + +import ( + "fmt" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestParseResourceGroups_NoAnnotations(t *testing.T) { + t.Parallel() + + result, warnings := parseResourceGroups(t, + makeManifest("plain-a", "chart/templates/plain-a.yaml", nil), + makeManifest("plain-b", "chart/templates/plain-b.yaml", nil), + ) + + assert.Empty(t, result.Groups) + assert.Empty(t, result.GroupDeps) + require.Len(t, result.Unsequenced, 2) + assert.Equal(t, []string{"chart/templates/plain-a.yaml", "chart/templates/plain-b.yaml"}, manifestPaths(result.Unsequenced)) + assert.Empty(t, warnings) +} + +func TestParseResourceGroups_GroupWithDependency(t *testing.T) { + t.Parallel() + + result, warnings, batches := parseResourceGroupBatches(t, + makeManifest("database", "chart/templates/database.yaml", map[string]string{ + AnnotationResourceGroup: "database", + }), + makeManifest("app", "chart/templates/app.yaml", map[string]string{ + AnnotationResourceGroup: "app", + AnnotationDependsOnResourceGroups: `["database"]`, + }), + ) + + require.Len(t, result.Groups, 2) + assert.Equal(t, [][]string{{"database"}, {"app"}}, batches) + assert.Empty(t, result.Unsequenced) + assert.Empty(t, warnings) +} + +func TestParseResourceGroups_MultipleResourcesPerGroup(t *testing.T) { + t.Parallel() + + result, warnings, batches := parseResourceGroupBatches(t, + makeManifest("database-config", "chart/templates/database-config.yaml", map[string]string{ + AnnotationResourceGroup: "database", + }), + makeManifest("database-secret", "chart/templates/database-secret.yaml", map[string]string{ + AnnotationResourceGroup: "database", + }), + makeManifest("app", "chart/templates/app.yaml", map[string]string{ + AnnotationResourceGroup: "app", + AnnotationDependsOnResourceGroups: `["database"]`, + }), + ) + + require.Len(t, result.Groups["database"], 2) + assert.Equal(t, []string{ + "chart/templates/database-config.yaml", + "chart/templates/database-secret.yaml", + }, manifestPaths(result.Groups["database"])) + assert.Equal(t, [][]string{{"database"}, {"app"}}, batches) + assert.Empty(t, warnings) +} + +func TestParseResourceGroups_ResourceAssignedToMultipleGroups(t *testing.T) { + t.Parallel() + + _, _, err := ParseResourceGroups([]Manifest{ + makeManifest("shared", "chart/templates/database.yaml", map[string]string{ + AnnotationResourceGroup: "database", + }), + makeManifest("shared", "chart/templates/cache.yaml", map[string]string{ + AnnotationResourceGroup: "cache", + }), + }) + + require.Error(t, err) + assert.ErrorContains(t, err, "assigned to multiple resource groups") + assert.ErrorContains(t, err, "database") + assert.ErrorContains(t, err, "cache") +} + +func TestParseResourceGroups_NonExistentGroupReferenceWarning(t *testing.T) { + t.Parallel() + + result, warnings := parseResourceGroups(t, + makeManifest("app", "chart/templates/app.yaml", map[string]string{ + AnnotationResourceGroup: "app", + AnnotationDependsOnResourceGroups: `["missing"]`, + }), + ) + + assert.Empty(t, result.Groups) + require.Len(t, result.Unsequenced, 1) + assert.Equal(t, []string{"chart/templates/app.yaml"}, manifestPaths(result.Unsequenced)) + require.Len(t, warnings, 1) + assert.Contains(t, warnings[0], "non-existent group") + assert.Contains(t, warnings[0], "missing") +} + +func TestParseResourceGroups_CascadingMissingDeps(t *testing.T) { + t.Parallel() + + manifests := []Manifest{ + makeManifest("app", "chart/templates/app.yaml", map[string]string{ + AnnotationResourceGroup: "app", + AnnotationDependsOnResourceGroups: `["database"]`, + }), + makeManifest("database", "chart/templates/database.yaml", map[string]string{ + AnnotationResourceGroup: "database", + AnnotationDependsOnResourceGroups: `["missing"]`, + }), + } + + for range 64 { + result, warnings := parseResourceGroups(t, manifests...) + + assert.Empty(t, result.Groups) + assert.Empty(t, result.GroupDeps) + require.Len(t, result.Unsequenced, 2) + assert.ElementsMatch(t, []string{ + "chart/templates/app.yaml", + "chart/templates/database.yaml", + }, manifestPaths(result.Unsequenced)) + require.Len(t, warnings, 2) + assert.Contains(t, warnings[0], `group "database" depends-on non-existent group "missing"`) + assert.Contains(t, warnings[1], `group "app" depends-on non-existent group "database"`) + + dag, err := BuildResourceGroupDAG(result) + require.NoError(t, err) + batches, err := dag.GetBatches() + require.NoError(t, err) + assert.Empty(t, batches) + } +} + +func TestParseResourceGroups_InvalidDependsOnJSON(t *testing.T) { + t.Parallel() + + result, warnings := parseResourceGroups(t, + makeManifest("app", "chart/templates/app.yaml", map[string]string{ + AnnotationResourceGroup: "app", + AnnotationDependsOnResourceGroups: `not-valid-json`, + }), + ) + + assert.Empty(t, result.Groups) + require.Len(t, result.Unsequenced, 1) + assert.Equal(t, []string{"chart/templates/app.yaml"}, manifestPaths(result.Unsequenced)) + require.Len(t, warnings, 1) + assert.Contains(t, warnings[0], "invalid JSON") +} + +func TestParseResourceGroups_MixedSequencedAndUnsequenced(t *testing.T) { + t.Parallel() + + result, warnings := parseResourceGroups(t, + makeManifest("database", "chart/templates/database.yaml", map[string]string{ + AnnotationResourceGroup: "database", + }), + makeManifest("plain", "chart/templates/plain.yaml", nil), + ) + + require.Len(t, result.Groups, 1) + assert.Contains(t, result.Groups, "database") + require.Len(t, result.Unsequenced, 1) + assert.Equal(t, []string{"chart/templates/plain.yaml"}, manifestPaths(result.Unsequenced)) + assert.Empty(t, warnings) +} + +func TestResourceGroupDAG_CycleDetection(t *testing.T) { + t.Parallel() + + result, _ := parseResourceGroups(t, + makeManifest("a", "chart/templates/a.yaml", map[string]string{ + AnnotationResourceGroup: "a", + AnnotationDependsOnResourceGroups: `["c"]`, + }), + makeManifest("b", "chart/templates/b.yaml", map[string]string{ + AnnotationResourceGroup: "b", + AnnotationDependsOnResourceGroups: `["a"]`, + }), + makeManifest("c", "chart/templates/c.yaml", map[string]string{ + AnnotationResourceGroup: "c", + AnnotationDependsOnResourceGroups: `["b"]`, + }), + ) + + dag, err := BuildResourceGroupDAG(result) + require.NoError(t, err) + + batches, err := dag.GetBatches() + require.Error(t, err) + assert.Nil(t, batches) + assert.ErrorContains(t, err, "cycle") +} + +func TestParseResourceGroups_DeduplicatesDependencies(t *testing.T) { + t.Parallel() + + result, warnings, batches := parseResourceGroupBatches(t, + makeManifest("database", "chart/templates/database.yaml", map[string]string{ + AnnotationResourceGroup: "database", + }), + makeManifest("app-config", "chart/templates/app-config.yaml", map[string]string{ + AnnotationResourceGroup: "app", + AnnotationDependsOnResourceGroups: `["database"]`, + }), + makeManifest("app-secret", "chart/templates/app-secret.yaml", map[string]string{ + AnnotationResourceGroup: "app", + AnnotationDependsOnResourceGroups: `["database"]`, + }), + ) + + assert.Equal(t, []string{"database"}, result.GroupDeps["app"]) + assert.Equal(t, [][]string{{"database"}, {"app"}}, batches) + assert.Empty(t, warnings) +} + +func TestParseResourceGroups_IsolatedGroupsRemainInBatch0(t *testing.T) { + t.Parallel() + + _, warnings, batches := parseResourceGroupBatches(t, + makeManifest("database", "chart/templates/database.yaml", map[string]string{ + AnnotationResourceGroup: "database", + }), + makeManifest("metrics", "chart/templates/metrics.yaml", map[string]string{ + AnnotationResourceGroup: "metrics", + }), + makeManifest("app", "chart/templates/app.yaml", map[string]string{ + AnnotationResourceGroup: "app", + AnnotationDependsOnResourceGroups: `["database"]`, + }), + ) + + assert.Equal(t, [][]string{{"database", "metrics"}, {"app"}}, batches) + assert.Empty(t, warnings) +} + +func TestParseResourceGroups_ComplexDAG(t *testing.T) { + t.Parallel() + + _, warnings, batches := parseResourceGroupBatches(t, + makeManifest("cache", "chart/templates/cache.yaml", map[string]string{ + AnnotationResourceGroup: "cache", + }), + makeManifest("database", "chart/templates/database.yaml", map[string]string{ + AnnotationResourceGroup: "database", + }), + makeManifest("queue", "chart/templates/queue.yaml", map[string]string{ + AnnotationResourceGroup: "queue", + }), + makeManifest("api", "chart/templates/api.yaml", map[string]string{ + AnnotationResourceGroup: "api", + AnnotationDependsOnResourceGroups: `["database","queue"]`, + }), + makeManifest("worker", "chart/templates/worker.yaml", map[string]string{ + AnnotationResourceGroup: "worker", + AnnotationDependsOnResourceGroups: `["cache","queue"]`, + }), + makeManifest("frontend", "chart/templates/frontend.yaml", map[string]string{ + AnnotationResourceGroup: "frontend", + AnnotationDependsOnResourceGroups: `["api"]`, + }), + ) + + assert.Equal(t, [][]string{ + {"cache", "database", "queue"}, + {"api", "worker"}, + {"frontend"}, + }, batches) + assert.Empty(t, warnings) +} + +func parseResourceGroups(t *testing.T, manifests ...Manifest) (ResourceGroupResult, []string) { + t.Helper() + + result, warnings, err := ParseResourceGroups(manifests) + require.NoError(t, err) + + return result, warnings +} + +func parseResourceGroupBatches(t *testing.T, manifests ...Manifest) (ResourceGroupResult, []string, [][]string) { + t.Helper() + + result, warnings := parseResourceGroups(t, manifests...) + dag, err := BuildResourceGroupDAG(result) + require.NoError(t, err) + + batches, err := dag.GetBatches() + require.NoError(t, err) + + return result, warnings, batches +} + +func makeManifest(name, sourcePath string, annotations map[string]string) Manifest { + var content strings.Builder + fmt.Fprintf(&content, "apiVersion: v1\nkind: ConfigMap\nmetadata:\n name: %s\n", name) + if len(annotations) > 0 { + content.WriteString(" annotations:\n") + for key, value := range annotations { + fmt.Fprintf(&content, " %s: %q\n", key, value) + } + } + + head := &SimpleHead{ + Version: "v1", + Kind: "ConfigMap", + Metadata: &struct { + Name string `json:"name"` + Namespace string `json:"namespace,omitempty"` + Annotations map[string]string `json:"annotations"` + }{ + Name: name, + Annotations: annotations, + }, + } + + return Manifest{ + Name: sourcePath, + Content: content.String(), + Head: head, + } +} + +func manifestPaths(manifests []Manifest) []string { + paths := make([]string, 0, len(manifests)) + for _, manifest := range manifests { + paths = append(paths, manifest.Name) + } + + return paths +} diff --git a/scripts/validate-license.sh b/scripts/validate-license.sh index f67812ca5..248c8b803 100755 --- a/scripts/validate-license.sh +++ b/scripts/validate-license.sh @@ -20,6 +20,7 @@ find_files() { find . -not \( \ \( \ -wholename './.git' \ + -o -wholename './.mission' \ -o -wholename '*testdata*' \ -o -wholename '*third_party*' \ \) -prune \