Repository: karmada-io/karmada

Issue:
SyncWorkFailed: [convert empty object back to empty slice failed: sjson set empty slice to empty struct failed: cannot set array element for non-numeric key 'f:mountPath', ...]
<!-- Please use this template while reporting a bug and provide as much info as possible. Not doing so may result in your bug not being addressed in a timely manner. Thanks!
-->


**What happened**:
I'm trying to dispatch ReplicaSet by Karmada, some error events exist, like below.
<img width="1856" height="1388" alt="Image" src="https://github.com/user-attachments/assets/55baf574-8158-494f-ae9c-0145e4f2a44b" />


But the workloads can be dispatched successfully although the error events exist. It looks like it has something to do with the `.metadata.managedFields` field.

**What you expected to happen**:
No error events take place.

**How to reproduce it (as minimally and precisely as possible)**:

**Anything else we need to know?**:

**Environment**:
- Karmada version: v1.14.4
- kubectl-karmada or karmadactl version (the result of `kubectl-karmada version` or `karmadactl version`):
- Others:


Discussion:
@ryanwuer: @zhzhuang-zju Pls take a look@zhzhuang-zju: /assign@zhzhuang-zju: @ryanwuer 
Thank you for your feedback. After analysis, this is an anomaly that only occurs under specific configurations. The root cause lies in the `sjson.Set` library's failure to properly handle `.` characters within JSON keys.

## Reproducible Configuration

```yaml
# observed.yml
apiVersion: apps/v1
kind: ReplicaSet
metadata:
  managedFields:
  - apiVersion: apps/v1
    fieldsType: FieldsV1
    fieldsV1:
      f:spec:
        f:template:
          f:spec:
            f:containers:
              k:{"name":"test"}:
                f:volumeMounts:
                  k:{"mountPath":"/test/.probe"}:
                    .: {}
                    f:mountPath: {}
spec:
  replicas: 3
```

Accompanied by the following ResourceInterpreterCustomization:

```yaml
# customization.yml
apiVersion: config.karmada.io/v1alpha1
kind: ResourceInterpreterCustomization
metadata:
  name: declarative-configuration-rs
spec:
  target:
    apiVersion: apps/v1
    kind: ReplicaSet
  customizations:
    replicaRevision:
      luaScript: >
        function ReviseReplica(obj, desiredReplica)
          obj.spec.replicas = desiredReplica
          return obj
        end
```

The issue can be reproduced using the following command (multiple runs may be required due to its probabilistic nature):

```bash
karmadactl interpret -f customization.yml --operation reviseReplica --observed-file observed.yml --desired-replica 2 -v=4
```

## Problem Mechanism

When the binding controller calls the resource interpreter (a Lua script) to revise replicas, it needs to convert the interpreter's result (`lua.LTable`) into JSON bytes. During this process, certain field values of `"[]"` must be corrected to `"{}"` (see details in #4656), i.e., replacing empty arrays with empty objects at specific field paths.

Using the current manifest as an example, the Lua execution generates the following JSON:

```json
{
  "metadata": {
    "managedFields": [
      {
        "fieldsV1": {
          "f:spec": {
            "f:template": {
              "f:spec": {
                "f:containers": {
                  "k:{\"name\":\"test\"}": {
                    "f:volumeMounts": {
                      "k:{\"mountPath\":\"/test/.probe\"}": {
                        ".": [],
                        "f:mountPath": []
                      }
                    }
                  }
                }
              }
            }
          }
        }
      }
    ]
  },
  "spec": { "replicas": 2 }
}
```

At this point, the values at the following two field paths need to be changed from `"[]"` to `"{}"`:

- `metadata.managedFields.0.fieldsV1.f:spec.f:template.f:spec.f:containers.k:{"name":"test"}.f:volumeMounts.k:{"mountPath":"/test/.probe"}..`
- `metadata.managedFields.0.fieldsV1.f:spec.f:template.f:spec.f:containers.k:{"name":"test"}.f:volumeMounts.k:{"mountPath":"/test/.probe"}.f:mountPath`

### Root Issue

https://github.com/karmada-io/karmada/blob/4594ca0b4b6fcdf0eb3678a0d93896ddc96aaa9a/pkg/resourceinterpreter/customized/declarative/luavm/lua_convert.go#L129-L134

- The order in which these paths are processed is non-deterministic.
- `sjson` treats `.` in a path as a separator.

Therefore:
- `"k:{"mountPath":"/test/.probe"}"` is incorrectly split into two levels: `"k:{"mountPath":"/test/"` and `"probe"}"`.
- The `.` is interpreted as an empty string field name.

#### If the first path is processed first:
1. Due to incorrect parent path parsing, `sjson.Set` does not modify the original field path but instead creates a value `"": {}` under the key `"probe"}"`.
2. `sjson` infers that `"probe"}"` is an array (because it's accessed via a non-named index).
3. When attempting to set `f:mountPath` later, it tries to write to the array using a non-numeric key, resulting in the error:
```bash
cannot set array element for non-numeric key 'f:mountPath'
```

#### If the second path is processed first:
1. Due to incorrect parent path parsing, `sjson.Set` creates a value `"f:mountPath": {}` under `"probe"}"`.
2. `sjson` infers that `"probe"}"` is a struct/object.
3. Subsequently writing `"": {}` succeeds without error.

Thus, this is a **non-deterministic (racy) issue**, depending on the map traversal order.

### Why Does It Eventually Succeed?

The binding controller automatically retries upon failure. As long as one retry happens to process the paths in the correct order, the conversion completes (although the `metadata.managedFields` content may be incorrect at that moment). Moreover, during work generation, the `metadata.managedFields` field is stripped out. Therefore, the final generated work is correct and can be successfully propagated.

## Risk

This issue is not limited to `managedFields`. Any JSON key containing a `.` character could trigger the same problem. For example:

```yaml
apiVersion: apps/v1
kind: ReplicaSet
spec:
  replicas: 3
  test.item: {}
```

After being processed by `sjson.Set`, it may become:

```yaml
apiVersion: apps/v1
kind: ReplicaSet
spec:
    replicas: 2
    test:
        item: {} # Newly created redundant field
    test.item: [] 
```

In such cases, the issue becomes more severe because while `metadata.managedFields` is stripped from the final work, other fields are not.

## Solution

> A path is a series of keys separated by a dot. The dot and colon characters can be escaped with \.

Refer to: https://pkg.go.dev/github.com/tidwall/sjson#section-readme

**Escape the dot characters (`.`) in the path using `\.`** when calling `sjson.Set` to ensure they are treated as literal characters rather than path separators. This will prevent incorrect path parsing and eliminate the issue.@RainbowMango: As we talked at the last community meeting. We are happy to maintain the default interpreter for `ReplicaSet`. After that you don't have to maintain the Lua interpreter anymore.  

@ryanwuer Let me know if you'd like to help. @ryanwuer: > As we talked at the last community meeting. We are happy to maintain the default interpreter for `ReplicaSet`. After that you don't have to maintain the Lua interpreter anymore.
> 
> [@ryanwuer](https://github.com/ryanwuer) Let me know if you'd like to help.

Sure, i'll try to implement this recently. Can you pls create an issue about this?@RainbowMango: OK. Will do after exploring the Argo Rollout feature.@RainbowMango: @ryanwuer Sent the #6794. 
Please assign it to you by leaving a comment on that issue. 

Solution patch:
diff --git a/pkg/resourceinterpreter/customized/declarative/luavm/lua_convert.go b/pkg/resourceinterpreter/customized/declarative/luavm/lua_convert.go
index 36b085dca..8115f0744 100644
--- a/pkg/resourceinterpreter/customized/declarative/luavm/lua_convert.go
+++ b/pkg/resourceinterpreter/customized/declarative/luavm/lua_convert.go
@@ -161,7 +161,7 @@ func traverseToFindEmptyField(root gjson.Result, fieldPath []string) (sets.Set[s
 		// the first element is key=0, value={"ruleName":[]}
 		// however, we expected to record the `ruleName` field path as `rules.ruleName`, rather than `rules.0.ruleName`.
 		if rootIsNotArray {
-			curFieldPath = append(fieldPath, key.String())
+			curFieldPath = append(fieldPath, escapeJSONPathDotAndColon(key.String()))
 		}
 		curFieldStr := strings.Join(curFieldPath, ".")
 
@@ -181,6 +181,16 @@ func traverseToFindEmptyField(root gjson.Result, fieldPath []string) (sets.Set[s
 	return fieldOfEmptySlice, fieldOfEmptyStruct
 }
 
+// escapeJSONPathDotAndColon escape the dot and colon characters in json path key.
+// sjson package use '.' and '.:' to split the json path, so if a json key contains these characters,
+// we need to escape them by '\' first.
+// More details can refer to https://pkg.go.dev/github.com/tidwall/sjson#readme-path-syntax.
+func escapeJSONPathDotAndColon(field string) string {
+	field = strings.ReplaceAll(field, ".", `\.`)
+	field = strings.ReplaceAll(field, ":", `\:`)
+	return field
+}
+
 // traverseToFindEmptyFieldNeededModify find the field with empty values which needed to be modified by traverse a gjson.Result
 //
 // e.g: root={"spec":{"aa":[],"bb":[],"cc":["x"],"dd":{"ee":[],"ff":[]}}}
@@ -208,9 +218,9 @@ func traverseToFindEmptyFieldNeededModify(root gjson.Result, fieldPath, fieldPat
 		// the first element is key=0, value={"ruleName":[]}
 		// we record `rules.ruleName` into curFieldPath, and record `rules.0.ruleName` into curFieldPathWithArrayIndex.
 		if rootIsNotArray {
-			curFieldPath = append(fieldPath, key.String())
+			curFieldPath = append(fieldPath, escapeJSONPathDotAndColon(key.String()))
 		}
-		curFieldPathWithArrayIndex := append(fieldPathWithArrayIndex, key.String())
+		curFieldPathWithArrayIndex := append(fieldPathWithArrayIndex, escapeJSONPathDotAndColon(key.String()))
 
 		if value.IsArray() && len(value.Array()) == 0 {
 			curFieldPathStr := strings.Join(curFieldPath, ".")



Test patch:
diff --git a/pkg/resourceinterpreter/customized/declarative/luavm/lua_convert_test.go b/pkg/resourceinterpreter/customized/declarative/luavm/lua_convert_test.go
index f6461bbc6..597566b6e 100644
--- a/pkg/resourceinterpreter/customized/declarative/luavm/lua_convert_test.go
+++ b/pkg/resourceinterpreter/customized/declarative/luavm/lua_convert_test.go
@@ -325,6 +325,25 @@ func Test_traverseToFindEmptyField(t *testing.T) {
 				fieldOfEmptyStruct: sets.New[string]("spec.aa", "spec.dd.ee"),
 			},
 		},
+		{
+			name: "traverse to find empty field with special characters in keys",
+			args: args{
+				root: gjson.Parse(`{
+					"spec": {
+						"replicas": 3,
+						":colons": {},
+						".dots": {},
+						":.colonAndDot": {},
+						".:dotAndColon": {}
+					}
+				}`),
+				fieldPath: nil,
+			},
+			wants: wants{
+				fieldOfEmptySlice:  sets.New[string](),
+				fieldOfEmptyStruct: sets.New[string](`spec.\:colons`, `spec.\.dots`, `spec.\:\.colonAndDot`, `spec.\.\:dotAndColon`),
+			},
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -374,6 +393,32 @@ func Test_traverseToFindEmptyFieldNeededModify(t *testing.T) {
 				fieldOfEmptySliceToDelete: sets.New[string]("spec.0.dd.ff"),
 			},
 		},
+		{
+			name: "traverse to find empty field needed modify with special characters in keys",
+			args: args{
+				root: gjson.Parse(`{
+					"spec": {
+						"replicas": 3,
+						":colons": [],
+						".dots": [],
+						":.colonAndDot": [],
+						".:dotAndColon": []
+					}
+				}`),
+				fieldPath:               nil,
+				fieldPathWithArrayIndex: nil,
+				fieldOfEmptySlice:       sets.New[string](`spec.\.dots`),
+				fieldOfEmptyStruct:      sets.New[string](`spec.\:colons`, `spec.\:\.colonAndDot`, `spec.\.\:dotAndColon`),
+			},
+			wants: wants{
+				fieldOfEmptySliceToStruct: sets.New[string](
+					`spec.\:colons`,
+					`spec.\:\.colonAndDot`,
+					`spec.\.\:dotAndColon`,
+				),
+				fieldOfEmptySliceToDelete: sets.New[string](),
+			},
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -435,6 +480,34 @@ func Test_convertEmptyObjectBackToEmptySlice(t *testing.T) {
 			wantErr: false,
 			want:    []byte(`[]`),
 		},
+		{
+			name: "convert empty object back to empty slice with special characters in keys",
+			args: args{
+				objBytes: []byte(`{"spec": {"replicas": 3, ":colons": [], ".dots": [], ":.colonAndDot": [], ".:dotAndColon": []}}`),
+				isStruct: true,
+				references: func() []*unstructured.Unstructured {
+					desired := &unstructured.Unstructured{}
+					observed := &unstructured.Unstructured{}
+					_ = json.Unmarshal([]byte(`{
+						"spec": {
+							"replicas": 3,
+							":colons": {},
+							":.colonAndDot": {},
+							".:dotAndColon": {}
+						}
+					}`), desired)
+					_ = json.Unmarshal([]byte(`{
+						"spec": {
+							"replicas": 3,
+							".dots": []
+						}
+					}`), observed)
+					return []*unstructured.Unstructured{desired, observed}
+				}(),
+			},
+			wantErr: false,
+			want:    []byte(`{"spec": {"replicas": 3, ":colons": {}, ".dots": [], ":.colonAndDot": {}, ".:dotAndColon": {}}}`),
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {