From f5848bf3552c8142633969eda40b5e5166addfaa Mon Sep 17 00:00:00 2001
From: Jenny Shu <28537278+jenshu@users.noreply.github.com>
Date: Thu, 7 Sep 2023 19:58:38 -0400
Subject: [PATCH] Fix yaml separator (#41)

* add newline at end of yaml separator

* ch

* move var

* add tests
---
 changelog/v0.5.1/fix-yaml-separator.yaml |  5 +++
 installutils/helmchart/manifests.go      |  4 +-
 manifesttestutils/test/example_test.go   | 51 ++++++++++++++++++++++++
 manifesttestutils/util.go                |  8 +---
 4 files changed, 59 insertions(+), 9 deletions(-)
 create mode 100644 changelog/v0.5.1/fix-yaml-separator.yaml

diff --git a/changelog/v0.5.1/fix-yaml-separator.yaml b/changelog/v0.5.1/fix-yaml-separator.yaml
new file mode 100644
index 0000000..add8b5c
--- /dev/null
+++ b/changelog/v0.5.1/fix-yaml-separator.yaml
@@ -0,0 +1,5 @@
+changelog:
+  - type: FIX
+    issueLink: https://github.com/solo-io/gloo/issues/8648
+    resolvesIssue: false
+    description: Add newline to end of yaml separator to better detect properly-formatted helm manifests.
\ No newline at end of file
diff --git a/installutils/helmchart/manifests.go b/installutils/helmchart/manifests.go
index d7ce6ee..6d31b93 100644
--- a/installutils/helmchart/manifests.go
+++ b/installutils/helmchart/manifests.go
@@ -73,10 +73,10 @@ func (m Manifests) CombinedString() string {
 	return buf.String()
 }
 
-var yamlSeparator = regexp.MustCompile("\n---")
+var YamlSeparator = regexp.MustCompile("\n---\n")
 
 func (m Manifests) ResourceList() (kuberesource.UnstructuredResources, error) {
-	snippets := yamlSeparator.Split(m.CombinedString(), -1)
+	snippets := YamlSeparator.Split(m.CombinedString(), -1)
 
 	var resources kuberesource.UnstructuredResources
 	for _, objectYaml := range snippets {
diff --git a/manifesttestutils/test/example_test.go b/manifesttestutils/test/example_test.go
index 23b29b4..f3f9407 100644
--- a/manifesttestutils/test/example_test.go
+++ b/manifesttestutils/test/example_test.go
@@ -277,4 +277,55 @@ var _ = Describe("Helm Test", func() {
 			testManifest.ExpectPermissions(permissions)
 		})
 	})
+
+	Context("manifest from yaml", func() {
+		It("has the right number of resources", func() {
+			manifestYaml := `
+apiVersion: v1
+kind: MyType
+metadata:
+  name: my-name
+  namespace: my-namespace
+spec:
+  blah: true
+---
+apiVersion: v1
+kind: AnotherType
+metadata:
+  name: another-name
+  namespace: another-namespace
+data:
+  some: thing
+`
+			manifestFromYaml := NewTestManifestFromYaml(manifestYaml)
+			Expect(manifestFromYaml.NumResources()).To(Equal(2))
+			manifestFromYaml.ExpectUnstructured("MyType", "my-namespace", "my-name").NotTo(BeNil())
+			manifestFromYaml.ExpectUnstructured("AnotherType", "another-namespace", "another-name").NotTo(BeNil())
+		})
+
+		// This is a test showing that yaml without proper splitting (e.g. `---` without a newline afterwards) results in
+		// the manifest not containing all the expected resources. This helps catch errors with missing newlines in manifests.
+		It("does not split resources properly when newline is missing", func() {
+			manifestYaml := `
+apiVersion: v1
+kind: MyType
+metadata:
+  name: my-name
+  namespace: my-namespace
+spec:
+  blah: true
+---apiVersion: v1
+kind: AnotherType
+metadata:
+  name: another-name
+  namespace: another-namespace
+data:
+  some: thing
+`
+			manifestFromYaml := NewTestManifestFromYaml(manifestYaml)
+			Expect(manifestFromYaml.NumResources()).To(Equal(1))
+			manifestFromYaml.ExpectUnstructured("MyType", "my-namespace", "my-name").To(BeNil())
+			manifestFromYaml.ExpectUnstructured("AnotherType", "another-namespace", "another-name").NotTo(BeNil())
+		})
+	})
 })
diff --git a/manifesttestutils/util.go b/manifesttestutils/util.go
index 44582f3..3789951 100644
--- a/manifesttestutils/util.go
+++ b/manifesttestutils/util.go
@@ -12,8 +12,6 @@ import (
 	"k8s.io/api/extensions/v1beta1"
 	rbacv1 "k8s.io/api/rbac/v1"
 
-	"regexp"
-
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	"github.com/solo-io/k8s-utils/installutils/helmchart"
@@ -313,17 +311,13 @@ func mustReadManifest(relativePathToManifest string) string {
 	return string(bytes)
 }
 
-var (
-	yamlSeparator = regexp.MustCompile("\n---")
-)
-
 func mustGetResourcesFromFile(relativePathToManifest string) kuberesource.UnstructuredResources {
 	manifest := mustReadManifest(relativePathToManifest)
 	return mustGetResourcesFromYaml(manifest)
 }
 
 func mustGetResourcesFromYaml(manifest string) kuberesource.UnstructuredResources {
-	snippets := yamlSeparator.Split(manifest, -1)
+	snippets := helmchart.YamlSeparator.Split(manifest, -1)
 
 	var resources kuberesource.UnstructuredResources
 	for _, objectYaml := range snippets {