diff --git a/api/tasks.go b/api/tasks.go index d09339f34..84765c71a 100644 --- a/api/tasks.go +++ b/api/tasks.go @@ -99,6 +99,7 @@ type Task struct { type TaskArtifact struct { GetterSource string GetterOptions map[string]string + RelativeDest string } // NewTask creates and initializes a new Task. diff --git a/jobspec/parse.go b/jobspec/parse.go index 853640f75..1baf29cf6 100644 --- a/jobspec/parse.go +++ b/jobspec/parse.go @@ -617,6 +617,7 @@ func parseArtifacts(result *[]*structs.TaskArtifact, list *ast.ObjectList) error valid := []string{ "source", "options", + "destination", } if err := checkHCLKeys(o.Val, valid); err != nil { return err @@ -629,6 +630,11 @@ func parseArtifacts(result *[]*structs.TaskArtifact, list *ast.ObjectList) error delete(m, "options") + // Default to downloading to the local directory. + if _, ok := m["destination"]; !ok { + m["destination"] = "local/" + } + var ta structs.TaskArtifact if err := mapstructure.WeakDecode(m, &ta); err != nil { return err diff --git a/jobspec/parse_test.go b/jobspec/parse_test.go index 28cc6afe8..4616db771 100644 --- a/jobspec/parse_test.go +++ b/jobspec/parse_test.go @@ -131,12 +131,14 @@ func TestParse(t *testing.T) { Artifacts: []*structs.TaskArtifact{ { GetterSource: "http://foo.com/artifact", + RelativeDest: "local/", GetterOptions: map[string]string{ "checksum": "md5:b8a4f3f72ecab0510a6a31e997461c5f", }, }, { GetterSource: "http://bar.com/artifact", + RelativeDest: "local/", GetterOptions: map[string]string{ "checksum": "md5:ff1cc0d3432dad54d607c1505fb7245c", }, @@ -320,6 +322,58 @@ func TestParse(t *testing.T) { nil, true, }, + + { + "artifacts.hcl", + &structs.Job{ + ID: "binstore-storagelocker", + Name: "binstore-storagelocker", + Type: "service", + Priority: 50, + Region: "global", + + TaskGroups: []*structs.TaskGroup{ + &structs.TaskGroup{ + Name: "binsl", + Count: 1, + Tasks: []*structs.Task{ + &structs.Task{ + Name: "binstore", + Driver: "docker", + Resources: &structs.Resources{ + CPU: 100, + MemoryMB: 10, + DiskMB: 300, + IOPS: 0, + }, + LogConfig: &structs.LogConfig{ + MaxFiles: 10, + MaxFileSizeMB: 10, + }, + Artifacts: []*structs.TaskArtifact{ + { + GetterSource: "http://foo.com/bar", + GetterOptions: map[string]string{}, + RelativeDest: "", + }, + { + GetterSource: "http://foo.com/baz", + GetterOptions: map[string]string{}, + RelativeDest: "local/", + }, + { + GetterSource: "http://foo.com/bam", + GetterOptions: map[string]string{}, + RelativeDest: "var/foo", + }, + }, + }, + }, + }, + }, + }, + false, + }, } for _, tc := range cases { diff --git a/jobspec/test-fixtures/artifacts.hcl b/jobspec/test-fixtures/artifacts.hcl new file mode 100644 index 000000000..361e8dc61 --- /dev/null +++ b/jobspec/test-fixtures/artifacts.hcl @@ -0,0 +1,21 @@ +job "binstore-storagelocker" { + group "binsl" { + task "binstore" { + driver = "docker" + + artifact { + source = "http://foo.com/bar" + destination = "" + } + + artifact { + source = "http://foo.com/baz" + } + artifact { + source = "http://foo.com/bam" + destination = "var/foo" + } + resources {} + } + } +} diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index 21153f9f6..a3fad3ee5 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -11,6 +11,7 @@ import ( "fmt" "io" "net/url" + "path/filepath" "reflect" "regexp" "strconv" @@ -1913,6 +1914,10 @@ type TaskArtifact struct { // GetterOptions are options to use when downloading the artifact using // go-getter. GetterOptions map[string]string `mapstructure:"options"` + + // RelativeDest is the download destination given relative to the task's + // directory. + RelativeDest string `mapstructure:"destination"` } func (ta *TaskArtifact) Copy() *TaskArtifact { @@ -1925,16 +1930,36 @@ func (ta *TaskArtifact) Copy() *TaskArtifact { return nta } +func (ta *TaskArtifact) GoString() string { + return fmt.Sprintf("%+v", ta) +} + func (ta *TaskArtifact) Validate() error { // Verify the source var mErr multierror.Error if ta.GetterSource == "" { mErr.Errors = append(mErr.Errors, fmt.Errorf("source must be specified")) + } else { + _, err := url.Parse(ta.GetterSource) + if err != nil { + mErr.Errors = append(mErr.Errors, fmt.Errorf("invalid source URL %q: %v", ta.GetterSource, err)) + } } - _, err := url.Parse(ta.GetterSource) + // Verify the destination doesn't escape the tasks directory + alloc := "/foo/bar/" + abs, err := filepath.Abs(filepath.Join(alloc, ta.RelativeDest)) if err != nil { - mErr.Errors = append(mErr.Errors, fmt.Errorf("invalid source URL %q: %v", ta.GetterSource, err)) + mErr.Errors = append(mErr.Errors, err) + return mErr.ErrorOrNil() + } + rel, err := filepath.Rel(alloc, abs) + if err != nil { + mErr.Errors = append(mErr.Errors, err) + return mErr.ErrorOrNil() + } + if strings.HasPrefix(rel, "..") { + mErr.Errors = append(mErr.Errors, fmt.Errorf("destination escapes task's directory")) } // Verify the checksum diff --git a/nomad/structs/structs_test.go b/nomad/structs/structs_test.go index 1b2ce9517..04f96635d 100644 --- a/nomad/structs/structs_test.go +++ b/nomad/structs/structs_test.go @@ -777,6 +777,28 @@ func TestTaskArtifact_Validate_Source(t *testing.T) { } } +func TestTaskArtifact_Validate_Dest(t *testing.T) { + valid := &TaskArtifact{GetterSource: "google.com"} + if err := valid.Validate(); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + valid.RelativeDest = "local/" + if err := valid.Validate(); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + valid.RelativeDest = "local/.." + if err := valid.Validate(); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + valid.RelativeDest = "local/../.." + if err := valid.Validate(); err == nil { + t.Fatalf("expected error: %v", err) + } +} + func TestTaskArtifact_Validate_Checksum(t *testing.T) { cases := []struct { Input *TaskArtifact