fix a patch files accept multiple patches

kubernetes-sigs · Jun 14, 2023 · a761588 · a761588
1 parent da31b96
commit a761588
Show file tree

Hide file tree

Showing 4 changed files with 79 additions and 65 deletions.
diff --git a/api/internal/builtins/PatchTransformer.go b/api/internal/builtins/PatchTransformer.go
diff --git a/api/krusty/multiplepatch_test.go b/api/krusty/multiplepatch_test.go
@@ -6,8 +6,6 @@ package krusty_test
 import (
 	"testing"
 
-	"github.com/stretchr/testify/assert"
-
 	kusttest_test "sigs.k8s.io/kustomize/api/testutils/kusttest"
 )
 
@@ -303,20 +301,15 @@ patchesStrategicMerge:
 	m = th.Run("overlay", th.MakeDefaultOptions())
 	th.AssertActualEqualsExpected(m, expected)
 
-	// Technique 4: "patches:" field, one patch file.  Fails.
+	// Technique 4: "patches:" field, one patch file.
 	th.WriteK("overlay", `
 resources:
 - ../base
 patches:
 - path: twoPatchesInOneFile.yaml
 `)
-	err := th.RunWithErr("overlay", th.MakeDefaultOptions())
-	assert.Error(t, err)
-	// This should fail, because the semantics of the `patches` field.
-	// That field allows specific patch targeting to a list of targets,
-	// while the `patchesStrategicMerge` field accepts patches that
-	// implicitly identify their targets via GVKN.
-	assert.Contains(t, err.Error(), "unable to parse SM or JSON patch from ")
+	m = th.Run("overlay", th.MakeDefaultOptions())
+	th.AssertActualEqualsExpected(m, expected)
 }
 
 func TestRemoveEmptyDirWithNullFieldInSmp(t *testing.T) {
@@ -1670,7 +1663,7 @@ spec:
   template:
     spec:
       containers:
-        - image: fluentd:latest 
+        - image: fluentd:latest
           name: fluentd
       serviceAccountName: fluentd-sa
 ---

diff --git a/api/resmap/reswrangler.go b/api/resmap/reswrangler.go
@@ -696,8 +696,7 @@ func (m *resWrangler) DeAnchor() (err error) {
 }
 
 // ApplySmPatch applies the patch, and errors on Id collisions.
-func (m *resWrangler) ApplySmPatch(
-	selectedSet *resource.IdSet, patch *resource.Resource) error {
+func (m *resWrangler) ApplySmPatch(selectedSet *resource.IdSet, patch *resource.Resource) error {
 	var list []*resource.Resource
 	for _, res := range m.rList {
 		if selectedSet.Contains(res.CurId()) {

diff --git a/plugin/builtin/patchtransformer/PatchTransformer.go b/plugin/builtin/patchtransformer/PatchTransformer.go
@@ -18,12 +18,12 @@ import (
 )
 
 type plugin struct {
-	loadedPatch  *resource.Resource
-	decodedPatch jsonpatch.Patch
-	Path         string          `json:"path,omitempty" yaml:"path,omitempty"`
-	Patch        string          `json:"patch,omitempty" yaml:"patch,omitempty"`
-	Target       *types.Selector `json:"target,omitempty" yaml:"target,omitempty"`
-	Options      map[string]bool `json:"options,omitempty" yaml:"options,omitempty"`
+	loadedPatches []*resource.Resource
+	decodedPatch  jsonpatch.Patch
+	Path          string          `json:"path,omitempty" yaml:"path,omitempty"`
+	Patch         string          `json:"patch,omitempty" yaml:"patch,omitempty"`
+	Target        *types.Selector `json:"target,omitempty" yaml:"target,omitempty"`
+	Options       map[string]bool `json:"options,omitempty" yaml:"options,omitempty"`
 }
 
 var KustomizePlugin plugin //nolint:gochecknoglobals
@@ -51,10 +51,10 @@ func (p *plugin) Config(
 		p.Patch = string(loaded)
 	}
 
-	patchSM, errSM := h.ResmapFactory().RF().FromBytes([]byte(p.Patch))
+	patchesSM, errSM := h.ResmapFactory().RF().SliceFromBytes([]byte(p.Patch))
 	patchJson, errJson := jsonPatchFromBytes([]byte(p.Patch))
 	if (errSM == nil && errJson == nil) ||
-		(patchSM != nil && patchJson != nil) {
+		(patchesSM != nil && patchJson != nil) {
 		return fmt.Errorf(
 			"illegally qualifies as both an SM and JSON patch: [%v]",
 			p.Patch)
@@ -64,12 +64,14 @@ func (p *plugin) Config(
 			"unable to parse SM or JSON patch from [%v]", p.Patch)
 	}
 	if errSM == nil {
-		p.loadedPatch = patchSM
-		if p.Options["allowNameChange"] {
-			p.loadedPatch.AllowNameChange()
-		}
-		if p.Options["allowKindChange"] {
-			p.loadedPatch.AllowKindChange()
+		p.loadedPatches = patchesSM
+		for _, loadedPatch := range p.loadedPatches {
+			if p.Options["allowNameChange"] {
+				loadedPatch.AllowNameChange()
+			}
+			if p.Options["allowKindChange"] {
+				loadedPatch.AllowKindChange()
+			}
 		}
 	} else {
 		p.decodedPatch = patchJson
@@ -78,29 +80,38 @@ func (p *plugin) Config(
 }
 
 func (p *plugin) Transform(m resmap.ResMap) error {
-	if p.loadedPatch == nil {
+	if p.loadedPatches == nil {
 		return p.transformJson6902(m, p.decodedPatch)
 	}
 	// The patch was a strategic merge patch
-	return p.transformStrategicMerge(m, p.loadedPatch)
+	return p.transformStrategicMerge(m)
 }
 
 // transformStrategicMerge applies the provided strategic merge patch
 // to all the resources in the ResMap that match either the Target or
 // the identifier of the patch.
-func (p *plugin) transformStrategicMerge(m resmap.ResMap, patch *resource.Resource) error {
-	if p.Target == nil {
-		target, err := m.GetById(patch.OrgId())
+func (p *plugin) transformStrategicMerge(m resmap.ResMap) error {
+	for _, patch := range p.loadedPatches {
+		if p.Target == nil {
+			target, err := m.GetById(patch.OrgId())
+			if err != nil {
+				return fmt.Errorf("unable to find patch target %s: %w", patch.OrgId().Name, err)
+			}
+			if err := target.ApplySmPatch(patch); err != nil {
+				return fmt.Errorf("%w", err)
+			}
+			continue
+		}
+
+		selected, err := m.Select(*p.Target)
 		if err != nil {
-			return err
+			return fmt.Errorf("unable to find patch target in ResMap %s: %w", p.Target, err)
+		}
+		if err := m.ApplySmPatch(resource.MakeIdSet(selected), patch); err != nil {
+			return fmt.Errorf("%w", err)
 		}
-		return target.ApplySmPatch(patch)
-	}
-	selected, err := m.Select(*p.Target)
-	if err != nil {
-		return err
 	}
-	return m.ApplySmPatch(resource.MakeIdSet(selected), patch)
+	return nil
 }
 
 // transformJson6902 applies the provided json6902 patch