Add support for an optional value in discovery fields#917
Add support for an optional value in discovery fields#917mrodm merged 11 commits intoelastic:mainfrom
Conversation
| "conditions.kibana.version must be ^8.8.0 or greater for non experimental input packages (version > 1.0.0)", | ||
| }, | ||
| "httpjson_input": []string{ | ||
| "httpjson_input": { |
There was a problem hiding this comment.
Remove these types to be as other cases in other tests.
| discovery: | ||
| fields: | ||
| - name: 12345 | ||
| - value: nginx.access |
There was a problem hiding this comment.
Examples of incorrect discovery fields:
namemust be a string.- a discovery field must contain a
namefield.valuefield is the optional one.
| - true | ||
| - 12345 | ||
| required: | ||
| - name |
There was a problem hiding this comment.
Ensure that name is always set in each array item.
| name: | ||
| description: Name of the field. | ||
| type: string | ||
| value: |
There was a problem hiding this comment.
Following the same as mentioned in this link: https://github.com/elastic/package-spec/pull/915/files#r2178695513
@kpollich could this new field be ignored by the current Kibana version ? or could it cause any issue ?
Depending on that, it should be added to the following patch version 3.4.1-next or add it to the following minor 3.5.0-next.
There was a problem hiding this comment.
This field will be ignored by Kibana IIUC - there are no user-facing features that make use of the discovery fields in Kibana right now so we are free to make this change without considering impact to Kibana.
There was a problem hiding this comment.
Perfect, then I think it can be kept as 3.4.1-next
| - type: string | ||
| minLength: 1 |
There was a problem hiding this comment.
Here, I'm forcing that if it is a string, it should not be an empty string.
For instance, this would not be allowed:
discovery:
fields:
- name: event.dataset
value: ""Would that make sense ?
Or, should we allow to compare a field with an empty string?
Depending on that, the validation in EPR could be different.
There was a problem hiding this comment.
should we allow to compare a field with an empty string?
I guess this is in theory possible, but I don't know what would be the use case. So feel free to keep this minLength.
There was a problem hiding this comment.
Looking around the spec, there is no usage of this minLength for other string values.
As you mention, I don't know of any use case where it is required to look for a package that a given field is empty. So, I'll keep that minLength for now.
| - type: string | ||
| minLength: 1 |
There was a problem hiding this comment.
should we allow to compare a field with an empty string?
I guess this is in theory possible, but I don't know what would be the use case. So feel free to keep this minLength.
Co-authored-by: Jaime Soriano Pastor <jaime.soriano@elastic.co>
💚 Build Succeeded
History
cc @mrodm |
What does this PR do?
Allow to declare discovery fields rules matching specific values.
Example of discovery field defining a specific value for a field:
Why is it important?
It will allow to show packages that use certain fields and also show packages that use fields with some specific data.
Checklist
test/packagesthat prove my change is effective.spec/changelog.yml.discovery.fieldsspec inmanifest.ymlsupports an optionalvaluefieldnameRelated issues