fix: hash scheduler-k3s cron-id label to fit Kubernetes' 63-byte cap

The `dokku.com/cron-id` label could exceed Kubernetes' 63-byte cap because the cron ID is `base36(appName === command === schedule)`, which expands roughly 1.5x per byte. The label is now keyed `dokku.com/cron-hash` and holds the `sha1` hex digest of the cron-id, a fixed 40-character value that always fits the cap. The same hex digest is mirrored into the `dokku.com/cron-hash` annotation, and the original base36 cron-id stays in the `dokku.com/cron-id` annotation that `cron:list` reads when surfacing user-facing IDs. Per-task lookups stay server-side via label selectors, so `cron:set --maintenance` and the forbid/replace concurrency checks on `dokku run --cron-id` keep working without any in-memory filtering.
This commit is contained in:
Jose Diaz-Gonzalez
2026-05-24 02:50:09 -04:00
parent 3c8cba08e5
commit bf2cfe1886
10 changed files with 235 additions and 51 deletions

View File

@@ -15,7 +15,8 @@ import (
) )
// TestCronIDLabelValue asserts the hashed cron ID fits inside Kubernetes' 63 // TestCronIDLabelValue asserts the hashed cron ID fits inside Kubernetes' 63
// byte label cap and is deterministic. Without this the label can exceed the // byte label cap, is a fixed 40-character sha1 hex digest, and is
// deterministic. Without this the dokku.com/cron-hash label can exceed the
// cap and the Kubernetes API server rejects the manifest. // cap and the Kubernetes API server rejects the manifest.
func TestCronIDLabelValue(t *testing.T) { func TestCronIDLabelValue(t *testing.T) {
cases := []string{ cases := []string{
@@ -25,6 +26,9 @@ func TestCronIDLabelValue(t *testing.T) {
} }
for _, in := range cases { for _, in := range cases {
out := cronIDLabelValue(in) out := cronIDLabelValue(in)
if len(out) != 40 {
t.Errorf("cronIDLabelValue(%q) = %q (len %d); want 40 hex chars", in, out, len(out))
}
if len(out) > 63 { if len(out) > 63 {
t.Errorf("cronIDLabelValue(%q) = %q (len %d); must be <= 63", in, out, len(out)) t.Errorf("cronIDLabelValue(%q) = %q (len %d); must be <= 63", in, out, len(out))
} }
@@ -35,11 +39,123 @@ func TestCronIDLabelValue(t *testing.T) {
} }
// TestCronJobTemplateQuotesAllDigitSuffix asserts that the rendered cron-job // TestCronJobTemplateQuotesAllDigitSuffix asserts that the rendered cron-job
// manifest produces string-typed annotation values even when the suffix and // manifest produces string-typed annotation and label values even when the
// cron-id consist entirely of digits. Without `| quote` in the template, YAML // suffix and cron-id consist entirely of digits. Without `| quote` in the
// would coerce these to numbers and the manifest would be rejected by the // template, YAML would coerce these to numbers and the manifest would be
// Kubernetes API server. // rejected by the Kubernetes API server.
func TestCronJobTemplateQuotesAllDigitSuffix(t *testing.T) { func TestCronJobTemplateQuotesAllDigitSuffix(t *testing.T) {
manifest := renderCronJobTemplate(t, map[string]interface{}{
"id": "1234567890",
"hash": "0123456789abcdef0123456789abcdef01234567",
"schedule": "5 5 5 5 5",
"suffix": "1234567890",
"suspend": false,
"concurrency_policy": "Allow",
})
forEachManifestMetadata(t, manifest, func(metadata map[string]interface{}) {
annotations, _ := metadata["annotations"].(map[string]interface{})
for key, value := range annotations {
if _, isString := value.(string); !isString {
t.Errorf("annotation %q has non-string value %v (type %T); helm template must apply | quote", key, value, value)
}
}
labels, _ := metadata["labels"].(map[string]interface{})
for key, value := range labels {
if _, isString := value.(string); !isString {
t.Errorf("label %q has non-string value %v (type %T); helm template must apply | quote", key, value, value)
}
}
})
}
// TestCronJobTemplateEmitsHashLabel asserts that the rendered manifest
// emits the sha1 hash as the dokku.com/cron-hash label and matching
// annotation, while keeping the original base36 cron-id in the
// dokku.com/cron-id annotation so cron:list can still surface it to users.
func TestCronJobTemplateEmitsHashLabel(t *testing.T) {
originalID := "app===echo hello-from-cron===5 5 5 5 5"
hashedID := cronIDLabelValue(originalID)
manifest := renderCronJobTemplate(t, map[string]interface{}{
"id": originalID,
"hash": hashedID,
"schedule": "5 5 5 5 5",
"suffix": "abcde",
"suspend": false,
"concurrency_policy": "Allow",
})
sawHashLabel := false
sawIDAnnotation := false
forEachManifestMetadata(t, manifest, func(metadata map[string]interface{}) {
if labels, ok := metadata["labels"].(map[string]interface{}); ok {
if value, ok := labels["dokku.com/cron-hash"]; ok {
sawHashLabel = true
if value != hashedID {
t.Errorf("dokku.com/cron-hash label = %q, want %q", value, hashedID)
}
}
}
annotations, _ := metadata["annotations"].(map[string]interface{})
if value, ok := annotations["dokku.com/cron-hash"]; ok {
if value != hashedID {
t.Errorf("dokku.com/cron-hash annotation = %q, want %q (must mirror the label)", value, hashedID)
}
}
if value, ok := annotations["dokku.com/cron-id"]; ok {
sawIDAnnotation = true
if value != originalID {
t.Errorf("dokku.com/cron-id annotation = %q, want %q", value, originalID)
}
}
})
if !sawHashLabel {
t.Errorf("rendered manifest did not include a dokku.com/cron-hash label")
}
if !sawIDAnnotation {
t.Errorf("rendered manifest did not include a dokku.com/cron-id annotation")
}
}
// TestCronJobTemplateRendersLongCronID is the direct regression test for
// dokku/dokku#8594: a cron-id well over the 63-byte Kubernetes label cap
// must render cleanly and every resulting label value must stay under the
// cap.
func TestCronJobTemplateRendersLongCronID(t *testing.T) {
longCronID := strings.Repeat("a-very-long-cron-id-that-would-far-exceed-the-label-cap-", 10)
manifest := renderCronJobTemplate(t, map[string]interface{}{
"id": longCronID,
"hash": cronIDLabelValue(longCronID),
"schedule": "5 5 5 5 5",
"suffix": "abcde",
"suspend": false,
"concurrency_policy": "Allow",
})
forEachManifestMetadata(t, manifest, func(metadata map[string]interface{}) {
labels, _ := metadata["labels"].(map[string]interface{})
for key, value := range labels {
str, ok := value.(string)
if !ok {
t.Errorf("label %q has non-string value %v (type %T)", key, value, value)
continue
}
if len(str) > 63 {
t.Errorf("label %q value %q exceeds Kubernetes' 63-byte cap (len %d)", key, str, len(str))
}
}
})
}
// renderCronJobTemplate renders templates/chart/cron-job.yaml with the
// supplied cron-process values and returns the YAML manifest as a string.
// The chart is materialised in a tempdir alongside the shared _helpers.tpl
// so the helm engine resolves named templates the same way it does in
// production.
func renderCronJobTemplate(t *testing.T, cronValues map[string]interface{}) string {
t.Helper()
chartDir := t.TempDir() chartDir := t.TempDir()
if err := os.MkdirAll(filepath.Join(chartDir, "templates"), 0o755); err != nil { if err := os.MkdirAll(filepath.Join(chartDir, "templates"), 0o755); err != nil {
t.Fatalf("mkdir: %v", err) t.Fatalf("mkdir: %v", err)
@@ -82,16 +198,9 @@ func TestCronJobTemplateQuotesAllDigitSuffix(t *testing.T) {
}, },
}, },
"processes": map[string]interface{}{ "processes": map[string]interface{}{
"cron-id-123": map[string]interface{}{ "cron-process": map[string]interface{}{
"args": []interface{}{"echo", "hello"}, "args": []interface{}{"echo", "hello"},
"cron": map[string]interface{}{ "cron": cronValues,
"id": "1234567890",
"hash": "abc123def",
"schedule": "5 5 5 5 5",
"suffix": "1234567890",
"suspend": false,
"concurrency_policy": "Allow",
},
}, },
}, },
} }
@@ -106,44 +215,49 @@ func TestCronJobTemplateQuotesAllDigitSuffix(t *testing.T) {
t.Fatalf("render: %v", err) t.Fatalf("render: %v", err)
} }
var manifest string
for name, content := range rendered { for name, content := range rendered {
if filepath.Base(name) == "cron-job.yaml" { if filepath.Base(name) == "cron-job.yaml" {
manifest = content return content
break
} }
} }
if manifest == "" { t.Fatalf("cron-job.yaml not rendered; got: %v", rendered)
t.Fatalf("cron-job.yaml not rendered; got: %v", rendered) return ""
} }
// forEachManifestMetadata invokes fn on every metadata block in a multi-doc
// YAML manifest, including the nested jobTemplate and pod template metadata
// blocks inside a CronJob spec.
func forEachManifestMetadata(t *testing.T, manifest string, fn func(metadata map[string]interface{})) {
t.Helper()
decoder := yaml.NewDecoder(strings.NewReader(manifest)) decoder := yaml.NewDecoder(strings.NewReader(manifest))
for { for {
var doc map[string]interface{} var doc map[string]interface{}
if err := decoder.Decode(&doc); err != nil { if err := decoder.Decode(&doc); err != nil {
if errors.Is(err, io.EOF) { if errors.Is(err, io.EOF) {
break return
} }
t.Fatalf("yaml decode failed (would also fail in Kubernetes API): %v\nrendered:\n%s", err, manifest) t.Fatalf("yaml decode failed (would also fail in Kubernetes API): %v\nrendered:\n%s", err, manifest)
} }
if doc == nil { if doc == nil {
continue continue
} }
metadata, ok := doc["metadata"].(map[string]interface{}) walkMetadata(doc, fn)
if !ok { }
continue }
func walkMetadata(node interface{}, fn func(metadata map[string]interface{})) {
switch typed := node.(type) {
case map[string]interface{}:
if metadata, ok := typed["metadata"].(map[string]interface{}); ok {
fn(metadata)
} }
annotations, _ := metadata["annotations"].(map[string]interface{}) for _, value := range typed {
for key, value := range annotations { walkMetadata(value, fn)
if _, isString := value.(string); !isString {
t.Errorf("annotation %q has non-string value %v (type %T); helm template must apply | quote", key, value, value)
}
} }
labels, _ := metadata["labels"].(map[string]interface{}) case []interface{}:
for key, value := range labels { for _, value := range typed {
if _, isString := value.(string); !isString { walkMetadata(value, fn)
t.Errorf("label %q has non-string value %v (type %T); helm template must apply | quote", key, value, value)
}
} }
} }
} }

View File

@@ -3,8 +3,9 @@ package scheduler_k3s
import ( import (
"bytes" "bytes"
"context" "context"
"crypto/sha256" "crypto/sha1"
"encoding/base64" "encoding/base64"
"encoding/hex"
"errors" "errors"
"fmt" "fmt"
"net" "net"
@@ -24,7 +25,6 @@ import (
nginxvhosts "github.com/dokku/dokku/plugins/nginx-vhosts" nginxvhosts "github.com/dokku/dokku/plugins/nginx-vhosts"
resty "github.com/go-resty/resty/v2" resty "github.com/go-resty/resty/v2"
kedav1alpha1 "github.com/kedacore/keda/v2/apis/keda/v1alpha1" kedav1alpha1 "github.com/kedacore/keda/v2/apis/keda/v1alpha1"
"github.com/multiformats/go-base36"
"golang.org/x/sync/errgroup" "golang.org/x/sync/errgroup"
"gopkg.in/yaml.v3" "gopkg.in/yaml.v3"
"helm.sh/helm/v3/pkg/strvals" "helm.sh/helm/v3/pkg/strvals"
@@ -2083,12 +2083,14 @@ func kubernetesNodeToNode(node v1.Node) Node {
} }
} }
// cronIDLabelValue returns a Kubernetes-label-safe hash of a cron ID. // cronIDLabelValue returns the sha1 hex digest of the supplied cron ID,
// The raw cron ID can exceed the 63-byte label cap, so we keep it as an // used as the dokku.com/cron-hash label/annotation value on cron resources.
// annotation and use this short hash for selectors. // The raw cron ID exceeds Kubernetes' 63-byte label cap for non-trivial
// commands, so we hash it to a fixed 40-character string and keep the
// original in the dokku.com/cron-id annotation for display.
func cronIDLabelValue(cronID string) string { func cronIDLabelValue(cronID string) string {
sum := sha256.Sum256([]byte(cronID)) sum := sha1.Sum([]byte(cronID))
return base36.EncodeToStringLc(sum[:16]) return hex.EncodeToString(sum[:])
} }
// parseMemoryQuantity parses a string into a valid memory quantity // parseMemoryQuantity parses a string into a valid memory quantity

View File

@@ -356,6 +356,7 @@ type ClusterIssuer struct {
} }
type Job struct { type Job struct {
Annotations map[string]string
AppName string AppName string
Command []string Command []string
DeploymentID int64 DeploymentID int64
@@ -429,6 +430,7 @@ func templateKubernetesJob(input Job) (batchv1.Job, error) {
"dokku.com/builder-type": input.ImageSourceType, "dokku.com/builder-type": input.ImageSourceType,
"dokku.com/managed": "true", "dokku.com/managed": "true",
} }
maps.Copy(annotations, input.Annotations)
maps.Copy(labels, input.Labels) maps.Copy(labels, input.Labels)
secretName := GetConfigSecretName(input.AppName) secretName := GetConfigSecretName(input.AppName)

View File

@@ -10,6 +10,7 @@ metadata:
annotations: annotations:
app.kubernetes.io/version: {{ $.Values.global.deployment_id | quote }} app.kubernetes.io/version: {{ $.Values.global.deployment_id | quote }}
dokku.com/builder-type: {{ $.Values.global.image.type | quote }} dokku.com/builder-type: {{ $.Values.global.image.type | quote }}
dokku.com/cron-hash: {{ $config.cron.hash | quote }}
dokku.com/cron-id: {{ $config.cron.id | quote }} dokku.com/cron-id: {{ $config.cron.id | quote }}
dokku.com/job-suffix: {{ $config.cron.suffix | quote }} dokku.com/job-suffix: {{ $config.cron.suffix | quote }}
dokku.com/managed: "true" dokku.com/managed: "true"
@@ -33,6 +34,7 @@ spec:
annotations: annotations:
app.kubernetes.io/version: {{ $.Values.global.deployment_id | quote }} app.kubernetes.io/version: {{ $.Values.global.deployment_id | quote }}
dokku.com/builder-type: {{ $.Values.global.image.type | quote }} dokku.com/builder-type: {{ $.Values.global.image.type | quote }}
dokku.com/cron-hash: {{ $config.cron.hash | quote }}
dokku.com/cron-id: {{ $config.cron.id | quote }} dokku.com/cron-id: {{ $config.cron.id | quote }}
dokku.com/job-suffix: {{ $config.cron.suffix | quote }} dokku.com/job-suffix: {{ $config.cron.suffix | quote }}
dokku.com/managed: "true" dokku.com/managed: "true"
@@ -56,6 +58,7 @@ spec:
annotations: annotations:
app.kubernetes.io/version: {{ $.Values.global.deployment_id | quote }} app.kubernetes.io/version: {{ $.Values.global.deployment_id | quote }}
dokku.com/builder-type: {{ $.Values.global.image.type | quote }} dokku.com/builder-type: {{ $.Values.global.image.type | quote }}
dokku.com/cron-hash: {{ $config.cron.hash | quote }}
dokku.com/cron-id: {{ $config.cron.id | quote }} dokku.com/cron-id: {{ $config.cron.id | quote }}
dokku.com/job-suffix: {{ $config.cron.suffix | quote }} dokku.com/job-suffix: {{ $config.cron.suffix | quote }}
dokku.com/managed: "true" dokku.com/managed: "true"

View File

@@ -795,10 +795,8 @@ func TriggerSchedulerDeploy(scheduler string, appName string, imageTag string) e
suffix := "" suffix := ""
for _, cronJob := range cronJobs { for _, cronJob := range cronJobs {
if cronJob.Annotations["dokku.com/cron-id"] == cronTask.ID { if cronJob.Annotations["dokku.com/cron-id"] == cronTask.ID {
var ok bool if value, ok := cronJob.Annotations["dokku.com/job-suffix"]; ok {
suffix, ok = cronJob.Annotations["dokku.com/job-suffix"] suffix = value
if !ok {
suffix = ""
} }
} }
} }
@@ -1355,6 +1353,7 @@ func TriggerSchedulerRun(scheduler string, appName string, envCount int, args []
labels := map[string]string{ labels := map[string]string{
"app.kubernetes.io/part-of": appName, "app.kubernetes.io/part-of": appName,
} }
annotations := map[string]string{}
if os.Getenv("DOKKU_TRACE") == "1" { if os.Getenv("DOKKU_TRACE") == "1" {
extraEnv["TRACE"] = "true" extraEnv["TRACE"] = "true"
@@ -1373,8 +1372,11 @@ func TriggerSchedulerRun(scheduler string, appName string, envCount int, args []
processType := "run" processType := "run"
if os.Getenv("DOKKU_CRON_ID") != "" { if os.Getenv("DOKKU_CRON_ID") != "" {
processType = "cron" processType = "cron"
cronHash := cronIDLabelValue(os.Getenv("DOKKU_CRON_ID")) cronID := os.Getenv("DOKKU_CRON_ID")
cronHash := cronIDLabelValue(cronID)
labels["dokku.com/cron-hash"] = cronHash labels["dokku.com/cron-hash"] = cronHash
annotations["dokku.com/cron-hash"] = cronHash
annotations["dokku.com/cron-id"] = cronID
concurrencyPolicy := strings.ToUpper(os.Getenv("DOKKU_CONCURRENCY_POLICY")) concurrencyPolicy := strings.ToUpper(os.Getenv("DOKKU_CONCURRENCY_POLICY"))
switch concurrencyPolicy { switch concurrencyPolicy {
case "forbid": case "forbid":
@@ -1498,6 +1500,8 @@ func TriggerSchedulerRun(scheduler string, appName string, envCount int, args []
workingDir := common.GetWorkingDir(appName, image) workingDir := common.GetWorkingDir(appName, image)
job, err := templateKubernetesJob(Job{ job, err := templateKubernetesJob(Job{
ActiveDeadlineSeconds: activeDeadlineSeconds,
Annotations: annotations,
AppName: appName, AppName: appName,
Command: command, Command: command,
DeploymentID: deploymentID, DeploymentID: deploymentID,
@@ -1513,7 +1517,6 @@ func TriggerSchedulerRun(scheduler string, appName string, envCount int, args []
RemoveContainer: rmContainer, RemoveContainer: rmContainer,
SecurityContext: securityContext, SecurityContext: securityContext,
WorkingDir: workingDir, WorkingDir: workingDir,
ActiveDeadlineSeconds: activeDeadlineSeconds,
}) })
if err != nil { if err != nil {
return fmt.Errorf("Error templating job: %w", err) return fmt.Errorf("Error templating job: %w", err)
@@ -1754,7 +1757,7 @@ func TriggerSchedulerRunList(scheduler string, appName string, format string) er
cronID, ok := cronJob.Annotations["dokku.com/cron-id"] cronID, ok := cronJob.Annotations["dokku.com/cron-id"]
if !ok { if !ok {
common.LogWarn(fmt.Sprintf("Cron job %s does not have a cron ID annotation", cronJob.Name)) common.LogWarn(fmt.Sprintf("Cron job %s does not have a dokku.com/cron-id annotation", cronJob.Name))
continue continue
} }

View File

@@ -1,7 +1,7 @@
{ {
"cron": [ "cron": [
{ {
"command": "python3 task.py", "command": "python3 task.py some cron task",
"schedule": "5 5 5 5 5" "schedule": "5 5 5 5 5"
} }
], ],

View File

@@ -265,7 +265,7 @@ teardown() {
echo "output: $output" echo "output: $output"
echo "status: $status" echo "status: $status"
assert_success assert_success
assert_output "['task.py']" assert_output "['task.py', 'some', 'cron', 'task']"
} }
@test "(builder-pack) cron:run with Procfile reference" { @test "(builder-pack) cron:run with Procfile reference" {

View File

@@ -103,11 +103,31 @@ teardown() {
assert_success assert_success
assert_output_exists assert_output_exists
cron_hash="$(printf '%s' "$cron_id" | sha1sum | awk '{print $1}')"
run /bin/bash -c "kubectl get cronjob -o json | jq -r '.items[0].metadata.annotations.\"dokku.com/cron-id\"'"
echo "output: $output"
echo "status: $status"
assert_success
assert_output "$cron_id"
run /bin/bash -c "kubectl get cronjob -o json | jq -r '.items[0].metadata.labels.\"dokku.com/cron-hash\"'"
echo "output: $output"
echo "status: $status"
assert_success
assert_output "$cron_hash"
run /bin/bash -c "kubectl get cronjob -o json | jq -r '.items[0].metadata.annotations.\"dokku.com/cron-hash\"'"
echo "output: $output"
echo "status: $status"
assert_success
assert_output "$cron_hash"
run /bin/bash -c "dokku --quiet cron:run $TEST_APP $cron_id" run /bin/bash -c "dokku --quiet cron:run $TEST_APP $cron_id"
echo "output: $output" echo "output: $output"
echo "status: $status" echo "status: $status"
assert_success assert_success
assert_output_contains "['task.py']" assert_output_contains "['task.py', 'some', 'cron', 'task']"
} }
@test "(scheduler-k3s) cnb dokku run uses launcher entrypoint" { @test "(scheduler-k3s) cnb dokku run uses launcher entrypoint" {

View File

@@ -94,6 +94,26 @@ teardown() {
assert_success assert_success
assert_output_exists assert_output_exists
cron_hash="$(printf '%s' "$cron_id" | sha1sum | awk '{print $1}')"
run /bin/bash -c "kubectl get cronjob -o json | jq -r '.items[0].metadata.annotations.\"dokku.com/cron-id\"'"
echo "output: $output"
echo "status: $status"
assert_success
assert_output "$cron_id"
run /bin/bash -c "kubectl get cronjob -o json | jq -r '.items[0].metadata.labels.\"dokku.com/cron-hash\"'"
echo "output: $output"
echo "status: $status"
assert_success
assert_output "$cron_hash"
run /bin/bash -c "kubectl get cronjob -o json | jq -r '.items[0].metadata.annotations.\"dokku.com/cron-hash\"'"
echo "output: $output"
echo "status: $status"
assert_success
assert_output "$cron_hash"
run /bin/bash -c "dokku --quiet cron:run $TEST_APP $cron_id" run /bin/bash -c "dokku --quiet cron:run $TEST_APP $cron_id"
echo "output: $output" echo "output: $output"
echo "status: $status" echo "status: $status"

View File

@@ -99,6 +99,26 @@ teardown() {
assert_success assert_success
assert_output_exists assert_output_exists
cron_hash="$(printf '%s' "$cron_id" | sha1sum | awk '{print $1}')"
run /bin/bash -c "kubectl get cronjob -o json | jq -r '.items[0].metadata.annotations.\"dokku.com/cron-id\"'"
echo "output: $output"
echo "status: $status"
assert_success
assert_output "$cron_id"
run /bin/bash -c "kubectl get cronjob -o json | jq -r '.items[0].metadata.labels.\"dokku.com/cron-hash\"'"
echo "output: $output"
echo "status: $status"
assert_success
assert_output "$cron_hash"
run /bin/bash -c "kubectl get cronjob -o json | jq -r '.items[0].metadata.annotations.\"dokku.com/cron-hash\"'"
echo "output: $output"
echo "status: $status"
assert_success
assert_output "$cron_hash"
run /bin/bash -c "dokku --quiet cron:run $TEST_APP $cron_id" run /bin/bash -c "dokku --quiet cron:run $TEST_APP $cron_id"
echo "output: $output" echo "output: $output"
echo "status: $status" echo "status: $status"