Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Preliminary support for @SUMMONDOCKERARGS #186

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ COPY go.mod go.sum ./

RUN apk add --no-cache bash \
build-base \
docker-cli \
git && \
go mod download && \
go get -u github.com/jstemmer/go-junit-report && \
Expand Down
3 changes: 2 additions & 1 deletion Dockerfile.acceptance
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ RUN apk add --no-cache bash \
git \
libffi-dev \
ruby-bundler \
ruby-dev
ruby-dev \
docker-cli
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be moved up to retain alphabetical listing?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done


# Install summon prerequisites
WORKDIR /summon
Expand Down
72 changes: 69 additions & 3 deletions docs/_includes/docker.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,74 @@ Since Summon has pluggable providers, you aren't locked into any one solution fo
managing your secrets.

Summon makes it easy to inject secrets as environment variables into your Docker
containers by taking advantage of Docker's `--env-file` argument. This is done
on-demand by using the variable `@SUMMONENVFILE` in the arguments of the process
containers by taking advantage of Docker's CLI arguments (`--env-file` or, `--env` and
`--volume`. There are two options available. It's possible to mix and match as you see fit.

## Docker --env and --volume arguments

This is done on-demand by using the variable `@SUMMONDOCKERARGS` in the arguments of the
process you are running with Summon. This variable is replaced by combinations of the
Docker arguments `--env` and `--volume` such that the secrets injected by summon are
passed into the Docker container. The `--volume` arguments allow memory-mapped temporary
files from variables with the `!file` tag to be resolvable inside the container.

**NOTE:** Using the `!file` tag with `@SUMMONDOCKERARGS` assumes that the Docker CLI is
being run on the host that is used to create volume mounts to the container. For when
this is not the case simply avoid using the `!file` tag, but be mindful that in that case
you lose the benefits of memory-mapped temporary files.

```bash
$ summon -p keyring.py -D env=dev docker run @SUMMONDOCKERARGS deployer
Checking credentials
Deploying application
```

### @SUMMONDOCKERARGS Example

The example below demonstrates the use of @SUMMONDOCKERARGS. For the sake of brevity
we use an inline `secrets.yml` and the `/bin/echo` provider. Some points to note:

1. `summon` is invoking `docker` as the child process.
2. `@SUMMONDOCKERARGS` is replaced with a combination of `--env` and `--volume` arguments.
3. Variable `D` uses the `!file` tag and therefore is the only one that
results in a `--volume` argument. The path to this variable inside the container
is as it is on the host.
Copy link
Contributor

@diverdane diverdane Jan 13, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm assuming that a temp file is being created for these !file tagged variables. Will the location of the temp file on the host be host-OS dependent, or are we depending this to always be in /tmp? If it's OS dependent, then the path inside the container will be host-OS dependent, and therefore not 100% predictable.
Maybe we should recommend setting TMPDIR to some path on the host so that we can rely on this path inside the container?
Or maybe there's some Summon config, or something we can add to secrets.yml to indicate the destination location in the container?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. This might get tricky if for host to container you're going from Windows to Linux or vice-versa. I'm wondering now if I should just altogether disregard the case for files since it adds complication. Having --env would already be a massive improvement.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we can add some new secrets.yml syntax to allow users to specify a
destination path for files that Summon creates. This could be similar to what is being proposed in this Issue: #190.

The syntax proposed in Issue #190 is something like this:

summon --yaml 'FOO: !file=<path> contents'
summon --yaml 'FOO: !file=<path> $env/aws/ec2/private_key'

For Issue #190, the <path> would specify a fixed, local location for Summon to create a file. (I don't think this syntax exists and/or is used for some other purpose.)

For @SUMMONDOCKERARGS, the <path> could potentially specify the location of the file as volume mounted in the container. So the resulting volume option for Docker could logically be:

    --volume <local-temp-file-path>:<path>

I think this would be easy for users to understand.

Maybe this can be added as a followup... along with changes to the secretsyml to add this syntax?

WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the idea of allowing the user to specify the location that Summon creates files. Even though not every platform supports /dev/shm, the benefit of summon creating files is that it is also responsible for cleaning them up... as much as possible we should retain this for the users in the new features we add.

For now I got rid of the implicit --volume arguments. We'll let the user do that themselves. Some things to note

  1. Summon does have a way to configure the tmp directory but it's difficult for the user to predict which one is being used. See https://github.com/cyberark/summon/blob/master/internal/command/temp_factory.go#L29. We try /dev/shm then os home directory then os tmp directory. It's not great!
  2. I suppose we should just give the user a way to more explicitly specify this directory, then they can volume mount themselves. However there'd still be an issue of going from say Windows to Linux Containers, and vice versa. I suppose we'll need to look into this more.


```bash
secretsyml='
A: |-
A_value with
multiple lines
B: B_value
C: !var C_value
D: !var:file D_value
'

# The substitution of @SUMMONDOCKERARGS the docker run command below results in
# something of the form:
#
# docker run --rm \
# --env A --env B --env C --env D \
# --volume /path/to/D:/path/to/D
# alpine ...
#
# The output from the command is shown below the command.

summon --provider /bin/echo --yaml "${secretsyml}" \
docker run --rm @SUMMONDOCKERARGS alpine sh -c '
printenv A;
printenv B;
printenv C;
cat $(printenv D);
'
# A_value with
# multiple lines
# B_value
# C_value
# D_value
```
## Docker --env-file argument
This is done on-demand by using the variable `@SUMMONENVFILE` in the arguments of the process
you are running with Summon. This variable points to a memory-mapped file containing
the variables and values from secrets.yml in VAR=VAL format.

Expand All @@ -27,7 +93,7 @@ Checking credentials
Deploying application
```

## Example
### @SUMMONENVFILE Example

Let's say we have a deploy script that needs to access our application servers on
AWS and pull the latest version of our code. It should record the outcome of the
Expand Down
3 changes: 3 additions & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@ module github.com/cyberark/summon

require (
github.com/codegangsta/cli v1.20.0
github.com/docker/docker v20.10.2+incompatible
github.com/docker/go-connections v0.4.0 // indirect
github.com/docker/go-units v0.4.0 // indirect
github.com/gopherjs/gopherjs v0.0.0-20181103185306-d547d1d9531e // indirect
github.com/jtolds/gls v4.20.0+incompatible // indirect
github.com/kr/pretty v0.1.0 // indirect
Expand Down
7 changes: 7 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,13 @@ github.com/codegangsta/cli v1.20.0 h1:iX1FXEgwzd5+XN6wk5cVHOGQj6Q3Dcp20lUeS4lHNT
github.com/codegangsta/cli v1.20.0/go.mod h1:/qJNoX69yVSKu5o4jLyXAENLRyk1uhi7zkbQ3slBdOA=
github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8=
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/docker/docker v1.13.1 h1:IkZjBSIc8hBjLpqeAbeE5mca5mNgeatLHBy3GO78BWo=
github.com/docker/docker v20.10.2+incompatible h1:vFgEHPqWBTp4pTjdLwjAA4bSo3gvIGOYwuJTlEjVBCw=
github.com/docker/docker v20.10.2+incompatible/go.mod h1:eEKB0N0r5NX/I1kEveEz05bcu8tLC/8azJZsviup8Sk=
github.com/docker/go-connections v0.4.0 h1:El9xVISelRB7BuFusrZozjnkIM5YnzCViNKohAFqRJQ=
github.com/docker/go-connections v0.4.0/go.mod h1:Gbd7IOopHjR8Iph03tsViu4nIes5XhDvyHbTtUxmeec=
github.com/docker/go-units v0.4.0 h1:3uh0PgVws3nIA0Q+MwDC8yjEPf9zjRfZZWXZYDct3Tw=
github.com/docker/go-units v0.4.0/go.mod h1:fgPhTUdO+D/Jk86RDLlptpiXQzgHJF7gydDDbaIK4Dk=
github.com/gopherjs/gopherjs v0.0.0-20181103185306-d547d1d9531e h1:JKmoR8x90Iww1ks85zJ1lfDGgIiMDuIptTOhJq+zKyg=
github.com/gopherjs/gopherjs v0.0.0-20181103185306-d547d1d9531e/go.mod h1:wJfORRmW1u3UXTncJ5qlYoELFm8eSnnEO6hX4iZ3EWY=
github.com/jtolds/gls v4.20.0+incompatible h1:xdiiI2gbIgH/gLH7ADydsJ1uDOEzR8yvV7C0MuV77Wo=
Expand Down
68 changes: 61 additions & 7 deletions internal/command/action.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,21 +3,27 @@ package command
import (
"bytes"
"fmt"
"io"
"os"
"os/exec"
"path/filepath"
"sort"
"strings"
"sync"
"syscall"

"github.com/codegangsta/cli"

prov "github.com/cyberark/summon/provider"
"github.com/cyberark/summon/secretsyml"
)

// ActionConfig is an object that holds all the info needed to run
// a Summon instance
type ActionConfig struct {
StdIn io.Reader
StdOut io.Writer
StdErr io.Writer
Args []string
Provider string
Filepath string
Expand All @@ -30,8 +36,9 @@ type ActionConfig struct {
ShowProviderVersions bool
}

const ENV_FILE_MAGIC = "@SUMMONENVFILE"
const SUMMON_ENV_KEY_NAME = "SUMMON_ENV"
const EnvFileMagic = "@SUMMONENVFILE"
const DockerArgsMagic = "@SUMMONDOCKERARGS"
const SummonEnvKeyName = "SUMMON_ENV"

// Action is the runner for the main program logic
var Action = func(c *cli.Context) {
Expand Down Expand Up @@ -122,6 +129,9 @@ func runAction(ac *ActionConfig) error {
results := make(chan Result, len(secrets))
var wg sync.WaitGroup

var dockerArgs []string
var dockerArgsMutex sync.Mutex

for key, spec := range secrets {
wg.Add(1)
go func(key string, spec secretsyml.SecretSpec) {
Expand All @@ -144,6 +154,15 @@ func runAction(ac *ActionConfig) error {
}

k, v := formatForEnv(key, value, spec, &tempFactory)

// Generate @SUMMONDOCKERARGS
dockerArgsMutex.Lock()
defer dockerArgsMutex.Unlock()
if spec.IsFile() {
dockerArgs = append(dockerArgs, "--volume", v+":"+v)
}
dockerArgs = append(dockerArgs, "--env", k)

results <- Result{k, v, nil}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we be zeroing results?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably a good idea, I don't know if Summon does any zeroization at all. But I think that piece is out of scope of this PR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't results being initialized as an empty channel (line 129 above)? So I think we shouldn't need to zero anything, unless I'm missing something?

wg.Done()
}(key, spec)
Expand Down Expand Up @@ -171,17 +190,48 @@ EnvLoop:

// Append environment variable if one is specified
if ac.Environment != "" {
env[SUMMON_ENV_KEY_NAME] = ac.Environment
env[SummonEnvKeyName] = ac.Environment
}

setupEnvFile(ac.Args, env, &tempFactory)

// Setup Docker args
var argsWithDockerArgs []string
for _, arg := range ac.Args {
// Replace entire argument
if arg == DockerArgsMagic {
// Replace argument with slice of docker options
argsWithDockerArgs = append(argsWithDockerArgs, dockerArgs...)
continue
}

// Replace argument substring
idx := strings.Index(arg, DockerArgsMagic)
if idx >= 0 {
// Replace substring in argument with slice of docker options
argsWithDockerArgs = append(
argsWithDockerArgs,
strings.Replace(arg, DockerArgsMagic, strings.Join(dockerArgs, " "), -1),
)
continue
}

argsWithDockerArgs = append(argsWithDockerArgs, arg)
}
ac.Args = argsWithDockerArgs

var e []string
for k, v := range env {
e = append(e, fmt.Sprintf("%s=%s", k, v))
}

return runSubcommand(ac.Args, append(os.Environ(), e...))
return runSubcommand(
ac.Args,
append(os.Environ(), e...),
ac.StdIn,
ac.StdOut,
ac.StdErr,
)
}

// formatForEnv returns a string in %k=%v format, where %k=namespace of the secret and
Expand All @@ -200,6 +250,10 @@ func joinEnv(env map[string]string) string {
for k, v := range env {
envs = append(envs, fmt.Sprintf("%s=%s", k, v))
}

// Sort to ensure predictable results
sort.Strings(envs)
doodlesbykumbi marked this conversation as resolved.
Show resolved Hide resolved

return strings.Join(envs, "\n") + "\n"
}

Expand Down Expand Up @@ -240,20 +294,20 @@ func findInParentTree(secretsFile string, leafDir string) (string, error) {
}
}

// scans arguments for the magic string; if found,
// scans arguments for the envfile magic string; if found,
// creates a tempfile to which all the environment mappings are dumped
// and replaces the magic string with its path.
// Returns the path if so, returns an empty string otherwise.
func setupEnvFile(args []string, env map[string]string, tempFactory *TempFactory) string {
var envFile = ""

for i, arg := range args {
idx := strings.Index(arg, ENV_FILE_MAGIC)
idx := strings.Index(arg, EnvFileMagic)
if idx >= 0 {
if envFile == "" {
envFile = tempFactory.Push(joinEnv(env))
}
args[i] = strings.Replace(arg, ENV_FILE_MAGIC, envFile, -1)
args[i] = strings.Replace(arg, EnvFileMagic, envFile, -1)
}
}

Expand Down
Loading