[main] fix configmap lookup in non-default namespaces (#9638)

* fix configmap lookup in non-default namespaces

o.Namespace is empty when Validate runs (Complete hasn't been called yet),
causing VerifyJSONConfigs to query the default namespace instead of the
intended one. Replace o.Namespace with f.Namespace() in all three ConfigMap
validation calls so the factory's already-resolved namespace is used.

Signed-off-by: Adam Zhang <adam.zhang@broadcom.com>

* switch the call order of validate/complete

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

Signed-off-by: Adam Zhang <adam.zhang@broadcom.com>

---------

Signed-off-by: Adam Zhang <adam.zhang@broadcom.com>

changelogs/unreleased/9638-adam-jian-zhang

@@ -0,0 +1 @@
+Fix issue #9636, fix configmap lookup in non-default namespaces

pkg/cmd/cli/install/install.go

@@ -381,8 +381,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))
 		},
 	}

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,168 @@ 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 correctly for all three ConfigMap flags.
+//
+// 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"
+
+	// Shared options that satisfy every other validation gate:
+	//   - NoDefaultBackupLocation=true + UseVolumeSnapshots=false skips provider/bucket/plugins checks
+	//   - NoSecret=true satisfies the secret-file check
+	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)
+
+			// 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())
+
+			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)
+			}
+		})
+	}
+}
+
+// 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())
+}