address comments

pkg/plugins/execplugin.go

@@ -134,17 +134,7 @@ func (p *ExecPlugin) writeConfig() (string, error) {
 }
 
 func (p *ExecPlugin) Generate() (resmap.ResMap, error) {
-	args, err := p.getArgs()
-	if err != nil {
-		return nil, err
-	}
-	cmd := exec.Command(p.name, args...)
-	cmd.Env = p.getEnv()
-	cmd.Stderr = os.Stderr
-	if _, err := os.Stat(p.ldr.Root()); err == nil {
-		cmd.Dir = p.ldr.Root()
-	}
-	output, err := cmd.Output()
+	output, err := p.invokePlugin(nil)
 	if err != nil {
 		return nil, err
 	}
@@ -152,30 +142,42 @@ func (p *ExecPlugin) Generate() (resmap.ResMap, error) {
 }
 
 func (p *ExecPlugin) Transform(rm resmap.ResMap) error {
-	args, err := p.getArgs()
+	// add ResIds as annotations to all objects so that we can add them back
+	inputRM, err := p.getResMapWithIdAnnotation(rm)
 	if err != nil {
 		return err
 	}
 
-	inputRM := p.getResMapWithIdAnnotation(rm)
+	// encode the ResMap so it can be fed to the plugin
 	resources, err := inputRM.EncodeAsYaml()
 	if err != nil {
 		return err
 	}
 
-	cmd := exec.Command(p.name, args...)
-	cmd.Env = p.getEnv()
-	cmd.Stdin = bytes.NewReader(resources)
-	cmd.Stderr = os.Stderr
-	if _, err := os.Stat(p.ldr.Root()); err == nil {
-		cmd.Dir = p.ldr.Root()
-	}
-	output, err := cmd.Output()
+	// invoke the plugin with resources as the input
+	output, err := p.invokePlugin(resources)
 	if err != nil {
 		return err
 	}
 
-	return p.parseResMapFromOutput(output, rm)
+	// update the original ResMap based on the output
+	return p.updateResMapValues(output, rm)
+}
+
+// invokePlugin invokes the plugin
+func (p *ExecPlugin) invokePlugin(input []byte) ([]byte, error) {
+	args, err := p.getArgs()
+	if err != nil {
+		return nil, err
+	}
+	cmd := exec.Command(p.name, args...)
+	cmd.Env = p.getEnv()
+	cmd.Stdin = bytes.NewReader(input)
+	cmd.Stderr = os.Stderr
+	if _, err := os.Stat(p.ldr.Root()); err == nil {
+		cmd.Dir = p.ldr.Root()
+	}
+	return cmd.Output()
 }
 
 // The first arg is always the absolute path to a temporary file
@@ -196,32 +198,46 @@ func (p *ExecPlugin) getEnv() []string {
 	return env
 }
 
-func (p *ExecPlugin) getResMapWithIdAnnotation(rm resmap.ResMap) resmap.ResMap {
+// Returns a new copy of the given ResMap with the ResIds annotated in each Resource
+func (p *ExecPlugin) getResMapWithIdAnnotation(rm resmap.ResMap) (resmap.ResMap, error) {
 	inputRM := rm.DeepCopy(p.rf.RF())
 	for id, r := range inputRM {
+		idString, err := yaml.Marshal(id)
+		if err != nil {
+			return nil, err
+		}
 		annotations := r.GetAnnotations()
 		if annotations == nil {
-			annotations = make(map[string]string)
+			annotations = map[string]string{
+				idAnnotation: string(idString),
+			}
+			r.SetAnnotations(annotations)
 		}
-		annotations[idAnnotation] = id.String()
-		r.SetAnnotations(annotations)
+		annotations[idAnnotation] = string(idString)
 	}
-	return inputRM
+	return inputRM, nil
 }
 
-func (p *ExecPlugin) parseResMapFromOutput(output []byte, rm resmap.ResMap) error {
+/*
+updateResMapValues updates the Resource value in the given ResMap
+with the emitted Resource values in output.
+*/
+func (p *ExecPlugin) updateResMapValues(output []byte, rm resmap.ResMap) error {
 	outputRM, err := p.rf.NewResMapFromBytes(output)
 	if err != nil {
 		return err
 	}
 	for _, r := range outputRM {
+		// for each emitted Resource, find the matching Resource in the original ResMap
+		// using its id
 		annotations := r.GetAnnotations()
 		idString, ok := annotations[idAnnotation]
 		if !ok {
 			return fmt.Errorf("the transformer %s should not remove annotation %s",
 				p.name, idAnnotation)
 		}
-		id, err := resid.NewResIdFromString(idString)
+		id := resid.ResId{}
+		err := yaml.Unmarshal([]byte(idString), &id)
 		if err != nil {
 			return err
 		}
@@ -229,8 +245,14 @@ func (p *ExecPlugin) parseResMapFromOutput(output []byte, rm resmap.ResMap) erro
 		if !ok {
 			return fmt.Errorf("unable to find id %s in resource map", id.String())
 		}
+		// remove the annotation set by Kustomize to track the resource
 		delete(annotations, idAnnotation)
+		if len(annotations) == 0 {
+			annotations = nil
+		}
 		r.SetAnnotations(annotations)
+
+		// update the ResMap resource value with the transformed object
 		res.Kunstructured = r.Kunstructured
 	}
 	return nil

pkg/resid/resid.go

@@ -17,7 +17,6 @@ limitations under the License.
 package resid
 
 import (
-	"fmt"
 	"strings"
 
 	"sigs.k8s.io/kustomize/pkg/gvk"
@@ -30,33 +29,33 @@ type ResId struct {
 	// An untransformed resource has no prefix.
 	// A fully transformed resource has an arbitrary
 	// number of prefixes concatenated together.
-	prefix string
+	Prefix string `json:"prefix,omitempty"`
 
 	// nameSuffix of the resource.
 	// An untransformed resource has no suffix.
 	// A fully transformed resource has an arbitrary
 	// number of suffixes concatenated together.
-	suffix string
+	Suffix string `json:"suffix,omitempty"`
 }
 
 // NewResIdWithPrefixSuffixNamespace creates new resource identifier with a prefix, suffix and a namespace
 func NewResIdWithPrefixSuffixNamespace(k gvk.Gvk, n, p, s, ns string) ResId {
-	return ResId{ItemId: ItemId{Gvk: k, Name: n, Namespace: ns}, prefix: p, suffix: s}
+	return ResId{ItemId: ItemId{Gvk: k, Name: n, Namespace: ns}, Prefix: p, Suffix: s}
 }
 
 // NewResIdWithPrefixNamespace creates new resource identifier with a prefix and a namespace
 func NewResIdWithPrefixNamespace(k gvk.Gvk, n, p, ns string) ResId {
-	return ResId{ItemId: ItemId{Gvk: k, Name: n, Namespace: ns}, prefix: p}
+	return ResId{ItemId: ItemId{Gvk: k, Name: n, Namespace: ns}, Prefix: p}
 }
 
 // NewResIdWithSuffixNamespace creates new resource identifier with a suffix and a namespace
 func NewResIdWithSuffixNamespace(k gvk.Gvk, n, s, ns string) ResId {
-	return ResId{ItemId: ItemId{Gvk: k, Name: n, Namespace: ns}, suffix: s}
+	return ResId{ItemId: ItemId{Gvk: k, Name: n, Namespace: ns}, Suffix: s}
 }
 
 // NewResIdWithPrefixSuffix creates new resource identifier with a prefix and suffix
 func NewResIdWithPrefixSuffix(k gvk.Gvk, n, p, s string) ResId {
-	return ResId{ItemId: ItemId{Gvk: k, Name: n}, prefix: p, suffix: s}
+	return ResId{ItemId: ItemId{Gvk: k, Name: n}, Prefix: p, Suffix: s}
 }
 
 // NewResId creates new resource identifier
@@ -83,7 +82,7 @@ func (n ResId) String() string {
 	if ns == "" {
 		ns = noNamespace
 	}
-	p := n.prefix
+	p := n.Prefix
 	if p == "" {
 		p = noPrefix
 	}
@@ -91,7 +90,7 @@ func (n ResId) String() string {
 	if nm == "" {
 		nm = noName
 	}
-	s := n.suffix
+	s := n.Suffix
 	if s == "" {
 		s = noSuffix
 	}
@@ -100,38 +99,6 @@ func (n ResId) String() string {
 		[]string{n.ItemId.Gvk.String(), ns, p, nm, s}, separator)
 }
 
-// NewResIdFromString makes a ResId from a string which represents a Resid
-func NewResIdFromString(s string) (ResId, error) {
-	values := strings.Split(s, separator)
-	if len(values) != 5 {
-		return ResId{}, fmt.Errorf("The input string %s doesn't represent a ResId", s)
-	}
-
-	g := gvk.FromString(values[0])
-	ns := values[1]
-	p := values[2]
-	nm := values[3]
-	su := values[4]
-	if ns == noNamespace {
-		ns = ""
-	}
-	if p == noPrefix {
-		p = ""
-	}
-	if nm == noName {
-		nm = ""
-	}
-	if su == noSuffix {
-		su = ""
-	}
-	return ResId{
-		ItemId: NewItemId(g, ns, nm),
-		prefix: p,
-		suffix: su,
-	}, nil
-
-}
-
 // GvknString of ResId based on GVK and name
 func (n ResId) GvknString() string {
 	return n.ItemId.Gvk.String() + separator + n.ItemId.Name
@@ -169,10 +136,10 @@ func (n ResId) Namespace() string {
 func (n ResId) CopyWithNewPrefixSuffix(p, s string) ResId {
 	result := n
 	if p != "" {
-		result.prefix = n.concatPrefix(p)
+		result.Prefix = n.concatPrefix(p)
 	}
 	if s != "" {
-		result.suffix = n.concatSuffix(s)
+		result.Suffix = n.concatSuffix(s)
 	}
 	return result
 }
@@ -202,28 +169,28 @@ func (n ResId) HasSameRightmostSuffix(id ResId) bool {
 
 func (n ResId) concatPrefix(p string) string {
 	if p == "" {
-		return n.prefix
+		return n.Prefix
 	}
-	if n.prefix == "" {
+	if n.Prefix == "" {
 		return p
 	}
-	return p + ":" + n.prefix
+	return p + ":" + n.Prefix
 }
 
 func (n ResId) concatSuffix(s string) string {
 	if s == "" {
-		return n.suffix
+		return n.Suffix
 	}
-	if n.suffix == "" {
+	if n.Suffix == "" {
 		return s
 	}
-	return n.suffix + ":" + s
+	return n.Suffix + ":" + s
 }
 
 func (n ResId) prefixList() []string {
-	return strings.Split(n.prefix, ":")
+	return strings.Split(n.Prefix, ":")
 }
 
 func (n ResId) suffixList() []string {
-	return strings.Split(n.suffix, ":")
+	return strings.Split(n.Suffix, ":")
 }

pkg/resid/resid_test.go

@@ -17,8 +17,8 @@ var stringTests = []struct {
 				Gvk:       gvk.Gvk{Group: "g", Version: "v", Kind: "k"},
 				Name:      "nm",
 			},
-			prefix: "p",
-			suffix: "s",
+			Prefix: "p",
+			Suffix: "s",
 		},
 		"g_v_k|ns|p|nm|s",
 	},
@@ -29,8 +29,8 @@ var stringTests = []struct {
 				Gvk:       gvk.Gvk{Version: "v", Kind: "k"},
 				Name:      "nm",
 			},
-			prefix: "p",
-			suffix: "s",
+			Prefix: "p",
+			Suffix: "s",
 		},
 		"~G_v_k|ns|p|nm|s",
 	},
@@ -41,8 +41,8 @@ var stringTests = []struct {
 				Gvk:       gvk.Gvk{Kind: "k"},
 				Name:      "nm",
 			},
-			prefix: "p",
-			suffix: "s",
+			Prefix: "p",
+			Suffix: "s",
 		},
 		"~G_~V_k|ns|p|nm|s",
 	},
@@ -53,8 +53,8 @@ var stringTests = []struct {
 				Gvk:       gvk.Gvk{},
 				Name:      "nm",
 			},
-			prefix: "p",
-			suffix: "s",
+			Prefix: "p",
+			Suffix: "s",
 		},
 		"~G_~V_~K|ns|p|nm|s",
 	},
@@ -64,8 +64,8 @@ var stringTests = []struct {
 				Gvk:  gvk.Gvk{},
 				Name: "nm",
 			},
-			prefix: "p",
-			suffix: "s",
+			Prefix: "p",
+			Suffix: "s",
 		},
 		"~G_~V_~K|~X|p|nm|s",
 	},
@@ -75,7 +75,7 @@ var stringTests = []struct {
 				Gvk:  gvk.Gvk{},
 				Name: "nm",
 			},
-			suffix: "s",
+			Suffix: "s",
 		},
 		"~G_~V_~K|~X|~P|nm|s",
 	},
@@ -84,7 +84,7 @@ var stringTests = []struct {
 			ItemId: ItemId{
 				Gvk: gvk.Gvk{},
 			},
-			suffix: "s",
+			Suffix: "s",
 		},
 		"~G_~V_~K|~X|~P|~N|s",
 	},
@@ -121,8 +121,8 @@ var gvknStringTests = []struct {
 				Gvk:       gvk.Gvk{Group: "g", Version: "v", Kind: "k"},
 				Name:      "nm",
 			},
-			prefix: "p",
-			suffix: "s",
+			Prefix: "p",
+			Suffix: "s",
 		},
 		"g_v_k|nm",
 	},
@@ -133,8 +133,8 @@ var gvknStringTests = []struct {
 				Gvk:       gvk.Gvk{Version: "v", Kind: "k"},
 				Name:      "nm",
 			},
-			prefix: "p",
-			suffix: "s",
+			Prefix: "p",
+			Suffix: "s",
 		},
 		"~G_v_k|nm",
 	},
@@ -145,8 +145,8 @@ var gvknStringTests = []struct {
 				Gvk:       gvk.Gvk{Kind: "k"},
 				Name:      "nm",
 			},
-			prefix: "p",
-			suffix: "s",
+			Prefix: "p",
+			Suffix: "s",
 		},
 		"~G_~V_k|nm",
 	},
@@ -157,8 +157,8 @@ var gvknStringTests = []struct {
 				Gvk:       gvk.Gvk{},
 				Name:      "nm",
 			},
-			prefix: "p",
-			suffix: "s",
+			Prefix: "p",
+			Suffix: "s",
 		},
 		"~G_~V_~K|nm",
 	},
@@ -168,8 +168,8 @@ var gvknStringTests = []struct {
 				Gvk:  gvk.Gvk{},
 				Name: "nm",
 			},
-			prefix: "p",
-			suffix: "s",
+			Prefix: "p",
+			Suffix: "s",
 		},
 		"~G_~V_~K|nm",
 	},
@@ -179,7 +179,7 @@ var gvknStringTests = []struct {
 				Gvk:  gvk.Gvk{},
 				Name: "nm",
 			},
-			suffix: "s",
+			Suffix: "s",
 		},
 		"~G_~V_~K|nm",
 	},
@@ -188,7 +188,7 @@ var gvknStringTests = []struct {
 			ItemId: ItemId{
 				Gvk: gvk.Gvk{},
 			},
-			suffix: "s",
+			Suffix: "s",
 		},
 		"~G_~V_~K|",
 	},
@@ -227,8 +227,8 @@ var GvknEqualsTest = []struct {
 				Gvk:       gvk.Gvk{Group: "g", Version: "v", Kind: "k"},
 				Name:      "nm",
 			},
-			prefix: "AA",
-			suffix: "aa",
+			Prefix: "AA",
+			Suffix: "aa",
 		},
 		id2: ResId{
 			ItemId: ItemId{
@@ -236,8 +236,8 @@ var GvknEqualsTest = []struct {
 				Gvk:       gvk.Gvk{Group: "g", Version: "v", Kind: "k"},
 				Name:      "nm",
 			},
-			prefix: "BB",
-			suffix: "bb",
+			Prefix: "BB",
+			Suffix: "bb",
 		},
 		gVknResult:   true,
 		nSgVknResult: true,
@@ -249,8 +249,8 @@ var GvknEqualsTest = []struct {
 				Gvk:       gvk.Gvk{Group: "g", Version: "v", Kind: "k"},
 				Name:      "nm",
 			},
-			prefix: "AA",
-			suffix: "aa",
+			Prefix: "AA",
+			Suffix: "aa",
 		},
 		id2: ResId{
 			ItemId: ItemId{
@@ -258,8 +258,8 @@ var GvknEqualsTest = []struct {
 				Gvk:       gvk.Gvk{Group: "g", Version: "v", Kind: "k"},
 				Name:      "nm",
 			},
-			prefix: "BB",
-			suffix: "bb",
+			Prefix: "BB",
+			Suffix: "bb",
 		},
 		gVknResult:   true,
 		nSgVknResult: false,
@@ -271,16 +271,16 @@ var GvknEqualsTest = []struct {
 				Gvk:       gvk.Gvk{Group: "g", Version: "v", Kind: "k"},
 				Name:      "nm",
 			},
-			prefix: "AA",
-			suffix: "aa",
+			Prefix: "AA",
+			Suffix: "aa",
 		},
 		id2: ResId{
 			ItemId: ItemId{
 				Gvk:  gvk.Gvk{Group: "g", Version: "v", Kind: "k"},
 				Name: "nm",
 			},
-			prefix: "BB",
-			suffix: "bb",
+			Prefix: "BB",
+			Suffix: "bb",
 		},
 		gVknResult:   true,
 		nSgVknResult: false,
@@ -292,8 +292,8 @@ var GvknEqualsTest = []struct {
 				Gvk:       gvk.Gvk{Version: "v", Kind: "k"},
 				Name:      "nm",
 			},
-			prefix: "AA",
-			suffix: "aa",
+			Prefix: "AA",
+			Suffix: "aa",
 		},
 		id2: ResId{
 			ItemId: ItemId{
@@ -301,8 +301,8 @@ var GvknEqualsTest = []struct {
 				Gvk:       gvk.Gvk{Version: "v", Kind: "k"},
 				Name:      "nm",
 			},
-			prefix: "BB",
-			suffix: "bb",
+			Prefix: "BB",
+			Suffix: "bb",
 		},
 		gVknResult:   true,
 		nSgVknResult: false,
@@ -314,8 +314,8 @@ var GvknEqualsTest = []struct {
 				Gvk:       gvk.Gvk{Kind: "k"},
 				Name:      "nm",
 			},
-			prefix: "AA",
-			suffix: "aa",
+			Prefix: "AA",
+			Suffix: "aa",
 		},
 		id2: ResId{
 			ItemId: ItemId{
@@ -323,8 +323,8 @@ var GvknEqualsTest = []struct {
 				Gvk:       gvk.Gvk{Kind: "k"},
 				Name:      "nm",
 			},
-			prefix: "BB",
-			suffix: "bb",
+			Prefix: "BB",
+			Suffix: "bb",
 		},
 		gVknResult:   true,
 		nSgVknResult: false,
@@ -335,16 +335,16 @@ var GvknEqualsTest = []struct {
 				Namespace: "X",
 				Name:      "nm",
 			},
-			prefix: "AA",
-			suffix: "aa",
+			Prefix: "AA",
+			Suffix: "aa",
 		},
 		id2: ResId{
 			ItemId: ItemId{
 				Namespace: "Z",
 				Name:      "nm",
 			},
-			prefix: "BB",
-			suffix: "bb",
+			Prefix: "BB",
+			Suffix: "bb",
 		},
 		gVknResult:   true,
 		nSgVknResult: false,
@@ -371,8 +371,8 @@ func TestCopyWithNewPrefixSuffix(t *testing.T) {
 			Gvk:       gvk.Gvk{Group: "g", Version: "v", Kind: "k"},
 			Name:      "nm",
 		},
-		prefix: "a",
-		suffix: "b",
+		Prefix: "a",
+		Suffix: "b",
 	}
 	r2 := r1.CopyWithNewPrefixSuffix("p-", "-s")
 	expected := ResId{
@@ -381,8 +381,8 @@ func TestCopyWithNewPrefixSuffix(t *testing.T) {
 			Gvk:       gvk.Gvk{Group: "g", Version: "v", Kind: "k"},
 			Name:      "nm",
 		},
-		prefix: "p-a",
-		suffix: "b-s",
+		Prefix: "p-a",
+		Suffix: "b-s",
 	}
 	if !r2.GvknEquals(expected) {
 		t.Fatalf("%v should equal %v", r2, expected)
@@ -396,8 +396,8 @@ func TestCopyWithNewNamespace(t *testing.T) {
 			Gvk:       gvk.Gvk{Group: "g", Version: "v", Kind: "k"},
 			Name:      "nm",
 		},
-		prefix: "a",
-		suffix: "b",
+		Prefix: "a",
+		Suffix: "b",
 	}
 	r2 := r1.CopyWithNewNamespace("zzz")
 	expected := ResId{
@@ -406,8 +406,8 @@ func TestCopyWithNewNamespace(t *testing.T) {
 			Gvk:       gvk.Gvk{Group: "g", Version: "v", Kind: "k"},
 			Name:      "nm",
 		},
-		prefix: "a",
-		suffix: "b",
+		Prefix: "a",
+		Suffix: "b",
 	}
 	if !r2.GvknEquals(expected) {
 		t.Fatalf("%v should equal %v", r2, expected)