feat: Dockerize DP 🐳 #22
Conversation
This commit introduces a dockerized workflow for data proxy.
This commit primarily focuses on enabling conditional logic for initialization via `bun start run <COND>`.
- Implement dynamic configuration handling in entrypoint script - Add support for three startup scenarios: 1. Both config.json and private key available 2. Only config.json available 3. Neither config.json nor private key available - Introduce automatic `bun start init` execution when needed - Improve error handling and logging in the entrypoint script
kaynetik
force-pushed
the
kaynetik/docker
branch
from
cebda43 to
0041f80
Compare
|
I'll update the docs in the separate PR, won't forget it ;) |
There was a problem hiding this comment.
Unless we standardise on something as a team I would expect these files to be ignored. In the explorer we use https://commitlint.js.org/guides/local-setup.html and Husky to manage git hooks, but given that a more diverse group works in this repo we should probably discuss it first :P
There was a problem hiding this comment.
I didn't notice Husky was used. pre-commit wouldn't force anyone, nor would it be active locally before you call pre-commit install.
Where is the adequate spot to initiate this discussion? @Thomasvdam
There was a problem hiding this comment.
Probably in the engineering Slack channel, or opening a PR that introduces the tool.
The reason I commented on this file is that if we want to commit this file (and this workflow) we should also document it in the repo, otherwise it's just clutter.
| # Neither config.json nor private key is provided | ||
| echo "Running with --disable-proof" | ||
| run_bun_command start init | ||
| RUN_CMD="start run --disable-proof" |
There was a problem hiding this comment.
This doesn't make sense: --disable-proof should only skip validating whether the incoming request has the correct proof, it doesn't deal with the private key or config at all.
There was a problem hiding this comment.
If neither of the two is provided, then my assumption was that we intend to run this in "debug" mode.
Otherwise, if we initiate bun start run when those two aren't provided and -dp isn't enabled the tool will call exit 2.
There was a problem hiding this comment.
How would you like me to address third scenario, or do we have 3rd and 4th that I missed?
There was a problem hiding this comment.
The data-proxy can not run when there is no config or no private key (file). --disable-proof only disables the request verification, so it should be a completely separate option.
I think the scenarios should be covered by the binary itself where it will error if it is unable to find a config and/or private key (file).
If someone wants to run it in debug mode they should pass the --disable-proof flag manually.
There was a problem hiding this comment.
yes, passing it manually makes sense, maybe we could have an container env variable like FLAGS or so..
| if [ "$CONFIG_EXISTS" = true ] && [ "$PK_EXISTS" = true ]; then | ||
| # Both config.json and private key are provided | ||
| echo "Running with config and private key" | ||
| RUN_CMD="start run --config /app/config.json --private-key-file /app/data-proxy-private-key.json" | ||
| elif [ "$CONFIG_EXISTS" = true ] && [ "$PK_EXISTS" = false ]; then | ||
| # Only config.json is provided | ||
| echo "Running with config only" | ||
| run_bun_command start init | ||
| RUN_CMD="start run --config /app/config.json" |
There was a problem hiding this comment.
I think the binary takes care of these checks, is there a reason to also do it in the entrypoint?
There was a problem hiding this comment.
So that I know how to execute the binary.
e.g.
if [ "$CONFIG_EXISTS" = true ] && [ "$PK_EXISTS" = true ]; then => bun start init not required
Also, if I don't know which ones were provided do we really wanna guarantee graceful handling of non-existent files. Or this scenario => we call bun start run --config /app/config.json --private-key-file /app/data-proxy-private-key.json" but file app/data-proxy-private-key.json" doesn't exist?
There was a problem hiding this comment.
I'm not entirely sure what the best approach would be here... 🤔
I'm partial to letting the binary take care of everything as this reduces the maintenance burden on our end since the container doesn't really need to know anything.
If I want to initialise I should do docker run <IMAGE_NAME> init and pass a volume so I can persist the config and generated key.
For registration I can then do docker run <IMAGE_NAME> register <PAYOUT_ADDRESS> <AMOUNT> and click the link from the output to complete the registration.
And to run it just do docker run <IMAGE_NAME> run.
I guess that get a little messy with compose though, don't know how to do that in an elegant fashion.
There was a problem hiding this comment.
My primary goal here was to dockerize in the best way possible that would enable first and foremost a functioning deployment.
Having a manual step in the deployment to devnetv4 would increase friction a lot.
Can we discuss these points on a call?
| # If private key file does not exist, check if the private key is provided via environment variable | ||
| echo "Private key provided via environment variable" | ||
| echo "$SEDA_DATA_PROXY_PRIVATE_KEY" >/app/data-proxy-private-key.json | ||
| PK_EXISTS=true |
There was a problem hiding this comment.
I don't think this will work, as the key file is a JSON file with both the public and private key. I'd imagine most people would only supply the private key as an environment variable.
There was a problem hiding this comment.
Then how do we use $SEDA_DATA_PROXY_PRIVATE_KEY?
There was a problem hiding this comment.
If the env var is set the data-proxy uses that to construct the private/public key pair and won't even attempt to read the key file.
TODOs:
docker/build)Commits
Motivation
(Write your motivation here)
Explanation of Changes
(Write your explanation here)
Testing
(Write your test plan here)
Related PRs and Issues