From 8064421e83314c564aed4f804d3295f4ab68ee3f Mon Sep 17 00:00:00 2001 From: Xun Jiang Date: Mon, 11 Apr 2022 20:49:20 +0800 Subject: [PATCH] Update integrated Restic version and add insecureSkipTLSVerify for Restic CLI 1. Add --insecure-tls for ResticManager's commands. 2. Add --insecure-tls in PodVolumeBackup and PodVolumeRestore controller. 3. Upgrade integrated Restic version to v0.13.1 4. Change --last flag in Restic command to --latest=1 due to Restic version update. Signed-off-by: Xun Jiang --- Makefile | 2 +- changelogs/unreleased/4839-jxun | 1 + .../pod_volume_backup_controller.go | 25 +++- .../pod_volume_restore_controller.go | 8 ++ pkg/restic/command_factory.go | 3 +- pkg/restic/command_factory_test.go | 9 +- pkg/restic/exec_commands.go | 19 ++- pkg/restic/repository_manager.go | 43 ++++++- pkg/restic/repository_manager_test.go | 121 ++++++++++++++++++ 9 files changed, 220 insertions(+), 11 deletions(-) create mode 100644 changelogs/unreleased/4839-jxun create mode 100644 pkg/restic/repository_manager_test.go diff --git a/Makefile b/Makefile index c17e24147..df82b92ef 100644 --- a/Makefile +++ b/Makefile @@ -82,7 +82,7 @@ see: https://velero.io/docs/main/build-from-source/#making-images-and-updating-v endef # The version of restic binary to be downloaded -RESTIC_VERSION ?= 0.12.1 +RESTIC_VERSION ?= 0.13.1 CLI_PLATFORMS ?= linux-amd64 linux-arm linux-arm64 darwin-amd64 darwin-arm64 windows-amd64 linux-ppc64le BUILDX_PLATFORMS ?= $(subst -,/,$(ARCH)) diff --git a/changelogs/unreleased/4839-jxun b/changelogs/unreleased/4839-jxun new file mode 100644 index 000000000..d3ee746d8 --- /dev/null +++ b/changelogs/unreleased/4839-jxun @@ -0,0 +1 @@ +Update integrated Restic version and add insecureSkipTLSVerify for Restic CLI. \ No newline at end of file diff --git a/pkg/controller/pod_volume_backup_controller.go b/pkg/controller/pod_volume_backup_controller.go index 609a7d712..31fae9f3d 100644 --- a/pkg/controller/pod_volume_backup_controller.go +++ b/pkg/controller/pod_volume_backup_controller.go @@ -139,8 +139,26 @@ func (r *PodVolumeBackupReconciler) Reconcile(ctx context.Context, req ctrl.Requ } defer os.Remove(resticDetails.credsFile) + backupLocation := &velerov1api.BackupStorageLocation{} + if err := r.Client.Get(context.Background(), client.ObjectKey{ + Namespace: pvb.Namespace, + Name: pvb.Spec.BackupStorageLocation, + }, backupLocation); err != nil { + return ctrl.Result{}, errors.Wrap(err, "error getting backup storage location") + } + + // #4820: restrieve insecureSkipTLSVerify from BSL configuration for + // AWS plugin. If nothing is return, that means insecureSkipTLSVerify + // is not enable for Restic command. + skipTLSRet := restic.GetInsecureSkipTLSVerifyFromBSL(backupLocation, log) + if len(skipTLSRet) > 0 { + resticCmd.ExtraFlags = append(resticCmd.ExtraFlags, skipTLSRet) + } + + var stdout, stderr string + var emptySnapshot bool - stdout, stderr, err := r.ResticExec.RunBackup(resticCmd, log, r.updateBackupProgressFunc(&pvb, log)) + stdout, stderr, err = r.ResticExec.RunBackup(resticCmd, log, r.updateBackupProgressFunc(&pvb, log)) if err != nil { if strings.Contains(stderr, "snapshot is empty") { emptySnapshot = true @@ -156,6 +174,11 @@ func (r *PodVolumeBackupReconciler) Reconcile(ctx context.Context, req ctrl.Requ cmd.Env = resticDetails.envs cmd.CACertFile = resticDetails.caCertFile + // #4820: also apply the insecureTLS flag to Restic snapshots command + if len(skipTLSRet) > 0 { + cmd.ExtraFlags = append(cmd.ExtraFlags, skipTLSRet) + } + snapshotID, err = r.ResticExec.GetSnapshotID(cmd) if err != nil { return r.updateStatusToFailed(ctx, &pvb, err, "getting snapshot id") diff --git a/pkg/controller/pod_volume_restore_controller.go b/pkg/controller/pod_volume_restore_controller.go index 841f02019..299af6fb9 100644 --- a/pkg/controller/pod_volume_restore_controller.go +++ b/pkg/controller/pod_volume_restore_controller.go @@ -385,6 +385,14 @@ func (c *podVolumeRestoreController) restorePodVolume(req *velerov1api.PodVolume } resticCmd.Env = env + // #4820: restrieve insecureSkipTLSVerify from BSL configuration for + // AWS plugin. If nothing is return, that means insecureSkipTLSVerify + // is not enable for Restic command. + skipTLSRet := restic.GetInsecureSkipTLSVerifyFromBSL(backupLocation, log) + if len(skipTLSRet) > 0 { + resticCmd.ExtraFlags = append(resticCmd.ExtraFlags, skipTLSRet) + } + var stdout, stderr string if stdout, stderr, err = restic.RunRestore(resticCmd, log, c.updateRestoreProgressFunc(req, log)); err != nil { diff --git a/pkg/restic/command_factory.go b/pkg/restic/command_factory.go index 16625cd3f..98bb6ffef 100644 --- a/pkg/restic/command_factory.go +++ b/pkg/restic/command_factory.go @@ -64,7 +64,8 @@ func GetSnapshotCommand(repoIdentifier, passwordFile string, tags map[string]str Command: "snapshots", RepoIdentifier: repoIdentifier, PasswordFile: passwordFile, - ExtraFlags: []string{"--json", "--last", getSnapshotTagFlag(tags)}, + // "--last" is replaced by "--latest=1" in restic v0.12.1 + ExtraFlags: []string{"--json", "--latest=1", getSnapshotTagFlag(tags)}, } } diff --git a/pkg/restic/command_factory_test.go b/pkg/restic/command_factory_test.go index e8145a28f..4a38bc190 100644 --- a/pkg/restic/command_factory_test.go +++ b/pkg/restic/command_factory_test.go @@ -58,7 +58,7 @@ func TestGetSnapshotCommand(t *testing.T) { assert.Equal(t, "password-file", c.PasswordFile) // set up expected flag names - expectedFlags := []string{"--json", "--last", "--tag"} + expectedFlags := []string{"--json", "--latest=1", "--tag"} // for tracking actual flag names actualFlags := []string{} // for tracking actual --tag values as a map @@ -68,10 +68,11 @@ func TestGetSnapshotCommand(t *testing.T) { for _, flag := range c.ExtraFlags { // split into 2 parts from the first = sign (if any) parts := strings.SplitN(flag, "=", 2) - // parts[0] is the flag name - actualFlags = append(actualFlags, parts[0]) + // convert --tag data to a map if parts[0] == "--tag" { + actualFlags = append(actualFlags, parts[0]) + // split based on , tags := strings.Split(parts[1], ",") // loop through each key-value tag pair @@ -81,6 +82,8 @@ func TestGetSnapshotCommand(t *testing.T) { // record actual key & value actualTags[kvs[0]] = kvs[1] } + } else { + actualFlags = append(actualFlags, flag) } } diff --git a/pkg/restic/exec_commands.go b/pkg/restic/exec_commands.go index 648dc68cc..a0d16d8d5 100644 --- a/pkg/restic/exec_commands.go +++ b/pkg/restic/exec_commands.go @@ -20,6 +20,7 @@ import ( "bytes" "encoding/json" "fmt" + "strings" "time" "github.com/pkg/errors" @@ -184,7 +185,15 @@ func getSummaryLine(b []byte) ([]byte, error) { // RunRestore runs a `restic restore` command and monitors the volume size to // provide progress updates to the caller. func RunRestore(restoreCmd *Command, log logrus.FieldLogger, updateFunc func(velerov1api.PodVolumeOperationProgress)) (string, string, error) { - snapshotSize, err := getSnapshotSize(restoreCmd.RepoIdentifier, restoreCmd.PasswordFile, restoreCmd.CACertFile, restoreCmd.Args[0], restoreCmd.Env) + insecureTLSFlag := "" + + for _, extraFlag := range restoreCmd.ExtraFlags { + if strings.Contains(extraFlag, resticInsecureTLSFlag) { + insecureTLSFlag = extraFlag + } + } + + snapshotSize, err := getSnapshotSize(restoreCmd.RepoIdentifier, restoreCmd.PasswordFile, restoreCmd.CACertFile, restoreCmd.Args[0], restoreCmd.Env, insecureTLSFlag) if err != nil { return "", "", errors.Wrap(err, "error getting snapshot size") } @@ -230,11 +239,15 @@ func RunRestore(restoreCmd *Command, log logrus.FieldLogger, updateFunc func(vel return stdout, stderr, err } -func getSnapshotSize(repoIdentifier, passwordFile, caCertFile, snapshotID string, env []string) (int64, error) { +func getSnapshotSize(repoIdentifier, passwordFile, caCertFile, snapshotID string, env []string, insecureTLS string) (int64, error) { cmd := StatsCommand(repoIdentifier, passwordFile, snapshotID) cmd.Env = env cmd.CACertFile = caCertFile + if len(insecureTLS) > 0 { + cmd.ExtraFlags = append(cmd.ExtraFlags, insecureTLS) + } + stdout, stderr, err := exec.RunCommand(cmd.Cmd()) if err != nil { return 0, errors.Wrapf(err, "error running command, stderr=%s", stderr) @@ -245,7 +258,7 @@ func getSnapshotSize(repoIdentifier, passwordFile, caCertFile, snapshotID string } if err := json.Unmarshal([]byte(stdout), &snapshotStats); err != nil { - return 0, errors.Wrap(err, "error unmarshalling restic stats result") + return 0, errors.Wrapf(err, "error unmarshalling restic stats result, stdout=%s", stdout) } return snapshotStats.TotalSize, nil diff --git a/pkg/restic/repository_manager.go b/pkg/restic/repository_manager.go index b456bfc02..f3ff735f9 100644 --- a/pkg/restic/repository_manager.go +++ b/pkg/restic/repository_manager.go @@ -20,6 +20,7 @@ import ( "context" "fmt" "os" + "strconv" "github.com/pkg/errors" "github.com/sirupsen/logrus" @@ -95,6 +96,16 @@ type repositoryManager struct { credentialsFileStore credentials.FileStore } +const ( + // insecureSkipTLSVerifyKey is the flag in BackupStorageLocation's config + // to indicate whether to skip TLS verify to setup insecure HTTPS connection. + insecureSkipTLSVerifyKey = "insecureSkipTLSVerify" + + // resticInsecureTLSFlag is the flag for Restic command line to indicate + // skip TLS verify on https connection. + resticInsecureTLSFlag = "--insecure-tls" +) + // NewRepositoryManager constructs a RepositoryManager. func NewRepositoryManager( ctx context.Context, @@ -184,10 +195,11 @@ func (rm *repositoryManager) ConnectToRepo(repo *velerov1api.ResticRepository) e defer rm.repoLocker.Unlock(repo.Name) snapshotsCmd := SnapshotsCommand(repo.Spec.ResticIdentifier) - // use the '--last' flag to minimize the amount of data fetched since + // use the '--latest=1' flag to minimize the amount of data fetched since // we're just validating that the repo exists and can be authenticated // to. - snapshotsCmd.ExtraFlags = append(snapshotsCmd.ExtraFlags, "--last") + // "--last" is replaced by "--latest=1" in restic v0.12.1 + snapshotsCmd.ExtraFlags = append(snapshotsCmd.ExtraFlags, "--latest=1") return rm.exec(snapshotsCmd, repo.Spec.BackupStorageLocation) } @@ -265,6 +277,14 @@ func (rm *repositoryManager) exec(cmd *Command, backupLocation string) error { } cmd.Env = env + // #4820: restrieve insecureSkipTLSVerify from BSL configuration for + // AWS plugin. If nothing is return, that means insecureSkipTLSVerify + // is not enable for Restic command. + skipTLSRet := GetInsecureSkipTLSVerifyFromBSL(loc, rm.log) + if len(skipTLSRet) > 0 { + cmd.ExtraFlags = append(cmd.ExtraFlags, skipTLSRet) + } + stdout, stderr, err := veleroexec.RunCommand(cmd.Cmd()) rm.log.WithFields(logrus.Fields{ "repository": cmd.RepoName(), @@ -278,3 +298,22 @@ func (rm *repositoryManager) exec(cmd *Command, backupLocation string) error { return nil } + +// GetInsecureSkipTLSVerifyFromBSL get insecureSkipTLSVerify flag from BSL configuration, +// Then return --insecure-tls flag with boolean value as result. +func GetInsecureSkipTLSVerifyFromBSL(backupLocation *velerov1api.BackupStorageLocation, logger logrus.FieldLogger) string { + result := "" + + if backupLocation == nil { + logger.Info("bsl is nil. return empty.") + return result + } + + if insecure, _ := strconv.ParseBool(backupLocation.Spec.Config[insecureSkipTLSVerifyKey]); insecure { + logger.Debugf("set --insecure-tls=true for Restic command according to BSL %s config", backupLocation.Name) + result = resticInsecureTLSFlag + "=true" + return result + } + + return result +} diff --git a/pkg/restic/repository_manager_test.go b/pkg/restic/repository_manager_test.go new file mode 100644 index 000000000..79d326bb8 --- /dev/null +++ b/pkg/restic/repository_manager_test.go @@ -0,0 +1,121 @@ +/* +Copyright the Velero contributors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package restic + +import ( + "testing" + + "github.com/sirupsen/logrus" + "github.com/stretchr/testify/assert" + + velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" +) + +func TestGetInsecureSkipTLSVerifyFromBSL(t *testing.T) { + log := logrus.StandardLogger() + tests := []struct { + name string + backupLocation *velerov1api.BackupStorageLocation + logger logrus.FieldLogger + expected string + }{ + { + "Test with nil BSL. Should return empty string.", + nil, + log, + "", + }, + { + "Test BSL with no configuration. Should return empty string.", + &velerov1api.BackupStorageLocation{ + Spec: velerov1api.BackupStorageLocationSpec{ + Provider: "azure", + }, + }, + log, + "", + }, + { + "Test with AWS BSL's insecureSkipTLSVerify set to false.", + &velerov1api.BackupStorageLocation{ + Spec: velerov1api.BackupStorageLocationSpec{ + Provider: "aws", + Config: map[string]string{ + "insecureSkipTLSVerify": "false", + }, + }, + }, + log, + "", + }, + { + "Test with AWS BSL's insecureSkipTLSVerify set to true.", + &velerov1api.BackupStorageLocation{ + Spec: velerov1api.BackupStorageLocationSpec{ + Provider: "aws", + Config: map[string]string{ + "insecureSkipTLSVerify": "true", + }, + }, + }, + log, + "--insecure-tls=true", + }, + { + "Test with Azure BSL's insecureSkipTLSVerify set to invalid.", + &velerov1api.BackupStorageLocation{ + Spec: velerov1api.BackupStorageLocationSpec{ + Provider: "azure", + Config: map[string]string{ + "insecureSkipTLSVerify": "invalid", + }, + }, + }, + log, + "", + }, + { + "Test with GCP without insecureSkipTLSVerify.", + &velerov1api.BackupStorageLocation{ + Spec: velerov1api.BackupStorageLocationSpec{ + Provider: "gcp", + Config: map[string]string{}, + }, + }, + log, + "", + }, + { + "Test with AWS without config.", + &velerov1api.BackupStorageLocation{ + Spec: velerov1api.BackupStorageLocationSpec{ + Provider: "aws", + }, + }, + log, + "", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + res := GetInsecureSkipTLSVerifyFromBSL(test.backupLocation, test.logger) + + assert.Equal(t, test.expected, res) + }) + } +}