pem.NewFiles() validates the base directory and crt getter.

pkg/k8s/client.go

@@ -155,7 +155,11 @@ func NewWatcher(
 	watcher.informer.AddEventHandler(evtFuncs)
 
 	watcher.getter = crt.NewGetter(lister)
-	watcher.files = pem.NewFiles(base, gid, watcher.getter)
+	var err error
+	watcher.files, err = pem.NewFiles(base, gid, watcher.getter)
+	if err != nil {
+		return nil, err
+	}
 	watcher.nsQs = NewNamespaceQueues(watcher.log, watcher.files, recorder)
 
 	// InitMetrics()

pkg/pem/pem.go

@@ -34,6 +34,7 @@ package pem
 import (
 	"crypto/sha512"
 	"encoding/base64"
+	"fmt"
 	"io/ioutil"
 	"os"
 	"path/filepath"
@@ -111,9 +112,20 @@ type Files struct {
 // is >= 0, then it is the group id set in PEM file permissions.
 // crtGetter is used to retrieve and validate the contents of TLS
 // Secrets.
-func NewFiles(base string, gid int, crtGetter *crt.Getter) *Files {
-	// XXX validate: base must be a writable directory, crtGetter
-	// must be != nil.
+func NewFiles(base string, gid int, crtGetter *crt.Getter) (*Files, error) {
+	if crtGetter == nil {
+		return nil, fmt.Errorf("crtGetter is nil")
+	}
+	info, err := os.Stat(base)
+	if err != nil {
+		return nil, fmt.Errorf("Cannot stat %s: %v", base, err)
+	}
+	if !info.IsDir() {
+		return nil, fmt.Errorf("%s: not a directory", base)
+	}
+	if err = unix.Access(base, unix.W_OK); err != nil {
+		return nil, fmt.Errorf("%s not writable: %v", base, err)
+	}
 	files := &Files{
 		Base:      base,
 		Gid:       -1,
@@ -124,7 +136,7 @@ func NewFiles(base string, gid int, crtGetter *crt.Getter) *Files {
 	if gid >= 0 {
 		files.Gid = gid
 	}
-	return files
+	return files, nil
 }
 
 func (pemfiles *Files) String() string {

pkg/pem/pem_test.go

@@ -83,20 +83,79 @@ func TestBasename(t *testing.T) {
 	}
 }
 
+func getTempDir() (string, error) {
+	return ioutil.TempDir("", "k8s-crt-dnldr_pem_test")
+}
+
+func TestNewFiles(t *testing.T) {
+	base, err := getTempDir()
+	if err != nil {
+		t.Fatalf("ioutil.TempDir(): %+v", err)
+	}
+	defer os.RemoveAll(base)
+
+	if _, err := NewFiles(base, -1, nil); err == nil {
+		t.Errorf("NewFiles(getter=nil) error: got nil want non-nil")
+	}
+
+	client := fake.NewSimpleClientset()
+	lister := setupSecretLister(client)
+	getter := crt.NewGetter(lister)
+
+	noSuchDir := filepath.Join(base, "nosuchdir")
+	if _, err := NewFiles(noSuchDir, -1, getter); err == nil {
+		t.Errorf("NewFiles(no such base) error: got nil want non-nil")
+	}
+	nonDir := filepath.Join(base, "nondir")
+	if err = ioutil.WriteFile(nonDir, []byte{}, os.FileMode(0664)); err != nil {
+		t.Fatalf("Cannot write test file %s: %v", nonDir, err)
+	}
+	if _, err := NewFiles(nonDir, -1, getter); err == nil {
+		t.Errorf("NewFiles(non dir base) error: got nil want non-nil")
+	}
+}
+
 func TestPath(t *testing.T) {
-	files := NewFiles("/foo/bar/baz", -1, nil)
+	base, err := getTempDir()
+	if err != nil {
+		t.Fatalf("ioutil.TempDir(): %+v", err)
+	}
+	defer os.RemoveAll(base)
+
+	client := fake.NewSimpleClientset()
+	lister := setupSecretLister(client)
+	getter := crt.NewGetter(lister)
+
+	files, err := NewFiles(base, -1, getter)
+	if err != nil {
+		t.Fatalf("NewFiles(): %v", err)
+	}
 	file := &File{
 		Namespace: "ns",
 		Name:      "name",
 	}
-	if files.Path(file) != "/foo/bar/baz/ns_name.pem" {
-		t.Errorf("Files.Path(ns/name): got %s want "+
-			"/foo/bar/baz/ns_name.pem", files.Path(file))
+	path := filepath.Join(base, "ns_name.pem")
+	if files.Path(file) != path {
+		t.Errorf("Files.Path(ns/name): got %s want %s",
+			files.Path(file), path)
 	}
 }
 
 func TestHave(t *testing.T) {
-	files := NewFiles("/foo/bar/baz", -1, nil)
+	base, err := getTempDir()
+	if err != nil {
+		t.Fatalf("ioutil.TempDir(): %+v", err)
+	}
+	defer os.RemoveAll(base)
+
+	client := fake.NewSimpleClientset()
+	lister := setupSecretLister(client)
+	getter := crt.NewGetter(lister)
+
+	files, err := NewFiles(base, -1, getter)
+	if err != nil {
+		t.Fatalf("NewFiles(): %v", err)
+	}
 	files.Files["ns/name"] = &File{
 		Namespace:       "ns",
 		Name:            "name",
@@ -129,7 +188,7 @@ func TestHave(t *testing.T) {
 
 func TestCheck(t *testing.T) {
 	gid := os.Getgid()
-	base, err := ioutil.TempDir("", "k8s-crt-dnldr_pem_test")
+	base, err := getTempDir()
 	if err != nil {
 		t.Fatalf("ioutil.TempDir(): %+v", err)
 	}
@@ -158,7 +217,10 @@ func TestCheck(t *testing.T) {
 	)
 	lister := setupSecretLister(client)
 	getter := crt.NewGetter(lister)
-	files := NewFiles(base, gid, getter)
+	files, err := NewFiles(base, gid, getter)
+	if err != nil {
+		t.Fatalf("NewFiles(): %v", err)
+	}
 	files.Files["ns/secret"] = file
 	path := files.Path(file)
 	bytes := []byte("crtSecret\nkeySecret")
@@ -325,7 +387,7 @@ func TestCheck(t *testing.T) {
 
 func TestWrite(t *testing.T) {
 	gid := os.Getgid()
-	base, err := ioutil.TempDir("", "k8s-crt-dnldr_pem_test")
+	base, err := getTempDir()
 	if err != nil {
 		t.Fatalf("ioutil.TempDir(): %+v", err)
 	}
@@ -413,7 +475,10 @@ func TestWrite(t *testing.T) {
 	)
 	lister := setupSecretLister(client)
 	getter := crt.NewGetter(lister)
-	files := NewFiles(base, gid, getter)
+	files, err := NewFiles(base, gid, getter)
+	if err != nil {
+		t.Fatalf("NewFiles(): %v", err)
+	}
 
 	found, valid, err := files.Write("namespace", "name",
 		"73e880f3-d7ca-447f-b95e-88f220d4820e", "654321")
@@ -610,7 +675,7 @@ func TestWrite(t *testing.T) {
 
 func TestDelete(t *testing.T) {
 	gid := os.Getgid()
-	base, err := ioutil.TempDir("", "k8s-crt-dnldr_pem_test")
+	base, err := getTempDir()
 	if err != nil {
 		t.Fatalf("ioutil.TempDir(): %+v", err)
 	}
@@ -639,7 +704,10 @@ func TestDelete(t *testing.T) {
 	)
 	lister := setupSecretLister(client)
 	getter := crt.NewGetter(lister)
-	files := NewFiles(base, gid, getter)
+	files, err := NewFiles(base, gid, getter)
+	if err != nil {
+		t.Fatalf("NewFiles(): %v", err)
+	}
 	files.Files["test/deleteme"] = file
 	path := files.Path(file)
 	bytes := []byte("crtSecret\nkeySecret")

pkg/rest/handlers_test.go

@@ -160,9 +160,13 @@ func setupSecretLister(client *fake.Clientset) core_v1_listers.SecretLister {
 	return lister
 }
 
+func getTempDir() (string, error) {
+	return ioutil.TempDir("", "k8s-crt-dnldr_handlers_test")
+}
+
 func TestPutPem(t *testing.T) {
 	gid := os.Getgid()
-	base, err := ioutil.TempDir("", "k8s-crt-dnldr_rest_test")
+	base, err := getTempDir()
 	if err != nil {
 		t.Fatalf("ioutil.TempDir(): %+v", err)
 	}
@@ -289,7 +293,10 @@ func TestPutPem(t *testing.T) {
 	)
 	lister := setupSecretLister(client)
 	getter := crt.NewGetter(lister)
-	files := pem.NewFiles(base, gid, getter)
+	files, err := pem.NewFiles(base, gid, getter)
+	if err != nil {
+		t.Fatalf("NewFiles(): %v", err)
+	}
 
 	hndlr := &pemsHndlr{
 		log:       &logrus.Logger{Out: ioutil.Discard},
@@ -516,7 +523,7 @@ func TestPutPem(t *testing.T) {
 
 func TestPostPem(t *testing.T) {
 	gid := os.Getgid()
-	base, err := ioutil.TempDir("", "k8s-crt-dnldr_rest_test")
+	base, err := getTempDir()
 	if err != nil {
 		t.Fatalf("ioutil.TempDir(): %+v", err)
 	}
@@ -539,7 +546,10 @@ func TestPostPem(t *testing.T) {
 	)
 	lister := setupSecretLister(client)
 	getter := crt.NewGetter(lister)
-	files := pem.NewFiles(base, gid, getter)
+	files, err := pem.NewFiles(base, gid, getter)
+	if err != nil {
+		t.Fatalf("NewFiles(): %v", err)
+	}
 
 	hndlr := &pemsHndlr{
 		log:       &logrus.Logger{Out: ioutil.Discard},
@@ -605,7 +615,7 @@ func TestPostPem(t *testing.T) {
 
 func TestDeletePem(t *testing.T) {
 	gid := os.Getgid()
-	base, err := ioutil.TempDir("", "k8s-crt-dnldr_rest_test")
+	base, err := getTempDir()
 	if err != nil {
 		t.Fatalf("ioutil.TempDir(): %+v", err)
 	}
@@ -641,7 +651,10 @@ func TestDeletePem(t *testing.T) {
 	)
 	lister := setupSecretLister(client)
 	getter := crt.NewGetter(lister)
-	files := pem.NewFiles(base, gid, getter)
+	files, err := pem.NewFiles(base, gid, getter)
+	if err != nil {
+		t.Fatalf("NewFiles(): %v", err)
+	}
 
 	hndlr := &pemsHndlr{
 		log:       &logrus.Logger{Out: ioutil.Discard},
@@ -727,7 +740,15 @@ func TestGetPem(t *testing.T) {
 	client := fake.NewSimpleClientset()
 	lister := setupSecretLister(client)
 	getter := crt.NewGetter(lister)
-	files := pem.NewFiles(os.DevNull, -1, getter)
+	base, err := getTempDir()
+	if err != nil {
+		t.Fatalf("ioutil.TempDir(): %+v", err)
+	}
+	defer os.RemoveAll(base)
+	files, err := pem.NewFiles(base, -1, getter)
+	if err != nil {
+		t.Fatalf("NewFiles(): %v", err)
+	}
 
 	hndlr := &pemsHndlr{
 		log:       &logrus.Logger{Out: ioutil.Discard},
@@ -762,7 +783,15 @@ func TestPem405(t *testing.T) {
 	client := fake.NewSimpleClientset()
 	lister := setupSecretLister(client)
 	getter := crt.NewGetter(lister)
-	files := pem.NewFiles(os.DevNull, -1, getter)
+	base, err := getTempDir()
+	if err != nil {
+		t.Fatalf("ioutil.TempDir(): %+v", err)
+	}
+	defer os.RemoveAll(base)
+	files, err := pem.NewFiles(base, -1, getter)
+	if err != nil {
+		t.Fatalf("NewFiles(): %v", err)
+	}
 
 	hndlr := &pemsHndlr{
 		log:       &logrus.Logger{Out: ioutil.Discard},
@@ -801,7 +830,15 @@ func TestAllPem(t *testing.T) {
 	client := fake.NewSimpleClientset()
 	lister := setupSecretLister(client)
 	getter := crt.NewGetter(lister)
-	files := pem.NewFiles(os.DevNull, -1, getter)
+	base, err := getTempDir()
+	if err != nil {
+		t.Fatalf("ioutil.TempDir(): %+v", err)
+	}
+	defer os.RemoveAll(base)
+	files, err := pem.NewFiles(base, -1, getter)
+	if err != nil {
+		t.Fatalf("NewFiles(): %v", err)
+	}
 
 	hndlr := &pemsHndlr{
 		log:       &logrus.Logger{Out: ioutil.Discard},