Loading the configuration file in the client from the server instead

[buxxi]

Resubmit of #1940 since I had some merge problems with git. Now with working tests too.

I took a first stab at making it possible to use environment variables in the configuration file ( #1756 ).

The biggest problem with this seemed to be that both the client and the server both loads the config file each, where the server has access to the environment variables and the client doesn't. But since the server already exposes the possiblity to get the config file like this: https://github.com/MichMich/MagicMirror/blob/5bf90ae31d600e3f595ffd242b99515fa7905b1a/js/server.js#L56-L58
I figured fetching that as config instead of just including it would cause it to actually have the servers evaluated config instead (instead of this line):
https://github.com/MichMich/MagicMirror/blob/5bf90ae31d600e3f595ffd242b99515fa7905b1a/index.html#L43

I've tested it by setting configuration values to

{
	module: "helloworld",
	position: "lower_third",
	config: {
		"text": process.env.HELLO_WORLD
	}
}

And it seems to work and even apply to the client. Maybe this shouldn't be the recommended way to use environment variables in the configuration file, but this pull request would probably be needed whatever is decided.

Hope its ok to use fetch() since it works with npm start and all modern browsers seems to have supported it for a few years ( https://caniuse.com/#feat=fetch )

[Legion2]

This changes the runtime the config.js is evaluated in and will properly break some configurations, that depend on being evaluated in the client.

[MichMich]

I agree with Legion2. This has some implications. It might be simpler to add a helper function which you can use in your config:

{
	module: "helloworld",
	position: "lower_third",
	config: {
		"text": env('HELLO_WORLD', 'Fackback String')
	}
}

In the above example the env will communicate with the server to retrieve the env variable.

[Legion2]

It is also possible to use a preprocessing step for config.js, that replaces all environment variables with their values at start time. This step must always be executed before starting MagicMirror.

@buxxi If you are using the docker image of MagicMirror you can use https://github.com/jwilder/dockerize to do this. It supports a template syntax and can "inject" not only environment variables into any text file.

[MichMich]

Check https://docs.magicmirror.builders/getting-started/manifesto.html#manifesto regarding preprocessors. Let's try to keep it simple as possible. :)

[buxxi]

That dockerize just seems way too complicated for my preferences and I'm not sure if I want my setup for MM to depend on running it in a container.

That helper-function looks good, but how would it work since as far as I know you can't make the ajax-call to server be blocking? Tried to do something like that first without much success.

Understand the concern for breaking configurations but it feels kinda strange to have a file evaluated both in the client and the server where the same property could get two different values. Stuff like that would be really annoying to debug. Not that it helps in this case, but you mean it should really be the other way around where the client sends the evaluated config to the server instead (don't want to do that, just want to understand)?

[buxxi]

Made an alternative attempt which actually gets almost the syntax @MichMich requested, but have yet to push it to github (first some refactoring, running tests etc). Not really liking this better than this pull request, but doing xmlhttprequest synchronous is deprecated so that doesn't seem to be a safe way to do it. The only other way I could possibly think of doing this is with some async+await+eval shenanigans.

It's working like this:

• Made a new module env that exposes 2 methods; get(name, defaultValue) and getAllPrevious() (gets an object with all previous calls to the get populated as key-value)
• server.js uses the getAllPrevious() to get all the environment-variables that was used when loading the server side config (So all unrelated environment variables isn't sent to the client) and injects that into the index.html like #VERSION# and #CONFIG_FILE#. For now I populated process.env with that object on the client side to make the env-module simpler.
• then the module is included in index.html too.

[Legion2]

@buxxi If your only concern is to use environment variables in the config/config.js you can use envsubst which is installed on most systems by default. Rename the config.js to config.template and use the shell variable syntax:

{
	module: "helloworld",
	position: "lower_third",
	config: {
		"text": "${HELLO_WORLD}"
	}
}

Create following script and make it executable.
run-start-with-env.sh:

#!/bin/bash
envsubst < config/config.template > config/config.js
./run-start.sh "$@"

Run run-start-with-env.sh instead of run-start.sh, it will read the environment variables on the server and create the config/config.js with the values of the variables.
envsubst does not support default values, but there are similar tools, that support more features.
Hint: all strings starting with $ are considered variables and are replaced even if they are empty or unset.

[buxxi]

Well sure, if we can't get to a solution where it is built in I'll probably have to make some templating/scripting, but I would like to avoid that if possible. I don't really want to have to maintain a custom startup script, dockerfile etc just to avoid having api-keys in the configfile.

I don't think I'm the only one (got 2 likes for this PR 😄) who wants the possiblity to set configuration values to environment variables.

[khassel]

Would also be happy to get a solution for this. The solution from @Legion2 could be implemented in a docker setup and/or in the script-repo from sam, but I think a "core-solution" is better.

Another point (want not to mix in another thing in this PR but it is similar): The config.js is public, so you can't run a mm under a public url without sharing all your api keys in the config.js.

[buxxi]

Created a new branch with my other alternative way of doing this, see this commit:
buxxi@c8d393f

Can be used in the config as this:

{
	module: "helloworld",
	position: "lower_third",
	config: {
		"text": env.get('HELLO_WORLD', 'Fackback String')
	}
}

Not totally happy with how the environment-object is loaded in index.js, but I figured it was better than using another global variable or having to escape it to put in an attribute.

And it doesn't seem to be possible to change the source branch for the pull request so please tell me if this is the way to go before I close this and open a new pull request again.

[buxxi]

Care to take a look at it @MichMich?

[MichMich]

The problem with this, is that I have the feeling that we are overcomplicating things for just some edge-cases. Maybe it's better to write a separate module that fetches ENV variables does some magic with it? This way, the login stays outside of the core.

[buxxi]

Well, I'm not really convinced it being an edge case. But will try to make a module instead (if I can make some magic 😄).

[Legion2]

I created bastilimbach/docker-MagicMirror#42 which add support for template variables in the configuration file to the docker image. The template variables are evaluated on the server side before the server is started.