client: fix interpolation in template source
While Nomad v0.12.8 fixed `NOMAD_{ALLOC,TASK,SECRETS}_DIR` use in
`template.destination`, interpolating these variables in
`template.source` caused a path escape error.
**Why not apply the destination fix to source?**
The destination fix forces destination to always be relative to the task
directory. This makes sense for the destination as a destination outside
the task directory would be unreachable by the task. There's no reason
to ever render a template outside the task directory. (Using `..` does
allow destinations to escape the task directory if
`template.disable_file_sandbox = true`. That's just awkward and unsafe
enough I hope no one uses it.)
There is a reason to source a template outside a task
directory. At least if there weren't then I can't think of why we
implemented `template.disable_file_sandbox`. So v0.12.8 left the
behavior of `template.source` the more straightforward "Interpolate and
validate."
However, since outside of `raw_exec` every other driver uses absolute
paths for `NOMAD_*_DIR` interpolation, this means those variables are
unusable unless `disable_file_sandbox` is set.
**The Fix**
The variables are now interpolated as relative paths *only for the
purpose of rendering templates.* This is an unfortunate special case,
but reflects the fact that the templates view of the filesystem is
completely different (unconstrainted) vs the task's view (chrooted).
Arguably the values of these variables *should be context-specific.*
I think it's more reasonable to think of the "hack" as templating
running uncontainerized than that giving templates different paths is a
hack.
**TODO**
- [ ] E2E tests
- [ ] Job validation may still be broken and prevent my fix from
working?
**raw_exec**
`raw_exec` is actually broken _a different way_ as exercised by tests in
this commit. I think we should probably remove these tests and fix that
in a followup PR/release, but I wanted to leave them in for the initial
review and discussion. Since non-containerized source paths are broken
anyway, perhaps there's another solution to this entire problem I'm
overlooking?
This commit is contained in:
Michael Schurter
parent
bb82e45363
commit
ff91bba70e
|
|
@ -2,6 +2,7 @@ package template
|
|||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"fmt"
|
||||
"math/rand"
|
||||
"os"
|
||||
|
|
@ -17,6 +18,7 @@ import (
|
|||
"github.com/hashicorp/consul-template/signals"
|
||||
envparse "github.com/hashicorp/go-envparse"
|
||||
multierror "github.com/hashicorp/go-multierror"
|
||||
"github.com/hashicorp/nomad/client/allocdir"
|
||||
"github.com/hashicorp/nomad/client/allocrunner/taskrunner/interfaces"
|
||||
"github.com/hashicorp/nomad/client/config"
|
||||
"github.com/hashicorp/nomad/client/taskenv"
|
||||
|
|
@ -38,6 +40,11 @@ const (
|
|||
DefaultMaxTemplateEventRate = 3 * time.Second
|
||||
)
|
||||
|
||||
var (
|
||||
sourceEscapesErr = errors.New("template source path escapes alloc directory")
|
||||
destEscapesErr = errors.New("template destination path escapes alloc directory")
|
||||
)
|
||||
|
||||
// TaskTemplateManager is used to run a set of templates for a given task
|
||||
type TaskTemplateManager struct {
|
||||
// config holds the template managers configuration
|
||||
|
|
@ -548,6 +555,18 @@ func parseTemplateConfigs(config *TaskTemplateManagerConfig) (map[*ctconf.Templa
|
|||
sandboxEnabled := !config.ClientConfig.TemplateConfig.DisableSandbox
|
||||
taskEnv := config.EnvBuilder.Build()
|
||||
|
||||
// Make NOMAD_{ALLOC,TASK,SECRETS}_DIR relative paths to avoid treating
|
||||
// them as sandbox escapes when using containers.
|
||||
if taskEnv.EnvMap[taskenv.AllocDir] == allocdir.SharedAllocContainerPath {
|
||||
taskEnv.EnvMap[taskenv.AllocDir] = allocdir.SharedAllocName
|
||||
}
|
||||
if taskEnv.EnvMap[taskenv.TaskLocalDir] == allocdir.TaskLocalContainerPath {
|
||||
taskEnv.EnvMap[taskenv.TaskLocalDir] = allocdir.TaskLocal
|
||||
}
|
||||
if taskEnv.EnvMap[taskenv.SecretsDir] == allocdir.TaskSecretsContainerPath {
|
||||
taskEnv.EnvMap[taskenv.SecretsDir] = allocdir.TaskSecrets
|
||||
}
|
||||
|
||||
ctmpls := make(map[*ctconf.TemplateConfig]*structs.Template, len(config.Templates))
|
||||
for _, tmpl := range config.Templates {
|
||||
var src, dest string
|
||||
|
|
@ -560,7 +579,7 @@ func parseTemplateConfigs(config *TaskTemplateManagerConfig) (map[*ctconf.Templa
|
|||
}
|
||||
escapes := helper.PathEscapesSandbox(config.TaskDir, src)
|
||||
if escapes && sandboxEnabled {
|
||||
return nil, fmt.Errorf("template source path escapes alloc directory")
|
||||
return nil, sourceEscapesErr
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -572,7 +591,7 @@ func parseTemplateConfigs(config *TaskTemplateManagerConfig) (map[*ctconf.Templa
|
|||
dest = filepath.Join(config.TaskDir, dest)
|
||||
escapes := helper.PathEscapesSandbox(config.TaskDir, dest)
|
||||
if escapes && sandboxEnabled {
|
||||
return nil, fmt.Errorf("template destination path escapes alloc directory")
|
||||
return nil, destEscapesErr
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -17,9 +17,11 @@ import (
|
|||
"time"
|
||||
|
||||
ctestutil "github.com/hashicorp/consul/sdk/testutil"
|
||||
"github.com/hashicorp/nomad/client/allocdir"
|
||||
"github.com/hashicorp/nomad/client/config"
|
||||
"github.com/hashicorp/nomad/client/taskenv"
|
||||
"github.com/hashicorp/nomad/helper"
|
||||
"github.com/hashicorp/nomad/helper/testlog"
|
||||
"github.com/hashicorp/nomad/helper/uuid"
|
||||
"github.com/hashicorp/nomad/nomad/mock"
|
||||
"github.com/hashicorp/nomad/nomad/structs"
|
||||
|
|
@ -1452,6 +1454,250 @@ func TestTaskTemplateManager_Config_VaultNamespace_TaskOverride(t *testing.T) {
|
|||
assert.Equal(overriddenNS, *ctconf.Vault.Namespace, "Vault Namespace Value")
|
||||
}
|
||||
|
||||
// TestTaskTemplateManager_Escapes asserts that when sandboxing is enabled
|
||||
// interpolated paths are not incorrectly treated as escaping the alloc dir.
|
||||
func TestTaskTemplateManager_Escapes(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
clientConf := config.DefaultConfig()
|
||||
require.False(t, clientConf.TemplateConfig.DisableSandbox, "expected sandbox to be disabled")
|
||||
|
||||
// Set a fake alloc dir to make test output more realistic
|
||||
clientConf.AllocDir = "/fake/allocdir"
|
||||
|
||||
clientConf.Node = mock.Node()
|
||||
alloc := mock.Alloc()
|
||||
task := alloc.Job.TaskGroups[0].Tasks[0]
|
||||
logger := testlog.HCLogger(t)
|
||||
allocDir := allocdir.NewAllocDir(logger, filepath.Join(clientConf.AllocDir, alloc.ID))
|
||||
taskDir := allocDir.NewTaskDir(task.Name)
|
||||
|
||||
containerEnv := func() *taskenv.Builder {
|
||||
// To emulate a Docker or exec tasks we must copy the
|
||||
// Set{Alloc,Task,Secrets}Dir logic in taskrunner/task_dir_hook.go
|
||||
b := taskenv.NewBuilder(clientConf.Node, alloc, task, clientConf.Region)
|
||||
b.SetAllocDir(allocdir.SharedAllocContainerPath)
|
||||
b.SetTaskLocalDir(allocdir.TaskLocalContainerPath)
|
||||
b.SetSecretsDir(allocdir.TaskSecretsContainerPath)
|
||||
return b
|
||||
}
|
||||
|
||||
rawExecEnv := func() *taskenv.Builder {
|
||||
// To emulate a unisolated tasks we must copy the
|
||||
// Set{Alloc,Task,Secrets}Dir logic in taskrunner/task_dir_hook.go
|
||||
b := taskenv.NewBuilder(clientConf.Node, alloc, task, clientConf.Region)
|
||||
b.SetAllocDir(taskDir.SharedAllocDir)
|
||||
b.SetTaskLocalDir(taskDir.LocalDir)
|
||||
b.SetSecretsDir(taskDir.SecretsDir)
|
||||
return b
|
||||
}
|
||||
|
||||
cases := []struct {
|
||||
Name string
|
||||
Config func() *TaskTemplateManagerConfig
|
||||
|
||||
// Expected paths to be returned if Err is nil
|
||||
SourcePath string
|
||||
DestPath string
|
||||
|
||||
// Err is the expected error to be returned or nil
|
||||
Err error
|
||||
}{
|
||||
{
|
||||
Name: "ContainerOk",
|
||||
Config: func() *TaskTemplateManagerConfig {
|
||||
return &TaskTemplateManagerConfig{
|
||||
ClientConfig: clientConf,
|
||||
TaskDir: taskDir.Dir,
|
||||
EnvBuilder: containerEnv(),
|
||||
Templates: []*structs.Template{
|
||||
{
|
||||
SourcePath: "${NOMAD_TASK_DIR}/src",
|
||||
DestPath: "${NOMAD_SECRETS_DIR}/dst",
|
||||
},
|
||||
},
|
||||
}
|
||||
},
|
||||
SourcePath: filepath.Join(taskDir.Dir, "local/src"),
|
||||
DestPath: filepath.Join(taskDir.Dir, "secrets/dst"),
|
||||
},
|
||||
{
|
||||
Name: "ContainerSrcEscapesErr",
|
||||
Config: func() *TaskTemplateManagerConfig {
|
||||
return &TaskTemplateManagerConfig{
|
||||
ClientConfig: clientConf,
|
||||
TaskDir: taskDir.Dir,
|
||||
EnvBuilder: containerEnv(),
|
||||
Templates: []*structs.Template{
|
||||
{
|
||||
SourcePath: "/etc/src_escapes",
|
||||
DestPath: "${NOMAD_SECRETS_DIR}/dst",
|
||||
},
|
||||
},
|
||||
}
|
||||
},
|
||||
Err: sourceEscapesErr,
|
||||
},
|
||||
{
|
||||
Name: "ContainerSrcEscapesOk",
|
||||
Config: func() *TaskTemplateManagerConfig {
|
||||
unsafeConf := clientConf.Copy()
|
||||
unsafeConf.TemplateConfig.DisableSandbox = true
|
||||
return &TaskTemplateManagerConfig{
|
||||
ClientConfig: unsafeConf,
|
||||
TaskDir: taskDir.Dir,
|
||||
EnvBuilder: containerEnv(),
|
||||
Templates: []*structs.Template{
|
||||
{
|
||||
SourcePath: "/etc/src_escapes_ok",
|
||||
DestPath: "${NOMAD_SECRETS_DIR}/dst",
|
||||
},
|
||||
},
|
||||
}
|
||||
},
|
||||
SourcePath: "/etc/src_escapes_ok",
|
||||
DestPath: filepath.Join(taskDir.Dir, "secrets/dst"),
|
||||
},
|
||||
{
|
||||
Name: "ContainerDstAbsoluteOk",
|
||||
Config: func() *TaskTemplateManagerConfig {
|
||||
return &TaskTemplateManagerConfig{
|
||||
ClientConfig: clientConf,
|
||||
TaskDir: taskDir.Dir,
|
||||
EnvBuilder: containerEnv(),
|
||||
Templates: []*structs.Template{
|
||||
{
|
||||
SourcePath: "${NOMAD_TASK_DIR}/src",
|
||||
DestPath: "/etc/absolutely_relative",
|
||||
},
|
||||
},
|
||||
}
|
||||
},
|
||||
SourcePath: filepath.Join(taskDir.Dir, "local/src"),
|
||||
DestPath: filepath.Join(taskDir.Dir, "etc/absolutely_relative"),
|
||||
},
|
||||
{
|
||||
Name: "ContainerDstAbsoluteEscapesErr",
|
||||
Config: func() *TaskTemplateManagerConfig {
|
||||
return &TaskTemplateManagerConfig{
|
||||
ClientConfig: clientConf,
|
||||
TaskDir: taskDir.Dir,
|
||||
EnvBuilder: containerEnv(),
|
||||
Templates: []*structs.Template{
|
||||
{
|
||||
SourcePath: "${NOMAD_TASK_DIR}/src",
|
||||
DestPath: "../escapes",
|
||||
},
|
||||
},
|
||||
}
|
||||
},
|
||||
Err: destEscapesErr,
|
||||
},
|
||||
{
|
||||
Name: "ContainerDstAbsoluteEscapesOk",
|
||||
Config: func() *TaskTemplateManagerConfig {
|
||||
unsafeConf := clientConf.Copy()
|
||||
unsafeConf.TemplateConfig.DisableSandbox = true
|
||||
return &TaskTemplateManagerConfig{
|
||||
ClientConfig: unsafeConf,
|
||||
TaskDir: taskDir.Dir,
|
||||
EnvBuilder: containerEnv(),
|
||||
Templates: []*structs.Template{
|
||||
{
|
||||
SourcePath: "${NOMAD_TASK_DIR}/src",
|
||||
DestPath: "../escapes",
|
||||
},
|
||||
},
|
||||
}
|
||||
},
|
||||
SourcePath: filepath.Join(taskDir.Dir, "local/src"),
|
||||
DestPath: filepath.Join(taskDir.Dir, "..", "escapes"),
|
||||
},
|
||||
//TODO: Fix this test. I *think* it should pass. The double
|
||||
// joining of the task dir onto the destination seems like
|
||||
// a bug.
|
||||
{
|
||||
Name: "RawExecOk",
|
||||
Config: func() *TaskTemplateManagerConfig {
|
||||
return &TaskTemplateManagerConfig{
|
||||
ClientConfig: clientConf,
|
||||
TaskDir: taskDir.Dir,
|
||||
EnvBuilder: rawExecEnv(),
|
||||
Templates: []*structs.Template{
|
||||
{
|
||||
SourcePath: "${NOMAD_TASK_DIR}/src",
|
||||
DestPath: "${NOMAD_SECRETS_DIR}/dst",
|
||||
},
|
||||
},
|
||||
}
|
||||
},
|
||||
SourcePath: filepath.Join(taskDir.Dir, "local/src"),
|
||||
DestPath: filepath.Join(taskDir.Dir, "secrets/dst"),
|
||||
},
|
||||
{
|
||||
Name: "RawExecSrcEscapesErr",
|
||||
Config: func() *TaskTemplateManagerConfig {
|
||||
return &TaskTemplateManagerConfig{
|
||||
ClientConfig: clientConf,
|
||||
TaskDir: taskDir.Dir,
|
||||
EnvBuilder: rawExecEnv(),
|
||||
Templates: []*structs.Template{
|
||||
{
|
||||
SourcePath: "/etc/src_escapes",
|
||||
DestPath: "${NOMAD_SECRETS_DIR}/dst",
|
||||
},
|
||||
},
|
||||
}
|
||||
},
|
||||
Err: sourceEscapesErr,
|
||||
},
|
||||
//TODO: Fix this test. I *think* it should fail instead of
|
||||
// forcing it relative. The double joining of the task dir
|
||||
// onto the destination seems like a bug.
|
||||
{
|
||||
Name: "RawExecDstEscapesErr",
|
||||
Config: func() *TaskTemplateManagerConfig {
|
||||
return &TaskTemplateManagerConfig{
|
||||
ClientConfig: clientConf,
|
||||
TaskDir: taskDir.Dir,
|
||||
EnvBuilder: rawExecEnv(),
|
||||
Templates: []*structs.Template{
|
||||
{
|
||||
SourcePath: "${NOMAD_TASK_DIR}/src",
|
||||
DestPath: "/etc/dst_escapes",
|
||||
},
|
||||
},
|
||||
}
|
||||
},
|
||||
Err: destEscapesErr,
|
||||
},
|
||||
}
|
||||
|
||||
for i := range cases {
|
||||
tc := cases[i]
|
||||
t.Run(tc.Name, func(t *testing.T) {
|
||||
config := tc.Config()
|
||||
mapping, err := parseTemplateConfigs(config)
|
||||
if tc.Err == nil {
|
||||
// Ok path
|
||||
require.NoError(t, err)
|
||||
require.NotNil(t, mapping)
|
||||
require.Len(t, mapping, 1)
|
||||
for k := range mapping {
|
||||
require.Equal(t, tc.SourcePath, *k.Source)
|
||||
require.Equal(t, tc.DestPath, *k.Destination)
|
||||
t.Logf("Rendering %s => %s", *k.Source, *k.Destination)
|
||||
}
|
||||
} else {
|
||||
// Err path
|
||||
assert.EqualError(t, err, tc.Err.Error())
|
||||
require.Nil(t, mapping)
|
||||
}
|
||||
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestTaskTemplateManager_BlockedEvents(t *testing.T) {
|
||||
// The tests sets a template that need keys 0, 1, 2, 3, 4,
|
||||
// then subsequently sets 0, 1, 2 keys
|
||||
|
|
|
|||
Loading…
Reference in New Issue