Keep new line at end of file in yamlprocessor (#4033)

* Keep new line at end of file in yamlprocessor * add a new line to the EOF in case of there is no at EOF * update comment * update eventwatcher_test * consider new line at EOF on parsing kubernetes manifest * update go-yaml v1.9.8 * remove TODO because PR of go-yaml was merged * consider to handle multiple new line at EOF * remove indent vioration of tab
pipe-cd · Jan 10, 2023 · d0bf92e · d0bf92e
1 parent 93dc8a4
commit d0bf92e
Show file tree

Hide file tree

Showing 11 changed files with 125 additions and 40 deletions.
diff --git a/go.mod b/go.mod
@@ -20,7 +20,7 @@ require (
 	github.com/envoyproxy/protoc-gen-validate v0.6.6
 	github.com/fsouza/fake-gcs-server v1.21.0
 	github.com/go-sql-driver/mysql v1.6.0
-	github.com/goccy/go-yaml v1.9.3
+	github.com/goccy/go-yaml v1.9.8
 	github.com/golang-jwt/jwt v3.2.1+incompatible
 	github.com/golang/mock v1.5.0
 	github.com/golang/protobuf v1.5.2

diff --git a/go.sum b/go.sum
@@ -272,6 +272,8 @@ github.com/go-test/deep v1.0.3 h1:ZrJSEWsXzPOxaZnFteGEfooLba+ju3FYIbOrS+rQd68=
 github.com/go-test/deep v1.0.3/go.mod h1:wGDj63lr65AM2AQyKZd/NYHGb0R+1RLqB8NKt3aSFNA=
 github.com/goccy/go-yaml v1.9.3 h1:9A7DkTBb7cZs5wqcqAhgR+2Ms8O7HTjT0SqOXO10HqM=
 github.com/goccy/go-yaml v1.9.3/go.mod h1:U/jl18uSupI5rdI2jmuCswEA2htH9eXfferR3KfscvA=
+github.com/goccy/go-yaml v1.9.8 h1:5gMyLUeU1/6zl+WFfR1hN7D2kf+1/eRGa7DFtToiBvQ=
+github.com/goccy/go-yaml v1.9.8/go.mod h1:JubOolP3gh0HpiBc4BLRD4YmjEjHAmIIB2aaXKkTfoE=
 github.com/godbus/dbus/v5 v5.0.4/go.mod h1:xhWf0FNVPg57R7Z0UbKHbJfkEywrmjJnf7w5xrFpKfA=
 github.com/godbus/dbus/v5 v5.0.6/go.mod h1:xhWf0FNVPg57R7Z0UbKHbJfkEywrmjJnf7w5xrFpKfA=
 github.com/gofrs/uuid v3.2.0+incompatible h1:y12jRkkFxsd7GpqdSZ+/KCs/fJbqpEXSGd4+jfEaewE=
@@ -877,6 +879,7 @@ golang.org/x/sys v0.0.0-20211025201205-69cdffdb9359/go.mod h1:oPkhp1MJrh7nUepCBc
 golang.org/x/sys v0.0.0-20211116061358-0a5406a5449c/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.0.0-20211216021012-1d35b9e2eb4e/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.0.0-20220209214540-3681064d5158/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
+golang.org/x/sys v0.0.0-20220406163625-3f8b81556e12/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.0.0-20220412211240-33da011f77ad h1:ntjMns5wyP/fN65tdBD4g8J5w8n015+iIIs9rtjXkY0=
 golang.org/x/sys v0.0.0-20220412211240-33da011f77ad/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=

diff --git a/pkg/app/piped/eventwatcher/eventwatcher_test.go b/pkg/app/piped/eventwatcher/eventwatcher_test.go
@@ -98,7 +98,7 @@ func TestModifyYAML(t *testing.T) {
 			path:         "testdata/a.yaml",
 			field:        "$.foo",
 			newValue:     "2",
-			wantNewYml:   []byte("foo: 2"),
+			wantNewYml:   []byte("foo: 2\n"),
 			wantUpToDate: false,
 			wantErr:      false,
 		},

diff --git a/pkg/app/piped/executor/kubernetes/kubernetes_test.go b/pkg/app/piped/executor/kubernetes/kubernetes_test.go
@@ -601,7 +601,7 @@ metadata:
   name: canary-by-config-change
 data:
   two: "2"
-			`,
+`,
 			expected: `
 apiVersion: apps/v1
 kind: Deployment
@@ -645,7 +645,7 @@ metadata:
   name: canary-by-config-change
 data:
   two: "2"
-			`,
+`,
 		},
 		{
 			name: "multiple configs",
@@ -704,7 +704,7 @@ metadata:
 type: my-type
 data:
   "one": "Mg=="
-			`,
+`,
 			expected: `
 apiVersion: apps/v1
 kind: Deployment
@@ -762,7 +762,7 @@ metadata:
 type: my-type
 data:
   "one": "Mg=="
-			`,
+`,
 		},
 	}
 

diff --git a/pkg/app/piped/executor/kubernetes/testdata/patch_configmap_field.yaml b/pkg/app/piped/executor/kubernetes/testdata/patch_configmap_field.yaml
@@ -3,7 +3,7 @@ kind: ConfigMap
 metadata:
   name: envoy-config
 data:
-  envoy-config: |-
+  envoy-config: |
     admin:
       address:
         socket_address:
@@ -42,7 +42,7 @@ kind: ConfigMap
 metadata:
   name: envoy-config
 data:
-  envoy-config: |-
+  envoy-config: |
     admin:
       address:
         socket_address:

diff --git a/pkg/app/piped/executor/kubernetes/testdata/patch_configmap_field_multi_ops.yaml b/pkg/app/piped/executor/kubernetes/testdata/patch_configmap_field_multi_ops.yaml
@@ -3,7 +3,7 @@ kind: ConfigMap
 metadata:
   name: envoy-config
 data:
-  envoy-config: |-
+  envoy-config: |
     admin:
       address:
         socket_address:
@@ -151,7 +151,7 @@ kind: ConfigMap
 metadata:
   name: envoy-config
 data:
-  envoy-config: |-
+  envoy-config: |
     admin:
       address:
         socket_address:

diff --git a/pkg/app/piped/platformprovider/kubernetes/deployment_test.go b/pkg/app/piped/platformprovider/kubernetes/deployment_test.go
@@ -61,7 +61,7 @@ spec:
           - server
         ports:
         - containerPort: 9085
-		  `,
+`,
 			expected: nil,
 		},
 		{
@@ -100,7 +100,7 @@ spec:
         - name: config
           configMap:
             name: canary-by-config-change
-			`,
+`,
 			expected: []string{
 				"canary-by-config-change",
 			},
@@ -164,7 +164,7 @@ spec:
         - name: config2
           configMap:
             name: configmap-2
-			`,
+`,
 			expected: []string{
 				"canary-by-config-change",
 				"configmap-1",
@@ -228,7 +228,7 @@ spec:
           - server
         ports:
         - containerPort: 9085
-		  `,
+`,
 			expected: nil,
 		},
 		{
@@ -267,7 +267,7 @@ spec:
         - name: config
           secret:
             secretName: canary-by-config-change
-			`,
+`,
 			expected: []string{
 				"canary-by-config-change",
 			},
@@ -331,7 +331,7 @@ spec:
         - name: config2
           secret:
             secretName: secret-2
-			`,
+`,
 			expected: []string{
 				"canary-by-config-change",
 				"init-secret-1",

diff --git a/pkg/app/piped/platformprovider/kubernetes/hasher_test.go b/pkg/app/piped/platformprovider/kubernetes/hasher_test.go
@@ -42,7 +42,7 @@ apiVersion: v1
 kind: ConfigMap
 data: {}
 binaryData: {}
-			`,
+`,
 			expected: "42745tchd9",
 		},
 		{
@@ -53,7 +53,7 @@ kind: ConfigMap
 data:
   one: ""
 binaryData: {}
-			`,
+`,
 			expected: "9g67k2htb6",
 		},
 		{
@@ -66,7 +66,7 @@ data:
   one: ""
   three: "3"
 binaryData: {}
-			`,
+`,
 			expected: "f5h7t85m9b",
 		},
 		{
@@ -76,7 +76,7 @@ apiVersion: v1
 kind: Secret
 type: my-type
 data: {}
-			`,
+`,
 			expected: "t75bgf6ctb",
 		},
 		{
@@ -87,7 +87,7 @@ kind: Secret
 type: my-type
 data:
   "one": ""
-			`,
+`,
 			expected: "74bd68bm66",
 		},
 		{
@@ -100,7 +100,7 @@ data:
   two: Mg==
   one: ""
   three: Mw==
-			`,
+`,
 			expected: "dgcb6h9tmk",
 		},
 		{
@@ -119,7 +119,7 @@ type: my-type
 data:
   one: ""
   three: Mw==
-			`,
+`,
 			expected: "57hhd7795k",
 		},
 		{
@@ -151,7 +151,7 @@ spec:
           - hello
         ports:
         - containerPort: 9085
-			`,
+`,
 			expected: "db48kd6689",
 		},
 	}

diff --git a/pkg/app/piped/platformprovider/kubernetes/manifest.go b/pkg/app/piped/platformprovider/kubernetes/manifest.go
@@ -224,12 +224,15 @@ func ParseManifests(data string) ([]Manifest, error) {
 		manifests = make([]Manifest, 0, len(parts))
 	)
 
-	for _, part := range parts {
+	for i, part := range parts {
 		// Ignore all the cases where no content between separator.
-		part = strings.TrimSpace(part)
-		if len(part) == 0 {
+		if len(strings.TrimSpace(part)) == 0 {
 			continue
 		}
+		// Append new line which trim by document separator.
+		if i != len(parts)-1 {
+			part += "\n"
+		}
 		var obj unstructured.Unstructured
 		if err := yaml.Unmarshal([]byte(part), &obj); err != nil {
 			return nil, err

diff --git a/pkg/app/piped/platformprovider/kubernetes/manifest_test.go b/pkg/app/piped/platformprovider/kubernetes/manifest_test.go
@@ -81,28 +81,104 @@ metadata:
 				}),
 			},
 		},
+		{
+			name: "contains new line at the end of file",
+			manifests: `
+apiVersion: v1
+kind: Kind1
+metadata:
+  name: config
+  extra: |
+    single-new-line
+`,
+			want: []Manifest{
+				maker("config", "Kind1", map[string]interface{}{
+					"name":  "config",
+					"extra": "single-new-line\n",
+				}),
+			},
+		},
+		{
+			name: "not contains new line at the end of file",
+			manifests: `
+apiVersion: v1
+kind: Kind1
+metadata:
+  name: config
+  extra: |
+    no-new-line`,
+			want: []Manifest{
+				maker("config", "Kind1", map[string]interface{}{
+					"name":  "config",
+					"extra": "no-new-line",
+				}),
+			},
+		},
 		{
 			name: "multiple manifests",
 			manifests: `
 apiVersion: v1
 kind: Kind1
 metadata:
   name: config1
+  extra: |-
+    no-new-line
 ---
 apiVersion: v1
 kind: Kind2
 metadata:
   name: config2
+  extra: |
+    single-new-line-1
 ---
 apiVersion: v1
 kind: Kind3
 metadata:
   name: config3
-			`,
+  extra: |
+    single-new-line-2
+
+
+---
+apiVersion: v1
+kind: Kind4
+metadata:
+  name: config4
+  extra: |+
+    multiple-new-line-1
+
+
+---
+apiVersion: v1
+kind: Kind5
+metadata:
+  name: config5
+  extra: |+
+    multiple-new-line-2
+
+
+`,
 			want: []Manifest{
-				maker("config1", "Kind1", map[string]interface{}{"name": "config1"}),
-				maker("config2", "Kind2", map[string]interface{}{"name": "config2"}),
-				maker("config3", "Kind3", map[string]interface{}{"name": "config3"}),
+				maker("config1", "Kind1", map[string]interface{}{
+					"name":  "config1",
+					"extra": "no-new-line",
+				}),
+				maker("config2", "Kind2", map[string]interface{}{
+					"name":  "config2",
+					"extra": "single-new-line-1\n",
+				}),
+				maker("config3", "Kind3", map[string]interface{}{
+					"name":  "config3",
+					"extra": "single-new-line-2\n",
+				}),
+				maker("config4", "Kind4", map[string]interface{}{
+					"name":  "config4",
+					"extra": "multiple-new-line-1\n\n\n",
+				}),
+				maker("config5", "Kind5", map[string]interface{}{
+					"name":  "config5",
+					"extra": "multiple-new-line-2\n\n\n",
+				}),
 			},
 		},
 	}