Skip to content

Commit

Permalink
Merge pull request #9391 from hashicorp/b-0129-backport
Browse files Browse the repository at this point in the history 
Backport of client: fix interpolation in template source #9383
  • Loading branch information
@schmichael
schmichael committed Nov 18, 2020
2 parents 99fa277 + 347f2f6 commit 9a838b8
Show file tree
Hide file tree
Showing 3 changed files with 277 additions and 2 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
## 0.12.9 (November 18, 2020)

BUG FIXES:
* client: Fixed a regression where `NOMAD_{ALLOC,TASK,SECRETS}_DIR` variables would cause an error when interpolated into `template.source` stanzas. [[GH-9391](https://github.com/hashicorp/nomad/issues/9391)]

## 0.12.8 (November 10, 2020)

SECURITY:
Expand Down
23 changes: 21 additions & 2 deletions client/allocrunner/taskrunner/template/template.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package template

import (
"context"
"errors"
"fmt"
"math/rand"
"os"
Expand All @@ -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"
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
}
}

Expand All @@ -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
}
}

Expand Down
251 changes: 251 additions & 0 deletions client/allocrunner/taskrunner/template/template_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -1452,6 +1454,255 @@ 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

// Set to skip a test; remove once bugs are fixed
Skip bool

// 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. https://github.com/hashicorp/nomad/issues/9389
{
Skip: true,
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,
},
{
Name: "RawExecDstAbsoluteOk",
Config: func() *TaskTemplateManagerConfig {
return &TaskTemplateManagerConfig{
ClientConfig: clientConf,
TaskDir: taskDir.Dir,
EnvBuilder: rawExecEnv(),
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"),
},
}

for i := range cases {
tc := cases[i]
t.Run(tc.Name, func(t *testing.T) {
if tc.Skip {
t.Skip("FIXME: Skipping broken test")
}
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
Expand Down

0 comments on commit 9a838b8

Please sign in to comment.