-
Notifications
You must be signed in to change notification settings - Fork 91
Stricter validation of data stream settings #587
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
Changes from 6 commits
043d972
77c7c0c
523c623
709751c
0383e6b
7e56432
2d31fd8
d946169
e71560a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -159,9 +159,142 @@ 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 # No idea why this doesn't work. But this will be also checked by Elasticsearch. | ||
| 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" | ||
|
Comment on lines
+288
to
+297
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looking at this mapping definition, when Checking those mappings, at least
These definitions would be in the built package, and this built package is going to be checked by our publishing CI jobs to validate them before publishing them. So I guess more fields would be needed here, wouldn't it?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added exception for these fields in d946169. |
||
| ingest_pipeline: | ||
| description: Elasticsearch ingest pipeline settings | ||
| type: object | ||
|
|
@@ -322,6 +455,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 | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tried to use this value, but I get this error:
I didn't find why it could be detected as a different type than integer :/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Workaround added in 2d31fd8
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for adding this!