Add support for elasticsearch.privileges.cluster #750
Conversation
💚 Build Succeeded
Expand to view the summary
Build stats
🤖 GitHub commentsTo re-run your PR in the CI, just comment with:
|
There was a problem hiding this comment.
I'm bit confused, because we don't process this option in the Package Registry at all. It isn't exposed via HTTP API and the package-spec and elastic-package's linter make sure that this is correct.
So, do we need to define it here? WDYT @jsoriano?
packages/datastream.go
Outdated
| IndexTemplateMappings map[string]interface{} `config:"index_template.mappings" json:"index_template.mappings,omitempty" yaml:"index_template.mappings,omitempty"` | ||
| IngestPipelineName string `config:"ingest_pipeline.name,omitempty" json:"ingest_pipeline.name,omitempty" yaml:"ingest_pipeline.name,omitempty"` | ||
| Privileges *ElasticsearchPrivileges `config:"privileges,omitempty" json:"privileges,omitempty" yaml:"privileges,omitempty"` | ||
| type DsElasticsearch struct { |
There was a problem hiding this comment.
nit: let's expand the Ds and replace it with DataStreamElasticsearch
packages/package.go
Outdated
| @@ -144,6 +145,14 @@ type Image struct { | |||
| Type string `config:"type" json:"type,omitempty"` | |||
| } | |||
|
|
|||
| type PkgElasticsearch struct { | |||
There was a problem hiding this comment.
nit: same situation, PackageElasticsearch
packages/package.go
Outdated
| Privileges *PkgElasticsearchPrivileges `config:"privileges,omitempty" json:"privileges,omitempty" yaml:"privileges,omitempty"` | ||
| } | ||
|
|
||
| type PkgElasticsearchPrivileges struct { |
There was a problem hiding this comment.
nit: PackageElasticsearchPrivileges
+1, no need to add anything that is not exposed through the API. |
|
Let's close this as it doesn't seem to be neccesary, @nchaulet please reopen if we are missing something, thanks! |
|
@mtojek @jsoriano the goal of that PR is to expose elasticsearch.privileges.cluster in the http API you can see it exposed here https://github.com/elastic/package-registry/blob/b7285e51b85d0ac58a12f3172de5d8d4ef1007a5/testdata/generated/package/elasticsearch_privileges/1.0.0/index.json |
|
Ah, thanks for pointing this out, Nicolas! Will this option be used by Kibana before downloading the package? The same information is in manifest hence our concerns. |
The information will be used in Kibana when installing the package, currently we never consume the manifest.yml directly in Kibana we only use the results from the HTTP api. |
…ture-support-elasticsearch-cluster-privileges
Umm, this is a bit concerning to me. |
@jsoriano I does not feel confident to change how we consume packages in Kibana right now. Do you think we could move forward with that PR and create an issue to address and discuss that in Kibana? |
|
Yep, let's move this on and discuss later if more things would need to change. |
|
@nchaulet oh btw, could you please add a changelog entry? |
Issue created to discuss the change in Kibana: elastic/kibana#115032 |
Description
Related to elastic/package-spec#226
This PR add support for
elasticsearch.privileges.clusterin the package manifest. This will allow packages to specify cluster privileges.