fix: HOOKS section should also be parsed when HELM_DIFF_USE_UPGRADE_DRY_RUN=true (#364)

* HOOKS section should also be parsed when HELM_DIFF_USE_UPGRADE_DRY_RUN=true

* fix for charts that don't have any hooks

* change condition operator on if statement

* fix sections extract if only HOOKS sections is present

* fixup! fix sections extract if only HOOKS sections is present

Co-authored-by: crmendes <[email protected]>
Co-authored-by: Yusuke Kuoka <[email protected]>

Author: carlosrmendes

cmd/helm3.go

@@ -87,7 +87,7 @@ func (d *diffCmd) template(isUpgrade bool) ([]byte, error) {
 	if d.devel {
 		flags = append(flags, "--devel")
 	}
-	if d.noHooks {
+	if d.noHooks && !d.useUpgradeDryRun {
 		flags = append(flags, "--no-hooks")
 	}
 	if d.chartVersion != "" {
@@ -171,19 +171,7 @@ func (d *diffCmd) template(isUpgrade bool) ([]byte, error) {
 		flags = append(flags, "--dry-run")
 		subcmd = "upgrade"
 		filter = func(s []byte) []byte {
-			if len(s) == 0 {
-				return s
-			}
-
-			i := bytes.Index(s, []byte("MANIFEST:"))
-			s = s[i:]
-			i = bytes.Index(s, []byte("---"))
-			s = s[i:]
-			i = bytes.Index(s, []byte("\nNOTES:"))
-			if i != -1 {
-				s = s[:i+1]
-			}
-			return s
+			return extractManifestFromHelmUpgradeDryRunOutput(s, d.noHooks)
 		}
 	} else {
 		if !d.disableValidation && !d.dryRun {
@@ -220,3 +208,45 @@ func (d *diffCmd) writeExistingValues(f *os.File) error {
 	cmd.Stdout = f
 	return cmd.Run()
 }
+
+func extractManifestFromHelmUpgradeDryRunOutput(s []byte, noHooks bool) []byte {
+	if len(s) == 0 {
+		return s
+	}
+
+	i := bytes.Index(s, []byte("HOOKS:"))
+	hooks := s[i:]
+
+	j := bytes.Index(hooks, []byte("MANIFEST:"))
+
+	manifest := hooks[j:]
+	hooks = hooks[:j]
+
+	k := bytes.Index(manifest, []byte("\nNOTES:"))
+
+	if k > -1 {
+		manifest = manifest[:k+1]
+	}
+
+	if noHooks {
+		hooks = nil
+	} else {
+		a := bytes.Index(hooks, []byte("---"))
+		if a > -1 {
+			hooks = hooks[a:]
+		} else {
+			hooks = nil
+		}
+	}
+
+	a := bytes.Index(manifest, []byte("---"))
+	if a > -1 {
+		manifest = manifest[a:]
+	}
+
+	r := []byte{}
+	r = append(r, manifest...)
+	r = append(r, hooks...)
+
+	return r
+}

cmd/helm3_test.go

@@ -0,0 +1,137 @@
+package cmd
+
+import (
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+)
+
+func TestExtractManifestFromHelmUpgradeDryRunOutput(t *testing.T) {
+	type testdata struct {
+		description string
+
+		s       string
+		noHooks bool
+
+		want string
+	}
+
+	manifest := `---
+# Source: mysql/templates/secrets.yaml
+apiVersion: v1
+kind: Secret
+metadata:
+  name: my1-mysql
+  namespace: default
+  labels:
+	app: my1-mysql
+	chart: "mysql-1.6.9"
+	release: "my1"
+	heritage: "Helm"
+type: Opaque
+data:
+  mysql-root-password: "ZlhEVGJseUhmeg=="
+  mysql-password: "YnRuU3pPOTJMVg=="
+---
+# Source: mysql/templates/tests/test-configmap.yaml
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: my1-mysql-test
+  namespace: default
+  labels:
+	app: my1-mysql
+	chart: "mysql-1.6.9"
+	heritage: "Helm"
+	release: "my1"
+data:
+  run.sh: |-
+
+`
+	hooks := `---
+# Source: mysql/templates/tests/test.yaml
+apiVersion: v1
+kind: Pod
+metadata:
+  name: my1-mysql-test
+  namespace: default
+  labels:
+	app: my1-mysql
+	chart: "mysql-1.6.9"
+	heritage: "Helm"
+	release: "my1"
+  annotations:
+	"helm.sh/hook": test-success
+spec:
+  containers:
+	- name: my1-test
+	  image: "bats/bats:1.2.1"
+	  imagePullPolicy: "IfNotPresent"
+	  command: ["/opt/bats/bin/bats", "-t", "/tests/run.sh"]
+`
+
+	header := `Release "my1" has been upgraded. Happy Helming!
+NAME: my1
+LAST DEPLOYED: Sun Feb 13 02:26:16 2022
+NAMESPACE: default
+STATUS: pending-upgrade
+REVISION: 2
+HOOKS:
+`
+
+	notes := `NOTES:
+MySQL can be accessed via port 3306 on the following DNS name from within your cluster:
+my1-mysql.default.svc.cluster.local
+	
+*snip*
+	
+To connect to your database directly from outside the K8s cluster:
+	MYSQL_HOST=127.0.0.1
+	MYSQL_PORT=3306
+
+	# Execute the following command to route the connection:
+	kubectl port-forward svc/my1-mysql 3306
+
+	mysql -h ${MYSQL_HOST} -P${MYSQL_PORT} -u root -p${MYSQL_ROOT_PASSWORD}	
+`
+
+	outputWithHooks := header + hooks + "MANIFEST:\n" + manifest + notes
+	outputWithNoHooks := header + "MANIFEST:\n" + manifest + notes
+
+	testcases := []testdata{
+		{
+			description: "should output manifest when noHooks specified",
+			s:           outputWithHooks,
+			noHooks:     true,
+			want:        manifest,
+		},
+		{
+			description: "should output manifest and hooks when noHooks unspecified",
+			s:           outputWithHooks,
+			noHooks:     false,
+			want:        manifest + hooks,
+		},
+		{
+			description: "should output manifest if noHooks specified but input did not contain hooks",
+			s:           outputWithNoHooks,
+			noHooks:     true,
+			want:        manifest,
+		},
+		{
+			description: "should output manifest if noHooks unspecified and input did not contain hooks",
+			s:           outputWithNoHooks,
+			noHooks:     false,
+			want:        manifest,
+		},
+	}
+
+	for _, tc := range testcases {
+		t.Run(tc.description, func(t *testing.T) {
+			got := extractManifestFromHelmUpgradeDryRunOutput([]byte(tc.s), tc.noHooks)
+
+			if d := cmp.Diff(tc.want, string(got)); d != "" {
+				t.Errorf("unexpected diff: %s", d)
+			}
+		})
+	}
+}