From 81b5cf65d64d9685b79c9fc02b2fe1c3b473e7d1 Mon Sep 17 00:00:00 2001 From: Jingfang Liu Date: Thu, 6 Sep 2018 11:18:00 -0700 Subject: [PATCH] remove inline json patch format --- pkg/patch/jsonpatch.go | 7 --- pkg/patch/transformer/factory.go | 27 +++++---- pkg/patch/transformer/factory_test.go | 80 +++++++++++++-------------- 3 files changed, 53 insertions(+), 61 deletions(-) diff --git a/pkg/patch/jsonpatch.go b/pkg/patch/jsonpatch.go index 4247cccb8..c109eb5e3 100644 --- a/pkg/patch/jsonpatch.go +++ b/pkg/patch/jsonpatch.go @@ -16,10 +16,6 @@ limitations under the License. package patch -import ( - "github.com/krishicks/yaml-patch" -) - // PatchJson6902 represents a json patch for an object // with format documented https://tools.ietf.org/html/rfc6902. type PatchJson6902 struct { @@ -30,9 +26,6 @@ type PatchJson6902 struct { // before addition of a namePrefix). Target *Target `json:"target" yaml:"target"` - // jsonPatch is a list of operations in YAML format that follows JSON 6902 rule. - JsonPatch yamlpatch.Patch `json:"jsonPatch,omitempty" yaml:"jsonPatch,omitempty"` - // relative file path for a json patch file inside a kustomization Path string `json:"path,omitempty" yaml:"path,omitempty"` } diff --git a/pkg/patch/transformer/factory.go b/pkg/patch/transformer/factory.go index c59f5e8e8..ebd973098 100644 --- a/pkg/patch/transformer/factory.go +++ b/pkg/patch/transformer/factory.go @@ -20,6 +20,7 @@ import ( "fmt" "github.com/evanphx/json-patch" + "github.com/krishicks/yaml-patch" "github.com/kubernetes-sigs/kustomize/pkg/loader" "github.com/kubernetes-sigs/kustomize/pkg/patch" "github.com/kubernetes-sigs/kustomize/pkg/resource" @@ -56,8 +57,8 @@ func (f PatchJson6902Factory) makeOnePatchJson6902Transformer(p patch.PatchJson6 if p.Target == nil { return nil, fmt.Errorf("must specify the target field in patchesJson6902") } - if p.Path != "" && p.JsonPatch != nil { - return nil, fmt.Errorf("cannot specify path and jsonPath at the same time") + if p.Path == "" { + return nil, fmt.Errorf("must specify the path for a json patch file") } targetId := resource.NewResIdWithPrefixNamespace( @@ -71,20 +72,24 @@ func (f PatchJson6902Factory) makeOnePatchJson6902Transformer(p patch.PatchJson6 p.Target.Namespace, ) - if p.JsonPatch != nil { - return newPatchJson6902YAMLTransformer(targetId, p.JsonPatch) + rawOp, err := f.loader.Load(p.Path) + if err != nil { + return nil, err } - if p.Path != "" { - rawOp, err := f.loader.Load(p.Path) - if err != nil { - return nil, err - } + if isJsonFormat(rawOp) { decodedPatch, err := jsonpatch.DecodePatch(rawOp) if err != nil { return nil, err } return newPatchJson6902JSONTransformer(targetId, decodedPatch) - } - return nil, nil + decodedPatch, err := yamlpatch.DecodePatch(rawOp) + if err != nil { + return nil, err + } + return newPatchJson6902YAMLTransformer(targetId, decodedPatch) +} + +func isJsonFormat(data []byte) bool { + return data[0] == '[' } diff --git a/pkg/patch/transformer/factory_test.go b/pkg/patch/transformer/factory_test.go index 7e94a8a58..426ab18d4 100644 --- a/pkg/patch/transformer/factory_test.go +++ b/pkg/patch/transformer/factory_test.go @@ -29,22 +29,6 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" ) -func TestNewPatchJson6902FactoryNull(t *testing.T) { - p := patch.PatchJson6902{ - Target: &patch.Target{ - Name: "some-name", - }, - } - f := NewPatchJson6902Factory(nil) - tr, err := f.makeOnePatchJson6902Transformer(p) - if err != nil { - t.Fatalf("unexpected error : %v", err) - } - if tr != nil { - t.Fatal("a nil should be returned") - } -} - func TestNewPatchJson6902FactoryNoTarget(t *testing.T) { p := patch.PatchJson6902{} _, err := NewPatchJson6902Factory(nil).makeOnePatchJson6902Transformer(p) @@ -61,14 +45,6 @@ func TestNewPatchJson6902FactoryConflict(t *testing.T) { target: name: some-name kind: Deployment -jsonPatch: - - op: replace - path: /spec/template/spec/containers/0/name - value: my-nginx - - op: add - path: /spec/template/spec/containers/0/command - value: [arg1,arg2,arg3] -path: /some/dir/some/file `) p := patch.PatchJson6902{} err := yaml.Unmarshal(jsonPatch, &p) @@ -80,7 +56,7 @@ path: /some/dir/some/file if err == nil { t.Fatal("expected error") } - if !strings.Contains(err.Error(), "cannot specify path and jsonPath at the same time") { + if !strings.Contains(err.Error(), "must specify the path for a json patch file") { t.Fatalf("incorrect error returned %v", err) } } @@ -109,8 +85,7 @@ path: /testpath/patch.json t.Fatal("expected error") } - f := NewPatchJson6902Factory(ldr) - tr, err := f.makeOnePatchJson6902Transformer(p) + tr, err := NewPatchJson6902Factory(ldr).makeOnePatchJson6902Transformer(p) if err != nil { t.Fatalf("unexpected error : %v", err) } @@ -120,11 +95,8 @@ path: /testpath/patch.json } func TestNewPatchJson6902FactoryYAML(t *testing.T) { - jsonPatch := []byte(` -target: - name: some-name - kind: Deployment -jsonPatch: + ldr := loadertest.NewFakeLoader("/testpath") + operations := []byte(` - op: replace path: /spec/template/spec/containers/0/name value: my-nginx @@ -134,15 +106,24 @@ jsonPatch: - op: add path: /spec/template/spec/containers/0/command value: ["arg1", "arg2", "arg3"] +`) + err := ldr.AddFile("/testpath/patch.yaml", operations) + if err != nil { + t.Fatalf("Failed to setup fake ldr.") + } + jsonPatch := []byte(` +target: + name: some-name + kind: Deployment +path: /testpath/patch.yaml `) p := patch.PatchJson6902{} - err := yaml.Unmarshal(jsonPatch, &p) + err = yaml.Unmarshal(jsonPatch, &p) if err != nil { t.Fatalf("unexpected error : %v", err) } - f := NewPatchJson6902Factory(nil) - tr, err := f.makeOnePatchJson6902Transformer(p) + tr, err := NewPatchJson6902Factory(ldr).makeOnePatchJson6902Transformer(p) if err != nil { t.Fatalf("unexpected error : %v", err) } @@ -162,6 +143,16 @@ func TestNewPatchJson6902FactoryMulti(t *testing.T) { t.Fatalf("Failed to setup fake ldr.") } + operations = []byte(` +- op: add + path: /spec/template/spec/containers/0/command + value: ["arg1", "arg2", "arg3"] +`) + err = ldr.AddFile("/testpath/patch.yaml", operations) + if err != nil { + t.Fatalf("Failed to setup fake ldr.") + } + jsonPatches := []byte(` - target: kind: foo @@ -171,10 +162,7 @@ func TestNewPatchJson6902FactoryMulti(t *testing.T) { - target: kind: foo name: some-name - jsonPatch: - - op: add - path: /spec/template/spec/containers/0/command - value: ["arg1", "arg2", "arg3"] + path: /testpath/patch.yaml `) var p []patch.PatchJson6902 err = yaml.Unmarshal(jsonPatches, &p) @@ -269,6 +257,15 @@ func TestNewPatchJson6902FactoryMultiConflict(t *testing.T) { if err != nil { t.Fatalf("Failed to setup fake ldr.") } + operations = []byte(` +- op: add + path: /spec/replica + value: 4 +`) + err = ldr.AddFile("/testpath/patch.yaml", operations) + if err != nil { + t.Fatalf("Failed to setup fake ldr.") + } jsonPatches := []byte(` - target: @@ -279,10 +276,7 @@ func TestNewPatchJson6902FactoryMultiConflict(t *testing.T) { - target: kind: foo name: some-name - jsonPatch: - - op: add - path: /spec/replica - value: 4 + path: /testpath/patch.yaml `) var p []patch.PatchJson6902 err = yaml.Unmarshal(jsonPatches, &p)