gha: add workflow to run migrations#70
Conversation
WalkthroughA new GitHub Actions workflow named "Migration Test" has been added. It sets up PostgreSQL and Redis services, checks out the code, prepares configuration, builds and runs a Go binary, verifies database migrations, and checks migration integrity, all within an automated CI environment triggered by push events. Changes
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Pull Request Overview
Adds a new GitHub Actions workflow to verify database migrations by running the DCA plugin binary and checking both table creation and the Goose version table.
- Introduces a
migration-testjob with Postgres and Redis services - Builds and runs the DCA binary against the test config
- Validates that expected tables exist and that Goose migration records are present
Comments suppressed due to low confidence (2)
.github/workflows/migration-test.yml:98
- [nitpick] This error message is generic. Including the DCA process exit code or logs would improve debuggability when migrations fail to start.
echo "DCA process failed to start"
.github/workflows/migration-test.yml:50
- The here-doc block is indented, which will include leading spaces in
config.jsonand may break JSON parsing. Consider using<<-EOFor left-aligning the JSON content.
cat > config.json <<EOF
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
.github/workflows/migration-test.yml (3)
93-94: Avoid fixed sleep for migration completion.Using
sleep 10can cause flakiness if migrations take a variable amount of time. Instead, implement a retry loop or health check that polls the database until the expected tables appear, with a timeout:TIMEOUT=60 ELAPSED=0 until PGPASSWORD=${{ secrets.POSTGRES_PASSWORD }} \ psql -h localhost -U ${{ secrets.POSTGRES_USER }} -d ${{ secrets.POSTGRES_DB }} \ -c "\dt" | grep -q "plugin_policy"; do sleep 1 ELAPSED=$((ELAPSED+1)) if [ $ELAPSED -ge $TIMEOUT ]; then echo "Migrations did not complete within the timeout period" kill $DCA_PID exit 1 fi done
81-82: DRY up theLD_LIBRARY_PATHexports.The
LD_LIBRARY_PATHis exported twice. Consolidate this into anenv:block at the job or step level:jobs: migration-test: env: LD_LIBRARY_PATH: ../go-wrappers/includes/linux/:$LD_LIBRARY_PATH runs-on: ubuntu-latest services: # ... steps: - name: Build dca binary run: go build -o dca cmd/dca/*.go - name: Run dca and check migrations run: | export VS_CONFIG_NAME=config ./dca & # ...Also applies to: 86-87
56-58:⚠️ Potential issueUse GitHub Actions secrets for the database DSN in
config.json.Embedding credentials directly in
config.jsonrisks exposure and complicates management. Refactor to template the DSN from secrets:"database": { - "dsn": "postgres://myuser:mypassword@localhost:5432/vultisig-plugin?sslmode=disable" + "dsn": "${{ secrets.DB_DSN }}" }🧰 Tools
🪛 Checkov (3.2.334)
[MEDIUM] 57-58: Basic Auth Credentials
(CKV_SECRET_4)
🧹 Nitpick comments (2)
.github/workflows/migration-test.yml (2)
44-47: Cache or vendor external dependencies instead of cloning.Cloning
go-wrapperson every run adds network overhead and risks rate limits. Consider:
- Adding
go-wrappersas a Git submodule- Using
actions/checkoutwithpath: go-wrappers- Leveraging
actions/cacheto cache the cloned repository🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 47-47: trailing spaces
(trailing-spaces)
9-9: Remove trailing whitespace and add newline at end of file.Several lines contain trailing spaces, and the file is missing a final newline. Cleaning these will satisfy the YAML linter:
sed -i 's/[[:space:]]\+$//' .github/workflows/migration-test.yml printf '\n' >> .github/workflows/migration-test.ymlAlso applies to: 24-24, 34-34, 38-38, 43-43, 47-47, 78-78, 83-83, 88-88, 92-92, 95-95, 101-101, 109-109, 111-111, 114-114, 123-124
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 9-9: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/migration-test.yml(1 hunks)
🧰 Additional context used
🪛 Checkov (3.2.334)
.github/workflows/migration-test.yml
[MEDIUM] 57-58: Basic Auth Credentials
(CKV_SECRET_4)
🪛 YAMLlint (1.37.1)
.github/workflows/migration-test.yml
[error] 9-9: trailing spaces
(trailing-spaces)
[error] 24-24: trailing spaces
(trailing-spaces)
[error] 34-34: trailing spaces
(trailing-spaces)
[error] 38-38: trailing spaces
(trailing-spaces)
[error] 43-43: trailing spaces
(trailing-spaces)
[error] 47-47: trailing spaces
(trailing-spaces)
[error] 78-78: trailing spaces
(trailing-spaces)
[error] 83-83: trailing spaces
(trailing-spaces)
[error] 88-88: trailing spaces
(trailing-spaces)
[error] 92-92: trailing spaces
(trailing-spaces)
[error] 95-95: trailing spaces
(trailing-spaces)
[error] 101-101: trailing spaces
(trailing-spaces)
[error] 109-109: trailing spaces
(trailing-spaces)
[error] 111-111: trailing spaces
(trailing-spaces)
[error] 114-114: trailing spaces
(trailing-spaces)
[error] 123-123: trailing spaces
(trailing-spaces)
[error] 124-124: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (1)
.github/workflows/migration-test.yml (1)
64-70: Verify block storage service availability.The
block_storage_configpoints tohttp://localhost:9000(MinIO), but no corresponding service container is defined. If DCA requires S3-compatible storage during migrations, this will fail. Please confirm whether you intend to:
- Add a MinIO service under
services:- Mock or disable block storage in
config.jsonfor testing- Or adjust the workflow to handle block storage dependencies
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
.github/workflows/migration-test.yml (3)
88-93: DRY up repeatedLD_LIBRARY_PATHexports.
You export the same path in both build (line 88) and run (line 93) steps. Extract it into a sharedenv:block at the job or step level to eliminate duplication.🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 90-90: trailing spaces
(trailing-spaces)
100-104: Replace fixedsleepwith a polling loop.
A hardcodedsleep 10may still be flaky under load. Use a retry loop that checks the DB (or a health endpoint) until migrations finish, as previously suggested.🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 102-102: trailing spaces
(trailing-spaces)
124-129: Strengthen the migration integrity check.
Grep"[0-9]+"will match0and pass even if no migrations ran. Parse the count and assert it’s > 0:count=$(PGPASSWORD=mypassword \ psql -h localhost -U myuser -d vultisig-plugin -t -c "SELECT COUNT(*) FROM goose_db_version;" | xargs) if [ "$count" -le 0 ]; then echo "No migrations applied; goose_db_version is empty" exit 1 fi
🧹 Nitpick comments (2)
.github/workflows/migration-test.yml (2)
48-58: Avoid duplicating DB credentials in the JSON config.
You’re re-stating the same Postgres credentials insidedca.json. Consider templating or referencing the service env vars to DRY this out.
Example diff:- "dsn": "postgres://myuser:mypassword@localhost:5432/vultisig-plugin?sslmode=disable" + "dsn": "${{ format('postgres://{0}:{1}@localhost:5432/{2}?sslmode=disable', env.POSTGRES_USER, env.POSTGRES_PASSWORD, env.POSTGRES_DB) }}"🧰 Tools
🪛 Checkov (3.2.334)
[MEDIUM] 57-58: Basic Auth Credentials
(CKV_SECRET_4)
9-9: Remove trailing whitespace & add final newline.
YAMLlint flagged trailing spaces on multiple lines and a missing newline at EOF. Please trim all trailing spaces and ensure the file ends with a single newline.Also applies to: 24-24, 34-34, 38-38, 43-43, 47-47, 85-85, 90-90, 95-95, 99-99, 102-102, 108-108, 116-116, 118-118, 121-121, 130-131
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 9-9: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/migration-test.yml(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: RaghavSood
PR: vultisig/plugin#70
File: .github/workflows/migration-test.yml:14-17
Timestamp: 2025-05-27T04:28:20.394Z
Learning: In GitHub Actions workflows for testing, hardcoded database credentials are acceptable when using ephemeral test services (like PostgreSQL containers) that only exist during the CI run, as there's no security risk with temporary test environments.
.github/workflows/migration-test.yml (1)
Learnt from: RaghavSood
PR: vultisig/plugin#70
File: .github/workflows/migration-test.yml:14-17
Timestamp: 2025-05-27T04:28:20.394Z
Learning: In GitHub Actions workflows for testing, hardcoded database credentials are acceptable when using ephemeral test services (like PostgreSQL containers) that only exist during the CI run, as there's no security risk with temporary test environments.
🪛 Checkov (3.2.334)
.github/workflows/migration-test.yml
[MEDIUM] 57-58: Basic Auth Credentials
(CKV_SECRET_4)
🪛 YAMLlint (1.37.1)
.github/workflows/migration-test.yml
[error] 9-9: trailing spaces
(trailing-spaces)
[error] 24-24: trailing spaces
(trailing-spaces)
[error] 34-34: trailing spaces
(trailing-spaces)
[error] 38-38: trailing spaces
(trailing-spaces)
[error] 43-43: trailing spaces
(trailing-spaces)
[error] 47-47: trailing spaces
(trailing-spaces)
[error] 85-85: trailing spaces
(trailing-spaces)
[error] 90-90: trailing spaces
(trailing-spaces)
[error] 95-95: trailing spaces
(trailing-spaces)
[error] 99-99: trailing spaces
(trailing-spaces)
[error] 102-102: trailing spaces
(trailing-spaces)
[error] 108-108: trailing spaces
(trailing-spaces)
[error] 116-116: trailing spaces
(trailing-spaces)
[error] 118-118: trailing spaces
(trailing-spaces)
[error] 121-121: trailing spaces
(trailing-spaces)
[error] 130-130: trailing spaces
(trailing-spaces)
[error] 131-131: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (3)
.github/workflows/migration-test.yml (3)
1-5: Workflow definition looks solid.
The new “Migration Test” workflow is correctly configured to trigger on push events and lays out the migration verification steps as intended.
6-9: Standardjobsandruns-onconfiguration; no issues detected.🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 9-9: trailing spaces
(trailing-spaces)
14-17: Hardcoded credentials are acceptable in this CI context.
Per our ephemeral-test-services guideline, embeddingmyuser/mypassword/vultisig-pluginhere does not introduce a security risk.
Closes #69
Mirror the workflow on verifier repo to run migrations and test their consistency.
This only runs the DCA plugin binary, since any of them should be able to run the current migrations
We can expand this as we go deeper into plugin-specific db flows.
Summary by CodeRabbit