screwdriver-cd / screwdriver

An open source build platform designed for continuous delivery.
http://screwdriver.cd
Other
1.02k stars 169 forks source link

Launcher passes CWD and Environment between Steps #393

Closed stjohnjohnson closed 7 years ago

stjohnjohnson commented 7 years ago

Currently we don't pass environment variables or cwd between steps because we execute all commands separately in sub-shells, this causes fun issues like:

$ eval `ssh-agent -s`
$ ssh-add -L
Error connecting to agent: No such file or directory

This happens because SSH_AUTH_SOCK is set in the first step, but not passed into the second one.


The proposed solution to this is to follow something similar to Wercker:

d2lam commented 7 years ago

This makes sense to me.

(how to handle quoting here?) What do you mean by this? Shouldn't it work similarly to currently: https://github.com/screwdriver-cd/launcher/blob/master/launch.go#L237-L246

So for each step we need to write to a temporary file, would that add too much overhead if someone has a lot of steps?

stjohnjohnson commented 7 years ago

@d2lam previously we could just spawn the command with environment set. In this case we're running a shell and executing export in the shell. This will allow users to do more "creative" environment setting, but at the risk of shell exploiting.

Maybe you're right, we should not support "creative" environment setting via the environment feature. Instead they need to use a step that calls export

jer commented 7 years ago

I agree on not making it easy to do weird environment things. Not for security and shell exploits or anything, it just makes for a confusing interface.

This seems like a reasonable plan. We just need to make sure the random id doesn't end up in logs as well.

FenrirUnbound commented 7 years ago

Task map

minzcmu commented 7 years ago

Summary

Instead of Starting /bin/sh with control of STDOUT, STDERR, STDIN, our current implementation for that part is as follows:

  1. Start /bin/sh in a pseudo-terminal using https://github.com/kr/pty
  2. Write and execute commands in the pty:
    • Run setup commands (e.g. set -e)
    • Source step script for each step
  3. Pipe the output from pty to emitter directly

Progress

d2lam commented 7 years ago

Fixed the unit test.

What we tried:

What worked

d2lam commented 7 years ago

Using export

screen shot 2017-01-26 at 7 51 40 pm

Using another script

screen shot 2017-01-26 at 7 48 47 pm
d2lam commented 7 years ago

Done!

Blog post: http://blog.screwdriver.cd/post/156911122972/launcher-passes-cwd-and-environment-variables Documentation: https://docs.screwdriver.cd/user-guide/configuration/environment