mirrors/velero
1
0
Fork 0
mirror of https://github.com/vmware-tanzu/velero.git synced 2026-09-04 07:07:11 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

switch the call order of validate/complete

Browse Source 
switch the call order of validate/complete which accomplish
the same effect.

Signed-off-by: Adam Zhang <adam.zhang@broadcom.com>
This commit is contained in:
Adam Zhang
2026-03-24 11:12:42 +08:00
parent cd7c9cba3e
commit 09795245e7
2 changed files with 42 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/cmd/cli/install/install.go
+4 -4
View File
@@ -385,8 +385,8 @@ This is useful as a starting point for more customized installations.
# velero install --provider azure --plugins velero/velero-plugin-for-microsoft-azure:v1.0.0 --bucket $BLOB_CONTAINER --secret-file ./credentials-velero --backup-location-config resourceGroup=$AZURE_BACKUP_RESOURCE_GROUP,storageAccount=$AZURE_STORAGE_ACCOUNT_ID[,subscriptionId=$AZURE_BACKUP_SUBSCRIPTION_ID] --snapshot-location-config apiTimeout=<YOUR_TIMEOUT>[,resourceGroup=$AZURE_BACKUP_RESOURCE_GROUP,subscriptionId=$AZURE_BACKUP_SUBSCRIPTION_ID]`,
Run: func(c *cobra.Command, args []string) {
cmd.CheckError(o.Validate(c, args, f))
cmd.CheckError(o.Complete(args, f))
cmd.CheckError(o.Validate(c, args, f))
cmd.CheckError(o.Run(c, f))
},
}
@@ -570,20 +570,20 @@ func (o *Options) Validate(c *cobra.Command, args []string, f client.Factory) er
}
if len(o.NodeAgentConfigMap) > 0 {
if err := kubeutil.VerifyJSONConfigs(c.Context(), f.Namespace(), crClient, o.NodeAgentConfigMap, &velerotypes.NodeAgentConfigs{}); err != nil {
if err := kubeutil.VerifyJSONConfigs(c.Context(), o.Namespace, crClient, o.NodeAgentConfigMap, &velerotypes.NodeAgentConfigs{}); err != nil {
return fmt.Errorf("--node-agent-configmap specified ConfigMap %s is invalid: %w", o.NodeAgentConfigMap, err)
}
}
if len(o.RepoMaintenanceJobConfigMap) > 0 {
if err := kubeutil.VerifyJSONConfigs(c.Context(), f.Namespace(), crClient, o.RepoMaintenanceJobConfigMap, &velerotypes.JobConfigs{}); err != nil {
if err := kubeutil.VerifyJSONConfigs(c.Context(), o.Namespace, crClient, o.RepoMaintenanceJobConfigMap, &velerotypes.JobConfigs{}); err != nil {
return fmt.Errorf("--repo-maintenance-job-configmap specified ConfigMap %s is invalid: %w", o.RepoMaintenanceJobConfigMap, err)
}
}
if len(o.BackupRepoConfigMap) > 0 {
config := make(map[string]any)
if err := kubeutil.VerifyJSONConfigs(c.Context(), f.Namespace(), crClient, o.BackupRepoConfigMap, &config); err != nil {
if err := kubeutil.VerifyJSONConfigs(c.Context(), o.Namespace, crClient, o.BackupRepoConfigMap, &config); err != nil {
return fmt.Errorf("--backup-repository-configmap specified ConfigMap %s is invalid: %w", o.BackupRepoConfigMap, err)
}
}
pkg/cmd/cli/install/install_test.go
+38 -4
View File
@@ -121,10 +121,11 @@ func configMapInNamespace(namespace, name, jsonValue string) *corev1api.ConfigMa
}
// TestValidateConfigMapsUseFactoryNamespace verifies that Validate resolves the target
// namespace via f.Namespace() (not o.Namespace) for all three ConfigMap flags.
// namespace correctly for all three ConfigMap flags.
//
// Before the fix, o.Namespace was "" at Validate time (Complete had not run yet),
// so VerifyJSONConfigs queried the "default" namespace instead of the intended one.
// The fix (Option B) calls Complete before Validate in NewCommand so that o.Namespace is
// populated from f.Namespace() before VerifyJSONConfigs runs. Tests mirror that order by
// calling Complete before Validate.
func TestValidateConfigMapsUseFactoryNamespace(t *testing.T) {
const targetNS = "tenant-b"
const defaultNS = "default"
@@ -132,7 +133,6 @@ func TestValidateConfigMapsUseFactoryNamespace(t *testing.T) {
// Shared options that satisfy every other validation gate:
// - NoDefaultBackupLocation=true + UseVolumeSnapshots=false skips provider/bucket/plugins checks
// - NoSecret=true satisfies the secret-file check
// - o.Namespace is intentionally left as "" to mirror the pre-Complete state
baseOptions := func() *Options {
o := NewInstallOptions()
o.NoDefaultBackupLocation = true
@@ -215,6 +215,9 @@ func TestValidateConfigMapsUseFactoryNamespace(t *testing.T) {
o := baseOptions()
tc.setupOpts(o, cmName)
// Mirror the NewCommand call order: Complete populates o.Namespace before Validate runs.
require.NoError(t, o.Complete([]string{}, f))
c := makeValidateCmd()
c.SetContext(context.Background())
@@ -229,3 +232,34 @@ func TestValidateConfigMapsUseFactoryNamespace(t *testing.T) {
})
}
}
// TestNewCommandRunClosureOrder covers the Run closure in NewCommand (the lines that were
// reordered by the fix: Complete → Validate → Run).
//
// The closure uses CheckError which calls os.Exit on any error, so the only safe path is one
// where all three steps return nil. DryRun=true causes o.Run to return after PrintWithFormat
// (which is a no-op when no --output flag is set) without touching any cluster clients.
func TestNewCommandRunClosureOrder(t *testing.T) {
const targetNS = "tenant-b"
const cmName = "my-config"
cm := configMapInNamespace(targetNS, cmName, `{}`)
kbClient := velerotest.NewFakeControllerRuntimeClient(t, cm)
f := &factorymocks.Factory{}
f.On("Namespace").Return(targetNS)
f.On("KubebuilderClient").Return(kbClient, nil)
c := NewCommand(f)
c.SetArgs([]string{
"--no-default-backup-location",
"--use-volume-snapshots=false",
"--no-secret",
"--dry-run",
"--node-agent-configmap", cmName,
})
// Execute drives the full Run closure: Complete populates o.Namespace, Validate
// looks up the ConfigMap in targetNS (succeeds), Run returns early via DryRun.
require.NoError(t, c.Execute())
}