diff --git a/pkg/patch/transformer/factory.go b/pkg/patch/transformer/factory.go index b7eb25839..b373dfb72 100644 --- a/pkg/patch/transformer/factory.go +++ b/pkg/patch/transformer/factory.go @@ -21,8 +21,6 @@ import ( "sigs.k8s.io/kustomize/pkg/ifc" "sigs.k8s.io/kustomize/pkg/resid" - "github.com/evanphx/json-patch" - "github.com/ghodss/yaml" "sigs.k8s.io/kustomize/pkg/gvk" "sigs.k8s.io/kustomize/pkg/patch" "sigs.k8s.io/kustomize/pkg/transformers" @@ -77,19 +75,7 @@ func (f PatchJson6902Factory) makeOnePatchJson6902Transformer(p patch.Json6902) return nil, err } - if !isJsonFormat(rawOp) { - // if it isn't JSON, try to parse it as YAML - rawOp, err = yaml.YAMLToJSON(rawOp) - if err != nil { - return nil, err - } - } - - decodedPatch, err := jsonpatch.DecodePatch(rawOp) - if err != nil { - return nil, err - } - return newPatchJson6902JSONTransformer(targetId, decodedPatch) + return newPatchJson6902JSONTransformer(targetId, rawOp) } func isJsonFormat(data []byte) bool { diff --git a/pkg/patch/transformer/patchjson6902json.go b/pkg/patch/transformer/patchjson6902json.go index c8f6e9e3e..1f09939d1 100644 --- a/pkg/patch/transformer/patchjson6902json.go +++ b/pkg/patch/transformer/patchjson6902json.go @@ -20,6 +20,8 @@ import ( "fmt" "github.com/evanphx/json-patch" + "github.com/ghodss/yaml" + "github.com/pkg/errors" "sigs.k8s.io/kustomize/pkg/resid" "sigs.k8s.io/kustomize/pkg/resmap" "sigs.k8s.io/kustomize/pkg/resource" @@ -30,17 +32,31 @@ import ( type patchJson6902JSONTransformer struct { target resid.ResId patch jsonpatch.Patch + rawOp []byte } var _ transformers.Transformer = &patchJson6902JSONTransformer{} // newPatchJson6902JSONTransformer constructs a PatchJson6902 transformer. func newPatchJson6902JSONTransformer( - id resid.ResId, p jsonpatch.Patch) (transformers.Transformer, error) { - if len(p) == 0 { + id resid.ResId, rawOp []byte) (transformers.Transformer, error) { + op := rawOp + var err error + if !isJsonFormat(op) { + // if it isn't JSON, try to parse it as YAML + op, err = yaml.YAMLToJSON(rawOp) + if err != nil { + return nil, err + } + } + decodedPatch, err := jsonpatch.DecodePatch(op) + if err != nil { + return nil, err + } + if len(decodedPatch) == 0 { return transformers.NewNoOpTransformer(), nil } - return &patchJson6902JSONTransformer{target: id, patch: p}, nil + return &patchJson6902JSONTransformer{target: id, patch: decodedPatch, rawOp: rawOp}, nil } // Transform apply the json patches on top of the base resources. @@ -55,7 +71,7 @@ func (t *patchJson6902JSONTransformer) Transform(m resmap.ResMap) error { } modifiedObj, err := t.patch.Apply(rawObj) if err != nil { - return err + return errors.Wrapf(err, "failed to apply json patch '%s'", string(t.rawOp)) } err = obj.UnmarshalJSON(modifiedObj) if err != nil { diff --git a/pkg/patch/transformer/patchjson6902json_test.go b/pkg/patch/transformer/patchjson6902json_test.go index 033ac1603..8e5c751cc 100644 --- a/pkg/patch/transformer/patchjson6902json_test.go +++ b/pkg/patch/transformer/patchjson6902json_test.go @@ -18,9 +18,9 @@ package transformer import ( "reflect" + "strings" "testing" - "github.com/evanphx/json-patch" "sigs.k8s.io/kustomize/k8sdeps/kunstruct" "sigs.k8s.io/kustomize/pkg/gvk" "sigs.k8s.io/kustomize/pkg/resid" @@ -67,10 +67,7 @@ func TestJsonPatchJSONTransformer_Transform(t *testing.T) { {"op": "add", "path": "/spec/replica", "value": "3"}, {"op": "add", "path": "/spec/template/spec/containers/0/command", "value": ["arg1", "arg2", "arg3"]} ]`) - patch, err := jsonpatch.DecodePatch(operations) - if err != nil { - t.Fatalf("unexpected error : %v", err) - } + expected := resmap.ResMap{ id: rf.FromMap( map[string]interface{}{ @@ -104,7 +101,7 @@ func TestJsonPatchJSONTransformer_Transform(t *testing.T) { }, }), } - jpt, err := newPatchJson6902JSONTransformer(id, patch) + jpt, err := newPatchJson6902JSONTransformer(id, operations) if err != nil { t.Fatalf("unexpected error : %v", err) } @@ -117,3 +114,52 @@ func TestJsonPatchJSONTransformer_Transform(t *testing.T) { t.Fatalf("actual doesn't match expected: %v", err) } } + +func TestJsonPatchJSONTransformer_UnHappyTransform(t *testing.T) { + rf := resource.NewFactory( + kunstruct.NewKunstructuredFactoryImpl()) + id := resid.NewResId(deploy, "deploy1") + base := resmap.ResMap{ + id: rf.FromMap( + map[string]interface{}{ + "apiVersion": "apps/v1", + "kind": "Deployment", + "metadata": map[string]interface{}{ + "name": "deploy1", + }, + "spec": map[string]interface{}{ + "template": map[string]interface{}{ + "metadata": map[string]interface{}{ + "labels": map[string]interface{}{ + "old-label": "old-value", + }, + }, + "spec": map[string]interface{}{ + "containers": []interface{}{ + map[string]interface{}{ + "name": "nginx", + "image": "nginx", + }, + }, + }, + }, + }, + }), + } + + operations := []byte(`[ + {"op": "add", "path": "/spec/template/spec/containers/0/command/", "value": ["arg1", "arg2", "arg3"]} +]`) + + jpt, err := newPatchJson6902JSONTransformer(id, operations) + if err != nil { + t.Fatalf("unexpected error : %v", err) + } + err = jpt.Transform(base) + if err == nil { + t.Fatalf("expected error didn't happen") + } + if !strings.HasPrefix(err.Error(), "failed to apply json patch") || !strings.Contains(err.Error(), string(operations)) { + t.Fatalf("expected error didn't happen, but got %v", err) + } +}