fix: honor init container order in spec.deployment.patch [RHDHBUGS-2900]

docs/configuration.md

@@ -837,6 +837,45 @@ spec:
                   claimName: dynamic-plugins-root
 ```
 
+##### List ordering
+
+When the patch introduces **new** list items (containers, init containers, volumes, etc.), they are **prepended** before the existing items in the default configuration. Items that match an existing entry by name are **merged in-place** and retain their original position.
+
+This is particularly relevant for init containers, where execution order [matters](https://kubernetes.io/docs/concepts/workloads/pods/init-containers/#detailed-behavior). For example, if the default configuration defines an `install-dynamic-plugins` init container, patching in a custom init container will place it **before** `install-dynamic-plugins`:
+
+```yaml
+spec:
+  deployment:
+    patch:
+      spec:
+        template:
+          spec:
+            initContainers:
+              - name: my-init
+                image: busybox
+                command: ["sh", "-c", "echo preparing"]
+```
+
+The resulting init container order will be: `my-init`, then `install-dynamic-plugins`.
+
+To place a custom init container **after** an existing one, reference the existing container by name in the patch before your custom entry:
+
+```yaml
+spec:
+  deployment:
+    patch:
+      spec:
+        template:
+          spec:
+            initContainers:
+              - name: install-dynamic-plugins
+              - name: my-init
+                image: busybox
+                command: ["sh", "-c", "echo preparing"]
+```
+
+The resulting init container order will be: `install-dynamic-plugins`, then `my-init`. Listing `install-dynamic-plugins` by name (without any other fields) anchors it in position, and new items listed after it are placed accordingly.
+
 ##### Handling Discriminated Unions
 
 When patching Kubernetes resources that contain **discriminated unions** (fields where one field determines which other fields are valid), you may need to use the `$patch: delete` directive to remove conflicting fields.

pkg/model/deployment.go

@@ -227,7 +227,20 @@
 				return fmt.Errorf("can not marshal deployment object: %w", err)
 			}
 
-			merged, err := merge2.MergeStrings(string(conf.Raw), string(deplStr), false, kyaml.MergeOptions{})
+			// TODO(asoro): once https://github.com/kubernetes-sigs/kustomize/issues/6146 is resolved,
+			// remove this two-pass merge and use only ListPrepend.
+			//
+			// RHDHBUGS-2900: Two-pass merge to work around a kyaml bug where ListPrepend
+			// silently breaks $patch directives (e.g. $patch: replace).
+			// Pass 1: ListPrepend to get correct ordering of new list items.
+			// Pass 2: default options to apply $patch directives. All items
+			// already exist from pass 1 so positions are preserved.
+			merged, err := merge2.MergeStrings(string(conf.Raw), string(deplStr), false, kyaml.MergeOptions{ListIncreaseDirection: kyaml.MergeOptionsListPrepend})
+			if err != nil {
+				return fmt.Errorf("can not merge spec.deployment: %w", err)
+			}
+
+			merged, err = merge2.MergeStrings(string(conf.Raw), merged, false, kyaml.MergeOptions{})
 			if err != nil {
 				return fmt.Errorf("can not merge spec.deployment: %w", err)
 			}

pkg/model/deployment_test.go

@@ -140,23 +140,112 @@ spec:
 	assert.Equal(t, "java", model.backstageDeployment.deployable.GetObject().GetLabels()["mylabel"])
 	assert.Equal(t, "backstage", model.backstageDeployment.deployable.PodObjectMeta().GetLabels()["pod"])
 
-	// sidecar added
+	// sidecar prepended
 	assert.Equal(t, 2, len(model.backstageDeployment.podSpec().Containers))
-	assert.Equal(t, "sidecar", model.backstageDeployment.podSpec().Containers[1].Name)
-	assert.Equal(t, "my-image:1.0.0", model.backstageDeployment.podSpec().Containers[1].Image)
+	assert.Equal(t, "sidecar", model.backstageDeployment.podSpec().Containers[0].Name)
+	assert.Equal(t, "my-image:1.0.0", model.backstageDeployment.podSpec().Containers[0].Image)
 
 	// backstage container resources updated
 	assert.Equal(t, "backstage-backend", model.backstageDeployment.container().Name)
 	assert.Equal(t, "257Mi", model.backstageDeployment.container().Resources.Requests.Memory().String())
 
-	// volumes
-	// dynamic-plugins-root, dynamic-plugins-npmrc, dynamic-plugins-auth, my-vol
+	// volumes: dynamic-plugins-root (merged in-place), my-vol (new), dynamic-plugins-npmrc, dynamic-plugins-registry-auth
 	assert.Equal(t, 4, len(model.backstageDeployment.podSpec().Volumes))
 	assert.Equal(t, "dynamic-plugins-root", model.backstageDeployment.podSpec().Volumes[0].Name)
 	// overrides StorageClassName
 	assert.Equal(t, "special", *model.backstageDeployment.podSpec().Volumes[0].Ephemeral.VolumeClaimTemplate.Spec.StorageClassName)
-	// adds new volume
-	assert.Equal(t, "my-vol", model.backstageDeployment.podSpec().Volumes[3].Name)
+	// new volume added
+	assert.Equal(t, "my-vol", model.backstageDeployment.podSpec().Volumes[1].Name)
+}
+
+// https://redhat.atlassian.net/browse/RHDHBUGS-2900
+func TestInitContainerOrderInSpecDeployment(t *testing.T) {
+	tests := []struct {
+		name     string
+		patch    string
+		expected []string
+	}{
+		{
+			name: "new init container runs before existing",
+			patch: `
+spec:
+ template:
+   spec:
+     initContainers:
+       - name: my-init
+         image: busybox
+         command: ["sh", "-c", "echo init"]
+`,
+			expected: []string{"my-init", "install-dynamic-plugins"},
+		},
+		{
+			name: "new init container runs before existing by anchoring",
+			patch: `
+spec:
+ template:
+   spec:
+     initContainers:
+       - name: my-init
+         image: busybox
+         command: ["sh", "-c", "echo init"]
+       - name: install-dynamic-plugins
+`,
+			expected: []string{"my-init", "install-dynamic-plugins"},
+		},
+		{
+			name: "new init container runs after existing by anchoring",
+			patch: `
+spec:
+ template:
+   spec:
+     initContainers:
+       - name: install-dynamic-plugins
+       - name: my-init
+         image: busybox
+         command: ["sh", "-c", "echo init"]
+`,
+			expected: []string{"install-dynamic-plugins", "my-init"},
+		},
+		{
+			name: "multiple new init containers with mixed ordering",
+			patch: `
+spec:
+ template:
+   spec:
+     initContainers:
+       - name: pre-init
+         image: busybox
+         command: ["sh", "-c", "echo pre"]
+       - name: install-dynamic-plugins
+       - name: post-init
+         image: busybox
+         command: ["sh", "-c", "echo post"]
+`,
+			expected: []string{"pre-init", "install-dynamic-plugins", "post-init"},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			bs := *deploymentTestBackstage.DeepCopy()
+			bs.Spec.Deployment = &api.BackstageDeployment{}
+			bs.Spec.Deployment.Patch = &apiextensionsv1.JSON{
+				Raw: []byte(tt.patch),
+			}
+
+			testObj := createBackstageTest(bs).withDefaultConfig(true).
+				addToDefaultConfig("deployment.yaml", "rhdh-deployment.yaml")
+
+			model, err := InitObjects(context.TODO(), bs, testObj.externalConfig, platform.OpenShift, testObj.scheme)
+			assert.NoError(t, err)
+
+			initContainers := model.backstageDeployment.podSpec().InitContainers
+			assert.Equal(t, len(tt.expected), len(initContainers))
+			for i, name := range tt.expected {
+				assert.Equal(t, name, initContainers[i].Name)
+			}
+		})
+	}
 }
 
 func TestImageInCRPrevailsOnEnvVar(t *testing.T) {