client: allow daemon to run as non-root with init-created dirs (#121)

- chmodIfNeeded: only chmod when current perms differ from desired, avoiding
  'operation not permitted' when init container already set correct permissions
- ensureDirExists: succeed when path already exists (e.g. created by init)
  so MkdirAll permission failure does not crash the sidecar
- Add table-driven tests for both helpers

Made-with: Cursor

Co-authored-by: Jeremy Talis <jtalis@pinterest.com>

Author: krockpot

client/daemon.go

@@ -122,40 +122,60 @@ func (d *daemon) loop(refresh time.Duration) {
 	}
 }
 
-func (d *daemon) initialize() error {
-	err := os.MkdirAll(d.dir, defaultDirPermission)
+// chmodIfNeeded sets mode on path only if it differs from the current mode.
+// This avoids "operation not permitted" when running as non-root when permissions are already correct.
+func chmodIfNeeded(path string, want os.FileMode) error {
+	info, err := os.Stat(path)
 	if err != nil {
+		return err
+	}
+	if info.Mode().Perm() == want.Perm() {
+		return nil
+	}
+	return os.Chmod(path, want)
+}
+
+// ensureDirExists creates path as a directory with perm, or returns nil if it already exists.
+// When running as non-root (e.g. in Kubernetes), the directory may have been created by an
+// init container; if MkdirAll fails with permission denied, we still succeed when the path exists.
+func ensureDirExists(path string, perm os.FileMode) error {
+	err := os.MkdirAll(path, perm)
+	if err == nil {
+		return nil
+	}
+	info, statErr := os.Stat(path)
+	if statErr == nil && info.IsDir() {
+		return nil
+	}
+	return err
+}
+
+func (d *daemon) initialize() error {
+	if err := ensureDirExists(d.dir, defaultDirPermission); err != nil {
 		return fmt.Errorf("failed to initialize /var/lib/knox (run 'sudo mkdir /var/lib/knox'?): %w", err)
 	}
 
-	// Need to chmod due to a umask set on masterless puppet machines
-	err = os.Chmod(d.dir, defaultDirPermission)
-	if err != nil {
+	// Need to chmod due to a umask set on masterless puppet machines (skip if already correct for non-root)
+	if err := chmodIfNeeded(d.dir, defaultDirPermission); err != nil {
 		return fmt.Errorf("failed to open up directory permissions: %w", err)
 	}
-	err = os.MkdirAll(d.keyDir(), defaultDirPermission)
-	if err != nil {
+	if err := ensureDirExists(d.keyDir(), defaultDirPermission); err != nil {
 		return fmt.Errorf("failed to make key folders: %w", err)
 	}
 
-	// Need to chmod due to a umask set on masterless puppet machines
-	err = os.Chmod(d.keyDir(), defaultDirPermission)
-	if err != nil {
+	if err := chmodIfNeeded(d.keyDir(), defaultDirPermission); err != nil {
 		return fmt.Errorf("failed to open up directory permissions: %w", err)
 	}
-	_, err = os.Stat(d.registerFilename())
+	_, err := os.Stat(d.registerFilename())
 	if os.IsNotExist(err) {
-		err := os.WriteFile(d.registerFilename(), []byte{}, defaultFilePermission)
-		if err != nil {
+		if err := os.WriteFile(d.registerFilename(), []byte{}, defaultFilePermission); err != nil {
 			return fmt.Errorf("failed to initialize registered key file: %w", err)
 		}
 	} else if err != nil {
 		return err
 	}
 
-	// Need to chmod due to a umask set on masterless puppet machines
-	err = os.Chmod(d.registerFilename(), defaultFilePermission)
-	if err != nil {
+	if err := chmodIfNeeded(d.registerFilename(), defaultFilePermission); err != nil {
 		return fmt.Errorf("failed to open up register file permissions: %w", err)
 	}
 	d.registerKeyFile = NewKeysFile(d.registerFilename())
@@ -319,8 +339,7 @@ func (d daemon) processKey(keyID string) error {
 		return fmt.Errorf("error renaming key %s temporary file: %w", keyID, err)
 	}
 
-	err = os.Chmod(d.keyFilename(keyID), defaultFilePermission)
-	if err != nil {
+	if err := chmodIfNeeded(d.keyFilename(keyID), defaultFilePermission); err != nil {
 		return fmt.Errorf("failed to open up key file permissions: %w", err)
 	}
 	return nil

client/daemon_test.go

@@ -105,6 +105,90 @@ func TearDownTest(dir string) {
 	os.RemoveAll(dir)
 }
 
+func TestChmodIfNeeded(t *testing.T) {
+	dir, err := os.MkdirTemp("", "knox-chmod-test")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(dir)
+
+	tests := []struct {
+		name     string
+		setup    func() string // returns path to chmod
+		wantPerm os.FileMode
+	}{
+		{"skips when dir perms match", func() string {
+			os.Chmod(dir, 0777)
+			return dir
+		}, 0777},
+		{"skips when file perms match", func() string {
+			f := path.Join(dir, "f")
+			os.WriteFile(f, []byte{}, 0666)
+			return f
+		}, 0666},
+		{"chmods when perms differ", func() string {
+			f := path.Join(dir, "g")
+			os.WriteFile(f, []byte{}, 0600)
+			return f
+		}, 0666},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			path := tt.setup()
+			want := tt.wantPerm
+			if want == 0666 {
+				err = chmodIfNeeded(path, defaultFilePermission)
+			} else {
+				err = chmodIfNeeded(path, defaultDirPermission)
+			}
+			if err != nil {
+				t.Fatalf("chmodIfNeeded: %v", err)
+			}
+			info, _ := os.Stat(path)
+			if got := info.Mode().Perm(); got != want {
+				t.Errorf("perms: got %o, want %o", got, want)
+			}
+		})
+	}
+}
+
+func TestEnsureDirExists(t *testing.T) {
+	dir, err := os.MkdirTemp("", "knox-ensure-test")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(dir)
+
+	tests := []struct {
+		name    string
+		setup   func() string
+		wantErr bool
+	}{
+		{"creates when missing", func() string { return path.Join(dir, "a", "b") }, false},
+		{"succeeds when already exists", func() string {
+			target := path.Join(dir, "existing")
+			os.MkdirAll(target, 0700)
+			return target
+		}, false},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			target := tt.setup()
+			err := ensureDirExists(target, 0755)
+			if (err != nil) != tt.wantErr {
+				t.Fatalf("ensureDirExists: err=%v, wantErr=%v", err, tt.wantErr)
+			}
+			info, err := os.Stat(target)
+			if err != nil {
+				t.Fatalf("stat: %v", err)
+			}
+			if !info.IsDir() {
+				t.Error("path is not a directory")
+			}
+		})
+	}
+}
+
 func TestProcessKey(t *testing.T) {
 	params, dir, d := setUpTest(t)
 	defer TearDownTest(dir)