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: never embed alloc_dir in chroot #11334

Merged
merged 3 commits into from
Nov 3, 2021
Merged

Commits on Oct 18, 2021

  1. client: never embed alloc_dir in chroot

    Fixes #2522
    
    Skip embedding client.alloc_dir when building chroot. If a user
    configures a Nomad client agent so that the chroot_env will embed the
    client.alloc_dir, Nomad will happily infinitely recurse while building
    the chroot until something horrible happens. The best case scenario is
    the filesystem's path length limit is hit. The worst case scenario is
    disk space is exhausted.
    
    A bad agent configuration will look something like this:
    
    ```hcl
    data_dir = "/tmp/nomad-badagent"
    
    client {
      enabled = true
    
      chroot_env {
        # Note that the source matches the data_dir
        "/tmp/nomad-badagent" = "/ohno"
        # ...
      }
    }
    ```
    
    Note that `/ohno/client` (the state_dir) will still be created but not
    `/ohno/alloc` (the alloc_dir).
    While I cannot think of a good reason why someone would want to embed
    Nomad's client (and possibly server) directories in chroots, there
    should be no cause for harm. chroots are only built when Nomad runs as
    root, and Nomad disables running exec jobs as root by default. Therefore
    even if client state is copied into chroots, it will be inaccessible to
    tasks.
    
    Skipping the `data_dir` and `{client,server}.state_dir` is possible, but
    this PR attempts to implement the minimum viable solution to reduce risk
    of unintended side effects or bugs.
    
    When running tests as root in a vm without the fix, the following error
    occurs:
    
    ```
    === RUN   TestAllocDir_SkipAllocDir
        alloc_dir_test.go:520:
                    Error Trace:    alloc_dir_test.go:520
                    Error:          Received unexpected error:
                                    Couldn't create destination file /tmp/TestAllocDir_SkipAllocDir1457747331/001/nomad/test/testtask/nomad/test/testtask/.../nomad/test/testtask/secrets/.nomad-mount: open /tmp/TestAllocDir_SkipAllocDir1457747331/001/nomad/test/.../testtask/secrets/.nomad-mount: file name too long
                    Test:           TestAllocDir_SkipAllocDir
    --- FAIL: TestAllocDir_SkipAllocDir (22.76s)
    ```
    
    Also removed unused Copy methods on AllocDir and TaskDir structs.
    
    Thanks to @eveld for not letting me forget about this!
    schmichael committed Oct 18, 2021
    Configuration menu
    Copy the full SHA
    37f053f View commit details
    Browse the repository at this point in the history
  2. docs: add #11334 to changelog

    schmichael committed Oct 18, 2021
    Configuration menu
    Copy the full SHA
    f41b625 View commit details
    Browse the repository at this point in the history

Commits on Oct 19, 2021

  1. test: update tests to properly use AllocDir

    Also use t.TempDir when possible.
    schmichael committed Oct 19, 2021
    Configuration menu
    Copy the full SHA
    eeb1da8 View commit details
    Browse the repository at this point in the history