From b8929712d405de6823fb0851fad35371d301e3d7 Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Sun, 31 May 2026 01:54:28 -0400 Subject: [PATCH 1/2] fix: persist scheduler-k3s chart overrides as JSON maps Chart property names containing `/` failed because per-key flat-file storage interpreted the slash as a filesystem path separator, and any move to line-based storage would silently truncate multi-line values. A new `PropertyMap*` helper family in `common` persists each map as a single JSON file so both `/` in keys and `\n` in values round-trip losslessly. Chart overrides move to `chart-overrides.` and an idempotent `TriggerInstall` migration rewrites any legacy `chart..` files into the new layout. The deprecated `scheduler-k3s:set --global chart.*` form is rerouted through the same map storage so it does not silently orphan writes. Closes #8717. --- docs/deployment/schedulers/k3s.md | 2 + plugins/common/properties.go | 94 ++++++++++ plugins/common/properties_test.go | 168 ++++++++++++++++++ plugins/scheduler-k3s/functions.go | 3 +- plugins/scheduler-k3s/report.go | 19 +- plugins/scheduler-k3s/scheduler_k3s.go | 1 + plugins/scheduler-k3s/subcommands.go | 36 ++-- plugins/scheduler-k3s/triggers.go | 44 +++++ plugins/scheduler-k3s/triggers_test.go | 86 +++++++++ ...harts.bats => scheduler-k3s-charts-1.bats} | 0 tests/unit/scheduler-k3s-charts-2.bats | 57 ++++++ 11 files changed, 488 insertions(+), 22 deletions(-) create mode 100644 plugins/common/properties_test.go create mode 100644 plugins/scheduler-k3s/triggers_test.go rename tests/unit/{scheduler-k3s-charts.bats => scheduler-k3s-charts-1.bats} (100%) create mode 100644 tests/unit/scheduler-k3s-charts-2.bats diff --git a/docs/deployment/schedulers/k3s.md b/docs/deployment/schedulers/k3s.md index 207562fa6..847dd46d5 100644 --- a/docs/deployment/schedulers/k3s.md +++ b/docs/deployment/schedulers/k3s.md @@ -716,6 +716,8 @@ dokku scheduler-k3s:charts:set cert-manager.version 1.13.3 > [!NOTE] > Properties follow dot-notation, and are expanded according to Helm's internal logic. See the [Helm documentation](https://helm.sh/docs/helm/helm_install/#helm-install) for `helm install` for further details. +Property names may contain `/` (e.g. for Kubernetes-style annotation keys such as `service.annotations.prometheus.io/scrape`) and values may span multiple lines; both are preserved verbatim. + To unset a chart property, omit the value from the `scheduler-k3s:charts:set` call: ```shell diff --git a/plugins/common/properties.go b/plugins/common/properties.go index a91f8d5e7..d5773541c 100644 --- a/plugins/common/properties.go +++ b/plugins/common/properties.go @@ -2,6 +2,7 @@ package common import ( "bufio" + "encoding/json" "errors" "fmt" "os" @@ -514,6 +515,99 @@ func PropertyWrite(pluginName string, appName string, property string, value str return nil } +// PropertyMapWrite persists a string→string map for a given property as a single +// JSON file. Unlike PropertyWrite/PropertyListWrite, map *keys* may contain any +// bytes (including '/' or '\n') because they live inside file content rather +// than in the on-disk filename. A nil map is persisted as "{}". +func PropertyMapWrite(pluginName string, appName string, property string, m map[string]string) error { + if err := propertyTouch(pluginName, appName, property); err != nil { + return err + } + + if m == nil { + m = map[string]string{} + } + + b, err := json.Marshal(m) + if err != nil { + return fmt.Errorf("Unable to marshal %s map property %s.%s: %w", pluginName, appName, property, err) + } + + propertyPath := getPropertyPath(pluginName, appName, property) + if err := os.WriteFile(propertyPath, b, 0600); err != nil { + return fmt.Errorf("Unable to write %s map property %s.%s: %w", pluginName, appName, property, err) + } + + if err := SetPermissions(SetPermissionInput{ + Filename: propertyPath, + Mode: os.FileMode(0600), + }); err != nil { + return fmt.Errorf("Unable to set permissions for %s map property %s.%s: %w", pluginName, appName, property, err) + } + + return nil +} + +// PropertyMapGet returns the string→string map persisted for a property. A +// missing or empty-file property is reported as an empty map without error. +func PropertyMapGet(pluginName string, appName string, property string) (map[string]string, error) { + m := map[string]string{} + if !PropertyExists(pluginName, appName, property) { + return m, nil + } + + b, err := os.ReadFile(getPropertyPath(pluginName, appName, property)) + if err != nil { + return nil, fmt.Errorf("Unable to read %s map property %s.%s: %w", pluginName, appName, property, err) + } + + if len(b) == 0 { + return m, nil + } + + if err := json.Unmarshal(b, &m); err != nil { + return nil, fmt.Errorf("Unable to parse %s map property %s.%s as JSON: %w", pluginName, appName, property, err) + } + return m, nil +} + +// PropertyMapSet reads the current map, sets one key, and writes the map back. +func PropertyMapSet(pluginName string, appName string, property string, key string, value string) error { + m, err := PropertyMapGet(pluginName, appName, property) + if err != nil { + return err + } + m[key] = value + return PropertyMapWrite(pluginName, appName, property, m) +} + +// PropertyMapDelete reads the current map, removes one key, and writes the map +// back. Removing a key that is not present is a no-op. +func PropertyMapDelete(pluginName string, appName string, property string, key string) error { + if !PropertyExists(pluginName, appName, property) { + return nil + } + m, err := PropertyMapGet(pluginName, appName, property) + if err != nil { + return err + } + if _, ok := m[key]; !ok { + return nil + } + delete(m, key) + return PropertyMapWrite(pluginName, appName, property, m) +} + +// PropertyMapLength returns the number of entries in the map persisted for a +// property. A missing property is reported as length 0 without error. +func PropertyMapLength(pluginName string, appName string, property string) (int, error) { + m, err := PropertyMapGet(pluginName, appName, property) + if err != nil { + return 0, err + } + return len(m), nil +} + // PropertySetup creates the plugin config root func PropertySetup(pluginName string) error { configRoot := filepath.Join(MustGetEnv("DOKKU_LIB_ROOT"), "config") diff --git a/plugins/common/properties_test.go b/plugins/common/properties_test.go new file mode 100644 index 000000000..3c2954720 --- /dev/null +++ b/plugins/common/properties_test.go @@ -0,0 +1,168 @@ +package common + +import ( + "os" + "path/filepath" + "testing" + + . "github.com/onsi/gomega" +) + +const ( + propertyTestPlugin = "test-plugin" + propertyTestApp = "--global" + propertyTestName = "chart-overrides.traefik" +) + +func setupPropertyMapTest(t *testing.T) { + t.Helper() + RegisterTestingT(t) + Expect(setupTests()).To(Succeed()) + t.Setenv("DOKKU_LIB_ROOT", t.TempDir()) + Expect(PropertySetup(propertyTestPlugin)).To(Succeed()) +} + +func TestPropertyMapWriteAndGetRoundTrip(t *testing.T) { + setupPropertyMapTest(t) + + want := map[string]string{"installCRDs": "false", "version": "1.13.3"} + Expect(PropertyMapWrite(propertyTestPlugin, propertyTestApp, propertyTestName, want)).To(Succeed()) + + got, err := PropertyMapGet(propertyTestPlugin, propertyTestApp, propertyTestName) + Expect(err).NotTo(HaveOccurred()) + Expect(got).To(Equal(want)) +} + +func TestPropertyMapHandlesSlashInKey(t *testing.T) { + setupPropertyMapTest(t) + + key := "service.annotations.prometheus.io/scrape" + Expect(PropertyMapSet(propertyTestPlugin, propertyTestApp, propertyTestName, key, "true")).To(Succeed()) + + got, err := PropertyMapGet(propertyTestPlugin, propertyTestApp, propertyTestName) + Expect(err).NotTo(HaveOccurred()) + Expect(got).To(HaveKeyWithValue(key, "true")) +} + +func TestPropertyMapHandlesNewlineInValue(t *testing.T) { + setupPropertyMapTest(t) + + value := "line one\nline two\nline three" + Expect(PropertyMapSet(propertyTestPlugin, propertyTestApp, propertyTestName, "controller.config", value)).To(Succeed()) + + got, err := PropertyMapGet(propertyTestPlugin, propertyTestApp, propertyTestName) + Expect(err).NotTo(HaveOccurred()) + Expect(got).To(HaveKeyWithValue("controller.config", value)) +} + +func TestPropertyMapEmptyMap(t *testing.T) { + setupPropertyMapTest(t) + + Expect(PropertyMapWrite(propertyTestPlugin, propertyTestApp, propertyTestName, map[string]string{})).To(Succeed()) + + got, err := PropertyMapGet(propertyTestPlugin, propertyTestApp, propertyTestName) + Expect(err).NotTo(HaveOccurred()) + Expect(got).To(BeEmpty()) +} + +func TestPropertyMapNilMapPersistsAsEmptyJSON(t *testing.T) { + setupPropertyMapTest(t) + + Expect(PropertyMapWrite(propertyTestPlugin, propertyTestApp, propertyTestName, nil)).To(Succeed()) + + b, err := os.ReadFile(getPropertyPath(propertyTestPlugin, propertyTestApp, propertyTestName)) + Expect(err).NotTo(HaveOccurred()) + Expect(string(b)).To(Equal("{}")) +} + +func TestPropertyMapGetMissingReturnsEmpty(t *testing.T) { + setupPropertyMapTest(t) + + got, err := PropertyMapGet(propertyTestPlugin, propertyTestApp, "chart-overrides.never-written") + Expect(err).NotTo(HaveOccurred()) + Expect(got).To(BeEmpty()) +} + +func TestPropertyMapSetCreatesMissingFile(t *testing.T) { + setupPropertyMapTest(t) + + Expect(PropertyExists(propertyTestPlugin, propertyTestApp, propertyTestName)).To(BeFalse()) + Expect(PropertyMapSet(propertyTestPlugin, propertyTestApp, propertyTestName, "version", "1.0")).To(Succeed()) + Expect(PropertyExists(propertyTestPlugin, propertyTestApp, propertyTestName)).To(BeTrue()) + + got, err := PropertyMapGet(propertyTestPlugin, propertyTestApp, propertyTestName) + Expect(err).NotTo(HaveOccurred()) + Expect(got).To(Equal(map[string]string{"version": "1.0"})) +} + +func TestPropertyMapDeleteRemovesKeyOnly(t *testing.T) { + setupPropertyMapTest(t) + + Expect(PropertyMapSet(propertyTestPlugin, propertyTestApp, propertyTestName, "version", "1.0")).To(Succeed()) + Expect(PropertyMapSet(propertyTestPlugin, propertyTestApp, propertyTestName, "installCRDs", "false")).To(Succeed()) + + Expect(PropertyMapDelete(propertyTestPlugin, propertyTestApp, propertyTestName, "version")).To(Succeed()) + + got, err := PropertyMapGet(propertyTestPlugin, propertyTestApp, propertyTestName) + Expect(err).NotTo(HaveOccurred()) + Expect(got).To(Equal(map[string]string{"installCRDs": "false"})) +} + +func TestPropertyMapDeleteMissingKeyIsNoOp(t *testing.T) { + setupPropertyMapTest(t) + + Expect(PropertyMapSet(propertyTestPlugin, propertyTestApp, propertyTestName, "version", "1.0")).To(Succeed()) + + Expect(PropertyMapDelete(propertyTestPlugin, propertyTestApp, propertyTestName, "missing")).To(Succeed()) + Expect(PropertyMapDelete(propertyTestPlugin, propertyTestApp, "chart-overrides.never-written", "anything")).To(Succeed()) + + got, err := PropertyMapGet(propertyTestPlugin, propertyTestApp, propertyTestName) + Expect(err).NotTo(HaveOccurred()) + Expect(got).To(Equal(map[string]string{"version": "1.0"})) +} + +func TestPropertyMapLength(t *testing.T) { + setupPropertyMapTest(t) + + length, err := PropertyMapLength(propertyTestPlugin, propertyTestApp, "chart-overrides.never-written") + Expect(err).NotTo(HaveOccurred()) + Expect(length).To(Equal(0)) + + Expect(PropertyMapSet(propertyTestPlugin, propertyTestApp, propertyTestName, "a", "1")).To(Succeed()) + Expect(PropertyMapSet(propertyTestPlugin, propertyTestApp, propertyTestName, "b", "2")).To(Succeed()) + + length, err = PropertyMapLength(propertyTestPlugin, propertyTestApp, propertyTestName) + Expect(err).NotTo(HaveOccurred()) + Expect(length).To(Equal(2)) +} + +func TestPropertyMapRewriteIsStable(t *testing.T) { + setupPropertyMapTest(t) + + Expect(PropertyMapSet(propertyTestPlugin, propertyTestApp, propertyTestName, "version", "1.0")).To(Succeed()) + Expect(PropertyMapSet(propertyTestPlugin, propertyTestApp, propertyTestName, "version", "1.0")).To(Succeed()) + + got, err := PropertyMapGet(propertyTestPlugin, propertyTestApp, propertyTestName) + Expect(err).NotTo(HaveOccurred()) + Expect(got).To(Equal(map[string]string{"version": "1.0"})) +} + +func TestPropertyMapFileMode(t *testing.T) { + setupPropertyMapTest(t) + + Expect(PropertyMapSet(propertyTestPlugin, propertyTestApp, propertyTestName, "version", "1.0")).To(Succeed()) + + fi, err := os.Stat(filepath.Clean(getPropertyPath(propertyTestPlugin, propertyTestApp, propertyTestName))) + Expect(err).NotTo(HaveOccurred()) + Expect(fi.Mode().Perm()).To(Equal(os.FileMode(0600))) +} + +func TestPropertyMapMalformedJSONReturnsError(t *testing.T) { + setupPropertyMapTest(t) + + Expect(PropertyMapWrite(propertyTestPlugin, propertyTestApp, propertyTestName, map[string]string{"k": "v"})).To(Succeed()) + Expect(os.WriteFile(getPropertyPath(propertyTestPlugin, propertyTestApp, propertyTestName), []byte("not-json"), 0600)).To(Succeed()) + + _, err := PropertyMapGet(propertyTestPlugin, propertyTestApp, propertyTestName) + Expect(err).To(HaveOccurred()) +} diff --git a/plugins/scheduler-k3s/functions.go b/plugins/scheduler-k3s/functions.go index fc0e2e7e3..e40ab7adb 100644 --- a/plugins/scheduler-k3s/functions.go +++ b/plugins/scheduler-k3s/functions.go @@ -1774,12 +1774,11 @@ func installHelmCharts(ctx context.Context, clientset KubernetesClient, shouldIn values = updateVectorValues(values) } - chartProperties, err := common.PropertyGetAllByPrefix("scheduler-k3s", "--global", "chart."+chart.ReleaseName+".") + chartProperties, err := common.PropertyMapGet("scheduler-k3s", "--global", "chart-overrides."+chart.ReleaseName) if err != nil { return fmt.Errorf("Error getting chart properties: %w", err) } for key, value := range chartProperties { - key = strings.TrimPrefix(key, "chart."+chart.ReleaseName+".") strval := fmt.Sprintf("%s=%s", key, value) if err := strvals.ParseInto(strval, values); err != nil { return fmt.Errorf("Error parsing chart property %s: %w", strval, err) diff --git a/plugins/scheduler-k3s/report.go b/plugins/scheduler-k3s/report.go index 31143d668..2b21d91af 100644 --- a/plugins/scheduler-k3s/report.go +++ b/plugins/scheduler-k3s/report.go @@ -105,14 +105,17 @@ func ReportSingleApp(appName string, format string, infoFlag string) error { } } - chartProperties, err := common.PropertyGetAllByPrefix("scheduler-k3s", "--global", "chart.") - if err != nil { - return fmt.Errorf("Unable to get property list: %w", err) - } - for name, value := range chartProperties { - flagName := "--scheduler-k3s-global-" + name - flags[flagName] = func(appName string) string { - return value + for _, chart := range HelmCharts { + chartOverrides, err := common.PropertyMapGet("scheduler-k3s", "--global", "chart-overrides."+chart.ReleaseName) + if err != nil { + return fmt.Errorf("Unable to get property list: %w", err) + } + for key, value := range chartOverrides { + flagName := fmt.Sprintf("--scheduler-k3s-global-chart.%s.%s", chart.ReleaseName, key) + value := value + flags[flagName] = func(appName string) string { + return value + } } } diff --git a/plugins/scheduler-k3s/scheduler_k3s.go b/plugins/scheduler-k3s/scheduler_k3s.go index 0f74b553f..0a1ada759 100644 --- a/plugins/scheduler-k3s/scheduler_k3s.go +++ b/plugins/scheduler-k3s/scheduler_k3s.go @@ -95,6 +95,7 @@ var LabelResourceTypes = []string{ // exclude such properties when scanning the property store. var reservedAnnotationPrefixes = []string{ "chart.", + "chart-overrides.", "labels.", "node-profile-", TriggerAuthPropertyPrefix, diff --git a/plugins/scheduler-k3s/subcommands.go b/plugins/scheduler-k3s/subcommands.go index e49b1d1b6..63ec17e20 100644 --- a/plugins/scheduler-k3s/subcommands.go +++ b/plugins/scheduler-k3s/subcommands.go @@ -471,16 +471,17 @@ func CommandChartsSet(propertyArg string, value string) error { } key := fmt.Sprintf("chart.%s.%s", chartName, chartProperty) + mapProperty := "chart-overrides." + chartName if value != "" { common.LogInfo2Quiet(fmt.Sprintf("Setting %s to %s", key, value)) - if err := common.PropertyWrite("scheduler-k3s", "--global", key, value); err != nil { + if err := common.PropertyMapSet("scheduler-k3s", "--global", mapProperty, chartProperty, value); err != nil { return fmt.Errorf("Unable to write property: %w", err) } return nil } common.LogInfo2Quiet(fmt.Sprintf("Unsetting %s", key)) - if err := common.PropertyDelete("scheduler-k3s", "--global", key); err != nil { + if err := common.PropertyMapDelete("scheduler-k3s", "--global", mapProperty, chartProperty); err != nil { return fmt.Errorf("Unable to delete property: %w", err) } return nil @@ -522,16 +523,12 @@ func CommandChartsReport(chartName string, format string, infoFlag string) error flatOverrides := map[string]string{} flagToValue := map[string]string{} for _, chart := range charts { - properties, err := common.PropertyGetAllByPrefix("scheduler-k3s", "--global", "chart."+chart.ReleaseName+".") + chartOverrides, err := common.PropertyMapGet("scheduler-k3s", "--global", "chart-overrides."+chart.ReleaseName) if err != nil { return fmt.Errorf("Unable to get chart properties: %w", err) } - chartOverrides := map[string]string{} - prefix := "chart." + chart.ReleaseName + "." - for key, value := range properties { - overrideKey := strings.TrimPrefix(key, prefix) - chartOverrides[overrideKey] = value + for overrideKey, value := range chartOverrides { flatKey := chart.ReleaseName + "." + overrideKey flatOverrides[flatKey] = value flagToValue["--scheduler-k3s-charts-"+flatKey] = value @@ -1372,17 +1369,32 @@ func CommandSet(appName string, property string, value string) error { } chartName := chartParts[1] + chartProperty := chartParts[2] + chartFound := false for _, chart := range HelmCharts { if chart.ReleaseName == chartName { - validProperties[property] = "" - globalProperties[property] = true + chartFound = true break } } - - if _, ok := validProperties[property]; !ok { + if !chartFound { return fmt.Errorf("Invalid chart property, no matching chart found: %s", property) } + + mapProperty := "chart-overrides." + chartName + if value != "" { + common.LogInfo2Quiet(fmt.Sprintf("Setting %s to %s", property, value)) + if err := common.PropertyMapSet("scheduler-k3s", "--global", mapProperty, chartProperty, value); err != nil { + return fmt.Errorf("Unable to write property: %w", err) + } + return nil + } + + common.LogInfo2Quiet(fmt.Sprintf("Unsetting %s", property)) + if err := common.PropertyMapDelete("scheduler-k3s", "--global", mapProperty, chartProperty); err != nil { + return fmt.Errorf("Unable to delete property: %w", err) + } + return nil } common.CommandPropertySet("scheduler-k3s", appName, property, value, validProperties, globalProperties) diff --git a/plugins/scheduler-k3s/triggers.go b/plugins/scheduler-k3s/triggers.go index fdd609f1d..8ff3c50f8 100644 --- a/plugins/scheduler-k3s/triggers.go +++ b/plugins/scheduler-k3s/triggers.go @@ -75,6 +75,10 @@ func TriggerInstall() error { return err } + if err := migrateChartPropertiesToMapFormat(); err != nil { + return fmt.Errorf("Unable to migrate chart properties: %w", err) + } + if err := syncExistingCertificates(); err != nil { common.LogWarn(fmt.Sprintf("Warning: failed to sync existing certificates: %v", err)) } @@ -82,6 +86,46 @@ func TriggerInstall() error { return nil } +// migrateChartPropertiesToMapFormat moves legacy per-key chart.. +// property files into a single chart-overrides. JSON map per chart. +// Without this migration, a dokku upgrade would silently strand any existing +// chart overrides because the new render path only reads chart-overrides.*. +// Idempotent: once the legacy files are gone the helper short-circuits per +// chart, and legacy values take precedence over any partial pre-existing +// entries on re-runs. +func migrateChartPropertiesToMapFormat() error { + for _, chart := range HelmCharts { + prefix := "chart." + chart.ReleaseName + "." + legacy, err := common.PropertyGetAllByPrefix("scheduler-k3s", "--global", prefix) + if err != nil { + return fmt.Errorf("Unable to read legacy chart properties for %s: %w", chart.ReleaseName, err) + } + if len(legacy) == 0 { + continue + } + + mapProperty := "chart-overrides." + chart.ReleaseName + merged, err := common.PropertyMapGet("scheduler-k3s", "--global", mapProperty) + if err != nil { + return fmt.Errorf("Unable to read existing chart-overrides for %s: %w", chart.ReleaseName, err) + } + for fullKey, value := range legacy { + merged[strings.TrimPrefix(fullKey, prefix)] = value + } + + if err := common.PropertyMapWrite("scheduler-k3s", "--global", mapProperty, merged); err != nil { + return fmt.Errorf("Unable to write chart-overrides for %s: %w", chart.ReleaseName, err) + } + + for fullKey := range legacy { + if err := common.PropertyDelete("scheduler-k3s", "--global", fullKey); err != nil { + return fmt.Errorf("Unable to remove legacy chart property %s: %w", fullKey, 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 new file mode 100644 index 000000000..3f142eaec --- /dev/null +++ b/plugins/scheduler-k3s/triggers_test.go @@ -0,0 +1,86 @@ +package scheduler_k3s + +import ( + "testing" + + "github.com/dokku/dokku/plugins/common" + . "github.com/onsi/gomega" +) + +func setupChartMigrationTest(t *testing.T) { + t.Helper() + RegisterTestingT(t) + t.Setenv("PLUGIN_PATH", "/var/lib/dokku/plugins") + t.Setenv("PLUGIN_ENABLED_PATH", "/var/lib/dokku/plugins/enabled") + t.Setenv("DOKKU_LIB_ROOT", t.TempDir()) + Expect(common.PropertySetup("scheduler-k3s")).To(Succeed()) +} + +func TestMigrateChartPropertiesToMapFormat(t *testing.T) { + setupChartMigrationTest(t) + + Expect(common.PropertyWrite("scheduler-k3s", "--global", "chart.cert-manager.installCRDs", "false")).To(Succeed()) + Expect(common.PropertyWrite("scheduler-k3s", "--global", "chart.cert-manager.version", "1.13.3")).To(Succeed()) + Expect(common.PropertyWrite("scheduler-k3s", "--global", "chart.traefik.replicas", "2")).To(Succeed()) + + Expect(migrateChartPropertiesToMapFormat()).To(Succeed()) + + certManager, err := common.PropertyMapGet("scheduler-k3s", "--global", "chart-overrides.cert-manager") + Expect(err).NotTo(HaveOccurred()) + Expect(certManager).To(Equal(map[string]string{ + "installCRDs": "false", + "version": "1.13.3", + })) + + traefik, err := common.PropertyMapGet("scheduler-k3s", "--global", "chart-overrides.traefik") + Expect(err).NotTo(HaveOccurred()) + Expect(traefik).To(Equal(map[string]string{"replicas": "2"})) + + Expect(common.PropertyExists("scheduler-k3s", "--global", "chart.cert-manager.installCRDs")).To(BeFalse()) + Expect(common.PropertyExists("scheduler-k3s", "--global", "chart.cert-manager.version")).To(BeFalse()) + Expect(common.PropertyExists("scheduler-k3s", "--global", "chart.traefik.replicas")).To(BeFalse()) +} + +func TestMigrateChartPropertiesToMapFormatIsIdempotent(t *testing.T) { + setupChartMigrationTest(t) + + Expect(common.PropertyWrite("scheduler-k3s", "--global", "chart.cert-manager.version", "1.13.3")).To(Succeed()) + + Expect(migrateChartPropertiesToMapFormat()).To(Succeed()) + Expect(migrateChartPropertiesToMapFormat()).To(Succeed()) + + got, err := common.PropertyMapGet("scheduler-k3s", "--global", "chart-overrides.cert-manager") + Expect(err).NotTo(HaveOccurred()) + Expect(got).To(Equal(map[string]string{"version": "1.13.3"})) +} + +func TestMigrateChartPropertiesToMapFormatPreservesExistingNonOverlappingKeys(t *testing.T) { + setupChartMigrationTest(t) + + Expect(common.PropertyMapWrite("scheduler-k3s", "--global", "chart-overrides.cert-manager", map[string]string{ + "installCRDs": "true", + "service.annotations.prometheus.io/scrape": "true", + })).To(Succeed()) + Expect(common.PropertyWrite("scheduler-k3s", "--global", "chart.cert-manager.installCRDs", "false")).To(Succeed()) + Expect(common.PropertyWrite("scheduler-k3s", "--global", "chart.cert-manager.version", "1.13.3")).To(Succeed()) + + Expect(migrateChartPropertiesToMapFormat()).To(Succeed()) + + got, err := common.PropertyMapGet("scheduler-k3s", "--global", "chart-overrides.cert-manager") + Expect(err).NotTo(HaveOccurred()) + Expect(got).To(Equal(map[string]string{ + "installCRDs": "false", + "service.annotations.prometheus.io/scrape": "true", + "version": "1.13.3", + })) +} + +func TestMigrateChartPropertiesToMapFormatNoLegacyIsNoOp(t *testing.T) { + setupChartMigrationTest(t) + + Expect(migrateChartPropertiesToMapFormat()).To(Succeed()) + + for _, chart := range HelmCharts { + Expect(common.PropertyExists("scheduler-k3s", "--global", "chart-overrides."+chart.ReleaseName)).To(BeFalse()) + } +} diff --git a/tests/unit/scheduler-k3s-charts.bats b/tests/unit/scheduler-k3s-charts-1.bats similarity index 100% rename from tests/unit/scheduler-k3s-charts.bats rename to tests/unit/scheduler-k3s-charts-1.bats diff --git a/tests/unit/scheduler-k3s-charts-2.bats b/tests/unit/scheduler-k3s-charts-2.bats new file mode 100644 index 000000000..e56f3ea4a --- /dev/null +++ b/tests/unit/scheduler-k3s-charts-2.bats @@ -0,0 +1,57 @@ +#!/usr/bin/env bats + +load test_helper + +setup() { + global_setup +} + +teardown() { + dokku scheduler-k3s:charts:set traefik.service.annotations.prometheus.io/scrape >/dev/null 2>/dev/null || true + dokku scheduler-k3s:charts:set traefik.controller.config >/dev/null 2>/dev/null || true + dokku scheduler-k3s:charts:set cert-manager.installCRDs >/dev/null 2>/dev/null || true + global_teardown +} + +@test "(scheduler-k3s:charts:set) accepts property names containing /" { + run /bin/bash -c "dokku scheduler-k3s:charts:set traefik.service.annotations.prometheus.io/scrape true" + assert_success + + run /bin/bash -c "dokku scheduler-k3s:charts:report traefik --format json | jq -r '.\"traefik.service.annotations.prometheus.io/scrape\"'" + assert_success + assert_output "true" + + run /bin/bash -c "dokku scheduler-k3s:charts:set traefik.service.annotations.prometheus.io/scrape" + assert_success + + run /bin/bash -c "dokku scheduler-k3s:charts:report traefik --format json" + assert_success + assert_output "{}" +} + +@test "(scheduler-k3s:charts:set) preserves multi-line values" { + local value=$'line one\nline two\nline three' + run /bin/bash -c "dokku scheduler-k3s:charts:set traefik.controller.config \"$value\"" + assert_success + + run /bin/bash -c "dokku scheduler-k3s:charts:report traefik --format json | jq -r '.\"traefik.controller.config\"'" + assert_success + assert_output "$value" +} + +@test "(scheduler-k3s:set) deprecated chart.* form writes through map storage" { + run /bin/bash -c "dokku scheduler-k3s:set --global chart.cert-manager.installCRDs false" + assert_success + assert_output_contains "deprecated" + + run /bin/bash -c "dokku scheduler-k3s:charts:report cert-manager --format json | jq -r '.\"cert-manager.installCRDs\"'" + assert_success + assert_output "false" + + run /bin/bash -c "dokku scheduler-k3s:set --global chart.cert-manager.installCRDs" + assert_success + + run /bin/bash -c "dokku scheduler-k3s:charts:report cert-manager --format json" + assert_success + assert_output "{}" +} From 4ae48c6ea01578ca55fe843516f01d5143579c84 Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Sun, 31 May 2026 02:23:35 -0400 Subject: [PATCH 2/2] test: pin test-env system identity and core-plugins path PropertyMap tests went through SetPermissions, which looks up the `dokku` user/group; the fresh `golang:1.26.2` container used by `go-test-plugin-in-docker` has neither, so the chown failed with `unknown group dokku`. Override `DOKKU_SYSTEM_USER` and `DOKKU_SYSTEM_GROUP` to `root` in the property and chart-migration test setups so the chown targets an identity that exists everywhere. The config tests also called `plugn trigger post-config-update` without `PLUGIN_CORE_AVAILABLE_PATH` set, so the hook tried to source `/common/functions` and logged a noisy warning on every test; point the env var at the canonical `/var/lib/dokku/core-plugins/available` location. --- plugins/common/properties_test.go | 2 ++ plugins/config/config_test.go | 6 +++++- plugins/scheduler-k3s/triggers_test.go | 2 ++ 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/plugins/common/properties_test.go b/plugins/common/properties_test.go index 3c2954720..cdf44a052 100644 --- a/plugins/common/properties_test.go +++ b/plugins/common/properties_test.go @@ -19,6 +19,8 @@ func setupPropertyMapTest(t *testing.T) { RegisterTestingT(t) Expect(setupTests()).To(Succeed()) t.Setenv("DOKKU_LIB_ROOT", t.TempDir()) + t.Setenv("DOKKU_SYSTEM_USER", "root") + t.Setenv("DOKKU_SYSTEM_GROUP", "root") Expect(PropertySetup(propertyTestPlugin)).To(Succeed()) } diff --git a/plugins/config/config_test.go b/plugins/config/config_test.go index e512fc42c..ef8b48d29 100644 --- a/plugins/config/config_test.go +++ b/plugins/config/config_test.go @@ -25,7 +25,11 @@ func setupTests() (err error) { return err } - return os.Setenv("PLUGIN_ENABLED_PATH", "/var/lib/dokku/plugins/enabled") + if err := os.Setenv("PLUGIN_ENABLED_PATH", "/var/lib/dokku/plugins/enabled"); err != nil { + return err + } + + return os.Setenv("PLUGIN_CORE_AVAILABLE_PATH", "/var/lib/dokku/core-plugins/available") } func setupTestApp() (err error) { diff --git a/plugins/scheduler-k3s/triggers_test.go b/plugins/scheduler-k3s/triggers_test.go index 3f142eaec..413512ede 100644 --- a/plugins/scheduler-k3s/triggers_test.go +++ b/plugins/scheduler-k3s/triggers_test.go @@ -13,6 +13,8 @@ func setupChartMigrationTest(t *testing.T) { t.Setenv("PLUGIN_PATH", "/var/lib/dokku/plugins") t.Setenv("PLUGIN_ENABLED_PATH", "/var/lib/dokku/plugins/enabled") t.Setenv("DOKKU_LIB_ROOT", t.TempDir()) + t.Setenv("DOKKU_SYSTEM_USER", "root") + t.Setenv("DOKKU_SYSTEM_GROUP", "root") Expect(common.PropertySetup("scheduler-k3s")).To(Succeed()) }