jobspec: add a chown option to artifact block (#24157)

* jobspec: add a chown option to artifact block

This PR adds a boolean 'chown' field to the artifact block.

It indicates whether the Nomad client should chown the downloaded files
and directories to be owned by the task.user. This is useful for drivers
like raw_exec and exec2 which are subject to the host filesystem user
permissions structure. Before, these drivers might not be able to use or
manage the downloaded artifacts since they would be owned by the root
user on a typical Nomad client configuration.

* api: no need for pointer of chown field

.changelog/24157.txt

@@ -0,0 +1,3 @@
+```release-note:improvement
+getter: Added option to chown artifact(s) to task user
+```

api/tasks.go

@@ -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() {

api/tasks_test.go

@@ -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) {

client/allocrunner/taskrunner/artifact_hook.go

@@ -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()
@@ -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,

client/allocrunner/taskrunner/getter/params.go

@@ -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 {

client/allocrunner/taskrunner/getter/params_test.go

@@ -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{
@@ -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) {

client/allocrunner/taskrunner/getter/sandbox.go

@@ -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 {
@@ -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
 }

client/allocrunner/taskrunner/getter/sandbox_test.go

@@ -8,6 +8,7 @@ import (
 	"net/http/httptest"
 	"os"
 	"path/filepath"
+	"syscall"
 	"testing"
 	"time"
 
@@ -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"))
@@ -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
+}

client/allocrunner/taskrunner/getter/util.go

@@ -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"
 )
 
@@ -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
 }

client/allocrunner/taskrunner/getter/z_getter_cmd.go

@@ -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
 	})

client/interfaces/client.go

@@ -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.

command/agent/job_endpoint.go

@@ -1426,6 +1426,7 @@ func ApiTaskToStructsTask(job *structs.Job, group *structs.TaskGroup,
 					GetterMode:     *ta.GetterMode,
 					GetterInsecure: *ta.GetterInsecure,
 					RelativeDest:   *ta.RelativeDest,
+					Chown:          ta.Chown,
 				})
 		}
 	}

command/agent/job_endpoint_test.go

@@ -2932,6 +2932,7 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) {
 								},
 								GetterMode:   pointer.Of("dir"),
 								RelativeDest: pointer.Of("dest"),
+								Chown:        true,
 							},
 						},
 						Vault: &api.Vault{
@@ -3387,6 +3388,7 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) {
 								},
 								GetterMode:   "dir",
 								RelativeDest: "dest",
+								Chown:        true,
 							},
 						},
 						Vault: &structs.Vault{

nomad/structs/diff_test.go

@@ -6059,6 +6059,7 @@ func TestTaskDiff(t *testing.T) {
 						},
 						GetterMode:   "dir",
 						RelativeDest: "bar",
+						Chown:        false,
 					},
 				},
 			},
@@ -6082,6 +6083,7 @@ func TestTaskDiff(t *testing.T) {
 						},
 						GetterMode:   "file",
 						RelativeDest: "bam",
+						Chown:        true,
 					},
 				},
 			},
@@ -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]",
@@ -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",

nomad/structs/structs.go

@@ -9815,6 +9815,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 {
@@ -9834,6 +9839,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
 }
@@ -9849,6 +9856,7 @@ func (ta *TaskArtifact) Copy() *TaskArtifact {
 		GetterMode:     ta.GetterMode,
 		GetterInsecure: ta.GetterInsecure,
 		RelativeDest:   ta.RelativeDest,
+		Chown:          ta.Chown,
 	}
 }
 
@@ -9890,6 +9898,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))
 }
 

nomad/structs/structs_test.go

@@ -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
@@ -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",
@@ -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) {

website/content/docs/job-specification/artifact.mdx

@@ -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
@@ -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