diff --git a/changelogs/unreleased/6917-27149chen b/changelogs/unreleased/6917-27149chen new file mode 100644 index 0000000000..94648eaa40 --- /dev/null +++ b/changelogs/unreleased/6917-27149chen @@ -0,0 +1 @@ +Support JSON Merge Patch and Strategic Merge Patch in Resource Modifiers \ No newline at end of file diff --git a/design/merge-patch-and-strategic-in-resource-modifier.md b/design/merge-patch-and-strategic-in-resource-modifier.md new file mode 100644 index 0000000000..a8127bc495 --- /dev/null +++ b/design/merge-patch-and-strategic-in-resource-modifier.md @@ -0,0 +1,193 @@ +# Proposal to Support JSON Merge Patch and Strategic Merge Patch in Resource Modifiers + +- [Proposal to Support JSON Merge Patch and Strategic Merge Patch in Resource Modifiers](#proposal-to-support-json-merge-patch-and-strategic-merge-patch-in-resource-modifiers) + - [Abstract](#abstract) + - [Goals](#goals) + - [Non Goals](#non-goals) + - [User Stories](#user-stories) + - [Scenario 1](#scenario-1) + - [Scenario 2](#scenario-2) + - [Detailed Design](#detailed-design) + - [How to choose the right patch type](#how-to-choose-the-right-patch-type) + - [New Field MergePatches](#new-field-mergepatches) + - [New Field StrategicPatches](#new-field-strategicpatches) + - [Conditional Patches in ALL Patch Types](#conditional-patches-in-all-patch-types) + - [Wildcard Support for GroupResource](#wildcard-support-for-groupresource) + - [Helper Command to Generate Merge Patch and Strategic Merge Patch](#helper-command-to-generate-merge-patch-and-strategic-merge-patch) + - [Security Considerations](#security-considerations) + - [Compatibility](#compatibility) + - [Implementation](#implementation) + - [Future Enhancements](#future-enhancements) + - [Open Issues](#open-issues) + +## Abstract +Velero introduced the concept of Resource Modifiers in v1.12.0. This feature allows the user to specify a configmap with a set of rules to modify the resources during restore. The user can specify the filters to select the resources and then specify the JSON Patch to apply on the resource. This feature is currently limited to the operations supported by JSON Patch RFC. +This proposal is to add support for JSON Merge Patch and Strategic Merge Patch in the Resource Modifiers. This will allow the user to use the same configmap to apply JSON Merge Patch and Strategic Merge Patch on the resources during restore. + +## Goals +- Allow the user to specify a JSON patch, JSON Merge Patch or Strategic Merge Patch for modification. +- Allow the user to specify multiple JSON Patch, JSON Merge Patch or Strategic Merge Patch. +- Allow the user to specify mixed JSON Patch, JSON Merge Patch and Strategic Merge Patch in the same configmap. + +## Non Goals +- Deprecating the existing RestoreItemAction plugins for standard substitutions(like changing the namespace, changing the storage class, etc.) + +## User Stories + +### Scenario 1 +- Alice has some Pods and part of them have an annotation `{"for": "bar"}`. +- Alice wishes to restore these Pods to a different cluster without this annotation. +- Alice can use this feature to remove this annotation during restore. + +### Scenario 2 +- Bob has a Pod with several containers and one container with name nginx has an image `repo1/nginx`. +- Bob wishes to restore this Pod to a different cluster, but new cluster can not access repo1, so he pushes the image to repo2. +- Bob can use this feature to update the image of container nginx to `repo2/nginx` during restore. + +## Detailed Design +- The design and approach is inspired by kubectl patch command and [this doc](https://kubernetes.io/docs/tasks/manage-kubernetes-objects/update-api-object-kubectl-patch/). +- New fields `MergePatches` and `StrategicPatches` will be added to the `ResourceModifierRule` struct to support all three patch types. +- Only one of the three patch types can be specified in a single `ResourceModifierRule`. +- Add wildcard support for `groupResource` in `conditions` struct. +- The workflow to create Resource Modifier ConfigMap and reference it in RestoreSpec will remain the same as described in document [Resource Modifiers](https://github.com/vmware-tanzu/velero/blob/main/site/content/docs/main/restore-resource-modifiers.md). + +### How to choose the right patch type +- [JSON Merge Patch](https://datatracker.ietf.org/doc/html/rfc7386) is a naively simple format, with limited usability. Probably it is a good choice if you are building something small, with very simple JSON Schema. +- [JSON Patch](https://datatracker.ietf.org/doc/html/rfc6902) is a more complex format, but it is applicable to any JSON documents. For a comparison of JSON patch and JSON merge patch, see [JSON Patch and JSON Merge Patch](https://erosb.github.io/post/json-patch-vs-merge-patch/). +- Strategic Merge Patch is a Kubernetes defined patch type, mainly used to process resources of type list. You can replace/merge a list, add/remove items from a list by key, change the order of items in a list, etc. Strategic merge patch is not supported for custom resources. For more details, see [this doc](https://kubernetes.io/docs/tasks/manage-kubernetes-objects/update-api-object-kubectl-patch/). + +### New Field MergePatches +MergePatches is a list to specify the merge patches to be applied on the resource. The merge patches will be applied in the order specified in the configmap. A subsequent patch is applied in order and if multiple patches are specified for the same path, the last patch will override the previous patches. + +Example of MergePatches in ResourceModifierRule +```yaml +version: v1 +resourceModifierRules: +- conditions: + groupResource: pods + namespaces: + - ns1 + mergePatches: + - patchData: | + { + "metadata": { + "annotations": { + "foo": null + } + } + } +``` +- The above configmap will apply the Merge Patch to all the pods in namespace ns1 and remove the annotation `foo` from the pods. +- Both json and yaml format are supported for the patchData. + +### New Field StrategicPatches +StrategicPatches is a list to specify the strategic merge patches to be applied on the resource. The strategic merge patches will be applied in the order specified in the configmap. A subsequent patch is applied in order and if multiple patches are specified for the same path, the last patch will override the previous patches. + +Example of StrategicPatches in ResourceModifierRule +```yaml +version: v1 +resourceModifierRules: +- conditions: + groupResource: pods + resourceNameRegex: "^my-pod$" + namespaces: + - ns1 + strategicPatches: + - patchData: | + { + "spec": { + "containers": [ + { + "name": "nginx", + "image": "repo2/nginx" + } + ] + } + } +``` +- The above configmap will apply the Strategic Merge Patch to the pod with name my-pod in namespace ns1 and update the image of container nginx to `repo2/nginx`. +- Both json and yaml format are supported for the patchData. + +### Conditional Patches in ALL Patch Types +Since JSON Merge Patch and Strategic Merge Patch do not support conditional patches, we will use the `test` operation of JSON Patch to support conditional patches in all patch types by adding it to `Conditions` struct in `ResourceModifierRule`. + +Example of test in conditions +```yaml +version: v1 +resourceModifierRules: +- conditions: + groupResource: persistentvolumeclaims.storage.k8s.io + matches: + - path: "/spec/storageClassName" + value: "premium" + mergePatches: + - patchData: | + { + "metadata": { + "annotations": { + "foo": null + } + } + } +``` +- The above configmap will apply the Merge Patch to all the PVCs in all namespaces with storageClassName premium and remove the annotation `foo` from the PVCs. +- You can specify multiple rules in the `matches` list. The patch will be applied only if all the matches are satisfied. + +### Wildcard Support for GroupResource +The user can specify a wildcard for groupResource in the conditions' struct. This will allow the user to apply the patches for all the resources of a particular group or all resources in all groups. For example, `*.apps` will apply to all the resources in the `apps` group, `*` will apply to all the resources in all groups. + +### Helper Command to Generate Merge Patch and Strategic Merge Patch +The patchData of Strategic Merge Patch is sometimes a bit complex for user to write. We can provide a helper command to generate the patchData for Strategic Merge Patch. The command will take the original resource and the modified resource as input and generate the patchData. +It can also be used in JSON Merge Patch. + +Here is a sample code snippet to achieve this: +```go +package main + +import ( + "fmt" + + corev1 "k8s.io/api/core/v1" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +func main() { + pod := &corev1.Pod{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "web", + Image: "nginx", + }, + }, + }, + } + newPod := pod.DeepCopy() + patch := client.StrategicMergeFrom(pod) + newPod.Spec.Containers[0].Image = "nginx1" + + data, _ := patch.Data(newPod) + fmt.Println(string(data)) + // Output: + // {"spec":{"$setElementOrder/containers":[{"name":"web"}],"containers":[{"image":"nginx1","name":"web"}]}} +} +``` + +## Security Considerations +No security impact. + +## Compatibility +Compatible with current Resource Modifiers. + +## Implementation +- Use "github.com/evanphx/json-patch" to support JSON Merge Patch. +- Use "k8s.io/apimachinery/pkg/util/strategicpatch" to support Strategic Merge Patch. +- Use glob to support wildcard for `groupResource` in `conditions` struct. +- Use `test` operation of JSON Patch to calculate the `matches` in `conditions` struct. + +## Future enhancements +- add a Velero subcommand to generate/validate the patchData for Strategic Merge Patch and JSON Merge Patch. +- add jq support for more complex conditions or patches, to meet the situations that the current conditions or patches can not handle. like [this issue](https://github.com/vmware-tanzu/velero/issues/6344) + +## Open Issues +N/A diff --git a/go.mod b/go.mod index 42b473e536..3b07926725 100644 --- a/go.mod +++ b/go.mod @@ -54,6 +54,7 @@ require ( k8s.io/metrics v0.25.6 k8s.io/utils v0.0.0-20220728103510-ee6ede2d64ed sigs.k8s.io/controller-runtime v0.12.2 + sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd sigs.k8s.io/yaml v1.3.0 ) @@ -155,7 +156,6 @@ require ( gopkg.in/yaml.v2 v2.4.0 // indirect k8s.io/component-base v0.24.2 // indirect k8s.io/kube-openapi v0.0.0-20220803162953-67bda5d908f1 // indirect - sigs.k8s.io/json v0.0.0-20220713155537-f223a00ba0e2 // indirect sigs.k8s.io/structured-merge-diff/v4 v4.2.3 // indirect ) diff --git a/go.sum b/go.sum index f7462c0dfa..5aaf8b275e 100644 --- a/go.sum +++ b/go.sum @@ -1376,8 +1376,8 @@ sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.0.30/go.mod h1:fEO7lR sigs.k8s.io/controller-runtime v0.12.2 h1:nqV02cvhbAj7tbt21bpPpTByrXGn2INHRsi39lXy9sE= sigs.k8s.io/controller-runtime v0.12.2/go.mod h1:qKsk4WE6zW2Hfj0G4v10EnNB2jMG1C+NTb8h+DwCoU0= sigs.k8s.io/json v0.0.0-20211208200746-9f7c6b3444d2/go.mod h1:B+TnT182UBxE84DiCz4CVE26eOSDAeYCpfDnC2kdKMY= -sigs.k8s.io/json v0.0.0-20220713155537-f223a00ba0e2 h1:iXTIw73aPyC+oRdyqqvVJuloN1p0AC/kzH07hu3NE+k= -sigs.k8s.io/json v0.0.0-20220713155537-f223a00ba0e2/go.mod h1:B8JuhiUyNFVKdsE8h686QcCxMaH6HrOAZj4vswFpcB0= +sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd h1:EDPBXCAspyGV4jQlpZSudPeMmr1bNJefnuqLsRAsHZo= +sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd/go.mod h1:B8JuhiUyNFVKdsE8h686QcCxMaH6HrOAZj4vswFpcB0= sigs.k8s.io/kustomize/api v0.8.11/go.mod h1:a77Ls36JdfCWojpUqR6m60pdGY1AYFix4AH83nJtY1g= sigs.k8s.io/kustomize/api v0.11.4/go.mod h1:k+8RsqYbgpkIrJ4p9jcdPqe8DprLxFUUO0yNOq8C+xI= sigs.k8s.io/kustomize/kyaml v0.11.0/go.mod h1:GNMwjim4Ypgp/MueD3zXHLRJEjz7RvtPae0AwlvEMFM= diff --git a/internal/resourcemodifiers/json_merge_patch.go b/internal/resourcemodifiers/json_merge_patch.go new file mode 100644 index 0000000000..5082e2cf59 --- /dev/null +++ b/internal/resourcemodifiers/json_merge_patch.go @@ -0,0 +1,45 @@ +package resourcemodifiers + +import ( + "fmt" + + jsonpatch "github.com/evanphx/json-patch" + "github.com/sirupsen/logrus" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "sigs.k8s.io/yaml" +) + +type JSONMergePatch struct { + PatchData string `json:"patchData,omitempty"` +} + +type JSONMergePatcher struct { + patches []JSONMergePatch +} + +func (p *JSONMergePatcher) Patch(u *unstructured.Unstructured, _ logrus.FieldLogger) (*unstructured.Unstructured, error) { + objBytes, err := u.MarshalJSON() + if err != nil { + return nil, fmt.Errorf("error in marshaling object %s", err) + } + + for _, patch := range p.patches { + patchBytes, err := yaml.YAMLToJSON([]byte(patch.PatchData)) + if err != nil { + return nil, fmt.Errorf("error in converting YAML to JSON %s", err) + } + + objBytes, err = jsonpatch.MergePatch(objBytes, patchBytes) + if err != nil { + return nil, fmt.Errorf("error in applying JSON Patch: %s", err.Error()) + } + } + + updated := &unstructured.Unstructured{} + err = updated.UnmarshalJSON(objBytes) + if err != nil { + return nil, fmt.Errorf("error in unmarshalling modified object %s", err.Error()) + } + + return updated, nil +} diff --git a/internal/resourcemodifiers/json_merge_patch_test.go b/internal/resourcemodifiers/json_merge_patch_test.go new file mode 100644 index 0000000000..b7323fc617 --- /dev/null +++ b/internal/resourcemodifiers/json_merge_patch_test.go @@ -0,0 +1,41 @@ +package resourcemodifiers + +import ( + "testing" + + "github.com/sirupsen/logrus" + "github.com/stretchr/testify/assert" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" + clientgoscheme "k8s.io/client-go/kubernetes/scheme" +) + +func TestJsonMergePatchFailure(t *testing.T) { + tests := []struct { + name string + data string + }{ + { + name: "patch with bad yaml", + data: "a: b:", + }, + { + name: "patch with bad json", + data: `{"a"::1}`, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + scheme := runtime.NewScheme() + err := clientgoscheme.AddToScheme(scheme) + assert.NoError(t, err) + pt := &JSONMergePatcher{ + patches: []JSONMergePatch{{PatchData: tt.data}}, + } + + u := &unstructured.Unstructured{} + _, err = pt.Patch(u, logrus.New()) + assert.Error(t, err) + }) + } +} diff --git a/internal/resourcemodifiers/json_patch.go b/internal/resourcemodifiers/json_patch.go new file mode 100644 index 0000000000..b5af7c3622 --- /dev/null +++ b/internal/resourcemodifiers/json_patch.go @@ -0,0 +1,96 @@ +package resourcemodifiers + +import ( + "errors" + "fmt" + "strconv" + "strings" + + jsonpatch "github.com/evanphx/json-patch" + "github.com/sirupsen/logrus" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" +) + +type JSONPatch struct { + Operation string `json:"operation"` + From string `json:"from,omitempty"` + Path string `json:"path"` + Value string `json:"value,omitempty"` +} + +func (p *JSONPatch) ToString() string { + if addQuotes(p.Value) { + return fmt.Sprintf(`{"op": "%s", "from": "%s", "path": "%s", "value": "%s"}`, p.Operation, p.From, p.Path, p.Value) + } + return fmt.Sprintf(`{"op": "%s", "from": "%s", "path": "%s", "value": %s}`, p.Operation, p.From, p.Path, p.Value) +} + +func addQuotes(value string) bool { + if value == "" { + return true + } + // if value is null, then don't add quotes + if value == "null" { + return false + } + // if value is a boolean, then don't add quotes + if _, err := strconv.ParseBool(value); err == nil { + return false + } + // if value is a json object or array, then don't add quotes. + if strings.HasPrefix(value, "{") || strings.HasPrefix(value, "[") { + return false + } + // if value is a number, then don't add quotes + if _, err := strconv.ParseFloat(value, 64); err == nil { + return false + } + return true +} + +type JSONPatcher struct { + patches []JSONPatch `yaml:"patches"` +} + +func (p *JSONPatcher) Patch(u *unstructured.Unstructured, logger logrus.FieldLogger) (*unstructured.Unstructured, error) { + modifiedObjBytes, err := p.applyPatch(u) + if err != nil { + if errors.Is(err, jsonpatch.ErrTestFailed) { + logger.Infof("Test operation failed for JSON Patch %s", err.Error()) + return u.DeepCopy(), nil + } + return nil, fmt.Errorf("error in applying JSON Patch %s", err.Error()) + } + + updated := &unstructured.Unstructured{} + err = updated.UnmarshalJSON(modifiedObjBytes) + if err != nil { + return nil, fmt.Errorf("error in unmarshalling modified object %s", err.Error()) + } + + return updated, nil +} + +func (p *JSONPatcher) applyPatch(u *unstructured.Unstructured) ([]byte, error) { + patchBytes := p.patchArrayToByteArray() + jsonPatch, err := jsonpatch.DecodePatch(patchBytes) + if err != nil { + return nil, fmt.Errorf("error in decoding json patch %s", err.Error()) + } + + objBytes, err := u.MarshalJSON() + if err != nil { + return nil, fmt.Errorf("error in marshaling object %s", err.Error()) + } + + return jsonPatch.Apply(objBytes) +} + +func (p *JSONPatcher) patchArrayToByteArray() []byte { + var patches []string + for _, patch := range p.patches { + patches = append(patches, patch.ToString()) + } + patchesStr := strings.Join(patches, ",\n\t") + return []byte(fmt.Sprintf(`[%s]`, patchesStr)) +} diff --git a/internal/resourcemodifiers/resource_modifiers.go b/internal/resourcemodifiers/resource_modifiers.go index dbcd8e7ba4..8d8d976d24 100644 --- a/internal/resourcemodifiers/resource_modifiers.go +++ b/internal/resourcemodifiers/resource_modifiers.go @@ -3,16 +3,16 @@ package resourcemodifiers import ( "fmt" "regexp" - "strconv" - "strings" jsonpatch "github.com/evanphx/json-patch" + "github.com/gobwas/glob" "github.com/pkg/errors" "github.com/sirupsen/logrus" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/runtime" "sigs.k8s.io/yaml" "github.com/vmware-tanzu/velero/pkg/util/collections" @@ -23,11 +23,9 @@ const ( ResourceModifierSupportedVersionV1 = "v1" ) -type JSONPatch struct { - Operation string `json:"operation"` - From string `json:"from,omitempty"` - Path string `json:"path"` - Value string `json:"value,omitempty"` +type MatchRule struct { + Path string `json:"path,omitempty"` + Value string `json:"value,omitempty"` } type Conditions struct { @@ -35,11 +33,14 @@ type Conditions struct { GroupResource string `json:"groupResource"` ResourceNameRegex string `json:"resourceNameRegex,omitempty"` LabelSelector *metav1.LabelSelector `json:"labelSelector,omitempty"` + Matches []MatchRule `json:"matches,omitempty"` } type ResourceModifierRule struct { - Conditions Conditions `json:"conditions"` - Patches []JSONPatch `json:"patches"` + Conditions Conditions `json:"conditions"` + Patches []JSONPatch `json:"patches,omitempty"` + MergePatches []JSONMergePatch `json:"mergePatches,omitempty"` + StrategicPatches []StrategicMergePatch `json:"strategicPatches,omitempty"` } type ResourceModifiers struct { @@ -68,10 +69,10 @@ func GetResourceModifiersFromConfig(cm *v1.ConfigMap) (*ResourceModifiers, error return resModifiers, nil } -func (p *ResourceModifiers) ApplyResourceModifierRules(obj *unstructured.Unstructured, groupResource string, log logrus.FieldLogger) []error { +func (p *ResourceModifiers) ApplyResourceModifierRules(obj *unstructured.Unstructured, groupResource string, scheme *runtime.Scheme, log logrus.FieldLogger) []error { var errs []error for _, rule := range p.ResourceModifierRules { - err := rule.Apply(obj, groupResource, log) + err := rule.apply(obj, groupResource, scheme, log) if err != nil { errs = append(errs, err) } @@ -80,13 +81,22 @@ func (p *ResourceModifiers) ApplyResourceModifierRules(obj *unstructured.Unstruc return errs } -func (r *ResourceModifierRule) Apply(obj *unstructured.Unstructured, groupResource string, log logrus.FieldLogger) error { - namespaceInclusion := collections.NewIncludesExcludes().Includes(r.Conditions.Namespaces...) - if !namespaceInclusion.ShouldInclude(obj.GetNamespace()) { - return nil +func (r *ResourceModifierRule) apply(obj *unstructured.Unstructured, groupResource string, scheme *runtime.Scheme, log logrus.FieldLogger) error { + ns := obj.GetNamespace() + if ns != "" { + namespaceInclusion := collections.NewIncludesExcludes().Includes(r.Conditions.Namespaces...) + if !namespaceInclusion.ShouldInclude(ns) { + return nil + } + } + + g, err := glob.Compile(r.Conditions.GroupResource, '.') + if err != nil { + log.Errorf("Bad glob pattern of groupResource in condition, groupResource: %s, err: %s", r.Conditions.GroupResource, err) + return err } - if r.Conditions.GroupResource != groupResource { + if !g.Match(groupResource) { return nil } @@ -110,87 +120,82 @@ func (r *ResourceModifierRule) Apply(obj *unstructured.Unstructured, groupResour } } - patches, err := r.PatchArrayToByteArray() + match, err := matchConditions(obj, r.Conditions.Matches, log) if err != nil { return err + } else if !match { + log.Info("Conditions do not match, skip it") + return nil } + log.Infof("Applying resource modifier patch on %s/%s", obj.GetNamespace(), obj.GetName()) - err = ApplyPatch(patches, obj, log) + err = r.applyPatch(obj, scheme, log) if err != nil { return err } return nil } -// PatchArrayToByteArray converts all JsonPatch to string array with the format of jsonpatch.Patch and then convert it to byte array -func (r *ResourceModifierRule) PatchArrayToByteArray() ([]byte, error) { - var patches []string - for _, patch := range r.Patches { - patches = append(patches, patch.ToString()) +func matchConditions(u *unstructured.Unstructured, rules []MatchRule, _ logrus.FieldLogger) (bool, error) { + if len(rules) == 0 { + return true, nil } - patchesStr := strings.Join(patches, ",\n\t") - return []byte(fmt.Sprintf(`[%s]`, patchesStr)), nil -} -func (p *JSONPatch) ToString() string { - if addQuotes(p.Value) { - return fmt.Sprintf(`{"op": "%s", "from": "%s", "path": "%s", "value": "%s"}`, p.Operation, p.From, p.Path, p.Value) - } - return fmt.Sprintf(`{"op": "%s", "from": "%s", "path": "%s", "value": %s}`, p.Operation, p.From, p.Path, p.Value) -} + var fixed []JSONPatch + for _, rule := range rules { + if rule.Path == "" { + return false, fmt.Errorf("path is required for match rule") + } -func ApplyPatch(patch []byte, obj *unstructured.Unstructured, log logrus.FieldLogger) error { - jsonPatch, err := jsonpatch.DecodePatch(patch) - if err != nil { - return fmt.Errorf("error in decoding json patch %s", err.Error()) - } - objBytes, err := obj.MarshalJSON() - if err != nil { - return fmt.Errorf("error in marshaling object %s", err.Error()) + fixed = append(fixed, JSONPatch{ + Operation: "test", + Path: rule.Path, + Value: rule.Value, + }) } - modifiedObjBytes, err := jsonPatch.Apply(objBytes) + + p := &JSONPatcher{patches: fixed} + _, err := p.applyPatch(u) if err != nil { if errors.Is(err, jsonpatch.ErrTestFailed) { - log.Infof("Test operation failed for JSON Patch %s", err.Error()) - return nil + return false, nil } - return fmt.Errorf("error in applying JSON Patch %s", err.Error()) - } - err = obj.UnmarshalJSON(modifiedObjBytes) - if err != nil { - return fmt.Errorf("error in unmarshalling modified object %s", err.Error()) + return false, err } - return nil + + return true, nil } func unmarshalResourceModifiers(yamlData []byte) (*ResourceModifiers, error) { resModifiers := &ResourceModifiers{} err := yaml.UnmarshalStrict(yamlData, resModifiers) if err != nil { - return nil, fmt.Errorf("failed to decode yaml data into resource modifiers %v", err) + return nil, fmt.Errorf("failed to decode yaml data into resource modifiers, err: %s", err) } return resModifiers, nil } -func addQuotes(value string) bool { - if value == "" { - return true - } - // if value is null, then don't add quotes - if value == "null" { - return false - } - // if value is a boolean, then don't add quotes - if _, err := strconv.ParseBool(value); err == nil { - return false - } - // if value is a json object or array, then don't add quotes. - if strings.HasPrefix(value, "{") || strings.HasPrefix(value, "[") { - return false +type patcher interface { + Patch(u *unstructured.Unstructured, logger logrus.FieldLogger) (*unstructured.Unstructured, error) +} + +func (r *ResourceModifierRule) applyPatch(u *unstructured.Unstructured, scheme *runtime.Scheme, logger logrus.FieldLogger) error { + var p patcher + if len(r.Patches) > 0 { + p = &JSONPatcher{patches: r.Patches} + } else if len(r.MergePatches) > 0 { + p = &JSONMergePatcher{patches: r.MergePatches} + } else if len(r.StrategicPatches) > 0 { + p = &StrategicMergePatcher{patches: r.StrategicPatches, scheme: scheme} + } else { + return fmt.Errorf("no patch data found") } - // if value is a number, then don't add quotes - if _, err := strconv.ParseFloat(value, 64); err == nil { - return false + + updated, err := p.Patch(u, logger) + if err != nil { + return fmt.Errorf("error in applying patch %s", err) } - return true + + u.SetUnstructuredContent(updated.Object) + return nil } diff --git a/internal/resourcemodifiers/resource_modifiers_test.go b/internal/resourcemodifiers/resource_modifiers_test.go index c2150bbc87..371482eee9 100644 --- a/internal/resourcemodifiers/resource_modifiers_test.go +++ b/internal/resourcemodifiers/resource_modifiers_test.go @@ -9,6 +9,10 @@ import ( v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/serializer/yaml" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" + clientgoscheme "k8s.io/client-go/kubernetes/scheme" ) func TestGetResourceModifiersFromConfig(t *testing.T) { @@ -116,6 +120,128 @@ func TestGetResourceModifiersFromConfig(t *testing.T) { }, } + cm5 := &v1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-configmap", + Namespace: "test-namespace", + }, + Data: map[string]string{ + "sub.yml": "version: v1\nresourceModifierRules:\n- conditions:\n groupResource: pods\n namespaces:\n - ns1\n matches:\n - path: /metadata/annotations/foo\n value: bar\n mergePatches:\n - patchData: |\n metadata:\n annotations:\n foo: null", + }, + } + + rules5 := &ResourceModifiers{ + Version: "v1", + ResourceModifierRules: []ResourceModifierRule{ + { + Conditions: Conditions{ + GroupResource: "pods", + Namespaces: []string{ + "ns1", + }, + Matches: []MatchRule{ + { + Path: "/metadata/annotations/foo", + Value: "bar", + }, + }, + }, + MergePatches: []JSONMergePatch{ + { + PatchData: "metadata:\n annotations:\n foo: null", + }, + }, + }, + }, + } + + cm6 := &v1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-configmap", + Namespace: "test-namespace", + }, + Data: map[string]string{ + "sub.yml": "version: v1\nresourceModifierRules:\n- conditions:\n groupResource: pods\n namespaces:\n - ns1\n strategicPatches:\n - patchData: |\n spec:\n containers:\n - name: nginx\n image: repo2/nginx", + }, + } + + rules6 := &ResourceModifiers{ + Version: "v1", + ResourceModifierRules: []ResourceModifierRule{ + { + Conditions: Conditions{ + GroupResource: "pods", + Namespaces: []string{ + "ns1", + }, + }, + StrategicPatches: []StrategicMergePatch{ + { + PatchData: "spec:\n containers:\n - name: nginx\n image: repo2/nginx", + }, + }, + }, + }, + } + + cm7 := &v1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-configmap", + Namespace: "test-namespace", + }, + Data: map[string]string{ + "sub.yml": "version: v1\nresourceModifierRules:\n- conditions:\n groupResource: pods\n namespaces:\n - ns1\n mergePatches:\n - patchData: |\n {\"metadata\":{\"annotations\":{\"foo\":null}}}", + }, + } + + rules7 := &ResourceModifiers{ + Version: "v1", + ResourceModifierRules: []ResourceModifierRule{ + { + Conditions: Conditions{ + GroupResource: "pods", + Namespaces: []string{ + "ns1", + }, + }, + MergePatches: []JSONMergePatch{ + { + PatchData: `{"metadata":{"annotations":{"foo":null}}}`, + }, + }, + }, + }, + } + + cm8 := &v1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-configmap", + Namespace: "test-namespace", + }, + Data: map[string]string{ + "sub.yml": "version: v1\nresourceModifierRules:\n- conditions:\n groupResource: pods\n namespaces:\n - ns1\n strategicPatches:\n - patchData: |\n {\"spec\":{\"containers\":[{\"name\": \"nginx\",\"image\": \"repo2/nginx\"}]}}", + }, + } + + rules8 := &ResourceModifiers{ + Version: "v1", + ResourceModifierRules: []ResourceModifierRule{ + { + Conditions: Conditions{ + GroupResource: "pods", + Namespaces: []string{ + "ns1", + }, + }, + StrategicPatches: []StrategicMergePatch{ + { + PatchData: `{"spec":{"containers":[{"name": "nginx","image": "repo2/nginx"}]}}`, + }, + }, + }, + }, + } + type args struct { cm *v1.ConfigMap } @@ -165,6 +291,38 @@ func TestGetResourceModifiersFromConfig(t *testing.T) { want: nil, wantErr: true, }, + { + name: "complex yaml data with json merge patch", + args: args{ + cm: cm5, + }, + want: rules5, + wantErr: false, + }, + { + name: "complex yaml data with strategic merge patch", + args: args{ + cm: cm6, + }, + want: rules6, + wantErr: false, + }, + { + name: "complex json data with json merge patch", + args: args{ + cm: cm7, + }, + want: rules7, + wantErr: false, + }, + { + name: "complex json data with strategic merge patch", + args: args{ + cm: cm8, + }, + want: rules8, + wantErr: false, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -487,6 +645,38 @@ func TestResourceModifiers_ApplyResourceModifierRules(t *testing.T) { wantErr: false, wantObj: deployNginxTwoReplica.DeepCopy(), }, + { + name: "nginx deployment: Empty Resource Regex", + fields: fields{ + Version: "v1", + ResourceModifierRules: []ResourceModifierRule{ + { + Conditions: Conditions{ + GroupResource: "deployments.apps", + Namespaces: []string{"foo"}, + }, + Patches: []JSONPatch{ + { + Operation: "test", + Path: "/spec/replicas", + Value: "1", + }, + { + Operation: "replace", + Path: "/spec/replicas", + Value: "2", + }, + }, + }, + }, + }, + args: args{ + obj: deployNginxOneReplica.DeepCopy(), + groupResource: "deployments.apps", + }, + wantErr: false, + wantObj: deployNginxTwoReplica.DeepCopy(), + }, { name: "nginx deployment: Empty Resource Regex and namespaces list", fields: fields{ @@ -704,7 +894,7 @@ func TestResourceModifiers_ApplyResourceModifierRules(t *testing.T) { Version: tt.fields.Version, ResourceModifierRules: tt.fields.ResourceModifierRules, } - got := p.ApplyResourceModifierRules(tt.args.obj, tt.args.groupResource, logrus.New()) + got := p.ApplyResourceModifierRules(tt.args.obj, tt.args.groupResource, nil, logrus.New()) assert.Equal(t, tt.wantErr, len(got) > 0) assert.Equal(t, *tt.wantObj, *tt.args.obj) @@ -712,6 +902,633 @@ func TestResourceModifiers_ApplyResourceModifierRules(t *testing.T) { } } +var podYAMLWithNginxImage = ` +apiVersion: v1 +kind: Pod +metadata: + name: pod1 + namespace: fake +spec: + containers: + - image: nginx + name: nginx +` + +var podYAMLWithNginx1Image = ` +apiVersion: v1 +kind: Pod +metadata: + name: pod1 + namespace: fake +spec: + containers: + - image: nginx1 + name: nginx +` + +var podYAMLWithNFSVolume = ` +apiVersion: v1 +kind: Pod +metadata: + name: pod1 + namespace: fake +spec: + containers: + - image: fake + name: fake + volumeMounts: + - mountPath: /fake1 + name: vol1 + - mountPath: /fake2 + name: vol2 + volumes: + - name: vol1 + nfs: + path: /fake2 + - name: vol2 + emptyDir: {} +` + +var podYAMLWithPVCVolume = ` +apiVersion: v1 +kind: Pod +metadata: + name: pod1 + namespace: fake +spec: + containers: + - image: fake + name: fake + volumeMounts: + - mountPath: /fake1 + name: vol1 + - mountPath: /fake2 + name: vol2 + volumes: + - name: vol1 + persistentVolumeClaim: + claimName: pvc1 + - name: vol2 + emptyDir: {} +` + +var svcYAMLWithPort8000 = ` +apiVersion: v1 +kind: Service +metadata: + name: svc1 + namespace: fake +spec: + ports: + - name: fake1 + port: 8001 + protocol: TCP + targetPort: 8001 + - name: fake + port: 8000 + protocol: TCP + targetPort: 8000 + - name: fake2 + port: 8002 + protocol: TCP + targetPort: 8002 +` + +var svcYAMLWithPort9000 = ` +apiVersion: v1 +kind: Service +metadata: + name: svc1 + namespace: fake +spec: + ports: + - name: fake1 + port: 8001 + protocol: TCP + targetPort: 8001 + - name: fake + port: 9000 + protocol: TCP + targetPort: 9000 + - name: fake2 + port: 8002 + protocol: TCP + targetPort: 8002 +` + +var cmYAMLWithLabelAToB = ` +apiVersion: v1 +kind: ConfigMap +metadata: + name: cm1 + namespace: fake + labels: + a: b + c: d +` + +var cmYAMLWithLabelAToC = ` +apiVersion: v1 +kind: ConfigMap +metadata: + name: cm1 + namespace: fake + labels: + a: c + c: d +` + +var cmYAMLWithoutLabelA = ` +apiVersion: v1 +kind: ConfigMap +metadata: + name: cm1 + namespace: fake + labels: + c: d +` + +func TestResourceModifiers_ApplyResourceModifierRules_StrategicMergePatch(t *testing.T) { + scheme := runtime.NewScheme() + utilruntime.Must(clientgoscheme.AddToScheme(scheme)) + unstructuredSerializer := yaml.NewDecodingSerializer(unstructured.UnstructuredJSONScheme) + o1, _, err := unstructuredSerializer.Decode([]byte(podYAMLWithNFSVolume), nil, nil) + assert.NoError(t, err) + podWithNFSVolume := o1.(*unstructured.Unstructured) + + o2, _, err := unstructuredSerializer.Decode([]byte(podYAMLWithPVCVolume), nil, nil) + assert.NoError(t, err) + podWithPVCVolume := o2.(*unstructured.Unstructured) + + o3, _, err := unstructuredSerializer.Decode([]byte(svcYAMLWithPort8000), nil, nil) + assert.NoError(t, err) + svcWithPort8000 := o3.(*unstructured.Unstructured) + + o4, _, err := unstructuredSerializer.Decode([]byte(svcYAMLWithPort9000), nil, nil) + assert.NoError(t, err) + svcWithPort9000 := o4.(*unstructured.Unstructured) + + o5, _, err := unstructuredSerializer.Decode([]byte(podYAMLWithNginxImage), nil, nil) + assert.NoError(t, err) + podWithNginxImage := o5.(*unstructured.Unstructured) + + o6, _, err := unstructuredSerializer.Decode([]byte(podYAMLWithNginx1Image), nil, nil) + assert.NoError(t, err) + podWithNginx1Image := o6.(*unstructured.Unstructured) + + tests := []struct { + name string + rm *ResourceModifiers + obj *unstructured.Unstructured + groupResource string + wantErr bool + wantObj *unstructured.Unstructured + }{ + { + name: "update image", + rm: &ResourceModifiers{ + Version: "v1", + ResourceModifierRules: []ResourceModifierRule{ + { + Conditions: Conditions{ + GroupResource: "pods", + Namespaces: []string{"fake"}, + }, + StrategicPatches: []StrategicMergePatch{ + { + PatchData: `{"spec":{"containers":[{"name":"nginx","image":"nginx1"}]}}`, + }, + }, + }, + }, + }, + obj: podWithNginxImage.DeepCopy(), + groupResource: "pods", + wantErr: false, + wantObj: podWithNginx1Image.DeepCopy(), + }, + { + name: "update image with yaml format", + rm: &ResourceModifiers{ + Version: "v1", + ResourceModifierRules: []ResourceModifierRule{ + { + Conditions: Conditions{ + GroupResource: "pods", + Namespaces: []string{"fake"}, + }, + StrategicPatches: []StrategicMergePatch{ + { + PatchData: `spec: + containers: + - name: nginx + image: nginx1`, + }, + }, + }, + }, + }, + obj: podWithNginxImage.DeepCopy(), + groupResource: "pods", + wantErr: false, + wantObj: podWithNginx1Image.DeepCopy(), + }, + { + name: "replace nfs with pvc in volume", + rm: &ResourceModifiers{ + Version: "v1", + ResourceModifierRules: []ResourceModifierRule{ + { + Conditions: Conditions{ + GroupResource: "pods", + Namespaces: []string{"fake"}, + }, + StrategicPatches: []StrategicMergePatch{ + { + PatchData: `{"spec":{"volumes":[{"nfs":null,"name":"vol1","persistentVolumeClaim":{"claimName":"pvc1"}}]}}`, + }, + }, + }, + }, + }, + obj: podWithNFSVolume.DeepCopy(), + groupResource: "pods", + wantErr: false, + wantObj: podWithPVCVolume.DeepCopy(), + }, + { + name: "replace any other volume source with pvc in volume", + rm: &ResourceModifiers{ + Version: "v1", + ResourceModifierRules: []ResourceModifierRule{ + { + Conditions: Conditions{ + GroupResource: "pods", + Namespaces: []string{"fake"}, + }, + StrategicPatches: []StrategicMergePatch{ + { + PatchData: `{"spec":{"volumes":[{"$retainKeys":["name","persistentVolumeClaim"],"name":"vol1","persistentVolumeClaim":{"claimName":"pvc1"}}]}}`, + }, + }, + }, + }, + }, + obj: podWithNFSVolume.DeepCopy(), + groupResource: "pods", + wantErr: false, + wantObj: podWithPVCVolume.DeepCopy(), + }, + { + name: "update a service port", + rm: &ResourceModifiers{ + Version: "v1", + ResourceModifierRules: []ResourceModifierRule{ + { + Conditions: Conditions{ + GroupResource: "services", + Namespaces: []string{"fake"}, + }, + StrategicPatches: []StrategicMergePatch{ + { + PatchData: `{"spec":{"$setElementOrder/ports":[{"port":8001},{"port":9000},{"port":8002}],"ports":[{"name":"fake","port":9000,"protocol":"TCP","targetPort":9000},{"$patch":"delete","port":8000}]}}`, + }, + }, + }, + }, + }, + obj: svcWithPort8000.DeepCopy(), + groupResource: "services", + wantErr: false, + wantObj: svcWithPort9000.DeepCopy(), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := tt.rm.ApplyResourceModifierRules(tt.obj, tt.groupResource, scheme, logrus.New()) + + assert.Equal(t, tt.wantErr, len(got) > 0) + assert.Equal(t, *tt.wantObj, *tt.obj) + }) + } +} + +func TestResourceModifiers_ApplyResourceModifierRules_JSONMergePatch(t *testing.T) { + unstructuredSerializer := yaml.NewDecodingSerializer(unstructured.UnstructuredJSONScheme) + o1, _, err := unstructuredSerializer.Decode([]byte(cmYAMLWithLabelAToB), nil, nil) + assert.NoError(t, err) + cmWithLabelAToB := o1.(*unstructured.Unstructured) + + o2, _, err := unstructuredSerializer.Decode([]byte(cmYAMLWithLabelAToC), nil, nil) + assert.NoError(t, err) + cmWithLabelAToC := o2.(*unstructured.Unstructured) + + o3, _, err := unstructuredSerializer.Decode([]byte(cmYAMLWithoutLabelA), nil, nil) + assert.NoError(t, err) + cmWithoutLabelA := o3.(*unstructured.Unstructured) + + tests := []struct { + name string + rm *ResourceModifiers + obj *unstructured.Unstructured + groupResource string + wantErr bool + wantObj *unstructured.Unstructured + }{ + { + name: "update labels", + rm: &ResourceModifiers{ + Version: "v1", + ResourceModifierRules: []ResourceModifierRule{ + { + Conditions: Conditions{ + GroupResource: "configmaps", + Namespaces: []string{"fake"}, + }, + MergePatches: []JSONMergePatch{ + { + PatchData: `{"metadata":{"labels":{"a":"c"}}}`, + }, + }, + }, + }, + }, + obj: cmWithLabelAToB.DeepCopy(), + groupResource: "configmaps", + wantErr: false, + wantObj: cmWithLabelAToC.DeepCopy(), + }, + { + name: "update labels in yaml format", + rm: &ResourceModifiers{ + Version: "v1", + ResourceModifierRules: []ResourceModifierRule{ + { + Conditions: Conditions{ + GroupResource: "configmaps", + Namespaces: []string{"fake"}, + }, + MergePatches: []JSONMergePatch{ + { + PatchData: `metadata: + labels: + a: c`, + }, + }, + }, + }, + }, + obj: cmWithLabelAToB.DeepCopy(), + groupResource: "configmaps", + wantErr: false, + wantObj: cmWithLabelAToC.DeepCopy(), + }, + { + name: "delete labels", + rm: &ResourceModifiers{ + Version: "v1", + ResourceModifierRules: []ResourceModifierRule{ + { + Conditions: Conditions{ + GroupResource: "configmaps", + Namespaces: []string{"fake"}, + }, + MergePatches: []JSONMergePatch{ + { + PatchData: `{"metadata":{"labels":{"a":null}}}`, + }, + }, + }, + }, + }, + obj: cmWithLabelAToB.DeepCopy(), + groupResource: "configmaps", + wantErr: false, + wantObj: cmWithoutLabelA.DeepCopy(), + }, + { + name: "add labels", + rm: &ResourceModifiers{ + Version: "v1", + ResourceModifierRules: []ResourceModifierRule{ + { + Conditions: Conditions{ + GroupResource: "configmaps", + Namespaces: []string{"fake"}, + }, + MergePatches: []JSONMergePatch{ + { + PatchData: `{"metadata":{"labels":{"a":"b"}}}`, + }, + }, + }, + }, + }, + obj: cmWithoutLabelA.DeepCopy(), + groupResource: "configmaps", + wantErr: false, + wantObj: cmWithLabelAToB.DeepCopy(), + }, + { + name: "delete non-existing labels", + rm: &ResourceModifiers{ + Version: "v1", + ResourceModifierRules: []ResourceModifierRule{ + { + Conditions: Conditions{ + GroupResource: "configmaps", + Namespaces: []string{"fake"}, + }, + MergePatches: []JSONMergePatch{ + { + PatchData: `{"metadata":{"labels":{"a":null}}}`, + }, + }, + }, + }, + }, + obj: cmWithoutLabelA.DeepCopy(), + groupResource: "configmaps", + wantErr: false, + wantObj: cmWithoutLabelA.DeepCopy(), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := tt.rm.ApplyResourceModifierRules(tt.obj, tt.groupResource, nil, logrus.New()) + + assert.Equal(t, tt.wantErr, len(got) > 0) + assert.Equal(t, *tt.wantObj, *tt.obj) + }) + } +} + +func TestResourceModifiers_wildcard_in_GroupResource(t *testing.T) { + unstructuredSerializer := yaml.NewDecodingSerializer(unstructured.UnstructuredJSONScheme) + o1, _, err := unstructuredSerializer.Decode([]byte(cmYAMLWithLabelAToB), nil, nil) + assert.NoError(t, err) + cmWithLabelAToB := o1.(*unstructured.Unstructured) + + o2, _, err := unstructuredSerializer.Decode([]byte(cmYAMLWithLabelAToC), nil, nil) + assert.NoError(t, err) + cmWithLabelAToC := o2.(*unstructured.Unstructured) + + tests := []struct { + name string + rm *ResourceModifiers + obj *unstructured.Unstructured + groupResource string + wantErr bool + wantObj *unstructured.Unstructured + }{ + { + name: "match all groups and resources", + rm: &ResourceModifiers{ + Version: "v1", + ResourceModifierRules: []ResourceModifierRule{ + { + Conditions: Conditions{ + GroupResource: "*", + Namespaces: []string{"fake"}, + }, + MergePatches: []JSONMergePatch{ + { + PatchData: `{"metadata":{"labels":{"a":"c"}}}`, + }, + }, + }, + }, + }, + obj: cmWithLabelAToB.DeepCopy(), + groupResource: "configmaps", + wantErr: false, + wantObj: cmWithLabelAToC.DeepCopy(), + }, + { + name: "match all resources in group apps", + rm: &ResourceModifiers{ + Version: "v1", + ResourceModifierRules: []ResourceModifierRule{ + { + Conditions: Conditions{ + GroupResource: "*.apps", + Namespaces: []string{"fake"}, + }, + MergePatches: []JSONMergePatch{ + { + PatchData: `{"metadata":{"labels":{"a":"c"}}}`, + }, + }, + }, + }, + }, + obj: cmWithLabelAToB.DeepCopy(), + groupResource: "fake.apps", + wantErr: false, + wantObj: cmWithLabelAToC.DeepCopy(), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := tt.rm.ApplyResourceModifierRules(tt.obj, tt.groupResource, nil, logrus.New()) + + assert.Equal(t, tt.wantErr, len(got) > 0) + assert.Equal(t, *tt.wantObj, *tt.obj) + }) + } +} + +func TestResourceModifiers_conditional_patches(t *testing.T) { + unstructuredSerializer := yaml.NewDecodingSerializer(unstructured.UnstructuredJSONScheme) + o1, _, err := unstructuredSerializer.Decode([]byte(cmYAMLWithLabelAToB), nil, nil) + assert.NoError(t, err) + cmWithLabelAToB := o1.(*unstructured.Unstructured) + + o2, _, err := unstructuredSerializer.Decode([]byte(cmYAMLWithLabelAToC), nil, nil) + assert.NoError(t, err) + cmWithLabelAToC := o2.(*unstructured.Unstructured) + + tests := []struct { + name string + rm *ResourceModifiers + obj *unstructured.Unstructured + groupResource string + wantErr bool + wantObj *unstructured.Unstructured + }{ + { + name: "match conditions and apply patches", + rm: &ResourceModifiers{ + Version: "v1", + ResourceModifierRules: []ResourceModifierRule{ + { + Conditions: Conditions{ + GroupResource: "*", + Namespaces: []string{"fake"}, + Matches: []MatchRule{ + { + Path: "/metadata/labels/a", + Value: "b", + }, + }, + }, + MergePatches: []JSONMergePatch{ + { + PatchData: `{"metadata":{"labels":{"a":"c"}}}`, + }, + }, + }, + }, + }, + obj: cmWithLabelAToB.DeepCopy(), + groupResource: "configmaps", + wantErr: false, + wantObj: cmWithLabelAToC.DeepCopy(), + }, + { + name: "mismatch conditions and skip patches", + rm: &ResourceModifiers{ + Version: "v1", + ResourceModifierRules: []ResourceModifierRule{ + { + Conditions: Conditions{ + GroupResource: "*", + Namespaces: []string{"fake"}, + Matches: []MatchRule{ + { + Path: "/metadata/labels/a", + Value: "c", + }, + }, + }, + MergePatches: []JSONMergePatch{ + { + PatchData: `{"metadata":{"labels":{"a":"c"}}}`, + }, + }, + }, + }, + }, + obj: cmWithLabelAToB.DeepCopy(), + groupResource: "configmaps", + wantErr: false, + wantObj: cmWithLabelAToB.DeepCopy(), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := tt.rm.ApplyResourceModifierRules(tt.obj, tt.groupResource, nil, logrus.New()) + + assert.Equal(t, tt.wantErr, len(got) > 0) + assert.Equal(t, *tt.wantObj, *tt.obj) + }) + } +} + func TestJSONPatch_ToString(t *testing.T) { type fields struct { Operation string diff --git a/internal/resourcemodifiers/resource_modifiers_validator.go b/internal/resourcemodifiers/resource_modifiers_validator.go index 24fe0dbb2e..9ad817ecad 100644 --- a/internal/resourcemodifiers/resource_modifiers_validator.go +++ b/internal/resourcemodifiers/resource_modifiers_validator.go @@ -9,6 +9,21 @@ func (r *ResourceModifierRule) Validate() error { if err := r.Conditions.Validate(); err != nil { return err } + + count := 0 + for _, size := range []int{ + len(r.Patches), + len(r.MergePatches), + len(r.StrategicPatches), + } { + if size != 0 { + count++ + } + if count >= 2 { + return fmt.Errorf("only one of patches, mergePatches, strategicPatches can be specified") + } + } + for _, patch := range r.Patches { if err := patch.Validate(); err != nil { return err diff --git a/internal/resourcemodifiers/resource_modifiers_validator_test.go b/internal/resourcemodifiers/resource_modifiers_validator_test.go index a46c465307..accd320a00 100644 --- a/internal/resourcemodifiers/resource_modifiers_validator_test.go +++ b/internal/resourcemodifiers/resource_modifiers_validator_test.go @@ -114,6 +114,32 @@ func TestResourceModifiers_Validate(t *testing.T) { }, wantErr: true, }, + { + name: "More than one patch type in a rule", + fields: fields{ + Version: "v1", + ResourceModifierRules: []ResourceModifierRule{ + { + Conditions: Conditions{ + GroupResource: "*", + }, + Patches: []JSONPatch{ + { + Operation: "test", + Path: "/spec/storageClassName", + Value: "premium", + }, + }, + MergePatches: []JSONMergePatch{ + { + PatchData: `{"metadata":{"labels":{"a":null}}}`, + }, + }, + }, + }, + }, + wantErr: true, + }, } for _, tt := range tests { diff --git a/internal/resourcemodifiers/strategic_merge_patch.go b/internal/resourcemodifiers/strategic_merge_patch.go new file mode 100644 index 0000000000..8d3fe4c511 --- /dev/null +++ b/internal/resourcemodifiers/strategic_merge_patch.go @@ -0,0 +1,143 @@ +package resourcemodifiers + +import ( + "fmt" + "net/http" + + "github.com/sirupsen/logrus" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/util/mergepatch" + "k8s.io/apimachinery/pkg/util/strategicpatch" + "k8s.io/apimachinery/pkg/util/validation/field" + kubejson "sigs.k8s.io/json" + "sigs.k8s.io/yaml" +) + +type StrategicMergePatch struct { + PatchData string `json:"patchData,omitempty"` +} + +type StrategicMergePatcher struct { + patches []StrategicMergePatch + scheme *runtime.Scheme +} + +func (p *StrategicMergePatcher) Patch(u *unstructured.Unstructured, _ logrus.FieldLogger) (*unstructured.Unstructured, error) { + gvk := u.GetObjectKind().GroupVersionKind() + schemaReferenceObj, err := p.scheme.New(gvk) + if err != nil { + return nil, err + } + + origin := u.DeepCopy() + updated := u.DeepCopy() + for _, patch := range p.patches { + patchBytes, err := yaml.YAMLToJSON([]byte(patch.PatchData)) + if err != nil { + return nil, fmt.Errorf("error in converting YAML to JSON %s", err) + } + + err = strategicPatchObject(origin, patchBytes, updated, schemaReferenceObj) + if err != nil { + return nil, fmt.Errorf("error in applying Strategic Patch %s", err.Error()) + } + + origin = updated.DeepCopy() + } + + return updated, nil +} + +// strategicPatchObject applies a strategic merge patch of `patchBytes` to +// `originalObject` and stores the result in `objToUpdate`. +// It additionally returns the map[string]interface{} representation of the +// `originalObject` and `patchBytes`. +// NOTE: Both `originalObject` and `objToUpdate` are supposed to be versioned. +func strategicPatchObject( + originalObject runtime.Object, + patchBytes []byte, + objToUpdate runtime.Object, + schemaReferenceObj runtime.Object, +) error { + originalObjMap, err := runtime.DefaultUnstructuredConverter.ToUnstructured(originalObject) + if err != nil { + return err + } + + patchMap := make(map[string]interface{}) + var strictErrs []error + strictErrs, err = kubejson.UnmarshalStrict(patchBytes, &patchMap) + if err != nil { + return apierrors.NewBadRequest(err.Error()) + } + + if err := applyPatchToObject(originalObjMap, patchMap, objToUpdate, schemaReferenceObj, strictErrs); err != nil { + return err + } + return nil +} + +// applyPatchToObject applies a strategic merge patch of to +// and stores the result in . +// NOTE: must be a versioned object. +func applyPatchToObject( + originalMap map[string]interface{}, + patchMap map[string]interface{}, + objToUpdate runtime.Object, + schemaReferenceObj runtime.Object, + strictErrs []error, +) error { + patchedObjMap, err := strategicpatch.StrategicMergeMapPatch(originalMap, patchMap, schemaReferenceObj) + if err != nil { + return interpretStrategicMergePatchError(err) + } + + // Rather than serialize the patched map to JSON, then decode it to an object, we go directly from a map to an object + converter := runtime.DefaultUnstructuredConverter + if err := converter.FromUnstructuredWithValidation(patchedObjMap, objToUpdate, true); err != nil { + strictError, isStrictError := runtime.AsStrictDecodingError(err) + switch { + case !isStrictError: + // disregard any sttrictErrs, because it's an incomplete + // list of strict errors given that we don't know what fields were + // unknown because StrategicMergeMapPatch failed. + // Non-strict errors trump in this case. + return apierrors.NewInvalid(schema.GroupKind{}, "", field.ErrorList{ + field.Invalid(field.NewPath("patch"), fmt.Sprintf("%+v", patchMap), err.Error()), + }) + //case validationDirective == metav1.FieldValidationWarn: + // addStrictDecodingWarnings(requestContext, append(strictErrs, strictError.Errors()...)) + default: + strictDecodingError := runtime.NewStrictDecodingError(append(strictErrs, strictError.Errors()...)) + return apierrors.NewInvalid(schema.GroupKind{}, "", field.ErrorList{ + field.Invalid(field.NewPath("patch"), fmt.Sprintf("%+v", patchMap), strictDecodingError.Error()), + }) + } + } else if len(strictErrs) > 0 { + switch { + //case validationDirective == metav1.FieldValidationWarn: + // addStrictDecodingWarnings(requestContext, strictErrs) + default: + return apierrors.NewInvalid(schema.GroupKind{}, "", field.ErrorList{ + field.Invalid(field.NewPath("patch"), fmt.Sprintf("%+v", patchMap), runtime.NewStrictDecodingError(strictErrs).Error()), + }) + } + } + + return nil +} + +// interpretStrategicMergePatchError interprets the error type and returns an error with appropriate HTTP code. +func interpretStrategicMergePatchError(err error) error { + switch err { + case mergepatch.ErrBadJSONDoc, mergepatch.ErrBadPatchFormatForPrimitiveList, mergepatch.ErrBadPatchFormatForRetainKeys, mergepatch.ErrBadPatchFormatForSetElementOrderList, mergepatch.ErrUnsupportedStrategicMergePatchFormat: + return apierrors.NewBadRequest(err.Error()) + case mergepatch.ErrNoListOfLists, mergepatch.ErrPatchContentNotMatchRetainKeys: + return apierrors.NewGenericServerResponse(http.StatusUnprocessableEntity, "", schema.GroupResource{}, "", err.Error(), 0, false) + default: + return err + } +} diff --git a/internal/resourcemodifiers/strategic_merge_patch_test.go b/internal/resourcemodifiers/strategic_merge_patch_test.go new file mode 100644 index 0000000000..6c3c700f54 --- /dev/null +++ b/internal/resourcemodifiers/strategic_merge_patch_test.go @@ -0,0 +1,52 @@ +package resourcemodifiers + +import ( + "testing" + + "github.com/sirupsen/logrus" + "github.com/stretchr/testify/assert" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + clientgoscheme "k8s.io/client-go/kubernetes/scheme" +) + +func TestStrategicMergePatchFailure(t *testing.T) { + tests := []struct { + name string + data string + kind string + }{ + { + name: "patch with unknown kind", + data: "{}", + kind: "BadKind", + }, + { + name: "patch with bad yaml", + data: "a: b:", + kind: "Pod", + }, + { + name: "patch with bad json", + data: `{"a"::1}`, + kind: "Pod", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + scheme := runtime.NewScheme() + err := clientgoscheme.AddToScheme(scheme) + assert.NoError(t, err) + pt := &StrategicMergePatcher{ + patches: []StrategicMergePatch{{PatchData: tt.data}}, + scheme: scheme, + } + + u := &unstructured.Unstructured{} + u.SetGroupVersionKind(schema.GroupVersionKind{Version: "v1", Kind: tt.kind}) + _, err = pt.Patch(u, logrus.New()) + assert.Error(t, err) + }) + } +} diff --git a/pkg/restore/restore.go b/pkg/restore/restore.go index 8355533300..bed2e56e95 100644 --- a/pkg/restore/restore.go +++ b/pkg/restore/restore.go @@ -1466,7 +1466,7 @@ func (ctx *restoreContext) restoreItem(obj *unstructured.Unstructured, groupReso } if ctx.resourceModifiers != nil { - if errList := ctx.resourceModifiers.ApplyResourceModifierRules(obj, groupResource.String(), ctx.log); errList != nil { + if errList := ctx.resourceModifiers.ApplyResourceModifierRules(obj, groupResource.String(), ctx.kbClient.Scheme(), ctx.log); errList != nil { for _, err := range errList { errs.Add(namespace, err) } diff --git a/site/content/docs/main/restore-resource-modifiers.md b/site/content/docs/main/restore-resource-modifiers.md index 0d59fad5a9..0c1f2f2172 100644 --- a/site/content/docs/main/restore-resource-modifiers.md +++ b/site/content/docs/main/restore-resource-modifiers.md @@ -105,3 +105,83 @@ resourceModifierRules: - Update a container's image using a json patch with positional arrays kubectl patch pod valid-pod -type='json' -p='[{"op": "replace", "path": "/spec/containers/0/image", "value":"new image"}]' - Before creating the resource modifier yaml, you can try it out using kubectl patch command. The same commands should work as it is. + +#### JSON Merge Patch +You can modify a resource using JSON Merge Patch +```yaml +version: v1 +resourceModifierRules: +- conditions: + groupResource: pods + namespaces: + - ns1 + mergePatches: + - patchData: | + { + "metadata": { + "annotations": { + "foo": null + } + } + } +``` +- The above configmap will apply the Merge Patch to all the pods in namespace ns1 and remove the annotation `foo` from the pods. +- Both json and yaml format are supported for the patchData. +- For more details, please refer to [this doc](https://kubernetes.io/docs/tasks/manage-kubernetes-objects/update-api-object-kubectl-patch/) + +#### Strategic Merge Patch +You can modify a resource using Strategic Merge Patch +```yaml +version: v1 +resourceModifierRules: +- conditions: + groupResource: pods + resourceNameRegex: "^my-pod$" + namespaces: + - ns1 + strategicPatches: + - patchData: | + { + "spec": { + "containers": [ + { + "name": "nginx", + "image": "repo2/nginx" + } + ] + } + } +``` +- The above configmap will apply the Strategic Merge Patch to the pod with name my-pod in namespace ns1 and update the image of container nginx to `repo2/nginx`. +- Both json and yaml format are supported for the patchData. +- For more details, please refer to [this doc](https://kubernetes.io/docs/tasks/manage-kubernetes-objects/update-api-object-kubectl-patch/) + + +### Conditional Patches in ALL Patch Types +A new field `matches` is added in conditions to support conditional patches. + +Example of matches in conditions +```yaml +version: v1 +resourceModifierRules: +- conditions: + groupResource: persistentvolumeclaims.storage.k8s.io + matches: + - path: "/spec/storageClassName" + value: "premium" + mergePatches: + - patchData: | + { + "metadata": { + "annotations": { + "foo": null + } + } + } +``` +- The above configmap will apply the Merge Patch to all the PVCs in all namespaces with storageClassName premium and remove the annotation `foo` from the PVCs. +- You can specify multiple rules in the `matches` list. The patch will be applied only if all the matches are satisfied. + +### Wildcard Support for GroupResource +The user can specify a wildcard for groupResource in the conditions' struct. This will allow the user to apply the patches for all the resources of a particular group or all resources in all groups. For example, `*.apps` will apply to all the resources in the `apps` group, `*` will apply to all the resources in core group, `*.*` will apply to all the resources in all groups. +- If both `*.groupName` and `namespaces` are specified, the patches will be applied to all the namespaced resources in this group in the specified namespaces and all the cluster resources in this group. \ No newline at end of file