Merge pull request #282 from wking/resource-merge-index-mutating-exis…

…ting Bug 1783221: lib/resourcemerge/core: Fix panic on container/port removal
openshift · Feb 4, 2020 · abfb48d · abfb48d
2 parents 54faf6f + e8e80c2
commit abfb48d
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 2 deletions.
diff --git a/lib/resourcemerge/core.go b/lib/resourcemerge/core.go
@@ -62,7 +62,8 @@ func ensurePodSpec(modified *bool, existing *corev1.PodSpec, required corev1.Pod
 }
 
 func ensureContainers(modified *bool, existing *[]corev1.Container, required []corev1.Container) {
-	for i, existingContainer := range *existing {
+	for i := len(*existing) - 1; i >= 0; i-- {
+		existingContainer := &(*existing)[i]
 		var existingCurr *corev1.Container
 		for _, requiredContainer := range required {
 			if existingContainer.Name == requiredContainer.Name {
@@ -198,7 +199,8 @@ func ensureContainerPort(modified *bool, existing *corev1.ContainerPort, require
 }
 
 func EnsureServicePorts(modified *bool, existing *[]corev1.ServicePort, required []corev1.ServicePort) {
-	for i, existingServicePort := range *existing {
+	for i := len(*existing) - 1; i >= 0; i-- {
+		existingServicePort := &(*existing)[i]
 		var existingCurr *corev1.ServicePort
 		for _, requiredServicePort := range required {
 			if existingServicePort.Name == requiredServicePort.Name {

diff --git a/lib/resourcemerge/core_test.go b/lib/resourcemerge/core_test.go
@@ -247,6 +247,27 @@ func TestEnsurePodSpec(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "remove a container",
+			existing: corev1.PodSpec{
+				Containers: []corev1.Container{
+					corev1.Container{Name: "test-A"},
+					corev1.Container{Name: "test-B"},
+				},
+			},
+			input: corev1.PodSpec{
+				Containers: []corev1.Container{
+					corev1.Container{Name: "test-B"},
+				},
+			},
+
+			expectedModified: true,
+			expected: corev1.PodSpec{
+				Containers: []corev1.Container{
+					corev1.Container{Name: "test-B"},
+				},
+			},
+		},
 	}
 
 	for _, test := range tests {