feat(satp-hermes): gateway runner and docker for SATP #3509
Conversation
Signed-off-by: Bruno Mateus <brumat315@gmail.com>
brunoffmateus
force-pushed
the
satp-3117-pr
branch
from
cf69a41
to
9bd1f19
Compare
| @@ -0,0 +1,71 @@ | |||
| FROM ubuntu:22.04 | |||
There was a problem hiding this comment.
Can you use a smaller / lighter image? ubuntu 22.04 is very heavy and we do not need most of it . I recommend the NodeJS v18.19.0 which is the official node version we use in cacti (plus necessary dependencies)
There was a problem hiding this comment.
or Alpine - probably even better
There was a problem hiding this comment.
@RafaelAPB @VRamakrishna Ubuntu is a little fat, I agree, but they have the best track record of fixing CVEs in a timely manner though (that's the reason why I advocate for us using ubuntu $LTS as the default (24.04 soon or already if I remember correctly).
I worked a lot in the past on trying to fix CVEs on alpine base images and also the thinner Debian based ones, but it usually ends with me not being able to install a combination of dependencies in the images that have no vulnerabilities and then I just end up going with Ubuntu.
When I say CVEs: I mean that the trivy scans we have for the images are failing with critical and high severity vulnerabilities in the images with the OS level dependencies (which are not in our code but if we are publishing images then we are on the hook for those as well in my opinion the same we as for our own code).
So in general I recommend keeping with ubuntu for the purpose of production readiness and security even if the slower image pulls are annoying.
The other thing with alpine is that the packages break more oftne than on ubuntu. Me and @outSH and I think @sandeepnRES too had to fix images several times where alpine broke something with their default openssh packages and then the images just would stop building overnight even with our best efforts to pin everything to a specific version.
There was a problem hiding this comment.
@petermetz Agree on the fact that Ubuntu images (and I think package repositories) are kept more up-to-date w.r.t package vulnerabilities compared to Alpine. In other (non-Cacti) projects, I've seen vulnerability reports that prove hard to fix when the images are built on Alpine, and which then required me to manually replace and rebuild packages at build time. (E.g., curl)
So, on convenience (and arguably security) considerations, Ubuntu wins.
OTOH Alpine wins on performance.
| # memory: 2G | ||
| # cpus: '0.5' | ||
|
|
||
| volumes: |
There was a problem hiding this comment.
we need 1 volume to store the logs; other to store relevant data (such as keys). But its only for one gateway. (one docker container per gateway)
|
|
||
| # satp-hermes-gateway2: | ||
| # image: ghcr.io/brunoffmateus/cactus-plugin-satp-hermes-gateway:2024-09-02-3117 | ||
| # ports: |
There was a problem hiding this comment.
also add the creation and deployment of a local database (see package.json and knex)
| @@ -65,6 +65,12 @@ | |||
| "codegen:openapi": "npm run generate-sdk", | |||
| "codegen:proto": "npm run generate-proto", | |||
| "codegen:abi": "yarn forge:all && abi-types-generator './src/solidity/generated/satp-wrapper.sol/SATPWrapperContract.json' --output='./src/main/typescript/generated'", | |||
| "db:init": "run-s db:init:local db:init:remote", | |||
There was a problem hiding this comment.
this looks ok, but I thought there were some changes on these commands. make sure you have the latest rebased version of satp-dev
| @@ -0,0 +1,36 @@ | |||
| #!/bin/bash | |||
|
|
|||
| generate_keys() { | |||
There was a problem hiding this comment.
please migrate this to the makefile
|
Also, please add a description to the PR |
|
Feel free to remove draft from the PR, so the lint passes |
petermetz
changed the title
There was a problem hiding this comment.
@brunoffmateus LGTM with comments => plus one for the things Rafael said, I just have a dissenting opinion on the base image ( https://github.com/hyperledger/cacti/pull/3509/files#r1744367913 )
|
@brunoffmateus can you please incorporate @petermetz 's feedback and my comments as well? With regard to the base image, in light of the arguments above, probably ubuntu is a better choice |
No description provided.