acobaugh / shinobi-docker

A custom docker build for Shinobi
19 stars 13 forks source link

Improvements before we create a PR to Shinobi's main repo #2

Open viktorsmari opened 5 years ago

viktorsmari commented 5 years ago

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:

acobaugh commented 5 years ago

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.

viktorsmari commented 5 years ago

Yes, I think at first we need to use the entrypoint file to use sed but I want to create an issue or a PR on the main Shinobi repo, and ask the maintainer to allow the Shinobi node application to use ENV vars if they are provided (else it will use the current default). Inspired by the 12 factor app.

Example mysql setup where SQLHOST=mysql is provided in the shinobi.env file

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 sed magic in an entrypoint file, and hopefully eliminate the need for it.

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 sed and again, eliminate the need for the entrypoint file.

viktorsmari commented 5 years ago

FYI: I opened up a discussion about using ENV vars in the main repo: https://gitlab.com/Shinobi-Systems/Shinobi/issues/52

acobaugh commented 5 years ago

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.