Skip to content

Commit 0267ec7

Browse files
schmichaelschmichael
authored
Merge pull request #9405 from hashicorp/b-0109-backport
Backport to 0.10 of client: fix interpolation in template source #9383
2 parents 75dcbee + 9878b77 commit 0267ec7

File tree

3 files changed

+277
-2
lines changed

3 files changed

+277
-2
lines changed

‎CHANGELOG.md

+5
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,8 @@
1+
## 0.10.9 (November 19, 2020)
2+
3+
BUG FIXES:
4+
* client: _Backport from v0.12.9_ - Fixed a regression where `NOMAD_{ALLOC,TASK,SECRETS}_DIR` variables would cause an error when interpolated into `template.source` stanzas. [[GH-9405](https://github.com/hashicorp/nomad/issues/9405)]
5+
16
## 0.10.8 (November 10, 2020)
27

38
SECURITY:

‎client/allocrunner/taskrunner/template/template.go

+21-2
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package template
22

33
import (
44
"context"
5+
"errors"
56
"fmt"
67
"math/rand"
78
"os"
@@ -17,6 +18,7 @@ import (
1718
"github.com/hashicorp/consul-template/signals"
1819
envparse "github.com/hashicorp/go-envparse"
1920
multierror "github.com/hashicorp/go-multierror"
21+
"github.com/hashicorp/nomad/client/allocdir"
2022
"github.com/hashicorp/nomad/client/allocrunner/taskrunner/interfaces"
2123
"github.com/hashicorp/nomad/client/config"
2224
"github.com/hashicorp/nomad/client/taskenv"
@@ -38,6 +40,11 @@ const (
3840
DefaultMaxTemplateEventRate = 3 * time.Second
3941
)
4042

43+
var (
44+
sourceEscapesErr = errors.New("template source path escapes alloc directory")
45+
destEscapesErr = errors.New("template destination path escapes alloc directory")
46+
)
47+
4148
// TaskTemplateManager is used to run a set of templates for a given task
4249
type TaskTemplateManager struct {
4350
// config holds the template managers configuration
@@ -545,6 +552,18 @@ func parseTemplateConfigs(config *TaskTemplateManagerConfig) (map[*ctconf.Templa
545552
sandboxEnabled := !config.ClientConfig.TemplateConfig.DisableSandbox
546553
taskEnv := config.EnvBuilder.Build()
547554

555+
// Make NOMAD_{ALLOC,TASK,SECRETS}_DIR relative paths to avoid treating
556+
// them as sandbox escapes when using containers.
557+
if taskEnv.EnvMap[taskenv.AllocDir] == allocdir.SharedAllocContainerPath {
558+
taskEnv.EnvMap[taskenv.AllocDir] = allocdir.SharedAllocName
559+
}
560+
if taskEnv.EnvMap[taskenv.TaskLocalDir] == allocdir.TaskLocalContainerPath {
561+
taskEnv.EnvMap[taskenv.TaskLocalDir] = allocdir.TaskLocal
562+
}
563+
if taskEnv.EnvMap[taskenv.SecretsDir] == allocdir.TaskSecretsContainerPath {
564+
taskEnv.EnvMap[taskenv.SecretsDir] = allocdir.TaskSecrets
565+
}
566+
548567
ctmpls := make(map[*ctconf.TemplateConfig]*structs.Template, len(config.Templates))
549568
for _, tmpl := range config.Templates {
550569
var src, dest string
@@ -557,7 +576,7 @@ func parseTemplateConfigs(config *TaskTemplateManagerConfig) (map[*ctconf.Templa
557576
}
558577
escapes := helper.PathEscapesSandbox(config.TaskDir, src)
559578
if escapes && sandboxEnabled {
560-
return nil, fmt.Errorf("template source path escapes alloc directory")
579+
return nil, sourceEscapesErr
561580
}
562581
}
563582

@@ -569,7 +588,7 @@ func parseTemplateConfigs(config *TaskTemplateManagerConfig) (map[*ctconf.Templa
569588
dest = filepath.Join(config.TaskDir, dest)
570589
escapes := helper.PathEscapesSandbox(config.TaskDir, dest)
571590
if escapes && sandboxEnabled {
572-
return nil, fmt.Errorf("template destination path escapes alloc directory")
591+
return nil, destEscapesErr
573592
}
574593
}
575594

‎client/allocrunner/taskrunner/template/template_test.go

+251
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,11 @@ import (
1313
"time"
1414

1515
ctestutil "github.com/hashicorp/consul/testutil"
16+
"github.com/hashicorp/nomad/client/allocdir"
1617
"github.com/hashicorp/nomad/client/config"
1718
"github.com/hashicorp/nomad/client/taskenv"
1819
"github.com/hashicorp/nomad/helper"
20+
"github.com/hashicorp/nomad/helper/testlog"
1921
"github.com/hashicorp/nomad/helper/uuid"
2022
"github.com/hashicorp/nomad/nomad/mock"
2123
"github.com/hashicorp/nomad/nomad/structs"
@@ -1455,6 +1457,255 @@ func TestTaskTemplateManager_Config_VaultNamespace(t *testing.T) {
14551457
assert.Equal(testNS, *ctconf.Vault.Namespace, "Vault Namespace Value")
14561458
}
14571459

1460+
// TestTaskTemplateManager_Escapes asserts that when sandboxing is enabled
1461+
// interpolated paths are not incorrectly treated as escaping the alloc dir.
1462+
func TestTaskTemplateManager_Escapes(t *testing.T) {
1463+
t.Parallel()
1464+
1465+
clientConf := config.DefaultConfig()
1466+
require.False(t, clientConf.TemplateConfig.DisableSandbox, "expected sandbox to be disabled")
1467+
1468+
// Set a fake alloc dir to make test output more realistic
1469+
clientConf.AllocDir = "/fake/allocdir"
1470+
1471+
clientConf.Node = mock.Node()
1472+
alloc := mock.Alloc()
1473+
task := alloc.Job.TaskGroups[0].Tasks[0]
1474+
logger := testlog.HCLogger(t)
1475+
allocDir := allocdir.NewAllocDir(logger, filepath.Join(clientConf.AllocDir, alloc.ID))
1476+
taskDir := allocDir.NewTaskDir(task.Name)
1477+
1478+
containerEnv := func() *taskenv.Builder {
1479+
// To emulate a Docker or exec tasks we must copy the
1480+
// Set{Alloc,Task,Secrets}Dir logic in taskrunner/task_dir_hook.go
1481+
b := taskenv.NewBuilder(clientConf.Node, alloc, task, clientConf.Region)
1482+
b.SetAllocDir(allocdir.SharedAllocContainerPath)
1483+
b.SetTaskLocalDir(allocdir.TaskLocalContainerPath)
1484+
b.SetSecretsDir(allocdir.TaskSecretsContainerPath)
1485+
return b
1486+
}
1487+
1488+
rawExecEnv := func() *taskenv.Builder {
1489+
// To emulate a unisolated tasks we must copy the
1490+
// Set{Alloc,Task,Secrets}Dir logic in taskrunner/task_dir_hook.go
1491+
b := taskenv.NewBuilder(clientConf.Node, alloc, task, clientConf.Region)
1492+
b.SetAllocDir(taskDir.SharedAllocDir)
1493+
b.SetTaskLocalDir(taskDir.LocalDir)
1494+
b.SetSecretsDir(taskDir.SecretsDir)
1495+
return b
1496+
}
1497+
1498+
cases := []struct {
1499+
Name string
1500+
Config func() *TaskTemplateManagerConfig
1501+
1502+
// Set to skip a test; remove once bugs are fixed
1503+
Skip bool
1504+
1505+
// Expected paths to be returned if Err is nil
1506+
SourcePath string
1507+
DestPath string
1508+
1509+
// Err is the expected error to be returned or nil
1510+
Err error
1511+
}{
1512+
{
1513+
Name: "ContainerOk",
1514+
Config: func() *TaskTemplateManagerConfig {
1515+
return &TaskTemplateManagerConfig{
1516+
ClientConfig: clientConf,
1517+
TaskDir: taskDir.Dir,
1518+
EnvBuilder: containerEnv(),
1519+
Templates: []*structs.Template{
1520+
{
1521+
SourcePath: "${NOMAD_TASK_DIR}/src",
1522+
DestPath: "${NOMAD_SECRETS_DIR}/dst",
1523+
},
1524+
},
1525+
}
1526+
},
1527+
SourcePath: filepath.Join(taskDir.Dir, "local/src"),
1528+
DestPath: filepath.Join(taskDir.Dir, "secrets/dst"),
1529+
},
1530+
{
1531+
Name: "ContainerSrcEscapesErr",
1532+
Config: func() *TaskTemplateManagerConfig {
1533+
return &TaskTemplateManagerConfig{
1534+
ClientConfig: clientConf,
1535+
TaskDir: taskDir.Dir,
1536+
EnvBuilder: containerEnv(),
1537+
Templates: []*structs.Template{
1538+
{
1539+
SourcePath: "/etc/src_escapes",
1540+
DestPath: "${NOMAD_SECRETS_DIR}/dst",
1541+
},
1542+
},
1543+
}
1544+
},
1545+
Err: sourceEscapesErr,
1546+
},
1547+
{
1548+
Name: "ContainerSrcEscapesOk",
1549+
Config: func() *TaskTemplateManagerConfig {
1550+
unsafeConf := clientConf.Copy()
1551+
unsafeConf.TemplateConfig.DisableSandbox = true
1552+
return &TaskTemplateManagerConfig{
1553+
ClientConfig: unsafeConf,
1554+
TaskDir: taskDir.Dir,
1555+
EnvBuilder: containerEnv(),
1556+
Templates: []*structs.Template{
1557+
{
1558+
SourcePath: "/etc/src_escapes_ok",
1559+
DestPath: "${NOMAD_SECRETS_DIR}/dst",
1560+
},
1561+
},
1562+
}
1563+
},
1564+
SourcePath: "/etc/src_escapes_ok",
1565+
DestPath: filepath.Join(taskDir.Dir, "secrets/dst"),
1566+
},
1567+
{
1568+
Name: "ContainerDstAbsoluteOk",
1569+
Config: func() *TaskTemplateManagerConfig {
1570+
return &TaskTemplateManagerConfig{
1571+
ClientConfig: clientConf,
1572+
TaskDir: taskDir.Dir,
1573+
EnvBuilder: containerEnv(),
1574+
Templates: []*structs.Template{
1575+
{
1576+
SourcePath: "${NOMAD_TASK_DIR}/src",
1577+
DestPath: "/etc/absolutely_relative",
1578+
},
1579+
},
1580+
}
1581+
},
1582+
SourcePath: filepath.Join(taskDir.Dir, "local/src"),
1583+
DestPath: filepath.Join(taskDir.Dir, "etc/absolutely_relative"),
1584+
},
1585+
{
1586+
Name: "ContainerDstAbsoluteEscapesErr",
1587+
Config: func() *TaskTemplateManagerConfig {
1588+
return &TaskTemplateManagerConfig{
1589+
ClientConfig: clientConf,
1590+
TaskDir: taskDir.Dir,
1591+
EnvBuilder: containerEnv(),
1592+
Templates: []*structs.Template{
1593+
{
1594+
SourcePath: "${NOMAD_TASK_DIR}/src",
1595+
DestPath: "../escapes",
1596+
},
1597+
},
1598+
}
1599+
},
1600+
Err: destEscapesErr,
1601+
},
1602+
{
1603+
Name: "ContainerDstAbsoluteEscapesOk",
1604+
Config: func() *TaskTemplateManagerConfig {
1605+
unsafeConf := clientConf.Copy()
1606+
unsafeConf.TemplateConfig.DisableSandbox = true
1607+
return &TaskTemplateManagerConfig{
1608+
ClientConfig: unsafeConf,
1609+
TaskDir: taskDir.Dir,
1610+
EnvBuilder: containerEnv(),
1611+
Templates: []*structs.Template{
1612+
{
1613+
SourcePath: "${NOMAD_TASK_DIR}/src",
1614+
DestPath: "../escapes",
1615+
},
1616+
},
1617+
}
1618+
},
1619+
SourcePath: filepath.Join(taskDir.Dir, "local/src"),
1620+
DestPath: filepath.Join(taskDir.Dir, "..", "escapes"),
1621+
},
1622+
//TODO: Fix this test. I *think* it should pass. The double
1623+
// joining of the task dir onto the destination seems like
1624+
// a bug. https://github.com/hashicorp/nomad/issues/9389
1625+
{
1626+
Skip: true,
1627+
Name: "RawExecOk",
1628+
Config: func() *TaskTemplateManagerConfig {
1629+
return &TaskTemplateManagerConfig{
1630+
ClientConfig: clientConf,
1631+
TaskDir: taskDir.Dir,
1632+
EnvBuilder: rawExecEnv(),
1633+
Templates: []*structs.Template{
1634+
{
1635+
SourcePath: "${NOMAD_TASK_DIR}/src",
1636+
DestPath: "${NOMAD_SECRETS_DIR}/dst",
1637+
},
1638+
},
1639+
}
1640+
},
1641+
SourcePath: filepath.Join(taskDir.Dir, "local/src"),
1642+
DestPath: filepath.Join(taskDir.Dir, "secrets/dst"),
1643+
},
1644+
{
1645+
Name: "RawExecSrcEscapesErr",
1646+
Config: func() *TaskTemplateManagerConfig {
1647+
return &TaskTemplateManagerConfig{
1648+
ClientConfig: clientConf,
1649+
TaskDir: taskDir.Dir,
1650+
EnvBuilder: rawExecEnv(),
1651+
Templates: []*structs.Template{
1652+
{
1653+
SourcePath: "/etc/src_escapes",
1654+
DestPath: "${NOMAD_SECRETS_DIR}/dst",
1655+
},
1656+
},
1657+
}
1658+
},
1659+
Err: sourceEscapesErr,
1660+
},
1661+
{
1662+
Name: "RawExecDstAbsoluteOk",
1663+
Config: func() *TaskTemplateManagerConfig {
1664+
return &TaskTemplateManagerConfig{
1665+
ClientConfig: clientConf,
1666+
TaskDir: taskDir.Dir,
1667+
EnvBuilder: rawExecEnv(),
1668+
Templates: []*structs.Template{
1669+
{
1670+
SourcePath: "${NOMAD_TASK_DIR}/src",
1671+
DestPath: "/etc/absolutely_relative",
1672+
},
1673+
},
1674+
}
1675+
},
1676+
SourcePath: filepath.Join(taskDir.Dir, "local/src"),
1677+
DestPath: filepath.Join(taskDir.Dir, "etc/absolutely_relative"),
1678+
},
1679+
}
1680+
1681+
for i := range cases {
1682+
tc := cases[i]
1683+
t.Run(tc.Name, func(t *testing.T) {
1684+
if tc.Skip {
1685+
t.Skip("FIXME: Skipping broken test")
1686+
}
1687+
config := tc.Config()
1688+
mapping, err := parseTemplateConfigs(config)
1689+
if tc.Err == nil {
1690+
// Ok path
1691+
require.NoError(t, err)
1692+
require.NotNil(t, mapping)
1693+
require.Len(t, mapping, 1)
1694+
for k := range mapping {
1695+
require.Equal(t, tc.SourcePath, *k.Source)
1696+
require.Equal(t, tc.DestPath, *k.Destination)
1697+
t.Logf("Rendering %s => %s", *k.Source, *k.Destination)
1698+
}
1699+
} else {
1700+
// Err path
1701+
assert.EqualError(t, err, tc.Err.Error())
1702+
require.Nil(t, mapping)
1703+
}
1704+
1705+
})
1706+
}
1707+
}
1708+
14581709
func TestTaskTemplateManager_BlockedEvents(t *testing.T) {
14591710
t.Parallel()
14601711
require := require.New(t)

0 commit comments

Comments
 (0)