Skip to content

Commit

Permalink
Support insecure flag on artifact (#20126)
Browse files Browse the repository at this point in the history
  • Loading branch information
amirabbas8 authored and philrenaud committed Apr 18, 2024
1 parent 0787d9c commit 005be79
Show file tree
Hide file tree
Showing 12 changed files with 97 additions and 18 deletions.
3 changes: 3 additions & 0 deletions .changelog/20126.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
artifact: Added support for downloading artifacts without validating the TLS certificate
```
14 changes: 9 additions & 5 deletions api/tasks.go
Original file line number Diff line number Diff line change
Expand Up @@ -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("")
Expand Down
1 change: 1 addition & 0 deletions api/tasks_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
7 changes: 4 additions & 3 deletions client/allocrunner/taskrunner/getter/params.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (
"strings"
"time"

"github.com/hashicorp/go-cleanhttp"
"github.com/hashicorp/go-getter"
)

Expand All @@ -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"`
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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{
Expand Down
1 change: 1 addition & 0 deletions client/allocrunner/taskrunner/getter/params_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down
2 changes: 2 additions & 0 deletions client/allocrunner/taskrunner/getter/sandbox.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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,
Expand Down
31 changes: 31 additions & 0 deletions client/allocrunner/taskrunner/getter/sandbox_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
package getter

import (
"net/http"
"net/http/httptest"
"os"
"path/filepath"
"testing"
Expand Down Expand Up @@ -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)
}
4 changes: 4 additions & 0 deletions client/allocrunner/taskrunner/getter/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
11 changes: 6 additions & 5 deletions command/agent/job_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
})
}
}
Expand Down
13 changes: 13 additions & 0 deletions nomad/structs/diff_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5625,6 +5625,12 @@ func TestTaskDiff(t *testing.T) {
Old: "",
New: "user2",
},
{
Type: DiffTypeAdded,
Name: "GetterInsecure",
Old: "",
New: "false",
},
{
Type: DiffTypeAdded,
Name: "GetterMode",
Expand Down Expand Up @@ -5661,6 +5667,13 @@ func TestTaskDiff(t *testing.T) {
Old: "user1",
New: "",
},

{
Type: DiffTypeDeleted,
Name: "GetterInsecure",
Old: "false",
New: "",
},
{
Type: DiffTypeDeleted,
Name: "GetterMode",
Expand Down
18 changes: 13 additions & 5 deletions nomad/structs/structs.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
}
Expand All @@ -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,
}
}

Expand Down Expand Up @@ -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))
}
Expand Down
10 changes: 10 additions & 0 deletions nomad/structs/structs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 005be79

Please sign in to comment.