Merge 03811bf515 into 35f6196167

pkg/action/action.go

@@ -162,7 +162,7 @@ func (cfg *Configuration) renderResources(ch *chart.Chart, values chartutil.Valu
 	// Sort hooks, manifests, and partials. Only hooks and manifests are returned,
 	// as partials are not used after renderer.Render. Empty manifests are also
 	// removed here.
-	hs, manifests, err := releaseutil.SortManifests(files, caps.APIVersions, releaseutil.InstallOrder)
+	hs, manifests, err := releaseutil.SortManifests(files, caps.APIVersions, false)
 	if err != nil {
 		// By catching parse errors here, we can prevent bogus releases from going
 		// to Kubernetes.

pkg/action/uninstall.go

@@ -191,7 +191,7 @@ func (u *Uninstall) deleteRelease(rel *release.Release) (kube.ResourceList, stri
 	}
 
 	manifests := releaseutil.SplitManifests(rel.Manifest)
-	_, files, err := releaseutil.SortManifests(manifests, caps.APIVersions, releaseutil.UninstallOrder)
+	_, files, err := releaseutil.SortManifests(manifests, caps.APIVersions, true)
 	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/kind_sorter.go

@@ -108,34 +108,30 @@ var UninstallOrder KindSortOrder = []string{
 // sort manifests by kind.
 //
 // Results are sorted by 'ordering', keeping order of items with equal kind/priority
-func sortManifestsByKind(manifests []Manifest, ordering KindSortOrder) []Manifest {
-	sort.SliceStable(manifests, func(i, j int) bool {
-		return lessByKind(manifests[i], manifests[j], manifests[i].Head.Kind, manifests[j].Head.Kind, ordering)
+func sortManifestsByKind(manifests []Manifest, uninstall bool) []Manifest {
+	m := manifests
+	sort.SliceStable(m, func(i, j int) bool {
+		return lessByKind(m[i], m[j], m[i].Head.Kind, m[j].Head.Kind, m[i].InstallBefore, m[j].InstallBefore, uninstall)
 	})
 
-	return manifests
+	return m
 }
 
 // sort hooks by kind, using an out-of-place sort to preserve the input parameters.
 //
 // Results are sorted by 'ordering', keeping order of items with equal kind/priority
-func sortHooksByKind(hooks []*release.Hook, ordering KindSortOrder) []*release.Hook {
+func sortHooksByKind(hooks []*release.Hook, uninstall bool) []*release.Hook {
 	h := hooks
 	sort.SliceStable(h, func(i, j int) bool {
-		return lessByKind(h[i], h[j], h[i].Kind, h[j].Kind, ordering)
+		return lessByKind(h[i], h[j], h[i].Kind, h[j].Kind, []string{}, []string{}, uninstall)
 	})
 
 	return h
 }
 
-func lessByKind(a interface{}, b interface{}, kindA string, kindB string, o KindSortOrder) bool {
-	ordering := make(map[string]int, len(o))
-	for v, k := range o {
-		ordering[k] = v
-	}
-
-	first, aok := ordering[kindA]
-	second, bok := ordering[kindB]
+func lessByKind(a interface{}, b interface{}, kindA string, kindB string, beforeA []string, beforeB []string, uninstall bool) bool {
+	first, aok := installOrderIndex(kindA, beforeA, uninstall)
+	second, bok := installOrderIndex(kindB, beforeB, uninstall)
 
 	if !aok && !bok {
 		// if both are unknown then sort alphabetically by kind, keep original order if same kind
@@ -154,3 +150,53 @@ func lessByKind(a interface{}, b interface{}, kindA string, kindB string, o Kind
 	// sort different kinds, keep original order if same priority
 	return first < second
 }
+
+// installOrderIndex returns the lowest index number of all beforeKinds
+func installOrderIndex(kind string, beforeKinds []string, uninstall bool) (int, bool) {
+	order := InstallOrder
+	if uninstall {
+		order = UninstallOrder
+	}
+
+	ordering := make(map[string]int, len(order))
+	for v, k := range order {
+		// In order to allow placing custom resources in between existing resources we need to double the index.
+		// for example
+		// NetworkPolicy has an index of 1
+		// ResourceQuota has an index of 2
+		// if we want to place a custom resource in between we would need to make the index of that resource 1.5
+		// since we use int numbers we cannot use floating point numbers, so instead we just DOUBLE the index of
+		// everything so that our Custom Resource fits in between (2 and 4 in this case)
+		ordering[k] = v * 2
+	}
+
+	orderIndex, foundIndex := ordering[kind]
+
+	// reset orderIndex for unknown resources
+	// when we're uninstalling we're actually searching for the HIGHEST index, so 0 is fine as initial value
+	if !foundIndex && !uninstall {
+		// see above why we use double the length
+		orderIndex = len(order) * 2
+	}
+
+	for _, kind := range beforeKinds {
+		i, ok := ordering[kind]
+		if !ok {
+			continue
+		}
+		// we're searching for the lowest index when installing
+		if i < orderIndex && !uninstall {
+			foundIndex = true
+			// set orderIndex 1 BEFORE the actual index, so it get installed BEFORE it
+			orderIndex = i - 1
+		}
+		// we're searching for the highest index when uninstalling
+		if i > orderIndex && uninstall {
+			foundIndex = true
+			// set orderIndex 1 AFTER the actual index, so it get uninstalled AFTER it
+			orderIndex = i + 1
+		}
+	}
+
+	return orderIndex, foundIndex
+}

pkg/releaseutil/kind_sorter_test.go

@@ -18,6 +18,7 @@ package releaseutil
 
 import (
 	"bytes"
+	"reflect"
 	"testing"
 
 	"helm.sh/helm/v3/pkg/release"
@@ -165,15 +166,30 @@ func TestKindSorter(t *testing.T) {
 			Name: "x",
 			Head: &SimpleHead{Kind: "HorizontalPodAutoscaler"},
 		},
+		{
+			Name:          "9",
+			Head:          &SimpleHead{Kind: "MyCustomResourceBeforeDeployment"},
+			InstallBefore: []string{"Deployment"},
+		},
+		{
+			Name:          "8",
+			Head:          &SimpleHead{Kind: "MyCustomResourceBeforeEverything"},
+			InstallBefore: InstallOrder,
+		},
+		{
+			Name:          "7",
+			Head:          &SimpleHead{Kind: "MyCustomResourceBeforeMultipleResources"},
+			InstallBefore: []string{"Deployment", "Service", "ServiceAccount"},
+		},
 	}
 
 	for _, test := range []struct {
 		description string
-		order       KindSortOrder
+		uninstall   bool
 		expected    string
 	}{
-		{"install", InstallOrder, "aAbcC3deEf1gh2iIjJkKlLmnopqrxstuvw!"},
-		{"uninstall", UninstallOrder, "wvmutsxrqponLlKkJjIi2hg1fEed3CcbAa!"},
+		{"install", false, "8aAbcC37deEf1gh2iIjJkKlLmnopq9rxstuvw!"},
+		{"uninstall", true, "wvmutsxr9qponLlKkJjIi2hg1fEed73CcbAa8!"},
 	} {
 		var buf bytes.Buffer
 		t.Run(test.description, func(t *testing.T) {
@@ -182,14 +198,14 @@ func TestKindSorter(t *testing.T) {
 			}
 			defer buf.Reset()
 			orig := manifests
-			for _, r := range sortManifestsByKind(manifests, test.order) {
+			for _, r := range sortManifestsByKind(manifests, test.uninstall) {
 				buf.WriteString(r.Name)
 			}
 			if got := buf.String(); got != test.expected {
 				t.Errorf("Expected %q, got %q", test.expected, got)
 			}
 			for i, manifest := range orig {
-				if manifest != manifests[i] {
+				if !reflect.DeepEqual(manifest, manifests[i]) {
 					t.Fatal("Expected input to sortManifestsByKind to stay the same")
 				}
 			}
@@ -239,16 +255,16 @@ func TestKindSorterKeepOriginalOrder(t *testing.T) {
 	}
 	for _, test := range []struct {
 		description string
-		order       KindSortOrder
+		uninstall   bool
 		expected    string
 	}{
 		// expectation is sorted by kind (unknown is last) and within each group of same kind, the order is kept
-		{"cm,clusterRole,clusterRoleBinding,Unknown,Unknown2", InstallOrder, "01aAz!u2u1t3"},
+		{"cm,clusterRole,clusterRoleBinding,Unknown,Unknown2", false, "01aAz!u2u1t3"},
 	} {
 		var buf bytes.Buffer
 		t.Run(test.description, func(t *testing.T) {
 			defer buf.Reset()
-			for _, r := range sortManifestsByKind(manifests, test.order) {
+			for _, r := range sortManifestsByKind(manifests, test.uninstall) {
 				buf.WriteString(r.Name)
 			}
 			if got := buf.String(); got != test.expected {
@@ -269,7 +285,7 @@ func TestKindSorterNamespaceAgainstUnknown(t *testing.T) {
 	}
 
 	manifests := []Manifest{unknown, namespace}
-	manifests = sortManifestsByKind(manifests, InstallOrder)
+	manifests = sortManifestsByKind(manifests, false)
 
 	expectedOrder := []Manifest{namespace, unknown}
 	for i, manifest := range manifests {
@@ -302,11 +318,11 @@ func TestKindSorterForHooks(t *testing.T) {
 
 	for _, test := range []struct {
 		description string
-		order       KindSortOrder
+		uninstall   bool
 		expected    string
 	}{
-		{"install", InstallOrder, "acij"},
-		{"uninstall", UninstallOrder, "jica"},
+		{"install", false, "acij"},
+		{"uninstall", true, "jica"},
 	} {
 		var buf bytes.Buffer
 		t.Run(test.description, func(t *testing.T) {
@@ -315,7 +331,7 @@ func TestKindSorterForHooks(t *testing.T) {
 			}
 			defer buf.Reset()
 			orig := hooks
-			for _, r := range sortHooksByKind(hooks, test.order) {
+			for _, r := range sortHooksByKind(hooks, test.uninstall) {
 				buf.WriteString(r.Name)
 			}
 			for i, hook := range orig {

pkg/releaseutil/manifest.go

@@ -23,6 +23,9 @@ import (
 	"strings"
 )
 
+// InstallOrderAnnotation the annotation that allows manipulating the install order of custom resources
+const InstallOrderAnnotation = "helm.sh/install-before"
+
 // SimpleHead defines what the structure of the head of a manifest file
 type SimpleHead struct {
 	Version  string `json:"apiVersion"`

pkg/releaseutil/manifest_sorter.go

@@ -35,6 +35,7 @@ type Manifest struct {
 	Name          string
 	Content       string
 	Head          *SimpleHead
+	InstallBefore []string
 }
 
 // manifestFile represents a file that contains a manifest.
@@ -75,7 +76,7 @@ var events = map[string]release.HookEvent{
 //
 // Files that do not parse into the expected format are simply placed into a map and
 // returned.
-func SortManifests(files map[string]string, apis chartutil.VersionSet, ordering KindSortOrder) ([]*release.Hook, []Manifest, error) {
+func SortManifests(files map[string]string, apis chartutil.VersionSet, uninstall bool) ([]*release.Hook, []Manifest, error) {
 	result := &result{}
 
 	var sortedFilePaths []string
@@ -108,7 +109,7 @@ func SortManifests(files map[string]string, apis chartutil.VersionSet, ordering
 		}
 	}
 
-	return sortHooksByKind(result.hooks, ordering), sortManifestsByKind(result.generic, ordering), nil
+	return sortHooksByKind(result.hooks, uninstall), sortManifestsByKind(result.generic, uninstall), nil
 }
 
 // sort takes a manifestFile object which may contain multiple resource definition
@@ -155,6 +156,30 @@ func (file *manifestFile) sort(result *result) error {
 			continue
 		}
 
+		installBeforeKinds, ok := entry.Metadata.Annotations[InstallOrderAnnotation]
+		// InstallOrderAnnotation is only supported for unknown Kinds e.g. Custom Resources
+		if ok && isKnownKind(entry.Kind) {
+			log.Printf("info: %v annotation is not supported for Kind %v", InstallOrderAnnotation, entry.Kind)
+		} else if ok {
+			var installBefore []string
+			for _, kind := range strings.Split(installBeforeKinds, ",") {
+				kind = strings.TrimSpace(kind)
+				if !isKnownKind(kind) {
+					log.Printf("info: skipping unknown install-before kind: %q", kind)
+					continue
+				}
+				installBefore = append(installBefore, kind)
+			}
+
+			result.generic = append(result.generic, Manifest{
+				Name:          file.path,
+				Content:       m,
+				Head:          &entry,
+				InstallBefore: installBefore,
+			})
+			continue
+		}
+
 		hookTypes, ok := entry.Metadata.Annotations[release.HookAnnotation]
 		if !ok {
 			result.generic = append(result.generic, Manifest{
@@ -231,3 +256,17 @@ func operateAnnotationValues(entry SimpleHead, annotation string, operate func(p
 		}
 	}
 }
+
+// isKnownKind returns true if the given kind exists in the InstallOrder AND UninstallOrder
+func isKnownKind(kind string) bool {
+	for _, k := range InstallOrder {
+		if k == kind {
+			for _, kk := range UninstallOrder {
+				if kk == kind {
+					return true
+				}
+			}
+		}
+	}
+	return false
+}

pkg/releaseutil/manifest_sorter_test.go

@@ -139,7 +139,7 @@ metadata:
 		manifests[o.path] = o.manifest
 	}
 
-	hs, generic, err := SortManifests(manifests, chartutil.VersionSet{"v1", "v1beta1"}, InstallOrder)
+	hs, generic, err := SortManifests(manifests, chartutil.VersionSet{"v1", "v1beta1"}, false)
 	if err != nil {
 		t.Fatalf("Unexpected error: %s", err)
 	}
@@ -219,7 +219,7 @@ metadata:
 		}
 	}
 
-	sorted = sortManifestsByKind(sorted, InstallOrder)
+	sorted = sortManifestsByKind(sorted, false)
 	for i, m := range generic {
 		if m.Content != sorted[i].Content {
 			t.Errorf("Expected %q, got %q", m.Content, sorted[i].Content)