Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: load config error #1145

Merged
merged 7 commits into from
Jan 14, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions internal/cmd/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,14 +44,14 @@
Usage: "signature envelope format, options: \"jws\", \"cose\"",
}
SetPflagSignatureFormat = func(fs *pflag.FlagSet, p *string) {
defaultSignatureFormat := envelope.JWS
// load config to get signatureFormat
config, err := configutil.LoadConfigOnce()
if err == nil && config.SignatureFormat != "" {
defaultSignatureFormat = config.SignatureFormat
if err != nil || config.SignatureFormat == "" {
fs.StringVar(p, PflagSignatureFormat.Name, envelope.JWS, PflagSignatureFormat.Usage)
return

Check warning on line 50 in internal/cmd/flags.go

View check run for this annotation

Codecov / codecov/patch

internal/cmd/flags.go#L49-L50

Added lines #L49 - L50 were not covered by tests
}

fs.StringVar(p, PflagSignatureFormat.Name, defaultSignatureFormat, PflagSignatureFormat.Usage)
// set signatureFormat from config
fs.StringVar(p, PflagSignatureFormat.Name, config.SignatureFormat, PflagSignatureFormat.Usage)
}

PflagID = &pflag.Flag{
Expand Down
36 changes: 18 additions & 18 deletions pkg/configutil/once.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,28 +21,28 @@ import (
"github.com/notaryproject/notation/internal/envelope"
)

var (
// configInfo is the config.json data
configInfo *config.Config
configOnce sync.Once
)
// loadConfigOnce is a function that invokes loadConfig only once.
var loadConfigOnce = sync.OnceValues(loadConfig)

// LoadConfigOnce returns the previously read config file.
// If previous config file does not exist, it reads the config from file
// or return a default config if not found.
// The returned config is only suitable for read only scenarios for short-lived processes.
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 == "" {
configInfo.SignatureFormat = envelope.JWS
}
})
return configInfo, err
return loadConfigOnce()
}

// loadConfig reads the config from file or return a default config if not
// found.
func loadConfig() (*config.Config, error) {
configInfo, err := config.LoadConfig()
if err != nil {
return nil, err
}
// set default value
configInfo.SignatureFormat = strings.ToLower(configInfo.SignatureFormat)
if configInfo.SignatureFormat == "" {
configInfo.SignatureFormat = envelope.JWS
}
return configInfo, nil
}
31 changes: 30 additions & 1 deletion pkg/configutil/once_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,19 @@
package configutil

import (
"os"
"path/filepath"
"strings"
"sync"
"testing"

"github.com/notaryproject/notation-go/dir"
)

func TestLoadConfigOnce(t *testing.T) {
defer func() {
loadConfigOnce = sync.OnceValues(loadConfig)
}()
config1, err := LoadConfigOnce()
if err != nil {
t.Fatal("LoadConfigOnce failed.")
Expand All @@ -27,6 +36,26 @@ func TestLoadConfigOnce(t *testing.T) {
t.Fatal("LoadConfigOnce failed.")
}
if config1 != config2 {
t.Fatal("LoadConfigOnce is invalid.")
t.Fatal("LoadConfigOnce should return the same config.")
}
}

func TestLoadConfigOnceError(t *testing.T) {
dir.UserConfigDir = t.TempDir()
defer func() {
dir.UserConfigDir = ""
loadConfigOnce = sync.OnceValues(loadConfig)
}()
if err := os.WriteFile(filepath.Join(dir.UserConfigDir, dir.PathConfigFile), []byte("invalid json"), 0600); err != nil {
t.Fatal("Failed to create file.")
}

_, err := LoadConfigOnce()
if err == nil || !strings.Contains(err.Error(), "invalid character") {
t.Fatal("LoadConfigOnce should fail.")
}
_, err2 := LoadConfigOnce()
if err != err2 {
t.Fatal("LoadConfigOnce should return the same error.")
}
}
9 changes: 4 additions & 5 deletions pkg/configutil/util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,10 @@ import (
)

func TestIsRegistryInsecure(t *testing.T) {
configOnce = sync.Once{}
// for restore dir
defer func(oldDir string) {
dir.UserConfigDir = oldDir
configOnce = sync.Once{}
loadConfigOnce = sync.OnceValues(loadConfig)
}(dir.UserConfigDir)
// update config dir
dir.UserConfigDir = "testdata"
Expand All @@ -56,11 +55,10 @@ func TestIsRegistryInsecure(t *testing.T) {
}

func TestIsRegistryInsecureMissingConfig(t *testing.T) {
configOnce = sync.Once{}
// for restore dir
defer func(oldDir string) {
dir.UserConfigDir = oldDir
configOnce = sync.Once{}
loadConfigOnce = sync.OnceValues(loadConfig)
}(dir.UserConfigDir)
// update config dir
dir.UserConfigDir = "./testdata2"
Expand Down Expand Up @@ -93,7 +91,7 @@ func TestIsRegistryInsecureConfigPermissionError(t *testing.T) {
defer func(oldDir string) error {
// restore permission
dir.UserConfigDir = oldDir
configOnce = sync.Once{}
loadConfigOnce = sync.OnceValues(loadConfig)
return os.Chmod(filepath.Join(configDir, "config.json"), 0644)
}(dir.UserConfigDir)

Expand All @@ -113,6 +111,7 @@ func TestIsRegistryInsecureConfigPermissionError(t *testing.T) {
func TestResolveKey(t *testing.T) {
defer func(oldDir string) {
dir.UserConfigDir = oldDir
loadConfigOnce = sync.OnceValues(loadConfig)
}(dir.UserConfigDir)

t.Run("valid e2e key", func(t *testing.T) {
Expand Down
Loading