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>
This commit is contained in:
Adam Zhang
2026-03-23 14:55:06 +08:00
parent 33b1fde8e1
commit cd7c9cba3e
3 changed files with 142 additions and 3 deletions
@@ -0,0 +1 @@
Fix issue #9636, fix configmap lookup in non-default namespaces
+3 -3
View File
@@ -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)
}
}
+138
View File
@@ -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)
}
})
}
}