job: Sanitize instead of validate the job names

The job names may come from many different sources and
it is too much of a foot-gun to reject the names by panicing.

Instead to keep the names within a nice constraint set of
characters and length, sanitize the names silently.

Related PR that had to work around the panics:
cilium/cilium#35031

Signed-off-by: Jussi Maki <[email protected]>

job/job.go

@@ -5,9 +5,9 @@ package job
 
 import (
 	"context"
+	"crypto/sha256"
 	"fmt"
 	"log/slog"
-	"regexp"
 	"runtime/pprof"
 	"sync"
 
@@ -232,11 +232,34 @@ func (sg *scopedGroup) Add(jobs ...Job) {
 	sg.group.add(sg.health, jobs...)
 }
 
-var nameRegex = regexp.MustCompile(`^[a-zA-Z][a-zA-Z0-9_\-]{0,100}$`)
-
-func validateName(name string) error {
-	if !nameRegex.MatchString(name) {
-		return fmt.Errorf("invalid job name: %q, expected to match %q", name, nameRegex)
+const maxNameLength = 100
+
+func sanitizeName(name string) string {
+	mangled := false
+	newLength := min(maxNameLength, len(name))
+	runes := make([]rune, 0, newLength)
+	for _, r := range name[:newLength] {
+		switch {
+		case r >= 'a' && r <= 'z':
+			fallthrough
+		case r >= 'A' && r <= 'Z':
+			fallthrough
+		case r >= '0' && r <= '9':
+			fallthrough
+		case r == '-' || r == '_':
+			runes = append(runes, r)
+		default:
+			// Skip invalid characters.
+			mangled = true
+		}
 	}
-	return nil
+	if mangled || len(name) > maxNameLength {
+		// Name was mangled or is too long, truncate and append hash.
+		const hashLen = 10
+		hash := fmt.Sprintf("%x", sha256.Sum256([]byte(name)))
+		newLen := min(maxNameLength-hashLen, len(runes))
+		runes = runes[:newLen]
+		return string(runes) + "-" + hash[:hashLen]
+	}
+	return string(runes)
 }

job/job_test.go

@@ -8,6 +8,7 @@ import (
 	"log/slog"
 	"runtime"
 	"runtime/pprof"
+	"strings"
 	"sync/atomic"
 	"testing"
 	"time"
@@ -157,36 +158,41 @@ func TestModuleDecoratedGroup(t *testing.T) {
 	assert.Equal(t, 2, callCount, "expected OneShot function to be called twice")
 }
 
-func TestOneShot_ValidateName(t *testing.T) {
+func Test_sanitizeName(t *testing.T) {
 	testCases := []struct {
-		name        string
-		jbName      string
-		expectError bool
+		name    string
+		in, out string
 	}{
 		{
-			name:        "valid",
-			jbName:      "valid_name",
-			expectError: false,
+			name: "valid",
+			in:   "valid_name-123ABC",
+			out:  "valid_name-123ABC",
 		},
 		{
-			name:        "invalid name",
-			jbName:      "$%^&",
-			expectError: true,
+			name: "allow upper",
+			in:   "AABB00",
+			out:  "AABB00",
 		},
 		{
-			name:        "allow upper",
-			jbName:      "AABB00",
-			expectError: false,
+			name: "invalid name",
+			in:   "X$%^&Y",
+			out:  "XY-4af3ba2ac8",
+		},
+		{
+			name: "unicode chars",
+			in:   "smile😀",
+			out:  "smile-aacf34c444",
+		},
+		{
+			name: "long name",
+			in:   strings.Repeat("a", maxNameLength*2),
+			out:  "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-c2a908d98f",
 		},
 	}
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			err := validateName(tc.jbName)
-			if tc.expectError {
-				assert.Error(t, err)
-			} else {
-				assert.NoError(t, err)
-			}
+			out := sanitizeName(tc.in)
+			assert.Equal(t, tc.out, out, "name mismatch")
 		})
 	}
 }

job/observer.go

@@ -20,9 +20,7 @@ import (
 // The Observer name must match regex "^[a-zA-Z][a-zA-Z0-9_\-]{0,100}$". If the `observable` completes, the job stops.
 // The context given to the observable is also canceled once the group stops.
 func Observer[T any](name string, fn ObserverFunc[T], observable stream.Observable[T], opts ...observerOpt[T]) Job {
-	if err := validateName(name); err != nil {
-		panic(err)
-	}
+	name = sanitizeName(name)
 	if fn == nil {
 		panic("`fn` must not be nil")
 	}

job/oneshot.go

@@ -23,9 +23,7 @@ import (
 // The given function is expected to exit as soon as the context given to it expires, this is especially important for
 // blocking or long running jobs.
 func OneShot(name string, fn OneShotFunc, opts ...jobOneShotOpt) Job {
-	if err := validateName(name); err != nil {
-		panic(err)
-	}
+	name = sanitizeName(name)
 	if fn == nil {
 		panic("`fn` must not be nil")
 	}

job/timer.go

@@ -26,9 +26,7 @@ import (
 // expires. This is especially important for long running functions. The signal created by a Trigger is coalesced so
 // multiple calls to trigger before the invocation takes place can result in just a single invocation.
 func Timer(name string, fn TimerFunc, interval time.Duration, opts ...timerOpt) Job {
-	if err := validateName(name); err != nil {
-		panic(err)
-	}
+	name = sanitizeName(name)
 	if fn == nil {
 		panic("`fn` must not be nil")
 	}