Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add initial support to validate the mappings in system tests #2214

Merged
merged 62 commits into from
Dec 12, 2024

Conversation

mrodm
Copy link
Contributor

@mrodm mrodm commented Nov 8, 2024

Closes #2206

Add validation of the mappings when running system tests.

This process compares the mappings installed by Fleet (preview mappings) with the ones obtained after ingesting new documents into Elasticsearch.

This validation for the time being is behind an environment variable as a technical preview:

# Default value and same behavior as now (validation of fields)
ELASTIC_PACKAGE_FIELD_VALIDATION_TEST_METHOD=fields elastic-package test system -v

# Test just validation of mappings 
ELASTIC_PACKAGE_FIELD_VALIDATION_TEST_METHOD=mappings elastic-package test system -v

# Test both validation of mappings and fields
ELASTIC_PACKAGE_FIELD_VALIDATION_TEST_METHOD=all elastic-package test system -v

Assumptions considered while working on this PR: #2206 (comment)

As part of this PR:

  • CI pipeline has been updated to remove steps related to running system tests with Elastic Agent.
    • It will be kept one step to ensure that the feature is working.
  • CI pipeline has been updated to add steps to run mapping validation in system tests.

Author's checklist

  • Update CI steps to remove old stack agents
  • Update CI to add new steps for validating fields using mappings
  • Add environment variable to validate that fields are documented with different methods
  • Add new environment variables to docs.
  • Remove debug/testing log statements
  • Test with Elastic stack 8.x
    • Some packages are failing like prometheus failing since some fields depend on dynamic templates (not implemented yet)
  • Test with Elastic stack 7.x (f5 package in integrations repository)
  • Test with integration and input packages (sql_input test package in elastic-package repo).
  • Test with Elastic stacks with LogsDB enabled (synthetics).
    • Tested in CI with ti_anomali_logsdb and auth0_logsdb

@mrodm mrodm self-assigned this Nov 8, 2024
@mrodm
Copy link
Contributor Author

mrodm commented Nov 12, 2024

/test


dataStreamName string

LocalSchema []FieldDefinition
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New variable to hold just the Field Definitions from the local directory (package).

It is needed to check if a given mapping is related to a field definition with type array.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not using Schema?

@@ -295,6 +328,8 @@ func createValidatorForDirectoryAndPackageRoot(fieldsParentDir string, finder pa
}

v.Schema = append(fields, v.Schema...)

v.LocalSchema = fields
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added here for completeness, but it is not used in this validator.

Should we remove it from here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If not used I would say to remove it.

@mrodm
Copy link
Contributor Author

mrodm commented Dec 9, 2024

test integrations

@elastic-vault-github-plugin-prod

Created or updated PR in integrations repository to test this version. Check elastic/integrations#11828

@elasticmachine
Copy link
Collaborator

elasticmachine commented Dec 11, 2024

💛 Build succeeded, but was flaky

Failed CI Steps

History

cc @mrodm

@mrodm
Copy link
Contributor Author

mrodm commented Dec 11, 2024

test integrations

@elastic-vault-github-plugin-prod

Created or updated PR in integrations repository to test this version. Check elastic/integrations#11828

@mrodm
Copy link
Contributor Author

mrodm commented Dec 11, 2024

Another package (tines) failing due to a missing mapping https://buildkite.com/elastic/integrations/builds/19362#0193b678-1607-4f12-8d46-c212e3cd4f1e

Should it be validated using dynamic templates?

Error:

test case failed: one or more errors found in mappings in logs-tines.audit_logs index template: [0] field "tines.audit_log.inputs.inputs.options" is undefined: missing definition for path (not in ECS)

Field definition in the package:

                - name: options
                  type: object
                  object_type: keyword
                  object_type_mapping_type: "*"

No mapping created in the preview, and this mapping is created in the actual mappings after ingesting some docs (no dynamic template present):

        "tines.audit_log.inputs.inputs.options": {
          "type": "keyword",
          "ignore_above": 1024
        },

cc @jsoriano

@mrodm mrodm requested a review from jsoriano December 11, 2024 18:04
@jsoriano
Copy link
Member

No mapping created in the preview

This is weird because the generated component template makes sense. In tines 1.13 subobjects: false` is moved from an object to the whole index, and this is the difference in the component template:

+++ /tmp/tines-subobjects-false-global/component_templates/logs-tines.audit_logs@package.json	2024-12-11 19:53:31.500743669 +0100
@@ -7,7 +7,7 @@
           "lifecycle": {
             "name": "logs"
           },
-          "default_pipeline": "logs-tines.audit_logs-1.12.2",
+          "default_pipeline": "logs-tines.audit_logs-1.13.0",
           "mapping": {
             "total_fields": {
               "limit": "1000"
@@ -113,8 +113,7 @@
                               "createFormEmptyState": {
                                 "type": "boolean"
                               }
-                            },
-                            "subobjects": false
+                            }
                           },
                           "eventName": {
                             "ignore_above": 1024,
@@ -347,7 +346,8 @@
               }
             }
           }
-        }
+        },
+        "subobjects": false
       }
     },
     "_meta": {

I can see mappings in the preview, the main difference is that when subobjects: false is used, as you mention, the properties are directly defined in the root, with dots in the names:

      ...
      "properties": {
        "@timestamp": {
          "type": "date",
          "ignore_malformed": false
        },
        "data_stream.dataset": {
          "type": "constant_keyword"
        },
        "data_stream.namespace": {
          "type": "constant_keyword"
        },
        "data_stream.type": {
          "type": "constant_keyword"
        },
        "event.agent_id_status": {
          "type": "keyword",
          "ignore_above": 1024
        },
        ...

With what version of the stack you are missing the mappings in the preview?

@jsoriano
Copy link
Member

Ah, you mean that no mapping is generated in the preview for this dynamic mapping. Yes, this looks like some issue in Fleet, it is not generating the dynamic mappings for subobjects.

@jsoriano
Copy link
Member

jsoriano commented Dec 11, 2024

The problem is not related to subobjects, it can be also reproduced without setting subobjects: false. I think the problem is related to tines.audit_log.inputs.inputs.options not being empty, it also defines a property:

                - name: options.createFormEmptyState
                  type: boolean

This looks like an issue in Fleet, it could generate both mappings, the more specific static mapping for the boolean, and the dynamic template as catch all for the rest.

Copy link
Member

@jsoriano jsoriano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can go on with this change and polish things in future PRs.

@mrodm
Copy link
Contributor Author

mrodm commented Dec 12, 2024

Ah, you mean that no mapping is generated in the preview for this dynamic mapping.

Exactly, that specific field definition had no mapping nor dynamic mapping in the preview.

The problem is not related to subobjects, it can be also reproduced without setting subobjects: false. I think the problem is related to tines.audit_log.inputs.inputs.options not being empty, it also defines a property:

                - name: options.createFormEmptyState
                  type: boolean

This looks like an issue in Fleet, it could generate both mappings, the more specific static mapping for the boolean, and the dynamic template as catch all for the rest.

Thanks for checking this out!

@mrodm mrodm merged commit 6def317 into elastic:main Dec 12, 2024
3 checks passed
@mrodm mrodm deleted the validate-mappings-first branch December 12, 2024 11:02
jsoriano added a commit to elastic/integrations that referenced this pull request Dec 16, 2024
No dynamic mapping was being generated for
tines.audit_log.inputs.inputs.options.*, and this package uses the
tines.audit_log.inputs.inputs.options field directly, without having any
mapping for it or its sub-properties.

The workaround ensures that there is a mapping for
tines.audit_log.inputs.inputs.* that serves for
tines.audit_log.inputs.inputs.options as well as for its subobjects.

The configured dynamic mapping was not being generated due to some issue in
Fleet that we are investigating.

We detected this issue while refactoring field mappings tests in
elastic-package, more about this in elastic/elastic-package#2214[1].

[1]elastic/elastic-package#2214 (comment)

Co-authored-by: Dan Kortschak <dan.kortschak@elastic.co>
harnish-elastic pushed a commit to harnish-elastic/integrations that referenced this pull request Feb 4, 2025
…ic#12082)

No dynamic mapping was being generated for
tines.audit_log.inputs.inputs.options.*, and this package uses the
tines.audit_log.inputs.inputs.options field directly, without having any
mapping for it or its sub-properties.

The workaround ensures that there is a mapping for
tines.audit_log.inputs.inputs.* that serves for
tines.audit_log.inputs.inputs.options as well as for its subobjects.

The configured dynamic mapping was not being generated due to some issue in
Fleet that we are investigating.

We detected this issue while refactoring field mappings tests in
elastic-package, more about this in elastic/elastic-package#2214[1].

[1]elastic/elastic-package#2214 (comment)

Co-authored-by: Dan Kortschak <dan.kortschak@elastic.co>
qcorporation pushed a commit to elastic/integrations that referenced this pull request Feb 4, 2025
No dynamic mapping was being generated for
tines.audit_log.inputs.inputs.options.*, and this package uses the
tines.audit_log.inputs.inputs.options field directly, without having any
mapping for it or its sub-properties.

The workaround ensures that there is a mapping for
tines.audit_log.inputs.inputs.* that serves for
tines.audit_log.inputs.inputs.options as well as for its subobjects.

The configured dynamic mapping was not being generated due to some issue in
Fleet that we are investigating.

We detected this issue while refactoring field mappings tests in
elastic-package, more about this in elastic/elastic-package#2214[1].

[1]elastic/elastic-package#2214 (comment)

Co-authored-by: Dan Kortschak <dan.kortschak@elastic.co>
harnish-elastic pushed a commit to harnish-elastic/integrations that referenced this pull request Feb 5, 2025
…ic#12082)

No dynamic mapping was being generated for
tines.audit_log.inputs.inputs.options.*, and this package uses the
tines.audit_log.inputs.inputs.options field directly, without having any
mapping for it or its sub-properties.

The workaround ensures that there is a mapping for
tines.audit_log.inputs.inputs.* that serves for
tines.audit_log.inputs.inputs.options as well as for its subobjects.

The configured dynamic mapping was not being generated due to some issue in
Fleet that we are investigating.

We detected this issue while refactoring field mappings tests in
elastic-package, more about this in elastic/elastic-package#2214[1].

[1]elastic/elastic-package#2214 (comment)

Co-authored-by: Dan Kortschak <dan.kortschak@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants