Repository: kubernetes/kubernetes (language: go)

Issue:
Server-side apply has trouble with empty map
### What happened?

Every time I server-side apply a `Deployment` with `spec.template.metadata.annotations: {}`, the `resourceVersion` updates. Nothing about my manifest changes between applies; if I map `annotations` to the empty map, `kubectl apply --server-side` the same manifest updates the `resourceVersion`. With client-side apply, the `resourceVersion` stays the same as long as my manifest remains constant, even if `annotations: {}`.

I read in #70281 that Kubernetes should treat absent keys, empty values (in this case, an empty map), and `null` values the same. I observe that leaving the `annotations` key out and `annotations: null` behave as expected, not modifying the `Deployment` and leaving `resourceVersion` the same, but empty map `{}` behaves as described above.

If we cannot change this behavior, I'd at least like to understand the rationale behind it.

### What did you expect to happen?

I expected my `Deployment` object to stay the same between identical server-side applies, where the manifest file content was exactly the same, regardless of the value of `spec.template.metadata.annotations`. A different `resourceVersion` every apply indicates that the object was event type `MODIFIED`. I expect no events to fire on applying the same manifest.

### How can we reproduce it (as minimally and precisely as possible)?

1. Say `deployment.yaml` contains https://kubernetes.io/docs/concepts/workloads/controllers/deployment/#creating-a-deployment. In actuality, any `Deployment` is fine.
2. Add `spec.template.metadata.annotatations: {}`.
3. `kubectl apply --server-side deployment.yaml`
4. `kubectl get deployment nginx-deployment -o yaml`

Every successive repeat of steps 3-4 should show different `metadata.resourceVersion` values.

Example of `Deployment` with `spec.template.metadata.annotatations: {}`:
```
apiVersion: apps/v1
kind: Deployment
metadata:
  name: nginx-deployment
  labels:
    app: nginx
spec:
  replicas: 3
  selector:
    matchLabels:
      app: nginx
  template:
    metadata:
      annotations: {}
      labels:
        app: nginx
    spec:
      containers:
      - name: nginx
        image: nginx:1.14.2
        ports:
        - containerPort: 80
```

### Anything else we need to know?

This is especially a problem when I server-side apply with code in my operator. My operator is built on https://github.com/kubernetes-sigs/kubebuilder-declarative-pattern and watches changes to the child objects it deploys. If I create a `Deployment` with `spec.template.metadata.annotations: {}`, the operator constantly reconciles because every server-side apply of the manifest registers as a type `MODIFIED` event on the object, which triggers the watching operator to reconcile.

### Kubernetes version

```console
$ kubectl version --short
Client Version: v1.27.11
Kustomize Version: v5.0.1
Server Version: v1.27.8-gke.1067004
```


### Cloud provider

N/A

### OS version

<details>

```console
# On Linux:
$ cat /etc/os-release
ID=debian
$ uname -a
# paste output here
```

</details>


### Install tools

<details>

</details>


### Container runtime (CRI) and version (if applicable)

<details>

</details>


### Related plugins (CNI, CSI, ...) and versions (if applicable)

<details>

</details>



Discussion / hints:
/sig api-machinery

/cc @Jefftree 
/triage accepted
Potentially similar case as #124605

Fix patch:
diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/equality.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/equality.go
index a1f27f1d106e4..2f0bb4e484155 100644
--- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/equality.go
+++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/equality.go
@@ -28,6 +28,7 @@ import (
 	"k8s.io/apimachinery/pkg/api/equality"
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/conversion"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apiserver/pkg/endpoints/metrics"
@@ -152,14 +153,20 @@ func IgnoreManagedFieldsTimestampsTransformer(
 		return newObj, nil
 	}
 
+	eqFn := equalities.DeepEqual
+	if _, ok := newObj.(*unstructured.Unstructured); ok {
+		// Use strict equality with unstructured
+		eqFn = equalities.DeepEqualWithNilDifferentFromEmpty
+	}
+
 	// This condition ensures the managed fields are always compared first. If
 	//	this check fails, the if statement will short circuit. If the check
 	// 	succeeds the slow path is taken which compares entire objects.
-	if !equalities.DeepEqualWithNilDifferentFromEmpty(oldManagedFields, newManagedFields) {
+	if !eqFn(oldManagedFields, newManagedFields) {
 		return newObj, nil
 	}
 
-	if equalities.DeepEqualWithNilDifferentFromEmpty(newObj, oldObj) {
+	if eqFn(newObj, oldObj) {
 		// Remove any changed timestamps, so that timestamp is not the only
 		// change seen by etcd.
 		//


Test patch:
diff --git a/test/integration/apiserver/apply/apply_test.go b/test/integration/apiserver/apply/apply_test.go
index 735498b2fcf80..ecd4060583b32 100644
--- a/test/integration/apiserver/apply/apply_test.go
+++ b/test/integration/apiserver/apply/apply_test.go
@@ -244,6 +244,237 @@ func TestNoOpUpdateSameResourceVersion(t *testing.T) {
 	}
 }
 
+// TestNoOpApplyWithEmptyMap
+func TestNoOpApplyWithEmptyMap(t *testing.T) {
+	client, closeFn := setup(t)
+	defer closeFn()
+
+	deploymentName := "no-op"
+	deploymentsResource := "deployments"
+	deploymentBytes := []byte(`{
+		"apiVersion": "apps/v1",
+		"kind": "Deployment",
+		"metadata": {
+			"name": "` + deploymentName + `",
+			"labels": {
+				"app": "nginx"
+			}
+		},
+		"spec": {
+			"replicas": 1,
+			"selector": {
+				"matchLabels": {
+					"app": "nginx"
+				}
+			},
+			"template": {
+				"metadata": {
+					"annotations": {},
+					"labels": {
+						"app": "nginx"
+					}
+				},
+				"spec": {
+					"containers": [{
+						"name": "nginx",
+						"image": "nginx:1.14.2",
+						"ports": [{
+							"containerPort": 80
+						}]
+					}]
+				}
+			}
+		}
+	}`)
+
+	_, err := client.AppsV1().RESTClient().Patch(types.ApplyPatchType).
+		Namespace("default").
+		Param("fieldManager", "apply_test").
+		Resource(deploymentsResource).
+		Name(deploymentName).
+		Body(deploymentBytes).
+		Do(context.TODO()).
+		Get()
+	if err != nil {
+		t.Fatalf("Failed to create object: %v", err)
+	}
+
+	// This sleep is necessary to consistently produce different timestamps because the time field in managedFields has
+	// 1 second granularity and if both apply requests happen during the same second, this test would flake.
+	time.Sleep(1 * time.Second)
+
+	createdObject, err := client.AppsV1().RESTClient().Get().Namespace("default").Resource(deploymentsResource).Name(deploymentName).Do(context.TODO()).Get()
+	if err != nil {
+		t.Fatalf("Failed to retrieve created object: %v", err)
+	}
+
+	createdAccessor, err := meta.Accessor(createdObject)
+	if err != nil {
+		t.Fatalf("Failed to get meta accessor for created object: %v", err)
+	}
+
+	createdBytes, err := json.MarshalIndent(createdObject, "\t", "\t")
+	if err != nil {
+		t.Fatalf("Failed to marshal created object: %v", err)
+	}
+
+	// Test that we can apply the same object and don't change the RV
+	_, err = client.AppsV1().RESTClient().Patch(types.ApplyPatchType).
+		Namespace("default").
+		Param("fieldManager", "apply_test").
+		Resource(deploymentsResource).
+		Name(deploymentName).
+		Body(deploymentBytes).
+		Do(context.TODO()).
+		Get()
+	if err != nil {
+		t.Fatalf("Failed to create object: %v", err)
+	}
+
+	updatedObject, err := client.AppsV1().RESTClient().Get().Namespace("default").Resource(deploymentsResource).Name(deploymentName).Do(context.TODO()).Get()
+	if err != nil {
+		t.Fatalf("Failed to retrieve updated object: %v", err)
+	}
+
+	updatedAccessor, err := meta.Accessor(updatedObject)
+	if err != nil {
+		t.Fatalf("Failed to get meta accessor for updated object: %v", err)
+	}
+
+	updatedBytes, err := json.MarshalIndent(updatedObject, "\t", "\t")
+	if err != nil {
+		t.Fatalf("Failed to marshal updated object: %v", err)
+	}
+
+	if createdAccessor.GetResourceVersion() != updatedAccessor.GetResourceVersion() {
+		t.Fatalf("Expected same resource version to be %v but got: %v\nold object:\n%v\nnew object:\n%v",
+			createdAccessor.GetResourceVersion(),
+			updatedAccessor.GetResourceVersion(),
+			string(createdBytes),
+			string(updatedBytes),
+		)
+	}
+}
+
+// TestApplyEmptyMarkerStructDifferentFromNil
+func TestApplyEmptyMarkerStructDifferentFromNil(t *testing.T) {
+	client, closeFn := setup(t)
+	defer closeFn()
+
+	podName := "pod-with-empty-dir"
+	podsResource := "pods"
+	podBytesWithEmptyDir := []byte(`{
+		"apiVersion": "v1",
+		"kind": "Pod",
+		"metadata": {
+			"name": "` + podName + `"
+		},
+		"spec": {
+			"containers": [{
+				"name":  "test-container-a",
+				"image": "test-image-one",
+				"volumeMounts": [{
+					"mountPath": "/cache",
+					"name": "cache-volume"
+				}],
+			}],
+			"volumes": [{
+				"name": "cache-volume",
+				"emptyDir": {}
+			}]
+		}
+	}`)
+
+	_, err := client.CoreV1().RESTClient().Patch(types.ApplyPatchType).
+		Namespace("default").
+		Param("fieldManager", "apply_test").
+		Resource(podsResource).
+		Name(podName).
+		Body(podBytesWithEmptyDir).
+		Do(context.TODO()).
+		Get()
+	if err != nil {
+		t.Fatalf("Failed to create object: %v", err)
+	}
+
+	// This sleep is necessary to consistently produce different timestamps because the time field in managedFields has
+	// 1 second granularity and if both apply requests happen during the same second, this test would flake.
+	time.Sleep(1 * time.Second)
+
+	createdObject, err := client.CoreV1().RESTClient().Get().Namespace("default").Resource(podsResource).Name(podName).Do(context.TODO()).Get()
+	if err != nil {
+		t.Fatalf("Failed to retrieve created object: %v", err)
+	}
+
+	createdAccessor, err := meta.Accessor(createdObject)
+	if err != nil {
+		t.Fatalf("Failed to get meta accessor for created object: %v", err)
+	}
+
+	createdBytes, err := json.MarshalIndent(createdObject, "\t", "\t")
+	if err != nil {
+		t.Fatalf("Failed to marshal created object: %v", err)
+	}
+
+	podBytesNoEmptyDir := []byte(`{
+		"apiVersion": "v1",
+		"kind": "Pod",
+		"metadata": {
+			"name": "` + podName + `"
+		},
+		"spec": {
+			"containers": [{
+				"name":  "test-container-a",
+				"image": "test-image-one",
+				"volumeMounts": [{
+					"mountPath": "/cache",
+					"name": "cache-volume"
+				}],
+			}],
+			"volumes": [{
+				"name": "cache-volume"
+			}]
+		}
+	}`)
+
+	// Test that an apply with no emptyDir is recognized as distinct from an empty marker struct emptyDir.
+	_, err = client.CoreV1().RESTClient().Patch(types.ApplyPatchType).
+		Namespace("default").
+		Param("fieldManager", "apply_test").
+		Resource(podsResource).
+		Name(podName).
+		Body(podBytesNoEmptyDir).
+		Do(context.TODO()).
+		Get()
+	if err != nil {
+		t.Fatalf("Failed to create object: %v", err)
+	}
+
+	updatedObject, err := client.CoreV1().RESTClient().Get().Namespace("default").Resource(podsResource).Name(podName).Do(context.TODO()).Get()
+	if err != nil {
+		t.Fatalf("Failed to retrieve updated object: %v", err)
+	}
+
+	updatedAccessor, err := meta.Accessor(updatedObject)
+	if err != nil {
+		t.Fatalf("Failed to get meta accessor for updated object: %v", err)
+	}
+
+	updatedBytes, err := json.MarshalIndent(updatedObject, "\t", "\t")
+	if err != nil {
+		t.Fatalf("Failed to marshal updated object: %v", err)
+	}
+
+	if createdAccessor.GetResourceVersion() == updatedAccessor.GetResourceVersion() {
+		t.Fatalf("Expected different resource version to be %v but got: %v\nold object:\n%v\nnew object:\n%v",
+			createdAccessor.GetResourceVersion(),
+			updatedAccessor.GetResourceVersion(),
+			string(createdBytes),
+			string(updatedBytes),
+		)
+	}
+}
+
 func getRV(obj runtime.Object) (string, error) {
 	acc, err := meta.Accessor(obj)
 	if err != nil {