Patchtransformers - drop copied code, improve deletion handling.

This commit is contained in:
jregan
2020-11-29 16:09:32 -08:00
parent c9ab1270fa
commit 4a55a07c14
12 changed files with 654 additions and 205 deletions

View File

@@ -5,14 +5,10 @@ package builtins
import (
"fmt"
"strings"
"github.com/pkg/errors"
"sigs.k8s.io/kustomize/api/filters/patchstrategicmerge"
"sigs.k8s.io/kustomize/api/resmap"
"sigs.k8s.io/kustomize/api/resource"
"sigs.k8s.io/kustomize/api/types"
"sigs.k8s.io/kustomize/kyaml/filtersutil"
"sigs.k8s.io/yaml"
)
@@ -73,44 +69,10 @@ func (p *PatchStrategicMergeTransformerPlugin) Transform(m resmap.ResMap) error
if err != nil {
return err
}
patchCopy := patch.DeepCopy()
patchCopy.SetName(target.GetName())
patchCopy.SetNamespace(target.GetNamespace())
patchCopy.SetGvk(target.GetGvk())
node, err := filtersutil.GetRNode(patchCopy)
if err != nil {
if err = m.ApplySmPatch(
resource.MakeIdSet([]*resource.Resource{target}), patch); err != nil {
return err
}
err = filtersutil.ApplyToJSON(patchstrategicmerge.Filter{
Patch: node,
}, target)
if err != nil {
// Check for an error string from UnmarshalJSON that's indicative
// of an object that's missing basic KRM fields, and thus may have been
// entirely deleted (an acceptable outcome). This error handling should
// be deleted along with use of ResMap and apimachinery functions like
// UnmarshalJSON.
if !strings.Contains(err.Error(), "Object 'Kind' is missing") {
// Some unknown error, let it through.
return err
}
if !target.IsEmpty() {
return errors.Wrapf(
err, "with unexpectedly non-empty object map of size %d",
len(target.Map()))
}
// Fall through to handle deleted object.
}
if target.IsEmpty() {
// This means all fields have been removed from the object.
// This can happen if a patch required deletion of the
// entire resource (not just a part of it). This means
// the overall resmap must shrink by one.
err = m.Remove(target.CurId())
if err != nil {
return err
}
}
}
return nil
}

View File

@@ -8,9 +8,7 @@ import (
"strings"
jsonpatch "github.com/evanphx/json-patch"
"github.com/pkg/errors"
"sigs.k8s.io/kustomize/api/filters/patchjson6902"
"sigs.k8s.io/kustomize/api/filters/patchstrategicmerge"
"sigs.k8s.io/kustomize/api/resmap"
"sigs.k8s.io/kustomize/api/resource"
"sigs.k8s.io/kustomize/api/types"
@@ -41,7 +39,6 @@ func (p *PatchTransformerPlugin) Config(
return fmt.Errorf(
"patch and path can't be set at the same time\n%s", string(c))
}
if p.Path != "" {
loaded, loadErr := h.Loader().Load(p.Path)
if loadErr != nil {
@@ -88,66 +85,13 @@ func (p *PatchTransformerPlugin) transformStrategicMerge(m resmap.ResMap, patch
if err != nil {
return err
}
return p.applySMPatch(target, patch)
return target.ApplySmPatch(patch)
}
resources, err := m.Select(*p.Target)
selected, err := m.Select(*p.Target)
if err != nil {
return err
}
for _, res := range resources {
patchCopy := patch.DeepCopy()
patchCopy.SetName(res.GetName())
patchCopy.SetNamespace(res.GetNamespace())
patchCopy.SetGvk(res.GetGvk())
err := p.applySMPatch(res, patchCopy)
if err != nil {
// Check for an error string from UnmarshalJSON that's indicative
// of an object that's missing basic KRM fields, and thus may have been
// entirely deleted (an acceptable outcome). This error handling should
// be deleted along with use of ResMap and apimachinery functions like
// UnmarshalJSON.
if !strings.Contains(err.Error(), "Object 'Kind' is missing") {
// Some unknown error, let it through.
return err
}
if !res.IsEmpty() {
return errors.Wrapf(
err, "with unexpectedly non-empty object map of size %d",
len(res.Map()))
}
// Fall through to handle deleted object.
}
if res.IsEmpty() {
// This means all fields have been removed from the object.
// This can happen if a patch required deletion of the
// entire resource (not just a part of it). This means
// the overall resmap must shrink by one.
err = m.Remove(res.CurId())
if err != nil {
return err
}
}
}
return nil
}
// applySMPatch applies the provided strategic merge patch to the
// given resource.
func (p *PatchTransformerPlugin) applySMPatch(resource, patch *resource.Resource) error {
node, err := filtersutil.GetRNode(patch)
if err != nil {
return err
}
n, ns := resource.GetName(), resource.GetNamespace()
err = filtersutil.ApplyToJSON(patchstrategicmerge.Filter{
Patch: node,
}, resource)
if !resource.IsEmpty() {
resource.SetName(n)
resource.SetNamespace(ns)
}
return err
return m.ApplySmPatch(resource.MakeIdSet(selected), patch)
}
// transformJson6902 applies the provided json6902 patch

View File

@@ -183,8 +183,7 @@ var transformerConfigurators = map[builtinhelpers.BuiltinPluginType]func(
return
}
var c struct {
Paths []types.PatchStrategicMerge `json:"paths,omitempty" yaml:"paths,omitempty"`
Patches string `json:"patches,omitempty" yaml:"patches,omitempty"`
Paths []types.PatchStrategicMerge `json:"paths,omitempty" yaml:"paths,omitempty"`
}
c.Paths = kt.kustomization.PatchesStrategicMerge
p := f()

View File

@@ -240,4 +240,9 @@ type ResMap interface {
// ToRNodeSlice converts the resources in the resmp
// to a list of RNodes
ToRNodeSlice() ([]*yaml.RNode, error)
// ApplySmPatch applies a strategic-merge patch to the
// selected set of resources.
ApplySmPatch(
selectedSet *resource.IdSet, patch *resource.Resource) error
}

View File

@@ -6,6 +6,7 @@ package resmap
import (
"bytes"
"fmt"
"strings"
"github.com/pkg/errors"
"sigs.k8s.io/kustomize/api/resid"
@@ -578,3 +579,46 @@ func (m *resWrangler) ToRNodeSlice() ([]*kyaml_yaml.RNode, error) {
}
return rnodes, nil
}
func (m *resWrangler) ApplySmPatch(
selectedSet *resource.IdSet, patch *resource.Resource) error {
newRm := New()
for _, res := range m.Resources() {
if !selectedSet.Contains(res.CurId()) {
newRm.Append(res)
continue
}
patchCopy := patch.DeepCopy()
patchCopy.SetName(res.GetName())
patchCopy.SetNamespace(res.GetNamespace())
patchCopy.SetGvk(res.GetGvk())
err := res.ApplySmPatch(patchCopy)
if err != nil {
// Check for an error string from UnmarshalJSON that's indicative
// of an object that's missing basic KRM fields, and thus may have been
// entirely deleted (an acceptable outcome). This error handling should
// be deleted along with use of ResMap and apimachinery functions like
// UnmarshalJSON.
if !strings.Contains(err.Error(), "Object 'Kind' is missing") {
// Some unknown error, let it through.
return err
}
if !res.IsEmpty() {
return errors.Wrapf(
err, "with unexpectedly non-empty object map of size %d",
len(res.Map()))
}
// Fall through to handle deleted object.
}
if !res.IsEmpty() {
// IsEmpty means all fields have been removed from the object.
// This can happen if a patch required deletion of the
// entire resource (not just a part of it). This means
// the overall resmap must shrink by one.
newRm.Append(res)
}
}
m.Clear()
m.AppendAll(newRm)
return nil
}

View File

@@ -9,9 +9,11 @@ import (
"reflect"
"strings"
"sigs.k8s.io/kustomize/api/filters/patchstrategicmerge"
"sigs.k8s.io/kustomize/api/ifc"
"sigs.k8s.io/kustomize/api/resid"
"sigs.k8s.io/kustomize/api/types"
"sigs.k8s.io/kustomize/kyaml/filtersutil"
"sigs.k8s.io/yaml"
)
@@ -396,6 +398,23 @@ func (r *Resource) AppendRefVarName(variable types.Var) {
r.refVarNames = append(r.refVarNames, variable.Name)
}
// ApplySmPatch applies the provided strategic merge patch.
func (r *Resource) ApplySmPatch(patch *Resource) error {
node, err := filtersutil.GetRNode(patch)
if err != nil {
return err
}
n, ns := r.GetName(), r.GetNamespace()
err = filtersutil.ApplyToJSON(patchstrategicmerge.Filter{
Patch: node,
}, r)
if !r.IsEmpty() {
r.SetName(n)
r.SetNamespace(ns)
}
return err
}
// TODO: Add BinaryData once we sync to new k8s.io/api
func mergeConfigmap(
mergedTo map[string]interface{},

View File

@@ -4,9 +4,11 @@
package resource_test
import (
"fmt"
"reflect"
"testing"
"github.com/stretchr/testify/assert"
"sigs.k8s.io/kustomize/api/provider"
"sigs.k8s.io/kustomize/api/resid"
. "sigs.k8s.io/kustomize/api/resource"
@@ -123,3 +125,575 @@ func TestDeepCopy(t *testing.T) {
t.Errorf("expected %v\nbut got%v", r, cr)
}
}
func TestApplySmPatch_1(t *testing.T) {
resource, err := factory.FromBytes([]byte(`
apiVersion: apps/v1
kind: Deployment
metadata:
annotations:
baseAnno: This is a base annotation
labels:
app: mungebot
foo: bar
name: bingo
spec:
replicas: 1
selector:
matchLabels:
foo: bar
template:
metadata:
labels:
app: mungebot
spec:
containers:
- env:
- name: foo
value: bar
image: nginx
name: nginx
ports:
- containerPort: 80
`))
assert.NoError(t, err)
patch, err := factory.FromBytes([]byte(`
apiVersion: apps/v1
kind: Deployment
metadata:
name: baseprefix-mungebot
spec:
template:
spec:
containers:
- image: nginx
name: nginx
ports:
- containerPort: 777
`))
assert.NoError(t, err)
assert.NoError(t, resource.ApplySmPatch(patch))
bytes, err := resource.AsYAML()
assert.NoError(t, err)
assert.Equal(t, `apiVersion: apps/v1
kind: Deployment
metadata:
annotations:
baseAnno: This is a base annotation
labels:
app: mungebot
foo: bar
name: bingo
spec:
replicas: 1
selector:
matchLabels:
foo: bar
template:
metadata:
labels:
app: mungebot
spec:
containers:
- env:
- name: foo
value: bar
image: nginx
name: nginx
ports:
- containerPort: 777
- containerPort: 80
`, string(bytes))
}
func TestApplySmPatch(t *testing.T) {
const (
myDeployment = "Deployment"
myCRD = "myCRD"
)
tests := map[string]struct {
base string
patch []string
expected string
errorExpected bool
errorMsg string
}{
"withschema-label-image-container": {
base: baseResource(myDeployment),
patch: []string{
addLabelAndEnvPatch(myDeployment),
changeImagePatch(myDeployment, "nginx:latest"),
addContainerAndEnvPatch(myDeployment),
},
errorExpected: false,
expected: expectedResultMultiPatch(myDeployment, false),
},
"withschema-image-container-label": {
base: baseResource(myDeployment),
patch: []string{
changeImagePatch(myDeployment, "nginx:latest"),
addContainerAndEnvPatch(myDeployment),
addLabelAndEnvPatch(myDeployment),
},
errorExpected: false,
expected: expectedResultMultiPatch(myDeployment, true),
},
"withschema-container-label-image": {
base: baseResource(myDeployment),
patch: []string{
addContainerAndEnvPatch(myDeployment),
addLabelAndEnvPatch(myDeployment),
changeImagePatch(myDeployment, "nginx:latest"),
},
errorExpected: false,
expected: expectedResultMultiPatch(myDeployment, true),
},
"noschema-label-image-container": {
base: baseResource(myCRD),
patch: []string{
addLabelAndEnvPatch(myCRD),
changeImagePatch(myCRD, "nginx:latest"),
addContainerAndEnvPatch(myCRD),
},
// Might be better if this complained about patch conflict.
// See plugin/builtin/patchstrategicmergetransformer/psmt_test.go
expected: `apiVersion: apps/v1
kind: myCRD
metadata:
name: deploy1
spec:
template:
metadata:
labels:
old-label: old-value
some-label: some-value
spec:
containers:
- env:
- name: ANOTHERENV
value: ANOTHERVALUE
name: nginx
- image: anotherimage
name: anothercontainer
`,
},
"noschema-image-container-label": {
base: baseResource(myCRD),
patch: []string{
changeImagePatch(myCRD, "nginx:latest"),
addContainerAndEnvPatch(myCRD),
addLabelAndEnvPatch(myCRD),
},
// Might be better if this complained about patch conflict.
expected: `apiVersion: apps/v1
kind: myCRD
metadata:
name: deploy1
spec:
template:
metadata:
labels:
old-label: old-value
some-label: some-value
spec:
containers:
- env:
- name: SOMEENV
value: SOMEVALUE
name: nginx
`,
},
"noschema-container-label-image": {
base: baseResource(myCRD),
patch: []string{
addContainerAndEnvPatch(myCRD),
addLabelAndEnvPatch(myCRD),
changeImagePatch(myCRD, "nginx:latest"),
},
// Might be better if this complained about patch conflict.
expected: `apiVersion: apps/v1
kind: myCRD
metadata:
name: deploy1
spec:
template:
metadata:
labels:
old-label: old-value
some-label: some-value
spec:
containers:
- image: nginx:latest
name: nginx
`,
},
"withschema-label-latest-someV-01": {
base: baseResource(myDeployment),
patch: []string{
addLabelAndEnvPatch(myDeployment),
changeImagePatch(myDeployment, "nginx:latest"),
changeImagePatch(myDeployment, "nginx:1.7.9"),
},
expected: `apiVersion: apps/v1
kind: Deployment
metadata:
name: deploy1
spec:
template:
metadata:
labels:
old-label: old-value
some-label: some-value
spec:
containers:
- env:
- name: SOMEENV
value: SOMEVALUE
image: nginx:1.7.9
name: nginx
`,
},
"withschema-latest-label-someV-02": {
base: baseResource(myDeployment),
patch: []string{
changeImagePatch(myDeployment, "nginx:latest"),
addLabelAndEnvPatch(myDeployment),
changeImagePatch(myDeployment, "nginx:1.7.9"),
},
expected: `apiVersion: apps/v1
kind: Deployment
metadata:
name: deploy1
spec:
template:
metadata:
labels:
old-label: old-value
some-label: some-value
spec:
containers:
- env:
- name: SOMEENV
value: SOMEVALUE
image: nginx:1.7.9
name: nginx
`,
},
"withschema-latest-label-someV-03": {
base: baseResource(myDeployment),
patch: []string{
changeImagePatch(myDeployment, "nginx:1.7.9"),
addLabelAndEnvPatch(myDeployment),
changeImagePatch(myDeployment, "nginx:latest"),
},
expected: `apiVersion: apps/v1
kind: Deployment
metadata:
name: deploy1
spec:
template:
metadata:
labels:
old-label: old-value
some-label: some-value
spec:
containers:
- env:
- name: SOMEENV
value: SOMEVALUE
image: nginx:latest
name: nginx
`,
},
"withschema-latest-label-someV-04": {
base: baseResource(myDeployment),
patch: []string{
changeImagePatch(myDeployment, "nginx:1.7.9"),
changeImagePatch(myDeployment, "nginx:latest"),
addLabelAndEnvPatch(myDeployment),
changeImagePatch(myDeployment, "nginx:nginx"),
},
expected: `apiVersion: apps/v1
kind: Deployment
metadata:
name: deploy1
spec:
template:
metadata:
labels:
old-label: old-value
some-label: some-value
spec:
containers:
- env:
- name: SOMEENV
value: SOMEVALUE
image: nginx:nginx
name: nginx
`,
},
"noschema-latest-label-someV-01": {
base: baseResource(myCRD),
patch: []string{
addLabelAndEnvPatch(myCRD),
changeImagePatch(myCRD, "nginx:latest"),
changeImagePatch(myCRD, "nginx:1.7.9"),
},
expected: `apiVersion: apps/v1
kind: myCRD
metadata:
name: deploy1
spec:
template:
metadata:
labels:
old-label: old-value
some-label: some-value
spec:
containers:
- image: nginx:1.7.9
name: nginx
`,
},
"noschema-latest-label-someV-02": {
base: baseResource(myCRD),
patch: []string{
changeImagePatch(myCRD, "nginx:latest"),
addLabelAndEnvPatch(myCRD),
changeImagePatch(myCRD, "nginx:1.7.9"),
},
expected: expectedResultJMP("nginx:1.7.9"),
},
"noschema-latest-label-someV-03": {
base: baseResource(myCRD),
patch: []string{
changeImagePatch(myCRD, "nginx:1.7.9"),
addLabelAndEnvPatch(myCRD),
changeImagePatch(myCRD, "nginx:latest"),
},
expected: `apiVersion: apps/v1
kind: myCRD
metadata:
name: deploy1
spec:
template:
metadata:
labels:
old-label: old-value
some-label: some-value
spec:
containers:
- image: nginx:latest
name: nginx
`,
},
"noschema-latest-label-someV-04": {
base: baseResource(myCRD),
patch: []string{
changeImagePatch(myCRD, "nginx:1.7.9"),
changeImagePatch(myCRD, "nginx:latest"),
addLabelAndEnvPatch(myCRD),
changeImagePatch(myCRD, "nginx:nginx"),
},
expected: `apiVersion: apps/v1
kind: myCRD
metadata:
name: deploy1
spec:
template:
metadata:
labels:
old-label: old-value
some-label: some-value
spec:
containers:
- image: nginx:nginx
name: nginx
`,
},
}
for name, test := range tests {
resource, err := factory.FromBytes([]byte(test.base))
assert.NoError(t, err)
for _, p := range test.patch {
patch, err := factory.FromBytes([]byte(p))
assert.NoError(t, err, name)
assert.NoError(t, resource.ApplySmPatch(patch), name)
}
bytes, err := resource.AsYAML()
if test.errorExpected {
assert.Error(t, err, name)
} else {
assert.NoError(t, err, name)
assert.Equal(t, test.expected, string(bytes), name)
}
}
}
// baseResource produces a base object which used to test
// patch transformation
// Also the structure is matching the Deployment syntax
// the kind can be replaced to allow testing using CRD
// without access to the schema
func baseResource(kind string) string {
res := `
apiVersion: apps/v1
kind: %s
metadata:
name: deploy1
spec:
template:
metadata:
labels:
old-label: old-value
spec:
containers:
- name: nginx
image: nginx`
return fmt.Sprintf(res, kind)
}
// addContainerAndEnvPatch produces a patch object which adds
// an entry in the env slice of the first/nginx container
// as well as adding a label in the metadata
// Note that for SMP/WithSchema merge, the name:nginx entry
// is mandatory
func addLabelAndEnvPatch(kind string) string {
return fmt.Sprintf(`
apiVersion: apps/v1
kind: %s
metadata:
name: deploy1
spec:
template:
metadata:
labels:
some-label: some-value
spec:
containers:
- name: nginx
env:
- name: SOMEENV
value: SOMEVALUE`, kind)
}
// addContainerAndEnvPatch produces a patch object which adds
// an entry in the env slice of the first/nginx container
// as well as adding a second container in the container list
// Note that for SMP/WithSchema merge, the name:nginx entry
// is mandatory
func addContainerAndEnvPatch(kind string) string {
return fmt.Sprintf(`
apiVersion: apps/v1
kind: %s
metadata:
name: deploy1
spec:
template:
spec:
containers:
- name: nginx
env:
- name: ANOTHERENV
value: ANOTHERVALUE
- name: anothercontainer
image: anotherimage`, kind)
}
// addContainerAndEnvPatch produces a patch object which replaces
// the value of the image field in the first/nginx container
// Note that for SMP/WithSchema merge, the name:nginx entry
// is mandatory
func changeImagePatch(kind string, newImage string) string {
return fmt.Sprintf(`
apiVersion: apps/v1
kind: %s
metadata:
name: deploy1
spec:
template:
spec:
containers:
- name: nginx
image: %s`, kind, newImage)
}
// utility method to build the expected result of a multipatch
// the order of the patches still have influence especially
// in the insertion location within arrays.
func expectedResultMultiPatch(kind string, reversed bool) string {
pattern := `apiVersion: apps/v1
kind: %s
metadata:
name: deploy1
spec:
template:
metadata:
labels:
old-label: old-value
some-label: some-value
spec:
containers:
- env:
%s
image: nginx:latest
name: nginx
- image: anotherimage
name: anothercontainer
`
if reversed {
return fmt.Sprintf(pattern, kind, `- name: SOMEENV
value: SOMEVALUE
- name: ANOTHERENV
value: ANOTHERVALUE`)
}
return fmt.Sprintf(pattern, kind, `- name: ANOTHERENV
value: ANOTHERVALUE
- name: SOMEENV
value: SOMEVALUE`)
}
// utility method building the expected output of a JMP.
// imagename parameter allows to build a result consistent
// with the JMP behavior which basically overrides the
// entire "containers" list.
func expectedResultJMP(imagename string) string {
if imagename == "" {
return `apiVersion: apps/v1
kind: myCRD
metadata:
name: deploy1
spec:
template:
metadata:
labels:
old-label: old-value
some-label: some-value
spec:
containers:
- env:
- name: SOMEENV
value: SOMEVALUE
name: nginx
`
}
return fmt.Sprintf(`apiVersion: apps/v1
kind: myCRD
metadata:
name: deploy1
spec:
template:
metadata:
labels:
old-label: old-value
some-label: some-value
spec:
containers:
- image: %s
name: nginx
`, imagename)
}