Skip to content
This repository has been archived by the owner on Feb 24, 2020. It is now read-only.

rkt: inherit environment #565

Closed
philips opened this issue Mar 2, 2015 · 12 comments
Closed

rkt: inherit environment #565

philips opened this issue Mar 2, 2015 · 12 comments

Comments

@philips
Copy link
Contributor

philips commented Mar 2, 2015

All of the applications should inherit the environment by default. This is because we should assume that the user is running rkt under a tool like an init system where the environment is significant. To turn off environment inheritance we should have something like:

rkt run --inherit-environment=false example.com/foo:v2.0.4

/cc @robszumski

@vcaputo
Copy link
Contributor

vcaputo commented Mar 2, 2015

What takes precedence when the app has its own explicit environment settings?
i.e. if the app's image.json sets PATH=/foo/bar, do we really want to inherit the user's PATH all the way down to the app's environment and override /foo/bar with "/usr/local/bin:/usr/bin..." as the default behavior?

This doesn't strike me as particularly well thought out. When implementing rkt enter I had originally wanted to support a UX like FOO=bar rkt enter UUID /usr/bin/foobar but there's no distinguishing the variables set presently via the bash commandline and the rest of the user's environment. I would have liked to set FOO= in the entered context since it's clearly explicit, and leave behind the rest in favor of the app's environment. It looked like the best way to approach the problem was to add a commandline option for rkt to specify environment variables explicitly.

@nederdirk
Copy link

@vcaputo raises a good point, this was something I stumbled upon while working on this. Unfortunately there is no way to distinguish between intentionally and unintentionally set env vars.
Allowing to set variables explicitly would seem to make more sense indeed

@vcaputo
Copy link
Contributor

vcaputo commented Mar 2, 2015

It also doesn't seem like a very wise default choice to automatically inherit all environment variables down to a container's execution environment from a security perspective. There's potentially significant information leaking into something the user has opted to execute as a container, that generally implies a desire for isolation.

@philips
Copy link
Contributor Author

philips commented Mar 2, 2015

Here is how I think of the inheritance:

  • rkt execution environment with inherit-environment
  • image environment
  • pod manifest environment

I am fine with having --inherit-environment be false by default but I can see it as being a very nice feature that I would use.

We could also add a --environment flag that would essentially belong to the "pod manifest" and explicitly set environment variables that would clobber anything else. But, that is a different feature.

@philips
Copy link
Contributor Author

philips commented Mar 4, 2015

Anyone have thoughts on this order of operations? Seems straightforward and easy to understand. The primary use case will be things like REDIS_HOST=foobar MAILSERVER=baz types of stuff so I think it is OK in most cases.

cc @vcaputo

@sgotti
Copy link
Contributor

sgotti commented Mar 4, 2015

@philips the inheritance makes sense. I'll prefer to leave --inherit-environment disabled by default as it can expose sensitive data but also change the app behavior in unexpected ways.

I also think that a per app --environment flag that will add env vars to the pod manifest (permitting to add new vars and override the app's and the inherited ones) will be very useful.

@vcaputo
Copy link
Contributor

vcaputo commented Mar 5, 2015

Fine with me provided wholesale inheritance is disabled by default.

Just to confirm my understanding, when --inherit-environment is specified everything in the user's environment will make it into the environment of all the apps. Even environment variables explicitly set by the app's image will be overridden by the inherited values of same name.

@philips
Copy link
Contributor Author

philips commented Mar 5, 2015

@vcaputo Yes, it will be in all apps. The environment of the app takes precedence. In order of evaluation I see:

  1. rkt execution environment with inherit-environment
  2. image environment
  3. pod manifest environment

If FOO=bar in step 1 and FOO=baz in step 3 then FOO=baz will be the final value.

@vcaputo
Copy link
Contributor

vcaputo commented Mar 5, 2015

Ok, the utility seems fairly limited when it doesn't influence variables defaulted by the app, as those variables are likely impactful ones you'd like to alter via FOO_THING=bar rkt run foo.aci. But if we cover that with --environment then I guess it's fine.

In an rkt prepare and rkt run-prepared world, where is --inherit-environment honored? From the implementation's perspective, it's a natural fit for rkt prepare as that's the phase which currently produces the environment files for stage[12]. Should rkt run-prepared support it additionally and amend the environment state prior to running?

@robszumski
Copy link
Contributor

That does seem kind of limited. Wouldn't you want it:

  1. image environment
  2. pod manifest environment
  3. rkt execution environment with inherit-environment

Another semi-related question: is there a way in a systemd unit to have the namespace/whatever inherit only env vars specified in the unit but not from anywhere else?

@philips
Copy link
Contributor Author

philips commented Mar 5, 2015

@robszumski The problem is that there are important environment variables already in the environment like PATH, TERM, HOME, etc. Making some sort of blacklist gets really tricky pretty fast.

@philips
Copy link
Contributor Author

philips commented Mar 5, 2015

cc @jedsmith

philips pushed a commit to philips/rkt that referenced this issue Mar 10, 2015
When enabled the caller's environment is used to set variables not set by
the app images.

Fixes rkt#565
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants