Skip to content

Commit

Permalink
fixed some tests
Browse files Browse the repository at this point in the history
  • Loading branch information
cgbaker committed Dec 18, 2020
1 parent 1ad1186 commit f8ef068
Show file tree
Hide file tree
Showing 7 changed files with 76 additions and 28 deletions.
4 changes: 3 additions & 1 deletion client/allocrunner/taskrunner/envoy_version_hook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import (
)

var (
taskEnvDefault = taskenv.NewTaskEnv(nil, nil, map[string]string{
taskEnvDefault = taskenv.NewTaskEnv(nil, nil, nil, map[string]string{
"meta.connect.sidecar_image": envoy.ImageFormat,
"meta.connect.gateway_image": envoy.ImageFormat,
}, "", "")
Expand Down Expand Up @@ -140,6 +140,8 @@ func TestEnvoyVersionHook_interpolateImage(t *testing.T) {
}
hook.interpolateImage(task, taskenv.NewTaskEnv(map[string]string{
"MY_ENVOY": "my/envoy",
}, map[string]string{
"MY_ENVOY": "my/envoy",
}, nil, nil, "", ""))
require.Equal(t, "my/envoy", task.Config["image"])
})
Expand Down
5 changes: 3 additions & 2 deletions client/allocrunner/taskrunner/getter/getter.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package getter

import (
"errors"
"fmt"
"net/http"
"net/url"
Expand Down Expand Up @@ -141,7 +140,9 @@ func GetArtifact(taskEnv EnvReplacer, artifact *structs.TaskArtifact) error {
// Verify the destination is still in the task sandbox after interpolation
if escapes {
return newGetError(artifact.RelativeDest,
errors.New("artifact destination path escapes the alloc directory"), false)
fmt.Errorf("artifact escapes the alloc directory rawDest=%s dest=%s", artifact.RelativeDest, dest),
// errors.New("artifact destination path escapes the alloc directory"),
false)
}

// Convert from string getter mode to go-getter const
Expand Down
71 changes: 50 additions & 21 deletions client/allocrunner/taskrunner/getter/getter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,42 +15,69 @@ import (
"testing"

"github.com/hashicorp/nomad/client/taskenv"
"github.com/hashicorp/nomad/helper"
"github.com/hashicorp/nomad/nomad/mock"
"github.com/hashicorp/nomad/nomad/structs"
"github.com/stretchr/testify/require"
)

// noopReplacer is a noop version of taskenv.TaskEnv.ReplaceEnv.
type noopReplacer struct{}
type noopReplacer struct {
taskDir string
}

func (noopReplacer) ReplaceEnv(s string) string {
return s
}

func (noopReplacer) ReplaceEnvClient(s string) string {
return s
func (r noopReplacer) ReplaceEnvClient(s string) string {
return r.ReplaceEnv(s)
}

func checkEscape(taskDir, testPath string) bool {
if taskDir != "" && !helper.PathEscapesSandbox(taskDir, testPath) {
return false
}
return true
}

func (noopReplacer) ClientPath(s string) (string, bool) {
return s, false
func (r noopReplacer) ClientPath(p string) (string, bool) {
path := r.ReplaceEnv(p)
if !filepath.IsAbs(path) {
path = filepath.Join(r.taskDir, path)
}
path = filepath.Clean(path)
escapes := checkEscape(r.taskDir, path)
return path, escapes
}

var noopTaskEnv = noopReplacer{}
func noopTaskEnv(taskDir string) EnvReplacer {
return noopReplacer{
taskDir: taskDir,
}
}

// upperReplacer is a version of taskenv.TaskEnv.ReplaceEnv that upper-cases
// the given input.
type upperReplacer struct{}
type upperReplacer struct {
taskDir string
}

func (upperReplacer) ReplaceEnv(s string) string {
return strings.ToUpper(s)
}

func (upperReplacer) ReplaceEnvClient(s string) string {
return s
func (u upperReplacer) ReplaceEnvClient(s string) string {
return u.ReplaceEnv(s)
}

func (upperReplacer) ClientPath(s string) (string, bool) {
return s, false
func (u upperReplacer) ClientPath(p string) (string, bool) {
path := u.ReplaceEnv(p)
if !filepath.IsAbs(path) {
path = filepath.Join(u.taskDir, path)
}
escapes := checkEscape(u.taskDir, path)
return path, escapes
}

func removeAllT(t *testing.T, path string) {
Expand All @@ -59,11 +86,11 @@ func removeAllT(t *testing.T, path string) {

func TestGetArtifact_getHeaders(t *testing.T) {
t.Run("nil", func(t *testing.T) {
require.Nil(t, getHeaders(noopTaskEnv, nil))
require.Nil(t, getHeaders(noopTaskEnv(""), nil))
})

t.Run("empty", func(t *testing.T) {
require.Nil(t, getHeaders(noopTaskEnv, make(map[string]string)))
require.Nil(t, getHeaders(noopTaskEnv(""), make(map[string]string)))
})

t.Run("set", func(t *testing.T) {
Expand Down Expand Up @@ -110,7 +137,9 @@ func TestGetArtifact_Headers(t *testing.T) {
}

// Download the artifact.
taskEnv := new(upperReplacer)
taskEnv := upperReplacer{
taskDir: taskDir,
}
err = GetArtifact(taskEnv, artifact)
require.NoError(t, err)

Expand Down Expand Up @@ -145,7 +174,7 @@ func TestGetArtifact_FileAndChecksum(t *testing.T) {
}

// Download the artifact
if err := GetArtifact(noopTaskEnv, artifact); err != nil {
if err := GetArtifact(noopTaskEnv(taskDir), artifact); err != nil {
t.Fatalf("GetArtifact failed: %v", err)
}

Expand Down Expand Up @@ -179,7 +208,7 @@ func TestGetArtifact_File_RelativeDest(t *testing.T) {
}

// Download the artifact
if err := GetArtifact(noopTaskEnv, artifact); err != nil {
if err := GetArtifact(noopTaskEnv(taskDir), artifact); err != nil {
t.Fatalf("GetArtifact failed: %v", err)
}

Expand Down Expand Up @@ -213,7 +242,7 @@ func TestGetArtifact_File_EscapeDest(t *testing.T) {
}

// attempt to download the artifact
err = GetArtifact(noopTaskEnv, artifact)
err = GetArtifact(noopTaskEnv(taskDir), artifact)
if err == nil || !strings.Contains(err.Error(), "escapes") {
t.Fatalf("expected GetArtifact to disallow sandbox escape: %v", err)
}
Expand Down Expand Up @@ -263,7 +292,7 @@ func TestGetArtifact_InvalidChecksum(t *testing.T) {
}

// Download the artifact and expect an error
if err := GetArtifact(noopTaskEnv, artifact); err == nil {
if err := GetArtifact(noopTaskEnv(taskDir), artifact); err == nil {
t.Fatalf("GetArtifact should have failed")
}
}
Expand Down Expand Up @@ -328,7 +357,7 @@ func TestGetArtifact_Archive(t *testing.T) {
},
}

if err := GetArtifact(noopTaskEnv, artifact); err != nil {
if err := GetArtifact(noopTaskEnv(taskDir), artifact); err != nil {
t.Fatalf("GetArtifact failed: %v", err)
}

Expand Down Expand Up @@ -361,7 +390,7 @@ func TestGetArtifact_Setuid(t *testing.T) {
},
}

require.NoError(t, GetArtifact(noopTaskEnv, artifact))
require.NoError(t, GetArtifact(noopTaskEnv(taskDir), artifact))

var expected map[string]int

Expand Down Expand Up @@ -486,7 +515,7 @@ func TestGetGetterUrl_Queries(t *testing.T) {

for _, c := range cases {
t.Run(c.name, func(t *testing.T) {
act, err := getGetterUrl(noopTaskEnv, c.artifact)
act, err := getGetterUrl(noopTaskEnv(""), c.artifact)
if err != nil {
t.Fatalf("want %q; got err %v", c.output, err)
} else if act != c.output {
Expand Down
1 change: 1 addition & 0 deletions client/allocrunner/taskrunner/task_dir_hook.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ func (h *taskDirHook) Prestart(ctx context.Context, req *interfaces.TaskPrestart
// setEnvvars sets path and host env vars depending on the FS isolation used.
func setEnvvars(envBuilder *taskenv.Builder, fsi drivers.FSIsolation, taskDir *allocdir.TaskDir, conf *cconfig.Config) {

envBuilder.SetClientTaskDir(taskDir.Dir)
envBuilder.SetClientAllocDir(taskDir.SharedAllocDir)
envBuilder.SetClientTaskLocalDir(taskDir.LocalDir)
envBuilder.SetClientSecretDir(taskDir.SecretsDir)
Expand Down
12 changes: 11 additions & 1 deletion client/taskenv/env.go
Original file line number Diff line number Diff line change
Expand Up @@ -376,6 +376,9 @@ type Builder struct {
// clientSecrets is the secrets dir from the client's perspective; eg <data-dir>/allocs/<task>/secrets
clientSecretsDir string

// clientTaskDir is the task working directory from the client's perspective; eg <data-dir>/allocs/<task>
clientTaskDir string

cpuLimit int64
memLimit int64
taskName string
Expand Down Expand Up @@ -595,7 +598,7 @@ func (b *Builder) Build() *TaskEnv {
envMap, deviceEnvs := b.buildEnv(b.allocDir, b.localDir, b.secretsDir, nodeAttrs)
envMapClient, _ := b.buildEnv(b.clientAllocDir, b.clientLocalDir, b.clientSecretsDir, nodeAttrs)

return NewTaskEnv(envMap, envMapClient, deviceEnvs, nodeAttrs, b.clientLocalDir, b.clientAllocDir)
return NewTaskEnv(envMap, envMapClient, deviceEnvs, nodeAttrs, b.clientTaskDir, b.clientAllocDir)
}

// Update task updates the environment based on a new alloc and task.
Expand Down Expand Up @@ -806,6 +809,13 @@ func (b *Builder) SetClientAllocDir(dir string) *Builder {
return b
}

func (b *Builder) SetClientTaskDir(dir string) *Builder {
b.mu.Lock()
b.clientTaskDir = dir
b.mu.Unlock()
return b
}

func (b *Builder) SetClientTaskLocalDir(dir string) *Builder {
b.mu.Lock()
b.clientLocalDir = dir
Expand Down
10 changes: 7 additions & 3 deletions client/taskenv/services_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,10 @@ func TestInterpolateServices(t *testing.T) {
require.Equal(t, exp, interpolated)
}

var testEnv = NewTaskEnv(map[string]string{"foo": "bar", "baz": "blah"}, nil, nil, "", "")
var testEnv = NewTaskEnv(
map[string]string{"foo": "bar", "baz": "blah"},
map[string]string{"foo": "bar", "baz": "blah"},
nil, nil, "", "")

func TestInterpolate_interpolateMapStringSliceString(t *testing.T) {
t.Parallel()
Expand Down Expand Up @@ -160,7 +163,7 @@ func TestInterpolate_interpolateMapStringInterface(t *testing.T) {
func TestInterpolate_interpolateConnect(t *testing.T) {
t.Parallel()

env := NewTaskEnv(map[string]string{
e := map[string]string{
"tag1": "_tag1",
"port1": "12345",
"address1": "1.2.3.4",
Expand Down Expand Up @@ -196,7 +199,8 @@ func TestInterpolate_interpolateConnect(t *testing.T) {
"protocol2": "_protocol2",
"service1": "_service1",
"host1": "_host1",
}, nil, nil, "", "")
}
env := NewTaskEnv(e, e, nil, nil, "", "")

connect := &structs.ConsulConnect{
Native: false,
Expand Down
1 change: 1 addition & 0 deletions plugins/drivers/testutils/testing.go
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,7 @@ func (d *MockDriver) ExecTaskStreaming(ctx context.Context, taskID string, execO
// SetEnvvars sets path and host env vars depending on the FS isolation used.
func SetEnvvars(envBuilder *taskenv.Builder, fsi drivers.FSIsolation, taskDir *allocdir.TaskDir, conf *config.Config) {

envBuilder.SetClientTaskDir(taskDir.Dir)
envBuilder.SetClientAllocDir(taskDir.SharedAllocDir)
envBuilder.SetClientTaskLocalDir(taskDir.LocalDir)
envBuilder.SetClientSecretDir(taskDir.SecretsDir)
Expand Down

0 comments on commit f8ef068

Please sign in to comment.