fix: add error handling for LoadConfigOnce() (#520)

* Added error handling and unit tests.
* WIP for #516 
* Resolves #525 

Signed-off-by: Junjie Gao <junjiegao@microsoft.com>

pkg/configutil/once.go

@@ -12,10 +12,6 @@ var (
 	// configInfo is the config.json data
 	configInfo *config.Config
 	configOnce sync.Once
-
-	// signingKeyInfo if the signingkeys.json data
-	signingKeysInfo *config.SigningKeys
-	signingKeysOnce sync.Once
 )
 
 // LoadConfigOnce returns the previously read config file.
@@ -26,6 +22,9 @@ func LoadConfigOnce() (*config.Config, error) {
 	var err error
 	configOnce.Do(func() {
 		configInfo, err = config.LoadConfig()
+		if err != nil {
+			return
+		}
 		// set default value
 		configInfo.SignatureFormat = strings.ToLower(configInfo.SignatureFormat)
 		if configInfo.SignatureFormat == "" {

pkg/configutil/testdata/config.json

@@ -0,0 +1,3 @@
+{
+    "insecureRegistries": ["reg1.io"]
+}

pkg/configutil/testdata/no_default_key_signingkeys/signingkeys.json

@@ -0,0 +1,9 @@
+{
+    "keys": [
+        {
+            "name": "e2e",
+            "keyPath": "notation/localkeys/e2e.key",
+            "certPath": "notation/localkeys/e2e.crt"
+        }
+    ]
+}

pkg/configutil/testdata/valid_signingkeys/signingkeys.json

@@ -0,0 +1,10 @@
+{
+    "default": "e2e",
+    "keys": [
+        {
+            "name": "e2e",
+            "keyPath": "notation/localkeys/e2e.key",
+            "certPath": "notation/localkeys/e2e.crt"
+        }
+    ]
+}

pkg/configutil/util.go

@@ -38,6 +38,6 @@ func ResolveKey(name string) (config.KeySuite, error) {
 	if name == "" {
 		return signingKeys.GetDefault()
 	}
-	
+
 	return signingKeys.Get(name)
 }

pkg/configutil/util_test.go

@@ -0,0 +1,139 @@
+package configutil
+
+import (
+	"os"
+	"path/filepath"
+	"strings"
+	"sync"
+	"testing"
+
+	"github.com/notaryproject/notation-go/dir"
+)
+
+func TestIsRegistryInsecure(t *testing.T) {
+	configOnce = sync.Once{}
+	// for restore dir
+	defer func(oldDir string) {
+		dir.UserConfigDir = oldDir
+		configOnce = sync.Once{}
+	}(dir.UserConfigDir)
+	// update config dir
+	dir.UserConfigDir = "testdata"
+
+	type args struct {
+		target string
+	}
+	tests := []struct {
+		name string
+		args args
+		want bool
+	}{
+		{name: "hit registry", args: args{target: "reg1.io"}, want: true},
+		{name: "miss registry", args: args{target: "reg2.io"}, want: false},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := IsRegistryInsecure(tt.args.target); got != tt.want {
+				t.Errorf("IsRegistryInsecure() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+
+}
+
+func TestIsRegistryInsecureMissingConfig(t *testing.T) {
+	configOnce = sync.Once{}
+	// for restore dir
+	defer func(oldDir string) {
+		dir.UserConfigDir = oldDir
+		configOnce = sync.Once{}
+	}(dir.UserConfigDir)
+	// update config dir
+	dir.UserConfigDir = "./testdata2"
+
+	type args struct {
+		target string
+	}
+	tests := []struct {
+		name string
+		args args
+		want bool
+	}{
+		{name: "missing config", args: args{target: "reg1.io"}, want: false},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := IsRegistryInsecure(tt.args.target); got != tt.want {
+				t.Errorf("IsRegistryInsecure() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
+
+func TestIsRegistryInsecureConfigPermissionError(t *testing.T) {
+	configDir := "./testdata"
+	// for restore dir
+	defer func(oldDir string) error {
+		// restore permission
+		dir.UserConfigDir = oldDir
+		configOnce = sync.Once{}
+		return os.Chmod(filepath.Join(configDir, "config.json"), 0644)
+	}(dir.UserConfigDir)
+
+	// update config dir
+	dir.UserConfigDir = configDir
+
+	// forbid reading the file
+	if err := os.Chmod(filepath.Join(configDir, "config.json"), 0000); err != nil {
+		t.Error(err)
+	}
+
+	if IsRegistryInsecure("reg1.io") {
+		t.Error("should false because of missing config.json read permission.")
+	}
+}
+
+func TestResolveKey(t *testing.T) {
+	defer func(oldDir string) {
+		dir.UserConfigDir = oldDir
+	}(dir.UserConfigDir)
+
+	t.Run("valid e2e key", func(t *testing.T) {
+		dir.UserConfigDir = "./testdata/valid_signingkeys"
+		keySuite, err := ResolveKey("e2e")
+		if err != nil {
+			t.Fatal(err)
+		}
+		if keySuite.Name != "e2e" {
+			t.Error("key name is not correct.")
+		}
+	})
+
+	t.Run("key name is empty (using default key)", func(t *testing.T) {
+		dir.UserConfigDir = "./testdata/valid_signingkeys"
+		keySuite, err := ResolveKey("")
+		if err != nil {
+			t.Fatal(err)
+		}
+		if keySuite.Name != "e2e" {
+			t.Error("key name is not correct.")
+		}
+	})
+
+	t.Run("signingkeys.json without read permission", func(t *testing.T) {
+		dir.UserConfigDir = "./testdata/valid_signingkeys"
+		defer func() error {
+			// restore the permission
+			return os.Chmod(filepath.Join(dir.UserConfigDir, "signingkeys.json"), 0644)
+		}()
+
+		// forbid reading the file
+		if err := os.Chmod(filepath.Join(dir.UserConfigDir, "signingkeys.json"), 0000); err != nil {
+			t.Error(err)
+		}
+		_, err := ResolveKey("")
+		if !strings.Contains(err.Error(), "permission denied") {
+			t.Error("should error with permission denied")
+		}
+	})
+}