Merge branch 'main' into resourcemodifier

Signed-off-by: Anshul Ahuja <anshul.ahu@gmail.com>

.github/workflows/push.yml

@@ -60,6 +60,13 @@ jobs:
         files: coverage.out
         verbose: true
 
+    # Use the JSON key in secret to login gcr.io
+    - uses: 'docker/login-action@v2'
+      with:
+        registry: 'gcr.io' # or REGION.docker.pkg.dev
+        username: '_json_key'
+        password: '${{ secrets.GCR_SA_KEY }}'
+
     # Only try to publish the container image from the root repo; forks don't have permission to do so and will always get failures.
     - name: Publish container image
       if: github.repository == 'vmware-tanzu/velero'
@@ -91,19 +98,3 @@ jobs:
         uploader ${VELERO_RESTORE_HELPER_IMAGE_FILE} ${GCS_BUCKET}
         uploader ${VELERO_IMAGE_BACKUP_FILE} ${GCS_BUCKET}
         uploader ${VELERO_RESTORE_HELPER_IMAGE_BACKUP_FILE} ${GCS_BUCKET}
-
-    # Use the JSON key in secret to login gcr.io
-    - uses: 'docker/login-action@v1'
-      with:
-        registry: 'gcr.io' # or REGION.docker.pkg.dev
-        username: '_json_key'
-        password: '${{ secrets.GCR_SA_KEY }}'
-
-    # Push image to GCR to facilitate some environments that have rate limitation to docker hub, e.g. vSphere.
-    - name: Publish container image to GCR
-      if: github.repository == 'vmware-tanzu/velero'
-      run: |
-        sudo swapoff -a
-        sudo rm -f /mnt/swapfile
-        docker image prune -a --force
-        REGISTRY=gcr.io/velero-gcp ./hack/docker-push.sh

Makefile

@@ -22,9 +22,11 @@ PKG := github.com/vmware-tanzu/velero
 
 # Where to push the docker image.
 REGISTRY ?= velero
+GCR_REGISTRY ?= gcr.io/velero-gcp
 
 # Image name
 IMAGE ?= $(REGISTRY)/$(BIN)
+GCR_IMAGE ?= $(GCR_REGISTRY)/$(BIN)
 
 # We allow the Dockerfile to be configurable to enable the use of custom Dockerfiles
 # that pull base images from different registries.
@@ -66,8 +68,10 @@ TAG_LATEST ?= false
 
 ifeq ($(TAG_LATEST), true)
 	IMAGE_TAGS ?= $(IMAGE):$(VERSION) $(IMAGE):latest
+	GCR_IMAGE_TAGS ?= $(GCR_IMAGE):$(VERSION) $(GCR_IMAGE):latest
 else
 	IMAGE_TAGS ?= $(IMAGE):$(VERSION)
+	GCR_IMAGE_TAGS ?= $(GCR_IMAGE):$(VERSION)
 endif
 
 ifeq ($(shell docker buildx inspect 2>/dev/null | awk '/Status/ { print $$2 }'), running)
@@ -183,6 +187,7 @@ endif
 	--output=type=$(BUILDX_OUTPUT_TYPE) \
 	--platform $(BUILDX_PLATFORMS) \
 	$(addprefix -t , $(IMAGE_TAGS)) \
+	$(addprefix -t , $(GCR_IMAGE_TAGS)) \
 	--build-arg=GOPROXY=$(GOPROXY) \
 	--build-arg=PKG=$(PKG) \
 	--build-arg=BIN=$(BIN) \

changelogs/unreleased/6469-blackpiglet

@@ -0,0 +1 @@
+Remove dependency of the legacy client code from pkg/cmd directory

changelogs/unreleased/6484-kaovilai

@@ -0,0 +1 @@
+Velero Plugins no longer need kopia indirect dependency in their go.mod

changelogs/unreleased/6491-Lyndon-Li

@@ -0,0 +1 @@
+Fix issue 6490, If a backup/restore has multiple async operations and one operation fails while others are still in-progress, when all the operations finish, the backup/restore will be set as Completed falsely

changelogs/unreleased/6493-allenxu404

@@ -0,0 +1 @@
+Add data upload and download metrics

changelogs/unreleased/6497-blackpiglet

@@ -0,0 +1 @@
+Remove dependency of the legacy client code from pkg/cmd directory part 2

design/json-substitution-action-design.md

@@ -0,0 +1,160 @@
+# Proposal to add support for Resource Modifiers (AKA JSON Substitutions) in Restore Workflow
+
+- [Proposal to add support for Resource Modifiers (AKA JSON Substitutions) in Restore Workflow](#proposal-to-add-support-for-resource-modifiers-aka-json-substitutions-in-restore-workflow)
+    - [Abstract](#abstract)
+    - [Goals](#goals)
+    - [Non Goals](#non-goals)
+    - [User Stories](#user-stories)
+        - [Scenario 1](#scenario-1)
+        - [Scenario 2](#scenario-2)
+    - [Detailed Design](#detailed-design)
+        - [Reference in velero API](#reference-in-velero-api)
+        - [ConfigMap Structure](#configmap-structure)
+        - [Operations supported by the JSON Patch library:](#operations-supported-by-the-json-patch-library)
+        - [Advance scenarios](#advance-scenarios)
+            - [Conditional patches using test operation](#conditional-patches-using-test-operation)
+    - [Alternatives Considered](#alternatives-considered)
+    - [Security Considerations](#security-considerations)
+    - [Compatibility](#compatibility)
+    - [Implementation](#implementation)
+    - [Future Enhancements](#future-enhancements)
+    - [Open Issues](#open-issues)
+
+## Abstract
+Currently velero supports substituting certain values in the K8s resources during restoration like changing the namespace, changing the storage class, etc. This proposal is to add generic support for JSON substitutions in the restore workflow. This will allow the user specify filters for particular resources and then specify a JSON patch (operator, path, value) to apply on a resource. This will allow the user to substitute any value in the K8s resource without having to write a new RestoreItemAction plugin for each kind of substitution.
+
+<!-- ## Background -->
+
+## Goals
+- Allow the user to specify a GroupKind, Name(optional), JSON patch for modification.
+- Allow the user to specify multiple JSON patch.
+
+## 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 a PVC which is encrypted using a DES(Disk Encryption Set - Azure example) mentioned in the PVC YAML through the StorageClass YAML. 
+- Alice wishes to restore this snapshot to a different cluster. The new cluster does not have access to the same DES to provision disk's out of the snapshot.
+- She wishes to use a different DES for all the PVCs which use the certain DES.
+- She can use this feature to substitute the DES in all StorageClass YAMLs with the new DES without having to create a fresh storageclass, or understanding the name of the storageclass.
+
+### Scenario 2
+- Bob has multi zone cluster where nodes are spread across zones. 
+- Bob has pinned certain pods to a particular zone using nodeSelector/ nodeaffinity on the pod spec.
+- In case of zone outage of the cloudprovider, Bob wishes to restore the workload to a different namespace in the same cluster, but change the zone pinning of the workload.
+- Bob can use this feature to substitute the nodeSelector/ nodeaffinity in the pod spec with the new zone pinning to quickly failover the workload to a different zone's nodes.
+
+## Detailed Design
+- The design and approach is inspired from [kubectl patch command](https://github.com/kubernetes/kubectl/blob/0a61782351a027411b8b45b1443ec3dceddef421/pkg/cmd/patch/patch.go#L102C2-L104C1)
+```bash
+# 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"}]'
+```
+- The user is expected to create a configmap with the desired Resource Modifications. Then the reference of the configmap will be provided in the RestoreSpec.
+- The core restore workflow before creating/updating a particular resource in the cluster will be checked against the filters provided and respective substitutions will be applied on it.
+
+### Reference in velero API 
+> Example of Reference to configmap in RestoreSpec
+```yaml
+apiVersion: velero.io/v1
+kind: Restore
+metadata:
+  name: restore-1
+spec:
+    resourceModifier:
+      refType: Configmap
+      ref: resourcemodifierconfigmap
+```
+> Example CLI Command
+```bash
+velero restore create --from-backup backup-1 --resource-modifier-configmap resourcemodifierconfigmap
+```
+
+### Resource Modifier ConfigMap Structure
+- User first needs to provide details on which resources the JSON Substitutions need to be applied. 
+    - For this the user will provide 4 inputs - Namespaces(for NS Scoped resources), GroupKind (kind.group format similar to includeResources field in velero) and Name Regex(optional).
+    - If the user does not provide the Name, the JSON Substitutions will be applied to all the resources of the given Group and Kind under the given namespaces.
+
+- Further the use will specify the JSON Patch using the structure of kubectl's "JSON Patch" based inputs.
+- Sample data in ConfigMap
+```yaml
+version: v1
+resourceModifierRules:
+- conditions:
+    groupKind: persistentvolumeclaims
+    resourceNameRegex: "mysql.*"
+    namespaces:
+    - bar
+    - foo
+  patches:
+  - operation: replace
+    path: "/spec/storageClassName"
+    value: "premium"
+  - operation: remove
+    path: "/metadata/labels/test"
+```
+- The above configmap will apply the JSON Patch to all the PVCs in the namespaces bar and foo with name starting with mysql. The JSON Patch will replace the storageClassName with "premium" and remove the label "test" from the PVCs.
+- The user can specify multiple JSON Patches for a particular resource. The 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.
+- The user can specify multiple resourceModifierRules in the configmap. The rules will be applied in the order specified in the configmap. 
+
+> Users need to create one configmap in Velero install namespace from a YAML file that defined resource modifiers. The creating command would be like the below:
+```bash
+kubectl create cm <configmap-name> --from-file <yaml-file> -n velero
+```
+
+### Operations supported by the JSON Patch library: 
+- add
+- remove
+- replace
+- move
+- copy
+- test (covered below)
+
+### Advance scenarios
+#### **Conditional patches using test operation**
+ The `test` operation can be used to check if a particular value is present in the resource. If the value is present, the patch will be applied. If the value is not present, the patch will not be applied. This can be used to apply a patch only if a particular value is present in the resource. For example, if the user wishes to change the storage class of a PVC only if the PVC is using a particular storage class, the user can use the following configmap.
+```yaml
+version: v1
+resourceModifierRules:
+- conditions:
+    groupKind: persistentvolumeclaims.storage.k8s.io
+    resourceNameRegex: ".*"
+    namespaces:
+    - bar
+    - foo
+  patches:
+  - operation: test
+    path: "/spec/storageClassName"
+    value: "premium"
+  - operation: replace
+    path: "/spec/storageClassName"
+    value: "standard"
+```
+
+## Alternatives Considered
+1. JSON Path based addressal of json fields in the resource
+    - This was the initial planned approach, but there is no open source library which gives satisfactory edit functionality with support for all operators supported by the JsonPath RFC.
+    - We attempted modifying the  [https://kubernetes.io/docs/reference/kubectl/jsonpath/](https://kubernetes.io/docs/reference/kubectl/jsonpath/) but given the complexity of the code it did not make sense to change it since it would become a long term maintainability problem.
+1. RestoreItemAction for each kind of standard substitution
+    - Not an extensible design. If a new kind of substitution is required, a new RestoreItemAction needs to be written.
+1. RIA for JSON Substitution: The approach of doing JSON Substitution through a RestoreItemAction plugin was considered. But it is likely to have performance implications as the plugin will be invoked for all the resources.
+
+## Security Considerations
+No security impact.
+
+## Compatibility
+Compatibility with existing StorageClass mapping RestoreItemAction and similar plugins needs to be evaluated. 
+
+## Implementation
+- Changes in Restore CRD. Add a new field to the RestoreSpec to reference the configmap.
+- One example of where code will be modified:  https://github.com/vmware-tanzu/velero/blob/eeee4e06d209df7f08bfabda326b27aaf0054759/pkg/restore/restore.go#L1266 On the obj before Creation, we can apply the conditions to check if the resource is filtered out using given parameters. Then using JsonPatch provided, we can update the resource.
+- For Jsonpatch - https://github.com/evanphx/json-patch library is used.
+- JSON Patch RFC https://datatracker.ietf.org/doc/html/rfc6902
+
+## Future enhancements
+- Additional features such as wildcard support in path, regex match support in value, etc. can be added in future. This would involve forking the https://github.com/evanphx/json-patch library and adding the required features, since those features are not supported by the library currently and are not part of jsonpatch RFC.
+
+## Open Issues
+NA

go.mod

@@ -1,6 +1,6 @@
 module github.com/vmware-tanzu/velero
 
-go 1.18
+go 1.20
 
 require (
 	cloud.google.com/go/storage v1.30.1
@@ -35,6 +35,7 @@ require (
 	github.com/stretchr/testify v1.8.2
 	github.com/vmware-tanzu/crash-diagnostics v0.3.7
 	go.uber.org/zap v1.24.0
+	golang.org/x/exp v0.0.0-20221028150844-83b7d23a625f
 	golang.org/x/mod v0.10.0
 	golang.org/x/net v0.9.0
 	golang.org/x/oauth2 v0.7.0
@@ -139,7 +140,6 @@ require (
 	go.uber.org/atomic v1.9.0 // indirect
 	go.uber.org/multierr v1.11.0 // indirect
 	golang.org/x/crypto v0.8.0 // indirect
-	golang.org/x/exp v0.0.0-20221028150844-83b7d23a625f // indirect
 	golang.org/x/sync v0.1.0 // indirect
 	golang.org/x/sys v0.7.0 // indirect
 	golang.org/x/term v0.7.0 // indirect

pkg/builder/data_download_builder.go

@@ -110,3 +110,9 @@ func (d *DataDownloadBuilder) ObjectMeta(opts ...ObjectMetaOpt) *DataDownloadBui
 
 	return d
 }
+
+// StartTimestamp sets the DataDownload's StartTimestamp.
+func (d *DataDownloadBuilder) StartTimestamp(startTime *metav1.Time) *DataDownloadBuilder {
+	d.object.Status.StartTimestamp = startTime
+	return d
+}

pkg/builder/data_upload_builder.go

@@ -113,3 +113,9 @@ func (d *DataUploadBuilder) CSISnapshot(cSISnapshot *velerov2alpha1api.CSISnapsh
 	d.object.Spec.CSISnapshot = cSISnapshot
 	return d
 }
+
+// StartTimestamp sets the DataUpload's StartTimestamp.
+func (d *DataUploadBuilder) StartTimestamp(startTime *metav1.Time) *DataUploadBuilder {
+	d.object.Status.StartTimestamp = startTime
+	return d
+}

pkg/client/factory.go

@@ -55,6 +55,10 @@ type Factory interface {
 	// types to its scheme. It uses the following priority to specify the cluster
 	// configuration: --kubeconfig flag, KUBECONFIG environment variable, in-cluster configuration.
 	KubebuilderClient() (kbclient.Client, error)
+	// KubebuilderWatchClient returns a client with watcher for the controller runtime framework.
+	// It adds Kubernetes and Velero types to its scheme. It uses the following priority to specify the cluster
+	// configuration: --kubeconfig flag, KUBECONFIG environment variable, in-cluster configuration.
+	KubebuilderWatchClient() (kbclient.WithWatch, error)
 	// SetBasename changes the basename for an already-constructed client.
 	// This is useful for generating clients that require a different user-agent string below the root `velero`
 	// command, such as the server subcommand.
@@ -182,6 +186,39 @@ func (f *factory) KubebuilderClient() (kbclient.Client, error) {
 	return kubebuilderClient, nil
 }
 
+func (f *factory) KubebuilderWatchClient() (kbclient.WithWatch, error) {
+	clientConfig, err := f.ClientConfig()
+	if err != nil {
+		return nil, err
+	}
+
+	scheme := runtime.NewScheme()
+	if err := velerov1api.AddToScheme(scheme); err != nil {
+		return nil, err
+	}
+	if err := velerov2alpha1api.AddToScheme(scheme); err != nil {
+		return nil, err
+	}
+	if err := k8scheme.AddToScheme(scheme); err != nil {
+		return nil, err
+	}
+	if err := apiextv1beta1.AddToScheme(scheme); err != nil {
+		return nil, err
+	}
+	if err := apiextv1.AddToScheme(scheme); err != nil {
+		return nil, err
+	}
+	kubebuilderWatchClient, err := kbclient.NewWithWatch(clientConfig, kbclient.Options{
+		Scheme: scheme,
+	})
+
+	if err != nil {
+		return nil, err
+	}
+
+	return kubebuilderWatchClient, nil
+}
+
 func (f *factory) SetBasename(name string) {
 	f.baseName = name
 }

pkg/client/factory_test.go

@@ -141,6 +141,10 @@ func TestFactory(t *testing.T) {
 			kubebuilderClient, e := f.KubebuilderClient()
 			assert.Contains(t, e.Error(), fmt.Sprintf("Get \"%s/api?timeout=", test.expectedHost))
 			assert.Nil(t, kubebuilderClient)
+
+			kbClientWithWatch, e := f.KubebuilderWatchClient()
+			assert.Contains(t, e.Error(), fmt.Sprintf("Get \"%s/api?timeout=", test.expectedHost))
+			assert.Nil(t, kbClientWithWatch)
 		})
 	}
 }