trynd / wren

Linux boot platform that provides a portable, multi-system, run-in-memory Linux environment.
GNU General Public License v3.0
0 stars 1 forks source link

Remove platform version and display name from environment variables #15

Closed codewithmichael closed 9 years ago

codewithmichael commented 9 years ago

Pull request #9 (Add platform version and display name to default environment variables) sounded like a good idea at the time, but there quickly became an obvious flaw. If the user overwrites the /etc/environment file to add additional variables, subsequent platform updates won't reflect themselves in the file, as the platform layer's version will be overshadowed by the user layer's version. In other words, if the user writes to the file, the reported version becomes forever locked for that save and updated versions will still report the older version number.

The platform name ($RUN_ENV_PLATFORM_NAME) is problematic to remove, since it's currently used to locate platform-env by most scripts. Perhaps we could update them to look for it within their own directory(?). In any case, since it's a value that's unlikely to change mid-save, we can probably just leave it where it is for now. It's improbable that someone will rename the platform and rebuild it in a new location mid-save — if they do they probably also know that a manual environment update may be required.

The version number is hard-set in platform-env, so even if it were wrong in /etc/environment, anyone sourcing platform-env would get the correct value. But the platform name and display name are only updated in platform-env if they don't already exist. Perhaps they should be hard-set as well, since platform-env is used in initramfs-script and by every runtime script, allowing that value to change externally doesn't really make any sense. Besides, /etc/sudoers was never updated with the new values in wrender, so all scripts run using sudo are getting their values from platform-env instead of /etc/environment, except for $RUN_ENV_PLATFORM_NAME which is explicitly listed in /etc/sudoers.

Anyway, if we do get truly variable environment variables, they belong in /var/run/wren/environment, so as to be dynamically allocated during startup. Maybe it would be better to update /etc/pam.d/login (and perhapse /etc/sudoers) with an extra environment file loader as is already done with /etc/default/locale, i.e.:

Add to /etc/pam.d/login: session required pam_env.so readenv=1 envfile=/var/run/wren/environment

Example of adding sudo accessible variable to /etc/sudoers: Defaults env_keep += "RUN_ENV_VARIABLE_NAME_HERE"

Of course, then we would have the issue that those files couldn't safely be user-layer updated either.

codewithmichael commented 9 years ago

This issue is address by pull request #17 (wrender: Remove platform version and display name environment additions from the COPY block), which is ready to go.

codewithmichael commented 9 years ago

Just a note — there's no need to address a new /var/run/wren/environment file or updates to /etc/pam.d/login for this issue and such changes are not in any way included in the associated release. The idea may be useful down the road.

codewithmichael commented 9 years ago

Released in v0.1.2.