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

Update the way to add settings in the ES keystore #1377

Merged

Conversation

thbkrkr
Copy link
Contributor

@thbkrkr thbkrkr commented Jul 24, 2019

This fixes an issue where the repository-gcs plugin fails if the
setting gcs.client.default.credentials_file is not a compacted JSON
and is added using elasticsearch-keystore add key --stdin.

Since the secure settings are now in a Secret and each key of a Secret is mounted in a container as files I think we can safely change the way elasticsearch-keystore is called to add settings.

Fixes #1349.

This fixes an issue where the repository-gcs plugin fails if the
setting gcs.client.default.credentials_file is not a compacted JSON
and is added using `elasticsearch-keystore add key --stdin`.
@thbkrkr
Copy link
Contributor Author

thbkrkr commented Jul 24, 2019

✔️ --- PASS: TestUpdateESSecureSettings (597.24s)

@thbkrkr thbkrkr added the >bug Something isn't working label Jul 24, 2019
@anyasabo
Copy link
Contributor

Jenkins test this please

Copy link
Contributor

@sebgl sebgl left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -46,7 +46,7 @@ import (
// initContainerParams is used to generate the init container that will load the secure settings into a keystore
var initContainerParams = keystore.InitContainerParameters{
KeystoreCreateCommand: "/usr/share/elasticsearch/bin/elasticsearch-keystore create",
KeystoreAddCommand: "/usr/share/elasticsearch/bin/elasticsearch-keystore add",
KeystoreAddCommand: "/usr/share/elasticsearch/bin/elasticsearch-keystore add-file",
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe KeystoreAddFileCommand for explicitness?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

InitContainerParameters is used between ES, Kibana and APM Server, so we have to stay a bit abstract.

Copy link
Contributor

Choose a reason for hiding this comment

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

You're right 👍

@sebgl
Copy link
Contributor

sebgl commented Jul 25, 2019

Do we want to include this in 0.9? (My take: yes.)

@thbkrkr thbkrkr added the v0.9.0 label Jul 25, 2019
@thbkrkr thbkrkr merged commit 8099f3d into elastic:master Jul 25, 2019
@pebrc
Copy link
Collaborator

pebrc commented Jul 25, 2019

Agreed. We should backport it

thbkrkr added a commit to thbkrkr/cloud-on-k8s that referenced this pull request Jul 25, 2019
This fixes an issue where the repository-gcs plugin fails if the
setting gcs.client.default.credentials_file is not a compacted JSON
and is added using `elasticsearch-keystore add key --stdin`.

Since the secure settings are now in a Secret and each key of a 
Secret is mounted in a container as files I think we can safely change
the way elasticsearch-keystore is called to add settings.
thbkrkr added a commit that referenced this pull request Jul 25, 2019
This fixes an issue where the repository-gcs plugin fails if the
setting gcs.client.default.credentials_file is not a compacted JSON
and is added using `elasticsearch-keystore add key --stdin`.

Since the secure settings are now in a Secret and each key of a 
Secret is mounted in a container as files I think we can safely change
the way elasticsearch-keystore is called to add settings.
@thbkrkr thbkrkr deleted the update-howto-add-settings-in-es-keystore branch July 30, 2019 08:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug Something isn't working v0.9.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

GCS Repository plugin not loading
4 participants