fix: Validate label values from workflowMetadata to avoid controller …

…crash. Fixes #10872 (#10892)

Signed-off-by: Yuan Tang <terrytangyuan@gmail.com>

workflow/controller/controller_test.go

@@ -632,6 +632,40 @@ func TestCheckAndInitWorkflowTmplRef(t *testing.T) {
 	assert.Equal(t, wftmpl.Spec.Templates, woc.execWf.Spec.Templates)
 }
 
+const wfWithInvalidMetadata = `
+apiVersion: argoproj.io/v1alpha1
+kind: Workflow
+metadata:
+  name: crash
+spec:
+  serviceAccountName: my-sa
+  entrypoint: test-container
+  arguments:
+    parameters:
+      - name: execution_label
+        value: some/special/char
+  workflowMetadata:
+    labelsFrom:
+      execution_label:
+        expression: workflow.parameters.execution_label
+  templates:
+  - name: test-container
+    container:
+      image: alpine:latest
+      command: ["echo", "bye"]
+`
+
+func TestInvalidWorkflowMetadata(t *testing.T) {
+	wf := wfv1.MustUnmarshalWorkflow(wfWithInvalidMetadata)
+	cancel, controller := newController(wf)
+	defer cancel()
+	woc := newWorkflowOperationCtx(wf, controller)
+	err := woc.setExecWorkflow(context.Background())
+	if assert.NotNil(t, err) {
+		assert.Contains(t, err.Error(), "invalid label value")
+	}
+}
+
 func TestIsArchivable(t *testing.T) {
 	cancel, controller := newController()
 	defer cancel()

workflow/controller/healthz.go

@@ -42,13 +42,13 @@ func (wfc *WorkflowController) Healthz(w http.ResponseWriter, r *http.Request) {
 		}
 		return nil
 	}()
-	log.WithField("err", err).
-		WithField("managedNamespace", wfc.managedNamespace).
-		WithField("instanceID", instanceID).
-		WithField("labelSelector", labelSelector).
-		WithField("age", age).
-		Info("healthz")
 	if err != nil {
+		log.WithField("err", err).
+			WithField("managedNamespace", wfc.managedNamespace).
+			WithField("instanceID", instanceID).
+			WithField("labelSelector", labelSelector).
+			WithField("age", age).
+			Info("healthz")
 		w.WriteHeader(500)
 		_, _ = w.Write([]byte(err.Error()))
 	} else {

workflow/controller/operator.go

@@ -30,6 +30,7 @@ import (
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/strategicpatch"
+	"k8s.io/apimachinery/pkg/util/validation"
 	"k8s.io/client-go/tools/cache"
 	"k8s.io/client-go/tools/record"
 	"k8s.io/utils/pointer"
@@ -542,6 +543,9 @@ func (woc *wfOperationCtx) updateWorkflowMetadata() error {
 			if !ok {
 				return fmt.Errorf("failed to evaluate label %q expression %q evaluted to %T but must be a string", n, f.Expression, r)
 			}
+			if errs := validation.IsValidLabelValue(v); errs != nil {
+				return errors.Errorf(errors.CodeBadRequest, "invalid label value %q for label %q and expression %q: %s", v, n, f.Expression, strings.Join(errs, ";"))
+			}
 			woc.wf.Labels[n] = v
 			woc.globalParams["workflow.labels."+n] = v
 			updatedParams["workflow.labels."+n] = v