From 71e4da11c2b83fb669ac14946e7d63b6b4d81a70 Mon Sep 17 00:00:00 2001 From: Kumbirai Tanekha Date: Wed, 30 Dec 2020 15:14:48 +0000 Subject: [PATCH 1/3] Preliminary support for @SUMMONDOCKERARGS --- Dockerfile | 1 + Dockerfile.acceptance | 3 +- docs/_includes/docker.md | 64 +++++++++++++++- go.mod | 3 + go.sum | 7 ++ internal/command/action.go | 67 ++++++++++++++++- internal/command/action_test.go | 126 +++++++++++++++++++++++++++++++- internal/command/subcommand.go | 26 ++++++- 8 files changed, 285 insertions(+), 12 deletions(-) diff --git a/Dockerfile b/Dockerfile index 85757055..714b2579 100644 --- a/Dockerfile +++ b/Dockerfile @@ -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 && \ diff --git a/Dockerfile.acceptance b/Dockerfile.acceptance index fb38338f..123f7b32 100644 --- a/Dockerfile.acceptance +++ b/Dockerfile.acceptance @@ -5,7 +5,8 @@ RUN apk add --no-cache bash \ git \ libffi-dev \ ruby-bundler \ - ruby-dev + ruby-dev \ + docker-cli # Install summon prerequisites WORKDIR /summon diff --git a/docs/_includes/docker.md b/docs/_includes/docker.md index b9645dd1..beaf46c1 100755 --- a/docs/_includes/docker.md +++ b/docs/_includes/docker.md @@ -16,8 +16,66 @@ 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. + +## --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 +``` + +### Example +The example below demonstrates the use @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. + +```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 +``` +## --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. @@ -27,7 +85,7 @@ Checking credentials Deploying application ``` -## Example +### 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 diff --git a/go.mod b/go.mod index dfb8894d..aa6cc7d9 100644 --- a/go.mod +++ b/go.mod @@ -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 diff --git a/go.sum b/go.sum index 0564e51e..24612961 100644 --- a/go.sum +++ b/go.sum @@ -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= diff --git a/internal/command/action.go b/internal/command/action.go index be9e0069..262ced8f 100644 --- a/internal/command/action.go +++ b/internal/command/action.go @@ -3,14 +3,17 @@ 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" ) @@ -18,6 +21,9 @@ import ( // 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 @@ -31,6 +37,7 @@ type ActionConfig struct { } const ENV_FILE_MAGIC = "@SUMMONENVFILE" +const DOCKER_ARGS_MAGIC = "@SUMMONDOCKERARGS" const SUMMON_ENV_KEY_NAME = "SUMMON_ENV" // Action is the runner for the main program logic @@ -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) { @@ -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} wg.Done() }(key, spec) @@ -176,12 +195,52 @@ EnvLoop: setupEnvFile(ac.Args, env, &tempFactory) + // Setup Docker args + var argsWithDockerArgs []string + for _, arg := range ac.Args { + if arg == DOCKER_ARGS_MAGIC { + // Replace argument with slice of docker options + argsWithDockerArgs = append(argsWithDockerArgs, dockerArgs...) + continue + } + + // TODO: we need to decide which of these if we want to support (2) + // 1. summon [...] docker run @SUMMONDOCKERARGS [...], replace only entire top-level arg. The + // top-level arg is replaced by is replaced by N>0 args equating to @SUMMONDOCKERARGS. + // 2. summon ... sh -c "docker run @SUMMONDOCKERARGS [...]", also replace substrings + // inside args but the replacement is as a single string. + // + // The code below should support (2). There'll be some ambiguity though... + // e.g. summon ... echo "@SUMMONDOCKERARGS" will fall under both (1) and (2), though (1) + // takes precedence. I'm not sure if the behaviors of (1) and (2) are equivalent + // when there's such ambiguity. + // + //idx := strings.Index(arg, DOCKER_ARGS_MAGIC) + //if idx >= 0 { + // // Replace argument with slice of docker options + // argsWithDockerArgs = append( + // argsWithDockerArgs, + // strings.Replace(arg, DOCKER_ARGS_MAGIC, 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 @@ -200,6 +259,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) + return strings.Join(envs, "\n") + "\n" } @@ -240,7 +303,7 @@ 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. diff --git a/internal/command/action_test.go b/internal/command/action_test.go index 5ec4986c..e6331acc 100644 --- a/internal/command/action_test.go +++ b/internal/command/action_test.go @@ -1,19 +1,26 @@ package command import ( + "encoding/json" "errors" "fmt" "io/ioutil" + "net/http" + "net/http/httptest" "os" "os/exec" "path/filepath" + "regexp" "strconv" + "strings" "testing" "time" - "github.com/cyberark/summon/secretsyml" + "github.com/docker/docker/api/types/container" . "github.com/smartystreets/goconvey/convey" _ "golang.org/x/net/context" + + "github.com/cyberark/summon/secretsyml" ) func TestConvertSubsToMap(t *testing.T) { @@ -82,7 +89,7 @@ func TestFormatForEnvString(t *testing.T) { func TestJoinEnv(t *testing.T) { Convey("adds a trailing newline", t, func() { result := joinEnv(map[string]string{"foo": "bar", "baz": "qux"}) - So(result, ShouldEqual, "foo=bar\nbaz=qux\n") + So(result, ShouldEqual, "baz=qux\nfoo=bar\n") }) } @@ -120,6 +127,121 @@ func TestRunAction(t *testing.T) { So(string(content), ShouldEqual, expectedValue) }) + + Convey("Docker options correctly injected", t, func() { + // This is a test case for @SUMMONDOCKERARGS. It exercises Docker CLI pointed to a mock + // server. It asserts on the request payload received on the container creation + // endpoint, the volume mounts and environment variables injected by summon are + // expected to be present. + + expected := map[string]string{ + "A": "A's multiple line\nvalue", + "B": "B_value", + "C": "C_value", + "D": "D_value", + } + const inlineSecretsYml = ` +A: |- + A's multiple line + value +B: !var B_value +C: !file C_value +D: !var:file D_value +` + + volumeBinds := map[string]struct{ + ContainerPath string + FileContents string + }{} + envvars := map[string]string{} + + // Mock server for handling API calls by `docker run` + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var payload struct { + *container.Config + HostConfig *container.HostConfig + } + + if !regexp.MustCompile("/.*/containers/create").MatchString(r.URL.Path) { + // Mock response to all the other endpoints called as part of `docker run` + w.WriteHeader(200) + fmt.Fprintln(w, "{}") + return + } + payloadBytes, err := ioutil.ReadAll(r.Body) + + if err != nil { + t.Errorf("failure reading payload from docker cli: %s", err) + return + } + err = json.Unmarshal(payloadBytes, &payload) + if err != nil { + t.Errorf("payload from docker cli could not be parsed: %s", err) + return + } + + for _, env := range payload.Env { + nameAndValue := strings.SplitN(env, "=", 2) + name := nameAndValue[0] + value := nameAndValue[1] + + envvars[name] = value + } + + for _, volumeBind := range payload.HostConfig.Binds { + fromAndTo := strings.SplitN(volumeBind, ":", 2) + from := fromAndTo[0] + to := fromAndTo[1] + + fileContents, _ := ioutil.ReadFile(from) + volumeBinds[from] = struct { + ContainerPath string + FileContents string + }{ + ContainerPath: to, + FileContents: string(fileContents), + } + } + + w.WriteHeader(201) + + // Mock response to container create endpoint + fmt.Fprintln(w, `{"Id": "e90e34656806", "Warnings": []}`) + })) + defer ts.Close() + + + // Run docker wrapped around summon and leveraging @SUMMONDOCKERARGS + var dockerCommand = []string{ + "docker", + "-H", strings.Replace(ts.URL, "http://", "tcp://", 1), + "run", + "--rm", "-d", "@SUMMONDOCKERARGS", + "alpine", + } + err := runAction(&ActionConfig{ + Provider: "/bin/echo", // Use /bin/echo provider for brevity + Args: dockerCommand, + YamlInline: inlineSecretsYml, + }) + + // Make assertions + code, err := returnStatusOfError(err) + So(err, ShouldBeNil) + So(code, ShouldEqual, 0) + + // The volume mount binds are expected to take the form + // 'host_path:container_path', where host_path is equal to container_path + for from, volumeBind := range volumeBinds { + So(from, ShouldEqual, volumeBind.ContainerPath) + } + + // Ensure envvars and volumemounts passed to Docker match expectations + So(envvars["A"], ShouldEqual, expected["A"]) + So(envvars["B"], ShouldEqual, expected["B"]) + So(volumeBinds[envvars["C"]].FileContents, ShouldEqual, expected["C"]) + So(volumeBinds[envvars["D"]].FileContents, ShouldEqual, expected["D"]) + }) } func TestDefaultVariableResolution(t *testing.T) { diff --git a/internal/command/subcommand.go b/internal/command/subcommand.go index c16b0562..2428145d 100644 --- a/internal/command/subcommand.go +++ b/internal/command/subcommand.go @@ -1,6 +1,7 @@ package command import ( + "io" "os" "os/exec" "os/signal" @@ -11,16 +12,33 @@ import ( // of an environment populated with secret values. Since we have to // clean up our temp directories, we remain resident and shuffle // signals around to the chld and back -func runSubcommand(command []string, env []string) error { +func runSubcommand( + command []string, + env []string, + Stdin io.Reader, + Stdout io.Writer, + Stderr io.Writer, +) error { binary, lookupErr := exec.LookPath(command[0]) if lookupErr != nil { return lookupErr } runner := exec.Command(binary, command[1:]...) - runner.Stdin = os.Stdin - runner.Stdout = os.Stdout - runner.Stderr = os.Stderr + + if Stdin == nil { + Stdin = os.Stdin + } + if Stdout == nil { + Stdout = os.Stdout + } + if Stderr == nil { + Stderr = os.Stderr + } + + runner.Stdin = Stdin + runner.Stdout = Stdout + runner.Stderr = Stderr runner.Env = env signalChannel := make(chan os.Signal, 1) From ba7a5c22f22430680f4a97dfb9144b42dda18b87 Mon Sep 17 00:00:00 2001 From: Kumbirai Tanekha Date: Tue, 19 Jan 2021 16:20:22 +0000 Subject: [PATCH 2/3] Address PR feedback --- docs/_includes/docker.md | 34 +++-- internal/command/action.go | 45 +++---- internal/command/action_test.go | 211 ++++++++++++++++++-------------- 3 files changed, 155 insertions(+), 135 deletions(-) diff --git a/docs/_includes/docker.md b/docs/_includes/docker.md index beaf46c1..2b222637 100755 --- a/docs/_includes/docker.md +++ b/docs/_includes/docker.md @@ -16,13 +16,21 @@ 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 CLI arguments (`--env-file` or, `--env` and `--volume`. There are two options available. It's possible to mix and match as you see fit. +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. -## --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. +## Docker --env and --volume arguments -**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. +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 @@ -30,13 +38,13 @@ Checking credentials Deploying application ``` -### Example -The example below demonstrates the use @SUMMONDOCKERARGS. For the sake of brevity +### @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. + +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. @@ -74,7 +82,7 @@ cat $(printenv D); # C_value # D_value ``` -## --env-file argument +## 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. @@ -85,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 diff --git a/internal/command/action.go b/internal/command/action.go index 262ced8f..4e76e664 100644 --- a/internal/command/action.go +++ b/internal/command/action.go @@ -36,9 +36,9 @@ type ActionConfig struct { ShowProviderVersions bool } -const ENV_FILE_MAGIC = "@SUMMONENVFILE" -const DOCKER_ARGS_MAGIC = "@SUMMONDOCKERARGS" -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) { @@ -190,7 +190,7 @@ 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) @@ -198,32 +198,23 @@ EnvLoop: // Setup Docker args var argsWithDockerArgs []string for _, arg := range ac.Args { - if arg == DOCKER_ARGS_MAGIC { + // Replace entire argument + if arg == DockerArgsMagic { // Replace argument with slice of docker options argsWithDockerArgs = append(argsWithDockerArgs, dockerArgs...) continue } - // TODO: we need to decide which of these if we want to support (2) - // 1. summon [...] docker run @SUMMONDOCKERARGS [...], replace only entire top-level arg. The - // top-level arg is replaced by is replaced by N>0 args equating to @SUMMONDOCKERARGS. - // 2. summon ... sh -c "docker run @SUMMONDOCKERARGS [...]", also replace substrings - // inside args but the replacement is as a single string. - // - // The code below should support (2). There'll be some ambiguity though... - // e.g. summon ... echo "@SUMMONDOCKERARGS" will fall under both (1) and (2), though (1) - // takes precedence. I'm not sure if the behaviors of (1) and (2) are equivalent - // when there's such ambiguity. - // - //idx := strings.Index(arg, DOCKER_ARGS_MAGIC) - //if idx >= 0 { - // // Replace argument with slice of docker options - // argsWithDockerArgs = append( - // argsWithDockerArgs, - // strings.Replace(arg, DOCKER_ARGS_MAGIC, strings.Join(dockerArgs, " "), -1), - // ) - // 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) } @@ -311,12 +302,12 @@ func setupEnvFile(args []string, env map[string]string, tempFactory *TempFactory 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) } } diff --git a/internal/command/action_test.go b/internal/command/action_test.go index e6331acc..81465f5a 100644 --- a/internal/command/action_test.go +++ b/internal/command/action_test.go @@ -1,6 +1,7 @@ package command import ( + "bytes" "encoding/json" "errors" "fmt" @@ -128,19 +129,45 @@ func TestRunAction(t *testing.T) { So(string(content), ShouldEqual, expectedValue) }) - Convey("Docker options correctly injected", t, func() { - // This is a test case for @SUMMONDOCKERARGS. It exercises Docker CLI pointed to a mock - // server. It asserts on the request payload received on the container creation - // endpoint, the volume mounts and environment variables injected by summon are - // expected to be present. + Convey("Docker options correctly injected for top-level command", t, func() { + RunDockerArgsTestCase(t, func (dockerDaemonSocket string) []string { + return []string{ + "docker", + "-H", dockerDaemonSocket, + "run", + "--rm", "-d", "@SUMMONDOCKERARGS", + "alpine", + } + }) + }) - expected := map[string]string{ - "A": "A's multiple line\nvalue", - "B": "B_value", - "C": "C_value", - "D": "D_value", - } - const inlineSecretsYml = ` + Convey("Docker options correctly injected for nested command", t, func() { + RunDockerArgsTestCase(t, func (dockerDaemonSocket string) []string { + return []string{ + "sh", + "-c", + "docker -H "+dockerDaemonSocket+" run --rm -d @SUMMONDOCKERARGS alpine", + } + }) + }) +} + +func RunDockerArgsTestCase( + t *testing.T, + dockerCommandGen func(dockerDaemonSocket string, +) []string) { + // This is a test case for @SUMMONDOCKERARGS. It exercises Docker CLI pointed to a mock + // server. It asserts on the request payload received on the container creation + // endpoint, the volume mounts and environment variables injected by summon are + // expected to be present. + + expected := map[string]string{ + "A": "A's multiple line\nvalue", + "B": "B_value", + "C": "C_value", + "D": "D_value", + } + const inlineSecretsYml = ` A: |- A's multiple line value @@ -149,99 +176,93 @@ C: !file C_value D: !var:file D_value ` - volumeBinds := map[string]struct{ - ContainerPath string - FileContents string - }{} - envvars := map[string]string{} - - // Mock server for handling API calls by `docker run` - ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - var payload struct { - *container.Config - HostConfig *container.HostConfig - } + volumeBinds := map[string]struct{ + ContainerPath string + FileContents string + }{} + envvars := map[string]string{} + + // Mock server for handling API calls by `docker run` + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var payload struct { + *container.Config + HostConfig *container.HostConfig + } - if !regexp.MustCompile("/.*/containers/create").MatchString(r.URL.Path) { - // Mock response to all the other endpoints called as part of `docker run` - w.WriteHeader(200) - fmt.Fprintln(w, "{}") - return - } - payloadBytes, err := ioutil.ReadAll(r.Body) + if !regexp.MustCompile("/.*/containers/create").MatchString(r.URL.Path) { + // Mock response to all the other endpoints called as part of `docker run` + w.WriteHeader(200) + fmt.Fprintln(w, "{}") + return + } + payloadBytes, err := ioutil.ReadAll(r.Body) - if err != nil { - t.Errorf("failure reading payload from docker cli: %s", err) - return - } - err = json.Unmarshal(payloadBytes, &payload) - if err != nil { - t.Errorf("payload from docker cli could not be parsed: %s", err) - return - } + if err != nil { + t.Errorf("failure reading payload from docker cli: %s", err) + return + } + err = json.Unmarshal(payloadBytes, &payload) + if err != nil { + t.Errorf("payload from docker cli could not be parsed: %s", err) + return + } - for _, env := range payload.Env { - nameAndValue := strings.SplitN(env, "=", 2) - name := nameAndValue[0] - value := nameAndValue[1] + for _, env := range payload.Env { + nameAndValue := strings.SplitN(env, "=", 2) + name := nameAndValue[0] + value := nameAndValue[1] - envvars[name] = value - } + envvars[name] = value + } - for _, volumeBind := range payload.HostConfig.Binds { - fromAndTo := strings.SplitN(volumeBind, ":", 2) - from := fromAndTo[0] - to := fromAndTo[1] - - fileContents, _ := ioutil.ReadFile(from) - volumeBinds[from] = struct { - ContainerPath string - FileContents string - }{ - ContainerPath: to, - FileContents: string(fileContents), - } + for _, volumeBind := range payload.HostConfig.Binds { + fromAndTo := strings.SplitN(volumeBind, ":", 2) + from := fromAndTo[0] + to := fromAndTo[1] + + fileContents, _ := ioutil.ReadFile(from) + volumeBinds[from] = struct { + ContainerPath string + FileContents string + }{ + ContainerPath: to, + FileContents: string(fileContents), } - - w.WriteHeader(201) - - // Mock response to container create endpoint - fmt.Fprintln(w, `{"Id": "e90e34656806", "Warnings": []}`) - })) - defer ts.Close() - - - // Run docker wrapped around summon and leveraging @SUMMONDOCKERARGS - var dockerCommand = []string{ - "docker", - "-H", strings.Replace(ts.URL, "http://", "tcp://", 1), - "run", - "--rm", "-d", "@SUMMONDOCKERARGS", - "alpine", } - err := runAction(&ActionConfig{ - Provider: "/bin/echo", // Use /bin/echo provider for brevity - Args: dockerCommand, - YamlInline: inlineSecretsYml, - }) - // Make assertions - code, err := returnStatusOfError(err) - So(err, ShouldBeNil) - So(code, ShouldEqual, 0) + w.WriteHeader(201) + + // Mock response to container create endpoint + fmt.Fprintln(w, `{"Id": "e90e34656806", "Warnings": []}`) + })) + defer ts.Close() + + var stdBuf bytes.Buffer + // Run docker wrapped around summon and leveraging @SUMMONDOCKERARGS + err := runAction(&ActionConfig{ + StdErr: &stdBuf, + StdOut: &stdBuf, + Provider: "/bin/echo", // Use /bin/echo provider for brevity + Args: dockerCommandGen(strings.Replace(ts.URL, "http://", "tcp://", 1)), + YamlInline: inlineSecretsYml, + }) - // The volume mount binds are expected to take the form - // 'host_path:container_path', where host_path is equal to container_path - for from, volumeBind := range volumeBinds { - So(from, ShouldEqual, volumeBind.ContainerPath) - } + // Make assertions + code, err := returnStatusOfError(err) + So(err, ShouldBeNil) + So(code, ShouldEqual, 0) - // Ensure envvars and volumemounts passed to Docker match expectations - So(envvars["A"], ShouldEqual, expected["A"]) - So(envvars["B"], ShouldEqual, expected["B"]) - So(volumeBinds[envvars["C"]].FileContents, ShouldEqual, expected["C"]) - So(volumeBinds[envvars["D"]].FileContents, ShouldEqual, expected["D"]) - }) + // The volume mount binds are expected to take the form + // 'host_path:container_path', where host_path is equal to container_path + for from, volumeBind := range volumeBinds { + So(from, ShouldEqual, volumeBind.ContainerPath) + } + + // Ensure envvars and volumemounts passed to Docker match expectations + So(envvars["A"], ShouldEqual, expected["A"]) + So(envvars["B"], ShouldEqual, expected["B"]) + So(volumeBinds[envvars["C"]].FileContents, ShouldEqual, expected["C"]) + So(volumeBinds[envvars["D"]].FileContents, ShouldEqual, expected["D"]) } func TestDefaultVariableResolution(t *testing.T) { From 4126897b749614e6cee41d9a8c9c0866a2437656 Mon Sep 17 00:00:00 2001 From: Kumbirai Tanekha Date: Mon, 25 Jan 2021 12:54:09 +0000 Subject: [PATCH 3/3] Remove support for --volume arguments in SUMMONDOCKERARGS --- Dockerfile.acceptance | 4 +- docs/_includes/docker.md | 30 +++++--------- internal/command/action.go | 27 +++++++------ internal/command/action_test.go | 67 ++++++++++++++++++-------------- internal/command/temp_factory.go | 2 +- 5 files changed, 65 insertions(+), 65 deletions(-) diff --git a/Dockerfile.acceptance b/Dockerfile.acceptance index 123f7b32..8442f607 100644 --- a/Dockerfile.acceptance +++ b/Dockerfile.acceptance @@ -2,11 +2,11 @@ FROM golang:1.13-alpine RUN apk add --no-cache bash \ build-base \ + docker-cli \ git \ libffi-dev \ ruby-bundler \ - ruby-dev \ - docker-cli + ruby-dev # Install summon prerequisites WORKDIR /summon diff --git a/docs/_includes/docker.md b/docs/_includes/docker.md index 2b222637..d92ab391 100755 --- a/docs/_includes/docker.md +++ b/docs/_includes/docker.md @@ -16,21 +16,16 @@ 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 CLI arguments (`--env-file` or, `--env` and -`--volume`. There are two options available. It's possible to mix and match as you see fit. +containers by taking advantage of Docker's CLI arguments (`--env-file` or `--env`). There are two options available. It's possible to mix and match as you see fit. -## Docker --env and --volume arguments +## Docker --env 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. + process you are running with Summon. This variable is replaced by instances of the + Docker argument `--env` such that the secrets injected by summon are + passed into the Docker 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. +**NOTE:** The `!file` tag can only be used with `@SUMMONDOCKERARGS` if the temp file directory that summon uses is mounted into the container, otherwise the file path won't be resolvable. ```bash $ summon -p keyring.py -D env=dev docker run @SUMMONDOCKERARGS deployer @@ -43,11 +38,8 @@ Deploying application 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. +1. `summon` is invoking the `docker` CLI as a child process. +2. `@SUMMONDOCKERARGS` is replaced with instances of the Docker argument `--env`. ```bash secretsyml=' @@ -56,15 +48,13 @@ A: |- 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 +# --env A --env B --env C \ # alpine ... # # The output from the command is shown below the command. @@ -74,13 +64,11 @@ summon --provider /bin/echo --yaml "${secretsyml}" \ 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 diff --git a/internal/command/action.go b/internal/command/action.go index 4e76e664..28e379ce 100644 --- a/internal/command/action.go +++ b/internal/command/action.go @@ -27,6 +27,7 @@ type ActionConfig struct { Args []string Provider string Filepath string + TmpPath string YamlInline string Subs map[string]string Ignores []string @@ -116,7 +117,7 @@ func runAction(ac *ActionConfig) error { } env := make(map[string]string) - tempFactory := NewTempFactory("") + tempFactory := NewTempFactory(ac.TmpPath) defer tempFactory.Cleanup() type Result struct { @@ -129,9 +130,6 @@ 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) { @@ -155,14 +153,6 @@ 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} wg.Done() }(key, spec) @@ -195,6 +185,19 @@ EnvLoop: setupEnvFile(ac.Args, env, &tempFactory) + + var keys []string + for key := range secrets { + keys = append(keys, key) + } + sort.Strings(keys) + + var dockerArgs []string + // Generate @SUMMONDOCKERARGS + for _, key := range keys { + dockerArgs = append(dockerArgs, "--env", key) + } + // Setup Docker args var argsWithDockerArgs []string for _, arg := range ac.Args { diff --git a/internal/command/action_test.go b/internal/command/action_test.go index 81465f5a..14cff74b 100644 --- a/internal/command/action_test.go +++ b/internal/command/action_test.go @@ -129,7 +129,35 @@ func TestRunAction(t *testing.T) { So(string(content), ShouldEqual, expectedValue) }) - Convey("Docker options correctly injected for top-level command", t, func() { + Convey("@SUMMONDOCKERARGS and @SUMMONENVFILE", t, func() { + var stdBuf bytes.Buffer + // Run docker wrapped around summon and leveraging @SUMMONDOCKERARGS + err := runAction(&ActionConfig{ + StdErr: &stdBuf, + StdOut: &stdBuf, + Provider: "/bin/echo", // Use /bin/echo provider for brevity + Args: []string{ + "/bin/sh", + "-c", + ` +echo @SUMMONDOCKERARGS; +cat @SUMMONENVFILE; +`, + }, + YamlInline: ` +A: A_value +B: B_value +`, + }) + + // Make assertions + code, err := returnStatusOfError(err) + So(err, ShouldBeNil) + So(code, ShouldEqual, 0) + So(stdBuf.String(), ShouldEqual, "--env A --env B\nA=A_value\nB=B_value\n") + }) + + Convey("Docker args correctly injected for top-level command", t, func() { RunDockerArgsTestCase(t, func (dockerDaemonSocket string) []string { return []string{ "docker", @@ -141,7 +169,7 @@ func TestRunAction(t *testing.T) { }) }) - Convey("Docker options correctly injected for nested command", t, func() { + Convey("Docker args correctly injected for nested command", t, func() { RunDockerArgsTestCase(t, func (dockerDaemonSocket string) []string { return []string{ "sh", @@ -176,11 +204,8 @@ C: !file C_value D: !var:file D_value ` - volumeBinds := map[string]struct{ - ContainerPath string - FileContents string - }{} envvars := map[string]string{} + fileContents := map[string]string{} // Mock server for handling API calls by `docker run` ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -213,21 +238,11 @@ D: !var:file D_value value := nameAndValue[1] envvars[name] = value - } - for _, volumeBind := range payload.HostConfig.Binds { - fromAndTo := strings.SplitN(volumeBind, ":", 2) - from := fromAndTo[0] - to := fromAndTo[1] - - fileContents, _ := ioutil.ReadFile(from) - volumeBinds[from] = struct { - ContainerPath string - FileContents string - }{ - ContainerPath: to, - FileContents: string(fileContents), - } + // Populate fileContents. If it's not a file it won't resolve and + //we ignore the error + contents, _ := ioutil.ReadFile(value) + fileContents[name] = string(contents) } w.WriteHeader(201) @@ -252,17 +267,11 @@ D: !var:file D_value So(err, ShouldBeNil) So(code, ShouldEqual, 0) - // The volume mount binds are expected to take the form - // 'host_path:container_path', where host_path is equal to container_path - for from, volumeBind := range volumeBinds { - So(from, ShouldEqual, volumeBind.ContainerPath) - } - - // Ensure envvars and volumemounts passed to Docker match expectations + // Ensure envvars passed to Docker match expectations So(envvars["A"], ShouldEqual, expected["A"]) So(envvars["B"], ShouldEqual, expected["B"]) - So(volumeBinds[envvars["C"]].FileContents, ShouldEqual, expected["C"]) - So(volumeBinds[envvars["D"]].FileContents, ShouldEqual, expected["D"]) + So(fileContents["C"], ShouldEqual, expected["C"]) + So(fileContents["D"], ShouldEqual, expected["D"]) } func TestDefaultVariableResolution(t *testing.T) { diff --git a/internal/command/temp_factory.go b/internal/command/temp_factory.go index eb8d4013..de00c810 100644 --- a/internal/command/temp_factory.go +++ b/internal/command/temp_factory.go @@ -9,7 +9,7 @@ import ( // DEVSHM is the default *nix shared-memory directory path const DEVSHM = "/dev/shm" -// TempFactory handels transient files that require cleaning up +// TempFactory handles transient files that require cleaning up // after the child process exits. type TempFactory struct { path string