Skip to content

Enroll additional nodes to cluster #77292

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

Merged
merged 15 commits into from
Sep 7, 2021

Conversation

jkakavas
Copy link
Member

@jkakavas jkakavas commented Sep 5, 2021

This introduces a new CLI tool elasticsearch-enroll-node.
It takes an Enrollment Token as a parameter and using the
information in it, it attempts to

  • Communicate with an existing node of the cluster
  • Receive necessary key/certificate material
  • Persist said material and configuration

This tool needs to be run before the first time the current
node starts and if it doesn't have any explicit security
related configuration already defined.

This introduces a new named argument (--enrollment-token) that can
be passed to the elasticsearch executables when starting a node.
It takes an Enrollment Token as a value and using the information
in it, it Elasticsearch will attempt to

- Communicate with an existing node of the cluster
- Receive necessary key/certificate material
- Persist said material and configuration

before the node is actually started. This can only happen if this
is the first time the current node starts and if it doesn't have
any explicit security related configuration defined.
@jkakavas jkakavas added >enhancement :Delivery/Packaging RPM and deb packaging, tar and zip archives, shell and batch scripts :Security/Security Security issues without another label v8.0.0 :Core/Infra/CLI CLI utilities, scripts, and infrastructure labels Sep 5, 2021
@elasticmachine elasticmachine added Team:Delivery Meta label for Delivery team Team:Security Meta label for security team Team:Core/Infra Meta label for core/infra team labels Sep 5, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-delivery (Team:Delivery)

@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (Team:Core/Infra)

@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-security (Team:Security)

@elasticsearchmachine
Copy link
Collaborator

Hi @jkakavas, I've created a changelog YAML for you.

@@ -4,9 +4,11 @@
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/
package org.elasticsearch.xpack.security.tool;
package org.elasticsearch.xpack.core.security;
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 moved CommandLineHttpClient, HttpResponse and EnrollmentToken to core so that I can use them in EnrollNodeToCluster without needing to add a dependency to the security plugin in security:cli. The issue with that would be that I'd have a dependency conflict for Guava (30-1 as a jimfs dependency in security:cli vs 19 in security) I couldn't think of a way to solve this dependency issue, but more than happy to get suggestions and I'll move this back to the security plugin

/**
* Configures a node to join an existing cluster with security features enabled.
*/
public class EnrollNodeToCluster extends KeyStoreAwareCommand {
Copy link
Member Author

Choose a reason for hiding this comment

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

This class shares a lot of code with ConfigInitialNode from #77291. As we were working on these two PRs simultaneously is was impractical to stay in sync and deal with all merge conflicts. Additionally, the error handling is different in certain cases. When both this and #77291 are merged, there will be a follow-up PR ASAP to move anything we can in a base class and remove code duplication

@@ -45,6 +49,14 @@ then
fi
fi

if [[ $ENROLL_TO_CLUSTER = true ]]; then
Copy link
Member Author

Choose a reason for hiding this comment

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

The idea here is that this part runs only when an explicit flag is passed by the user. We will attempt to do our auto-configuration of TLS based on the enrollment process and only if everything went well will the node start with that configuration. If anything fails, we roll-back the configuration and fail with an error message.

@jkakavas
Copy link
Member Author

jkakavas commented Sep 7, 2021

I know we only intend to support our tooling here, so this isn't really an issue, but I think I would prefer to pin to chain[1] rather than chain[length-1]
That is, pin to the issuer, not the root CA.

However, that then fails for self-signed certs, so there's no perfect solution that's also concise.

Both work for me as it doesn't matter for all current intents and purposes. I'll change to chain[1] and revisit if/when we want to handle a more generic enrollment use case

@jkakavas
Copy link
Member Author

jkakavas commented Sep 7, 2021

@rjernst , @pugnascotia , given that we are not merging #77231 now, it wouldn't make much sense to introduce this functionality as a parameter to the elasticsearch startup script. So I changed this to be invoked only via a CLI now to allow for testing. I am more than interested and I'd welcome a review but I plan on merging this and we can have a longer discussion around it too when we wire it to the startup script.

Copy link
Contributor

@BigPandaToo BigPandaToo left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@pugnascotia pugnascotia left a comment

Choose a reason for hiding this comment

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

Script revisions look good. Didn't look at the Java changes.

@jkakavas jkakavas merged commit 1399fb6 into elastic:master Sep 7, 2021
jkakavas added a commit to jkakavas/elasticsearch that referenced this pull request Sep 14, 2021
The functionality to enroll a new node to a cluster was
introduced in elastic#77292 as a CLI tool. This change brings forward two
additions:

- It allows to trigger the enrollment functionality on startup of
elasticsearch via a named argument that can be passed to the es
startup script (--enrollment-token) so that the users that want to
do this do not need to run two commands instead of one.
- It adds a `-f, --force` flag to the standalone CLI version of
this which bypasses the checks for whether there are data in the
current node or whether security related features are already
configured. The primary use case for this flag is enrolling new
nodes in packaged installations. There we auto-configure each
installation as a first node of a cluster by default because we
don't have a user-friendly and generic way to allow the user
decide on installation time. Thus, we want to allow users to
easily reconfigure the node to enroll to an existing cluster
without requiring many manual steps.
jkakavas added a commit that referenced this pull request Oct 27, 2021
The functionality to enroll a new node to a cluster was
introduced in #77292 as a CLI tool. This change replaces this
CLI tool with the option to trigger the enrollment functionality 
on startup of elasticsearch via a named argument that can be 
passed to the elasticsearch startup script (--enrollment-token)
so that the users that want to enroll a node to a cluster can do 
this with one command instead of two. 

In a followup PR we are introducing a CLI tool version of this
functionality, that can be used to reconfigure packaged
installations.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Core/Infra/CLI CLI utilities, scripts, and infrastructure :Delivery/Packaging RPM and deb packaging, tar and zip archives, shell and batch scripts >enhancement :Security/Security Security issues without another label Team:Core/Infra Meta label for core/infra team Team:Delivery Meta label for Delivery team Team:Security Meta label for security team v8.0.0-alpha2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants