fix: dir no longer panics when HOME and XDG_CONFIG_HOME are not set (#…

…449) This PR addresses the issue #446 In this PR I: - I removed the `init()` function from `dir/path` - When `userConfigDir()` returns an error, instead of `panic(err)` I default to the current directory instead - Split `loadUserPath()` into two new functions used to setup and return the values for `UserConfigDir` and `UserLibexecDir` - Added additional unit tests for the two new functions and to test the default directory is used when `HOME` is set to `""` --------- Signed-off-by: Jason <jagoodse@microsoft.com> Signed-off-by: JasonTheDeveloper <jagoodse@microsoft.com> Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com> Co-authored-by: Shiwei Zhang <shizh@microsoft.com> Co-authored-by: Patrick Zheng <patrickzheng@microsoft.com>
notaryproject · Sep 4, 2024 · 4d76f9a · 4d76f9a
1 parent 115509e
commit 4d76f9a
Show file tree

Hide file tree

Showing 4 changed files with 58 additions and 26 deletions.
diff --git a/dir/fs.go b/dir/fs.go
@@ -51,10 +51,10 @@ func NewSysFS(root string) SysFS {
 
 // ConfigFS is the config SysFS
 func ConfigFS() SysFS {
-	return NewSysFS(UserConfigDir)
+	return NewSysFS(userConfigDirPath())
 }
 
 // PluginFS is the plugin SysFS
 func PluginFS() SysFS {
-	return NewSysFS(filepath.Join(UserLibexecDir, PathPlugins))
+	return NewSysFS(filepath.Join(userLibexecDirPath(), PathPlugins))
 }
diff --git a/dir/fs_test.go b/dir/fs_test.go
@@ -67,7 +67,7 @@ func TestPluginFS(t *testing.T) {
 	if err != nil {
 		t.Fatalf("SysPath() failed. err = %v", err)
 	}
-	if path != filepath.Join(UserLibexecDir, PathPlugins, "plugin") {
-		t.Fatalf(`SysPath() failed. got: %q, want: %q`, path, filepath.Join(UserLibexecDir, PathPlugins, "plugin"))
+	if path != filepath.Join(userLibexecDirPath(), PathPlugins, "plugin") {
+		t.Fatalf(`SysPath() failed. got: %q, want: %q`, path, filepath.Join(userLibexecDirPath(), PathPlugins, "plugin"))
 	}
 }
diff --git a/dir/path.go b/dir/path.go
@@ -79,21 +79,28 @@ const (
 
 var userConfigDir = os.UserConfigDir // for unit test
 
-func init() {
-	loadUserPath()
+// userConfigDirPath returns the user level {NOTATION_CONFIG} path.
+func userConfigDirPath() string {
+	if UserConfigDir == "" {
+		userDir, err := userConfigDir()
+		if err != nil {
+			// fallback to current directory
+			UserConfigDir = "." + notation
+			return UserConfigDir
+		}
+		// set user config
+		UserConfigDir = filepath.Join(userDir, notation)
+	}
+	return UserConfigDir
 }
 
-// loadUserPath function defines UserConfigDir and UserLibexecDir.
-func loadUserPath() {
-	// set user config
-	userDir, err := userConfigDir()
-	if err != nil {
-		panic(err)
+// userLibexecDirPath returns the user level {NOTATION_LIBEXEC} path.
+func userLibexecDirPath() string {
+	if UserLibexecDir == "" {
+		// set user libexec
+		UserLibexecDir = userConfigDirPath()
 	}
-	UserConfigDir = filepath.Join(userDir, notation)
-
-	// set user libexec
-	UserLibexecDir = UserConfigDir
+	return UserLibexecDir
 }
 
 // LocalKeyPath returns the local key and local cert relative paths.

diff --git a/dir/path_test.go b/dir/path_test.go
@@ -14,30 +14,53 @@
 package dir
 
 import (
-	"path/filepath"
+	"os"
 	"testing"
 )
 
 func mockGetUserConfig() (string, error) {
 	return "/path/", nil
 }
 
-func Test_loadPath(t *testing.T) {
-	wantDir := filepath.FromSlash("/path/notation")
+func setup() {
+	UserConfigDir = ""
+	UserLibexecDir = ""
+}
+
+func Test_UserConfigDirPath(t *testing.T) {
 	userConfigDir = mockGetUserConfig
-	loadUserPath()
-	if UserConfigDir != wantDir {
-		t.Fatalf(`loadPath() UserConfigDir is incorrect. got: %q, want: %q`, UserConfigDir, wantDir)
+	setup()
+	got := userConfigDirPath()
+	if got != "/path/notation" {
+		t.Fatalf(`UserConfigDirPath() = %q, want "/path/notation"`, got)
 	}
+}
 
-	if UserLibexecDir != UserConfigDir {
-		t.Fatalf(`loadPath() UserLibexecDir is incorrect. got: %q, want: %q`, UserLibexecDir, wantDir)
+func Test_NoHomeVariable(t *testing.T) {
+	t.Setenv("HOME", "")
+	t.Setenv("XDG_CONFIG_HOME", "")
+	setup()
+	userConfigDir = os.UserConfigDir
+	got := userConfigDirPath()
+	if got != ".notation" {
+		t.Fatalf(`UserConfigDirPath() = %q, want ".notation"`, UserConfigDir)
+	}
+}
+
+func Test_UserLibexecDirPath(t *testing.T) {
+	userConfigDir = mockGetUserConfig
+	setup()
+	got := userLibexecDirPath()
+	if got != "/path/notation" {
+		t.Fatalf(`UserConfigDirPath() = %q, want "/path/notation"`, got)
 	}
 }
 
 func TestLocalKeyPath(t *testing.T) {
 	userConfigDir = mockGetUserConfig
-	loadUserPath()
+	setup()
+	_ = userConfigDirPath()
+	_ = userLibexecDirPath()
 	gotKeyPath, gotCertPath := LocalKeyPath("web")
 	if gotKeyPath != "localkeys/web.key" {
 		t.Fatalf(`LocalKeyPath() gotKeyPath = %q, want "localkeys/web.key"`, gotKeyPath)
@@ -49,7 +72,9 @@ func TestLocalKeyPath(t *testing.T) {
 
 func TestX509TrustStoreDir(t *testing.T) {
 	userConfigDir = mockGetUserConfig
-	loadUserPath()
+	setup()
+	_ = userConfigDirPath()
+	_ = userLibexecDirPath()
 	if got := X509TrustStoreDir("ca", "web"); got != "truststore/x509/ca/web" {
 		t.Fatalf(`X509TrustStoreDir() = %q, want "truststore/x509/ca/web"`, got)
 	}