Diamond Import

Allow diamond structure of kustomize import

- This situation happens in project which try to aggregate multiple
  components in different folders sharing the same base folder.
- This commit demonstrates how a common resource can be included
  accross different components.
- Address conflicting bases by converting resources into patches
- Improve variables diamond conflicts detection

Add and update tests around diamond import of folders.

1. Test simple import of common resources.
2. Test simple import of common variables.
3. Test simple import of variables without their target
   resources in the parent folders.
4. Test that the algorithm is reporting proper errors
   in case of not supported diamond import of resources.
5. Test that the algorithm is reporting proper errors
   in case of not supported diamond import of variables.
6. Update errors messages in existing tests.

Prepare diamond import feature for upcoming high level merge feature.

Add unit tests for resaccumulator

Improve variable conflict detection. Fix bug 506

pkg/accumulator/resaccumulator.go

@@ -6,10 +6,13 @@ package accumulator
 import (
 	"fmt"
 	"log"
+	"reflect"
 	"strings"
 
+	"sigs.k8s.io/kustomize/v3/pkg/gvk"
 	"sigs.k8s.io/kustomize/v3/pkg/resid"
 	"sigs.k8s.io/kustomize/v3/pkg/resmap"
+	"sigs.k8s.io/kustomize/v3/pkg/resource"
 	"sigs.k8s.io/kustomize/v3/pkg/transformers"
 	"sigs.k8s.io/kustomize/v3/pkg/transformers/config"
 	"sigs.k8s.io/kustomize/v3/pkg/types"
@@ -19,16 +22,20 @@ import (
 // used to customize those resources.  It's a ResMap
 // plus stuff needed to modify the ResMap.
 type ResAccumulator struct {
-	resMap  resmap.ResMap
-	tConfig *config.TransformerConfig
-	varSet  types.VarSet
+	resMap         resmap.ResMap
+	tConfig        *config.TransformerConfig
+	varSet         types.VarSet
+	patchSet       []types.Patch
+	unresolvedVars types.VarSet
 }
 
 func MakeEmptyAccumulator() *ResAccumulator {
 	ra := &ResAccumulator{}
 	ra.resMap = resmap.New()
 	ra.tConfig = &config.TransformerConfig{}
 	ra.varSet = types.NewVarSet()
+	ra.patchSet = []types.Patch{}
+	ra.unresolvedVars = types.NewVarSet()
 	return ra
 }
 
@@ -39,7 +46,85 @@ func (ra *ResAccumulator) ResMap() resmap.ResMap {
 
 // Vars returns a copy of underlying vars.
 func (ra *ResAccumulator) Vars() []types.Var {
-	return ra.varSet.AsSlice()
+	completeset := types.NewVarSet()
+	completeset.AbsorbSet(ra.varSet)
+	completeset.AbsorbSet(ra.unresolvedVars)
+	return completeset.AsSlice()
+}
+
+// accumlatePatch accumulates the information regarding conflicting
+// resources as patches.
+func (ra *ResAccumulator) accumlatePatch(id resid.ResId, conflicting ...*resource.Resource) error {
+	target := types.Selector{
+		Gvk: gvk.Gvk{
+			Group:   id.Group,
+			Version: id.Version,
+			Kind:    id.Kind,
+		},
+		Namespace:          id.Namespace,
+		Name:               id.Name,
+		AnnotationSelector: "",
+		LabelSelector:      "",
+	}
+
+	for _, res := range conflicting {
+		out, err := res.AsYAML()
+		if err != nil {
+			return err
+		}
+		newPatch := types.Patch{
+			Path:   "",
+			Patch:  string(out),
+			Target: &target,
+		}
+		ra.patchSet = append(ra.patchSet, newPatch)
+	}
+	return nil
+}
+
+// HandoverConflictingResources removes conflicting resources from the local accumulator
+// and add the conflicting resources list in the other accumulator.
+// Conflicting is defined as have the same CurrentId but different Values.
+func (ra *ResAccumulator) HandoverConflictingResources(other *ResAccumulator) error {
+	for _, rightResource := range other.ResMap().Resources() {
+		rightId := rightResource.CurId()
+		leftResources := ra.resMap.GetMatchingResourcesByCurrentId(rightId.Equals)
+
+		if len(leftResources) == 0 {
+			// no conflict
+			continue
+		}
+
+		// TODO(jeb): Not sure we want to use DeepEqual here since there are some fields
+		// which are artifacts (nameprefix, namesuffix, refvar, refby) added to the resources
+		// by the algorithm here.
+		// Also we may be dropping here some of the artifacts (nameprefix, namesuffix,...)
+		// during the conversion from Resource/ResMap to Patch.
+		if len(leftResources) != 1 || !reflect.DeepEqual(leftResources[0], rightResource) {
+			// conflict detected. More than one resource or left and right are different.
+			if err := other.accumlatePatch(rightId, rightResource); err != nil {
+				return err
+			}
+			if err := other.accumlatePatch(rightId, leftResources...); err != nil {
+				return err
+			}
+		}
+
+		// Remove the resource from that resMap
+		err := ra.resMap.Remove(rightId)
+		if err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
+// PatchSet return the list of resources that have been
+// put aside. It will let the PatchTransformer decide how to handle
+// the conflict, assuming the Transformer can.
+func (ra *ResAccumulator) PatchSet() []types.Patch {
+	return ra.patchSet
 }
 
 func (ra *ResAccumulator) AppendAll(
@@ -62,8 +147,61 @@ func (ra *ResAccumulator) GetTransformerConfig() *config.TransformerConfig {
 	return ra.tConfig
 }
 
+// AppendUnresolvedVars accumulates the non conflicting and unresolved variables
+// from one accumulator into another. Returns an error if a conflict is detected.
+func (ra *ResAccumulator) AppendUnresolvedVars(otherSet types.VarSet) error {
+	return ra.unresolvedVars.AbsorbSet(otherSet)
+}
+
+// AppendResolvedVars accumulates the non conflicting and resolved variables
+// from one accumulator into another. Returns an error is a conflict is detected.
+func (ra *ResAccumulator) AppendResolvedVars(otherSet types.VarSet) error {
+	mergeableVars := []types.Var{}
+	for _, v := range otherSet.AsSlice() {
+		conflicting := ra.varSet.Get(v.Name)
+		if conflicting == nil {
+			// no conflict. The var is valid.
+			mergeableVars = append(mergeableVars, v)
+			continue
+		}
+
+		if !v.DeepEqual(*conflicting) {
+			// two vars with the same name are pointing at two
+			// different resources.
+			return fmt.Errorf(
+				"var '%s' already encountered", v.Name)
+		}
+
+		// Behavior has been modified.
+		// Conflict detection moved to VarMap object
+		// =============================================
+		// matched := ra.resMap.GetMatchingResourcesByOriginalId(
+		// 	resid.NewResId(v.ObjRef.GVK(), v.ObjRef.Name).GvknEquals)
+		// if len(matched) > 1 {
+		// 	// We detected a diamond import of kustomization
+		// 	// context where one variable pointing at one resource
+		// 	// in each context is now pointing at two resources
+		// 	// (different CurrId) because the two contexts have
+		// 	// been merged.
+		// 	return fmt.Errorf(
+		// 		"found %d resId matches for var %s "+
+		// 			"(unable to disambiguate)",
+		// 		len(matched), v)
+		// }
+	}
+	return ra.varSet.MergeSlice(mergeableVars)
+}
+
 func (ra *ResAccumulator) MergeVars(incoming []types.Var) error {
-	for _, v := range incoming {
+	// Absorb the new slice of vars into the previously
+	// unresolved vars (from the base folders)
+	toresolve := ra.unresolvedVars.Copy()
+	if err := toresolve.AbsorbSlice(incoming); err != nil {
+		return err
+	}
+	ra.unresolvedVars = types.NewVarSet()
+
+	for _, v := range toresolve.AsSlice() {
 		targetId := resid.NewResIdWithNamespace(v.ObjRef.GVK(), v.ObjRef.Name, v.ObjRef.Namespace)
 		idMatcher := targetId.GvknEquals
 		if targetId.Namespace != "" || !targetId.IsNamespaceableKind() {
@@ -72,17 +210,35 @@ func (ra *ResAccumulator) MergeVars(incoming []types.Var) error {
 			idMatcher = targetId.Equals
 		}
 		matched := ra.resMap.GetMatchingResourcesByOriginalId(idMatcher)
-		if len(matched) > 1 {
-			return fmt.Errorf(
-				"found %d resId matches for var %s "+
-					"(unable to disambiguate)",
-				len(matched), v)
+
+		// Behavior has been modified.
+		// Conflict detection moved to VarMap object
+		// =============================================
+		// if len(matched) > 1 {
+		// 	return fmt.Errorf(
+		// 		"found %d resId matches for var %s "+
+		// 			"(unable to disambiguate)",
+		// 		len(matched), v)
+		// }
+
+		if len(matched) == 0 {
+			// no associated resources yet.
+			if err := ra.unresolvedVars.Absorb(v); err != nil {
+				return err
+			}
+			continue
 		}
-		if len(matched) == 1 {
-			matched[0].AppendRefVarName(v)
+
+		// Found one or more associated resource.
+		if err := ra.varSet.Absorb(v); err != nil {
+			return err
+		}
+		for _, match := range matched {
+			match.AppendRefVarName(v)
 		}
 	}
-	return ra.varSet.MergeSlice(incoming)
+
+	return nil
 }
 
 func (ra *ResAccumulator) MergeAccumulator(other *ResAccumulator) (err error) {
@@ -94,42 +250,50 @@ func (ra *ResAccumulator) MergeAccumulator(other *ResAccumulator) (err error) {
 	if err != nil {
 		return err
 	}
-	return ra.varSet.MergeSet(other.varSet)
+	err = ra.AppendUnresolvedVars(other.unresolvedVars)
+	if err != nil {
+		return err
+	}
+	return ra.AppendResolvedVars(other.varSet)
 }
 
-func (ra *ResAccumulator) findVarValueFromResources(v types.Var) (interface{}, error) {
+func (ra *ResAccumulator) findVarValueFromResources(v types.Var, varMap *resmap.VarMap) error {
+	foundCount := 0
 	for _, res := range ra.resMap.Resources() {
 		for _, varName := range res.GetRefVarNames() {
 			if varName == v.Name {
 				s, err := res.GetFieldValue(v.FieldRef.FieldPath)
 				if err != nil {
-					return "", fmt.Errorf(
+					return fmt.Errorf(
 						"field specified in var '%v' "+
 							"not found in corresponding resource", v)
 				}
 
-				return s, nil
+				foundCount++
+				varMap.Append(v.Name, res, s)
 			}
 		}
 	}
 
-	return "", fmt.Errorf(
-		"var '%v' cannot be mapped to a field "+
-			"in the set of known resources", v)
+	if foundCount == 0 {
+		return fmt.Errorf(
+			"var '%v' cannot be mapped to a field "+
+				"in the set of known resources", v)
+	}
+
+	return nil
 }
 
 // makeVarReplacementMap returns a map of Var names to
 // their final values. The values are strings intended
 // for substitution wherever the $(var.Name) occurs.
-func (ra *ResAccumulator) makeVarReplacementMap() (map[string]interface{}, error) {
-	result := map[string]interface{}{}
+func (ra *ResAccumulator) makeVarReplacementMap() (resmap.VarMap, error) {
+	result := resmap.NewEmptyVarMap()
 	for _, v := range ra.Vars() {
-		s, err := ra.findVarValueFromResources(v)
+		err := ra.findVarValueFromResources(v, &result)
 		if err != nil {
-			return nil, err
+			return result, err
 		}
-
-		result[v.Name] = s
 	}
 
 	return result, nil
@@ -144,7 +308,7 @@ func (ra *ResAccumulator) ResolveVars() error {
 	if err != nil {
 		return err
 	}
-	if len(replacementMap) == 0 {
+	if len(replacementMap.VarNames()) == 0 {
 		return nil
 	}
 	t := transformers.NewRefVarTransformer(