From 96cbf337c15c8203113c9d2e794bd78599cc3771 Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Thu, 10 Nov 2016 11:47:43 -0800 Subject: [PATCH 1/5] Move parsing of vault token above validation such that it works in dev mode --- command/agent/command.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/command/agent/command.go b/command/agent/command.go index fbfd5f9e8..f9260f1b3 100644 --- a/command/agent/command.go +++ b/command/agent/command.go @@ -207,6 +207,13 @@ func (c *Command) readConfig() *Config { return nil } + // Check to see if we should read the Vault token from the environment + if config.Vault.Token == "" { + if token, ok := os.LookupEnv("VAULT_TOKEN"); ok { + config.Vault.Token = token + } + } + if dev { // Skip validation for dev mode return config @@ -278,13 +285,6 @@ func (c *Command) readConfig() *Config { c.Ui.Error("WARNING: Bootstrap mode enabled! Potentially unsafe operation.") } - // Check to see if we should read the Vault token from the environment - if config.Vault.Token == "" { - if token, ok := os.LookupEnv("VAULT_TOKEN"); ok { - config.Vault.Token = token - } - } - return config } From e246167dc524f7e3d86e4bbdff8373f91e0d74ff Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Thu, 10 Nov 2016 14:47:54 -0800 Subject: [PATCH 2/5] Changes the relative path from joining against the alloc dir to the task's directory. This PR changes the behavior when given a relative host path when mounting docker containers. Prior to this, the behavior was to mount by joining against the alloc/ directory. This PR changes it to be against the task/ directory. --- client/driver/docker.go | 2 +- client/driver/docker_test.go | 8 +++++++- website/source/docs/drivers/docker.html.md | 9 +++++---- 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/client/driver/docker.go b/client/driver/docker.go index 88a514a1e..c3a0268d3 100644 --- a/client/driver/docker.go +++ b/client/driver/docker.go @@ -609,7 +609,7 @@ func (d *DockerDriver) containerBinds(driverConfig *DockerDriverConfig, alloc *a // Relative paths are always allowed as they mount within a container // Expand path relative to alloc dir - parts[0] = filepath.Join(shared, parts[0]) + parts[0] = filepath.Join(taskDir, parts[0]) binds = append(binds, strings.Join(parts, ":")) } diff --git a/client/driver/docker_test.go b/client/driver/docker_test.go index b97142236..ed798854d 100644 --- a/client/driver/docker_test.go +++ b/client/driver/docker_test.go @@ -1083,13 +1083,19 @@ func TestDockerDriver_VolumesDisabled(t *testing.T) { t.Fatalf("timeout") } - if _, err := ioutil.ReadFile(filepath.Join(execCtx.AllocDir.SharedDir, fn)); err != nil { + taskDir, ok := execCtx.AllocDir.TaskDirs[task.Name] + if !ok { + t.Fatalf("Failed to get task dir") + } + + if _, err := ioutil.ReadFile(filepath.Join(taskDir, fn)); err != nil { t.Fatalf("unexpected error reading %s: %v", fn, err) } } } +// TODO The test should really check that the container can access the file. func TestDockerDriver_VolumesEnabled(t *testing.T) { cfg := testConfig() diff --git a/website/source/docs/drivers/docker.html.md b/website/source/docs/drivers/docker.html.md index 661e58eff..b4a8de9ad 100644 --- a/website/source/docs/drivers/docker.html.md +++ b/website/source/docs/drivers/docker.html.md @@ -166,9 +166,10 @@ The `docker` driver supports the following configuration in the job spec: ``` * `volumes` - (Optional) A list of `host_path:container_path` strings to bind - host paths to container paths. Mounting host paths outside of the alloc - directory tasks normally have access to can be disabled on clients by setting - the `docker.volumes.enabled` option set to false. + host paths to container paths. Mounting host paths outside of the allocation + directory can be disabled on clients by setting the `docker.volumes.enabled` + option set to false. This will limit volumes to directories that exist inside + the allocation directory. ```hcl config { @@ -177,7 +178,7 @@ The `docker` driver supports the following configuration in the job spec: "/path/on/host:/path/in/container", # Use relative paths to rebind paths already in the allocation dir - "relative/to/alloc:/also/in/container" + "relative/to/task:/also/in/container" ] } ``` From ca9738f4aac06962e5de8a98eae0688f647727d4 Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Thu, 10 Nov 2016 15:16:08 -0800 Subject: [PATCH 3/5] Always disable renew_token for CT config This PR makes Nomad always disable token renewal even if Vault is disabled. The problem was when there was a vault token in the environment variable and Nomad/Vault integration was disabled, the template runner would still try to renew the token. --- client/consul_template.go | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/client/consul_template.go b/client/consul_template.go index 19150d71b..b28c10c2d 100644 --- a/client/consul_template.go +++ b/client/consul_template.go @@ -448,13 +448,15 @@ func runnerConfig(config *config.Config, vaultToken string) (*ctconf.Config, err } // Setup the Vault config + // Always set these to ensure nothing is picked up from the environment + conf.Vault = &ctconf.VaultConfig{ + RenewToken: false, + } + set([]string{"vault", "vault.token", "vault.renew_token"}) if config.VaultConfig != nil && config.VaultConfig.IsEnabled() { - conf.Vault = &ctconf.VaultConfig{ - Address: config.VaultConfig.Addr, - Token: vaultToken, - RenewToken: false, - } - set([]string{"vault", "vault.address", "vault.token", "vault.renew_token"}) + conf.Vault.Address = config.VaultConfig.Addr + conf.Vault.Token = vaultToken + set([]string{"vault.address"}) if strings.HasPrefix(config.VaultConfig.Addr, "https") || config.VaultConfig.TLSCertFile != "" { verify := config.VaultConfig.TLSSkipVerify == nil || !*config.VaultConfig.TLSSkipVerify From 0aad09c23946ccf291e452fbbc47559cf8aaa7aa Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Thu, 10 Nov 2016 15:20:19 -0800 Subject: [PATCH 4/5] Remove todo --- client/driver/docker_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/client/driver/docker_test.go b/client/driver/docker_test.go index ed798854d..69cd9f01c 100644 --- a/client/driver/docker_test.go +++ b/client/driver/docker_test.go @@ -1095,7 +1095,6 @@ func TestDockerDriver_VolumesDisabled(t *testing.T) { } -// TODO The test should really check that the container can access the file. func TestDockerDriver_VolumesEnabled(t *testing.T) { cfg := testConfig() From 2c403db925c30bd1fed0cf71297c8f11e358fd02 Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Thu, 10 Nov 2016 15:24:08 -0800 Subject: [PATCH 5/5] changelog --- CHANGELOG.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 277b29289..d9660f64f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,8 +33,6 @@ IMPROVEMENTS: * cli: `nomad node-status` shows node metadata in verbose mode [GH-1841] * client: Failed RPCs are retried on all servers [GH-1735] * client: Fingerprint and driver blacklist support [GH-1949] - * client: Enforce shared allocation directory disk usage [GH-1580] - * client: Do not validate the command does not contain spaces [GH-1974] * client: Introduce a `secrets/` directory to tasks where sensitive data can be written [GH-1681] * client/jobspec: Add support for templates that can render static files, @@ -63,6 +61,7 @@ BUG FIXES: [GH-1844] * client: Prevent race when persisting state file [GH-1682] * client: Retry recoverable errors when starting a driver [GH-1891] + * client: Do not validate the command does not contain spaces [GH-1974] * client: Fix old services not getting removed from consul on update [GH-1668] * client: Preserve permissions of nested directories while chrooting [GH-1960] * client: Folder permissions are dropped even when not running as root [GH-1888]