fix(synkronus-quickstart): support docker and podman quickstart flows#1
fix(synkronus-quickstart): support docker and podman quickstart flows#1
Conversation
bokchan
force-pushed
the
revise-and-validate-docs
branch
from
9567f88 to
d0fbbf6
Compare
bokchan
force-pushed
the
revise-and-validate-docs
branch
from
d0fbbf6 to
65c560b
Compare
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
There was a problem hiding this comment.
Pull request overview
This PR aims to make the Synkronus quickstart flow work consistently across Docker and Podman by updating helper scripts/docs and adding CI validation for both runtimes.
Changes:
- Added a data-volume migration utility and an automated upgrade-path validation workflow.
- Updated backup/bootstrap scripts to support selecting Docker vs Podman via
SYNK_RUNTIME. - Added GitHub Actions coverage for Docker/Podman manual + installer flows and adjusted installer/docs for rootless-friendly localhost access.
Reviewed changes
Copilot reviewed 14 out of 16 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| utilities/migrate-synkronus-data.sh | New script to migrate legacy volume layout into the current /app/data structure. |
| utilities/backup-db.sh | Adds runtime selection via SYNK_RUNTIME; adjusts output path handling. |
| utilities/backup-attachments.sh | Adds runtime selection via SYNK_RUNTIME. |
| utilities/README.md | Documents the utilities list in a clearer table format. |
| install.sh | Switches localhost Caddy binding to host port 8081 and updates printed next steps. |
| docker-compose.yml | Adds container_name for synkronus, changes default credentials, and publishes Postgres port 5432. |
| create_sync_db.sh | Adds runtime detection + container discovery for Docker/Podman and uses it for DB bootstrap. |
| README.md | Updates quickstart/manual-install docs and clarifies localhost + rootless Podman behavior. |
| .gitignore | Ignores generated Caddyfile and docker-compose.override.yml. |
| .github/workflows/test-upgrade-path.yml | New workflow to test migration flow on Docker and Podman. |
| .github/workflows/test-podman-runtime.yml | New workflow to test Podman quickstart flows via reusable workflow. |
| .github/workflows/test-docker-runtime.yml | New workflow to test Docker quickstart flows via reusable workflow. |
| .github/workflows/reusable-runtime-flow.yml | Reusable workflow to run runtime scenario scripts. |
| .github/scripts/upgrade-path-flow.sh | Scripted end-to-end upgrade-path/migration test used by CI. |
| .github/scripts/runtime-flow.sh | Scripted end-to-end manual + installer quickstart tests used by CI. |
| .github/actions/setup-runtime/action.yml | Composite action to install/auth/verify Docker or Podman tooling in CI. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| DB_CONNECTION: "postgres://synkronus_user:im97C1wauDvbHDREP51Vk2OfVeFp092@db:5432/synkronus?sslmode=disable" # create with ~/create_synk_db demo | ||
| JWT_SECRET: "ib2Xz57D7VQhkmMTWL0AVubCZblH8fvQzUKiEz6T8M" # Generate a new one with: openssl rand -base64 32 | ||
| ADMIN_USERNAME: "admin_0d16c0" | ||
| ADMIN_PASSWORD: "orK5BcLcpDPamf5OKiYFC0hw" |
There was a problem hiding this comment.
The compose file now hard-codes DB/JWT/admin credentials. This is a security risk (secrets committed to git) and it also breaks install.sh, which currently does string replacement on the old placeholder values (e.g., "strong_password", "please_change_the_username", and the previous JWT seed) and will no longer update these fields. Consider reverting these values back to placeholders (or moving secrets into an untracked .env / override file) so the installer can inject generated credentials without committing them.
| ports: | ||
| - "5432:5432" |
There was a problem hiding this comment.
Publishing Postgres on host port 5432 by default increases exposure (any local-network access to the host can attempt DB connections) and can cause port conflicts on developer machines/CI runners. Unless there is a strong reason to access Postgres directly from the host, prefer removing this ports mapping and relying on the internal compose network (or documenting an optional override for users who need host access).
| ports: | |
| - "5432:5432" |
| USERNAME="$1" | ||
| RECREATE=false | ||
|
|
||
| if [ "$2" == "--recreate" ]; then | ||
| RECREATE=true | ||
| fi | ||
|
|
||
| DB_USER="synk_$USERNAME" | ||
| DB_NAME="synk_$USERNAME" | ||
| PASSWORD=$(openssl rand -base64 30 | tr -d /=+ | cut -c1-40) |
There was a problem hiding this comment.
USERNAME is incorporated into DB_USER/DB_NAME and then interpolated into SQL identifiers without validation/quoting. A username containing spaces, quotes, or SQL metacharacters can fail unexpectedly or (in the worst case) alter the executed SQL. Add a strict validation step (e.g., allow only [a-zA-Z0-9_]+) and fail fast with a clear message before constructing SQL.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Pull Request Title
fix(synkronus-quickstart): support docker and podman quickstart flows
Description
This draft PR tightens the Synkronus quickstart flow so the documented manual setup works consistently across Docker and Podman.
It updates the runtime scripts and docs, fixes the manual database bootstrap flow, and adds GitHub Actions coverage for both runtimes.
TODO:
- update-docsfromon.push.branchesbefore mergingType of Change
Component(s) Affected
Related Issue(s)
Closes/Fixes/Resolves: None
Testing
Breaking Changes
If breaking changes, please describe migration steps:
None.
Documentation Updates
Checklist
Thank you for contributing to Open Data Ensemble (ODE)!