diff --git a/cmd/crowdsec-cli/alerts.go b/cmd/crowdsec-cli/alerts.go index 81753e49e..a70847d03 100644 --- a/cmd/crowdsec-cli/alerts.go +++ b/cmd/crowdsec-cli/alerts.go @@ -18,6 +18,7 @@ import ( "github.com/crowdsecurity/crowdsec/pkg/models" "github.com/go-openapi/strfmt" "github.com/olekukonko/tablewriter" + "github.com/pkg/errors" log "github.com/sirupsen/logrus" "github.com/spf13/cobra" "gopkg.in/yaml.v2" @@ -197,20 +198,14 @@ func NewAlertsCmd() *cobra.Command { Short: "Manage alerts", Args: cobra.MinimumNArgs(1), DisableAutoGenTag: true, - PersistentPreRun: func(cmd *cobra.Command, args []string) { + PersistentPreRunE: func(cmd *cobra.Command, args []string) error { var err error if err := csConfig.LoadAPIClient(); err != nil { - log.Fatalf("loading api client: %s", err.Error()) - } - if csConfig.API.Client == nil { - log.Fatalln("There is no configuration on 'api_client:'") - } - if csConfig.API.Client.Credentials == nil { - log.Fatalf("Please provide credentials for the API in '%s'", csConfig.API.Client.CredentialsFilePath) + return errors.Wrap(err, "loading api client") } apiURL, err := url.Parse(csConfig.API.Client.Credentials.URL) if err != nil { - log.Fatalf("parsing api url: %s", apiURL) + return errors.Wrapf(err, "parsing api url %s", apiURL) } Client, err = apiclient.NewClient(&apiclient.Config{ MachineID: csConfig.API.Client.Credentials.Login, @@ -221,8 +216,9 @@ func NewAlertsCmd() *cobra.Command { }) if err != nil { - log.Fatalf("new api client: %s", err.Error()) + return errors.Wrap(err, "new api client") } + return nil }, } diff --git a/cmd/crowdsec-cli/decisions.go b/cmd/crowdsec-cli/decisions.go index ec3891bbd..7eccff15d 100644 --- a/cmd/crowdsec-cli/decisions.go +++ b/cmd/crowdsec-cli/decisions.go @@ -19,6 +19,7 @@ import ( "github.com/go-openapi/strfmt" "github.com/jszwec/csvutil" "github.com/olekukonko/tablewriter" + "github.com/pkg/errors" log "github.com/sirupsen/logrus" "github.com/spf13/cobra" ) @@ -147,20 +148,14 @@ func NewDecisionsCmd() *cobra.Command { /*TBD example*/ Args: cobra.MinimumNArgs(1), DisableAutoGenTag: true, - PersistentPreRun: func(cmd *cobra.Command, args []string) { + PersistentPreRunE: func(cmd *cobra.Command, args []string) error { if err := csConfig.LoadAPIClient(); err != nil { - log.Fatalf(err.Error()) - } - if csConfig.API.Client == nil { - log.Fatalln("There is no configuration on 'api_client:'") - } - if csConfig.API.Client.Credentials == nil { - log.Fatalf("Please provide credentials for the API in '%s'", csConfig.API.Client.CredentialsFilePath) + return errors.Wrap(err, "loading api client") } password := strfmt.Password(csConfig.API.Client.Credentials.Password) apiurl, err := url.Parse(csConfig.API.Client.Credentials.URL) if err != nil { - log.Fatalf("parsing api url ('%s'): %s", csConfig.API.Client.Credentials.URL, err) + return errors.Wrapf(err, "parsing api url %s", csConfig.API.Client.Credentials.URL) } Client, err = apiclient.NewClient(&apiclient.Config{ MachineID: csConfig.API.Client.Credentials.Login, @@ -170,8 +165,9 @@ func NewDecisionsCmd() *cobra.Command { VersionPrefix: "v1", }) if err != nil { - log.Fatalf("creating api client : %s", err) + return errors.Wrap(err, "creating api client") } + return nil }, } diff --git a/cmd/crowdsec-cli/lapi.go b/cmd/crowdsec-cli/lapi.go index 88e4bb004..b02167fbc 100644 --- a/cmd/crowdsec-cli/lapi.go +++ b/cmd/crowdsec-cli/lapi.go @@ -13,6 +13,7 @@ import ( "github.com/crowdsecurity/crowdsec/pkg/cwversion" "github.com/crowdsecurity/crowdsec/pkg/models" "github.com/go-openapi/strfmt" + "github.com/pkg/errors" log "github.com/sirupsen/logrus" "github.com/spf13/cobra" "gopkg.in/yaml.v2" @@ -29,13 +30,7 @@ func NewLapiCmd() *cobra.Command { DisableAutoGenTag: true, PersistentPreRunE: func(cmd *cobra.Command, args []string) error { if err := csConfig.LoadAPIClient(); err != nil { - return fmt.Errorf("loading api client: %s", err.Error()) - } - if csConfig.API.Client == nil { - log.Fatalln("There is no API->client configuration") - } - if csConfig.API.Client.Credentials == nil { - log.Fatalf("no configuration for Local API (LAPI) in '%s'", *csConfig.FilePath) + return errors.Wrap(err, "loading api client") } return nil }, diff --git a/cmd/crowdsec-cli/main.go b/cmd/crowdsec-cli/main.go index 9a2025cc9..388570ecf 100644 --- a/cmd/crowdsec-cli/main.go +++ b/cmd/crowdsec-cli/main.go @@ -183,6 +183,6 @@ It is meant to allow you to manage bans, parsers/scenarios/etc, api and generall rootCmd.AddCommand(NewHubTestCmd()) rootCmd.AddCommand(NewNotificationsCmd()) if err := rootCmd.Execute(); err != nil { - log.Fatalf("While executing root command : %s", err) + log.Fatal(err) } } diff --git a/cmd/crowdsec/api.go b/cmd/crowdsec/api.go index 2dbce31b0..e6ff17c84 100644 --- a/cmd/crowdsec/api.go +++ b/cmd/crowdsec/api.go @@ -14,7 +14,7 @@ import ( func initAPIServer(cConfig *csconfig.Config) (*apiserver.APIServer, error) { apiServer, err := apiserver.NewServer(cConfig.API.Server) if err != nil { - return nil, fmt.Errorf("unable to run local API: %s", err) + return nil, errors.Wrap(err, "unable to run local API") } if hasPlugins(cConfig.API.Server.Profiles) { diff --git a/cmd/crowdsec/pour.go b/cmd/crowdsec/pour.go index 9b61b1b38..c5400f3df 100644 --- a/cmd/crowdsec/pour.go +++ b/cmd/crowdsec/pour.go @@ -42,7 +42,7 @@ func runPour(input chan types.Event, holders []leaky.BucketFactory, buckets *lea //here we can bucketify with parsed poured, err := leaky.PourItemToHolders(parsed, holders, buckets) if err != nil { - log.Fatalf("bucketify failed for: %v", parsed) + log.Errorf("bucketify failed for: %v", parsed) return fmt.Errorf("process of event failed : %v", err) } if poured { diff --git a/cmd/crowdsec/run_in_svc.go b/cmd/crowdsec/run_in_svc.go index d1197aeda..11d6f06ea 100644 --- a/cmd/crowdsec/run_in_svc.go +++ b/cmd/crowdsec/run_in_svc.go @@ -51,7 +51,11 @@ func StartRunSvc() { if exitCode, err := Serve(cConfig); err != nil { if err != nil { - log.Errorf(err.Error()) + // this method of logging a fatal error does not + // trigger a program exit (as stated by the authors, it + // is not going to change in logrus to keep backward + // compatibility), and allows us to report coverage. + log.NewEntry(log.StandardLogger()).Log(log.FatalLevel, err) if !bincoverTesting { os.Exit(exitCode) } diff --git a/pkg/apiserver/apiserver.go b/pkg/apiserver/apiserver.go index e84655049..b8228383f 100644 --- a/pkg/apiserver/apiserver.go +++ b/pkg/apiserver/apiserver.go @@ -94,7 +94,7 @@ func NewServer(config *csconfig.LocalApiServerCfg) (*APIServer, error) { var flushScheduler *gocron.Scheduler dbClient, err := database.NewClient(config.DbConfig) if err != nil { - return &APIServer{}, fmt.Errorf("unable to init database client: %s", err) + return &APIServer{}, errors.Wrap(err, "unable to init database client") } if config.DbConfig.Flush != nil { diff --git a/pkg/apiserver/apiserver_test.go b/pkg/apiserver/apiserver_test.go index ece5ec91e..d1236681e 100644 --- a/pkg/apiserver/apiserver_test.go +++ b/pkg/apiserver/apiserver_test.go @@ -289,7 +289,7 @@ func TestWithWrongDBConfig(t *testing.T) { apiServer, err := NewServer(config.API.Server) assert.Equal(t, apiServer, &APIServer{}) - assert.Equal(t, "unable to init database client: unknown database type", err.Error()) + assert.Equal(t, "unable to init database client: unknown database type 'test'", err.Error()) } func TestWithWrongFlushConfig(t *testing.T) { diff --git a/pkg/csconfig/api.go b/pkg/csconfig/api.go index b77b5a378..b546b1b56 100644 --- a/pkg/csconfig/api.go +++ b/pkg/csconfig/api.go @@ -58,18 +58,20 @@ func (l *LocalApiClientCfg) Load() error { patcher := yamlpatch.NewPatcher(l.CredentialsFilePath, ".local") fcontent, err := patcher.MergedPatchContent() if err != nil { - return errors.Wrapf(err, "failed to read api client credential configuration file '%s'", l.CredentialsFilePath) + return err } err = yaml.UnmarshalStrict(fcontent, &l.Credentials) if err != nil { return errors.Wrapf(err, "failed unmarshaling api client credential configuration file '%s'", l.CredentialsFilePath) } + if l.Credentials == nil || l.Credentials.URL == "" { + return fmt.Errorf("no credentials or URL found in api client configuration '%s'", l.CredentialsFilePath) + } + if l.Credentials != nil && l.Credentials.URL != "" { if !strings.HasSuffix(l.Credentials.URL, "/") { l.Credentials.URL = l.Credentials.URL + "/" } - } else { - log.Warningf("no credentials or URL found in api client configuration '%s'", l.CredentialsFilePath) } if l.InsecureSkipVerify == nil { apiclient.InsecureSkipVerify = false @@ -177,13 +179,13 @@ func (c *Config) LoadAPIServer() error { } func (c *Config) LoadAPIClient() error { - if c.API != nil && c.API.Client != nil && c.API.Client.CredentialsFilePath != "" && !c.DisableAgent { - if err := c.API.Client.Load(); err != nil { - return err - } - } else { + if c.API == nil || c.API.Client == nil || c.API.Client.CredentialsFilePath == "" || c.DisableAgent { return fmt.Errorf("no API client section in configuration") } + if err := c.API.Client.Load(); err != nil { + return err + } + return nil } diff --git a/pkg/csconfig/config_test.go b/pkg/csconfig/config_test.go index cfa16726d..d49e25afb 100644 --- a/pkg/csconfig/config_test.go +++ b/pkg/csconfig/config_test.go @@ -18,9 +18,9 @@ func TestNormalLoad(t *testing.T) { _, err = NewConfig("./tests/xxx.yaml", false, false) if runtime.GOOS != "windows" { - assert.EqualError(t, err, "while reading ./tests/xxx.yaml: open ./tests/xxx.yaml: no such file or directory") + assert.EqualError(t, err, "while reading yaml file: open ./tests/xxx.yaml: no such file or directory") } else { - assert.EqualError(t, err, "while reading ./tests/xxx.yaml: open ./tests/xxx.yaml: The system cannot find the file specified.") + assert.EqualError(t, err, "while reading yaml file: open ./tests/xxx.yaml: The system cannot find the file specified.") } _, err = NewConfig("./tests/simulation.yaml", false, false) diff --git a/pkg/csconfig/simulation_test.go b/pkg/csconfig/simulation_test.go index 080d3212a..a64c69a97 100644 --- a/pkg/csconfig/simulation_test.go +++ b/pkg/csconfig/simulation_test.go @@ -89,7 +89,7 @@ func TestSimulationLoading(t *testing.T) { }, Crowdsec: &CrowdsecServiceCfg{}, }, - err: fmt.Sprintf("while reading %s: open %s: The system cannot find the file specified.", testXXFullPath, testXXFullPath), + err: fmt.Sprintf("while reading yaml file: open %s: The system cannot find the file specified.", testXXFullPath), }) } else { tests = append(tests, struct { @@ -106,7 +106,7 @@ func TestSimulationLoading(t *testing.T) { }, Crowdsec: &CrowdsecServiceCfg{}, }, - err: fmt.Sprintf("while reading %s: open %s: no such file or directory", testXXFullPath, testXXFullPath), + err: fmt.Sprintf("while reading yaml file: open %s: no such file or directory", testXXFullPath), }) } diff --git a/pkg/database/database.go b/pkg/database/database.go index b5bb29496..cd745fa26 100644 --- a/pkg/database/database.go +++ b/pkg/database/database.go @@ -100,7 +100,7 @@ func NewClient(config *csconfig.DatabaseCfg) (*Client, error) { } client = ent.NewClient(ent.Driver(drv), entOpt) default: - return &Client{}, fmt.Errorf("unknown database type") + return &Client{}, fmt.Errorf("unknown database type '%s'", config.Type) } if config.LogLevel != nil && *config.LogLevel >= log.DebugLevel { diff --git a/pkg/types/dataset_test.go b/pkg/types/dataset_test.go index f6436836d..4e42a5cb2 100644 --- a/pkg/types/dataset_test.go +++ b/pkg/types/dataset_test.go @@ -2,6 +2,7 @@ package types import ( "io/ioutil" + "os" "testing" "github.com/stretchr/testify/assert" @@ -10,6 +11,9 @@ import ( ) func TestDownladFile(t *testing.T) { + examplePath := "./example.txt" + defer os.Remove(examplePath) + httpmock.Activate() defer httpmock.DeactivateAndReset() //OK @@ -23,16 +27,16 @@ func TestDownladFile(t *testing.T) { "https://example.com/x", httpmock.NewStringResponder(404, "not found"), ) - err := downloadFile("https://example.com/xx", "./example.txt") + err := downloadFile("https://example.com/xx", examplePath) assert.NoError(t, err) - content, err := ioutil.ReadFile("./example.txt") + content, err := ioutil.ReadFile(examplePath) assert.Equal(t, "example content oneoneone", string(content)) assert.NoError(t, err) //bad uri - err = downloadFile("https://zz.com", "./example.txt") + err = downloadFile("https://zz.com", examplePath) assert.Error(t, err) //404 - err = downloadFile("https://example.com/x", "./example.txt") + err = downloadFile("https://example.com/x", examplePath) assert.Error(t, err) //bad target err = downloadFile("https://example.com/xx", "") diff --git a/pkg/yamlpatch/patcher.go b/pkg/yamlpatch/patcher.go index 2da696c91..6281437b2 100644 --- a/pkg/yamlpatch/patcher.go +++ b/pkg/yamlpatch/patcher.go @@ -29,7 +29,7 @@ func readYAML(filePath string) ([]byte, error) { var err error if content, err = os.ReadFile(filePath); err != nil { - return nil, errors.Wrapf(err, "while reading %s", filePath) + return nil, errors.Wrap(err, "while reading yaml file") } var yamlMap map[interface{}]interface{} diff --git a/tests/bats/01_base.bats b/tests/bats/01_base.bats index 6a48cdaee..17994de14 100644 --- a/tests/bats/01_base.bats +++ b/tests/bats/01_base.bats @@ -26,14 +26,14 @@ declare stderr #---------- -@test "$FILE cscli - usage" { +@test "${FILE} cscli - usage" { run -0 cscli assert_output --partial "Usage:" assert_output --partial "cscli [command]" assert_output --partial "Available Commands:" } -@test "$FILE cscli version" { +@test "${FILE} cscli version" { run -0 cscli version assert_output --partial "version:" assert_output --partial "Codename:" @@ -51,7 +51,7 @@ declare stderr assert_output --partial "version:" } -@test "$FILE cscli help" { +@test "${FILE} cscli help" { run -0 cscli help assert_line "Available Commands:" assert_line --regexp ".* help .* Help about any command" @@ -62,7 +62,7 @@ declare stderr assert_line "Available Commands:" } -@test "$FILE cscli alerts list: at startup returns at least one entry: community pull" { +@test "${FILE} cscli alerts list: at startup returns at least one entry: community pull" { is_db_postgres && skip # it should have been received while preparing the fixture run -0 cscli alerts list -o json @@ -80,7 +80,7 @@ declare stderr # refute_output 0 } -@test "$FILE cscli capi status" { +@test "${FILE} cscli capi status" { run -0 cscli capi status assert_output --partial "Loaded credentials from" assert_output --partial "Trying to authenticate with username" @@ -88,7 +88,7 @@ declare stderr assert_output --partial "You can successfully interact with Central API (CAPI)" } -@test "$FILE cscli config show -o human" { +@test "${FILE} cscli config show -o human" { run -0 cscli config show -o human assert_output --partial "Global:" assert_output --partial "Crowdsec:" @@ -96,7 +96,7 @@ declare stderr assert_output --partial "Local API Server:" } -@test "$FILE cscli config show -o json" { +@test "${FILE} cscli config show -o json" { run -0 cscli config show -o json assert_output --partial '"API":' assert_output --partial '"Common":' @@ -109,7 +109,7 @@ declare stderr assert_output --partial '"Prometheus":' } -@test "$FILE cscli config show -o raw" { +@test "${FILE} cscli config show -o raw" { run -0 cscli config show -o raw assert_line "api:" assert_line "common:" @@ -121,45 +121,112 @@ declare stderr assert_line "prometheus:" } -@test "$FILE cscli config show --key" { +@test "${FILE} cscli config show --key" { run -0 cscli config show --key Config.API.Server.ListenURI assert_output "127.0.0.1:8080" } -@test "$FILE cscli config backup" { +@test "${FILE} cscli config backup" { backupdir=$(TMPDIR="${BATS_TEST_TMPDIR}" mktemp -u) run -0 cscli config backup "${backupdir}" assert_output --partial "Starting configuration backup" run -1 --separate-stderr cscli config backup "${backupdir}" - run -0 echo "$stderr" + run -0 echo "${stderr}" assert_output --partial "Failed to backup configurations" assert_output --partial "file exists" rm -rf -- "${backupdir:?}" } -@test "$FILE cscli lapi status" { +@test "${FILE} cscli lapi status" { if is_db_postgres; then sleep 4; fi run -0 --separate-stderr cscli lapi status - run -0 echo "$stderr" + run -0 echo "${stderr}" assert_output --partial "Loaded credentials from" assert_output --partial "Trying to authenticate with username" assert_output --partial " on http://127.0.0.1:8080/" assert_output --partial "You can successfully interact with Local API (LAPI)" } -@test "$FILE cscli metrics" { +@test "${FILE} cscli - missing LAPI credentials file" { + LOCAL_API_CREDENTIALS=$(config_yq '.api.client.credentials_path') + rm -f "${LOCAL_API_CREDENTIALS}" + run -1 --separate-stderr cscli lapi status + run -0 echo "${stderr}" + assert_output --partial "loading api client: while reading yaml file: open ${LOCAL_API_CREDENTIALS}: no such file or directory" + + run -1 --separate-stderr cscli alerts list + run -0 echo "${stderr}" + assert_output --partial "loading api client: while reading yaml file: open ${LOCAL_API_CREDENTIALS}: no such file or directory" + + run -1 --separate-stderr cscli decisions list + run -0 echo "${stderr}" + assert_output --partial "loading api client: while reading yaml file: open ${LOCAL_API_CREDENTIALS}: no such file or directory" +} + +@test "${FILE} cscli - empty LAPI credentials file" { + LOCAL_API_CREDENTIALS=$(config_yq '.api.client.credentials_path') + truncate -s 0 "${LOCAL_API_CREDENTIALS}" + run -1 --separate-stderr cscli lapi status + run -0 echo "${stderr}" + assert_output --partial "no credentials or URL found in api client configuration '${LOCAL_API_CREDENTIALS}'" + + run -1 --separate-stderr cscli alerts list + run -0 echo "${stderr}" + assert_output --partial "no credentials or URL found in api client configuration '${LOCAL_API_CREDENTIALS}'" + + run -1 --separate-stderr cscli decisions list + run -0 echo "${stderr}" + assert_output --partial "no credentials or URL found in api client configuration '${LOCAL_API_CREDENTIALS}'" +} + +@test "${FILE} cscli - missing LAPI client settings" { + yq e 'del(.api.client)' -i "${CONFIG_YAML}" + run -1 --separate-stderr cscli lapi status + run -0 echo "${stderr}" + assert_output --partial "loading api client: no API client section in configuration" + + run -1 --separate-stderr cscli alerts list + run -0 echo "${stderr}" + assert_output --partial "loading api client: no API client section in configuration" + + run -1 --separate-stderr cscli decisions list + run -0 echo "${stderr}" + assert_output --partial "loading api client: no API client section in configuration" +} + +@test "${FILE} cscli - malformed LAPI url" { + LOCAL_API_CREDENTIALS=$(config_yq '.api.client.credentials_path') + yq e '.url="https://127.0.0.1:-80"' -i "${LOCAL_API_CREDENTIALS}" + + run -1 --separate-stderr cscli lapi status + run -0 echo "${stderr}" + assert_output --partial 'parsing api url' + assert_output --partial 'invalid port \":-80\" after host' + + run -1 --separate-stderr cscli alerts list + run -0 echo "${stderr}" + assert_output --partial 'parsing api url' + assert_output --partial 'invalid port ":-80" after host' + + run -1 --separate-stderr cscli decisions list + run -0 echo "${stderr}" + assert_output --partial 'parsing api url' + assert_output --partial 'invalid port ":-80" after host' +} + +@test "${FILE} cscli metrics" { run -0 cscli lapi status run -0 --separate-stderr cscli metrics assert_output --partial "ROUTE" assert_output --partial '/v1/watchers/login' - run -0 echo "$stderr" + run -0 echo "${stderr}" assert_output --partial "Local Api Metrics:" } -@test "$FILE 'cscli completion' with or without configuration file" { +@test "${FILE} 'cscli completion' with or without configuration file" { run -0 cscli completion bash assert_output --partial "# bash completion for cscli" run -0 cscli completion zsh @@ -172,7 +239,7 @@ declare stderr assert_output --partial "# zsh completion for cscli" } -@test "$FILE cscli hub list" { +@test "${FILE} cscli hub list" { # we check for the presence of some objects. There may be others when we # use $PACKAGE_TESTING, so the order is not important. diff --git a/tests/bats/06_crowdsec.bats b/tests/bats/06_crowdsec.bats new file mode 100755 index 000000000..a26d8927c --- /dev/null +++ b/tests/bats/06_crowdsec.bats @@ -0,0 +1,36 @@ +#!/usr/bin/env bats +# vim: ft=bats:list:ts=8:sts=4:sw=4:et:ai:si: + +set -u + +setup_file() { + load "../lib/setup_file.sh" +} + +teardown_file() { + load "../lib/teardown_file.sh" +} + +setup() { + load "../lib/setup.sh" + ./instance-data load +} + +teardown() { + ./instance-crowdsec stop +} + +# to silence shellcheck +declare stderr + +#---------- + +@test "${FILE} crowdsec - print error on exit" { + # errors that cause program termination are printed to stderr, not only logs + yq e '.db_config.type="meh"' -i "${CONFIG_YAML}" + run -1 --separate-stderr "${BIN_DIR}/crowdsec" + refute_output + run -0 echo "${stderr}" + assert_output --partial "api server init: unable to run local API: unable to init database client: unknown database type 'meh'" +} +