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

Tests fail due to ECS expected values #1472

Closed
chrisberkhout opened this issue Sep 26, 2023 · 3 comments · Fixed by elastic/package-spec#616
Closed

Tests fail due to ECS expected values #1472

chrisberkhout opened this issue Sep 26, 2023 · 3 comments · Fixed by elastic/package-spec#616
Labels
bug Something isn't working

Comments

@chrisberkhout
Copy link
Contributor

chrisberkhout commented Sep 26, 2023

Expected values defined for an ECS field, such as for threat.indicator.type, are not the only valid values (def, emphasis added):

expected_values (optional): An array of expected values for the field. Schema consumers can validate integrations and mapped data against the listed values. These values are the recommended convention, but users may also use other values.

The expected_values field was intended to indicate the "SHOULD" requirement level (as defined for RFCs). For this level it may be appropriate to warn about the use of other values, but not to fail a build or test run.

Currently, pipeline and system tests will fail if they find values other than the expected ones.

A previous workaround when using ECS dynamic mappings was to override with explicit definitions. That makes tests pass now if those definitions include the full set of values or an empty array (example), but now elastic-package will not successfully build a package if it defines its own expected_values, so this workaround no longer works.

Some related links:

@chrisberkhout
Copy link
Contributor Author

As discussed today with @andrewkroh and @P1llus, and earlier with @kgeller and @ebeahan.

@jsoriano
Copy link
Member

Thanks for the detailed description.

The expected_values field was intended to indicate the "SHOULD" requirement level (as defined for RFCs). For this level it may be appropriate to warn about the use of other values, but not to fail a build or test run.

By now I would stay on the restrictive side of things. I don't see other packages using expected_values in integrations.

In any case, it sounds good to allow to define expected_values per field. I am opening elastic/package-spec#616 by now to support that.

Also, once elastic/package-spec#558 is done, we can also add support to selectively disable this validation.

@chrisberkhout
Copy link
Contributor Author

IMHO breaking a build due to a valid though not expected value is something I'd rather opt into than have as default.

It's not critical for me now since I found a variation on my earlier workaround that still works.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants