From f8938e7feddfd9810f3d14c44e55b34b4020cab4 Mon Sep 17 00:00:00 2001
From: Xun Jiang/Bruce Jiang <59276555+blackpiglet@users.noreply.github.com>
Date: Tue, 30 Sep 2025 03:08:05 +0800
Subject: [PATCH] VerifyJSONConfigs verify every elements in Data. (#9302)

Add error message in the velero install CLI output if VerifyJSONConfigs fail.
Only allow one element in node-agent-configmap's Data.

Signed-off-by: Xun Jiang <xun.jiang@broadcom.com>
---
 changelogs/unreleased/9302-blackpiglet |  1 +
 pkg/cmd/cli/install/install.go         | 10 ++++------
 pkg/nodeagent/node_agent.go            |  4 ++++
 pkg/nodeagent/node_agent_test.go       |  9 +++++++++
 pkg/util/kube/utils.go                 | 11 ++++++-----
 5 files changed, 24 insertions(+), 11 deletions(-)
 create mode 100644 changelogs/unreleased/9302-blackpiglet

diff --git a/changelogs/unreleased/9302-blackpiglet b/changelogs/unreleased/9302-blackpiglet
new file mode 100644
index 000000000..63576a535
--- /dev/null
+++ b/changelogs/unreleased/9302-blackpiglet
@@ -0,0 +1 @@
+VerifyJSONConfigs verify every elements in Data.
diff --git a/pkg/cmd/cli/install/install.go b/pkg/cmd/cli/install/install.go
index c7a7dfe7a..6698010bd 100644
--- a/pkg/cmd/cli/install/install.go
+++ b/pkg/cmd/cli/install/install.go
@@ -545,24 +545,22 @@ func (o *Options) Validate(c *cobra.Command, args []string, f client.Factory) er
 		return fmt.Errorf("fail to create go-client %w", err)
 	}
 
-	// If either Linux or Windows node-agent is installed, and the node-agent-configmap
-	// is specified, need to validate the ConfigMap.
-	if (o.UseNodeAgent || o.UseNodeAgentWindows) && len(o.NodeAgentConfigMap) > 0 {
+	if len(o.NodeAgentConfigMap) > 0 {
 		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", o.NodeAgentConfigMap)
+			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 {
-			return fmt.Errorf("--repo-maintenance-job-configmap specified ConfigMap %s is invalid", o.RepoMaintenanceJobConfigMap)
+			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 {
-			return fmt.Errorf("--backup-repository-configmap specified ConfigMap %s is invalid", o.BackupRepoConfigMap)
+			return fmt.Errorf("--backup-repository-configmap specified ConfigMap %s is invalid: %w", o.BackupRepoConfigMap, err)
 		}
 	}
 
diff --git a/pkg/nodeagent/node_agent.go b/pkg/nodeagent/node_agent.go
index 7268b589a..a5de2465c 100644
--- a/pkg/nodeagent/node_agent.go
+++ b/pkg/nodeagent/node_agent.go
@@ -143,6 +143,10 @@ func GetConfigs(ctx context.Context, namespace string, kubeClient kubernetes.Int
 		return nil, errors.Errorf("data is not available in config map %s", configName)
 	}
 
+	if len(cm.Data) > 1 {
+		return nil, errors.Errorf("more than one keys are found in ConfigMap %s's data. only expect one", configName)
+	}
+
 	jsonString := ""
 	for _, v := range cm.Data {
 		jsonString = v
diff --git a/pkg/nodeagent/node_agent_test.go b/pkg/nodeagent/node_agent_test.go
index bdc1085b4..cb46ee569 100644
--- a/pkg/nodeagent/node_agent_test.go
+++ b/pkg/nodeagent/node_agent_test.go
@@ -249,6 +249,7 @@ func TestGetConfigs(t *testing.T) {
 	cmWithValidData := builder.ForConfigMap("fake-ns", "node-agent-config").Data("fake-key", "{\"loadConcurrency\":{\"globalConfig\": 5}}").Result()
 	cmWithPriorityClass := builder.ForConfigMap("fake-ns", "node-agent-config").Data("fake-key", "{\"priorityClassName\": \"high-priority\"}").Result()
 	cmWithPriorityClassAndOther := builder.ForConfigMap("fake-ns", "node-agent-config").Data("fake-key", "{\"priorityClassName\": \"low-priority\", \"loadConcurrency\":{\"globalConfig\": 3}}").Result()
+	cmWithMultipleKeysInData := builder.ForConfigMap("fake-ns", "node-agent-config").Data("fake-key-1", "{}", "fake-key-2", "{}").Result()
 
 	tests := []struct {
 		name          string
@@ -331,6 +332,14 @@ func TestGetConfigs(t *testing.T) {
 				},
 			},
 		},
+		{
+			name:      "ConfigMap's Data has more than one key",
+			namespace: "fake-ns",
+			kubeClientObj: []runtime.Object{
+				cmWithMultipleKeysInData,
+			},
+			expectErr: "more than one keys are found in ConfigMap node-agent-config's data. only expect one",
+		},
 	}
 
 	for _, test := range tests {
diff --git a/pkg/util/kube/utils.go b/pkg/util/kube/utils.go
index 002070376..5e5e97603 100644
--- a/pkg/util/kube/utils.go
+++ b/pkg/util/kube/utils.go
@@ -371,15 +371,16 @@ func VerifyJSONConfigs(ctx context.Context, namespace string, crClient client.Cl
 		return errors.Errorf("data is not available in ConfigMap %s", configName)
 	}
 
+	// Verify all the keys in ConfigMap's data.
 	jsonString := ""
 	for _, v := range cm.Data {
 		jsonString = v
-	}
 
-	configs := configType
-	err = json.Unmarshal([]byte(jsonString), configs)
-	if err != nil {
-		return errors.Wrapf(err, "error to unmarshall data from ConfigMap %s", configName)
+		configs := configType
+		err = json.Unmarshal([]byte(jsonString), configs)
+		if err != nil {
+			return errors.Wrapf(err, "error to unmarshall data from ConfigMap %s", configName)
+		}
 	}
 
 	return nil