test: cover migration via go unit test

The bats test for migration idempotency invoked the install binary directly via sudo, which kept hitting fresh missing env vars on each iteration (DOKKU_LIB_ROOT, then PLUGIN_PATH, etc). The dokku launcher script exports a chain of vars that the install path reads via common.MustGetEnv, and mirroring that chain in a bats test is fragile. Migration is pure file IO plus a property marker - perfect for a Go unit test that isolates itself with t.TempDir and t.Setenv. The new test covers parsing comments and blank lines, the marker-based no-op on re-run, and the rule that a manually re-created legacy file is left untouched after the marker is set.

plugins/docker-options/migration_test.go

@@ -0,0 +1,148 @@
+package dockeroptions
+
+import (
+	"os"
+	"os/user"
+	"path/filepath"
+	"testing"
+
+	"github.com/dokku/dokku/plugins/common"
+)
+
+// setupMigrationEnv points the dokku env at temporary directories and tells the
+// permission helpers to chown files to the current user (a no-op) so the test
+// works without root.
+func setupMigrationEnv(t *testing.T) (dokkuRoot string) {
+	t.Helper()
+
+	libRoot := t.TempDir()
+	dokkuRoot = t.TempDir()
+
+	t.Setenv("DOKKU_LIB_ROOT", libRoot)
+	t.Setenv("DOKKU_ROOT", dokkuRoot)
+	t.Setenv("PLUGIN_PATH", filepath.Join(libRoot, "plugins"))
+
+	current, err := user.Current()
+	if err != nil {
+		t.Fatalf("user.Current: %v", err)
+	}
+	group, err := user.LookupGroupId(current.Gid)
+	if err != nil {
+		t.Fatalf("user.LookupGroupId: %v", err)
+	}
+	t.Setenv("DOKKU_SYSTEM_USER", current.Username)
+	t.Setenv("DOKKU_SYSTEM_GROUP", group.Name)
+
+	return dokkuRoot
+}
+
+func writeLegacyDockerOptionsFile(t *testing.T, dokkuRoot, app, phase, contents string) {
+	t.Helper()
+	if err := os.MkdirAll(filepath.Join(dokkuRoot, app), 0755); err != nil {
+		t.Fatalf("MkdirAll: %v", err)
+	}
+	path := filepath.Join(dokkuRoot, app, "DOCKER_OPTIONS_"+phase)
+	if err := os.WriteFile(path, []byte(contents), 0644); err != nil {
+		t.Fatalf("WriteFile: %v", err)
+	}
+}
+
+func TestMigrateLegacyDockerOptionsFiles_MigratesAndIsIdempotent(t *testing.T) {
+	dokkuRoot := setupMigrationEnv(t)
+
+	writeLegacyDockerOptionsFile(t, dokkuRoot, "alpha", "DEPLOY", "-v /var/log:/log\n# a comment\n\n--restart=on-failure:5\n")
+	writeLegacyDockerOptionsFile(t, dokkuRoot, "alpha", "BUILD", "--build-arg FOO=bar\n")
+	writeLegacyDockerOptionsFile(t, dokkuRoot, "beta", "DEPLOY", "-p 8080:5000\n")
+
+	if err := migrateLegacyDockerOptionsFiles(); err != nil {
+		t.Fatalf("first migration: %v", err)
+	}
+
+	deploy, err := common.PropertyListGet("docker-options", "alpha", "_default_.deploy")
+	if err != nil {
+		t.Fatalf("PropertyListGet alpha deploy: %v", err)
+	}
+	wantDeploy := []string{"-v /var/log:/log", "--restart=on-failure:5"}
+	if !equalStrings(deploy, wantDeploy) {
+		t.Errorf("alpha deploy = %v, want %v", deploy, wantDeploy)
+	}
+
+	build, err := common.PropertyListGet("docker-options", "alpha", "_default_.build")
+	if err != nil {
+		t.Fatalf("PropertyListGet alpha build: %v", err)
+	}
+	wantBuild := []string{"--build-arg FOO=bar"}
+	if !equalStrings(build, wantBuild) {
+		t.Errorf("alpha build = %v, want %v", build, wantBuild)
+	}
+
+	betaDeploy, err := common.PropertyListGet("docker-options", "beta", "_default_.deploy")
+	if err != nil {
+		t.Fatalf("PropertyListGet beta deploy: %v", err)
+	}
+	wantBetaDeploy := []string{"-p 8080:5000"}
+	if !equalStrings(betaDeploy, wantBetaDeploy) {
+		t.Errorf("beta deploy = %v, want %v", betaDeploy, wantBetaDeploy)
+	}
+
+	if !common.PropertyExists("docker-options", "--global", "migrated-from-files") {
+		t.Errorf("migrated-from-files marker not set after first migration")
+	}
+
+	for _, app := range []string{"alpha", "beta"} {
+		for _, phase := range []string{"BUILD", "DEPLOY", "RUN"} {
+			legacy := filepath.Join(dokkuRoot, app, "DOCKER_OPTIONS_"+phase)
+			migrated := legacy + ".migrated"
+			if app == "alpha" && (phase == "BUILD" || phase == "DEPLOY") || app == "beta" && phase == "DEPLOY" {
+				if _, err := os.Stat(migrated); err != nil {
+					t.Errorf("expected %s to exist: %v", migrated, err)
+				}
+				if _, err := os.Stat(legacy); !os.IsNotExist(err) {
+					t.Errorf("expected %s to be gone, got err=%v", legacy, err)
+				}
+			} else {
+				if _, err := os.Stat(legacy); !os.IsNotExist(err) {
+					t.Errorf("did not expect %s to exist, got err=%v", legacy, err)
+				}
+			}
+		}
+	}
+
+	sneakyPath := filepath.Join(dokkuRoot, "alpha", "DOCKER_OPTIONS_DEPLOY")
+	sneakyContents := "-v /tmp/sneaky:/sneaky\n"
+	if err := os.WriteFile(sneakyPath, []byte(sneakyContents), 0644); err != nil {
+		t.Fatalf("re-create legacy file: %v", err)
+	}
+
+	if err := migrateLegacyDockerOptionsFiles(); err != nil {
+		t.Fatalf("second migration: %v", err)
+	}
+
+	deployAfter, err := common.PropertyListGet("docker-options", "alpha", "_default_.deploy")
+	if err != nil {
+		t.Fatalf("PropertyListGet alpha deploy (post re-run): %v", err)
+	}
+	if !equalStrings(deployAfter, wantDeploy) {
+		t.Errorf("alpha deploy after rerun = %v, want %v (idempotency violated)", deployAfter, wantDeploy)
+	}
+
+	gotSneaky, err := os.ReadFile(sneakyPath)
+	if err != nil {
+		t.Fatalf("expected sneaky legacy file untouched, got err=%v", err)
+	}
+	if string(gotSneaky) != sneakyContents {
+		t.Errorf("sneaky legacy file modified: %q", gotSneaky)
+	}
+}
+
+func equalStrings(a, b []string) bool {
+	if len(a) != len(b) {
+		return false
+	}
+	for i := range a {
+		if a[i] != b[i] {
+			return false
+		}
+	}
+	return true
+}

tests/unit/docker-options-2.bats

@@ -189,37 +189,6 @@ teardown() {
   assert_output_contains "-v /logs:/logs"
 }
 
-@test "(docker-options) install migration is idempotent" {
-  sudo rm -f "/var/lib/dokku/config/docker-options/--global/migrated-from-files"
-  sudo rm -rf "/var/lib/dokku/config/docker-options/$TEST_APP"
-  sudo bash -c "echo '-v /tmp/legacy:/legacy' >\"$DOKKU_ROOT/$TEST_APP/DOCKER_OPTIONS_DEPLOY\""
-
-  run /bin/bash -c "sudo DOKKU_LIB_ROOT=/var/lib/dokku DOKKU_ROOT=$DOKKU_ROOT /var/lib/dokku/plugins/enabled/docker-options/install"
-  echo "output: $output"
-  echo "status: $status"
-  assert_success
-
-  run /bin/bash -c "dokku docker-options:list $TEST_APP --phase deploy"
-  echo "output: $output"
-  assert_output_contains "-v /tmp/legacy:/legacy"
-
-  [[ -f "$DOKKU_ROOT/$TEST_APP/DOCKER_OPTIONS_DEPLOY.migrated" ]]
-  [[ ! -f "$DOKKU_ROOT/$TEST_APP/DOCKER_OPTIONS_DEPLOY" ]]
-
-  sudo bash -c "echo '-v /tmp/sneaky:/sneaky' >\"$DOKKU_ROOT/$TEST_APP/DOCKER_OPTIONS_DEPLOY\""
-  run /bin/bash -c "sudo DOKKU_LIB_ROOT=/var/lib/dokku DOKKU_ROOT=$DOKKU_ROOT /var/lib/dokku/plugins/enabled/docker-options/install"
-  echo "output: $output"
-  assert_success
-
-  run /bin/bash -c "dokku docker-options:list $TEST_APP --phase deploy"
-  echo "output: $output"
-  assert_output_contains "-v /tmp/legacy:/legacy"
-
-  [[ -f "$DOKKU_ROOT/$TEST_APP/DOCKER_OPTIONS_DEPLOY" ]]
-  run /bin/bash -c "cat $DOKKU_ROOT/$TEST_APP/DOCKER_OPTIONS_DEPLOY"
-  assert_output_contains "/tmp/sneaky"
-}
-
 @test "(docker-options) clone copies default and per-process options" {
   local CLONE_APP="${TEST_APP}-clone"
   run /bin/bash -c "dokku docker-options:add --process web $TEST_APP deploy '-p 8080:5000'"