Repository: mesos Updated Branches: refs/heads/1.3.x 567775dc7 -> 624cdb304
Fixed the sandbox_path volume source path ownership. This bugfix addresses the issue from MESOS-7830. Basically, the sandbox path volume ownership was not set correctly. This issue can be exposed if a framework user is non-root while the agent process runs as root. Then, the non-root user does not have permissions to write to this volume. The correct solution should be giving permissions to corresponding users by leveraging supplementary groups. But we can still introduce a workaround in this patch by changing the ownership of the sandbox path volume to its sandbox's ownership. Review: https://reviews.apache.org/r/61120/ Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/0df7258b Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/0df7258b Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/0df7258b Branch: refs/heads/1.3.x Commit: 0df7258b6c63e27b00ff0268496a1fb5d640d8b6 Parents: 567775d Author: Gilbert Song <songzihao1...@gmail.com> Authored: Fri Jul 28 12:27:49 2017 -0700 Committer: Gilbert Song <songzihao1...@gmail.com> Committed: Fri Jul 28 12:37:45 2017 -0700 ---------------------------------------------------------------------- .../mesos/isolators/volume/sandbox_path.cpp | 34 +++++++++++++++++--- 1 file changed, 29 insertions(+), 5 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/0df7258b/src/slave/containerizer/mesos/isolators/volume/sandbox_path.cpp ---------------------------------------------------------------------- diff --git a/src/slave/containerizer/mesos/isolators/volume/sandbox_path.cpp b/src/slave/containerizer/mesos/isolators/volume/sandbox_path.cpp index 6f7304d..b321b86 100644 --- a/src/slave/containerizer/mesos/isolators/volume/sandbox_path.cpp +++ b/src/slave/containerizer/mesos/isolators/volume/sandbox_path.cpp @@ -166,11 +166,35 @@ Future<Option<ContainerLaunchInfo>> VolumeSandboxPathIsolatorProcess::prepare( sandboxes[containerId.parent()], sandboxPath.path()); - Try<Nothing> mkdir = os::mkdir(source); - if (mkdir.isError()) { - return Failure( - "Failed to create the directory in the parent sandbox: " + - mkdir.error()); + // NOTE: Chown should be avoided if the source directory already + // exists because it may be owned by some other user and should + // not be mutated. + if (!os::exists(source)) { + Try<Nothing> mkdir = os::mkdir(source); + if (mkdir.isError()) { + return Failure( + "Failed to create the directory in the parent sandbox: " + + mkdir.error()); + } + + // Get the parent sandbox user and group info for the source path. + struct stat s; + if (::stat(sandboxes[containerId.parent()].c_str(), &s) < 0) { + return Failure(ErrnoError( + "Failed to stat '" + sandboxes[containerId.parent()] + "'")); + } + + LOG(INFO) << "Changing the ownership of the sandbox_path volume at '" + << source << "' with UID " << s.st_uid << " and GID " + << s.st_gid; + + Try<Nothing> chown = os::chown(s.st_uid, s.st_gid, source, false); + if (chown.isError()) { + return Failure( + "Failed to change the ownership of the sandbox_path volume at '" + + source + "' with UID " + stringify(s.st_uid) + " and GID " + + stringify(s.st_gid) + ": " + chown.error()); + } } // Prepare the target.
