Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

jobspec: add a chown option to artifact block #24157

Merged
merged 2 commits into from
Oct 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .changelog/24157.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
getter: Added option to chown artifact(s) to task user
```
1 change: 1 addition & 0 deletions api/tasks.go
Original file line number Diff line number Diff line change
Expand Up @@ -864,6 +864,7 @@ type TaskArtifact struct {
GetterMode *string `mapstructure:"mode" hcl:"mode,optional"`
GetterInsecure *bool `mapstructure:"insecure" hcl:"insecure,optional"`
RelativeDest *string `mapstructure:"destination" hcl:"destination,optional"`
Chown bool `mapstructure:"chown" hcl:"chown,optional"`
}

func (a *TaskArtifact) Canonicalize() {
Expand Down
1 change: 1 addition & 0 deletions api/tasks_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,7 @@ func TestTask_Artifact(t *testing.T) {
must.Eq(t, "local/foo.txt", filepath.ToSlash(*a.RelativeDest))
must.Nil(t, a.GetterOptions)
must.Nil(t, a.GetterHeaders)
must.Eq(t, false, a.Chown)
}

func TestTask_VolumeMount(t *testing.T) {
Expand Down
11 changes: 9 additions & 2 deletions client/allocrunner/taskrunner/artifact_hook.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,14 @@ func newArtifactHook(e ti.EventEmitter, getter ci.ArtifactGetter, logger log.Log
return h
}

func (h *artifactHook) doWork(req *interfaces.TaskPrestartRequest, resp *interfaces.TaskPrestartResponse, jobs chan *structs.TaskArtifact, errorChannel chan error, wg *sync.WaitGroup, responseStateMutex *sync.Mutex) {
func (h *artifactHook) doWork(
req *interfaces.TaskPrestartRequest,
resp *interfaces.TaskPrestartResponse,
jobs chan *structs.TaskArtifact,
errorChannel chan error,
wg *sync.WaitGroup,
responseStateMutex *sync.Mutex,
) {
defer wg.Done()
for artifact := range jobs {
aid := artifact.Hash()
Expand All @@ -45,7 +52,7 @@ func (h *artifactHook) doWork(req *interfaces.TaskPrestartRequest, resp *interfa

h.logger.Debug("downloading artifact", "artifact", artifact.GetterSource, "aid", aid)

if err := h.getter.Get(req.TaskEnv, artifact); err != nil {
if err := h.getter.Get(req.TaskEnv, artifact, req.Task.User); err != nil {
wrapped := structs.NewRecoverableError(
fmt.Errorf("failed to download artifact %q: %v", artifact.GetterSource, err),
true,
Expand Down
2 changes: 2 additions & 0 deletions client/allocrunner/taskrunner/getter/params.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ type parameters struct {
// Task Filesystem
AllocDir string `json:"alloc_dir"`
TaskDir string `json:"task_dir"`
User string `json:"user"`
Chown bool `json:"chown"`
}

func (p *parameters) reader() io.Reader {
Expand Down
6 changes: 5 additions & 1 deletion client/allocrunner/taskrunner/getter/params_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,9 @@ const paramsAsJSON = `
"X-Nomad-Artifact": ["hi"]
},
"alloc_dir": "/path/to/alloc",
"task_dir": "/path/to/alloc/task"
"task_dir": "/path/to/alloc/task",
"chown": true,
"user":"nobody"
}`

var paramsAsStruct = &parameters{
Expand All @@ -65,6 +67,8 @@ var paramsAsStruct = &parameters{
Headers: map[string][]string{
"X-Nomad-Artifact": {"hi"},
},
User: "nobody",
Chown: true,
}

func TestParameters_reader(t *testing.T) {
Expand Down
7 changes: 5 additions & 2 deletions client/allocrunner/taskrunner/getter/sandbox.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ type Sandbox struct {
ac *config.ArtifactConfig
}

func (s *Sandbox) Get(env interfaces.EnvReplacer, artifact *structs.TaskArtifact) error {
s.logger.Debug("get", "source", artifact.GetterSource, "destination", artifact.RelativeDest)
func (s *Sandbox) Get(env interfaces.EnvReplacer, artifact *structs.TaskArtifact, user string) error {
s.logger.Debug("get", "source", artifact.GetterSource, "destination", artifact.RelativeDest, "user", user)

source, err := getURL(env, artifact)
if err != nil {
Expand Down Expand Up @@ -66,10 +66,13 @@ func (s *Sandbox) Get(env interfaces.EnvReplacer, artifact *structs.TaskArtifact
// task filesystem
AllocDir: allocDir,
TaskDir: taskDir,
User: user,
Chown: artifact.Chown,
}

if err = s.runCmd(params); err != nil {
return err
}

return nil
}
33 changes: 30 additions & 3 deletions client/allocrunner/taskrunner/getter/sandbox_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"net/http/httptest"
"os"
"path/filepath"
"syscall"
"testing"
"time"

Expand Down Expand Up @@ -46,7 +47,7 @@ func TestSandbox_Get_http(t *testing.T) {
RelativeDest: "local/downloads",
}

err := sbox.Get(env, artifact)
err := sbox.Get(env, artifact, "nobody")
must.NoError(t, err)

b, err := os.ReadFile(filepath.Join(taskDir, "local", "downloads", "go.mod"))
Expand Down Expand Up @@ -74,11 +75,37 @@ func TestSandbox_Get_insecure_http(t *testing.T) {
RelativeDest: "local/downloads",
}

err := sbox.Get(env, artifact)
err := sbox.Get(env, artifact, "nobody")
must.Error(t, err)
must.StrContains(t, err.Error(), "x509: certificate signed by unknown authority")

artifact.GetterInsecure = true
err = sbox.Get(env, artifact)
err = sbox.Get(env, artifact, "nobody")
must.NoError(t, err)
}

func TestSandbox_Get_chown(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)

artifact := &structs.TaskArtifact{
GetterSource: "https://raw.githubusercontent.com/hashicorp/go-set/main/go.mod",
RelativeDest: "local/downloads",
Chown: true,
}

err := sbox.Get(env, artifact, "nobody")
must.NoError(t, err)

info, err := os.Stat(filepath.Join(taskDir, "local", "downloads"))
must.NoError(t, err)

uid := info.Sys().(*syscall.Stat_t).Uid
must.Eq(t, 65534, uid) // nobody's conventional uid
}
28 changes: 28 additions & 0 deletions client/allocrunner/taskrunner/getter/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,15 @@ import (
"os"
"os/exec"
"path/filepath"
"runtime"
"sort"
"strings"
"unicode"

"github.com/hashicorp/go-getter"
"github.com/hashicorp/nomad/client/interfaces"
"github.com/hashicorp/nomad/helper/subproc"
"github.com/hashicorp/nomad/helper/users"
"github.com/hashicorp/nomad/nomad/structs"
)

Expand Down Expand Up @@ -84,6 +86,32 @@ func getMode(artifact *structs.TaskArtifact) getter.ClientMode {
}
}

func chownDestination(destination, username string) error {
if destination == "" || username == "" {
return nil
}

if os.Geteuid() != 0 {
return nil
}

if runtime.GOOS == "windows" {
return nil
}

uid, gid, _, err := users.LookupUnix(username)
if err != nil {
return err
}

return filepath.Walk(destination, func(path string, info os.FileInfo, err error) error {
if err != nil {
return err
}
return os.Chown(path, uid, gid)
})
}

func isInsecure(artifact *structs.TaskArtifact) bool {
return artifact.GetterInsecure
}
Expand Down
10 changes: 10 additions & 0 deletions client/allocrunner/taskrunner/getter/z_getter_cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,16 @@ func init() {
return subproc.ExitFailure
}

// chown the resulting artifact to the task user, but only if configured
// to do so in the artifact block (for compatibility)
if env.Chown {
err := chownDestination(env.Destination, env.User)
if err != nil {
subproc.Print("failed to chown artifact: %v", err)
return subproc.ExitFailure
}
}

subproc.Print("artifact download was a success")
return subproc.ExitSuccess
})
Expand Down
2 changes: 1 addition & 1 deletion client/interfaces/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ type EnvReplacer interface {
// ArtifactGetter is an interface satisfied by the getter package.
type ArtifactGetter interface {
// Get artifact and put it in the task directory.
Get(EnvReplacer, *structs.TaskArtifact) error
Get(EnvReplacer, *structs.TaskArtifact, string) error
}

// ProcessWranglers is an interface satisfied by the proclib package.
Expand Down
1 change: 1 addition & 0 deletions command/agent/job_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -1426,6 +1426,7 @@ func ApiTaskToStructsTask(job *structs.Job, group *structs.TaskGroup,
GetterMode: *ta.GetterMode,
GetterInsecure: *ta.GetterInsecure,
RelativeDest: *ta.RelativeDest,
Chown: ta.Chown,
})
}
}
Expand Down
2 changes: 2 additions & 0 deletions command/agent/job_endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2924,6 +2924,7 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) {
},
GetterMode: pointer.Of("dir"),
RelativeDest: pointer.Of("dest"),
Chown: true,
},
},
Vault: &api.Vault{
Expand Down Expand Up @@ -3371,6 +3372,7 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) {
},
GetterMode: "dir",
RelativeDest: "dest",
Chown: true,
},
},
Vault: &structs.Vault{
Expand Down
15 changes: 14 additions & 1 deletion nomad/structs/diff_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6059,6 +6059,7 @@ func TestTaskDiff(t *testing.T) {
},
GetterMode: "dir",
RelativeDest: "bar",
Chown: false,
},
},
},
Expand All @@ -6082,6 +6083,7 @@ func TestTaskDiff(t *testing.T) {
},
GetterMode: "file",
RelativeDest: "bam",
Chown: true,
},
},
},
Expand All @@ -6104,6 +6106,12 @@ func TestTaskDiff(t *testing.T) {
Type: DiffTypeAdded,
Name: "Artifact",
Fields: []*FieldDiff{
{
Type: DiffTypeAdded,
Name: "Chown",
Old: "",
New: "true",
},
{
Type: DiffTypeAdded,
Name: "GetterHeaders[User-Agent]",
Expand Down Expand Up @@ -6152,13 +6160,18 @@ func TestTaskDiff(t *testing.T) {
Type: DiffTypeDeleted,
Name: "Artifact",
Fields: []*FieldDiff{
{
Type: DiffTypeDeleted,
Name: "Chown",
Old: "false",
New: "",
},
{
Type: DiffTypeDeleted,
Name: "GetterHeaders[User]",
Old: "user1",
New: "",
},

{
Type: DiffTypeDeleted,
Name: "GetterInsecure",
Expand Down
9 changes: 9 additions & 0 deletions nomad/structs/structs.go
Original file line number Diff line number Diff line change
Expand Up @@ -9807,6 +9807,11 @@ type TaskArtifact struct {
// RelativeDest is the download destination given relative to the task's
// directory.
RelativeDest string

// Chown the resulting files and directories to the user of the task.
//
// Defaults to false.
Chown bool
}

func (ta *TaskArtifact) Equal(o *TaskArtifact) bool {
Expand All @@ -9826,6 +9831,8 @@ func (ta *TaskArtifact) Equal(o *TaskArtifact) bool {
return false
case ta.RelativeDest != o.RelativeDest:
return false
case ta.Chown != o.Chown:
return false
}
return true
}
Expand All @@ -9841,6 +9848,7 @@ func (ta *TaskArtifact) Copy() *TaskArtifact {
GetterMode: ta.GetterMode,
GetterInsecure: ta.GetterInsecure,
RelativeDest: ta.RelativeDest,
Chown: ta.Chown,
}
}

Expand Down Expand Up @@ -9882,6 +9890,7 @@ func (ta *TaskArtifact) Hash() string {
_, _ = h.Write([]byte(ta.GetterMode))
_, _ = h.Write([]byte(strconv.FormatBool(ta.GetterInsecure)))
_, _ = h.Write([]byte(ta.RelativeDest))
_, _ = h.Write([]byte(strconv.FormatBool(ta.Chown)))
return base64.RawStdEncoding.EncodeToString(h.Sum(nil))
}

Expand Down
19 changes: 17 additions & 2 deletions nomad/structs/structs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5065,6 +5065,17 @@ func TestTaskArtifact_Hash(t *testing.T) {
GetterInsecure: true,
RelativeDest: "i",
},
{
GetterSource: "b",
GetterOptions: map[string]string{
"c": "c",
"d": "e",
},
GetterMode: "g",
GetterInsecure: true,
RelativeDest: "i",
Chown: true,
},
}

// Map of hash to source
Expand Down Expand Up @@ -7860,7 +7871,7 @@ func TestTaskArtifact_Equal(t *testing.T) {
ci.Parallel(t)

must.Equal[*TaskArtifact](t, nil, nil)
must.NotEqual[*TaskArtifact](t, nil, new(TaskArtifact))
must.NotEqual(t, nil, new(TaskArtifact))

must.StructEqual(t, &TaskArtifact{
GetterSource: "source",
Expand All @@ -7883,7 +7894,11 @@ func TestTaskArtifact_Equal(t *testing.T) {
}, {
Field: "RelativeDest",
Apply: func(ta *TaskArtifact) { ta.RelativeDest = "./alloc" },
}})
}, {
Field: "Chown",
Apply: func(ta *TaskArtifact) { ta.Chown = true },
},
})
}

func TestVault_Equal(t *testing.T) {
Expand Down
4 changes: 4 additions & 0 deletions website/content/docs/job-specification/artifact.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,9 @@ automatically unarchived before the starting the task.
- `source` `(string: <required>)` - Specifies the URL of the artifact to download.
See [`go-getter`][go-getter] for details.

- `chown` `(bool: false)` - Specifies whether Nomad should recursively `chown`
the downloaded artifact to be owned by the [`task.user`][task_user] uid and gid.

## Operation Limits

The client [`artifact`][client_artifact] configuration can set limits to
Expand Down Expand Up @@ -279,5 +282,6 @@ client configuration.
[s3-region-endpoints]: http://docs.aws.amazon.com/general/latest/gr/rande.html#s3_region 'Amazon S3 Region Endpoints'
[iam-instance-profiles]: https://docs.aws.amazon.com/IAM/latest/UserGuide/id_roles_use_switch-role-ec2_instance-profiles.html 'EC2 IAM instance profiles'
[task's working directory]: /nomad/docs/runtime/environment#task-directories 'Task Directories'
[task_user]: /nomad/docs/job-specification/task#user
[filesystem internals]: /nomad/docs/concepts/filesystem#templates-artifacts-and-dispatch-payloads
[do_spaces]: https://www.digitalocean.com/products/spaces
Loading