nicolasff / webdis

A Redis HTTP interface with JSON output
https://webd.is
BSD 2-Clause "Simplified" License
2.82k stars 307 forks source link

Environment variable dirven alternative to webdis.json #174

Closed sirmmo closed 4 years ago

sirmmo commented 4 years ago

For usage with docker-compose a simple environment vairable dirven setup would be great. this way I would not need a volume to store the file while setting up the infrastructure

nicolasff commented 4 years ago

Hi Marco,

I'm not sure I follow. The webdis binary itself is a file, and so are its dependencies:

# ldd /usr/local/bin/webdis
    /lib/ld-musl-x86_64.so.1 (0x7f4d37ad9000)
    libevent-2.1.so.7 => /usr/lib/libevent-2.1.so.7 (0x7f4d37a65000)
    libc.musl-x86_64.so.1 => /lib/ld-musl-x86_64.so.1 (0x7f4d37ad9000)

All of these are files. A docker image is mostly a file system snapshot with some metadata added to it. You can put the config file anywhere in this tree.

In addition to this, some of the fields in the webdis config file are not scalar values. Sure, the redis host and redis port could use environment variables, but the ACL section is made of nested data structures and wouldn't be representable at all with environment variables.

sirmmo commented 4 years ago

sure, but if I am running a Docker Swarm setup some variables are way more important than others. Specifically, for example, the redis host i want. If I want to use a specific ACL, I agree the best way is to use a file, but in a more general structure it would be great to be able to go with, for example

docker run -e REDIS_HOST=redis -e REDIS_PORT=9028 -e REDIS_DB=2 nicolas/webdis

because this would be very docker-swarm friendly

nicolasff commented 4 years ago

Hey Marco,

As indicated by the title Try in Docker on the README, the current purpose of the image published to the Docker Index is not production use, but a way to try out webdis without having to configure anything. This is why the image embeds Redis instead of requiring a separate container be configured with Redis itself.

This being the current situation doesn't mean it can't change, though! I think that the best way to achieve what you are suggesting would require two changes:

  1. It would certainly be useful to have production images published in Docker Index, in a way that is compatible with Docker Compose or Docker Swarm (or both). I'd like to keep at least one demo image that embeds Redis, but that doesn't preclude having another tag that's for production use and attempts to connect to a remote Redis instance.
  2. I do agree that environment variables can be useful for the purpose you describe, just not as a full replacement of webdis.json for the reasons stated above. I had given this some thought in the past, and my idea for such a feature was to add support for uppercase strings starting with a dollar as valid values for most fields in the JSON file, in which case the corresponding environment variable would be used to read the value. Something like the Spring Boot expression syntax could also work, e.g. ${REDIS_HOST:127.0.0.1} where the value after the : provides a default to use if the environment variable is not set.

A combination of these two new additions would likely address the issues you mention. As such, let me remove the WONTFIX label. That said, as I have mentioned in the past I am not able to contribute code changes to Webdis at this time and my involvement has been limited to reviewing and merging external contributions for the past few years. I would certainly welcome pull requests for a new swarm/compose Dockerfile and for environment variable support in src/conf.c.

sirmmo commented 4 years ago

perfect! thanks!

nicolasff commented 4 years ago

Hi @sirmmo! An update: a PR was submitted following my earlier call for contributions to support environment variables in the config, so you should now be able to build a Docker image from Webdis 0.1.10 and configure webdis.json using something like this:

{
    "redis_host": "$REDIS_HOST",
    "redis_port": "$REDIS_PORT",
    "database": "$REDIS_DB",

(followed by all the other fields of course).

Then you'll be able to run it with the command you had suggested:

docker run -e REDIS_HOST=redis -e REDIS_PORT=9028 -e REDIS_DB=2 webdis

Since this is a custom use case there's no image with this exact syntax on Docker Hub, but it's pretty easy to build.

sirmmo commented 4 years ago

great! thanks!!