fix: validate envFrom sources and ignore configmap binary data

Author: ThomasK33

api/v1alpha1/codercontrolplane_types.go

@@ -90,6 +90,7 @@ type CoderControlPlaneSpec struct {
 	// +optional
 	Expose *ExposeSpec `json:"expose,omitempty"`
 
+	// +kubebuilder:validation:XValidation:rule="self.all(e, !(has(e.configMapRef) && has(e.secretRef)))",message="each envFrom entry may specify at most one of configMapRef or secretRef"
 	// EnvFrom injects environment variables from ConfigMaps/Secrets.
 	EnvFrom []corev1.EnvFromSource `json:"envFrom,omitempty"`
 	// Volumes are additional volumes to add to the pod.

config/crd/bases/coder.com_codercontrolplanes.yaml

@@ -1022,6 +1022,10 @@ spec:
                       x-kubernetes-map-type: atomic
                   type: object
                 type: array
+                x-kubernetes-validations:
+                - message: each envFrom entry may specify at most one of configMapRef
+                    or secretRef
+                  rule: self.all(e, !(has(e.configMapRef) && has(e.secretRef)))
               envUseClusterAccessURL:
                 default: true
                 description: EnvUseClusterAccessURL injects a default CODER_ACCESS_URL

internal/controller/codercontrolplane_controller.go

@@ -2102,10 +2102,6 @@ func (r *CoderControlPlaneReconciler) envFromDefinesEnvVar(
 
 	for i := range envFromSources {
 		envFromSource := envFromSources[i]
-		if envFromSource.ConfigMapRef != nil && envFromSource.SecretRef != nil {
-			return false, fmt.Errorf("assertion failed: envFrom[%d] must not set both configMapRef and secretRef", i)
-		}
-
 		lookupKey, includeSource, err := envFromLookupKeyForEnvVar(envFromSource.Prefix, envVarName)
 		if err != nil {
 			return false, err
@@ -2123,18 +2119,12 @@ func (r *CoderControlPlaneReconciler) envFromDefinesEnvVar(
 			configMap := &corev1.ConfigMap{}
 			err := reader.Get(ctx, types.NamespacedName{Namespace: namespace, Name: configMapName}, configMap)
 			if err != nil {
-				if apierrors.IsNotFound(err) {
-					continue
+				if !apierrors.IsNotFound(err) {
+					return false, fmt.Errorf("get envFrom[%d] configmap %s/%s: %w", i, namespace, configMapName, err)
 				}
-				return false, fmt.Errorf("get envFrom[%d] configmap %s/%s: %w", i, namespace, configMapName, err)
-			}
-			if _, ok := configMap.Data[lookupKey]; ok {
+			} else if _, ok := configMap.Data[lookupKey]; ok {
 				return true, nil
 			}
-			if _, ok := configMap.BinaryData[lookupKey]; ok {
-				return true, nil
-			}
-			continue
 		}
 
 		if envFromSource.SecretRef != nil {
@@ -2146,12 +2136,10 @@ func (r *CoderControlPlaneReconciler) envFromDefinesEnvVar(
 			secret := &corev1.Secret{}
 			err := reader.Get(ctx, types.NamespacedName{Namespace: namespace, Name: secretName}, secret)
 			if err != nil {
-				if apierrors.IsNotFound(err) {
-					continue
+				if !apierrors.IsNotFound(err) {
+					return false, fmt.Errorf("get envFrom[%d] secret %s/%s: %w", i, namespace, secretName, err)
 				}
-				return false, fmt.Errorf("get envFrom[%d] secret %s/%s: %w", i, namespace, secretName, err)
-			}
-			if _, ok := secret.Data[lookupKey]; ok {
+			} else if _, ok := secret.Data[lookupKey]; ok {
 				return true, nil
 			}
 		}

internal/controller/codercontrolplane_controller_test.go

@@ -3031,6 +3031,81 @@ func TestReconcile_DeploymentAlignment(t *testing.T) {
 		}
 	})
 
+	t.Run("EnvFromBinaryDataDoesNotSuppressDefault", func(t *testing.T) {
+		configMap := &corev1.ConfigMap{
+			ObjectMeta: metav1.ObjectMeta{Name: "test-deployment-alignment-envfrom-binary", Namespace: "default"},
+			BinaryData: map[string][]byte{
+				"CODER_ACCESS_URL": []byte("https://binary-data.example.test"),
+			},
+		}
+		if err := k8sClient.Create(ctx, configMap); err != nil {
+			t.Fatalf("create configmap: %v", err)
+		}
+		t.Cleanup(func() {
+			_ = k8sClient.Delete(ctx, configMap)
+		})
+
+		cp := &coderv1alpha1.CoderControlPlane{
+			ObjectMeta: metav1.ObjectMeta{Name: "test-deployment-alignment-envfrom-binary", Namespace: "default"},
+			Spec: coderv1alpha1.CoderControlPlaneSpec{
+				Image: "test-deployment-alignment:latest",
+				EnvFrom: []corev1.EnvFromSource{{
+					ConfigMapRef: &corev1.ConfigMapEnvSource{
+						LocalObjectReference: corev1.LocalObjectReference{Name: configMap.Name},
+					},
+				}},
+			},
+		}
+		if err := k8sClient.Create(ctx, cp); err != nil {
+			t.Fatalf("create control plane: %v", err)
+		}
+		t.Cleanup(func() {
+			_ = k8sClient.Delete(ctx, cp)
+		})
+
+		r := &controller.CoderControlPlaneReconciler{Client: k8sClient, APIReader: k8sClient, Scheme: scheme}
+		if _, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: types.NamespacedName{Name: cp.Name, Namespace: cp.Namespace}}); err != nil {
+			t.Fatalf("reconcile control plane: %v", err)
+		}
+
+		deployment := &appsv1.Deployment{}
+		if err := k8sClient.Get(ctx, types.NamespacedName{Name: cp.Name, Namespace: cp.Namespace}, deployment); err != nil {
+			t.Fatalf("get deployment: %v", err)
+		}
+		container := deployment.Spec.Template.Spec.Containers[0]
+		expectedAccessURL := "http://" + cp.Name + "." + cp.Namespace + ".svc.cluster.local"
+		if got := mustFindEnvVar(t, container.Env, "CODER_ACCESS_URL").Value; got != expectedAccessURL {
+			t.Fatalf("expected default CODER_ACCESS_URL %q, got %q", expectedAccessURL, got)
+		}
+	})
+
+	t.Run("EnvFromWithConfigMapAndSecretRefsIsRejectedAtAdmission", func(t *testing.T) {
+		cp := &coderv1alpha1.CoderControlPlane{
+			ObjectMeta: metav1.ObjectMeta{Name: "test-deployment-alignment-envfrom-both-refs", Namespace: "default"},
+			Spec: coderv1alpha1.CoderControlPlaneSpec{
+				Image: "test-deployment-alignment:latest",
+				EnvFrom: []corev1.EnvFromSource{{
+					ConfigMapRef: &corev1.ConfigMapEnvSource{
+						LocalObjectReference: corev1.LocalObjectReference{Name: "both-refs-configmap"},
+					},
+					SecretRef: &corev1.SecretEnvSource{
+						LocalObjectReference: corev1.LocalObjectReference{Name: "both-refs-secret"},
+					},
+				}},
+			},
+		}
+		err := k8sClient.Create(ctx, cp)
+		if err == nil {
+			t.Fatal("expected create to fail when envFrom entry sets both configMapRef and secretRef")
+		}
+		if !apierrors.IsInvalid(err) {
+			t.Fatalf("expected invalid error for envFrom entry with both refs, got: %v", err)
+		}
+		if !strings.Contains(err.Error(), "envFrom") {
+			t.Fatalf("expected envFrom validation error, got: %v", err)
+		}
+	})
+
 	t.Run("ResourcesAndSecurityContextsApplied", func(t *testing.T) {
 		runAsUser := int64(1001)
 		allowPrivilegeEscalation := false