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

client: enable specifying user/group permissions in the template stanza #13755

Merged
merged 20 commits into from
Aug 2, 2022

Conversation

pkazmierczak
Copy link
Contributor

@pkazmierczak pkazmierczak commented Jul 14, 2022

This PR adds support for specifying uid and gid of the template file. Resolves #5020.

@hashicorp-cla
Copy link

hashicorp-cla commented Jul 14, 2022

CLA assistant check
All committers have signed the CLA.

@pkazmierczak pkazmierczak changed the title Adds Uid/Gid parameters to template. Enable specifying user/group permissions in the template stanza Jul 14, 2022
Copy link
Contributor

@lgfa29 lgfa29 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good!

There's one piece missing to fully pipe the values from the job file all the way to the task runner, which is to set the uid and gid values in ApiTaskToStructsTask. This is the function that transforms an api.Task (external representation used by the api package and SDK) into a structs.Task (internal representation).

More over though, there's a fundamental challenge here related to how Nomad creates the task file system.

To illustrate the problem, after updating ApiTaskToStructsTask, run a Docker task with a template using the new fields:

job "example" {
  datacenters = ["dc1"]

  group "cache" {
    network {
      port "db" {
        to = 6379
      }
    }

    task "redis" {
      driver = "docker"

      template {
        data        = <<EOF
hello
        EOF
        destination = "local/test"
        uid         = 1000
        gid         = 1000
      }

      config {
        image          = "redis:7"
        ports          = ["db"]
        auth_soft_fail = true
      }

      resources {
        cpu    = 500
        memory = 256
      }
    }
  }
}

Then exec into the task and take a look at the file ownership:

$ nomad job allocs example
ID        Node ID   Task Group  Version  Desired  Status   Created  Modified
0d067f1a  6322f391  cache       0        run      running  33s ago  22s ago

$ nomad alloc exec 0d /bin/bash
root@2c548b828bf9:/data# ls -l /local/test
-rw-r--r-- 1 root root 6 Jul 14 21:58 /local/test

And then look at the file on your machine (you can grab the path from Nomad's logs, there will be a message like 2022-07-14T17:58:45.402-0400 [DEBUG] agent: (runner) final config: {"Consul":... with the full template config, one of the values is the rendered file path):

$ ls -l /private/tmp/NomadClient1242048785/0d067f1a-515a-506a-9848-716952d5ffd0/redis/local/test
-rw-r--r-- 1 1000 6 Jul 14 17:58 /private/tmp/NomadClient1242048785/0d067f1a-515a-506a-9848-716952d5ffd0/redis/local/test

So consul-template operates at the host level and does the right thing: in my machine the file owner ID is 1000, but inside the container the owner is still root. We do this because Nomad doesn't know which user is running within the container, so permissions for the task directory are set broadly.

We'll need to figure out how to go around this 😅

client/allocrunner/taskrunner/template/template.go Outdated Show resolved Hide resolved
@pkazmierczak
Copy link
Contributor Author

There's one piece missing to fully pipe the values from the job file all the way to the task runner, which is to set the uid and gid values in ApiTaskToStructsTask. This is the function that transforms an api.Task (external representation used by the api package and SDK) into a structs.Task (internal representation).

Done, thanks for catching this 👍

So consul-template operates at the host level and does the right thing: in my machine the file owner ID is 1000, but inside the container the owner is still root. We do this because Nomad doesn't know which user is running within the container, so permissions for the task directory are set broadly.

So this is somewhat tricky. Nomad does a bind type of mount of the task directory, and that preserves the uid/gid of files as long as there is a corresponding uid/gid inside the container. This, sadly and infuriatingly, does not work with docker desktop, but I just tested this branch on a linux machine and it works as expected.

$ ./bin/nomad job run ../example-permissions.nomad
==> 2022-08-01T09:35:15Z: Monitoring evaluation "bc6e3b40"
    2022-08-01T09:35:15Z: Evaluation triggered by job "example"
    2022-08-01T09:35:15Z: Allocation "26772d76" created: node "5f7242ce", group "cache"
    2022-08-01T09:35:16Z: Evaluation within deployment: "7d9a5a19"
    2022-08-01T09:35:16Z: Evaluation status changed: "pending" -> "complete"
==> 2022-08-01T09:35:16Z: Evaluation "bc6e3b40" finished with status "complete"
==> 2022-08-01T09:35:16Z: Monitoring deployment "7d9a5a19"
  ✓ Deployment "7d9a5a19" successful

    2022-08-01T09:35:31Z
    ID          = 7d9a5a19
    Job ID      = example
    Job Version = 1
    Status      = successful
    Description = Deployment completed successfully

    Deployed
    Task Group  Desired  Placed  Healthy  Unhealthy  Progress Deadline
    cache       1        1       1        0          2022-08-01T09:45:30Z

$ ./bin/nomad job allocs example
ID        Node ID   Task Group  Version  Desired  Status   Created    Modified
26772d76  5f7242ce  cache       1        run      running  1m2s ago   47s ago
f6aa2f1c  5f7242ce  cache       0        stop     failed   1m33s ago  1m2s ago

$ ./bin/nomad alloc exec 26 ls -l /local/test
-rw-r--r-- 1 www-data www-data 6 Aug  1 09:35 /local/test

I set uid and gid to 33, which corresponds to www-data user/group on most linux systems. I think as long as we make this clear in the documentation, I'd say this is acceptable.

Copy link
Contributor

@lgfa29 lgfa29 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor comments, and it also needs a changelog entry but LGTM!

client/allocrunner/taskrunner/template/template_test.go Outdated Show resolved Hide resolved
website/content/api-docs/json-jobs.mdx Outdated Show resolved Hide resolved
@pkazmierczak pkazmierczak changed the title Enable specifying user/group permissions in the template stanza agent: enable specifying user/group permissions in the template stanza Aug 2, 2022
@pkazmierczak pkazmierczak added theme/template backport/1.3.x backport to 1.3.x release line labels Aug 2, 2022
@pkazmierczak pkazmierczak changed the title agent: enable specifying user/group permissions in the template stanza client: enable specifying user/group permissions in the template stanza Aug 2, 2022
@pkazmierczak pkazmierczak merged commit 2e0b875 into main Aug 2, 2022
@pkazmierczak pkazmierczak deleted the f-user-group-template-params branch August 2, 2022 20:15
sycured added a commit to sycured/nomad-pgsql-patroni that referenced this pull request Oct 27, 2022
@github-actions
Copy link

github-actions bot commented Dec 1, 2022

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 1, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport/1.3.x backport to 1.3.x release line theme/template
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[feature]: Enable specifying user/group permissions in the template stanza
3 participants