Add CLI option to add env vars using "-e" for cloud/clustered execution

[robingustafsson]

Today k6 exposes the machine's environment variables to JS via the global variable __ENV when running a test locally. But, when running in the cloud it doesn't make sense to read the environment variables of the local node (in a clustered mode it might still make sense).

I suggest we add a CLI option -e MY_ENV_VAR=hello (that can be specified multiple times) to the run and cloud commands (by adding the option the optionFlagSet [1]) and save the env vars in the Options [2] and then either add the them to __ENV for the local execution case or store them in the "metadata.json" file that is part of the archive [3] created when triggering a cloud executed test (the env vars will then be made available through __ENV on each cloud node).

[1] - https://github.com/loadimpact/k6/blob/master/cmd/options.go
[2] - https://github.com/loadimpact/k6/blob/master/lib/options.go
[3] - https://github.com/loadimpact/k6/blob/master/lib/archive.go

[na--]

I'll soon submit a pull request for this issue, but here are some pertinent excepts from an email discussion with @robingustafsson about some changes in the implementation details from the original issue description:

(me) ... it seems that currently the actual environment variables are collected and the goja runtime (and __ENV variable) is instantiated before we pass the Options to the runner. I'm not sure which will be cleaner and/or easier - assembling the options and passing them on when creating the new runner or updating the __ENV variable after the runtime is instantiated. Right now I think the first one is the better approach and I'm not sure if the second one is even possible or easy...

(Robin) Hm, this is a bit tricky I guess. k6 runs the JS in two phases, an init phase where it executes code in the global scope importing modules and exporting the in-script options and a default/main function. In the second phase when the test is actually executing it runs the default/main function over and over. But, the important bit here is that the in-script options exported in the first phase are used in the options cascading. We need to get the -e specified env vars into __ENV already in the init phase as they could be used to influence the in-script options or for open()'ing files, so they need to be passed to the "Bundle" via the "runner".
We can see the -e specified env vars as "runtime options" rather than "config options", so I'd say it's warranted to have them outside of the regular Options struct (and cascade/assembly), but they need to be serialized/marschalled to metadata.json when creating an archive so we can get access to and can set the env vars properly when running the test through the Load Impact cloud service (when triggering a cloud run we upload an archive to the cloud service).

And a follow-up question by me:

Please tell me if I correctly understand what's happening. Here the values from r.GetOptions() can be returned by the first phase of the JS execution in the runner and they're merged into the other configuration?
If so, can't I just split the long "conf := ...." line into two parts? Move the initialization of cliConf and fileConf and the first part "conf := cliConf.Apply(fileConf)" so they happen before the newRunner() call and pass the conf to it. After the runner is initialized I can just call "conf.Apply(Config{Options: r.GetOptions()}).Apply(envConf).Apply(cliConf)". Not sure why envConf is applied only after the runner options, but are there any reasons such a split will not work?

With the following answer from Robin:

Yes, r.GetOptions() returns the options extracted from the first phase of the JS execution and are then merged/cascaded with the other sources of options into a final set of options. The order of importance is (-> meaning overrides):

CLI -> Env vars -> Script options -> Config file options
As long as the above order-of-importance is maintained we can move things around. But, I don't think we necessarily need to do that in this case as the options resulting in the final "conf" variable are not really used in the script at runtime, whereas the -e specified env vars would be (the env vars referred to as "envConf" are more execution related; how many VUs to run etc.). So I think we could see them as two different types of options, "test/script options" vs "config options", and just collect the env vars needed (system env vars if needed and any env vars specified as -e) for Bundle and pass it to newRunner().

And about an extra flag that disables the passing of system environment variables entirely:

(me) ... maybe it would be a good idea to have another command-line flag that disables the passing of actual environment variables entirely? That way only the ones that were explicitly passed with the CLI flag will be present. This seems useful if you don't want user scripts to have access to information about your cloud environment for security reasons or simply because it's wrong or not needed anyway.

(Robin) Yes, that sounds like a good idea, something like a --no-system-envvars. When we run k6 tests in the Load Impact cloud service we sandbox the process with an empty environment, but agree that the flag would be handy.

I probably should have asked the questions directly in this issue in the first place... In any case, I decided to copy the discussion here because due to it my implementation deviates from the original issue text (ex. cmd.optionFlagSet/lib.Options is not used for parsing and passing the environment vars, --no-system-envvars is added, etc.)