Skip to content

Commit

Permalink
Merge pull request #7049 from 27149chen/cherry-pick-6917
Browse files Browse the repository at this point in the history
Cherry-pick #6917 to release 1.12
  • Loading branch information
Lyndon-Li authored Nov 17, 2023
2 parents 3325a0c + 5c44ed4 commit c042d47
Show file tree
Hide file tree
Showing 15 changed files with 1,588 additions and 74 deletions.
1 change: 1 addition & 0 deletions changelogs/unreleased/7049-27149chen
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Cherry-pick #6917 - Support JSON Merge Patch and Strategic Merge Patch in Resource Modifiers
193 changes: 193 additions & 0 deletions design/merge-patch-and-strategic-in-resource-modifier.md
Original file line number Diff line number Diff line change
@@ -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
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
)

Expand Down Expand Up @@ -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
)

Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down
45 changes: 45 additions & 0 deletions internal/resourcemodifiers/json_merge_patch.go
Original file line number Diff line number Diff line change
@@ -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
}
41 changes: 41 additions & 0 deletions internal/resourcemodifiers/json_merge_patch_test.go
Original file line number Diff line number Diff line change
@@ -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)
})
}
}
Loading

0 comments on commit c042d47

Please sign in to comment.