atomic: add '--env' option

[praiskup]

.. to 'install', 'uninstall' and 'run' commands. This way we can
now do stuff like this:

sudo atomic install -e CONFIG='some value' ...

Or:

sudo CONFIG="some value" atomic install -e CONFIG ...

While INSTALL script can do simply:

'docker run ... -e CONFIG ... SCRIPT'.

So far we could use only --opt2 or 'args' for this to specify
CONFIG for underlying command (running in container). The --opt2,
however, can't be used to specify '--opt2 "-e VAR="some val""',
because resulting ${OPT2} variable is split based on spaces before
used as 'docker run' arguments.
Although specifying CONFIG='some value' via 'args' is treated by
pipes.quote() correctly, it requires additional parsing on the
script side.

[praiskup]

@hhorak fyi

[rhatdan]

Can't we just fix the --opt1 parsing to handle this? I really don't want to add additional options, that do the same thing.

[praiskup]

On Friday 11 of September 2015 05:30:07 Daniel J Walsh wrote:

Can't we just fix the --opt1 parsing to handle this?

Probably yes, but I'm afraid we need to change semantics and do some
additional parsing/escaping.

I really don't want to add additional options, that do the same thing.

I understand your concerns about not adding additional options, but this
is not entirely the same thing because it allows us to enrich the
environment of command run from LABEL; OPT1/2 OTOH is designed to directly
put something onto docker's command (or any other command).

This sounds like natural complement to 'args' feature, especially with
approach like in sclorg/mysql-container/pull/92 -- its would be terribly easy to
add environment variables for the 'bash -s --' command.

[rhatdan]

Except this is hard coding docker commands into the atomic command. What is the expected behaviour of --env when I run a command other then docker in the label INSTALL?

I still believe this is the responsibility of your install scripts and not the atomic command.

[praiskup]

On Friday 11 of September 2015 05:59:27 Daniel J Walsh wrote:

Except this is hard coding docker commands into the atomic command.
What is the expected behaviour of --env when I run a command other then
docker in the label INSTALL?

You meant s/docker commands/docker options/ probably.

This proposal is not directly related to docker -- even though I've
mentioned in commit message (I should probably rephrase it), any command
run from 'INSTALL' will have environment "enhanced" by --env option, would
that be docker or any other (bash -s --, or other container-system tool).

I still believe this is the responsibility of your install scripts and
not the atomic command.

The problem is exactly here -- so far I believe the install script was run
from privileged container mounting the host's / directory into itself ..
.. then, to enhance script's environment you need to use --optX by
"hardcoding" for docker-only (--opt2="-e SOMEVAR=SOMEVAL").

Except that from the quick look at image/container you are unable to say
where exactly the $OPTX will be put (no semantics, everybody might
reimplement INSTALL content) you are right it would be fine; but we would
need to allow passing "shell special characters" and spaces withing
SOMEVAL in --opt2.

Other than that, with this PR we are able do something like this:

Generate variable or reuse already existing variable in env

export SOME_CONFIG="do sth..; and sth else"
atomic install -e SOME_CONFIG IMAGE

[praiskup]

Huh, @github, why the message above is formatted like that ^^.
Please click on '...' symbol to see whole response (as 'edit & preview'
formats it correctly, dunno how to fix that).

[rhatdan]

@aweiteka @vpavlin doesn't this belong to Nulecule?

[hhorak]

@aweiteka @vpavlin doesn't this belong to Nulecule?

Is it expected that as soon as we have Nulecule ready on Atomic host, then the atomic run ... will only be supposed to be used for system daemons (like rsyslog)?

[vpavlin]

Not really, Atomic App/Nulecule is already able to do this via parametrization of artifacts. I am also bothered by not being able to pass environment variable through atomic command. So, from my point of view, I'd like to see this added to atomic command, though I wouldn't probably require new argument like --env but would simply go with env variables set for the process as suggested in

sudo CONFIG="some value" atomic install

[rhatdan]

How would we know which environment variables you want to leak? We don't want to leak the entire environment into the container.

If I had

label INSTALL "docker run ${IMAGE} /usr/bin/install.sh"

What would you expect to be executed?

CONFIG="some value"  docker run <image> /usr/bin/install.sh

[vpavlin]

I'd more expect something like

LABEL RUN docker run -e USER=$DB_USER $IMAGE

atomic finds all env variables in RUN label and then passes them to the process

DB_USER=somebody atomic run mydb
docker run -e USER=somebody mydb

[praiskup]

On Friday 11 of September 2015 07:45:26 Vaclav Pavlin wrote:

I'd more expect something like

LABEL RUN docker run -e USER=$DB_USER $IMAGE

atomic finds all env variables in RUN label and then passes them to the
process

DB_USER=somebody atomic run mydb
docker run -e USER=somebody mydb

This is actually not happening within atomic itself, but it is done in
subsequent shell .. So you really do 'docker run -e USER=$DB_USER' ..
which has the same consequences as before (because $DB_USER could contain
spaces), and shell itself does the variable substitution from cmd_env.

But yes, if we can leak whole environment into the INSTALL command from
the security POV, it would be fine for me, too.

[vpavlin]

I don't follow. If I want to be sure I deal with white spaces properly isn't it enough to specify label as

LABLE RUN docker run -e USER="$DB_USER" $IMAGE

and the atomic run then as

DB_USER="somebody else" atomic run mydb

I haven't tried it, but I'd expect the whole string somebody else would then get to the docker run command if the env passing works as I described.

[praiskup]

On Friday 11 of September 2015 07:54:51 Vaclav Pavlin wrote:

I don't follow. If I want to be sure I deal with white spaces properly
isn't it enough to specify label as

LABLE RUN docker run -e USER="$DB_USER" $IMAGE

+1, just wanted to underline the fact that it is not done by atomic
itself (it is important to know while you are designing the commands) but,
also, it seems it is enough to use (as far as you do not need change the
variable name):

LABEL RUN docker run -e DB_USER $IMAGE

and the atomic run then as

DB_USER="somebody else" atomic run mydb

I haven't tried it, but I'd expect the whole string somebody else
would then get to the docker run command if the env passing works as I
described.

Correct.

[rhatdan]

How about you try this patch, is this what you want?

diff --git a/Atomic/atomic.py b/Atomic/atomic.py
index 6726c0b..a38301b 100644
--- a/Atomic/atomic.py
+++ b/Atomic/atomic.py
@@ -546,11 +546,12 @@ class Atomic(object):

     @property
     def cmd_env(self):
-        env = {'NAME': self.name,
-               'IMAGE': self.image,
-               'CONFDIR': "/etc/%s" % self.name,
-               'LOGDIR': "/var/log/%s" % self.name,
-               'DATADIR': "/var/lib/%s" % self.name}
+        env = dict(os.environ)
+        env.update({'NAME': self.name,
+                'IMAGE': self.image,
+                'CONFDIR': "/etc/%s" % self.name,
+                'LOGDIR': "/var/log/%s" % self.name,
+                'DATADIR': "/var/lib/%s" % self.name})

         if hasattr(self.args, 'opt1') and self.args.opt1:
             env['OPT1'] = self.args.opt1

[vpavlin]

I like this as it make the things more transparent (at least for me). For example in atomic app we call pwd in RUN label to get current directory. We could use $PWD with this patch (which I like more than calling a binary to get such information)

[praiskup]

On Friday 11 of September 2015 08:11:01 Daniel J Walsh wrote:

How about you try this patch, is this what you want?

Yes, +1, this is OK.

[rhatdan]

Since passing the host environment got merged, closing this one.