acquisition: add some test and warning for wrong source type (#3362)

.gitignore

@@ -21,6 +21,9 @@
 # Test dependencies
 test/tools/*
 
+# Saved test status
+test/bats/.bats/run-logs
+
 # VMs used for dev/test
 
 .vagrant

pkg/acquisition/acquisition.go

@@ -140,7 +140,7 @@ func DataSourceConfigure(commonConfig configuration.DataSourceCommonCfg, metrics
 	}
 	/* configure the actual datasource */
 	if err := dataSrc.Configure(yamlConfig, subLogger, metricsLevel); err != nil {
-		return nil, fmt.Errorf("failed to configure datasource %s: %w", commonConfig.Source, err)
+		return nil, err
 	}
 
 	return &dataSrc, nil
@@ -164,8 +164,6 @@ func detectBackwardCompatAcquis(sub configuration.DataSourceCommonCfg) string {
 }
 
 func LoadAcquisitionFromDSN(dsn string, labels map[string]string, transformExpr string) ([]DataSource, error) {
-	var sources []DataSource
-
 	frags := strings.Split(dsn, ":")
 	if len(frags) == 1 {
 		return nil, fmt.Errorf("%s isn't valid dsn (no protocol)", dsn)
@@ -197,9 +195,7 @@ func LoadAcquisitionFromDSN(dsn string, labels map[string]string, transformExpr
 		return nil, fmt.Errorf("while configuration datasource for %s: %w", dsn, err)
 	}
 
-	sources = append(sources, dataSrc)
-
-	return sources, nil
+	return []DataSource{dataSrc}, nil
 }
 
 func GetMetricsLevelFromPromCfg(prom *csconfig.PrometheusCfg) int {
@@ -249,7 +245,7 @@ func LoadAcquisitionFromFile(config *csconfig.CrowdsecServiceCfg, prom *csconfig
 			err = dec.Decode(&sub)
 			if err != nil {
 				if !errors.Is(err, io.EOF) {
-					return nil, fmt.Errorf("failed to yaml decode %s: %w", acquisFile, err)
+					return nil, fmt.Errorf("failed to parse %s: %w", acquisFile, err)
 				}
 
 				log.Tracef("End of yaml file")
@@ -259,6 +255,12 @@ func LoadAcquisitionFromFile(config *csconfig.CrowdsecServiceCfg, prom *csconfig
 
 			// for backward compat ('type' was not mandatory, detect it)
 			if guessType := detectBackwardCompatAcquis(sub); guessType != "" {
+				log.Debugf("datasource type missing in %s (position %d): detected 'source=%s'", acquisFile, idx, guessType)
+
+				if sub.Source != "" && sub.Source != guessType {
+					log.Warnf("datasource type mismatch in %s (position %d): found '%s' but should probably be '%s'", acquisFile, idx, sub.Source, guessType)
+				}
+
 				sub.Source = guessType
 			}
 			// it's an empty item, skip it
@@ -270,18 +272,18 @@ func LoadAcquisitionFromFile(config *csconfig.CrowdsecServiceCfg, prom *csconfig
 
 				if sub.Source != "docker" {
 					// docker is the only source that can be empty
-					return nil, fmt.Errorf("missing labels in %s (position: %d)", acquisFile, idx)
+					return nil, fmt.Errorf("missing labels in %s (position %d)", acquisFile, idx)
 				}
 			}
 
 			if sub.Source == "" {
-				return nil, fmt.Errorf("data source type is empty ('source') in %s (position: %d)", acquisFile, idx)
+				return nil, fmt.Errorf("data source type is empty ('source') in %s (position %d)", acquisFile, idx)
 			}
 
 			// pre-check that the source is valid
 			_, err := GetDataSourceIface(sub.Source)
 			if err != nil {
-				return nil, fmt.Errorf("in file %s (position: %d) - %w", acquisFile, idx, err)
+				return nil, fmt.Errorf("in file %s (position %d) - %w", acquisFile, idx, err)
 			}
 
 			uniqueId := uuid.NewString()
@@ -295,13 +297,13 @@ func LoadAcquisitionFromFile(config *csconfig.CrowdsecServiceCfg, prom *csconfig
 					continue
 				}
 
-				return nil, fmt.Errorf("while configuring datasource of type %s from %s (position: %d): %w", sub.Source, acquisFile, idx, err)
+				return nil, fmt.Errorf("while configuring datasource of type %s from %s (position %d): %w", sub.Source, acquisFile, idx, err)
 			}
 
 			if sub.TransformExpr != "" {
 				vm, err := expr.Compile(sub.TransformExpr, exprhelpers.GetExprOptions(map[string]interface{}{"evt": &types.Event{}})...)
 				if err != nil {
-					return nil, fmt.Errorf("while compiling transform expression '%s' for datasource %s in %s (position: %d): %w", sub.TransformExpr, sub.Source, acquisFile, idx, err)
+					return nil, fmt.Errorf("while compiling transform expression '%s' for datasource %s in %s (position %d): %w", sub.TransformExpr, sub.Source, acquisFile, idx, err)
 				}
 
 				transformRuntimes[uniqueId] = vm
@@ -344,6 +346,7 @@ func copyEvent(evt types.Event, line string) types.Event {
 	evtCopy.Line = evt.Line
 	evtCopy.Line.Raw = line
 	evtCopy.Line.Labels = make(map[string]string)
+
 	for k, v := range evt.Line.Labels {
 		evtCopy.Line.Labels[k] = v
 	}
@@ -386,6 +389,7 @@ func transform(transformChan chan types.Event, output chan types.Event, AcquisTo
 					if !ok {
 						logger.Errorf("transform expression returned []interface{}, but cannot assert an element to string")
 						output <- evt
+
 						continue
 					}
 

pkg/acquisition/acquisition_test.go

@@ -140,7 +140,7 @@ log_level: debug
 source: mock
 toto: test_value1
 `,
-			ExpectedError: "failed to configure datasource mock: mode ratata is not supported",
+			ExpectedError: "mode ratata is not supported",
 		},
 		{
 			TestName: "bad_type_config",
@@ -182,7 +182,8 @@ wowo: ajsajasjas
 	for _, tc := range tests {
 		t.Run(tc.TestName, func(t *testing.T) {
 			common := configuration.DataSourceCommonCfg{}
-			yaml.Unmarshal([]byte(tc.String), &common)
+			err := yaml.Unmarshal([]byte(tc.String), &common)
+			require.NoError(t, err)
 			ds, err := DataSourceConfigure(common, configuration.METRICS_NONE)
 			cstest.RequireErrorContains(t, err, tc.ExpectedError)
 
@@ -236,7 +237,7 @@ func TestLoadAcquisitionFromFile(t *testing.T) {
 			Config: csconfig.CrowdsecServiceCfg{
 				AcquisitionFiles: []string{"test_files/badyaml.yaml"},
 			},
-			ExpectedError: "failed to yaml decode test_files/badyaml.yaml: yaml: unmarshal errors",
+			ExpectedError: "failed to parse test_files/badyaml.yaml: yaml: unmarshal errors",
 			ExpectedLen:   0,
 		},
 		{
@@ -272,7 +273,7 @@ func TestLoadAcquisitionFromFile(t *testing.T) {
 			Config: csconfig.CrowdsecServiceCfg{
 				AcquisitionFiles: []string{"test_files/bad_source.yaml"},
 			},
-			ExpectedError: "in file test_files/bad_source.yaml (position: 0) - unknown data source does_not_exist",
+			ExpectedError: "in file test_files/bad_source.yaml (position 0) - unknown data source does_not_exist",
 		},
 		{
 			TestName: "invalid_filetype_config",

test/bats/01_crowdsec.bats

@@ -1,5 +1,4 @@
 #!/usr/bin/env bats
-# vim: ft=bats:list:ts=8:sts=4:sw=4:et:ai:si:
 
 set -u
 
@@ -138,6 +137,8 @@ teardown() {
     rune -0 ./instance-crowdsec stop
 }
 
+# TODO: move acquisition tests to test/bats/crowdsec-acquisition.bats
+
 @test "crowdsec (error if the acquisition_path file is defined but missing)" {
     ACQUIS_YAML=$(config_get '.crowdsec_service.acquisition_path')
     rm -f "$ACQUIS_YAML"
@@ -278,7 +279,7 @@ teardown() {
     # if filenames are missing, it won't be able to detect source type
     config_set "$ACQUIS_YAML" '.source="file"'
     rune -1 wait-for "$CROWDSEC"
-    assert_stderr --partial "failed to configure datasource file: no filename or filenames configuration provided"
+    assert_stderr --partial "while configuring datasource of type file from $ACQUIS_YAML (position 0): no filename or filenames configuration provided"
 
     config_set "$ACQUIS_YAML" '.filenames=["file.log"]'
     config_set "$ACQUIS_YAML" '.meh=3'

test/bats/crowdsec-acquisition.bats

@@ -0,0 +1,78 @@
+#!/usr/bin/env bats
+
+set -u
+
+setup_file() {
+    load "../lib/setup_file.sh"
+}
+
+teardown_file() {
+    load "../lib/teardown_file.sh"
+}
+
+setup() {
+    load "../lib/setup.sh"
+    load "../lib/bats-file/load.bash"
+    ./instance-data load
+    ACQUIS_DIR=$(config_get '.crowdsec_service.acquisition_dir')
+    mkdir -p "$ACQUIS_DIR"
+}
+
+teardown() {
+    ./instance-crowdsec stop
+}
+
+#----------
+
+@test "malformed acqusition file" {
+    cat >"$ACQUIS_DIR/file.yaml" <<-EOT
+	filename:
+	- /path/to/file.log
+	labels:
+	  type: syslog
+	EOT
+
+    rune -1 "$CROWDSEC" -t
+    assert_stderr --partial "crowdsec init: while loading acquisition config: while configuring datasource of type file from $ACQUIS_DIR/file.yaml (position 0): cannot parse FileAcquisition configuration: yaml: unmarshal errors:\n  line 6: cannot unmarshal !!seq into string"
+}
+
+@test "datasource type detection" {
+    config_set '.common.log_level="debug" | .common.log_media="stdout"'
+
+    # for backward compatibility, a missing source type is not a problem if it can be detected by the presence of other fields
+
+    cat >"$ACQUIS_DIR/file.yaml" <<-EOT
+	filename: /path/to/file.log
+	labels:
+	  type: syslog
+	---
+	filenames:
+	 - /path/to/file.log
+	labels:
+	  type: syslog
+	EOT
+
+    cat >"$ACQUIS_DIR"/journal.yaml <<-EOT
+	journalctl_filter:
+	 - "_SYSTEMD_UNIT=ssh.service"
+	labels:
+	  type: syslog
+	EOT
+
+    # However, a wrong source type will raise a brow.
+    # This is currently not a fatal error because it has been tolerated in the past.
+
+    cat >"$ACQUIS_DIR"/bad.yaml <<-EOT
+	source: docker
+	journalctl_filter:
+	 - "_SYSTEMD_UNIT=ssh.service"
+	labels:
+	  type: syslog
+	EOT
+
+    rune -0 "$CROWDSEC" -t
+    assert_stderr --partial "datasource type missing in $ACQUIS_DIR/file.yaml (position 0): detected 'source=file'"
+    assert_stderr --partial "datasource type missing in $ACQUIS_DIR/file.yaml (position 1): detected 'source=file'"
+    assert_stderr --partial "datasource type missing in $ACQUIS_DIR/journal.yaml (position 0): detected 'source=journalctl'"
+    assert_stderr --partial "datasource type mismatch in $ACQUIS_DIR/bad.yaml (position 0): found 'docker' but should probably be 'journalctl'"
+}