Refactor template addon get values func

Signed-off-by: zhujian <jiazhu@redhat.com>
open-cluster-management-io · Apr 22, 2024 · dce6494 · dce6494
1 parent a7adf5d
commit dce6494
Show file tree

Hide file tree

Showing 3 changed files with 69 additions and 22 deletions.
diff --git a/pkg/addon/templateagent/template_agent.go b/pkg/addon/templateagent/template_agent.go
@@ -24,6 +24,8 @@ import (
 )
 
 const (
+	// Private value keys that are used internally by the addon template controller, should not be exposed to users.
+	// All private value keys should begin with "__"
 	NodePlacementPrivateValueKey    = "__NODE_PLACEMENT"
 	RegistriesPrivateValueKey       = "__REGISTRIES"
 	InstallNamespacePrivateValueKey = "__INSTALL_NAMESPACE"
@@ -42,8 +44,7 @@ type templateCRDBuiltinValues struct {
 // to convert it to Values by JsonStructToValues.
 // the default values can be overridden by getValuesFuncs
 type templateCRDDefaultValues struct {
-	HubKubeConfigPath     string `json:"HUB_KUBECONFIG,omitempty"`
-	ManagedKubeConfigPath string `json:"MANAGED_KUBECONFIG,omitempty"`
+	HubKubeConfigPath string `json:"HUB_KUBECONFIG,omitempty"`
 }
 
 type CRDTemplateAgentAddon struct {

diff --git a/pkg/addon/templateagent/values.go b/pkg/addon/templateagent/values.go
@@ -58,8 +58,11 @@ func (a *CRDTemplateAgentAddon) getValues(
 	template *addonapiv1alpha1.AddOnTemplate,
 ) (orderedValues, map[string]interface{}, map[string]interface{}, error) {
 
+	// preset values are combined by default values and builtin values
 	presetValues := make([]keyValuePair, 0)
+	// override values are values that users can use in the template
 	overrideValues := map[string]interface{}{}
+	// private values are values that are not exposed to users
 	privateValues := map[string]interface{}{}
 
 	defaultSortedKeys, defaultValues, err := a.getDefaultValues(cluster, addon, template)

diff --git a/pkg/addon/templateagent/values_test.go b/pkg/addon/templateagent/values_test.go
@@ -110,20 +110,20 @@ func TestGetValues(t *testing.T) {
 		name             string
 		templateSpec     addonapiv1alpha1.AddOnTemplateSpec
 		values           addonfactory.Values
-		expectedDefault  orderedValues
+		expectedPreset   orderedValues
 		expectedOverride map[string]interface{}
 		expectedPrivate  map[string]interface{}
 	}{
 		{
-			name: "no override",
+			name: "with default value, registration set in template",
 			templateSpec: addonapiv1alpha1.AddOnTemplateSpec{
 				Registration: []addonapiv1alpha1.RegistrationSpec{
 					{
 						Type: addonapiv1alpha1.RegistrationTypeKubeClient,
 					},
 				},
 			},
-			expectedDefault: orderedValues{
+			expectedPreset: orderedValues{
 				{
 					name:  "HUB_KUBECONFIG",
 					value: "/managed/hub-kubeconfig/kubeconfig",
@@ -140,12 +140,12 @@ func TestGetValues(t *testing.T) {
 			expectedPrivate: map[string]interface{}{},
 		},
 		{
-			name:         "overrides builtin value",
+			name:         "without default value, registration not set in template",
 			templateSpec: addonapiv1alpha1.AddOnTemplateSpec{},
 			values: addonfactory.Values{
 				InstallNamespacePrivateValueKey: "default-ns",
 			},
-			expectedDefault: orderedValues{
+			expectedPreset: orderedValues{
 				{
 					name:  "CLUSTER_NAME",
 					value: "test-cluster",
@@ -159,13 +159,13 @@ func TestGetValues(t *testing.T) {
 			},
 		},
 		{
-			name:         "overrides public value",
+			name:         "with private and user defined values",
 			templateSpec: addonapiv1alpha1.AddOnTemplateSpec{},
 			values: addonfactory.Values{
 				InstallNamespacePrivateValueKey: "default-ns",
 				"key1":                          "value1",
 			},
-			expectedDefault: orderedValues{
+			expectedPreset: orderedValues{
 				{
 					name:  "CLUSTER_NAME",
 					value: "test-cluster",
@@ -180,20 +180,63 @@ func TestGetValues(t *testing.T) {
 			},
 		},
 		{
-			name:         "default should not be overridden",
-			templateSpec: addonapiv1alpha1.AddOnTemplateSpec{},
+			name: "default value should be overridden",
+			templateSpec: addonapiv1alpha1.AddOnTemplateSpec{
+				Registration: []addonapiv1alpha1.RegistrationSpec{
+					{
+						Type: addonapiv1alpha1.RegistrationTypeKubeClient,
+					},
+				},
+			},
 			values: addonfactory.Values{
 				InstallNamespacePrivateValueKey: "default-ns",
-				"CLUSTER_NAME":                  "another-cluster",
+				"HUB_KUBECONFIG":                "/managed/hub-kubeconfig/kubeconfig-test",
 			},
-			expectedDefault: orderedValues{
+			expectedPreset: orderedValues{
+				{
+					name:  "HUB_KUBECONFIG",
+					value: "/managed/hub-kubeconfig/kubeconfig-test",
+				},
 				{
 					name:  "CLUSTER_NAME",
 					value: "test-cluster",
 				},
 			},
 			expectedOverride: map[string]interface{}{
-				"CLUSTER_NAME": "test-cluster",
+				"HUB_KUBECONFIG": "/managed/hub-kubeconfig/kubeconfig-test",
+				"CLUSTER_NAME":   "test-cluster",
+			},
+			expectedPrivate: map[string]interface{}{
+				InstallNamespacePrivateValueKey: "default-ns",
+			},
+		},
+		{
+			name: "builtIn value should not be overridden",
+			templateSpec: addonapiv1alpha1.AddOnTemplateSpec{
+				Registration: []addonapiv1alpha1.RegistrationSpec{
+					{
+						Type: addonapiv1alpha1.RegistrationTypeKubeClient,
+					},
+				},
+			},
+			values: addonfactory.Values{
+				InstallNamespacePrivateValueKey: "default-ns",
+				"HUB_KUBECONFIG":                "/managed/hub-kubeconfig/kubeconfig-test",
+				"CLUSTER_NAME":                  "cluster1",
+			},
+			expectedPreset: orderedValues{
+				{
+					name:  "HUB_KUBECONFIG",
+					value: "/managed/hub-kubeconfig/kubeconfig-test",
+				},
+				{
+					name:  "CLUSTER_NAME",
+					value: "test-cluster",
+				},
+			},
+			expectedOverride: map[string]interface{}{
+				"HUB_KUBECONFIG": "/managed/hub-kubeconfig/kubeconfig-test",
+				"CLUSTER_NAME":   "test-cluster",
 			},
 			expectedPrivate: map[string]interface{}{
 				InstallNamespacePrivateValueKey: "default-ns",
@@ -225,21 +268,21 @@ func TestGetValues(t *testing.T) {
 				Spec:       c.templateSpec,
 			}
 
-			defaultValue, overrideValue, builtInValue, err := agentAddon.getValues(cluster, addon, addonTemplate)
+			presetValues, overrideValues, privateValues, err := agentAddon.getValues(cluster, addon, addonTemplate)
 			if err != nil {
 				t.Fatal(err)
 			}
-			fmt.Printf("defaultValue is %v\n", defaultValue)
-			if !orderedValueEquals(defaultValue, c.expectedDefault) {
-				t.Errorf("default value is not correct, expect %v, got %v", c.expectedDefault, defaultValue)
+			fmt.Printf("presetValues is %v\n", presetValues)
+			if !orderedValueEquals(presetValues, c.expectedPreset) {
+				t.Errorf("preset value is not correct, expect %v, got %v", c.expectedPreset, presetValues)
 			}
 
-			if !apiequality.Semantic.DeepEqual(overrideValue, c.expectedOverride) {
-				t.Errorf("override value is not correct, expect %v, got %v", c.expectedOverride, overrideValue)
+			if !apiequality.Semantic.DeepEqual(overrideValues, c.expectedOverride) {
+				t.Errorf("override value is not correct, expect %v, got %v", c.expectedOverride, overrideValues)
 			}
 
-			if !apiequality.Semantic.DeepEqual(builtInValue, c.expectedPrivate) {
-				t.Errorf("builtin value is not correct, expect %v, got %v", c.expectedPrivate, builtInValue)
+			if !apiequality.Semantic.DeepEqual(privateValues, c.expectedPrivate) {
+				t.Errorf("builtin value is not correct, expect %v, got %v", c.expectedPrivate, privateValues)
 			}
 		})
 	}