-
Notifications
You must be signed in to change notification settings - Fork 14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improvements before we create a PR to Shinobi's main repo #2
Comments
The dualstack was something specific to my specific use case, where it needed to be on the same docker network as another compose stack running nginx/letsencrypt, on a dual stack v4/v6 network so I could reach it from outside my house on ipv6, and within the house on ipv4. Give me the weekend to try separating out my configuration locally here. We should definitely do something about super.json. ENV vars might work here, I forget how shinobi is using that. Maybe a simple sed in the entrypoint to replace some placeholders in super.json? Open to suggestions. |
Yes, I think at first we need to use the entrypoint file to use Example mysql setup where if (process.env.SQLHOST) {
host = process.env.SQLHOST; // Will become the 'mysql' container if using Docker
}else{
host = '127.0.0.1'; // Default for people not using Docker
}
//EDIT: Or using a one-liner
host = process.env.SQLHOST || 'mysql' SQLHOST could also be called DOCKER_SQLHOST or whatever makes sense. That means when we supply these env vars with Docker, the app will use them. This way we should not have to do some I am not sure which files need to support this in the main Shinobi application, but I found these in the entrypoint file. Are these the values we should provide ENV support for? /opt/shinobi/conf.json
/opt/shinobi/super.json
If we perfect this list, we can make a PR to the main repo just to accept ENV vars, then it should be easier for us, not having to use |
FYI: I opened up a discussion about using ENV vars in the main repo: |
Yes, those are the values we'd want to source from ENV. Ideally we could abstract that so the ENV var name is predictable in some manner, based on JSON path, so we don't have tons of if{} statements, or have to modify the code every time a new variable is added. |
Awesome work @acobaugh !
Just tried it and it works out of the box with 2 commands.
I noted down some ideas in order to make this even simpler to setup:
docker-compose.yml
general, and add adocker-compose.prod.yml
for customization (TLS, network setup, more containers etc)Dockerfile
,docker-compose.yml
andshinobi.env.example
docker-compose -f docker-compose.yml -f docker-compose.prod.yml up -d
docker-compose up
because it requires you to add adualstack
network first. Do we need it? It can be extended in adocker-compose.prod.yml
file f.x.super.sample.json
and thedocker-entrypoint.sh
files?The text was updated successfully, but these errors were encountered: