Skip to content
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

configuration: support environment variables #3543

Closed
muxator opened this issue Jan 23, 2019 · 7 comments
Closed

configuration: support environment variables #3543

muxator opened this issue Jan 23, 2019 · 7 comments
Assignees
Labels
Milestone

Comments

@muxator
Copy link
Contributor

muxator commented Jan 23, 2019

A common use case when running an application inside Docker is passing configuration parameters via Environment variables.

We could introduce support for reading the values of configuration parameters from environment variables in settings.json, taking inspiration from ElasticSearch's environment variable substitution:

Environment variables referenced with the ${...} notation within the configuration file will be replaced with the value of the environment variable, for instance:

node.name:    ${HOSTNAME}
network.host: ${ES_NETWORK_HOST}
@muxator muxator changed the title configuration: supprt environment variables configuration: support environment variables Jan 23, 2019
@muxator muxator added the docker label Jan 23, 2019
@muxator muxator added this to the 1.8 milestone Mar 6, 2019
@poVoq
Copy link

poVoq commented Mar 6, 2019

There are already some unofficial Docker images that support this. Maybe worth a look?

Important would be to be able to set the API key secret via an ENV variable so that on redeployment of the Docker image one doesn't have to manually change all external services to a new random API key. Thanks!

muxator added a commit that referenced this issue Mar 8, 2019
This is a super simple start.
At minimum, configuration via environment variables (see #3543) needs to be
integrated in Etherpad to make this user-friendly.

Resolves #3524.
@muxator
Copy link
Contributor Author

muxator commented Mar 10, 2019

Implemented. The PR is #3572.

Please note that there is going to be a little Etherpad-specific remark, due to being tied to use a JSON file for configuration.
Even when replacing a variable whose value would be a boolean or a number, quotes must be used. Etherpad, then, takes care of converting back the configuration parameter from string to the right type.

The documentation is in https://github.com/ether/etherpad-lite/blob/3b6b12e8482bb641252c2aa8fff225d61cd5225b/settings.json.template.

   "port":   9001,          <-- Literal values. When not using substitution,
   "minify": false              only strings must be quoted: booleans and
   "skin":   "colibris"         numbers must not.

   "port":   ${PORT}        <-- ERROR: this is not valid json
   "minify": ${MINIFY}
   "skin":   ${SKIN_NAME}

   "port":   "${PORT}"      <-- CORRECT: if you want to use a variable
   "minify": "${MINIFY}"        substitution, put quotes around its name,
   "skin":   "${SKIN_NAME}"     even if the required value is a number or a
                                boolean.
                                Etherpad will take care of rewriting it to
                                the proper type if necessary.

@muxator muxator self-assigned this Mar 10, 2019
@poVoq
Copy link

poVoq commented Mar 10, 2019

That looks great already.

Any chance to also implement setting the APIKEY.txt (as explained here: https://github.com/ether/etherpad-lite/wiki/HTTP-API ), similar to how this Docker image does it: https://github.com/tvelocity/dockerfiles/tree/master/etherpad-lite ?

Or is there another way (with a volume?) to prevent the API key changing when recreating/updating the docker image?

My use-case: I have Etherpad integrated in Dokuwiki and Nextcloud, and both require setting the API key, and ideally I would not have to change it manually every time the Docker image is updated.

Thanks a lot!

@gobengo
Copy link

gobengo commented May 20, 2019

This is awesome! Thanks for implementing it @muxator

Any possibility of releasing this in a tagged version soon without waiting for other issues slotted for 1.8?

I'd love to be able to use this from an etherpad/etherpad docker image without pinning to the much-looser developer tag.

Thanks again!

@gobengo
Copy link

gobengo commented Jun 23, 2019

@muxator Here is something I made to run etherpad-lite on kubernetes. https://github.com/gobengo/etherpad-lite

Configuring the database credentials is most appropriate with environment variables (from a k8s secret) in that context, which requires this feature.

But since there isn't a release that has this feature yet, I had to use the 'latest' tag of the docker image.

Just sharing the use case. Thanks again for adding this. Look forward to having it in 1.8

@muxator
Copy link
Contributor Author

muxator commented Jun 23, 2019

Hi, @gobengo, I am glad this enabled a new deployment possibility.

A rough breakdown of the remaining issues for the 1.8 milestone is as follows:

  1. there are two remaining security issues that really have to be fixed before releasing 1.8:
    1.a Fix vulnerability in tar < 4.2.2 #3598, a problem with tar library in a transitive dependency of npm
    1.b the clean-css library requires a major version bump, and this requires some work (see Fix a few vulnerabilities #3597 (comment) Regex Denial of Service requires a semver major update to clean-css #3616)
  2. the async migration introduced many important changes, but there are still two issues that need to be addressed (see HTTP API needs POST for large parameters in NodeJS 8.14+ #3568 and Async API changes may break plugins #3566)
  3. there is some feature work that could probably be postponed to a later version

If you have the possibility of working on 1.a, 1.b or 2, this could help a lot.
Thanks!

@raybellis
Copy link
Contributor

The code for #3568 is done - it mostly requires a documentation change.

I can look at #3566 again. AFAIK the patch I proposed works, but I believe I can simplify it so that it doesn't require the use of eval.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants