From d0bf92efc25ec126618204f04b6827932002af31 Mon Sep 17 00:00:00 2001 From: Kurochan Date: Tue, 10 Jan 2023 11:06:31 +0900 Subject: [PATCH] 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 --- go.mod | 2 +- go.sum | 3 + .../piped/eventwatcher/eventwatcher_test.go | 2 +- .../executor/kubernetes/kubernetes_test.go | 8 +- .../testdata/patch_configmap_field.yaml | 4 +- .../patch_configmap_field_multi_ops.yaml | 4 +- .../kubernetes/deployment_test.go | 12 +-- .../kubernetes/hasher_test.go | 16 ++-- .../platformprovider/kubernetes/manifest.go | 9 +- .../kubernetes/manifest_test.go | 84 ++++++++++++++++++- pkg/yamlprocessor/yamlprocessor_test.go | 21 +++-- 11 files changed, 125 insertions(+), 40 deletions(-) diff --git a/go.mod b/go.mod index ca2c922453..7c7971dbf9 100644 --- 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 index bc961e5196..b0e2662ea2 100644 --- 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 index 291d658e78..5164f427fd 100644 --- 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 index 039e19201d..d2406e2f4a 100644 --- 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 index adc653e76f..56e406b95c 100644 --- 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 index 3553d6b2d1..efeb218de7 100644 --- 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 index d96efa48e4..5c1b1c3e78 100644 --- 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 index 709d8c4c8f..19b486a05b 100644 --- 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 index fe3d0af38a..3bbe31e323 100644 --- 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 index fdf8a4fa76..3066cfcb31 100644 --- a/pkg/app/piped/platformprovider/kubernetes/manifest_test.go +++ b/pkg/app/piped/platformprovider/kubernetes/manifest_test.go @@ -81,6 +81,39 @@ 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: ` @@ -88,21 +121,64 @@ 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", + }), }, }, } diff --git a/pkg/yamlprocessor/yamlprocessor_test.go b/pkg/yamlprocessor/yamlprocessor_test.go index f418e178fe..8599da84cc 100644 --- a/pkg/yamlprocessor/yamlprocessor_test.go +++ b/pkg/yamlprocessor/yamlprocessor_test.go @@ -48,7 +48,7 @@ b: bv c: - 1 - 2 - `, +`, }, } for _, tc := range testcases { @@ -190,7 +190,7 @@ func TestReplaceString(t *testing.T) { yml: "foo: bar", path: "$.foo", value: "", - want: []byte("foo: "), + want: []byte("foo: \n"), wantErr: false, }, { @@ -198,7 +198,7 @@ func TestReplaceString(t *testing.T) { yml: "foo: bar", path: "$.foo", value: "new-text", - want: []byte("foo: new-text"), + want: []byte("foo: new-text\n"), wantErr: false, }, { @@ -208,16 +208,17 @@ foo: bar`, path: "$.foo", value: "new-text", want: []byte(`# comments -foo: new-text`), +foo: new-text +`), wantErr: false, }, { name: "valid value with comment at the same line", yml: `foo: bar # comments - `, +`, path: "$.foo", value: "new-text", - want: []byte("foo: new-text # comments"), + want: []byte("foo: new-text # comments\n"), wantErr: false, }, { @@ -229,7 +230,8 @@ foo: new-text`), value: "new-text", want: []byte(`foo: - new-text - - baz`), + - baz +`), wantErr: false, }, { @@ -237,7 +239,7 @@ foo: new-text`), yml: `foo: [bar, baz]`, path: "$.foo[0]", value: "new-text", - want: []byte(`foo: [new-text, baz]`), + want: []byte("foo: [new-text, baz]\n"), wantErr: false, }, { @@ -250,7 +252,8 @@ foo: value: "new-text", want: []byte(`foo: - new-text - - baz`), + - baz +`), wantErr: false, }, }