Support patch directives in pod/container overrides

pkg/library/overrides/containers.go

@@ -33,19 +33,18 @@ func ApplyContainerOverrides(component *dw.Component, container *corev1.Containe
 	if err := component.Attributes.GetInto(constants.ContainerOverridesAttribute, override); err != nil {
 		return nil, fmt.Errorf("failed to parse %s attribute on component %s: %w", constants.ContainerOverridesAttribute, component.Name, err)
 	}
-	override = restrictContainerOverride(override)
-
-	overrideBytes, err := json.Marshal(override)
-	if err != nil {
-		return nil, fmt.Errorf("error applying container overrides: %w", err)
+	if err := restrictContainerOverride(override); err != nil {
+		return nil, fmt.Errorf("failed to parse %s attribute on component %s: %w", constants.ContainerOverridesAttribute, component.Name, err)
 	}
 
+	overrideJSON := component.Attributes[constants.ContainerOverridesAttribute]
+
 	originalBytes, err := json.Marshal(container)
 	if err != nil {
 		return nil, fmt.Errorf("failed to marshal container to yaml: %w", err)
 	}
 
-	patchedBytes, err := strategicpatch.StrategicMergePatch(originalBytes, overrideBytes, &corev1.Container{})
+	patchedBytes, err := strategicpatch.StrategicMergePatch(originalBytes, overrideJSON.Raw, &corev1.Container{})
 	if err != nil {
 		return nil, fmt.Errorf("failed to apply container overrides: %w", err)
 	}
@@ -54,24 +53,41 @@ func ApplyContainerOverrides(component *dw.Component, container *corev1.Containe
 	if err := json.Unmarshal(patchedBytes, patched); err != nil {
 		return nil, fmt.Errorf("error applying container overrides: %w", err)
 	}
-	// Applying the patch will overwrite the container's name as corev1.Container.Name
+	// Applying the patch will overwrite the container's name and image as corev1.Container.Name
 	// does not have the omitempty json tag.
 	patched.Name = container.Name
+	patched.Image = container.Image
 	return patched, nil
 }
 
 // restrictContainerOverride unsets fields on a container that should not be
 // considered for container overrides. These fields are generally available to
 // set as fields on the container component itself.
-func restrictContainerOverride(override *corev1.Container) *corev1.Container {
-	result := override.DeepCopy()
-	result.Name = ""
-	result.Image = ""
-	result.Command = nil
-	result.Args = nil
-	result.Ports = nil
-	result.VolumeMounts = nil
-	result.Env = nil
-
-	return result
+func restrictContainerOverride(override *corev1.Container) error {
+	invalidField := ""
+	if override.Name != "" {
+		invalidField = "name"
+	}
+	if override.Image != "" {
+		invalidField = "image"
+	}
+	if override.Command != nil {
+		invalidField = "command"
+	}
+	if override.Args != nil {
+		invalidField = "args"
+	}
+	if override.Ports != nil {
+		invalidField = "ports"
+	}
+	if override.VolumeMounts != nil {
+		invalidField = "volumeMounts"
+	}
+	if override.Env != nil {
+		invalidField = "env"
+	}
+	if invalidField != "" {
+		return fmt.Errorf("cannot use container-overrides to override container %s", invalidField)
+	}
+	return nil
 }

pkg/library/overrides/pods.go

@@ -18,6 +18,7 @@ import (
 
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
+	apiext "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	"k8s.io/apimachinery/pkg/util/json"
 	"k8s.io/apimachinery/pkg/util/strategicpatch"
 
@@ -50,17 +51,15 @@ func ApplyPodOverrides(workspace *common.DevWorkspaceWithConfig, deployment *app
 	// will be interpreted as "delete all containers" as the serialized patch will include "containers": null.
 	// To avoid this, save the original containers and reset them at the end.
 	originalContainers := patched.Spec.Template.Spec.Containers
+	// Save fields we do not allow to be configured in pod-overrides
+	originalInitContainers := patched.Spec.Template.Spec.InitContainers
+	originalVolumes := patched.Spec.Template.Spec.Volumes
 	patchedTemplateBytes, err := json.Marshal(patched.Spec.Template)
 	if err != nil {
 		return nil, fmt.Errorf("failed to marshal deployment to yaml: %w", err)
 	}
 	for _, override := range overrides {
-		patchBytes, err := json.Marshal(override)
-		if err != nil {
-			return nil, fmt.Errorf("error applying pod overrides: %w", err)
-		}
-
-		patchedTemplateBytes, err = strategicpatch.StrategicMergePatch(patchedTemplateBytes, patchBytes, &corev1.PodTemplateSpec{})
+		patchedTemplateBytes, err = strategicpatch.StrategicMergePatch(patchedTemplateBytes, override.Raw, &corev1.PodTemplateSpec{})
 		if err != nil {
 			return nil, fmt.Errorf("error applying pod overrides: %w", err)
 		}
@@ -72,28 +71,37 @@ func ApplyPodOverrides(workspace *common.DevWorkspaceWithConfig, deployment *app
 	}
 	patched.Spec.Template = patchedPodSpecTemplate
 	patched.Spec.Template.Spec.Containers = originalContainers
+	patched.Spec.Template.Spec.InitContainers = originalInitContainers
+	patched.Spec.Template.Spec.Volumes = originalVolumes
 	return patched, nil
 }
 
 // getPodOverrides returns PodTemplateSpecOverrides for every instance of the pod overrides attribute
 // present in the DevWorkspace. The order of elements is
 // 1. Pod overrides defined on Container components, in the order they appear in the DevWorkspace
 // 2. Pod overrides defined in the global attributes field (.spec.template.attributes)
-func getPodOverrides(workspace *common.DevWorkspaceWithConfig) ([]corev1.PodTemplateSpec, error) {
-	var allOverrides []corev1.PodTemplateSpec
+func getPodOverrides(workspace *common.DevWorkspaceWithConfig) ([]apiext.JSON, error) {
+	var allOverrides []apiext.JSON
 
 	for _, component := range workspace.Spec.Template.Components {
 		if component.Attributes.Exists(constants.PodOverridesAttribute) {
 			override := corev1.PodTemplateSpec{}
-			err := component.Attributes.GetInto(constants.PodOverridesAttribute, &override)
-			if err != nil {
+			// Check format of pod-overrides to detect errors early
+			if err := component.Attributes.GetInto(constants.PodOverridesAttribute, &override); err != nil {
 				return nil, fmt.Errorf("failed to parse %s attribute on component %s: %w", constants.PodOverridesAttribute, component.Name, err)
 			}
-			// Do not allow overriding containers
-			override.Spec.Containers = nil
-			override.Spec.InitContainers = nil
-			override.Spec.Volumes = nil
-			allOverrides = append(allOverrides, override)
+			// Do not allow overriding containers or volumes
+			if override.Spec.Containers != nil {
+				return nil, fmt.Errorf("cannot use pod-overrides to override pod containers (component %s)", component.Name)
+			}
+			if override.Spec.InitContainers != nil {
+				return nil, fmt.Errorf("cannot use pod-overrides to override pod initContainers (component %s)", component.Name)
+			}
+			if override.Spec.Volumes != nil {
+				return nil, fmt.Errorf("cannot use pod-overrides to override pod volumes (component %s)", component.Name)
+			}
+			patchData := component.Attributes[constants.PodOverridesAttribute]
+			allOverrides = append(allOverrides, patchData)
 		}
 	}
 	if workspace.Spec.Template.Attributes.Exists(constants.PodOverridesAttribute) {
@@ -103,10 +111,17 @@ func getPodOverrides(workspace *common.DevWorkspaceWithConfig) ([]corev1.PodTemp
 			return nil, fmt.Errorf("failed to parse %s attribute for workspace: %w", constants.PodOverridesAttribute, err)
 		}
 		// Do not allow overriding containers or volumes
-		override.Spec.Containers = nil
-		override.Spec.InitContainers = nil
-		override.Spec.Volumes = nil
-		allOverrides = append(allOverrides, override)
+		if override.Spec.Containers != nil {
+			return nil, fmt.Errorf("cannot use pod-overrides to override pod containers")
+		}
+		if override.Spec.InitContainers != nil {
+			return nil, fmt.Errorf("cannot use pod-overrides to override pod initContainers")
+		}
+		if override.Spec.Volumes != nil {
+			return nil, fmt.Errorf("cannot use pod-overrides to override pod volumes")
+		}
+		patchData := workspace.Spec.Template.Attributes[constants.PodOverridesAttribute]
+		allOverrides = append(allOverrides, patchData)
 	}
 	return allOverrides, nil
 }

pkg/library/overrides/testdata/container-overrides/container-cannot-set-restricted-fields.yaml

@@ -36,17 +36,4 @@ input:
 
 
 output:
-  container:
-    name: test-component
-    image: test-image
-    command: ["original"]
-    args: ["original"]
-    ports:
-      - name: original-port
-        containerPort: 8080
-    volumeMounts:
-      - name: original-volume
-        mountPath: original-mountPath
-    env:
-      - name: original_env
-        value: original_val
+  errRegexp: "cannot use container-overrides to override container env"

pkg/library/overrides/testdata/container-overrides/overrides-can-override-securityContext.yaml

@@ -0,0 +1,26 @@
+name: "container overrides can override securityContext"
+
+input:
+  component:
+    name: test-component
+    attributes:
+      container-overrides:
+        securityContext:
+          runAsUser: 1001
+    container:
+      image: test-image
+
+  container:
+    name: test-component
+    image: test-image
+    securityContext:
+      runAsUser: 1000
+      runAsGroup: 2000
+
+output:
+  container:
+    name: test-component
+    image: test-image
+    securityContext:
+      runAsUser: 1001
+      runAsGroup: 2000

pkg/library/overrides/testdata/container-overrides/overrides-can-use-delete-directive.yaml

@@ -0,0 +1,24 @@
+name: "container overrides can use $patch: delete to delete fields rather than merging"
+
+input:
+  component:
+    name: test-component
+    attributes:
+      container-overrides:
+        securityContext:
+          $patch: delete
+    container:
+      image: test-image
+
+  container:
+    name: test-component
+    image: test-image
+    securityContext:
+      runAsUser: 1000
+      runAsGroup: 2000
+
+output:
+  container:
+    name: test-component
+    image: test-image
+    securityContext: {}

pkg/library/overrides/testdata/container-overrides/overrides-can-use-replace-directive.yaml

@@ -0,0 +1,26 @@
+name: "container overrides can use $patch: replace to overwrite fields rather than merging"
+
+input:
+  component:
+    name: test-component
+    attributes:
+      container-overrides:
+        securityContext:
+          runAsUser: 1001
+          $patch: replace
+    container:
+      image: test-image
+
+  container:
+    name: test-component
+    image: test-image
+    securityContext:
+      runAsUser: 1000
+      runAsGroup: 2000
+
+output:
+  container:
+    name: test-component
+    image: test-image
+    securityContext:
+      runAsUser: 1001

pkg/library/overrides/testdata/pod-overrides/error_cannot-override-volumes.yaml

@@ -0,0 +1,25 @@
+name: "Returns error when attempting to patch pod volumes"
+
+input:
+  workspace:
+    attributes:
+      pod-overrides:
+        spec:
+          volumes:
+            - name: test-volume
+    components:
+      - name: test-component
+        container:
+          image: test-image
+
+  podTemplateSpec:
+    metadata:
+      labels:
+        controller.devfile.io/devworkspace-id: test-id
+    spec:
+      containers:
+      - name: test-component
+        image: test-image
+
+output:
+  errRegexp: "cannot use pod-overrides to override pod volumes"

pkg/library/overrides/testdata/pod-overrides/overrides-can-override-securityContext.yaml

@@ -0,0 +1,41 @@
+name: "Pod overrides can override securityContext"
+
+input:
+  workspace:
+    attributes:
+      pod-overrides:
+        spec:
+          securityContext:
+            runAsUser: 1001
+    components:
+      - name: test-component
+        container:
+          image: test-image
+
+  podTemplateSpec:
+    metadata:
+      labels:
+        controller.devfile.io/devworkspace-id: test-id
+    spec:
+      containers:
+      - name: test-component
+        image: test-image
+      securityContext:
+        fsGroup: 2000
+        runAsGroup: 3000
+        runAsUser: 1000
+
+
+output:
+  podTemplateSpec:
+    metadata:
+      labels:
+        controller.devfile.io/devworkspace-id: test-id
+    spec:
+      containers:
+      - name: test-component
+        image: test-image
+      securityContext:
+        fsGroup: 2000
+        runAsGroup: 3000
+        runAsUser: 1001

pkg/library/overrides/testdata/pod-overrides/overrides-can-use-delete-directive.yaml

@@ -0,0 +1,38 @@
+name: "Pod overrides can use $patch: delete to delete fields rather than merging"
+
+input:
+  workspace:
+    attributes:
+      pod-overrides:
+        spec:
+          securityContext:
+            $patch: delete
+    components:
+      - name: test-component
+        container:
+          image: test-image
+
+  podTemplateSpec:
+    metadata:
+      labels:
+        controller.devfile.io/devworkspace-id: test-id
+    spec:
+      containers:
+      - name: test-component
+        image: test-image
+      securityContext:
+        fsGroup: 2000
+        runAsGroup: 3000
+        runAsUser: 1000
+
+
+output:
+  podTemplateSpec:
+    metadata:
+      labels:
+        controller.devfile.io/devworkspace-id: test-id
+    spec:
+      containers:
+      - name: test-component
+        image: test-image
+      securityContext: {}