Local docker compose#108
Conversation
WalkthroughThis update introduces new multi-stage Dockerfiles for the Payroll server and worker, restructures configuration files, removes obsolete Makefile targets, and revises Docker Compose and infrastructure scripts to support the Payroll system. Configuration management is refactored, including changes to how encryption secrets are handled and environment variables are mapped. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant Docker as Docker Build
participant App as Payroll Server/Worker
participant Config as Config Files
participant Redis as Redis
participant DB as Postgres (Payroll)
participant MinIO as MinIO (Payroll)
Dev->>Docker: Build Payroll server/worker image
Docker->>Config: Copy example config files
Docker->>App: Build Go binary
Docker->>App: Copy native libraries
Docker->>App: Set entrypoint and env vars
Dev->>Docker: Run container
App->>Config: Load config.json
App->>Redis: Connect to Redis
App->>DB: Connect to Payroll DB
App->>MinIO: Connect to MinIO storage
Possibly related PRs
Suggested reviewers
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
This PR adds local Docker Compose support for the payroll plugin, introduces example configuration files for the payroll server and worker, and updates initialization scripts and Dockerfiles to reflect the new “vultisig-payroll” setup.
- Added
payroll.worker.example.jsonandpayroll.server.example.jsonwith updated settings. - Extended
docker-compose.yamlto spin up payroll DB, Redis, MinIO, server, and worker with healthchecks and shared networking. - Refactored SQL init scripts, config loading, and Dockerfiles for Go-based payroll components.
Reviewed Changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| payroll.worker.example.json | New example config for the payroll worker plugin |
| payroll.server.example.json | New example config for the payroll server plugin |
| init-scripts/01_create_vultisig_plugin.sql | Updated DB creation to use vultisig-payroll |
| init-scripts/02_create_vultisig_verifier.sql | Removed obsolete verifier DB script |
| docker-compose.yaml | Added payroll services, healthchecks, volumes, and network |
| create_buckets.sh | Extended bucket creation list |
| cmd/payroll/worker/main.go | Updated plugin constructor to use nested VaultServiceConfig |
| cmd/payroll/worker/config.go | Removed top-level EncryptionSecret and added env key replacer |
| cmd/payroll/server/main.go | Updated plugin constructor to use Server.EncryptionSecret |
| cmd/payroll/server/config.go | Removed top-level EncryptionSecret and added env key replacer |
| README.md | Documented shared network creation and make up/down commands |
| Makefile | Cleaned up old run targets |
| Dockerfile.Payroll.worker | New multi-stage Dockerfile for payroll worker |
| Dockerfile.Payroll.server | New multi-stage Dockerfile for payroll server |
Comments suppressed due to low confidence (5)
cmd/payroll/worker/config.go:25
- Removing the top-level
EncryptionSecretbreaks mapping ofencryption_secretfrompayroll.worker.example.json. Either restore this field or map it under a newVaultServiceConfigsub-struct so Viper can unmarshal correctly.
BaseConfigPath string `mapstructure:"base_file_path" json:"base_file_path,omitempty"`
docker-compose.yaml:105
- Compose uses
shared_network(underscore) but README instructs to createshared-network(dash). Align the network name in both places to prevent connectivity errors.
shared_network:
docker-compose.yaml:102
- The
redis_datavolume is defined but no service mounts it (redis-payroll does not reference it). Consider removing the unused volume or reattaching it.
redis_data:
cmd/payroll/worker/main.go:84
- PayrollWorkerConfig does not define a
VaultServiceConfigfield, socfg.VaultServiceConfig.EncryptionSecretwill not compile. You need to add a nestedVaultServiceConfigstruct toPayrollWorkerConfigand mapencryption_secretundervault_service.
p, err := payroll.NewPayrollPlugin(postgressDB, vaultStorage, cfg.BaseConfigPath, txIndexerService, client, inspector, cfg.VaultServiceConfig.EncryptionSecret)
cmd/payroll/server/main.go:74
- PayrollServerConfig has no
EncryptionSecretunderServer, socfg.Server.EncryptionSecretwill not compile. Introduce aVaultServiceConfigor equivalent in the config struct to holdencryption_secretunder theserverorvault_serviceblock.
p, err := payroll.NewPayrollPlugin(db, vaultStorage, cfg.BaseConfigPath, txIndexerService, client, inspector, cfg.Server.EncryptionSecret)
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
♻️ Duplicate comments (1)
init-scripts/01_create_vultisig_plugin.sql (1)
1-1: Add IF NOT EXISTS for idempotent script execution.The database name change aligns with the payroll migration, but removing the IF NOT EXISTS clause can cause errors on repeated runs.
-CREATE DATABASE "vultisig-payroll"; +CREATE DATABASE IF NOT EXISTS "vultisig-payroll";
🧹 Nitpick comments (2)
Dockerfile.Payroll.server (1)
28-30: Slim the runtime image & avoid needless packages.
--no-install-recommendsplusca-certificateskeeps the layer small and TLS happy.-RUN apt-get update && \ - apt-get install -y wget && \ +RUN apt-get update && \ + apt-get install --no-install-recommends -y wget ca-certificates && \ rm -rf /var/lib/apt/lists/*docker-compose.yaml (1)
101-104:redis_datavolume is declared but unused; trailing whitespace present.Delete the orphan volume and clean up formatting.
db_data: - redis_data: - minio_data: + minio_data:Also add a final newline to satisfy YAML-lint.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
Dockerfile.Payroll.server(1 hunks)Dockerfile.Payroll.worker(1 hunks)Makefile(0 hunks)README.md(1 hunks)cmd/payroll/server/config.go(2 hunks)cmd/payroll/server/main.go(1 hunks)cmd/payroll/worker/config.go(3 hunks)cmd/payroll/worker/main.go(1 hunks)config-server.yaml(0 hunks)config-verifier.yaml(0 hunks)config.example.json(0 hunks)create_buckets.sh(1 hunks)docker-compose.yaml(1 hunks)init-scripts/01_create_vultisig_plugin.sql(1 hunks)init-scripts/02_create_vultisig_verifier.sql(0 hunks)payroll.server.example.json(1 hunks)payroll.worker.example.json(1 hunks)
💤 Files with no reviewable changes (5)
- init-scripts/02_create_vultisig_verifier.sql
- config.example.json
- Makefile
- config-server.yaml
- config-verifier.yaml
🧰 Additional context used
🧠 Learnings (5)
cmd/payroll/worker/main.go (3)
Learnt from: webpiratt
PR: vultisig/plugin#96
File: plugin/payroll/transaction.go:0-0
Timestamp: 2025-06-18T18:23:20.077Z
Learning: In the payroll plugin (plugin/payroll/transaction.go), signRequest.Transaction is stored with the "0x" prefix, making it compatible with gcommon.FromHex which requires 0x-prefixed hex strings.
Learnt from: RaghavSood
PR: vultisig/plugin#75
File: storage/postgres/db_plugin.go:25-40
Timestamp: 2025-05-30T02:44:31.711Z
Learning: The embed.FS directive `//go:embed migrations/plugin/*.sql` in storage/postgres/db_plugin.go correctly embeds plugin migration files that exist in the storage/postgres/migrations/plugin/ directory. The path "migrations/plugin" passed to goose.Up() references the embedded filesystem structure, not the physical file system path.
Learnt from: webpiratt
PR: vultisig/plugin#96
File: plugin/payroll/transaction.go:510-514
Timestamp: 2025-06-18T18:20:59.510Z
Learning: The erc20ABI constant is defined in plugin/payroll/constants.go within the payroll package, making it accessible to other files in the same package like transaction.go.
cmd/payroll/server/main.go (3)
Learnt from: webpiratt
PR: vultisig/plugin#96
File: plugin/payroll/transaction.go:0-0
Timestamp: 2025-06-18T18:23:20.077Z
Learning: In the payroll plugin (plugin/payroll/transaction.go), signRequest.Transaction is stored with the "0x" prefix, making it compatible with gcommon.FromHex which requires 0x-prefixed hex strings.
Learnt from: webpiratt
PR: vultisig/plugin#96
File: plugin/payroll/transaction.go:510-514
Timestamp: 2025-06-18T18:20:59.510Z
Learning: The erc20ABI constant is defined in plugin/payroll/constants.go within the payroll package, making it accessible to other files in the same package like transaction.go.
Learnt from: RaghavSood
PR: vultisig/plugin#75
File: storage/postgres/db_plugin.go:25-40
Timestamp: 2025-05-30T02:44:31.711Z
Learning: The embed.FS directive `//go:embed migrations/plugin/*.sql` in storage/postgres/db_plugin.go correctly embeds plugin migration files that exist in the storage/postgres/migrations/plugin/ directory. The path "migrations/plugin" passed to goose.Up() references the embedded filesystem structure, not the physical file system path.
cmd/payroll/worker/config.go (2)
Learnt from: webpiratt
PR: vultisig/plugin#96
File: plugin/payroll/transaction.go:0-0
Timestamp: 2025-06-18T18:23:20.077Z
Learning: In the payroll plugin (plugin/payroll/transaction.go), signRequest.Transaction is stored with the "0x" prefix, making it compatible with gcommon.FromHex which requires 0x-prefixed hex strings.
Learnt from: RaghavSood
PR: vultisig/plugin#36
File: api/server.go:21-33
Timestamp: 2025-05-07T08:23:45.882Z
Learning: The import path `github.com/vultisig/verifier/plugin` refers to an external dependency that provides the plugin interface, and should not be changed to `github.com/vultisig/plugin/plugin` as these are distinct packages with different purposes.
cmd/payroll/server/config.go (3)
Learnt from: webpiratt
PR: vultisig/plugin#96
File: plugin/payroll/transaction.go:0-0
Timestamp: 2025-06-18T18:23:20.077Z
Learning: In the payroll plugin (plugin/payroll/transaction.go), signRequest.Transaction is stored with the "0x" prefix, making it compatible with gcommon.FromHex which requires 0x-prefixed hex strings.
Learnt from: webpiratt
PR: vultisig/plugin#96
File: plugin/payroll/transaction.go:510-514
Timestamp: 2025-06-18T18:20:59.510Z
Learning: The erc20ABI constant is defined in plugin/payroll/constants.go within the payroll package, making it accessible to other files in the same package like transaction.go.
Learnt from: RaghavSood
PR: vultisig/plugin#36
File: api/server.go:21-33
Timestamp: 2025-05-07T08:23:45.882Z
Learning: The import path `github.com/vultisig/verifier/plugin` refers to an external dependency that provides the plugin interface, and should not be changed to `github.com/vultisig/plugin/plugin` as these are distinct packages with different purposes.
payroll.server.example.json (1)
Learnt from: RaghavSood
PR: vultisig/plugin#70
File: .github/workflows/migration-test.yml:14-17
Timestamp: 2025-05-27T04:28:20.414Z
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.
🧬 Code Graph Analysis (1)
cmd/payroll/worker/main.go (1)
plugin/payroll/payroll.go (1)
NewPayrollPlugin(31-69)
🪛 Checkov (3.2.334)
payroll.server.example.json
[MEDIUM] 8-9: Basic Auth Credentials
(CKV_SECRET_4)
payroll.worker.example.json
[MEDIUM] 28-29: Basic Auth Credentials
(CKV_SECRET_4)
docker-compose.yaml
[MEDIUM] 68-69: Basic Auth Credentials
(CKV_SECRET_4)
🪛 YAMLlint (1.37.1)
docker-compose.yaml
[error] 103-103: trailing spaces
(trailing-spaces)
[error] 106-106: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (12)
create_buckets.sh (1)
8-8: LGTM! Bucket names updated to align with payroll system migration.The bucket list change from verifier/plugin to payroll/dca/fee is consistent with the broader system migration described in the PR objectives.
cmd/payroll/server/main.go (1)
74-74: ✅ EncryptionSecret field verified in ServerConfig
- Confirmed
api/server_config.godefines
EncryptionSecret stringmapstructure:"encryption_secret" json:"encryption_secret,omitempty"`- Usage in
cmd/payroll/server/main.gopassingcfg.Server.EncryptionSecrettoNewPayrollPluginis correctNo further changes required.
README.md (1)
145-151: LGTM! Improved infrastructure setup with better lifecycle management.The updated instructions provide better control over the infrastructure lifecycle by:
- Explicitly creating the shared Docker network
- Using make targets for abstraction
- Including both startup and teardown commands
This aligns with the docker-compose.yaml changes that use an external shared network.
cmd/payroll/server/config.go (2)
6-6: LGTM! Import added for environment variable key replacement.The
stringsimport is necessary for theviper.SetEnvKeyReplacerfunctionality added below.
41-41: Good improvement for container-friendly environment variables.The env key replacer allows Docker environment variables with underscores to override nested configuration keys (e.g.,
SERVER_HOSTforserver.host). This enhances deployment flexibility in containerized environments.cmd/payroll/worker/config.go (2)
6-6: LGTM! Consistent with server configuration approach.The
stringsimport supports the environment variable key replacement functionality, maintaining consistency with the server configuration.
39-39: Good consistency in environment variable handling.The env key replacer implementation matches the server configuration, ensuring consistent behavior across both payroll components for Docker environment variable mapping.
payroll.server.example.json (2)
8-8: Example credentials are acceptable for template files.The hardcoded database credentials flagged by static analysis are appropriate for this example configuration file. Production deployments should replace these with secure credentials.
19-20: Standard MinIO development credentials.The "minioadmin" credentials are MinIO's default development credentials, which is appropriate for an example configuration. Production deployments should use unique, secure credentials.
Dockerfile.Payroll.worker (2)
26-30: Good choice of base image and minimal dependencies.Using Ubuntu 24.04 as the runtime base is appropriate, and installing only
wgetkeeps the image lean. The cleanup of apt cache is a good practice.
38-39: Configuration file paths verifiedBoth configuration files referenced in Dockerfile.Payroll.worker exist in the build context:
- payroll.worker.example.json
- etc/vultisig/payroll.yml
No further action required.
Dockerfile.Payroll.server (1)
2-2: Confirm thegolang:1.24tag actually exists (and pin to a minor patch).
1.24isn’t released at the time of writing; the CI registry may not have that tag, causing build breaks. Prefer a fully-qualified, existing image such asgolang:1.23.4(or whatever the latest stable tag is).-FROM golang:1.24 as builder +FROM golang:1.23.4 as builder # <- update when 1.24 is officially published
… and clarify plugin setup instructions
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
README.md (3)
43-45: Grammar + clarity tweak for the “Get Vault” description
The sentence reads “this endpoint allow user to get the vault information”.
Quick fix:-`GET` `/vault/get/:pluginId/{publicKeyECDSA}` , this endpoint allow user to get the vault information +`GET` `/vault/get/:pluginId/{publicKeyECDSA}` – this endpoint **allows** users to retrieve vault information
83-85: Missing article in heading
Minor readability nit: “How to setup plugin to run locally?” → “How to set up a plugin to run locally?”
105-105: Section title scoped too narrowly?
## 2. Access MinIOimplies MinIO is the only next step. If additional required steps (e.g., seeding the DB) were removed unintentionally, ensure they’re captured elsewhere to avoid partial setups.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
README.md(4 hunks)
🧰 Additional context used
🧠 Learnings (1)
README.md (1)
Learnt from: webpiratt
PR: vultisig/plugin#105
File: plugin/dca/dca.go:734-736
Timestamp: 2025-07-01T17:35:35.248Z
Learning: The DCA plugin in the vultisig/plugin codebase is currently not fully implemented. Methods like getCompletedSwapTransactionsCount() return placeholder values (e.g., 0) with TODO comments, which is expected behavior during the development phase. The plugin implementation is intentionally incomplete while other parts of the system are being refactored.
🪛 LanguageTool
README.md
[grammar] ~44-~44: “Endpoint” is a singular noun. It appears that the verb form is incorrect.
Context: ...ginId/{publicKeyECDSA}` , this endpoint allow user to get the vault information ### ...
(PCT_SINGULAR_NOUN_PLURAL_VERB_AGREEMENT)
[uncategorized] ~83-~83: You might be missing the article “a” here.
Context: ...List of old party IDs ## How to setup plugin to run locally? # Setup Guide ## Prer...
(AI_EN_LECTOR_MISSING_DETERMINER_A)
🔇 Additional comments (2)
README.md (2)
69-73: Inconsistent casing between path (pluginId) and body (plugin_id)
The example mixes camel-case (pluginId) in the route with snake-case (plugin_id) in the JSON body. Verify the actual server contract; clients will break if the field name differs.If the backend expects the same identifier everywhere, normalise it:
- "plugin_id": "payroll-plugin-0000" + "pluginId": "payroll-plugin-0000"—or vice-versa.
97-103: Confirm Makefile targets exist & are documented
The README referencesmake up/make down, but previous commits removed/renamed several Makefile targets. Double-check that these rules still exist or update the docs; otherwise newcomers will hit “no rule for target” errors.
RaghavSood
left a comment
RaghavSood
left a comment
There was a problem hiding this comment.
LGTM
We can probably have a base image with go-wrappers and any future common dependencies though - non-blocking
Summary by CodeRabbit
New Features
Bug Fixes
Chores