Merge pull request #323 from Liujingfang1/master

remove inline json patch format

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"`
 }

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] == '['
 }

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)