From fef555185b699d3bcd0adbc5487ff9ac2351f728 Mon Sep 17 00:00:00 2001 From: Orpheus Lummis Date: Mon, 10 Apr 2023 23:00:37 -0400 Subject: [PATCH 1/4] handle more usage errors --- cli/cli.go | 10 +++++++--- cli/errors.go | 4 ++-- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/cli/cli.go b/cli/cli.go index 0f3c896c5e..0c0d813fe7 100644 --- a/cli/cli.go +++ b/cli/cli.go @@ -28,14 +28,18 @@ import ( const badgerDatastoreName = "badger" -// List of cobra errors indicating an error occurred in the way the command was invoked. -// They are subject to change with new versions of cobra. +// Errors with how the command is invoked by user var usageErrors = []string{ + // cobra errors - subject to change with new versions of cobra "flag needs an argument", "invalid syntax", "unknown flag", "unknown shorthand flag", - "missing argument", // custom to defradb + "unknown command", + // custom defradb errors + errMissingArg, + errMissingArgs, + errTooManyArgs, } var log = logging.MustNewLogger("defra.cli") diff --git a/cli/errors.go b/cli/errors.go index bd04c008a5..be0b75ff5f 100644 --- a/cli/errors.go +++ b/cli/errors.go @@ -13,8 +13,8 @@ package cli import "github.com/sourcenetwork/defradb/errors" const ( - errMissingArg string = "missing arguement" - errMissingArgs string = "missing arguements" + errMissingArg string = "missing argument" + errMissingArgs string = "missing arguments" errTooManyArgs string = "too many arguments" errEmptyStdin string = "empty stdin" errEmptyFile string = "empty file" From bf957c445beb7c8d6cede316117e468eecbb720c Mon Sep 17 00:00:00 2001 From: Orpheus Lummis Date: Mon, 10 Apr 2023 23:02:12 -0400 Subject: [PATCH 2/4] fix Conflicting arguments in CLI command init --- cli/init.go | 16 ++++++---------- cli/root.go | 10 +++------- cli/start.go | 5 +---- tests/integration/cli/log_config_test.go | 2 +- 4 files changed, 11 insertions(+), 22 deletions(-) diff --git a/cli/init.go b/cli/init.go index 420db58c0c..75d2186d7a 100644 --- a/cli/init.go +++ b/cli/init.go @@ -30,7 +30,7 @@ It covers three possible situations: - root dir exists and contains a config file */ var initCmd = &cobra.Command{ - Use: "init [rootdir]", + Use: "init", Short: "Initialize DefraDB's root directory and configuration file", Long: "Initialize a directory for configuration and data at the given path.", // Load a default configuration, considering env. variables and CLI flags. @@ -41,15 +41,6 @@ var initCmd = &cobra.Command{ return nil }, RunE: func(cmd *cobra.Command, args []string) error { - cfg.Rootdir = config.DefaultRootDir() - if len(args) == 1 { - cfg.Rootdir = args[0] - } else if len(args) > 1 { - if err := cmd.Usage(); err != nil { - return err - } - return errors.New("init command requires one rootdir argument, or no argument") - } if config.FolderExists(cfg.Rootdir) { if cfg.ConfigFileExists() { if reinitialize { @@ -89,4 +80,9 @@ func init() { &reinitialize, "reinitialize", false, "Reinitialize the configuration file", ) + + initCmd.Flags().StringVar( + &cfg.Rootdir, "rootdir", config.DefaultRootDir(), + "Directory for data and configuration to use", + ) } diff --git a/cli/root.go b/cli/root.go index 1b639133bc..8a5695c782 100644 --- a/cli/root.go +++ b/cli/root.go @@ -16,11 +16,10 @@ import ( "github.com/spf13/cobra" + "github.com/sourcenetwork/defradb/config" "github.com/sourcenetwork/defradb/errors" ) -var rootDirParam string - var rootCmd = &cobra.Command{ Use: "defradb", Short: "DefraDB Edge Database", @@ -35,9 +34,6 @@ See https://docs.source.network/BSL.txt for more information. // Loads the rootDir containing the configuration file, otherwise warn about it and load a default configuration. // This allows some subcommands (`init`, `start`) to override the PreRun to create a rootDir by default. PersistentPreRunE: func(cmd *cobra.Command, _ []string) error { - if rootDirParam != "" { - cfg.Rootdir = rootDirParam - } if cfg.ConfigFileExists() { if err := cfg.LoadWithRootdir(true); err != nil { return errors.Wrap("failed to load config", err) @@ -55,8 +51,8 @@ See https://docs.source.network/BSL.txt for more information. func init() { rootCmd.PersistentFlags().StringVar( - &rootDirParam, "rootdir", "", - "Directory for data and configuration to use (default \"$HOME/.defradb\")", + &cfg.Rootdir, "rootdir", config.DefaultRootDir(), + "Directory for data and configuration to use", ) rootCmd.PersistentFlags().String( diff --git a/cli/start.go b/cli/start.go index 9716ae461e..5dce11c104 100644 --- a/cli/start.go +++ b/cli/start.go @@ -45,9 +45,6 @@ var startCmd = &cobra.Command{ Long: "Start a new instance of DefraDB node.", // Load the root config if it exists, otherwise create it. PersistentPreRunE: func(cmd *cobra.Command, _ []string) error { - if rootDirParam != "" { - cfg.Rootdir = rootDirParam - } if cfg.ConfigFileExists() { if err := cfg.LoadWithRootdir(true); err != nil { return errors.Wrap("failed to load config", err) @@ -65,10 +62,10 @@ var startCmd = &cobra.Command{ if err := cfg.CreateRootDirAndConfigFile(); err != nil { return err } - } } log.FeedbackInfo(cmd.Context(), fmt.Sprintf("Configuration loaded from DefraDB directory %v", cfg.Rootdir)) + return nil }, RunE: func(cmd *cobra.Command, args []string) error { diff --git a/tests/integration/cli/log_config_test.go b/tests/integration/cli/log_config_test.go index fffdcfbef6..043eae5b7c 100644 --- a/tests/integration/cli/log_config_test.go +++ b/tests/integration/cli/log_config_test.go @@ -77,7 +77,7 @@ func captureLogLines(t *testing.T, setup func(), predicate func()) []string { // Set the default logger output path to a file in the temp dir // so that production logs don't polute and confuse the tests // os.Args = append(os.Args, "--logoutput", directory+"/log.txt") - os.Args = append(os.Args, "init", directory) + os.Args = append(os.Args, "init", "--rootdir", directory) setup() cli.Execute() From 04deae0668411ec7dde246a4437711c1a394472d Mon Sep 17 00:00:00 2001 From: Orpheus Lummis Date: Mon, 10 Apr 2023 23:02:24 -0400 Subject: [PATCH 3/4] fix "Configuration loaded from DefraDB directory..." shown twice on start --- cli/start.go | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/cli/start.go b/cli/start.go index 5dce11c104..c43a62891d 100644 --- a/cli/start.go +++ b/cli/start.go @@ -49,7 +49,6 @@ var startCmd = &cobra.Command{ if err := cfg.LoadWithRootdir(true); err != nil { return errors.Wrap("failed to load config", err) } - log.FeedbackInfo(cmd.Context(), fmt.Sprintf("Configuration loaded from DefraDB directory %v", cfg.Rootdir)) } else { if err := cfg.LoadWithRootdir(false); err != nil { return errors.Wrap("failed to load config", err) @@ -216,11 +215,7 @@ func start(ctx context.Context) (*defraInstance, error) { var err error if cfg.Datastore.Store == badgerDatastoreName { - log.FeedbackInfo( - ctx, - "Opening badger store", - logging.NewKV("Path", cfg.Datastore.Badger.Path), - ) + log.FeedbackInfo(ctx, "Opening badger store", logging.NewKV("Path", cfg.Datastore.Badger.Path)) rootstore, err = badgerds.NewDatastore( cfg.Datastore.Badger.Path, cfg.Datastore.Badger.Options, From afbd42d1cfab7d10c4042e7fb8babf54af8c216d Mon Sep 17 00:00:00 2001 From: Orpheus Lummis Date: Mon, 10 Apr 2023 23:02:52 -0400 Subject: [PATCH 4/4] fix Erroneous badger path when using start with relative rootdir and TLS certificates pathing --- config/config.go | 21 +++++++++++++++++---- config/configfile_yaml.gotmpl | 8 +++----- 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/config/config.go b/config/config.go index 17595bcfed..2e67eadc49 100644 --- a/config/config.go +++ b/config/config.go @@ -101,6 +101,8 @@ func DefaultConfig() *Config { // Use on a Config struct already loaded with default values from DefaultConfig(). // To be executed once at the beginning of the program. func (cfg *Config) LoadWithRootdir(withRootdir bool) error { + var err error + // Use default logging configuration here, so that // we can log errors in a consistent way even in the case of early failure. defaultLogCfg := defaultLogConfig() @@ -112,6 +114,12 @@ func (cfg *Config) LoadWithRootdir(withRootdir bool) error { return err } + // using absolute rootdir for robustness. + cfg.Rootdir, err = filepath.Abs(cfg.Rootdir) + if err != nil { + return err + } + if withRootdir { cfg.v.AddConfigPath(cfg.Rootdir) if err := cfg.v.ReadInConfig(); err != nil { @@ -180,10 +188,16 @@ func (cfg *Config) validate() error { } func (cfg *Config) paramsPreprocessing() error { - // We prefer using absolute paths. + // We prefer using absolute paths, relative to the rootdir. if !filepath.IsAbs(cfg.v.GetString("datastore.badger.path")) { cfg.v.Set("datastore.badger.path", filepath.Join(cfg.Rootdir, cfg.v.GetString("datastore.badger.path"))) } + if !filepath.IsAbs(cfg.v.GetString("api.privkeypath")) { + cfg.v.Set("api.privkeypath", filepath.Join(cfg.Rootdir, cfg.v.GetString("api.privkeypath"))) + } + if !filepath.IsAbs(cfg.v.GetString("api.pubkeypath")) { + cfg.v.Set("api.pubkeypath", filepath.Join(cfg.Rootdir, cfg.v.GetString("api.pubkeypath"))) + } // log.logger configuration as a string logloggerAsStringSlice := cfg.v.GetStringSlice("log.logger") @@ -269,12 +283,11 @@ type APIConfig struct { } func defaultAPIConfig() *APIConfig { - rootDir := DefaultRootDir() return &APIConfig{ Address: "localhost:9181", TLS: false, - PubKeyPath: filepath.Join(rootDir, "certs/server.key"), - PrivKeyPath: filepath.Join(rootDir, "certs/server.crt"), + PubKeyPath: "certs/server.key", + PrivKeyPath: "certs/server.crt", Email: DefaultAPIEmail, } } diff --git a/config/configfile_yaml.gotmpl b/config/configfile_yaml.gotmpl index d1d7e5db7a..77dc7f541e 100644 --- a/config/configfile_yaml.gotmpl +++ b/config/configfile_yaml.gotmpl @@ -1,8 +1,7 @@ # DefraDB configuration (YAML) -# NOTE: Paths below are relative to the DefraDB directory unless otherwise stated. -# By default, the DefraDB directory is "$HOME/.defradb", but -# can be changed via the $DEFRA_ROOTDIR env variable or --rootdir CLI flag. +# The default DefraDB directory is "$HOME/.defradb". It can be changed via the --rootdir CLI flag. +# Relative paths are interpreted as being rooted in the DefraDB directory. datastore: # Store can be badger | memory @@ -10,8 +9,7 @@ datastore: # memory: in-memory version of badger store: {{ .Datastore.Store }} badger: - # The path to the database data file(s), relative paths here will be converted to absolute file paths - # on database start. + # The path to the database data file(s). path: {{ .Datastore.Badger.Path }} # Maximum file size of the value log files. The in-memory file size will be 2*valuelogfilesize. # Human friendly units can be used (ex: 500MB).