Skip to content

Commit

Permalink
Fix yaml separator [v0.1.x] (#43)
Browse files Browse the repository at this point in the history
* add newline at end of yaml separator

* cl

* move var

* add tests
  • Loading branch information
jenshu authored Sep 8, 2023
1 parent 8176b42 commit 376290d
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 9 deletions.
5 changes: 5 additions & 0 deletions changelog/v0.1.11/fix-yaml-separator.yaml
Original file line number Diff line number Diff line change
@@ -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.
4 changes: 2 additions & 2 deletions installutils/helmchart/manifests.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
51 changes: 51 additions & 0 deletions manifesttestutils/test/example_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
})
})
})
8 changes: 1 addition & 7 deletions manifesttestutils/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@ import (
"k8s.io/api/extensions/v1beta1"
rbacv1 "k8s.io/api/rbac/v1"

"regexp"

. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"
"github.com/solo-io/k8s-utils/installutils/helmchart"
Expand Down Expand Up @@ -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 {
Expand Down

0 comments on commit 376290d

Please sign in to comment.