Create keystore on package install by jasontedor · Pull Request #28928 · elastic/elasticsearch

jasontedor · 2018-03-07T19:53:25Z

This commit removes the ability to specify that a plugin requires the keystore and instead creates the keystore on package installation or when Elasticsearch is started for the first time. The reason that we opt to create the keystore on package installation is to ensure that the keystore has the correct permissions (the package installation scripts run as root as opposed to Elasticsearch running as the elasticsearch user) and to enable removing the keystore on package removal if the keystore is not modified.

jasontedor · 2018-03-07T19:55:12Z

run packaging tests

jasontedor · 2018-03-07T19:55:23Z

FYI @elastic/es-core-infra.

rjernst

LGTM, just a suggestion about file naming

rjernst · 2018-03-07T20:34:17Z

distribution/packages/src/common/scripts/postinst

+    /usr/share/elasticsearch/bin/elasticsearch-keystore create
+    chown root:elasticsearch "$ES_PATH_CONF"/elasticsearch.keystore
+    chmod 660 "$ES_PATH_CONF"/elasticsearch.keystore
+    md5sum "$ES_PATH_CONF"/elasticsearch.keystore > "$ES_PATH_CONF"/elasticsearch.keystore.md5sum


Maybe name this something to indicate it is only the "initial" md5 sum? Otherwise someone might look at it and expect it to be kept up to date as changes are made to the keystone?

It could also have "install" in the name instead of "initial" (or anything to indicate is is only for the version created at install time).

I pushed 861d80f.

jasontedor · 2018-03-07T22:12:32Z

run packaging tests

rjernst

LGTM

rjernst · 2018-03-07T22:16:18Z

qa/vagrant/src/test/resources/packaging/utils/packages.bash

    assert_file "$ESCONFIG" d root elasticsearch 2750
+    assert_file "$ESCONFIG/elasticsearch.keystore" f root elasticsearch 660
+
+    sudo -u elasticsearch "$ESHOME/bin/elasticsearch-keystore" list | grep "keystore.seed"


Is this keystore list command meant to be checked? Is this run with set -e somewhere?

All commands in the packaging tests are run with set -e (unless explicitly flipped, which we do not currently do anywhere).

jasontedor · 2018-03-07T22:27:39Z

run packaging tests

* master: [TEST] AwaitsFix QueryRescorerIT.testRescoreAfterCollapse Decouple XContentType from StreamInput/Output (elastic#28927) Remove BytesRef usage from XContentParser and its subclasses (elastic#28792)

jasontedor · 2018-03-07T22:39:06Z

run packaging tests

jasontedor · 2018-03-07T23:11:57Z

test this please

jasontedor · 2018-03-08T00:45:21Z

run packaging tests

jasontedor · 2018-03-08T15:51:39Z

run packaging tests

jasontedor · 2018-03-12T09:41:38Z

run packaging tests

This commit removes the ability to specify that a plugin requires the keystore and instead creates the keystore on package installation or when Elasticsearch is started for the first time. The reason that we opt to create the keystore on package installation is to ensure that the keystore has the correct permissions (the package installation scripts run as root as opposed to Elasticsearch running as the elasticsearch user) and to enable removing the keystore on package removal if the keystore is not modified.

lcawl · 2018-06-15T19:41:24Z

@jasontedor This item appears in the Breaking changes section of the release notes, but I don't think I see it in the "Breaking Changes" page. Should we add info there?

Elasticsearch's keystore initial md5sum was added in elastic#28928 with the intention to allow us to remove the elasticsearch.keystore file upon package removal, if this hadn't been altered after installation. At that time this decision made perfect sense as the elasticsearch keystore only contains transient data by default ( keystore.seed ) that is meant to be useful for bootstrap related actions, and doesn't need to survive re-installations. With Security ON by default, we will be storing additional settings in the keystore upon installation(namely, the passwords for the PKCS#12 keystores used for TLS) and these have a more persistent nature. Since `remove` doesn't delete the configuration directories and files where said PKCS#12 keystores are stored, it makes sense to also not delete the elasticsearch.keystore which stores the passwords.

Elasticsearch's keystore initial md5sum was added in #28928 with the intention to allow us to remove the elasticsearch.keystore file upon package removal, if this hadn't been altered after installation. At that time this decision made perfect sense as the elasticsearch keystore only contains transient data by default ( keystore.seed ) that is meant to be useful for bootstrap related actions, and doesn't need to survive re-installations. With Security ON by default, we will be storing additional settings in the keystore upon installation(namely, the passwords for the PKCS#12 keystores used for TLS) and these have a more persistent nature. Since `remove` doesn't delete the configuration directories and files where said PKCS#12 keystores are stored, it makes sense to also not delete the elasticsearch.keystore which stores the passwords.

jasontedor added review :Delivery/Packaging RPM and deb packaging, tar and zip archives, shell and batch scripts v7.0.0 v6.3.0 labels Mar 7, 2018

jasontedor requested a review from rjernst March 7, 2018 19:53

rjernst approved these changes Mar 7, 2018

View reviewed changes

jasontedor added 2 commits March 7, 2018 17:08

Disable BWC tests

50c2a09

Rename and hide md5 file

336998e

rjernst approved these changes Mar 7, 2018

View reviewed changes

Reenable BWC tests

97058a7

Merge branch 'master' into package-create-keystore

617fb35

* master: [TEST] AwaitsFix QueryRescorerIT.testRescoreAfterCollapse Decouple XContentType from StreamInput/Output (elastic#28927) Remove BytesRef usage from XContentParser and its subclasses (elastic#28792)

More removal of requires keystore

d2f8fa9

jasontedor force-pushed the package-create-keystore branch from df95dcf to d2f8fa9 Compare March 8, 2018 10:43

jasontedor added the >breaking label Mar 8, 2018

jasontedor merged commit 6331bca into elastic:master Mar 12, 2018

jasontedor mentioned this pull request Mar 17, 2018

Fix creating keystore when upgrading #29121

Merged

jasontedor mentioned this pull request Apr 23, 2018

Abort package install if JAVA_HOME is not set and java is not in the path #29665

Closed

roock mentioned this pull request Aug 13, 2018

Elsticsearch 6.3 Upgrade Broken (in Debian based Systems) voxpupuli/puppet-elasticsearch#977

Open

colings86 added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019

williamrandolph mentioned this pull request Jan 30, 2020

RPM update to 7.5.1 prevents start of elasticsearch #50631

Closed

mark-vieira added the Team:Delivery Meta label for Delivery team label Nov 11, 2020

jkakavas mentioned this pull request Aug 23, 2021

Remove keystore initial_md5sum #76835

Merged

Conversation

jasontedor commented Mar 7, 2018

Uh oh!

jasontedor commented Mar 7, 2018

Uh oh!

jasontedor commented Mar 7, 2018

Uh oh!

rjernst left a comment

Choose a reason for hiding this comment

Uh oh!

rjernst Mar 7, 2018

Choose a reason for hiding this comment

Uh oh!

rjernst Mar 7, 2018

Choose a reason for hiding this comment

Uh oh!

jasontedor Mar 7, 2018

Choose a reason for hiding this comment

Uh oh!

jasontedor commented Mar 7, 2018

Uh oh!

rjernst left a comment

Choose a reason for hiding this comment

Uh oh!

rjernst Mar 7, 2018

Choose a reason for hiding this comment

Uh oh!

jasontedor Mar 7, 2018

Choose a reason for hiding this comment

Uh oh!

jasontedor commented Mar 7, 2018

Uh oh!

jasontedor commented Mar 7, 2018

Uh oh!

jasontedor commented Mar 7, 2018

Uh oh!

jasontedor commented Mar 8, 2018

Uh oh!

jasontedor commented Mar 8, 2018

Uh oh!

jasontedor commented Mar 12, 2018

Uh oh!

lcawl commented Jun 15, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants