diff --git a/docs/deployment/schedulers/k3s.md b/docs/deployment/schedulers/k3s.md index 847dd46d5..55145b112 100644 --- a/docs/deployment/schedulers/k3s.md +++ b/docs/deployment/schedulers/k3s.md @@ -376,6 +376,8 @@ The following resource types are supported: - `traefik_ingressroute` - `traefik_middleware` +Annotation keys may contain `/` (e.g. Kubernetes-style keys such as `prometheus.io/scrape`) and values may span multiple lines; both are preserved verbatim. + A `ps:restart` is required after setting annotations in order to have them apply to running resources. #### Removing an annotation @@ -446,6 +448,8 @@ The following resource types are supported: - `traefik_ingressroute` - `traefik_middleware` +Label keys may contain `/` (e.g. Kubernetes-style keys such as `app.kubernetes.io/part-of`) and values may span multiple lines; both are preserved verbatim. + A `ps:restart` is required after setting labels in order to have them apply to running resources. #### Removing a label diff --git a/plugins/scheduler-k3s/functions.go b/plugins/scheduler-k3s/functions.go index e40ab7adb..5483ce95b 100644 --- a/plugins/scheduler-k3s/functions.go +++ b/plugins/scheduler-k3s/functions.go @@ -775,22 +775,7 @@ func getGlobalAnnotations(appName string) (ProcessAnnotations, error) { // getAnnotation retrieves an annotation for a given app, process type, and resource type func getAnnotation(appName string, processType string, resourceType string) (map[string]string, error) { - annotations := map[string]string{} - annotationsList, err := common.PropertyListGet("scheduler-k3s", appName, fmt.Sprintf("%s.%s", processType, resourceType)) - if err != nil { - return annotations, err - } - - for _, annotation := range annotationsList { - parts := strings.SplitN(annotation, ": ", 2) - if len(parts) != 2 { - return annotations, fmt.Errorf("Invalid annotation format: %s", annotation) - } - - annotations[parts[0]] = parts[1] - } - - return annotations, nil + return common.PropertyMapGet("scheduler-k3s", appName, fmt.Sprintf("%s.%s", processType, resourceType)) } func getDeployTimeout(appName string) string { @@ -1216,24 +1201,9 @@ func getGlobalLabel(appName string) (ProcessLabels, error) { return getLabels(appName, GlobalProcessType) } -// getLabel retrieves an label for a given app, process type, and resource type +// getLabel retrieves a label for a given app, process type, and resource type func getLabel(appName string, processType string, resourceType string) (map[string]string, error) { - labels := map[string]string{} - labelsList, err := common.PropertyListGet("scheduler-k3s", appName, fmt.Sprintf("labels.%s.%s", processType, resourceType)) - if err != nil { - return labels, err - } - - for _, label := range labelsList { - parts := strings.SplitN(label, ": ", 2) - if len(parts) != 2 { - return labels, fmt.Errorf("Invalid label format: %s", label) - } - - labels[parts[0]] = parts[1] - } - - return labels, nil + return common.PropertyMapGet("scheduler-k3s", appName, fmt.Sprintf("labels.%s.%s", processType, resourceType)) } func getLetsencryptServer(appName string) string { diff --git a/plugins/scheduler-k3s/subcommands.go b/plugins/scheduler-k3s/subcommands.go index 63ec17e20..422d22bfd 100644 --- a/plugins/scheduler-k3s/subcommands.go +++ b/plugins/scheduler-k3s/subcommands.go @@ -31,31 +31,15 @@ func CommandAnnotationsSet(appName string, processType string, resourceType stri } property := fmt.Sprintf("%s.%s", processType, resourceType) - annotationsList, err := common.PropertyListGet("scheduler-k3s", appName, property) - if err != nil { - return fmt.Errorf("Unable to get property list: %w", err) - } - - annotations := []string{} - for _, annotation := range annotationsList { - parts := strings.SplitN(annotation, ": ", 2) - if len(parts) != 2 { - return fmt.Errorf("Invalid annotation: %s", annotation) + if value == "" { + if err := common.PropertyMapDelete("scheduler-k3s", appName, property, key); err != nil { + return fmt.Errorf("Unable to delete property map entry: %w", err) } - if key == parts[0] { - continue - } - - annotations = append(annotations, annotation) + return nil } - if value != "" { - annotations = append(annotations, fmt.Sprintf("%s: %s", key, value)) - } - - sort.Strings(annotations) - if err := common.PropertyListWrite("scheduler-k3s", appName, property, annotations); err != nil { - return fmt.Errorf("Unable to write property list: %w", err) + if err := common.PropertyMapSet("scheduler-k3s", appName, property, key, value); err != nil { + return fmt.Errorf("Unable to set property map entry: %w", err) } return nil @@ -1183,31 +1167,15 @@ func CommandLabelsSet(appName string, processType string, resourceType string, k } property := fmt.Sprintf("labels.%s.%s", processType, resourceType) - labelsList, err := common.PropertyListGet("scheduler-k3s", appName, property) - if err != nil { - return fmt.Errorf("Unable to get property list: %w", err) - } - - labels := []string{} - for _, annotation := range labelsList { - parts := strings.SplitN(annotation, ": ", 2) - if len(parts) != 2 { - return fmt.Errorf("Invalid annotation: %s", annotation) + if value == "" { + if err := common.PropertyMapDelete("scheduler-k3s", appName, property, key); err != nil { + return fmt.Errorf("Unable to delete property map entry: %w", err) } - if key == parts[0] { - continue - } - - labels = append(labels, annotation) + return nil } - if value != "" { - labels = append(labels, fmt.Sprintf("%s: %s", key, value)) - } - - sort.Strings(labels) - if err := common.PropertyListWrite("scheduler-k3s", appName, property, labels); err != nil { - return fmt.Errorf("Unable to write property list: %w", err) + if err := common.PropertyMapSet("scheduler-k3s", appName, property, key, value); err != nil { + return fmt.Errorf("Unable to set property map entry: %w", err) } return nil diff --git a/plugins/scheduler-k3s/triggers.go b/plugins/scheduler-k3s/triggers.go index 8ff3c50f8..f6cfcae4a 100644 --- a/plugins/scheduler-k3s/triggers.go +++ b/plugins/scheduler-k3s/triggers.go @@ -79,6 +79,10 @@ func TriggerInstall() error { return fmt.Errorf("Unable to migrate chart properties: %w", err) } + if err := migrateAnnotationsLabelsToMapFormat(); err != nil { + return fmt.Errorf("Unable to migrate annotations and labels: %w", err) + } + if err := syncExistingCertificates(); err != nil { common.LogWarn(fmt.Sprintf("Warning: failed to sync existing certificates: %v", err)) } @@ -126,6 +130,105 @@ func migrateChartPropertiesToMapFormat() error { return nil } +// migrateAnnotationsLabelsToMapFormat converts the legacy line-formatted +// annotation and label property files ("key: value" per line) into JSON maps +// written via PropertyMapWrite. The property name (e.g. "web.deployment", +// "labels.global.service") is unchanged - only the file content layout shifts. +// Without this migration, post-upgrade reads via PropertyMapGet would fail to +// parse the legacy line format as JSON. Idempotent: any property whose content +// is already JSON (or empty) is skipped by probing PropertyMapGet first. +func migrateAnnotationsLabelsToMapFormat() error { + annotationResources := map[string]bool{} + for _, rt := range AnnotationResourceTypes { + annotationResources[rt] = true + } + labelResources := map[string]bool{} + for _, rt := range LabelResourceTypes { + labelResources[rt] = true + } + + scopes := []string{"--global"} + apps, err := common.UnfilteredDokkuApps() + if err != nil && !errors.Is(err, common.NoAppsExist) { + return fmt.Errorf("Unable to list apps for annotation/label migration: %w", err) + } + scopes = append(scopes, apps...) + + for _, scope := range scopes { + properties, err := common.PropertyGetAllByPrefix("scheduler-k3s", scope, "") + if err != nil { + return fmt.Errorf("Unable to list properties for %s: %w", scope, err) + } + + for propertyName := range properties { + isLabel := strings.HasPrefix(propertyName, "labels.") + checkName := propertyName + if isLabel { + checkName = strings.TrimPrefix(propertyName, "labels.") + } else if isReservedAnnotationProperty(propertyName) { + continue + } + + dot := strings.LastIndex(checkName, ".") + if dot <= 0 || dot == len(checkName)-1 { + continue + } + + processType := checkName[:dot] + resourceType := checkName[dot+1:] + if processType == "" { + continue + } + + if isLabel { + if !labelResources[resourceType] { + continue + } + } else { + if !annotationResources[resourceType] { + continue + } + } + + if err := migrateAnnotationLabelProperty(scope, propertyName); err != nil { + return err + } + } + } + return nil +} + +// migrateAnnotationLabelProperty rewrites one legacy line-formatted property +// file as a JSON map. It is a no-op when the file is already valid JSON or +// empty (idempotent). +func migrateAnnotationLabelProperty(scope string, propertyName string) error { + if _, err := common.PropertyMapGet("scheduler-k3s", scope, propertyName); err == nil { + return nil + } + + lines, err := common.PropertyListGet("scheduler-k3s", scope, propertyName) + if err != nil { + return fmt.Errorf("Unable to read legacy property %s/%s: %w", scope, propertyName, err) + } + + m := map[string]string{} + for _, line := range lines { + if line == "" { + continue + } + parts := strings.SplitN(line, ": ", 2) + if len(parts) != 2 { + return fmt.Errorf("Invalid legacy entry in %s/%s: %s", scope, propertyName, line) + } + m[parts[0]] = parts[1] + } + + if err := common.PropertyMapWrite("scheduler-k3s", scope, propertyName, m); err != nil { + return fmt.Errorf("Unable to write %s/%s as map: %w", scope, propertyName, err) + } + return nil +} + // TriggerPostCertsUpdate handles post-certs-update trigger func TriggerPostCertsUpdate(appName string) error { scheduler := common.PropertyGetDefault("scheduler", appName, "selected", "") diff --git a/plugins/scheduler-k3s/triggers_test.go b/plugins/scheduler-k3s/triggers_test.go index 413512ede..3b8afa20e 100644 --- a/plugins/scheduler-k3s/triggers_test.go +++ b/plugins/scheduler-k3s/triggers_test.go @@ -1,6 +1,9 @@ package scheduler_k3s import ( + "encoding/json" + "os" + "path/filepath" "testing" "github.com/dokku/dokku/plugins/common" @@ -18,6 +21,28 @@ func setupChartMigrationTest(t *testing.T) { Expect(common.PropertySetup("scheduler-k3s")).To(Succeed()) } +func setupAnnotationsLabelsMigrationTest(t *testing.T, apps ...string) { + t.Helper() + setupChartMigrationTest(t) + dokkuRoot := t.TempDir() + t.Setenv("DOKKU_ROOT", dokkuRoot) + for _, appName := range apps { + Expect(os.MkdirAll(filepath.Join(dokkuRoot, appName), 0755)).To(Succeed()) + } +} + +// writeLegacyAnnotationLabelFile seeds a property file directly using +// PropertyListWrite, matching the pre-migration on-disk layout (one +// "key: value" entry per line). +func writeLegacyAnnotationLabelFile(t *testing.T, scope string, property string, entries map[string]string) { + t.Helper() + lines := []string{} + for k, v := range entries { + lines = append(lines, k+": "+v) + } + Expect(common.PropertyListWrite("scheduler-k3s", scope, property, lines)).To(Succeed()) +} + func TestMigrateChartPropertiesToMapFormat(t *testing.T) { setupChartMigrationTest(t) @@ -86,3 +111,86 @@ func TestMigrateChartPropertiesToMapFormatNoLegacyIsNoOp(t *testing.T) { Expect(common.PropertyExists("scheduler-k3s", "--global", "chart-overrides."+chart.ReleaseName)).To(BeFalse()) } } + +func TestMigrateAnnotationsLabelsToMapFormat(t *testing.T) { + setupAnnotationsLabelsMigrationTest(t, "node-js-app") + + writeLegacyAnnotationLabelFile(t, "node-js-app", "web.deployment", map[string]string{ + "prometheus.io/scrape": "true", + "team": "platform", + }) + writeLegacyAnnotationLabelFile(t, "node-js-app", "labels.global.service", map[string]string{ + "app.kubernetes.io/managed-by": "dokku", + }) + writeLegacyAnnotationLabelFile(t, "--global", "global.deployment", map[string]string{ + "owner": "ops", + }) + + Expect(migrateAnnotationsLabelsToMapFormat()).To(Succeed()) + + appAnnotations, err := common.PropertyMapGet("scheduler-k3s", "node-js-app", "web.deployment") + Expect(err).NotTo(HaveOccurred()) + Expect(appAnnotations).To(Equal(map[string]string{ + "prometheus.io/scrape": "true", + "team": "platform", + })) + + appLabels, err := common.PropertyMapGet("scheduler-k3s", "node-js-app", "labels.global.service") + Expect(err).NotTo(HaveOccurred()) + Expect(appLabels).To(Equal(map[string]string{ + "app.kubernetes.io/managed-by": "dokku", + })) + + globalAnnotations, err := common.PropertyMapGet("scheduler-k3s", "--global", "global.deployment") + Expect(err).NotTo(HaveOccurred()) + Expect(globalAnnotations).To(Equal(map[string]string{"owner": "ops"})) + + raw, err := os.ReadFile(filepath.Join(common.MustGetEnv("DOKKU_LIB_ROOT"), "config", "scheduler-k3s", "node-js-app", "web.deployment")) + Expect(err).NotTo(HaveOccurred()) + var decoded map[string]string + Expect(json.Unmarshal(raw, &decoded)).To(Succeed()) + Expect(decoded).To(Equal(appAnnotations)) +} + +func TestMigrateAnnotationsLabelsToMapFormatIsIdempotent(t *testing.T) { + setupAnnotationsLabelsMigrationTest(t, "node-js-app") + + writeLegacyAnnotationLabelFile(t, "node-js-app", "web.deployment", map[string]string{ + "prometheus.io/scrape": "true", + }) + + Expect(migrateAnnotationsLabelsToMapFormat()).To(Succeed()) + Expect(migrateAnnotationsLabelsToMapFormat()).To(Succeed()) + + got, err := common.PropertyMapGet("scheduler-k3s", "node-js-app", "web.deployment") + Expect(err).NotTo(HaveOccurred()) + Expect(got).To(Equal(map[string]string{"prometheus.io/scrape": "true"})) +} + +func TestMigrateAnnotationsLabelsToMapFormatPreservesAlreadyMigrated(t *testing.T) { + setupAnnotationsLabelsMigrationTest(t, "node-js-app") + + preserved := map[string]string{ + "prometheus.io/scrape": "true", + "controller.config": "line one\nline two\nline three", + } + Expect(common.PropertyMapWrite("scheduler-k3s", "node-js-app", "web.deployment", preserved)).To(Succeed()) + + Expect(migrateAnnotationsLabelsToMapFormat()).To(Succeed()) + + got, err := common.PropertyMapGet("scheduler-k3s", "node-js-app", "web.deployment") + Expect(err).NotTo(HaveOccurred()) + Expect(got).To(Equal(preserved)) +} + +func TestMigrateAnnotationsLabelsToMapFormatSkipsReservedPrefixes(t *testing.T) { + setupAnnotationsLabelsMigrationTest(t) + + Expect(common.PropertyWrite("scheduler-k3s", "--global", "chart.cert-manager.deployment", "irrelevant")).To(Succeed()) + Expect(common.PropertyWrite("scheduler-k3s", "--global", "node-profile-foo.json", "{}")).To(Succeed()) + + Expect(migrateAnnotationsLabelsToMapFormat()).To(Succeed()) + + Expect(common.PropertyGet("scheduler-k3s", "--global", "chart.cert-manager.deployment")).To(Equal("irrelevant")) + Expect(common.PropertyGet("scheduler-k3s", "--global", "node-profile-foo.json")).To(Equal("{}")) +} diff --git a/tests/unit/scheduler-k3s-annotations.bats b/tests/unit/scheduler-k3s-annotations.bats index 2ae147d4e..709e61369 100644 --- a/tests/unit/scheduler-k3s-annotations.bats +++ b/tests/unit/scheduler-k3s-annotations.bats @@ -86,3 +86,13 @@ teardown() { assert_output_contains "app1-val" assert_output_contains "app2-val" } + +@test "(scheduler-k3s:annotations:set) preserves multi-line values and / in keys" { + local value=$'line one\nline two\nline three' + run /bin/bash -c "dokku scheduler-k3s:annotations:set $TEST_APP --resource-type deployment prometheus.io/scrape \"$value\"" + assert_success + + run /bin/bash -c "dokku scheduler-k3s:annotations:report $TEST_APP --format json | jq -r '.\"global.deployment.prometheus.io/scrape\"'" + assert_success + assert_output "$value" +} diff --git a/tests/unit/scheduler-k3s-labels.bats b/tests/unit/scheduler-k3s-labels.bats index 98d7360b3..3d417d21a 100644 --- a/tests/unit/scheduler-k3s-labels.bats +++ b/tests/unit/scheduler-k3s-labels.bats @@ -86,3 +86,13 @@ teardown() { assert_output_contains "app1-val" assert_output_contains "app2-val" } + +@test "(scheduler-k3s:labels:set) preserves multi-line values and / in keys" { + local value=$'line one\nline two\nline three' + run /bin/bash -c "dokku scheduler-k3s:labels:set $TEST_APP --resource-type deployment app.kubernetes.io/part-of \"$value\"" + assert_success + + run /bin/bash -c "dokku scheduler-k3s:labels:report $TEST_APP --format json | jq -r '.\"global.deployment.app.kubernetes.io/part-of\"'" + assert_success + assert_output "$value" +}