From eff134cd5f9f5d2aa0439a7d2169c154d46aee9c Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Mon, 2 Jan 2017 22:27:51 -0500 Subject: [PATCH] Use chmod to bypass umask on termination log file os.Create() will obey the umask which results in the file being 0644 when injected in the container. --- pkg/kubelet/container/os.go | 7 +++++++ pkg/kubelet/container/testing/os.go | 5 +++++ pkg/kubelet/dockertools/docker_manager.go | 13 ++++++++++--- pkg/kubelet/kuberuntime/kuberuntime_container.go | 11 ++++++++++- 4 files changed, 32 insertions(+), 4 deletions(-) diff --git a/pkg/kubelet/container/os.go b/pkg/kubelet/container/os.go index 18e5221e124..6126063b308 100644 --- a/pkg/kubelet/container/os.go +++ b/pkg/kubelet/container/os.go @@ -32,6 +32,7 @@ type OSInterface interface { Remove(path string) error RemoveAll(path string) error Create(path string) (*os.File, error) + Chmod(path string, perm os.FileMode) error Hostname() (name string, err error) Chtimes(path string, atime time.Time, mtime time.Time) error Pipe() (r *os.File, w *os.File, err error) @@ -73,6 +74,12 @@ func (RealOS) Create(path string) (*os.File, error) { return os.Create(path) } +// Chmod will change the permissions on the specified path or return +// an error. +func (RealOS) Chmod(path string, perm os.FileMode) error { + return os.Chmod(path, perm) +} + // Hostname will call os.Hostname to return the hostname. func (RealOS) Hostname() (name string, err error) { return os.Hostname() diff --git a/pkg/kubelet/container/testing/os.go b/pkg/kubelet/container/testing/os.go index e70c2809f8a..ee8d255b087 100644 --- a/pkg/kubelet/container/testing/os.go +++ b/pkg/kubelet/container/testing/os.go @@ -83,6 +83,11 @@ func (FakeOS) Create(path string) (*os.File, error) { return nil, nil } +// Chmod is a fake call that returns nil. +func (FakeOS) Chmod(path string, perm os.FileMode) error { + return nil +} + // Hostname is a fake call that returns nil. func (f *FakeOS) Hostname() (name string, err error) { return f.HostName, nil diff --git a/pkg/kubelet/dockertools/docker_manager.go b/pkg/kubelet/dockertools/docker_manager.go index 5c1ac78e624..653a6fed4cb 100644 --- a/pkg/kubelet/dockertools/docker_manager.go +++ b/pkg/kubelet/dockertools/docker_manager.go @@ -672,17 +672,24 @@ func (dm *DockerManager) runContainer( fs, err := os.Create(containerLogPath) if err != nil { // TODO: Clean up the previously created dir? return the error? - glog.Errorf("Error on creating termination-log file %q: %v", containerLogPath, err) + utilruntime.HandleError(fmt.Errorf("error creating termination-log file %q: %v", containerLogPath, err)) } else { fs.Close() // Close immediately; we're just doing a `touch` here - b := fmt.Sprintf("%s:%s", containerLogPath, container.TerminationMessagePath) + + // Chmod is needed because ioutil.WriteFile() ends up calling + // open(2) to create the file, so the final mode used is "mode & + // ~umask". But we want to make sure the specified mode is used + // in the file no matter what the umask is. + if err := os.Chmod(containerLogPath, 0666); err != nil { + utilruntime.HandleError(fmt.Errorf("unable to set termination-log file permissions %q: %v", containerLogPath, err)) + } // Have docker relabel the termination log path if SELinux is // enabled. + b := fmt.Sprintf("%s:%s", containerLogPath, container.TerminationMessagePath) if selinux.SELinuxEnabled() { b += ":Z" } - binds = append(binds, b) } } diff --git a/pkg/kubelet/kuberuntime/kuberuntime_container.go b/pkg/kubelet/kuberuntime/kuberuntime_container.go index e823362427e..529d0ba5e5a 100644 --- a/pkg/kubelet/kuberuntime/kuberuntime_container.go +++ b/pkg/kubelet/kuberuntime/kuberuntime_container.go @@ -272,9 +272,18 @@ func (m *kubeGenericRuntimeManager) makeMounts(opts *kubecontainer.RunContainerO containerLogPath := filepath.Join(opts.PodContainerDir, cid) fs, err := m.osInterface.Create(containerLogPath) if err != nil { - glog.Errorf("Error on creating termination-log file %q: %v", containerLogPath, err) + utilruntime.HandleError(fmt.Errorf("error on creating termination-log file %q: %v", containerLogPath, err)) } else { fs.Close() + + // Chmod is needed because ioutil.WriteFile() ends up calling + // open(2) to create the file, so the final mode used is "mode & + // ~umask". But we want to make sure the specified mode is used + // in the file no matter what the umask is. + if err := m.osInterface.Chmod(containerLogPath, 0666); err != nil { + utilruntime.HandleError(fmt.Errorf("unable to set termination-log file permissions %q: %v", containerLogPath, err)) + } + selinuxRelabel := selinux.SELinuxEnabled() volumeMounts = append(volumeMounts, &runtimeapi.Mount{ HostPath: containerLogPath,