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 support for ES index-template configs #552

Merged
merged 3 commits into from
Jun 30, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ Files to be loaded into the Elastic Stack:
{service}/{type}/{filename}
```

Service in the above can be `elasticsearch`, `kibana` or any other component in the Elastic Stack. The type is specific to each service. In the case of Elasticsearch it can be `ingest-pipeline`, `index-template` or could also be `index` data. For Kibana it could be `dashboard`, `visualization` or any other saved object type or other types. The names are taken from the API endpoints in each service. The file name needs to be unique inside the directory and best has a descriptive nature or unique id.
Service in the above can be `elasticsearch`, `kibana` or any other component in the Elastic Stack. The type is specific to each service. In the case of Elasticsearch it can be `ingest_pipeline`, `index_template` or could also be `index` data. For Kibana it could be `dashboard`, `visualization` or any other saved object type or other types. The names are taken from the API endpoints in each service. The file name needs to be unique inside the directory and best has a descriptive nature or unique id.

Each package can contain 2 additional directories:

Expand Down
12 changes: 12 additions & 0 deletions testdata/generated/package/reference/1.0.0/index.json
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,18 @@
}
],
"package": "reference",
"elasticsearch": {
"index_template.settings": {
"index": {
"lifecycle": {
"name": "reference"
}
}
},
"index_template.mappings": {
"dynamic": false
}
},
"path": "reference"
}
],
Expand Down
12 changes: 12 additions & 0 deletions testdata/generated/package/yamlpipeline/1.0.0/index.json
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,18 @@
}
],
"package": "yamlpipeline",
"elasticsearch": {
"index_template.settings": {
"index": {
"lifecycle": {
"name": "reference"
}
}
},
"index_template.mappings": {
"dynamic": false
}
},
"path": "log"
}
]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,12 @@ release: beta
# Allowed values: logs, metrics, events
type: logs

elasticsearch:
index_template.mappings:
dynamic: false
index_template.settings:
index.lifecycle.name: reference
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: you could break out the index_template key into it's own level and nest mappings and settings under it, like you did in testdata/package/yamlpipeline/1.0.0/dataset/log/manifest.yml.

Copy link
Member Author

Choose a reason for hiding this comment

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

I actually didn't do it on purpose to make sure I test that both options work.


# This is the ingest pipeline which should be used. If none is define,
# it checks if a default pipeline exists.
#ingest_pipeline: default
Expand Down
7 changes: 7 additions & 0 deletions testdata/package/yamlpipeline/1.0.0/dataset/log/manifest.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,13 @@ type: logs

ingest_pipeline: pipeline-entry

elasticsearch:
index_template:
mappings:
dynamic: false
settings:
index.lifecycle.name: reference

streams:
- input: logs
title: Yamlpipline example logs
Expand Down
18 changes: 12 additions & 6 deletions util/dataset.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,12 @@ type Dataset struct {
Type string `config:"type" json:"type" validate:"required"`
Name string `config:"name" json:"name,omitempty" yaml:"name,omitempty"`

Title string `config:"title" json:"title" validate:"required"`
Release string `config:"release" json:"release"`
IngestPipeline string `config:"ingest_pipeline,omitempty" config:"ingest_pipeline" json:"ingest_pipeline,omitempty" yaml:"ingest_pipeline,omitempty"`
Streams []Stream `config:"streams" json:"streams,omitempty" yaml:"streams,omitempty" `
Package string `json:"package,omitempty" yaml:"package,omitempty"`
Title string `config:"title" json:"title" validate:"required"`
Release string `config:"release" json:"release"`
IngestPipeline string `config:"ingest_pipeline,omitempty" config:"ingest_pipeline" json:"ingest_pipeline,omitempty" yaml:"ingest_pipeline,omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

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

Now that we have an Elasticsearch field, should IngestPipeline be moved under it?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point. I would say yes!

Copy link
Contributor

@ycombinator ycombinator Jun 25, 2020

Choose a reason for hiding this comment

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

Will such a change pose a problem in terms of BWC for existing manifests?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes and no. After 7.9 we must stop doing these changes but until then we can still update old ones. The way I plan such changes are:

  • Update registry and support both format. Read in and output.
  • Update Kibana and packages / integrations scripts
  • Remove legacy code

@ycombinator Any help getting this changes in is appreciated ;-)

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe I'm missing something about the plan but shouldn't there be an IngestPipeline field in the Elasticsearch struct below?

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh I see, the plan is to introduce it in #564.

Streams []Stream `config:"streams" json:"streams,omitempty" yaml:"streams,omitempty" `
Package string `json:"package,omitempty" yaml:"package,omitempty"`
Elasticsearch *Elasticsearch `config:"elasticsearch,omitempty" json:"elasticsearch,omitempty" yaml:"elasticsearch,omitempty"`

// Generated fields
Path string `json:"path,omitempty" yaml:"path,omitempty"`
Expand Down Expand Up @@ -79,6 +80,11 @@ type Variable struct {
Default interface{} `config:"default" json:"default,omitempty" yaml:"default,omitempty"`
}

type Elasticsearch struct {
IndexTemplateSettings map[string]interface{} `config:"index_template.settings" json:"index_template.settings,omitempty" yaml:"index_template.settings,omitempty"`
IndexTemplateMappings map[string]interface{} `config:"index_template.mappings" json:"index_template.mappings,omitempty" yaml:"index_template.mappings,omitempty"`
}

type fieldEntry struct {
name string
aType string
Expand Down Expand Up @@ -106,7 +112,7 @@ func NewDataset(basePath string, p *Package) (*Dataset, error) {
}

// go-ucfg automatically calls the `Validate` method on the Dataset object here
err = manifest.Unpack(d)
err = manifest.Unpack(d, ucfg.PathSep("."))
if err != nil {
return nil, errors.Wrapf(err, "error building dataset (path: %s) in package: %s", datasetPath, p.Name)
}
Expand Down