Merge branch 'main' into fix/backup-name-validation

2026-09-26 18:04:31 +00:00 · 2026-08-13 09:46:24 +05:30
parent c27a661d1d b353fafe5e
commit b731710e81
106 changed files with 4270 additions and 2625 deletions
@@ -262,11 +262,17 @@ func (o *CreateOptions) validateFromScheduleFlag(c *cobra.Command) error {
 	return nil
 }

+// validateBackupType check the backupType value and return the valid value.
 func (o *CreateOptions) validateBackupType() error {
-	backupType := strings.TrimSpace(o.BackupType)
+	// Allow full, and incremental from the CLI, and ignore case of the input string's case.
+	backupType := strings.ToLower(strings.TrimSpace(o.BackupType))

 	switch backupType {
-	case "", "Incremental", "Full":
+	case "":
+	case "incremental":
+		o.BackupType = string(velerov1api.BackupTypeIncremental)
+	case "full":
+		o.BackupType = string(velerov1api.BackupTypeFull)
 	default:
 		return fmt.Errorf("invalid backup type %s - valid values are 'Incremental', and 'Full'", backupType)
 	}
@@ -129,30 +129,34 @@ func TestCreateOptions_ValidateBackupType(t *testing.T) {
 		o.BackupType = ""
 		err := o.validateBackupType()
 		require.NoError(t, err)
+		require.Empty(t, o.BackupType)

 		o.BackupType = "Incremental"
 		err = o.validateBackupType()
 		require.NoError(t, err)
+		require.EqualValues(t, velerov1api.BackupTypeIncremental, o.BackupType)

 		o.BackupType = "Full"
 		err = o.validateBackupType()
 		require.NoError(t, err)
+		require.EqualValues(t, velerov1api.BackupTypeFull, o.BackupType)

 		o.BackupType = " Incremental "
 		err = o.validateBackupType()
 		require.NoError(t, err)
+		require.EqualValues(t, velerov1api.BackupTypeIncremental, o.BackupType)
+
+		o.BackupType = "iNcReMeNtAl"
+		err = o.validateBackupType()
+		require.NoError(t, err)
+		require.EqualValues(t, velerov1api.BackupTypeIncremental, o.BackupType)
 	})

 	t.Run("invalid backup type", func(t *testing.T) {
 		o := NewCreateOptions()

-		o.BackupType = "incremental"
-		err := o.validateBackupType()
-		require.Error(t, err)
-		require.Equal(t, "invalid backup type incremental - valid values are 'Incremental', and 'Full'", err.Error())
-
 		o.BackupType = "invalid"
-		err = o.validateBackupType()
+		err := o.validateBackupType()
 		require.Error(t, err)
 		require.Equal(t, "invalid backup type invalid - valid values are 'Incremental', and 'Full'", err.Error())
 	})
@@ -23,8 +23,9 @@ import (
 	"time"

 	"github.com/spf13/cobra"
+	"github.com/spf13/pflag"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
-	ctrlclient "sigs.k8s.io/controller-runtime/pkg/client"
+	kbclient "sigs.k8s.io/controller-runtime/pkg/client"

 	velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
 	"github.com/vmware-tanzu/velero/pkg/client"
@@ -34,59 +35,94 @@ import (
 	"github.com/vmware-tanzu/velero/pkg/cmd/util/downloadrequest"
 )

-func NewLogsCommand(f client.Factory) *cobra.Command {
+// LogsOptions holds the state for the restore logs command, mirroring
+// pkg/cmd/cli/backup.LogsOptions so both commands are shaped the same way.
+type LogsOptions struct {
+	Timeout               time.Duration
+	InsecureSkipTLSVerify bool
+	CaCertFile            string
+	Client                kbclient.Client
+	RestoreName           string
+}
+
+func NewLogsOptions() LogsOptions {
 	config, err := client.LoadConfig()
 	if err != nil {
 		fmt.Fprintf(os.Stderr, "WARNING: Error reading config file: %v\n", err)
 	}

-	timeout := time.Minute
-	insecureSkipTLSVerify := false
-	caCertFile := config.CACertFile()
+	return LogsOptions{
+		Timeout:               time.Minute,
+		InsecureSkipTLSVerify: false,
+		CaCertFile:            config.CACertFile(),
+	}
+}
+
+func (l *LogsOptions) BindFlags(flags *pflag.FlagSet) {
+	flags.DurationVar(&l.Timeout, "timeout", l.Timeout, "How long to wait to receive logs.")
+	flags.BoolVar(&l.InsecureSkipTLSVerify, "insecure-skip-tls-verify", l.InsecureSkipTLSVerify, "If true, the object store's TLS certificate will not be checked for validity. This is insecure and susceptible to man-in-the-middle attacks. Not recommended for production.")
+	flags.StringVar(&l.CaCertFile, "cacert", l.CaCertFile, "Path to a certificate bundle to use when verifying TLS connections. If not specified, the CA certificate from the BackupStorageLocation will be used if available.")
+}
+
+func (l *LogsOptions) Run(c *cobra.Command, f client.Factory) error {
+	restore := new(velerov1api.Restore)
+	err := l.Client.Get(context.Background(), kbclient.ObjectKey{Namespace: f.Namespace(), Name: l.RestoreName}, restore)
+	if apierrors.IsNotFound(err) {
+		return fmt.Errorf("restore %q does not exist", l.RestoreName)
+	} else if err != nil {
+		return fmt.Errorf("error checking for restore %q: %v", l.RestoreName, err)
+	}
+
+	switch restore.Status.Phase {
+	case velerov1api.RestorePhaseCompleted, velerov1api.RestorePhaseFailed, velerov1api.RestorePhasePartiallyFailed, velerov1api.RestorePhaseWaitingForPluginOperations, velerov1api.RestorePhaseWaitingForPluginOperationsPartiallyFailed:
+		// terminal and waiting for plugin operations phases, do nothing.
+	default:
+		return fmt.Errorf("logs for restore %q are not available until it's finished processing, please wait "+
+			"until the restore has a phase of Completed or Failed and try again", l.RestoreName)
+	}
+
+	// Get BSL cacert if available
+	bslCACert, err := cacert.GetCACertFromRestore(context.Background(), l.Client, f.Namespace(), restore)
+	if err != nil {
+		// Log the error but don't fail - we can still try to download without the BSL cacert
+		fmt.Fprintf(os.Stderr, "WARNING: Error getting cacert from BSL: %v\n", err)
+		bslCACert = ""
+	}
+
+	return downloadrequest.StreamWithBSLCACert(context.Background(), l.Client, f.Namespace(), l.RestoreName, velerov1api.DownloadTargetKindRestoreLog, os.Stdout, l.Timeout, l.InsecureSkipTLSVerify, l.CaCertFile, bslCACert)
+}
+
+func (l *LogsOptions) Complete(args []string, f client.Factory) error {
+	if len(args) > 0 {
+		l.RestoreName = args[0]
+	}
+
+	kbClient, err := f.KubebuilderClient()
+	if err != nil {
+		return err
+	}
+	l.Client = kbClient
+	return nil
+}
+
+func NewLogsCommand(f client.Factory) *cobra.Command {
+	l := NewLogsOptions()

 	c := &cobra.Command{
 		Use:   "logs RESTORE",
 		Short: "Get restore logs",
 		Args:  cobra.ExactArgs(1),
 		Run: func(c *cobra.Command, args []string) {
-			restoreName := args[0]
-
-			kbClient, err := f.KubebuilderClient()
+			err := l.Complete(args, f)
 			cmd.CheckError(err)

-			restore := new(velerov1api.Restore)
-			err = kbClient.Get(context.Background(), ctrlclient.ObjectKey{Namespace: f.Namespace(), Name: restoreName}, restore)
-			if apierrors.IsNotFound(err) {
-				cmd.Exit("Restore %q does not exist.", restoreName)
-			} else if err != nil {
-				cmd.Exit("Error checking for restore %q: %v", restoreName, err)
-			}
-
-			switch restore.Status.Phase {
-			case velerov1api.RestorePhaseCompleted, velerov1api.RestorePhaseFailed, velerov1api.RestorePhasePartiallyFailed, velerov1api.RestorePhaseWaitingForPluginOperations, velerov1api.RestorePhaseWaitingForPluginOperationsPartiallyFailed:
-				// terminal and waiting for plugin operations phases, don't exit.
-			default:
-				cmd.Exit("Logs for restore %q are not available until it's finished processing. Please wait "+
-					"until the restore has a phase of Completed or Failed and try again.", restoreName)
-			}
-
-			// Get BSL cacert if available
-			bslCACert, err := cacert.GetCACertFromRestore(context.Background(), kbClient, f.Namespace(), restore)
-			if err != nil {
-				// Log the error but don't fail - we can still try to download without the BSL cacert
-				fmt.Fprintf(os.Stderr, "WARNING: Error getting cacert from BSL: %v\n", err)
-				bslCACert = ""
-			}
-
-			err = downloadrequest.StreamWithBSLCACert(context.Background(), kbClient, f.Namespace(), restoreName, velerov1api.DownloadTargetKindRestoreLog, os.Stdout, timeout, insecureSkipTLSVerify, caCertFile, bslCACert)
+			err = l.Run(c, f)
 			cmd.CheckError(err)
 		},
 	}

 	c.ValidArgsFunction = cli.CompleteRestoreNames(f)
-	c.Flags().DurationVar(&timeout, "timeout", timeout, "How long to wait to receive logs.")
-	c.Flags().BoolVar(&insecureSkipTLSVerify, "insecure-skip-tls-verify", insecureSkipTLSVerify, "If true, the object store's TLS certificate will not be checked for validity. This is insecure and susceptible to man-in-the-middle attacks. Not recommended for production.")
-	c.Flags().StringVar(&caCertFile, "cacert", caCertFile, "Path to a certificate bundle to use when verifying TLS connections. If not specified, the CA certificate from the BackupStorageLocation will be used if available.")
+	l.BindFlags(c.Flags())

 	return c
 }
@@ -17,10 +17,12 @@ limitations under the License.
 package restore

 import (
+	"fmt"
 	"os"
 	"testing"
 	"time"

+	flag "github.com/spf13/pflag"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	kbclient "sigs.k8s.io/controller-runtime/pkg/client"
@@ -77,13 +79,20 @@ func TestNewLogsCommand(t *testing.T) {
 		c := NewLogsCommand(f)
 		assert.Equal(t, "Get restore logs", c.Short)

-		// The restore command exits with an error message when restore is not complete
-		// We can't easily test this since it calls cmd.Exit, which exits the process
-		// So we'll skip this test case
-		t.Skip("Cannot test restore not complete case due to cmd.Exit() call")
+		l := NewLogsOptions()
+		flags := new(flag.FlagSet)
+		l.BindFlags(flags)
+		err = l.Complete([]string{restoreName}, f)
+		require.NoError(t, err)
+
+		err = l.Run(c, f)
+		require.Error(t, err)
+		require.ErrorContains(t, err, fmt.Sprintf("logs for restore %q are not available until it's finished processing", restoreName))
 	})

 	t.Run("Restore not exist test", func(t *testing.T) {
+		restoreName := "not-exist"
+
 		// create a factory
 		f := &factorymocks.Factory{}

@@ -95,10 +104,15 @@ func TestNewLogsCommand(t *testing.T) {
 		c := NewLogsCommand(f)
 		assert.Equal(t, "Get restore logs", c.Short)

-		// The restore command exits with an error message when restore doesn't exist
-		// We can't easily test this since it calls cmd.Exit, which exits the process
-		// So we'll skip this test case
-		t.Skip("Cannot test restore not exist case due to cmd.Exit() call")
+		l := NewLogsOptions()
+		flags := new(flag.FlagSet)
+		l.BindFlags(flags)
+		err := l.Complete([]string{restoreName}, f)
+		require.NoError(t, err)
+
+		err = l.Run(c, f)
+		require.Error(t, err)
+		require.Equal(t, fmt.Sprintf("restore %q does not exist", restoreName), err.Error())
 	})

 	t.Run("Restore with BSL cacert test", func(t *testing.T) {