Allow uninstall to process ALL manifests (including hooks)

Signed-off-by: Sean Dukehart <tomcruise81@users.noreply.github.com>

pkg/action/uninstall.go

@@ -174,8 +174,8 @@ func (u *Uninstall) deleteRelease(rel *release.Release) (string, []error) {
 		return rel.Manifest, []error{errors.Wrap(err, "could not get apiVersions from Kubernetes")}
 	}
 
-	manifests := releaseutil.SplitManifests(rel.Manifest)
-	_, files, err := releaseutil.SortManifests(manifests, caps.APIVersions, releaseutil.UninstallOrder)
+	manifests := releaseutil.SplitAllManifests(rel)
+	files, err := releaseutil.SortAllManifests(manifests, caps.APIVersions, releaseutil.UninstallOrder)
 	if err != nil {
 		// We could instead just delete everything in no particular order.
 		// FIXME: One way to delete at this point would be to try a label-based

pkg/releaseutil/manifest.go

@@ -21,6 +21,8 @@ import (
 	"regexp"
 	"strconv"
 	"strings"
+
+	rspb "helm.sh/helm/v3/pkg/release"
 )
 
 // SimpleHead defines what the structure of the head of a manifest file
@@ -35,13 +37,14 @@ type SimpleHead struct {
 
 var sep = regexp.MustCompile("(?:^|\\s*\n)---\\s*")
 
+const manifestTpl = "manifest-%d"
+
 // SplitManifests takes a string of manifest and returns a map contains individual manifests
 func SplitManifests(bigFile string) map[string]string {
 	// Basically, we're quickly splitting a stream of YAML documents into an
 	// array of YAML docs. The file name is just a place holder, but should be
 	// integer-sortable so that manifests get output in the same order as the
 	// input (see `BySplitManifestsOrder`).
-	tpl := "manifest-%d"
 	res := map[string]string{}
 	// Making sure that any extra whitespace in YAML stream doesn't interfere in splitting documents correctly.
 	bigFileTmp := strings.TrimSpace(bigFile)
@@ -53,7 +56,32 @@ func SplitManifests(bigFile string) map[string]string {
 		}
 
 		d = strings.TrimSpace(d)
-		res[fmt.Sprintf(tpl, count)] = d
+		res[fmt.Sprintf(manifestTpl, count)] = d
+		count = count + 1
+	}
+	return res
+}
+
+// SplitAllManifests takes a Release and returns a map contains ALL individual manifests, including manifests with hooks
+func SplitAllManifests(rel *rspb.Release) map[string]string {
+	res := SplitManifests(rel.Manifest)
+
+	hookManifests := getHookManifests(rel, res)
+	for k, v := range hookManifests {
+		res[k] = v
+	}
+	return res
+}
+
+func getHookManifests(rel *rspb.Release, baseManifests map[string]string) map[string]string {
+	res := map[string]string{}
+	var count int = len(baseManifests)
+	for _, d := range rel.Hooks {
+		if d == nil {
+			continue
+		}
+
+		res[fmt.Sprintf(manifestTpl, count)] = fmt.Sprintf("# Source: %s\n%s", d.Path, d.Manifest)
 		count = count + 1
 	}
 	return res

pkg/releaseutil/manifest_sorter.go

@@ -203,6 +203,78 @@ func (file *manifestFile) sort(result *result) error {
 	return nil
 }
 
+// SortAllManifests takes a map of filename/YAML contents, splits the file
+// by manifest entries and sorts them based on the given ordering. No regard is given
+// to wether the manifest contains a hook.
+//
+// Files that do not parse into the expected format are simply placed into a map and
+// returned.
+func SortAllManifests(files map[string]string, apis chartutil.VersionSet, ordering KindSortOrder) ([]Manifest, error) {
+	result := &result{}
+
+	var sortedFilePaths []string
+	for filePath := range files {
+		sortedFilePaths = append(sortedFilePaths, filePath)
+	}
+	sort.Strings(sortedFilePaths)
+
+	for _, filePath := range sortedFilePaths {
+		content := files[filePath]
+
+		// Skip partials. We could return these as a separate map, but there doesn't
+		// seem to be any need for that at this time.
+		if strings.HasPrefix(path.Base(filePath), "_") {
+			continue
+		}
+		// Skip empty files and log this.
+		if strings.TrimSpace(content) == "" {
+			continue
+		}
+
+		manifestFile := &manifestFile{
+			entries: SplitManifests(content),
+			path:    filePath,
+			apis:    apis,
+		}
+
+		if err := manifestFile.sortAll(result); err != nil {
+			return result.generic, err
+		}
+	}
+
+	return sortByKind(result.generic, ordering), nil
+}
+
+// sortAll takes a manifestFile object which may contain multiple resource definition
+// entries and sorts them based on the given ordering. No regard is given
+// to wether the manifest contains a hook.
+//
+func (file *manifestFile) sortAll(result *result) error {
+	// Go through manifests in order found in file (function `SplitAllManifests` creates integer-sortable keys)
+	var sortedEntryKeys []string
+	for entryKey := range file.entries {
+		sortedEntryKeys = append(sortedEntryKeys, entryKey)
+	}
+	sort.Sort(BySplitManifestsOrder(sortedEntryKeys))
+
+	for _, entryKey := range sortedEntryKeys {
+		m := file.entries[entryKey]
+
+		var entry SimpleHead
+		if err := yaml.Unmarshal([]byte(m), &entry); err != nil {
+			return errors.Wrapf(err, "YAML parse error on %s", file.path)
+		}
+
+		result.generic = append(result.generic, Manifest{
+			Name:    file.path,
+			Content: m,
+			Head:    &entry,
+		})
+	}
+
+	return nil
+}
+
 // hasAnyAnnotation returns true if the given entry has any annotations at all.
 func hasAnyAnnotation(entry SimpleHead) bool {
 	return entry.Metadata != nil &&

pkg/releaseutil/manifest_sorter_test.go

@@ -17,6 +17,7 @@ limitations under the License.
 package releaseutil
 
 import (
+	"fmt"
 	"reflect"
 	"testing"
 
@@ -226,3 +227,152 @@ metadata:
 		}
 	}
 }
+
+func TestSortAllManifests(t *testing.T) {
+
+	data := []struct {
+		name     []string
+		path     string
+		kind     []string
+		hooks    map[string][]release.HookEvent
+		manifest string
+	}{
+		{
+			name:  []string{"first"},
+			path:  "one",
+			kind:  []string{"Job"},
+			hooks: map[string][]release.HookEvent{"first": {release.HookPreInstall}},
+			manifest: `apiVersion: v1
+kind: Job
+metadata:
+  name: first
+  labels:
+    doesnot: matter
+  annotations:
+    "helm.sh/hook": pre-install
+`,
+		},
+		{
+			name:  []string{"second"},
+			path:  "two",
+			kind:  []string{"ReplicaSet"},
+			hooks: map[string][]release.HookEvent{"second": {release.HookPostInstall}},
+			manifest: `kind: ReplicaSet
+apiVersion: v1beta1
+metadata:
+  name: second
+  annotations:
+    "helm.sh/hook": post-install
+`,
+		}, {
+			name:  []string{"third"},
+			path:  "three",
+			kind:  []string{"ReplicaSet"},
+			hooks: map[string][]release.HookEvent{"third": {release.HookPreDelete}},
+			manifest: `kind: ReplicaSet
+apiVersion: v1beta1
+metadata:
+  name: third
+  annotations:
+    "helm.sh/hook": pre-delete
+`,
+		}, {
+			name:  []string{"fourth"},
+			path:  "four",
+			kind:  []string{"Pod"},
+			hooks: map[string][]release.HookEvent{"fourth": nil},
+			manifest: `kind: Pod
+apiVersion: v1
+metadata:
+  name: fourth
+  annotations:
+    nothing: here`,
+		}, {
+			name:  []string{"fifth"},
+			path:  "five",
+			kind:  []string{"ReplicaSet"},
+			hooks: map[string][]release.HookEvent{"fifth": {release.HookPostDelete, release.HookPostInstall}},
+			manifest: `kind: ReplicaSet
+apiVersion: v1beta1
+metadata:
+  name: fifth
+  annotations:
+    "helm.sh/hook": post-delete, post-install
+`,
+		}, {
+			name:  []string{"sixth", "example-test"},
+			path:  "six",
+			kind:  []string{"ConfigMap", "Pod"},
+			hooks: map[string][]release.HookEvent{"sixth": nil, "example-test": {release.HookTest}},
+			manifest: `kind: ConfigMap
+apiVersion: v1
+metadata:
+  name: sixth
+data:
+  name: value
+---
+apiVersion: v1
+kind: Pod
+metadata:
+  name: example-test
+  annotations:
+    "helm.sh/hook": test
+`,
+		},
+	}
+
+	manifests := make(map[string]string, len(data))
+	for i, o := range data {
+		manifests[fmt.Sprintf(manifestTpl, i)] = o.manifest
+	}
+
+	generic, err := SortAllManifests(manifests, chartutil.VersionSet{"v1", "v1beta1"}, InstallOrder)
+	if err != nil {
+		t.Fatalf("Unexpected error: %s", err)
+	}
+
+	// NOTE: 'six' includes two manifests.
+	if len(generic) != 7 {
+		t.Errorf("Expected 7 generic manifests, got %d", len(generic))
+	}
+
+	// Verify the sort order
+	noHooks := []Manifest{}
+	hooks := []Manifest{}
+	for i, s := range data {
+		manifests := SplitManifests(s.manifest)
+
+		for _, m := range manifests {
+			var sh SimpleHead
+			if err := yaml.Unmarshal([]byte(m), &sh); err != nil {
+				// This is expected for manifests that are corrupt or empty.
+				t.Log(err)
+				continue
+			}
+
+			name := sh.Metadata.Name
+
+			if s.hooks[name] == nil {
+				noHooks = append(noHooks, Manifest{
+					Content: m,
+					Name:    fmt.Sprintf(manifestTpl, i),
+					Head:    &sh,
+				})
+			} else {
+				hooks = append(hooks, Manifest{
+					Content: m,
+					Name:    fmt.Sprintf(manifestTpl, i),
+					Head:    &sh,
+				})
+			}
+		}
+	}
+
+	sorted := append(noHooks, hooks...)
+	sorted = sortByKind(sorted, InstallOrder)
+	for i, m := range generic {
+		if m.Content != sorted[i].Content {
+			t.Errorf("Expected %q, got %q", m.Content, sorted[i].Content)
+		}
+	}
+}

pkg/releaseutil/manifest_test.go

@@ -19,6 +19,8 @@ package releaseutil // import "helm.sh/helm/v3/pkg/releaseutil"
 import (
 	"reflect"
 	"testing"
+
+	"helm.sh/helm/v3/pkg/release"
 )
 
 const mockManifestFile = `
@@ -59,3 +61,18 @@ func TestSplitManifest(t *testing.T) {
 		t.Errorf("Expected %v, got %v", expected, manifests)
 	}
 }
+
+func TestSplitAllManifests(t *testing.T) {
+	mockRelease := release.Mock(&release.MockReleaseOptions{})
+	manifests := SplitAllManifests(mockRelease)
+	if len(manifests) != 2 {
+		t.Errorf("Expected 2 manifests (including one containing a hook), got %v", len(manifests))
+	}
+	if len(mockRelease.Hooks) != 1 {
+		t.Errorf("Expected 1 hook, got %v", len(mockRelease.Hooks))
+	}
+	manifestsLessHooks := SplitManifests(mockRelease.Manifest)
+	if len(manifestsLessHooks) != 1 {
+		t.Errorf("Expected 1 manifest without a hook, got %v", len(manifestsLessHooks))
+	}
+}