Disallow importing ECS fields root level

internal/builder/external_fields.go

@@ -11,12 +11,17 @@ import (
 
 	"gopkg.in/yaml.v3"
 
+	"github.com/Masterminds/semver/v3"
+
 	"github.com/elastic/elastic-package/internal/common"
 	"github.com/elastic/elastic-package/internal/fields"
 	"github.com/elastic/elastic-package/internal/logger"
+	"github.com/elastic/elastic-package/internal/packages"
 	"github.com/elastic/elastic-package/internal/packages/buildmanifest"
 )
 
+var semver3_0_0 = semver.MustParse("3.0.0")
+
 func resolveExternalFields(packageRoot, destinationDir string) error {
 	bm, ok, err := buildmanifest.ReadBuildManifest(packageRoot)
 	if err != nil {
@@ -42,14 +47,27 @@ func resolveExternalFields(packageRoot, destinationDir string) error {
 		return fmt.Errorf("failed to list fields files under \"%s\": %w", destinationDir, err)
 	}
 
+	manifest, err := packages.ReadPackageManifestFromPackageRoot(packageRoot)
+	if err != nil {
+		return fmt.Errorf("failed to read package manifest from \"%s\"", packageRoot)
+	}
+	sv, err := semver.NewVersion(manifest.SpecVersion)
+	if err != nil {
+		return fmt.Errorf("failed to obtain spec version from package manifest in \"%s\"", packageRoot)
+	}
+	var options fields.InjectFieldsOptions
+	if !sv.LessThan(semver3_0_0) {
+		options.DisallowReusableECSFieldsAtTopLevel = true
+	}
+
 	for _, file := range fieldsFiles {
 		data, err := os.ReadFile(file)
 		if err != nil {
 			return err
 		}
 
 		rel, _ := filepath.Rel(destinationDir, file)
-		output, injected, err := injectFields(fdm, data)
+		output, injected, err := injectFields(fdm, data, options)
 		if err != nil {
 			return err
 		} else if injected {
@@ -89,14 +107,14 @@ func listAllFieldsFiles(dir string) ([]string, error) {
 	return paths, nil
 }
 
-func injectFields(fdm *fields.DependencyManager, content []byte) ([]byte, bool, error) {
+func injectFields(fdm *fields.DependencyManager, content []byte, options fields.InjectFieldsOptions) ([]byte, bool, error) {
 	var f []common.MapStr
 	err := yaml.Unmarshal(content, &f)
 	if err != nil {
 		return nil, false, fmt.Errorf("can't unmarshal source file: %w", err)
 	}
 
-	f, changed, err := fdm.InjectFields(f)
+	f, changed, err := fdm.InjectFieldsWithOptions(f, options)
 	if err != nil {
 		return nil, false, fmt.Errorf("can't resolve fields: %w", err)
 	}

internal/fields/dependency_manager.go

@@ -148,6 +148,10 @@ type InjectFieldsOptions struct {
 	// SkipEmptyFields can be set to true to skip empty groups when injecting fields.
 	SkipEmptyFields bool
 
+	// DisallowReusableECSFieldsAtTopLevel can be set to true to disallow importing reusable
+	// ECS fields at the top level, when they cannot be reused there.
+	DisallowReusableECSFieldsAtTopLevel bool
+
 	root string
 }
 
@@ -174,6 +178,9 @@ func (dm *DependencyManager) injectFieldsWithOptions(defs []common.MapStr, optio
 			if err != nil {
 				return nil, false, fmt.Errorf("can't import field: %w", err)
 			}
+			if imported.disallowAtTopLevel && options.DisallowReusableECSFieldsAtTopLevel {
+				return nil, false, fmt.Errorf("field %s cannot be reused at top level", fieldPath)
+			}
 
 			transformed := transformImportedField(imported)
 

internal/fields/dependency_manager_test.go

@@ -462,6 +462,40 @@ func TestDependencyManagerInjectExternalFields(t *testing.T) {
 			valid:   true,
 			changed: true,
 		},
+		{
+			title: "disallowed reusable field at lop level",
+			defs: []common.MapStr{
+				{
+					"name":     "geo.city_name",
+					"external": "test",
+				},
+			},
+			options: InjectFieldsOptions{
+				DisallowReusableECSFieldsAtTopLevel: true,
+			},
+			valid: false,
+		},
+		{
+			title: "legacy support to reuse field at lop level",
+			defs: []common.MapStr{
+				{
+					"name":     "geo.city_name",
+					"external": "test",
+				},
+			},
+			options: InjectFieldsOptions{
+				DisallowReusableECSFieldsAtTopLevel: false,
+			},
+			result: []common.MapStr{
+				{
+					"name":        "geo.city_name",
+					"description": "City name",
+					"type":        "keyword",
+				},
+			},
+			changed: true,
+			valid:   true,
+		},
 	}
 
 	indexFalse := false
@@ -533,6 +567,27 @@ func TestDependencyManagerInjectExternalFields(t *testing.T) {
 					Description: "Hostname of the host",
 					Type:        "keyword",
 				},
+				{
+					Name:        "geo.city_name",
+					Description: "City name",
+					Type:        "keyword",
+				},
+			},
+		},
+		{
+			Name:        "geo",
+			Description: "Location info",
+			Type:        "group",
+			Fields: []FieldDefinition{
+				{
+					Name:               "city_name",
+					Description:        "City name",
+					Type:               "keyword",
+					disallowAtTopLevel: true,
+				},
+			},
+			Reusable: &ReusableConfig{
+				TopLevel: false,
 			},
 		},
 	}}

internal/fields/model.go

@@ -31,6 +31,15 @@ type FieldDefinition struct {
 	Normalize      []string          `yaml:"normalize,omitempty"`
 	Fields         FieldDefinitions  `yaml:"fields,omitempty"`
 	MultiFields    []FieldDefinition `yaml:"multi_fields,omitempty"`
+	Reusable       *ReusableConfig   `yaml:"reusable,omitempty"`
+
+	// disallowAtTopLevel transfers the reusability config from parent groups to nested fields.
+	// It is negated respect to Reusable.TopLevel, so it is disabled by default.
+	disallowAtTopLevel bool
+}
+
+type ReusableConfig struct {
+	TopLevel bool `yaml:"top_level"`
 }
 
 func (orig *FieldDefinition) Update(fd FieldDefinition) {
@@ -184,6 +193,10 @@ func (fds *FieldDefinitions) UnmarshalYAML(value *yaml.Node) error {
 func cleanNested(parent *FieldDefinition) (base []FieldDefinition) {
 	var nested []FieldDefinition
 	for _, field := range parent.Fields {
+		if reusable := parent.Reusable; reusable != nil {
+			field.disallowAtTopLevel = !reusable.TopLevel
+		}
+
 		// If the field name is prefixed by the name of its parent,
 		// this is a normal nested field. If not, it is a base field.
 		if strings.HasPrefix(field.Name, parent.Name+".") {

scripts/test-build-zip.sh

@@ -22,6 +22,10 @@ cleanup() {
   exit $r
 }
 
+testype() {
+  echo $(basename $(dirname $1))
+}
+
 trap cleanup EXIT
 
 OLDPWD=$PWD
@@ -31,6 +35,10 @@ export ELASTIC_PACKAGE_SIGNER_PASSPHRASE=$(cat "$OLDPWD/scripts/gpg-pass.txt")
 export ELASTIC_PACKAGE_LINKS_FILE_PATH="$(pwd)/scripts/links_table.yml"
 
 for d in test/packages/*/*/; do
+  # Packages in false_positives can have issues.
+  if [ "$(testype $d)" == "false_positives" ]; then
+    continue
+  fi
   (
     cd $d
     elastic-package build --zip --sign -v
@@ -46,6 +54,10 @@ elastic-package stack up -d -v
 
 # Install zipped packages
 for d in test/packages/*/*/; do
+  # Packages in false_positives can have issues.
+  if [ "$(testype $d)" == "false_positives" ]; then
+    continue
+  fi
   (
     cd $d
     elastic-package install -v

scripts/test-check-false-positives.sh

@@ -2,7 +2,7 @@
 
 set -euxo pipefail
 
-cleanup() {
+function cleanup() {
   r=$?
 
   # Dump stack logs
@@ -19,54 +19,75 @@ cleanup() {
     )
   done
 
-  # This is a false positive scenario and tests that the test case failure is a success scenario
-  if [ "${PACKAGE_TEST_TYPE:-false_positives}" == "false_positives" ]; then
-    if [ $r == 1 ]; then
-      EXPECTED_ERRORS_FILE="test/packages/false_positives/${PACKAGE_UNDER_TEST}.expected_errors"
-      if [ ! -f ${EXPECTED_ERRORS_FILE} ]; then
-        echo "Error: Missing expected errors file: ${EXPECTED_ERRORS_FILE}"
-      fi
-      RESULTS_NO_SPACES="build/test-results-no-spaces.xml"
-      cat build/test-results/*.xml | tr -d '\n' > ${RESULTS_NO_SPACES}
-
-      # check number of expected errors
-      number_errors=$(cat build/test-results/*.xml | grep "<failure>" | wc -l)
-      expected_errors=$(cat ${EXPECTED_ERRORS_FILE} | wc -l)
-
-      if [ ${number_errors} -ne ${expected_errors} ]; then
-          echo "Error: There are unexpected errors in ${PACKAGE_UNDER_TEST}"
-          exit 1
-      fi
-
-      # check whether or not the expected errors exist in the xml files
-      while read -r line; do
-        cat ${RESULTS_NO_SPACES} | grep -E "${line}"
-      done < ${EXPECTED_ERRORS_FILE}
-      rm -f build/test-results/*.xml
-      rm -f ${RESULTS_NO_SPACES}
-      exit 0
-    elif [ $r == 0 ]; then
-      echo "Error: Expected to fail tests, but there was none failing"
+  exit $r
+}
+
+function check_expected_errors() {
+  local package_root=$1
+  local package_name=$(basename $1)
+  local expected_errors_file="${package_root%/}.expected_errors"
+  local result_tests="build/test-results/${package_name}_*.xml"
+  local results_no_spaces="build/test-results-no-spaces.xml"
+
+  if [ ! -f ${expected_errors_file} ]; then
+    echo "No unexpected errors file in ${expected_errors_file}"
+    return
+  fi
+
+  rm -f ${result_tests}
+  (
+    cd $package_root
+    elastic-package test -v --report-format xUnit --report-output file --test-coverage --defer-cleanup 1s || true
+  )
+
+  cat ${result_tests} | tr -d '\n' > ${results_no_spaces}
+
+  # check number of expected errors
+  local number_errors=$(cat ${result_tests} | grep "<failure>" | wc -l)
+  local expected_errors=$(cat ${expected_errors_file} | wc -l)
+
+  if [ ${number_errors} -ne ${expected_errors} ]; then
+      echo "Error: There are unexpected errors in ${package_name}"
       exit 1
-    fi
   fi
 
-  exit $r
+  # check whether or not the expected errors exist in the xml files
+  while read -r line; do
+    cat ${results_no_spaces} | grep -E "${line}"
+  done < ${expected_errors_file}
+
+  rm -f ${result_tests}
+  rm -f ${results_no_spaces}
 }
 
-trap cleanup EXIT
+function check_build_output() {
+  local package_root=$1
+  local expected_build_output="${package_root%/}.build_output"
+  local output_file="$PWD/build/elastic-package-output"
 
-export ELASTIC_PACKAGE_LINKS_FILE_PATH="$(pwd)/scripts/links_table.yml"
+  if [ ! -f ${expected_build_output} ]; then
+    (
+      cd $package_root
+      elastic-package build -v
+    )
+    return
+  fi
 
-OLDPWD=$PWD
-# Build/check packages
-for d in test/packages/${PACKAGE_TEST_TYPE:-false_positives}/${PACKAGE_UNDER_TEST:-*}/; do
   (
-    cd $d
-    elastic-package check -v
+    cd $package_root
+    mkdir -p $(dirname $output_file)
+    elastic-package build 2>&1 | tee $output_file || true # Ignore errors here
   )
-done
-cd -
+
+  diff -w -u $expected_build_output $output_file || (
+    echo "Error: Build output has differences with expected output"
+    exit 1
+  )
+}
+
+trap cleanup EXIT
+
+export ELASTIC_PACKAGE_LINKS_FILE_PATH="$(pwd)/scripts/links_table.yml"
 
 # Update the stack
 elastic-package stack update -v
@@ -78,11 +99,6 @@ elastic-package stack status
 
 # Run package tests
 for d in test/packages/${PACKAGE_TEST_TYPE:-false_positives}/${PACKAGE_UNDER_TEST:-*}/; do
-  (
-    cd $d
-
-    # defer-cleanup is set to a short period to verify that the option is available
-    elastic-package test -v --report-format xUnit --report-output file --defer-cleanup 1s --test-coverage
-  )
-cd -
+  check_build_output $d
+  check_expected_errors $d
 done

scripts/test-install-zip.sh

@@ -26,6 +26,10 @@ cleanup() {
   exit $r
 }
 
+testype() {
+  echo $(basename $(dirname $1))
+}
+
 trap cleanup EXIT
 
 installAndVerifyPackage() {
@@ -96,6 +100,10 @@ OLDPWD=$PWD
 
 # Build packages
 for d in test/packages/*/*/; do
+  # Packages in false_positives can have issues.
+  if [ "$(testype $d)" == "false_positives" ]; then
+    continue
+  fi
   (
     cd $d
     elastic-package build

test/packages/false_positives/ecs_external_false_positive_asserts.build_output

@@ -0,0 +1,2 @@
+Build the package
+Error: building package failed: resolving external fields failed: can't resolve fields: field geo.city_name cannot be reused at top level