From df7c51f34ec0051de79fec146b3f810dadfbba74 Mon Sep 17 00:00:00 2001 From: mmetc <92726601+mmetc@users.noreply.github.com> Date: Tue, 31 May 2022 10:01:30 +0200 Subject: [PATCH] fixed coverage reporting for functional tests; added cscli (#1568) --- Makefile | 3 ++- cmd/crowdsec-cli/capi.go | 7 ++++--- cmd/crowdsec-cli/main.go | 15 ++++++++++++++- cmd/crowdsec/main.go | 2 +- cmd/crowdsec/run_in_svc.go | 7 +++++-- cmd/crowdsec/run_in_svc_windows.go | 12 ++++++++++-- tests/bats.mk | 7 ++++--- tests/bats/01_base.bats | 6 ++++++ tests/bats/02_nolapi.bats | 3 ++- tests/crowdsec-wrapper | 24 +++++++++++++++++++++--- tests/cscli-wrapper | 18 ++++++++++-------- tests/lib/init/crowdsec-daemon | 26 +++++++++++++------------- tests/run-tests | 3 +++ 13 files changed, 95 insertions(+), 38 deletions(-) diff --git a/Makefile b/Makefile index dd15ec243..978097529 100644 --- a/Makefile +++ b/Makefile @@ -51,7 +51,8 @@ MINIMUM_SUPPORTED_GO_MINOR_VERSION = 17 GO_VERSION_VALIDATION_ERR_MSG = Golang version ($(BUILD_GOVERSION)) is not supported, please use at least $(MINIMUM_SUPPORTED_GO_MAJOR_VERSION).$(MINIMUM_SUPPORTED_GO_MINOR_VERSION) LD_OPTS_VARS= \ --X github.com/crowdsecurity/crowdsec/cmd/crowdsec/main.bincoverTesting=$(BINCOVER_TESTING) \ +-X github.com/crowdsecurity/crowdsec/cmd/crowdsec.bincoverTesting=$(BINCOVER_TESTING) \ +-X github.com/crowdsecurity/crowdsec/cmd/crowdsec-cli.bincoverTesting=$(BINCOVER_TESTING) \ -X github.com/crowdsecurity/crowdsec/pkg/cwversion.Version=$(BUILD_VERSION) \ -X github.com/crowdsecurity/crowdsec/pkg/cwversion.BuildDate=$(BUILD_TIMESTAMP) \ -X github.com/crowdsecurity/crowdsec/pkg/cwversion.Codename=$(BUILD_CODENAME) \ diff --git a/cmd/crowdsec-cli/capi.go b/cmd/crowdsec-cli/capi.go index 78b4ee0c9..c82c17f15 100644 --- a/cmd/crowdsec-cli/capi.go +++ b/cmd/crowdsec-cli/capi.go @@ -12,6 +12,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" @@ -30,7 +31,7 @@ func NewCapiCmd() *cobra.Command { DisableAutoGenTag: true, PersistentPreRunE: func(cmd *cobra.Command, args []string) error { if err := csConfig.LoadAPIServer(); err != nil || csConfig.DisableAPI { - log.Fatal("Local API is disabled, please run this command on the local API machine") + return errors.Wrap(err, "Local API is disabled, please run this command on the local API machine") } if csConfig.API.Server.OnlineClient == nil { log.Fatalf("no configuration for Central API in '%s'", *csConfig.FilePath) @@ -113,7 +114,7 @@ func NewCapiCmd() *cobra.Command { Run: func(cmd *cobra.Command, args []string) { var err error if csConfig.API.Server == nil { - log.Fatalln("There is no configuration on 'api_client:'") + log.Fatalln("There is no configuration on 'api.server:'") } if csConfig.API.Server.OnlineClient == nil { log.Fatalf("Please provide credentials for the Central API (CAPI) in '%s'", csConfig.API.Server.OnlineClient.CredentialsFilePath) @@ -134,8 +135,8 @@ func NewCapiCmd() *cobra.Command { } if err := cwhub.GetHubIdx(csConfig.Hub); err != nil { - log.Fatalf("Failed to load hub index : %s", err) log.Infoln("Run 'sudo cscli hub update' to get the hub index") + log.Fatalf("Failed to load hub index : %s", err) } scenarios, err := cwhub.GetInstalledScenariosAsString() if err != nil { diff --git a/cmd/crowdsec-cli/main.go b/cmd/crowdsec-cli/main.go index d3e393a4a..5f480a8d0 100644 --- a/cmd/crowdsec-cli/main.go +++ b/cmd/crowdsec-cli/main.go @@ -7,6 +7,7 @@ import ( "path/filepath" "strings" + "github.com/confluentinc/bincover" "github.com/crowdsecurity/crowdsec/pkg/csconfig" "github.com/crowdsecurity/crowdsec/pkg/cwhub" "github.com/crowdsecurity/crowdsec/pkg/cwversion" @@ -17,6 +18,8 @@ import ( "github.com/spf13/cobra/doc" ) +var bincoverTesting = "" + var trace_lvl, dbg_lvl, nfo_lvl, wrn_lvl, err_lvl bool var ConfigFilePath string @@ -184,7 +187,17 @@ It is meant to allow you to manage bans, parsers/scenarios/etc, api and generall rootCmd.AddCommand(NewExplainCmd()) rootCmd.AddCommand(NewHubTestCmd()) rootCmd.AddCommand(NewNotificationsCmd()) + if err := rootCmd.Execute(); err != nil { - log.Fatal(err) + if bincoverTesting != "" { + log.Debug("coverage report is enabled") + } + + exitCode := 1 + log.NewEntry(log.StandardLogger()).Log(log.FatalLevel, err) + if bincoverTesting == "" { + os.Exit(exitCode) + } + bincover.ExitCode = exitCode } } diff --git a/cmd/crowdsec/main.go b/cmd/crowdsec/main.go index e2c31efb8..3587f13bd 100644 --- a/cmd/crowdsec/main.go +++ b/cmd/crowdsec/main.go @@ -49,7 +49,7 @@ var ( pluginBroker csplugin.PluginBroker ) -const bincoverTesting = false +var bincoverTesting = "" type Flags struct { ConfigFile string diff --git a/cmd/crowdsec/run_in_svc.go b/cmd/crowdsec/run_in_svc.go index 11d6f06ea..cf70f537a 100644 --- a/cmd/crowdsec/run_in_svc.go +++ b/cmd/crowdsec/run_in_svc.go @@ -15,7 +15,6 @@ import ( ) func StartRunSvc() { - var ( cConfig *csconfig.Config err error @@ -44,6 +43,10 @@ func StartRunSvc() { log.Infof("Crowdsec %s", cwversion.VersionStr()) + if bincoverTesting != "" { + log.Debug("coverage report is enabled") + } + // Enable profiling early if cConfig.Prometheus != nil { go registerPrometheus(cConfig.Prometheus) @@ -56,7 +59,7 @@ func StartRunSvc() { // 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 { + if bincoverTesting == "" { os.Exit(exitCode) } bincover.ExitCode = exitCode diff --git a/cmd/crowdsec/run_in_svc_windows.go b/cmd/crowdsec/run_in_svc_windows.go index af29b34fe..1f23dba8e 100644 --- a/cmd/crowdsec/run_in_svc_windows.go +++ b/cmd/crowdsec/run_in_svc_windows.go @@ -84,6 +84,10 @@ func WindowsRun() { log.Infof("Crowdsec %s", cwversion.VersionStr()) + if bincoverTesting != "" { + log.Debug("coverage report is enabled") + } + // Enable profiling early if cConfig.Prometheus != nil { go registerPrometheus(cConfig.Prometheus) @@ -91,8 +95,12 @@ func WindowsRun() { if exitCode, err := Serve(cConfig); err != nil { if err != nil { - log.Errorf(err.Error()) - if !bincoverTesting { + // 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) } bincover.ExitCode = exitCode diff --git a/tests/bats.mk b/tests/bats.mk index 03af05a14..4a09e3af4 100644 --- a/tests/bats.mk +++ b/tests/bats.mk @@ -28,15 +28,16 @@ PLUGIN_DIR = $(LOCAL_DIR)/lib/crowdsec/plugins DB_BACKEND ?= sqlite ifdef TEST_COVERAGE - CROWDSEC = "$(TEST_DIR)/crowdsec-wrapper" - CSCLI = "$(TEST_DIR)/cscli-wrapper" + CROWDSEC = $(TEST_DIR)/crowdsec-wrapper + CSCLI = $(TEST_DIR)/cscli-wrapper BINCOVER_TESTING = true else # the wrappers should work here too - it detects TEST_COVERAGE - but we allow # overriding the path to the binaries CROWDSEC ?= "$(BIN_DIR)/crowdsec" CSCLI ?= "$(BIN_DIR)/cscli" - BINCOVER_TESTING = false + # any value is considered true + BINCOVER_TESTING = endif # If you change the name of the crowdsec executable, make sure the pgrep diff --git a/tests/bats/01_base.bats b/tests/bats/01_base.bats index 90420e5fb..18ab07268 100644 --- a/tests/bats/01_base.bats +++ b/tests/bats/01_base.bats @@ -93,6 +93,12 @@ declare stderr assert_output --partial "Trying to authenticate with username" assert_output --partial " on https://api.crowdsec.net/" assert_output --partial "You can successfully interact with Central API (CAPI)" + + ONLINE_API_CREDENTIALS_YAML="$(config_yq '.api.server.online_client.credentials_path')" + rm "${ONLINE_API_CREDENTIALS_YAML}" + run -1 --separate-stderr cscli capi status + run -0 echo "${stderr}" + assert_output --partial "Local API is disabled, please run this command on the local API machine: loading online client credentials: failed to read api server credentials configuration file '${ONLINE_API_CREDENTIALS_YAML}': open ${ONLINE_API_CREDENTIALS_YAML}: no such file or directory" } @test "${FILE} cscli config show -o human" { diff --git a/tests/bats/02_nolapi.bats b/tests/bats/02_nolapi.bats index 04be980cc..5250e72fa 100644 --- a/tests/bats/02_nolapi.bats +++ b/tests/bats/02_nolapi.bats @@ -49,7 +49,8 @@ declare stderr run -1 --separate-stderr cscli capi status run -0 echo "$stderr" - assert_output --partial "Local API is disabled, please run this command on the local API machine" + assert_output --partial "crowdsec local API is disabled" + assert_output --partial "There is no configuration on 'api.server:'" } @test "$FILE cscli config show -o human" { diff --git a/tests/crowdsec-wrapper b/tests/crowdsec-wrapper index 886e70be4..62d7f1188 100755 --- a/tests/crowdsec-wrapper +++ b/tests/crowdsec-wrapper @@ -12,11 +12,29 @@ THIS_DIR=$(CDPATH= cd -- "$(dirname -- "$0")" && pwd) #shellcheck disable=SC1090 . "${THIS_DIR}/.environment.sh" +set -o pipefail # don't let sed hide the statuscode +mkdir -p "${LOCAL_DIR}/var/lib/coverage" + +# we collect rc and output by hand, because setting -o pipefail would trigger a +# SIGPIPE. +set +e + # Arguments to crowdsec are passed through a temporary, newline-delimited # file courtesy of github.com/confluentinc/bincover. Coverage data will be # merged at the end of the test run. # The '=' between flags and values is required. -exec "${BIN_DIR}/crowdsec.cover" \ +output=$("${BIN_DIR}/crowdsec.cover" \ -test.run="^TestBincoverRunMain$" \ - -test.coverprofile="${LOCAL_DIR}/var/lib/coverage/crowdsec-$(date +'%s')-$$.out" \ - -args-file=<(for i; do echo "$i"; done) # Behold the amazing parameter contraption! + -test.coverprofile="${LOCAL_DIR}/var/lib/coverage/crowdsec-$(date +'%s')-$$-${RANDOM}.out" \ + -args-file=<(for i; do echo "$i"; done)) +rc=$? + +# If there is bincover metadata, we take the status code from there. Otherwise, +# we keep the status from the above command. +if [[ ${output} =~ (.*)(START_BINCOVER_METADATA[[:space:]]*)(.*)([[:space:]]END_BINCOVER_METADATA) ]]; then + echo -n "${BASH_REMATCH[1]}" + exit "$(jq '.exit_code' <<< "${BASH_REMATCH[3]}")" +fi + +echo -n "${output}" +exit "${rc}" diff --git a/tests/cscli-wrapper b/tests/cscli-wrapper index e72f02a83..5295c15cb 100755 --- a/tests/cscli-wrapper +++ b/tests/cscli-wrapper @@ -25,14 +25,16 @@ set +e # The '=' between flags and values is required. output=$("${BIN_DIR}/cscli.cover" \ -test.run="^TestBincoverRunMain$" \ - -test.coverprofile="${LOCAL_DIR}/var/lib/coverage/cscli-$(date +'%s')-$$.out" \ - -args-file=<(for i; do echo "$i"; done)) + -test.coverprofile="${LOCAL_DIR}/var/lib/coverage/cscli-$(date +'%s')-$$-${RANDOM}.out" \ + -args-file=<(for i; do echo "${i}"; done)) rc=$? -# We also cut the metadata stuff that we don't need. -echo -n "$output" | tr '\n' '\f' | sed 's/START_BINCOVER_METADATA.*//' | tr '\f' '\n' +# If there is bincover metadata, we take the status code from there. Otherwise, +# we keep the status from the above command. +if [[ ${output} =~ (.*)(START_BINCOVER_METADATA[[:space:]]*)(.*)([[:space:]]END_BINCOVER_METADATA) ]]; then + echo -n "${BASH_REMATCH[1]}" + exit "$(jq '.exit_code' <<< "${BASH_REMATCH[3]}")" +fi -# this does not work because cscli output does not always end with \n -# echo -n "$output" | sed -n '/START_BINCOVER_METADATA/q;p' - -exit $rc +echo -n "${output}" +exit "${rc}" diff --git a/tests/lib/init/crowdsec-daemon b/tests/lib/init/crowdsec-daemon index 144572a5b..f8a331d41 100755 --- a/tests/lib/init/crowdsec-daemon +++ b/tests/lib/init/crowdsec-daemon @@ -1,20 +1,20 @@ - #!/usr/bin/env bash +#!/usr/bin/env bash - set -eu - script_name=$0 +set -eu +script_name=$0 - die() { - echo >&2 "$@" - exit 1 - } +die() { + echo >&2 "$@" + exit 1 +} - about() { - die "usage: $script_name [ start | stop ]" - } +about() { + die "usage: $script_name [ start | stop ]" +} - #shellcheck disable=SC1007 - THIS_DIR=$(CDPATH= cd -- "$(dirname -- "$0")" && pwd) - cd "${THIS_DIR}"/../../ +#shellcheck disable=SC1007 +THIS_DIR=$(CDPATH= cd -- "$(dirname -- "$0")" && pwd) +cd "${THIS_DIR}"/../../ #shellcheck disable=SC1091 . ./.environment.sh diff --git a/tests/run-tests b/tests/run-tests index 194709172..ccb3d291b 100755 --- a/tests/run-tests +++ b/tests/run-tests @@ -45,6 +45,9 @@ else fi if [ -n "$TEST_COVERAGE" ]; then + # empty files just to avoid merge errors + touch "${LOCAL_DIR}"/var/lib/coverage/crowdsec- + touch "${LOCAL_DIR}"/var/lib/coverage/cscli- gocovmerge "${LOCAL_DIR}"/var/lib/coverage/crowdsec-* > "${LOCAL_DIR}/var/lib/coverage/coverage-crowdsec.out" gocovmerge "${LOCAL_DIR}"/var/lib/coverage/cscli-* > "${LOCAL_DIR}/var/lib/coverage/coverage-cscli.out" fi