Validate fields using ECS JSON schema when all stack versions support ecs@mappings #1711

zmoog · 2024-03-05T16:24:52Z

Change description

Enable elastic-package to import the ECS JSON schema to validate fields in two conditions:

When package developers enabled import_mappings (existing behavior).
When package developers did NOT enable import_mappings, but all stack versions in the Kibana constraints support the ecs@mappings in Elasticsearch (8.13.0+).

If the package supports one or more stack versions without ECS mappings, devs must support ECS using import_mappings (deprecated) or ecs.yml file.

Motivation

On 8.13, we enabled Fleet to include the ecs@mappings component template in all the integration index templates. So, starting from 8.13, integration developers can stop using import_mappings and the ecs.yml.

However, elastic-package should import the ECS JSON schema only if ALL stack versions in the Kibana constraints have ECS mappings from Elasticsearch.

Additional Notes

Migrate to ecs@mappings template #1649

Reviewer checklist

PR address a single concern.
PR title and description are appropriately filled.
Changes will be merged in main.
Changes are covered by tests.
Logging is meaningful in case of troubleshooting.
Docs are updated (at least the README.md, if needed).
History is clean, commit messages are meaningful (see CONTRIBUTING.md) and are well-formatted.

With the change elastic-package load JSON schema when: - package-spec > 2.3.0 and import_mappings: true - package-spec > 3.1.3 If package-spec is >= 2.3.0 and < 3.1.3 elastic-package continue to embed the legacy ecs@mappings.

We should continue support import mappings for integrations that want to. We don't want to force integrations devs to migrate.

This is an experiment to try inferring if all supported stack versions in the Kibana constraints support ECS mappings. If not, we don't include the ECS JSON schema mappings. In this case, users can opt for one of the following: - enable import mappings - add field mappings - bump the Kibana constraints >= 8.13.0

zmoog · 2024-03-07T10:31:51Z

@jsoriano, the gist of this change is to enable elastic-package to import the ECS JSON schema in two conditions:

When package developers enabled import_mappings (existing behavior).
When package developers did NOT enable import_mappings, and all stack versions in the Kibana constraints support the ecs@mappings in Elasticsearch.

Kibana constraints can be as simple as 8.0.0 or more sophisticated, like ^7.14.0 || ^8.0.0.

The semver package we're using does not offer to get the minimum supported version out of a constraints expression, and I want to try something simpler before custom parsing it.

So I ran a little experiment to infer if the Kibana constrains support a stack version without ECS support.

Let me know what you think.

We compare all ES releases without ECS support against the kibana constraints to assess if the package can use the ECS JSON schema or not.

zmoog · 2024-03-07T18:14:07Z

internal/fields/validate.go

+	// This check works under the assumption the constraints are not limited
+	// upwards.
+	//
+	// For example, if the constraint is `>= 8.12.0` and the stack version is
+	// `8.12.999`, the constraint will be satisfied.
+	//
+	// However, if the constraint is `>= 8.0.0, < 8.10.0` the check will not
+	// return the right result.
+	//
+	// To support this, we would need to check the constraint against a larger
+	// set of versions, and check if the constraint is satisfied for all
+	// of them, like in the commented out example above.
+	//
+	// lastStackVersionWithoutEcsMappings := semver.MustParse("8.12.999")
+	// return !kibanaConstraints.Check(lastStackVersionWithoutEcsMappings)


This is a simpler implementation that checks the Kibana constraints agains the latest known stack version that does NOT support ecs@mappings.

This check works under the assumption the constraints are not limited upwards (for example, ^8.10.0 or ^7.14.0 || ^8.0.0).

I plan to remove at the end of the review process, but I want to mention it as an option.

This option is actually not bad, but yeah, let's go with the other one that looks safer.

jsoriano · 2024-03-07T19:04:52Z

/test integrations

jsoriano

Looks good overall.

It would be good though to add a test case for a package with kibana.version: '^8.13.0', that uses ECS fields and doesn't embed them. I think that currently all tests are used with the default version of the stack, so maybe we need to change this for the test to work.

Pinging @mrodm to take another look on the change, and help with adding a test if needed.

internal/fields/validate.go

jsoriano · 2024-03-07T19:00:50Z

internal/fields/validate.go

+	// This check works under the assumption the constraints are not limited
+	// upwards.
+	//
+	// For example, if the constraint is `>= 8.12.0` and the stack version is
+	// `8.12.999`, the constraint will be satisfied.
+	//
+	// However, if the constraint is `>= 8.0.0, < 8.10.0` the check will not
+	// return the right result.
+	//
+	// To support this, we would need to check the constraint against a larger
+	// set of versions, and check if the constraint is satisfied for all
+	// of them, like in the commented out example above.
+	//
+	// lastStackVersionWithoutEcsMappings := semver.MustParse("8.12.999")
+	// return !kibanaConstraints.Check(lastStackVersionWithoutEcsMappings)


This option is actually not bad, but yeah, let's go with the other one that looks safer.

internal/fields/validate_test.go

internal/fields/validate.go

jsoriano · 2024-03-07T19:05:53Z

internal/fields/validate.go

 	var schema []FieldDefinition
-	if buildManifest.ImportMappings() && !specVersion.LessThan(semver2_3_0) && importECSSchema {
+	if (packageEmbedsEcsMappings || stackIncludesEcsMapping) && importECSSchema {


Please, add or modify a test package so we have a test case with a package that uses ECS fields, but doesn't embeds the mappings.

If we want to test the scenario where the package uses the ECS in Elasticsearch, we need a stack version 8.13.0.

@mrodm, which version of the stack the tests run on? Can we have stack version 8.13.0 (for all tests or just one)?

@mrodm, which version of the stack the tests run on? Can we have stack version 8.13.0 (for all tests or just one)?

Currently, the steps in the CI to test packages run with the default version. IIRC that version now should be 8.12.0.

If this needs to be changed, it needs to be updated scripts/test-check-packages.sh to allow define --version flag in the elastic-package stack up command through an environment variable or so. This will require also update

Makefile: to add a new target

Add into the buildkite script this new target

In this implementation, the ECS fields are imported before validation by the test runners (e.g. kibana version condition is ^8.13.0), so there should not be errors related to field definitions. Is it required to start an Elastic stack ^8.13.0 too? @zmoog

In this implementation, the ECS fields are imported before validation by the test runners (e.g. kibana version condition is ^8.13.0), so there should not be errors related to field definitions. Is it required to start an Elastic stack ^8.13.0 too?

Correct!

elastic-package validates the ECS fields using the JSON schema during the static checks, so it does not need to run on Elastic stack ^8.13.0.

Co-authored-by: Jaime Soriano Pastor <[email protected]>

There are no package versions for older stacks. Sort the stack version numbers (`sort -V` is your friend here).

Since most of current integrations support 8.x: $ cat all_kibana_version.txt | awk -F',' '{print $3}' | sort | uniq -c 3 ^7.14.0 || ^8.0.0 5 ^7.14.1 || ^8.0.0 1 ^7.14.1 || ^8.8.0 10 ^7.16.0 || ^8.0.0 4 ^7.17.0 || ^8.0.0 6 ^8.0.0 7 ^8.10.1 6 ^8.10.2 3 ^8.11.0 2 ^8.11.2 75 ^8.12.0 1 ^8.12.1 1 ^8.12.2 44 ^8.13.0 1 ^8.2.0 2 ^8.2.1 6 ^8.3.0 2 ^8.4.0 4 ^8.5.0 1 ^8.5.1 2 ^8.6.0 1 ^8.6.1 1 ^8.7.0 24 ^8.7.1 27 ^8.8.0 1 ^8.8.1 1 ^8.8.2 11 ^8.9.0 It's more efficient to check for recent stack versions.

Co-authored-by: Jaime Soriano Pastor <[email protected]>

flash1293

New to the project so can't comment on small stylistic things or anything.

About the approach itself - I know I suggested this approach with listing all existing stack versions @zmoog , but looking at it now there is actually another case we don't capture here. Due to security updates, it's possible to see new patch versions for older major/minor versions (e.g. 7.17.19 could be added). As this new version won't be included in the check, it opens a small theoretical gap in this check.

Two options:

Just live with this - it probably won't cause issues in practice
Come up with a process to update this list either in a runtime fashion (load the list of versions from some central place during execution) or via a code change to elastic-package

It feels like the coverage by this check is good enough and we should just move forward with this approach for now - we can always revisit and I honestly don't expect this to cause issues as I can't see why someone would retroactively add Kibana version compatibility with just a single patch release of a previous version.

I'm also trying to improve naming and flow.

zmoog · 2024-03-08T11:59:31Z

Two options:

Just live with this - it probably won't cause issues in practice

Come up with a process to update this list either in a runtime fashion (load the list of versions from some central place during execution) or via a code change to elastic-package

It feels like the coverage by this check is good enough and we should just move forward with this approach for now - we can always revisit and I honestly don't expect this to cause issues as I can't see why someone would retroactively add Kibana version compatibility with just a single patch release of a previous version.

It's a fair point.

Wha't the kibana constraints expression that would break the current implementation?

flash1293 · 2024-03-08T12:09:03Z

Wha't the kibana constraints expression that would break the current implementation?

7.17.19 || > 8.13

zmoog · 2024-03-08T14:24:29Z

7.17.19 || > 8.13

Even if unlikely, it's still possible.

I would address this potential risk by adding a few extra upcoming 7.17.x versions, and move the PR forward with this implementation.

I can create a separate issue to reimplement the allVersionsIncludeECS() with an HTTP call to something like the GitHub releases.

I want to enable the elastic package to deal with ECS mappings and allow the remaining step of the ecs@mappings migration to happen as soon as possible.

@flash1293, WDYT?

flash1293 · 2024-03-08T14:28:45Z

@zmoog sounds good to me, let's move forward with this for now and create a follow-up issue.

Adding a few extra releases to support unlikely yet possible edge cases.

mrodm

Thanks @zmoog !

Just minor suggestions.

mrodm · 2024-03-12T11:34:39Z

internal/fields/validate.go

 	var schema []FieldDefinition
-	if buildManifest.ImportMappings() && !specVersion.LessThan(semver2_3_0) && importECSSchema {
+	if (packageEmbedsEcsMappings || stackSupportsEcsMapping) && importECSSchema {


Probably it could be added here a logger.Debug message to mention that all field definitions from the external schema are going to be imported for validation. WDYT? Wondering if that would help in case there are errors related to some field is defined.

if (packageEmbedsEcsMappings || stackSupportsEcsMapping) && importECSSchema { // Import all fields from external schema (most likely ECS) to // validate the package fields against it. ecsSchema, err := fdm.ImportAllFields(defaultExternal) if err != nil { return nil, nil, err } schema = ecsSchema logger.Debug("Imported field definitions from external schema") } return fdm, schema, nil }

Good point! And I remember considering adding it, but but it slipped my mind. I'm doing it now.

internal/fields/validate_test.go

internal/fields/validate.go

mrodm · 2024-03-12T11:51:55Z

test/packages/other/ecs_mappings/_dev/build/docs/README.md

+Test package to verify support for ECS mappings available to integrations running on stack version 8.13.0 and later.
+
+Please note that the package:
+
+- does not embed the legacy ECS mappings (no `import_mappings`).
+- does not define fields in the `ecs.yml` file. 
+
+Mappings for ECS fields (for example, `ecs.version`) come from the `ecs@mappings` component template in the integration index template, which has been available since 8.13.0. 


Thanks for this test package 👍

mrodm · 2024-03-12T12:13:36Z

internal/fields/validate.go

 	var schema []FieldDefinition
-	if buildManifest.ImportMappings() && !specVersion.LessThan(semver2_3_0) && importECSSchema {
+	if (packageEmbedsEcsMappings || stackIncludesEcsMapping) && importECSSchema {


@mrodm, which version of the stack the tests run on? Can we have stack version 8.13.0 (for all tests or just one)?

Currently, the steps in the CI to test packages run with the default version. IIRC that version now should be 8.12.0.

If this needs to be changed, it needs to be updated scripts/test-check-packages.sh to allow define --version flag in the elastic-package stack up command through an environment variable or so. This will require also update

Makefile: to add a new target

Add into the buildkite script this new target

In this implementation, the ECS fields are imported before validation by the test runners (e.g. kibana version condition is ^8.13.0), so there should not be errors related to field definitions. Is it required to start an Elastic stack ^8.13.0 too? @zmoog

Co-authored-by: Mario Rodriguez Molins <[email protected]>

elasticmachine · 2024-03-13T17:05:05Z

💚 Build Succeeded

Buildkite Build
Commit: 9e333ac

History

💚 Build #2579 succeeded f15fd18
💔 Build #2575 failed e8f0bae
💚 Build #2574 succeeded cb9f689
💚 Build #2572 succeeded bb16985
💚 Build #2567 succeeded 1eb3df1
💔 Build #2566 failed edc0bf9

cc @zmoog

mrodm

Thanks @zmoog ! 👍

Load JSON schema for ECS when package-spec >= 3.1.3

4395eb3

With the change elastic-package load JSON schema when: - package-spec > 2.3.0 and import_mappings: true - package-spec > 3.1.3 If package-spec is >= 2.3.0 and < 3.1.3 elastic-package continue to embed the legacy ecs@mappings.

zmoog mentioned this pull request Mar 5, 2024

Migrate to ecs@mappings template #1649

Closed

zmoog self-assigned this Mar 5, 2024

zmoog added the enhancement New feature or request label Mar 5, 2024

zmoog changed the title ~~[WIP] Load JSON schema for ECS when package-spec >= 3.1.3~~ [WIP] Validate fields using JSON schema for ECS when package-spec >= 3.1.3 Mar 6, 2024

zmoog added 2 commits March 6, 2024 13:01

Continue support import mappings (deprecated only)

91c9085

We should continue support import mappings for integrations that want to. We don't want to force integrations devs to migrate.

Cleanup

02c8e9c

zmoog force-pushed the zmoog/deprecate-import-mappings branch from 3bc07d7 to 02c8e9c Compare March 6, 2024 12:03

zmoog changed the title ~~[WIP] Validate fields using JSON schema for ECS when package-spec >= 3.1.3~~ Validate fields using JSON schema for ECS when package-spec >= 3.1.3 Mar 6, 2024

zmoog changed the title ~~Validate fields using JSON schema for ECS when package-spec >= 3.1.3~~ Validate fields using ECS JSON schema when all stack versions support ecs@mappings Mar 7, 2024

zmoog added 2 commits March 7, 2024 18:36

Use all ES releases < 8.13.0 to infer ECS support

edc0bf9

We compare all ES releases without ECS support against the kibana constraints to assess if the package can use the ECS JSON schema or not.

Fix linter objections (sorry pal, you're right!)

1eb3df1

zmoog marked this pull request as ready for review March 7, 2024 18:09

zmoog requested review from jsoriano, mrodm and flash1293 March 7, 2024 18:09

zmoog commented Mar 7, 2024

View reviewed changes

jsoriano reviewed Mar 7, 2024

View reviewed changes

zmoog and others added 5 commits March 8, 2024 11:29

Update internal/fields/validate.go

bb16985

Co-authored-by: Jaime Soriano Pastor <[email protected]>

Remove stack versions that predate Fleet GA

bb05a75

There are no package versions for older stacks. Sort the stack version numbers (`sort -V` is your friend here).

Pre-parse all stack versions and move them out fn

cf82a7d

Update internal/fields/validate_test.go

cb9f689

Co-authored-by: Jaime Soriano Pastor <[email protected]>

flash1293 reviewed Mar 8, 2024

View reviewed changes

Move ECS support checking in dedicated function

e8f0bae

I'm also trying to improve naming and flow.

zmoog added 2 commits March 8, 2024 18:44

Add upcoming 7.17.x patch releases

151c527

Adding a few extra releases to support unlikely yet possible edge cases.

Add package to test ecs@mappings support

f15fd18

mrodm reviewed Mar 12, 2024

View reviewed changes

zmoog and others added 3 commits March 13, 2024 17:22

Apply suggestions from code review

f0a1ee1

Co-authored-by: Mario Rodriguez Molins <[email protected]>

Log on importing JSON schema from external source

15f2529

Move log message to the right place

9e333ac

mrodm approved these changes Mar 14, 2024

View reviewed changes

jsoriano approved these changes Mar 18, 2024

View reviewed changes

zmoog merged commit 30b16dc into main Mar 18, 2024
3 checks passed

zmoog deleted the zmoog/deprecate-import-mappings branch March 18, 2024 13:57

jsoriano mentioned this pull request May 9, 2024

[MySQL] Add SSL Support elastic/integrations#9453

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Validate fields using ECS JSON schema when all stack versions support ecs@mappings #1711

Validate fields using ECS JSON schema when all stack versions support ecs@mappings #1711

zmoog commented Mar 5, 2024 •

edited

Loading

zmoog commented Mar 7, 2024 •

edited

Loading

zmoog Mar 7, 2024

jsoriano Mar 7, 2024

jsoriano commented Mar 7, 2024

jsoriano left a comment

jsoriano Mar 7, 2024

jsoriano Mar 7, 2024

zmoog Mar 8, 2024

mrodm Mar 12, 2024

zmoog Mar 13, 2024

flash1293 left a comment •

edited

Loading

zmoog commented Mar 8, 2024

flash1293 commented Mar 8, 2024

zmoog commented Mar 8, 2024 •

edited

Loading

flash1293 commented Mar 8, 2024

mrodm left a comment

mrodm Mar 12, 2024

zmoog Mar 13, 2024

mrodm Mar 12, 2024

mrodm Mar 12, 2024

elasticmachine commented Mar 13, 2024

mrodm left a comment

Validate fields using ECS JSON schema when all stack versions support ecs@mappings #1711

Validate fields using ECS JSON schema when all stack versions support ecs@mappings #1711

Conversation

zmoog commented Mar 5, 2024 • edited Loading

Change description

Motivation

Additional Notes

Reviewer checklist

zmoog commented Mar 7, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jsoriano commented Mar 7, 2024

jsoriano left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

flash1293 left a comment • edited Loading

Choose a reason for hiding this comment

zmoog commented Mar 8, 2024

flash1293 commented Mar 8, 2024

zmoog commented Mar 8, 2024 • edited Loading

flash1293 commented Mar 8, 2024

mrodm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elasticmachine commented Mar 13, 2024

💚 Build Succeeded

History

mrodm left a comment

Choose a reason for hiding this comment

zmoog commented Mar 5, 2024 •

edited

Loading

zmoog commented Mar 7, 2024 •

edited

Loading

flash1293 left a comment •

edited

Loading

zmoog commented Mar 8, 2024 •

edited

Loading