Merge pull request #10702 from sbueringer/pr-repro-json-encoding

✨ Fix GetObjectVariableInto util func
kubernetes-sigs · May 31, 2024 · 32323cf · 32323cf
2 parents 16baac8 + 373aec3
commit 32323cf
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 13 deletions.
diff --git a/exp/runtime/topologymutation/variables.go b/exp/runtime/topologymutation/variables.go
@@ -19,7 +19,6 @@ package topologymutation
 import (
 	"encoding/json"
 	"strconv"
-	"strings"
 
 	"github.com/pkg/errors"
 	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
@@ -77,18 +76,13 @@ func GetObjectVariableInto(templateVariables map[string]apiextensionsv1.JSON, va
 		return err
 	}
 
-	if err := json.Unmarshal(sanitizeJSON(value.Raw), into); err != nil {
+	if err := json.Unmarshal(value.Raw, into); err != nil {
 		return errors.Wrapf(err, "failed to unmarshal variable json %q into %q", string(value.Raw), into)
 	}
 
 	return nil
 }
 
-func sanitizeJSON(input []byte) (output []byte) {
-	output = []byte(strings.ReplaceAll(string(input), "\\", ""))
-	return output
-}
-
 // ToMap converts a list of Variables to a map of apiextensionsv1.JSON (name is the map key).
 // This is usually used to convert the Variables in a GeneratePatchesRequestItem into a format
 // that is used by MergeVariableMaps.

diff --git a/exp/runtime/topologymutation/variables_test.go b/exp/runtime/topologymutation/variables_test.go
@@ -205,6 +205,7 @@ func Test_GetVariableObjectWithNestedType(t *testing.T) {
 		AddressesFromPools *[]AddressesFromPool `json:"addressesFromPools,omitempty"`
 		Ipv6Primary        *bool                `json:"ipv6Primary,omitempty"`
 	}
+	type WorkerKubeletExtraArgs map[string]string
 
 	g := NewWithT(t)
 
@@ -214,7 +215,7 @@ func Test_GetVariableObjectWithNestedType(t *testing.T) {
 		variableName           string
 		expectedNotFoundError  bool
 		expectedErr            bool
-		object                 *Network
+		object                 interface{}
 		expectedVariableObject interface{}
 	}{
 		{
@@ -223,7 +224,7 @@ func Test_GetVariableObjectWithNestedType(t *testing.T) {
 			variableName:           "invalid[",
 			expectedNotFoundError:  false,
 			object:                 &Network{},
-			expectedVariableObject: Network{},
+			expectedVariableObject: &Network{},
 			expectedErr:            true,
 		},
 		{
@@ -232,7 +233,7 @@ func Test_GetVariableObjectWithNestedType(t *testing.T) {
 			variableName:           "notEsists",
 			expectedNotFoundError:  true,
 			object:                 &Network{},
-			expectedVariableObject: Network{},
+			expectedVariableObject: &Network{},
 			expectedErr:            true,
 		},
 		{
@@ -244,7 +245,7 @@ func Test_GetVariableObjectWithNestedType(t *testing.T) {
 			variableName:           "network",
 			expectedNotFoundError:  false,
 			object:                 &Network{},
-			expectedVariableObject: Network{},
+			expectedVariableObject: &Network{},
 			expectedErr:            true,
 		},
 		{
@@ -257,7 +258,7 @@ func Test_GetVariableObjectWithNestedType(t *testing.T) {
 			expectedNotFoundError: false,
 			expectedErr:           false,
 			object:                &Network{},
-			expectedVariableObject: Network{
+			expectedVariableObject: &Network{
 				Ipv6Primary: ptr.To(true),
 				AddressesFromPools: &[]AddressesFromPool{
 					{
@@ -266,6 +267,23 @@ func Test_GetVariableObjectWithNestedType(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "valid variable with encoded character",
+			variables: map[string]apiextensionsv1.JSON{
+				// Note: When a user uses `<` in a string in a variable it will be encoded as `\u003c`
+				// This is already done by the APIserver and e.g. visible when doing a simple get cluster call.
+				// This test case makes sure that variables that contain `<` are unmarshalled correctly.
+				"workerKubeletExtraArgs": {Raw: []byte(`{"eviction-hard":"memory.available\u003c512M,nodefs.available\u003c5%","eviction-soft":"memory.available\u003c1024M,nodefs.available\u003c10%"}`)},
+			},
+			variableName:          "workerKubeletExtraArgs",
+			expectedNotFoundError: false,
+			expectedErr:           false,
+			object:                &WorkerKubeletExtraArgs{},
+			expectedVariableObject: &WorkerKubeletExtraArgs{
+				"eviction-hard": "memory.available<512M,nodefs.available<5%",
+				"eviction-soft": "memory.available<1024M,nodefs.available<10%",
+			},
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(*testing.T) {
@@ -278,7 +296,7 @@ func Test_GetVariableObjectWithNestedType(t *testing.T) {
 			if tt.expectedNotFoundError {
 				g.Expect(IsNotFoundError(err)).To(BeTrue())
 			}
-			g.Expect(*tt.object).To(Equal(tt.expectedVariableObject))
+			g.Expect(tt.object).To(Equal(tt.expectedVariableObject))
 		})
 	}
 }