From 7ded31d7a834c06c1f1b55dfc561271510b48c43 Mon Sep 17 00:00:00 2001 From: Manuel de Brito Fontes Date: Tue, 12 Jun 2018 08:40:40 -0400 Subject: [PATCH] Create file permission constants --- internal/file/filesystem.go | 8 +++++++- internal/ingress/annotations/auth/main.go | 16 +--------------- internal/ingress/controller/checker_test.go | 3 ++- internal/ingress/controller/nginx.go | 6 +++--- internal/ingress/controller/store/store.go | 2 +- internal/net/dns/dns_test.go | 14 ++++++++------ internal/watch/file_watcher_test.go | 12 +++++++----- 7 files changed, 29 insertions(+), 32 deletions(-) diff --git a/internal/file/filesystem.go b/internal/file/filesystem.go index 3ad2f531b..4a66ed263 100644 --- a/internal/file/filesystem.go +++ b/internal/file/filesystem.go @@ -25,6 +25,12 @@ import ( "k8s.io/kubernetes/pkg/util/filesystem" ) +// ReadWriteByUser defines linux permission to read and write files for the owner user +const ReadWriteByUser = 0660 + +// ReadByUserGroup defines linux permission to read files by the user and group owner/s +const ReadByUserGroup = 0640 + // Filesystem is an interface that we can use to mock various filesystem operations type Filesystem interface { filesystem.Filesystem @@ -35,7 +41,7 @@ func NewLocalFS() (Filesystem, error) { fs := filesystem.DefaultFs{} for _, directory := range directories { - err := fs.MkdirAll(directory, 0777) + err := fs.MkdirAll(directory, ReadWriteByUser) if err != nil { return nil, err } diff --git a/internal/ingress/annotations/auth/main.go b/internal/ingress/annotations/auth/main.go index 5b465f9df..3a6a3fae4 100644 --- a/internal/ingress/annotations/auth/main.go +++ b/internal/ingress/annotations/auth/main.go @@ -19,8 +19,6 @@ package auth import ( "fmt" "io/ioutil" - "os" - "path" "regexp" "github.com/pkg/errors" @@ -86,17 +84,6 @@ type auth struct { // NewParser creates a new authentication annotation parser func NewParser(authDirectory string, r resolver.Resolver) parser.IngressAnnotation { - os.MkdirAll(authDirectory, 0755) - - currPath := authDirectory - for currPath != "/" { - currPath = path.Dir(currPath) - err := os.Chmod(currPath, 0755) - if err != nil { - break - } - } - return auth{r, authDirectory} } @@ -157,8 +144,7 @@ func dumpSecret(filename string, secret *api.Secret) error { } } - // TODO: check permissions required - err := ioutil.WriteFile(filename, val, 0777) + err := ioutil.WriteFile(filename, val, file.ReadWriteByUser) if err != nil { return ing_errors.LocationDenied{ Reason: errors.Wrap(err, "unexpected error creating password file"), diff --git a/internal/ingress/controller/checker_test.go b/internal/ingress/controller/checker_test.go index 050eb1e46..4e2385cf4 100644 --- a/internal/ingress/controller/checker_test.go +++ b/internal/ingress/controller/checker_test.go @@ -27,6 +27,7 @@ import ( "k8s.io/apiserver/pkg/server/healthz" "k8s.io/kubernetes/pkg/util/filesystem" + "k8s.io/ingress-nginx/internal/file" ngx_config "k8s.io/ingress-nginx/internal/ingress/controller/config" ) @@ -60,7 +61,7 @@ func TestNginxCheck(t *testing.T) { }) // create pid file - fs.MkdirAll("/tmp", 0655) + fs.MkdirAll("/tmp", file.ReadWriteByUser) pidFile, err := fs.Create(nginxPID) if err != nil { t.Fatalf("unexpected error: %v", err) diff --git a/internal/ingress/controller/nginx.go b/internal/ingress/controller/nginx.go index d300629f8..87df06979 100644 --- a/internal/ingress/controller/nginx.go +++ b/internal/ingress/controller/nginx.go @@ -423,7 +423,7 @@ func (n NGINXController) testTemplate(cfg []byte) error { return err } defer tmpfile.Close() - err = ioutil.WriteFile(tmpfile.Name(), cfg, 0644) + err = ioutil.WriteFile(tmpfile.Name(), cfg, file.ReadWriteByUser) if err != nil { return err } @@ -647,7 +647,7 @@ func (n *NGINXController) OnUpdate(ingressCfg ingress.Configuration) error { return err } defer tmpfile.Close() - err = ioutil.WriteFile(tmpfile.Name(), content, 0644) + err = ioutil.WriteFile(tmpfile.Name(), content, file.ReadWriteByUser) if err != nil { return err } @@ -666,7 +666,7 @@ func (n *NGINXController) OnUpdate(ingressCfg ingress.Configuration) error { } } - err = ioutil.WriteFile(cfgPath, content, 0644) + err = ioutil.WriteFile(cfgPath, content, file.ReadWriteByUser) if err != nil { return err } diff --git a/internal/ingress/controller/store/store.go b/internal/ingress/controller/store/store.go index ac4cceab2..ea697179b 100644 --- a/internal/ingress/controller/store/store.go +++ b/internal/ingress/controller/store/store.go @@ -699,7 +699,7 @@ func (s *k8sStore) setConfig(cmap *corev1.ConfigMap) { glog.Warningf("unexpected error decoding key ssl-session-ticket-key: %v", err) s.backendConfig.SSLSessionTicketKey = "" } - ioutil.WriteFile("/etc/nginx/tickets.key", d, 0644) + ioutil.WriteFile("/etc/nginx/tickets.key", d, file.ReadWriteByUser) } } diff --git a/internal/net/dns/dns_test.go b/internal/net/dns/dns_test.go index 979d65c32..bd2243ae7 100644 --- a/internal/net/dns/dns_test.go +++ b/internal/net/dns/dns_test.go @@ -21,6 +21,8 @@ import ( "net" "os" "testing" + + "k8s.io/ingress-nginx/internal/file" ) func TestGetDNSServers(t *testing.T) { @@ -32,22 +34,22 @@ func TestGetDNSServers(t *testing.T) { t.Error("expected at least 1 nameserver in /etc/resolv.conf") } - file, err := ioutil.TempFile("", "fw") + f, err := ioutil.TempFile("", "fw") if err != nil { t.Fatalf("unexpected error: %v", err) } - defer file.Close() - defer os.Remove(file.Name()) + defer f.Close() + defer os.Remove(f.Name()) - ioutil.WriteFile(file.Name(), []byte(` + ioutil.WriteFile(f.Name(), []byte(` # comment ; comment nameserver 2001:4860:4860::8844 nameserver 2001:4860:4860::8888 nameserver 8.8.8.8 - `), 0644) + `), file.ReadWriteByUser) - defResolvConf = file.Name() + defResolvConf = f.Name() s, err = GetSystemNameServers() if err != nil { t.Fatalf("unexpected error reading /etc/resolv.conf file: %v", err) diff --git a/internal/watch/file_watcher_test.go b/internal/watch/file_watcher_test.go index 5733cd07c..83a37ea0b 100644 --- a/internal/watch/file_watcher_test.go +++ b/internal/watch/file_watcher_test.go @@ -21,6 +21,8 @@ import ( "os" "testing" "time" + + "k8s.io/ingress-nginx/internal/file" ) func prepareTimeout() chan bool { @@ -33,15 +35,15 @@ func prepareTimeout() chan bool { } func TestFileWatcher(t *testing.T) { - file, err := ioutil.TempFile("", "fw") + f, err := ioutil.TempFile("", "fw") if err != nil { t.Fatalf("unexpected error: %v", err) } - defer file.Close() - defer os.Remove(file.Name()) + defer f.Close() + defer os.Remove(f.Name()) count := 0 events := make(chan bool, 10) - fw, err := NewFileWatcher(file.Name(), func() { + fw, err := NewFileWatcher(f.Name(), func() { count++ if count != 1 { t.Fatalf("expected 1 but returned %v", count) @@ -58,7 +60,7 @@ func TestFileWatcher(t *testing.T) { t.Fatalf("expected no events before writing a file") case <-timeoutChan: } - ioutil.WriteFile(file.Name(), []byte{}, 0644) + ioutil.WriteFile(f.Name(), []byte{}, file.ReadWriteByUser) select { case <-events: case <-timeoutChan: