Skip by default compareResults in pipeline tests for serverless (#1521)

This PR skips the compare results function for serverless by default. This function is in charge of comparing the documents received by elasticsearch with the samples. The issue is just in Serverless, since there are fields related to Geo IP database that cannot be checked properly and causing differences in JSONs.
elastic · Oct 24, 2023 · a68f5ba · a68f5ba
1 parent af6a093
commit a68f5ba
Show file tree

Hide file tree

Showing 2 changed files with 39 additions and 53 deletions.
diff --git a/internal/testrunner/runners/pipeline/runner.go b/internal/testrunner/runners/pipeline/runner.go
@@ -19,6 +19,7 @@ import (
 
 	"github.com/elastic/elastic-package/internal/common"
 	"github.com/elastic/elastic-package/internal/elasticsearch/ingest"
+	"github.com/elastic/elastic-package/internal/environment"
 	"github.com/elastic/elastic-package/internal/fields"
 	"github.com/elastic/elastic-package/internal/logger"
 	"github.com/elastic/elastic-package/internal/multierror"
@@ -33,9 +34,15 @@ const (
 	TestType testrunner.TestType = "pipeline"
 )
 
+var (
+	serverlessDisableCompareResults = environment.WithElasticPackagePrefix("SERVERLESS_PIPELINE_TEST_DISABLE_COMPARE_RESULTS")
+)
+
 type runner struct {
 	options   testrunner.TestOptions
 	pipelines []ingest.Pipeline
+
+	runCompareResults bool
 }
 
 type IngestPipelineReroute struct {
@@ -61,6 +68,22 @@ func (r *runner) String() string {
 // Run runs the pipeline tests defined under the given folder
 func (r *runner) Run(options testrunner.TestOptions) ([]testrunner.TestResult, error) {
 	r.options = options
+
+	stackConfig, err := stack.LoadConfig(r.options.Profile)
+	if err != nil {
+		return nil, err
+	}
+
+	r.runCompareResults = true
+	if stackConfig.Provider == stack.ProviderServerless {
+		r.runCompareResults = true
+
+		v, ok := os.LookupEnv(serverlessDisableCompareResults)
+		if ok && strings.ToLower(v) == "true" {
+			r.runCompareResults = false
+		}
+	}
+
 	return r.run()
 }
 
@@ -302,21 +325,15 @@ func (r *runner) verifyResults(testCaseFile string, config *testConfig, result *
 		}
 	}
 
-	// TODO: currently GeoIP related fields are being removed when the serverless provider is used.
-	stackConfig, err := stack.LoadConfig(r.options.Profile)
-	if err != nil {
-		return err
-	}
-	skipGeoIP := false
-	if stackConfig.Provider == stack.ProviderServerless {
-		skipGeoIP = true
-	}
-	err = compareResults(testCasePath, config, result, skipGeoIP, *specVersion)
-	if _, ok := err.(testrunner.ErrTestCaseFailed); ok {
-		return err
-	}
-	if err != nil {
-		return fmt.Errorf("comparing test results failed: %w", err)
+	// TODO: temporary workaround untill there could be implemented other approach for deterministic geoip in serverless.
+	if r.runCompareResults {
+		err = compareResults(testCasePath, config, result, *specVersion)
+		if _, ok := err.(testrunner.ErrTestCaseFailed); ok {
+			return err
+		}
+		if err != nil {
+			return fmt.Errorf("comparing test results failed: %w", err)
+		}
 	}
 
 	result = stripEmptyTestResults(result)

diff --git a/internal/testrunner/runners/pipeline/test_result.go b/internal/testrunner/runners/pipeline/test_result.go
@@ -24,28 +24,6 @@ import (
 
 const expectedTestResultSuffix = "-expected.json"
 
-var geoIPKeys = []string{
-	"as",
-	"geo",
-	"client.as",
-	"client.geo",
-	"destination.as",
-	"destination.geo",
-	"host.geo",     // not defined host.as in ECS
-	"observer.geo", // not defined observer.as in ECS
-	"server.as",
-	"server.geo",
-	"source.as",
-	"source.geo",
-	"threat.enrichments.indicateor.as",
-	"threat.enrichments.indicateor.geo",
-	"threat.indicateor.as",
-	"threat.indicateor.geo",
-	// packages using geo fields in nested objects
-	"netskope.alerts.user.geo",
-	"netskope.events.user.geo",
-}
-
 type testResult struct {
 	events []json.RawMessage
 }
@@ -69,8 +47,8 @@ func writeTestResult(testCasePath string, result *testResult, specVersion semver
 	return nil
 }
 
-func compareResults(testCasePath string, config *testConfig, result *testResult, skipGeoip bool, specVersion semver.Version) error {
-	resultsWithoutDynamicFields, err := adjustTestResult(result, config, skipGeoip)
+func compareResults(testCasePath string, config *testConfig, result *testResult, specVersion semver.Version) error {
+	resultsWithoutDynamicFields, err := adjustTestResult(result, config)
 	if err != nil {
 		return fmt.Errorf("can't adjust test results: %w", err)
 	}
@@ -80,7 +58,7 @@ func compareResults(testCasePath string, config *testConfig, result *testResult,
 		return fmt.Errorf("marshalling actual test results failed: %w", err)
 	}
 
-	expectedResults, err := readExpectedTestResult(testCasePath, config, skipGeoip)
+	expectedResults, err := readExpectedTestResult(testCasePath, config)
 	if err != nil {
 		return fmt.Errorf("reading expected test result failed: %w", err)
 	}
@@ -161,7 +139,7 @@ func diffJson(want, got []byte, specVersion semver.Version) (string, error) {
 	return buf.String(), err
 }
 
-func readExpectedTestResult(testCasePath string, config *testConfig, skipGeoIP bool) (*testResult, error) {
+func readExpectedTestResult(testCasePath string, config *testConfig) (*testResult, error) {
 	testCaseDir := filepath.Dir(testCasePath)
 	testCaseFile := filepath.Base(testCasePath)
 
@@ -176,15 +154,15 @@ func readExpectedTestResult(testCasePath string, config *testConfig, skipGeoIP b
 		return nil, fmt.Errorf("unmarshalling expected test result failed: %w", err)
 	}
 
-	adjusted, err := adjustTestResult(u, config, skipGeoIP)
+	adjusted, err := adjustTestResult(u, config)
 	if err != nil {
 		return nil, fmt.Errorf("adjusting test result failed: %w", err)
 	}
 	return adjusted, nil
 }
 
-func adjustTestResult(result *testResult, config *testConfig, skipGeoIP bool) (*testResult, error) {
-	if !skipGeoIP && (config == nil || config.DynamicFields == nil) {
+func adjustTestResult(result *testResult, config *testConfig) (*testResult, error) {
+	if config == nil || config.DynamicFields == nil {
 		return result, nil
 	}
 	var stripped testResult
@@ -210,15 +188,6 @@ func adjustTestResult(result *testResult, config *testConfig, skipGeoIP bool) (*
 			}
 		}
 
-		if skipGeoIP {
-			for _, key := range geoIPKeys {
-				err := m.Delete(key)
-				if err != nil && err != common.ErrKeyNotFound {
-					return nil, fmt.Errorf("can't remove geoIP field: %w", err)
-				}
-			}
-		}
-
 		b, err := json.Marshal(&m)
 		if err != nil {
 			return nil, fmt.Errorf("can't marshal event: %w", err)