introduces SS_EXTRA_DOT_ENVS var for declaring additonal variables

[andrewandante]

This will allow the user to declare SS_EXTRA_DOT_ENVS in their primary .env file in order to include other ENV-setting files in their BASE_PATH and above.

Syntax:

SS_EXTRA_DOT_ENVS='.env2, .env3'

These will be exploded on "," and looped over. They will be called with overload, meaning any vars set in .env will be over-written, and this will proceed all the way down the chain.

This allows us to include temporary or conditional env vars as necessary.

[kinglozzer]

I’m a bit confused, what’s the use case for this?

---

Edit: sorry, just saw:

This allows us to include temporary or conditional env vars as necessary.

Shouldn’t that be handled at an environment level, or by loading them manually in _config.php?

[andrewandante]

We are currently using conditionals in our _ss_environment.php to set temporary SS_DEFAULT_ADMIN_* values. We do this by including an additional file if it exists, and removing that file via cron once it has existed for a set period of time. The file is created by a script triggered by a user. Hope that part makes sense.

This seeks to replicate the existing feature for an SS4 site. I'm sure there are other use cases as well.

[kinglozzer]

I see, could the logic in this PR not be put in mysite/_config.php in that case? Or in a module?

Part of the reason for switching to .env files was to encourage the use of proper environment variables, this is kinda encouraging the opposite (I can already see .env-dev and .env-live appearing in document roots 😜).

[andrewandante]

I have made a module version of this already - so that is possible; however, it means enforcing this module's inclusion for all of the sites on our hosting. _config.php won't solve the problem for us as we don't have control over each individual site's code, so we'd be hacking in post-deployment, which is gross.

[dhensby]

My views on this are as follows:

1. .env files are only intended for use on dev environments. I can't foresee a usecase on dev environments where this is a simpler solution than editing the .env directly. Whilst it may be likely these are used in prod, we shouldn't provide the tools to encourage it any more than is strictly needed.
2. This limitation (not having includes) was accepted and is intentional as of the PR that moved to .env (see NEW replace _ss_environment.php with .env and environment vars #6337)
3. The use of overload instead of load is dangerous and a security issue. Overload replaces even firstclass environment variables and the core should not provide a means to do this. This will have to be changed to load if this PR gets to the point of merge.
4. This can be achieved with the use of a module (as you've pointed out - https://github.com/andrewandante/silverstripe-extradotenvs) or in usercode.
5. My understanding is that the work being done on the App object (RFC: App object to encapsulate global state and services #6681) will allow you to hook into the bootstrap process early enough that anything you're trying to solve this way can be solved by user code and/or modules.

I think this change means we slip towards encouraging bad practice and idiosyncrasies (which we are trying to move away from), it feels unnecessary and adds yet more to the boot process that serves a very slim usecase over the benefits of everyone else.

We can't hope to cover every usecase, for that there are modules.

[using a module] means enforcing this module's inclusion for all of the sites on our hosting

This feels reasonable to me - are there not modules that are already a requirement to be on the hosting? Alternatively you could look at generating environment variables using some automated process that would write them to a unified .env or better make them first class env vars.

[dhensby]

see #7001 (comment)

[tractorcow]

1. .env files are only intended for use on dev environments. I can't foresee a usecase on dev environments where this is a simpler solution than editing the .env directly. Whilst it may be likely these are used in prod, we shouldn't provide the tools to encourage it any more than is strictly needed.

No my view is that it's not. .env is for the environment config, regardless of whether that's live or dev. It replaced _ss_environment.php which was necessary for live configuration. A global per-machine env is not a live replacement, especially on shared hosting.

2. This limitation (not having includes) was accepted and is intentional as of the PR that moved to .env (see NEW replace _ss_environment.php with .env and environment vars #6337)

Not having PHP includes was the intention, to eliminate script execution risk.

3. The use of overload instead of load is dangerous and a security issue. Overload replaces even firstclass environment variables and the core should not provide a means to do this. This will have to be changed to load if this PR gets to the point of merge.

Agree with this.

4. This can be achieved with the use of a module (as you've pointed out - https://github.com/andrewandante/silverstripe-extradotenvs) or in usercode.

I also argued this point earlier, but this is a limitation we want the platform to enforce on the usercode, rather than something usercode opts in to. That being said, I could go either way. :P

[andrewandante]

Not having overload largely defeats the purpose of this I think, so if that is a deal-breaker then we may as well close it. Thanks for a reasoned discussion team!

[tractorcow]

Not having overload largely defeats the purpose of this I think

Sorry, I think you may need to put it into your own module at this point. =( Or otherwise come up with some other .env file generator that exists outside of the ss project itself.