Skip to content

Commit

Permalink
add error handling when using not allowed multiple strategic-merge pa…
Browse files Browse the repository at this point in the history
…tches
  • Loading branch information
koba1t committed Jul 3, 2023
1 parent 549935d commit 0bb40bc
Show file tree
Hide file tree
Showing 3 changed files with 85 additions and 10 deletions.
16 changes: 11 additions & 5 deletions api/internal/builtins/PatchTransformer.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

16 changes: 11 additions & 5 deletions plugin/builtin/patchtransformer/PatchTransformer.go
Expand Up @@ -52,9 +52,12 @@ func (p *plugin) Config(
}

patchesSM, errSM := h.ResmapFactory().RF().SliceFromBytes([]byte(p.Patch))
patchJson, errJson := jsonPatchFromBytes([]byte(p.Patch))
patchesJson, errJson := jsonPatchFromBytes([]byte(p.Patch))

// TODO(5049): In the case of multiple yaml documents, return error instead of ignoring all but first
// detail: https://github.com/kubernetes-sigs/kustomize/issues/5049#issuecomment-1440604403
if (errSM == nil && errJson == nil) ||
(patchesSM != nil && patchJson != nil) {
(patchesSM != nil && patchesJson != nil) {
return fmt.Errorf(
"illegally qualifies as both an SM and JSON patch: [%v]",
p.Patch)
Expand All @@ -74,14 +77,14 @@ func (p *plugin) Config(
}
}
} else {
p.jsonPatch = patchJson
p.jsonPatch = patchesJson
}
return nil
}

func (p *plugin) Transform(m resmap.ResMap) error {
if p.smPatches == nil {
return p.transformJson6902(m, p.jsonPatch)
return p.transformJson6902(m)
}
// The patch was a strategic merge patch
return p.transformStrategicMerge(m)
Expand All @@ -101,6 +104,9 @@ func (p *plugin) transformStrategicMerge(m resmap.ResMap) error {
return fmt.Errorf("%w", err)
}
continue
} else if len(p.smPatches) > 1 {
// detail: https://github.com/kubernetes-sigs/kustomize/issues/5049#issuecomment-1440604403
return fmt.Errorf("Multiple Strategic-Merge Patch in 'patches' is not allowed to set 'patches.target' field.")
}

selected, err := m.Select(*p.Target)
Expand All @@ -116,7 +122,7 @@ func (p *plugin) transformStrategicMerge(m resmap.ResMap) error {

// transformJson6902 applies the provided json6902 patch
// to all the resources in the ResMap that match the Target.
func (p *plugin) transformJson6902(m resmap.ResMap, patch jsonpatch.Patch) error {
func (p *plugin) transformJson6902(m resmap.ResMap) error {
if p.Target == nil {
return fmt.Errorf("must specify a target for patch %s", p.Patch)
}
Expand Down
63 changes: 63 additions & 0 deletions plugin/builtin/patchtransformer/PatchTransformer_test.go
Expand Up @@ -198,6 +198,69 @@ Patch: "something"
})
}

func TestPatchTransformerNotAllowedMultipleStrategicMergePatches(t *testing.T) {
th := kusttest_test.MakeEnhancedHarness(t).
PrepBuiltin("PatchTransformer")
defer th.Reset()

th.WriteF(`multiplepatches.yaml`, `
apiVersion: apps/v1
kind: Deployment
metadata:
name: audit
namespace: system
spec:
template:
spec:
containers:
- image: AUDIT_IMAGE
name: manager
args:
- --port=8443
- --logtostderr
- --emit-admission-events
- --exempt-namespace=gatekeeper-system
- --operation=webhook
- --disable-opa-builtin=http.send
---
apiVersion: apps/v1
kind: Deployment
metadata:
name: controller-manager
namespace: system
spec:
template:
spec:
containers:
- image: CONTROLLER_IMAGE
name: manager
args:
- --emit-audit-events
- --operation=audit
- --operation=status
- --logtostderr`)

th.RunTransformerAndCheckError(`
apiVersion: builtin
kind: PatchTransformer
metadata:
name: notImportantHere
Path: multiplepatches.yaml
target:
name: .*Deploy
kind: Deployment
`, someDeploymentResources, func(t *testing.T, err error) {
t.Helper()
if err == nil {
t.Fatalf("expected error")
}
if !strings.Contains(err.Error(),
"Multiple Strategic-Merge Patch in 'patches' is not allowed to set 'patches.target' field.") {
t.Fatalf("unexpected err: %v", err)
}
})
}

func TestPatchTransformerFromFiles(t *testing.T) {
th := kusttest_test.MakeEnhancedHarness(t).
PrepBuiltin("PatchTransformer")
Expand Down

0 comments on commit 0bb40bc

Please sign in to comment.