Skip to content

fix: target the dokploy container in the auth secret migration script - #183

Open
Hayyan612 wants to merge 1 commit into
Dokploy:mainfrom
Hayyan612:fix/auth-secret-migration-container
Open

Hayyan612 wants to merge 1 commit into
Dokploy:mainfrom
Hayyan612:fix/auth-secret-migration-container

Conversation

@Hayyan612

Copy link
Copy Markdown

Fixes Dokploy/dokploy#5359

The 2FA migration step in security/0.29.3.sh finds the Dokploy container with:

docker ps --filter "name=dokploy" --format "{{.ID}}" | head -n1

Docker's name filter is a substring match, so this also matches dokploy-postgres and dokploy-traefik, and docker ps lists the newest container first. When Postgres or Traefik was started after Dokploy, for example after a reboot, the migration ran inside the wrong container and failed with can't cd to /app.

This selects the task by the label Swarm sets on every service task, com.docker.swarm.service.name=dokploy, as suggested in the issue. Label filters compare the value exactly, so only the dokploy service's container matches.

Testing

Ran the original and the fixed script against a local Swarm laid out like install.sh: a dokploy service, a standalone dokploy-traefik container, and a dokploy-postgres service created last so it is listed first. id and timeout were shimmed to run on macOS.

original: docker ps order: dokploy-postgres.1.… dokploy-traefik dokploy.1.…
          sh: cd: line 0: can't cd to /app: No such file or directory
          Error: 2FA migration failed (exit code 2)        exit 1

fixed:    docker ps order: dokploy-postgres.1.… dokploy-traefik dokploy.1.…
          ✅ 2FA records migrated
          🔄 Updating Dokploy service...                   exit 0

docker ps --filter name=dokploy is a substring match, so it also matched
dokploy-postgres and dokploy-traefik. docker ps lists the newest container
first, so after a restart the 2FA migration could run in the wrong
container and fail with "can't cd to /app". Select the task by its Swarm
service-name label instead, which is compared exactly.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BETTER_AUTH_SECRET migration script targets wrong container

1 participant