From de1b8e076a3f25429fb10734edd455432440be46 Mon Sep 17 00:00:00 2001
From: Lalit Chauhan <lalitchauhan@google.com>
Date: Tue, 2 Dec 2025 23:27:43 +0000
Subject: [PATCH] Fix MAP failure on objects with duplicate list items

When a Mutating Admission Policy (MAP) using ApplyConfiguration was applied to an existing object containing duplicate items in a list (e.g., duplicate environment variables), the operation would fail with a conversion error from Structured Merge Diff.

This change updates the `ApplyStructuredMergeDiff` function to use `typed.AllowDuplicates` when converting the `originalObject`. This allows MAP to process existing objects that may have technically invalid (duplicate) entries but are permitted by legacy validation.

New patches generated by the policy are still strictly validated and cannot introduce new duplicates.

Tests have been added to verify:
1. MAP can process an object with duplicate env vars.
2. MAP can modify an existing env var in an object with duplicates (which has the side effect of deduplicating the entry).
---
 .../plugin/policy/mutating/patch/smd.go       |   2 +-
 .../plugin/policy/mutating/patch/smd_test.go  | 101 ++++++++++++++++++
 2 files changed, 102 insertions(+), 1 deletion(-)

diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/mutating/patch/smd.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/mutating/patch/smd.go
index 3be79df54ea..7135aca3a25 100644
--- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/mutating/patch/smd.go
+++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/mutating/patch/smd.go
@@ -138,7 +138,7 @@ func ApplyStructuredMergeDiff(
 		return nil, fmt.Errorf("invalid ApplyConfiguration: %w", err)
 	}
 
-	liveObjTyped, err := typeConverter.ObjectToTyped(originalObject)
+	liveObjTyped, err := typeConverter.ObjectToTyped(originalObject, typed.AllowDuplicates)
 	if err != nil {
 		return nil, fmt.Errorf("failed to convert original object to typed object: %w", err)
 	}
diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/mutating/patch/smd_test.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/mutating/patch/smd_test.go
index e4e3810a0d6..3f0b87233dc 100644
--- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/mutating/patch/smd_test.go
+++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/mutating/patch/smd_test.go
@@ -233,6 +233,107 @@ func TestApplyConfiguration(t *testing.T) {
 			object:      &appsv1.Deployment{Spec: appsv1.DeploymentSpec{Replicas: ptr.To[int32](1)}},
 			expectedErr: "must evaluate to Object but got Object.spec.metadata",
 		},
+		{
+			name: "apply configuration with duplicate env vars in original object",
+			expression: `Object{
+					spec: Object.spec{
+						replicas: 3
+					}
+				}`,
+			gvr: deploymentGVR,
+			object: &appsv1.Deployment{
+				Spec: appsv1.DeploymentSpec{
+					Replicas: ptr.To[int32](1),
+					Template: corev1.PodTemplateSpec{
+						Spec: corev1.PodSpec{
+							Containers: []corev1.Container{{
+								Name: "nginx",
+								Env: []corev1.EnvVar{
+									{Name: "test", Value: "a"},
+									{Name: "test", Value: "b"},
+								},
+							}},
+						},
+					},
+				},
+			},
+			expectedResult: &appsv1.Deployment{
+				Spec: appsv1.DeploymentSpec{
+					Replicas: ptr.To[int32](3),
+					Template: corev1.PodTemplateSpec{
+						Spec: corev1.PodSpec{
+							Containers: []corev1.Container{{
+								Name: "nginx",
+								Env: []corev1.EnvVar{
+									{Name: "test", Value: "a"},
+									{Name: "test", Value: "b"},
+								},
+							}},
+						},
+					},
+				},
+			},
+		},
+		// This test verifies that modifying an existing environment variable works correctly,
+		// even if the original object contains duplicates.
+		// Because 'env' is defined with `listType=map` and `listMapKey=name` in the schema,
+		// Structured Merge Diff (SMD) treats 'name' as the unique key.
+		// When the patch updates the entry with name="test", SMD merges the changes.
+		// As a side effect of the merge process on a map-type list, duplicate entries for the
+		// same key in the original object are consolidated into a single entry in the result.
+		// This matches the behavior of Server-Side Apply (SSA) and kubectl apply.
+		{
+			name: "apply configuration modify existing env variable",
+			expression: `Object{
+					spec: Object.spec{
+						template: Object.spec.template{
+							spec: Object.spec.template.spec{
+								containers: [Object.spec.template.spec.containers{
+									name: "nginx",
+									env: [Object.spec.template.spec.containers.env{
+										name: "test",
+										value: "c"
+									}]
+								}]
+							}
+						}
+					}
+				}`,
+			gvr: deploymentGVR,
+			object: &appsv1.Deployment{
+				Spec: appsv1.DeploymentSpec{
+					Replicas: ptr.To[int32](1),
+					Template: corev1.PodTemplateSpec{
+						Spec: corev1.PodSpec{
+							Containers: []corev1.Container{{
+								Name: "nginx",
+								Env: []corev1.EnvVar{
+									{Name: "test", Value: "a"},
+									{Name: "test", Value: "b"},
+									{Name: "foo", Value: "bar"},
+								},
+							}},
+						},
+					},
+				},
+			},
+			expectedResult: &appsv1.Deployment{
+				Spec: appsv1.DeploymentSpec{
+					Replicas: ptr.To[int32](1),
+					Template: corev1.PodTemplateSpec{
+						Spec: corev1.PodSpec{
+							Containers: []corev1.Container{{
+								Name: "nginx",
+								Env: []corev1.EnvVar{
+									{Name: "test", Value: "c"},
+									{Name: "foo", Value: "bar"},
+								},
+							}},
+						},
+					},
+				},
+			},
+		},
 	}
 
 	compiler, err := cel.NewCompositedCompiler(environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion()))