Stricter validation of data stream settings

code/go/internal/yamlschema/schema_loader.go

@@ -5,15 +5,18 @@
 package yamlschema
 
 import (
-	"io"
+	"bytes"
+	"encoding/json"
+	"errors"
+	"fmt"
 	"io/fs"
 	"net/url"
 	"path"
 	"strings"
 
 	"github.com/Masterminds/semver/v3"
 	"github.com/elastic/gojsonschema"
-	"github.com/pkg/errors"
+
 	"github.com/xeipuuv/gojsonreference"
 	"gopkg.in/yaml.v3"
 )
@@ -35,41 +38,56 @@ func NewReferenceLoaderFileSystem(source string, fs fs.FS, version semver.Versio
 	}
 }
 
-func (l *yamlReferenceLoader) JsonSource() interface{} { // golint:ignore
+func (l *yamlReferenceLoader) JsonSource() any { // golint:ignore
 	return l.source
 }
 
-func (l *yamlReferenceLoader) LoadJSON() (interface{}, error) {
+func (l *yamlReferenceLoader) LoadJSON() (any, error) {
 	parsed, err := url.Parse(l.source)
 	if err != nil {
-		return nil, errors.Wrapf(err, "parsing source failed (source: %s)", l.source)
+		return nil, fmt.Errorf("parsing source failed (source: %s): %w", l.source, err)
 	}
 	resourcePath := strings.TrimPrefix(parsed.Path, "/")
 
-	itemSchemaFile, err := l.fs.Open(resourcePath)
-	if err != nil {
-		return nil, errors.Wrapf(err, "opening schema file failed (path: %s)", resourcePath)
-	}
-	defer itemSchemaFile.Close()
-
-	itemSchemaData, err := io.ReadAll(itemSchemaFile)
+	itemSchemaData, err := fs.ReadFile(l.fs, resourcePath)
 	if err != nil {
-		return nil, errors.Wrap(err, "reading schema file failed")
+		return nil, fmt.Errorf("reading schema file failed: %w", err)
 	}
-
 	if len(itemSchemaData) == 0 {
 		return nil, errors.New("schema file is empty")
 	}
 
 	var schema itemSchemaSpec
 	err = yaml.Unmarshal(itemSchemaData, &schema)
 	if err != nil {
-		return nil, errors.Wrapf(err, "schema unmarshalling failed (path: %s)", l.source)
+		return nil, fmt.Errorf("schema unmarshalling failed (path: %s): %w", l.source, err)
+	}
+
+	// fixJSONNumbers ensures that the numbers in the resulting spec are of type `json.Number`, that is
+	// what the gojsonschema library expects. Without this, gojsonschema complains about some integer types
+	// not being integers.
+	// TODO: This shouldn't probably be needed, we could try to fix gojsonschema to accept other integers, or
+	// look for a YAML parser that can be customized to use `json.Number`.
+	schema.Spec, err = fixJSONNumbers(schema.Spec)
+	if err != nil {
+		return nil, fmt.Errorf("fixing numbers in parsed schema failed (path %s): %w", l.source, err)
 	}
 
 	return schema.resolve(l.version)
 }
 
+// fixJSONNumbers converts number types to `json.Number` by converting the struct to JSON and decoding it again.
+func fixJSONNumbers[T any](v T) (result T, err error) {
+	d, err := json.Marshal(v)
+	if err != nil {
+		return result, err
+	}
+
+	dec := json.NewDecoder(bytes.NewReader(d))
+	dec.UseNumber()
+	return result, dec.Decode(&result)
+}
+
 func (l *yamlReferenceLoader) JsonReference() (gojsonreference.JsonReference, error) {
 	r, err := gojsonreference.NewJsonReference(l.JsonSource().(string))
 	if err != nil {

code/go/internal/yamlschema/schema_spec.go

@@ -5,6 +5,7 @@
 package yamlschema
 
 import (
+	"bytes"
 	"encoding/json"
 	"fmt"
 
@@ -32,8 +33,13 @@ func (i *itemSchemaSpec) resolve(target semver.Version) (map[string]interface{},
 		return nil, fmt.Errorf("failed to apply patch: %w", err)
 	}
 
+	// Doesn't seem to be needed to use a decoder with UseNumber here, but it doesn't do
+	// any harm, and gojsonschema expects the use of `json.Number`.
+	dec := json.NewDecoder(bytes.NewReader(spec))
+	dec.UseNumber()
+
 	var resolved map[string]interface{}
-	err = json.Unmarshal(spec, &resolved)
+	err = dec.Decode(&resolved)
 	if err != nil {
 		return nil, fmt.Errorf("failed to unmarshal resolved spec: %w", err)
 	}

spec/changelog.yml

@@ -7,6 +7,9 @@
   - description: Validate processors used in ingest pipelines
     type: breaking-change
     link: https://github.com/elastic/package-spec/pull/586
+  - description: Stricter validation of index template settings in manifests
+    type: breaking-change
+    link: https://github.com/elastic/package-spec/pull/587
 - version: 2.10.1-next
   changes:
   - description: Prepare for next version

spec/integration/data_stream/manifest.spec.yml

@@ -159,9 +159,172 @@ spec:
         settings:
           description: Settings section of index template
           type: object
+          additionalProperties: false
+          properties:
+            analysis:
+              type: object
+              additionalProperties: false
+              properties:
+                analyzer:
+                  description: Definition of custom analyzer.
+                  type: object
+                  additionalProperties:
+                    type: object
+                    additionalProperties: false
+                    properties:
+                      type:
+                        description: Type of analyzer
+                        type: string
+                        enum:
+                          - pattern
+                      pattern:
+                        type: string
+            index:
+              type: object
+              additionalProperties: false
+              properties:
+                codec:
+                  description: >
+                    Type of compression to use. The default is to use LZ4, `best_compression` uses DEFLATE,
+                    with higher compression but lower ingestion performance.
+                  type: string
+                  enum:
+                    - default
+                    - best_compression
+                mapping:
+                  type: object
+                  additionalProperties: false
+                  properties:
+                    dimension_fields:
+                      type: object
+                      additionalProperties: false
+                      properties:
+                        limit:
+                          description: Limit on the number of dimension fields on this data stream.
+                          type: integer
+                sort:
+                  type: object
+                  additionalProperties: false
+                  properties:
+                    field:
+                      description: Fields used to sort the document in the Lucene segments.
+                      oneOf:
+                        - type: string
+                        - type: array
+                          items:
+                            type: string
+                    order:
+                      description: The sort order to use for each field.
+                      oneOf:
+                        - type: string
+                          enum:
+                            - asc
+                            - desc
+                        - type: array
+                          items:
+                            type: string
+                            enum:
+                              - asc
+                              - desc
         mappings:
           description: Mappings section of index template
           type: object
+          additionalProperties: false
+          properties:
+            date_detection:
+              description: >
+                If true, new string fields will be checked to see if their contents match with `dynamic_date_formats`.
+                If they do, the field is dynamically added with type `date`. Set to false to disable this feature.
+              type: boolean
+            dynamic:
+              description: >
+                Control if new fields can be added dynamically. When set to `false`, new fields are not indexed.
+                When set to `strict`, fields must be explicitly added to the mapping. With `runtime`, fields are
+                not indexed, but added as runtime fields. Defaults to `true`, where Elasticsearch tries to index
+                any new field.
+              oneOf:
+                - type: boolean
+                - type: string
+                  enum:
+                    - runtime
+                    - strict
+            dynamic_date_formats:
+              # Added to support `date_detection`, but usually `date_detection` is only used to disable it.
+              description: Custom date formats to use for date detection.
+              type: array
+              items:
+                type: string
+                examples:
+                  - ["strict_date_optional_time","yyyy/MM/dd HH:mm:ss Z||yyyy/MM/dd Z"] # This is the default if not set.
+                  - ["MM/dd/yyyy"]
+
+            dynamic_templates:
+              type: array
+              items:
+                type: object
+                maxProperties: 1
+                additionalProperties:
+                  type: object
+                  additionalProperties: false
+                  properties:
+                    # path_match: false # Dynamic mappings with path match can be defined in fields definitions.
+                    match_mapping_type:
+                      oneOf:
+                        - type: boolean
+                        - type: string
+                          enum:
+                          - "null"
+                          - "true"
+                          - "false"
+                          - double
+                          - long
+                          - string
+                          - object
+                          - array
+                    mapping:
+                      type: object
+                      additionalProperties: false
+                      properties:
+                        type:
+                          $ref: "./fields/fields.spec.yml#/items/properties/type"
+                        scaling_factor:
+                          $ref: "./fields/fields.spec.yml#/items/properties/scaling_factor"
+                        metrics:
+                          $ref: "./fields/fields.spec.yml#/items/properties/metrics"
+                        default_metric:
+                          $ref: "./fields/fields.spec.yml#/items/properties/default_metric"
+                        ignore_above:
+                          $ref: "./fields/fields.spec.yml#/items/properties/ignore_above"
+                patternProperties:
+                  # Exception for fields imported by elastic-package when import_mappings is used.
+                  # TODO: Allow this only on built packages.
+                  "^_embedded_ecs":
+                    type: object
+                    additionalProperties: false
+                    properties:
+                      path_match:
+                        type: string
+                      path_unmatch:
+                        type: string
+                      match:
+                        type: string
+                      match_mapping_type:
+                        $ref: "#/definitions/elasticsearch_index_template/properties/mappings/properties/dynamic_templates/items/additionalProperties/properties/match_mapping_type"
+                      mapping:
+                        type: object
+                        additionalProperties: false
+                        properties:
+                          type:
+                             $ref: "./fields/fields.spec.yml#/items/properties/type"
+                          scaling_factor:
+                            $ref: "./fields/fields.spec.yml#/items/properties/scaling_factor"
+                          ignore_malformed:
+                            $ref: "./fields/fields.spec.yml#/items/properties/ignore_malformed"
+                          fields:
+                            type: object
+                            minProperties: 1
+                            additionalProperties:
+                              $ref: "#/definitions/elasticsearch_index_template/properties/mappings/properties/dynamic_templates/items/patternProperties/^_embedded_ecs/properties/mapping"
         ingest_pipeline:
           description: Elasticsearch ingest pipeline settings
           type: object
@@ -322,6 +485,17 @@ spec:
   - title
 # JSON patches for newer versions should be placed on top
 versions:
+  - before: 3.0.0
+    patch:
+      # Stricter validation of elasticsearch settings and mappings.
+      - op: remove
+        path: /definitions/elasticsearch_index_template/properties/settings/additionalProperties
+      - op: remove
+        path: /definitions/elasticsearch_index_template/properties/settings/properties
+      - op: remove
+        path: /definitions/elasticsearch_index_template/properties/mappings/additionalProperties
+      - op: remove
+        path: /definitions/elasticsearch_index_template/properties/mappings/properties
   - before: 2.9.0
     patch:
       - op: remove

spec/integration/elasticsearch/pipeline.spec.yml

@@ -9,6 +9,7 @@ spec:
     processor:
       type: object
       additionalProperties: false
+      maxProperties: 1
       properties:
         append: { type: object }
         attachment: { type: object }

test/packages/good_v3/_dev/deploy/tf/data.json

@@ -1,3 +1,3 @@
 {
-  "a": "data file containing json"
-}
+    "a": "data file containing json"
+}

test/packages/good_v3/data_stream/ecs_import_mappings/agent/stream/stream.yml.hbs

@@ -0,0 +1,7 @@
+paths:
+{{#each paths as |path i|}}
+  - {{path}}
+{{/each}}
+exclude_files: [".gz$"]
+processors:
+  - add_locale: ~

test/packages/good_v3/data_stream/ecs_import_mappings/elasticsearch/ingest_pipeline/default.yml

@@ -0,0 +1,10 @@
+---
+description: Pipeline for processing sample logs
+processors:
+- set:
+    field: sample_field
+    value: "1"
+on_failure:
+- set:
+    field: error.message
+    value: '{{ _ingest.on_failure_message }}'

test/packages/good_v3/data_stream/ecs_import_mappings/fields/base-fields.yml

@@ -0,0 +1,12 @@
+- name: data_stream.type
+  type: constant_keyword
+  description: Data stream type.
+- name: data_stream.dataset
+  type: constant_keyword
+  description: Data stream dataset.
+- name: data_stream.namespace
+  type: constant_keyword
+  description: Data stream namespace.
+- name: '@timestamp'
+  type: date
+  description: Event timestamp.