diff --git a/changelogs/unreleased/9637-adam-jian-zhang b/changelogs/unreleased/9637-adam-jian-zhang new file mode 100644 index 000000000..3fd99e5e7 --- /dev/null +++ b/changelogs/unreleased/9637-adam-jian-zhang @@ -0,0 +1 @@ +Fix issue #9636, fix configmap lookup in non-default namespaces diff --git a/pkg/cmd/cli/install/install.go b/pkg/cmd/cli/install/install.go index 4cba276e9..baaab1024 100644 --- a/pkg/cmd/cli/install/install.go +++ b/pkg/cmd/cli/install/install.go @@ -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(), o.Namespace, crClient, o.NodeAgentConfigMap, &velerotypes.NodeAgentConfigs{}); err != nil { + if err := kubeutil.VerifyJSONConfigs(c.Context(), f.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(), o.Namespace, crClient, o.RepoMaintenanceJobConfigMap, &velerotypes.JobConfigs{}); err != nil { + if err := kubeutil.VerifyJSONConfigs(c.Context(), f.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(), o.Namespace, crClient, o.BackupRepoConfigMap, &config); err != nil { + if err := kubeutil.VerifyJSONConfigs(c.Context(), f.Namespace(), crClient, o.BackupRepoConfigMap, &config); err != nil { return fmt.Errorf("--backup-repository-configmap specified ConfigMap %s is invalid: %w", o.BackupRepoConfigMap, err) } } diff --git a/pkg/cmd/cli/install/install_test.go b/pkg/cmd/cli/install/install_test.go index c5d147646..e46d0e8db 100644 --- a/pkg/cmd/cli/install/install_test.go +++ b/pkg/cmd/cli/install/install_test.go @@ -17,11 +17,18 @@ limitations under the License. package install import ( + "context" "testing" + "github.com/spf13/cobra" "github.com/spf13/pflag" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + corev1api "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + factorymocks "github.com/vmware-tanzu/velero/pkg/client/mocks" + velerotest "github.com/vmware-tanzu/velero/pkg/test" ) func TestPriorityClassNameFlag(t *testing.T) { @@ -91,3 +98,134 @@ func TestPriorityClassNameFlag(t *testing.T) { }) } } + +// makeValidateCmd returns a minimal *cobra.Command that satisfies output.ValidateFlags. +func makeValidateCmd() *cobra.Command { + c := &cobra.Command{} + // output.ValidateFlags only inspects the "output" flag; add it so validation passes. + c.Flags().StringP("output", "o", "", "output format") + return c +} + +// configMapInNamespace builds a ConfigMap with a single JSON data entry in the given namespace. +func configMapInNamespace(namespace, name, jsonValue string) *corev1api.ConfigMap { + return &corev1api.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: namespace, + Name: name, + }, + Data: map[string]string{ + "config": jsonValue, + }, + } +} + +// TestValidateConfigMapsUseFactoryNamespace verifies that Validate resolves the target +// namespace via f.Namespace() (not o.Namespace) 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. +func TestValidateConfigMapsUseFactoryNamespace(t *testing.T) { + const targetNS = "tenant-b" + const defaultNS = "default" + + // 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 + o.UseVolumeSnapshots = false + o.NoSecret = true + return o + } + + tests := []struct { + name string + setupOpts func(o *Options, cmName string) + cmJSON string + wantErrMsg string // substring expected in error; empty means success + }{ + { + name: "NodeAgentConfigMap found in factory namespace", + setupOpts: func(o *Options, cmName string) { + o.NodeAgentConfigMap = cmName + }, + cmJSON: `{}`, + }, + { + name: "NodeAgentConfigMap not found when only in default namespace", + setupOpts: func(o *Options, cmName string) { + o.NodeAgentConfigMap = cmName + }, + cmJSON: `{}`, + wantErrMsg: "--node-agent-configmap specified ConfigMap", + }, + { + name: "RepoMaintenanceJobConfigMap found in factory namespace", + setupOpts: func(o *Options, cmName string) { + o.RepoMaintenanceJobConfigMap = cmName + }, + cmJSON: `{}`, + }, + { + name: "RepoMaintenanceJobConfigMap not found when only in default namespace", + setupOpts: func(o *Options, cmName string) { + o.RepoMaintenanceJobConfigMap = cmName + }, + cmJSON: `{}`, + wantErrMsg: "--repo-maintenance-job-configmap specified ConfigMap", + }, + { + name: "BackupRepoConfigMap found in factory namespace", + setupOpts: func(o *Options, cmName string) { + o.BackupRepoConfigMap = cmName + }, + cmJSON: `{}`, + }, + { + name: "BackupRepoConfigMap not found when only in default namespace", + setupOpts: func(o *Options, cmName string) { + o.BackupRepoConfigMap = cmName + }, + cmJSON: `{}`, + wantErrMsg: "--backup-repository-configmap specified ConfigMap", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + const cmName = "my-config" + + // Decide where to place the ConfigMap: + // "not found" cases put it in "default", so the factory namespace lookup misses it. + cmNamespace := targetNS + if tc.wantErrMsg != "" { + cmNamespace = defaultNS + } + + cm := configMapInNamespace(cmNamespace, cmName, tc.cmJSON) + kbClient := velerotest.NewFakeControllerRuntimeClient(t, cm) + + f := &factorymocks.Factory{} + f.On("Namespace").Return(targetNS) + f.On("KubebuilderClient").Return(kbClient, nil) + + o := baseOptions() + tc.setupOpts(o, cmName) + + c := makeValidateCmd() + c.SetContext(context.Background()) + + err := o.Validate(c, []string{}, f) + + if tc.wantErrMsg == "" { + require.NoError(t, err) + } else { + require.Error(t, err) + assert.Contains(t, err.Error(), tc.wantErrMsg) + } + }) + } +}