Pass env vars set in workflow file to docker environment

[tobyspark]

Context

It’s standard practice to set environment variables in GitHub actions, e.g. Unity - Builder uses env to pass UNITY_LICENSE. However arbitrary env variables are not passed to the docker container where Unity runs, so they cannot be accessed in any Unity-based tooling for the game. While understandable in hindsight, this is not what I expected going in, and there’s no indication in the documentation that arbitrary env vars will fail.

Suggested solution

Passing environment variables set in the workflow file to the docker environment running Unity would be good general practice for all such Game-CI actions.

Considered alternatives

Searching Discord I see the alternative is to set custom parameters, and from Unity use Environment.GetCommandLineArgs(). I am concerned this may leak secrets into logs (am about to implement this so will see).

Additional details

Pointed in the right direction, I should be able to work up a PR for this.

[webbertakken]

Thanks for opening an issue for this @tobyspark

This is by design, let me try to outline a tl;dr: Our actions aren't a thin wrapper around a docker container. Their goal is actually to take away as much complexity for the user as possible. We do this by exposing a public API in the form of configuration options.

We allow people passing in custom parameters so that you can basically pass in anything, while at the same time having a documented place of what's passed into Unity, which also needs to be used in the build script, making users implementations cleaner and with less hidden complexity. This also means it's easier to share configurations and build scripts with other teams.

Finally, most teams need exactly the same functionality of a CI workflow. We much prefer making features visible (especially for people that are newer to Unity, or to CI). This is done by adding common features to said public API.

---

That said, perhaps it's useful to add a concise line in the docs about env variables not being transparently passed into the container.

Although please note that that isn't exactly the default behaviour in other actions either, oftentimes you want to abstract away from "it using a container under the hood" completely. This is what we are doing.

Hope this helps :)

[tobyspark]

That does help, thank you.

Also, having implemented the command-line reading version, my fears of leaking secrets were unfounded. At least, GitHub Actions does an excellent job of scrubbing them wherever they appear.

For what it’s worth, I share your motivation, but I struggle with the design conclusions here.

• Passing env is exactly what you would want to do given the goal of abstracting away that “it uses a container under the hood”.
• Passing variables via env is what GitHub Actions promote, and is generally expected in CI environments. customParameters is, well, custom and so not “removing complexity” for “people that are newer to Unity or to CI” or otherwise.

Plus, on taste –

• It’s ugly. Both in the workflow file and code required to parse them out.

One final thought is that because our app doesn’t have tests (at least, not that would run here, yet) I skipped that part of the getting started. And that’s where use of customParameters is discussed, not Builder.

[tobyspark]

https://github.com/game-ci/documentation/pull/422

[webbertakken]

1. Passing env is exactly what you would want to do given the goal of abstracting away that “it uses a container under the hood”.
2. Passing variables via env is what GitHub Actions promote, and is generally expected in CI environments. customParameters is, well, custom and so not “removing complexity” for “people that are newer to Unity or to CI” or otherwise.

The second point isn't completely accurate in my opinion. Actually they promote either using with or env to let's say "communicate" with the action. We have decided to go with the in our opinion cleaner with variant where possible (after initially having used env exclusively. note that with still uses INPUT_* env variables internally). For for most intents and purposes we already use what they promote. The only thing we don't do is allow arbitrary (and undocumented) env variables, which brings me to your other point.

First off I think your point 1 is an excellent point.

It would however create a duality of APIs (the programmatic API and env variables). The latter can have conflicts that can be hard to debug. Allowing arbitrary env variables adds a lot of variability to the behaviour of the container (for example date, timezone, charset, unintended variables, SDK version values etc. which are covered in actual logic instead - abstracting away from quite a bit of other complexity at the same time, like how to call the Unity Editor from command line in for different use cases - I promise, it's not straightforward like we initially thought too), which can end up making it hard to debug differences between different CI systems (we don't only support GHA). Instead we chose to keep it a bit more agnostic.

It is in fact very hard to find out how people use our actions, by having people collaborate on a shared API we believe we end up solving more problems and we're able to make it more user-friendly towards newer developers. Part of this rationale is also described in https://github.com/game-ci/cli/issues/9.

I'd say your approach would have been sound too.

[tobyspark]

Quibbles aside, it’s good to see such thought put into it. Thanks for pushing the documentation clarification, and your efforts generally.

[webbertakken]

Thanks for your contributions!

Closing this but feel free to reopen at any time.