From 3aac211a8392023723637fb46f1389aab08c5b4a Mon Sep 17 00:00:00 2001 From: Amir Abbas Date: Tue, 12 Mar 2024 23:14:17 +0330 Subject: [PATCH] Support insecure flag on artifact --- .changelog/20126.txt | 3 ++ api/tasks.go | 14 ++++++--- api/tasks_test.go | 1 + .../allocrunner/taskrunner/getter/params.go | 7 +++-- .../taskrunner/getter/params_test.go | 1 + .../allocrunner/taskrunner/getter/sandbox.go | 2 ++ .../taskrunner/getter/sandbox_test.go | 31 +++++++++++++++++++ client/allocrunner/taskrunner/getter/util.go | 4 +++ command/agent/job_endpoint.go | 11 ++++--- nomad/structs/diff_test.go | 13 ++++++++ nomad/structs/structs.go | 18 ++++++++--- nomad/structs/structs_test.go | 10 ++++++ 12 files changed, 97 insertions(+), 18 deletions(-) create mode 100644 .changelog/20126.txt diff --git a/.changelog/20126.txt b/.changelog/20126.txt new file mode 100644 index 00000000000..08efed5a00b --- /dev/null +++ b/.changelog/20126.txt @@ -0,0 +1,3 @@ +```release-note:improvement +artifact: Added support for downloading artifacts without validating the TLS certificate +``` diff --git a/api/tasks.go b/api/tasks.go index db94b76a915..e9a574cee32 100644 --- a/api/tasks.go +++ b/api/tasks.go @@ -856,17 +856,21 @@ func (t *Task) Canonicalize(tg *TaskGroup, job *Job) { // TaskArtifact is used to download artifacts before running a task. type TaskArtifact struct { - GetterSource *string `mapstructure:"source" hcl:"source,optional"` - GetterOptions map[string]string `mapstructure:"options" hcl:"options,block"` - GetterHeaders map[string]string `mapstructure:"headers" hcl:"headers,block"` - GetterMode *string `mapstructure:"mode" hcl:"mode,optional"` - RelativeDest *string `mapstructure:"destination" hcl:"destination,optional"` + GetterSource *string `mapstructure:"source" hcl:"source,optional"` + GetterOptions map[string]string `mapstructure:"options" hcl:"options,block"` + GetterHeaders map[string]string `mapstructure:"headers" hcl:"headers,block"` + GetterMode *string `mapstructure:"mode" hcl:"mode,optional"` + GetterInsecure *bool `mapstructure:"insecure" hcl:"insecure,optional"` + RelativeDest *string `mapstructure:"destination" hcl:"destination,optional"` } func (a *TaskArtifact) Canonicalize() { if a.GetterMode == nil { a.GetterMode = pointerOf("any") } + if a.GetterInsecure == nil { + a.GetterInsecure = pointerOf(false) + } if a.GetterSource == nil { // Shouldn't be possible, but we don't want to panic a.GetterSource = pointerOf("") diff --git a/api/tasks_test.go b/api/tasks_test.go index 78a6cbf618a..e98d6ef1d1c 100644 --- a/api/tasks_test.go +++ b/api/tasks_test.go @@ -317,6 +317,7 @@ func TestTask_Artifact(t *testing.T) { } a.Canonicalize() must.Eq(t, "file", *a.GetterMode) + must.Eq(t, false, *a.GetterInsecure) must.Eq(t, "local/foo.txt", filepath.ToSlash(*a.RelativeDest)) must.Nil(t, a.GetterOptions) must.Nil(t, a.GetterHeaders) diff --git a/client/allocrunner/taskrunner/getter/params.go b/client/allocrunner/taskrunner/getter/params.go index 1daf30093e5..51a7b45a267 100644 --- a/client/allocrunner/taskrunner/getter/params.go +++ b/client/allocrunner/taskrunner/getter/params.go @@ -13,7 +13,6 @@ import ( "strings" "time" - "github.com/hashicorp/go-cleanhttp" "github.com/hashicorp/go-getter" ) @@ -36,6 +35,7 @@ type parameters struct { // Artifact Mode getter.ClientMode `json:"artifact_mode"` + Insecure bool `json:"artifact_insecure"` Source string `json:"artifact_source"` Destination string `json:"artifact_destination"` Headers map[string][]string `json:"artifact_headers"` @@ -102,6 +102,8 @@ func (p *parameters) Equal(o *parameters) bool { return false case p.Mode != o.Mode: return false + case p.Insecure != o.Insecure: + return false case p.Source != o.Source: return false case p.Destination != o.Destination: @@ -130,7 +132,6 @@ const ( func (p *parameters) client(ctx context.Context) *getter.Client { httpGetter := &getter.HttpGetter{ Netrc: true, - Client: cleanhttp.DefaultClient(), Header: p.Headers, // Do not support the custom X-Terraform-Get header and @@ -162,8 +163,8 @@ func (p *parameters) client(ctx context.Context) *getter.Client { Src: p.Source, Dst: p.Destination, Mode: p.Mode, + Insecure: p.Insecure, Umask: umask, - Insecure: false, DisableSymlinks: true, Decompressors: decompressors, Getters: map[string]getter.Getter{ diff --git a/client/allocrunner/taskrunner/getter/params_test.go b/client/allocrunner/taskrunner/getter/params_test.go index 386b8b1ff76..c262bb3a86e 100644 --- a/client/allocrunner/taskrunner/getter/params_test.go +++ b/client/allocrunner/taskrunner/getter/params_test.go @@ -27,6 +27,7 @@ const paramsAsJSON = ` "disable_filesystem_isolation": true, "set_environment_variables": "", "artifact_mode": 2, + "artifact_insecure": false, "artifact_source": "https://example.com/file.txt", "artifact_destination": "local/out.txt", "artifact_headers": { diff --git a/client/allocrunner/taskrunner/getter/sandbox.go b/client/allocrunner/taskrunner/getter/sandbox.go index c114a11bf92..fdaca51b56c 100644 --- a/client/allocrunner/taskrunner/getter/sandbox.go +++ b/client/allocrunner/taskrunner/getter/sandbox.go @@ -38,6 +38,7 @@ func (s *Sandbox) Get(env interfaces.EnvReplacer, artifact *structs.TaskArtifact } mode := getMode(artifact) + insecure := isInsecure(artifact) headers := getHeaders(env, artifact) allocDir, taskDir := getWritableDirs(env) @@ -56,6 +57,7 @@ func (s *Sandbox) Get(env interfaces.EnvReplacer, artifact *structs.TaskArtifact // artifact configuration Mode: mode, + Insecure: insecure, Source: source, Destination: destination, Headers: headers, diff --git a/client/allocrunner/taskrunner/getter/sandbox_test.go b/client/allocrunner/taskrunner/getter/sandbox_test.go index 6028d211cd6..7906c7668fa 100644 --- a/client/allocrunner/taskrunner/getter/sandbox_test.go +++ b/client/allocrunner/taskrunner/getter/sandbox_test.go @@ -4,6 +4,8 @@ package getter import ( + "net/http" + "net/http/httptest" "os" "path/filepath" "testing" @@ -51,3 +53,32 @@ func TestSandbox_Get_http(t *testing.T) { must.NoError(t, err) must.StrContains(t, string(b), "module github.com/hashicorp/go-set") } + +func TestSandbox_Get_insecure_http(t *testing.T) { + testutil.RequireRoot(t) + logger := testlog.HCLogger(t) + + ac := artifactConfig(10 * time.Second) + sbox := New(ac, logger) + + _, taskDir := SetupDir(t) + env := noopTaskEnv(taskDir) + + srv := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + })) + defer srv.Close() + + artifact := &structs.TaskArtifact{ + GetterSource: srv.URL, + RelativeDest: "local/downloads", + } + + err := sbox.Get(env, artifact) + must.Error(t, err) + must.StrContains(t, err.Error(), "x509: certificate signed by unknown authority") + + artifact.GetterInsecure = true + err = sbox.Get(env, artifact) + must.NoError(t, err) +} diff --git a/client/allocrunner/taskrunner/getter/util.go b/client/allocrunner/taskrunner/getter/util.go index 04bdb3b38f5..afe961fe83c 100644 --- a/client/allocrunner/taskrunner/getter/util.go +++ b/client/allocrunner/taskrunner/getter/util.go @@ -84,6 +84,10 @@ func getMode(artifact *structs.TaskArtifact) getter.ClientMode { } } +func isInsecure(artifact *structs.TaskArtifact) bool { + return artifact.GetterInsecure +} + func getHeaders(env interfaces.EnvReplacer, artifact *structs.TaskArtifact) map[string][]string { m := artifact.GetterHeaders if len(m) == 0 { diff --git a/command/agent/job_endpoint.go b/command/agent/job_endpoint.go index 04290a1e725..33770122695 100644 --- a/command/agent/job_endpoint.go +++ b/command/agent/job_endpoint.go @@ -1355,11 +1355,12 @@ func ApiTaskToStructsTask(job *structs.Job, group *structs.TaskGroup, for _, ta := range apiTask.Artifacts { structsTask.Artifacts = append(structsTask.Artifacts, &structs.TaskArtifact{ - GetterSource: *ta.GetterSource, - GetterOptions: maps.Clone(ta.GetterOptions), - GetterHeaders: maps.Clone(ta.GetterHeaders), - GetterMode: *ta.GetterMode, - RelativeDest: *ta.RelativeDest, + GetterSource: *ta.GetterSource, + GetterOptions: maps.Clone(ta.GetterOptions), + GetterHeaders: maps.Clone(ta.GetterHeaders), + GetterMode: *ta.GetterMode, + GetterInsecure: *ta.GetterInsecure, + RelativeDest: *ta.RelativeDest, }) } } diff --git a/nomad/structs/diff_test.go b/nomad/structs/diff_test.go index d123b69f777..71c598731c7 100644 --- a/nomad/structs/diff_test.go +++ b/nomad/structs/diff_test.go @@ -5625,6 +5625,12 @@ func TestTaskDiff(t *testing.T) { Old: "", New: "user2", }, + { + Type: DiffTypeAdded, + Name: "GetterInsecure", + Old: "", + New: "false", + }, { Type: DiffTypeAdded, Name: "GetterMode", @@ -5661,6 +5667,13 @@ func TestTaskDiff(t *testing.T) { Old: "user1", New: "", }, + + { + Type: DiffTypeDeleted, + Name: "GetterInsecure", + Old: "false", + New: "", + }, { Type: DiffTypeDeleted, Name: "GetterMode", diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index 45b73d6d423..418ebb5aca9 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -9449,6 +9449,10 @@ type TaskArtifact struct { // Defaults to "any" but can be set to "file" or "dir". GetterMode string + // GetterInsecure is a flag to disable SSL certificate verification when + // downloading the artifact using go-getter. + GetterInsecure bool + // RelativeDest is the download destination given relative to the task's // directory. RelativeDest string @@ -9467,6 +9471,8 @@ func (ta *TaskArtifact) Equal(o *TaskArtifact) bool { return false case ta.GetterMode != o.GetterMode: return false + case ta.GetterInsecure != o.GetterInsecure: + return false case ta.RelativeDest != o.RelativeDest: return false } @@ -9478,11 +9484,12 @@ func (ta *TaskArtifact) Copy() *TaskArtifact { return nil } return &TaskArtifact{ - GetterSource: ta.GetterSource, - GetterOptions: maps.Clone(ta.GetterOptions), - GetterHeaders: maps.Clone(ta.GetterHeaders), - GetterMode: ta.GetterMode, - RelativeDest: ta.RelativeDest, + GetterSource: ta.GetterSource, + GetterOptions: maps.Clone(ta.GetterOptions), + GetterHeaders: maps.Clone(ta.GetterHeaders), + GetterMode: ta.GetterMode, + GetterInsecure: ta.GetterInsecure, + RelativeDest: ta.RelativeDest, } } @@ -9522,6 +9529,7 @@ func (ta *TaskArtifact) Hash() string { hashStringMap(h, ta.GetterHeaders) _, _ = h.Write([]byte(ta.GetterMode)) + _, _ = h.Write([]byte(strconv.FormatBool(ta.GetterInsecure))) _, _ = h.Write([]byte(ta.RelativeDest)) return base64.RawStdEncoding.EncodeToString(h.Sum(nil)) } diff --git a/nomad/structs/structs_test.go b/nomad/structs/structs_test.go index 6188c183022..b1584c1996f 100644 --- a/nomad/structs/structs_test.go +++ b/nomad/structs/structs_test.go @@ -4785,6 +4785,16 @@ func TestTaskArtifact_Hash(t *testing.T) { GetterMode: "g", RelativeDest: "i", }, + { + GetterSource: "b", + GetterOptions: map[string]string{ + "c": "c", + "d": "e", + }, + GetterMode: "g", + GetterInsecure: true, + RelativeDest: "i", + }, } // Map of hash to source