-
Notifications
You must be signed in to change notification settings - Fork 25k
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
Allow node to enroll to cluster on startup #77718
Merged
Merged
Changes from 2 commits
Commits
Show all changes
68 commits
Select commit
Hold shift + click to select a range
aeec13d
Allow node to enroll to cluster on startup
jkakavas 4c17742
Merge remote-tracking branch 'origin/master' into enroll-as-a-param
jkakavas f9a14b8
Add packaging tests(first of many commits)
jkakavas 226e965
supress forbidden warning for necessary use
jkakavas a15b6bc
close if not null
jkakavas 99afe27
proper settings
jkakavas ca0836c
Shift only when there are more args
jkakavas 068e45d
temp
jkakavas be92ca1
correct mock esNode ssl config to return the entire chain
jkakavas ba11466
Merge remote-tracking branch 'origin/master' into enroll-as-a-param
jkakavas 5c0c1f0
fixes
jkakavas 062cb85
more fixes
jkakavas 2f3c337
final fixes
jkakavas 74ef40b
(mostly) fix batch startup script
jkakavas 140d2c4
Merge remote-tracking branch 'origin/master' into enroll-as-a-param
jkakavas fb8686c
update batch file
jkakavas b480b4e
feedback
jkakavas 7941370
adjust test
jkakavas 8ca65fa
Call CLI tool from batch script
jkakavas 2d8b395
Merge remote-tracking branch 'origin/master' into enroll-as-a-param
jkakavas f631474
change how we populate SANs
jkakavas 1efbc06
Merge remote-tracking branch 'origin/master' into enroll-as-a-param
jkakavas 0c1fee9
non null hostnames
jkakavas 345329e
Merge remote-tracking branch 'origin/master' into enroll-as-a-param
jkakavas 36fc590
Scope down the PR to just allow users to pass the enrollment-token on…
jkakavas fd14936
cleanup qa
jkakavas 6dbb3fa
remove redundant testing
jkakavas 5dbfba3
Revert "remove redundant testing"
jkakavas 2daabc5
remove redundant test - correctly this time
jkakavas caa5880
reintroduce a simpler version of packaging tests
jkakavas d6018d3
Merge remote-tracking branch 'origin/master' into enroll-as-a-param
jkakavas 14b679a
windows tests
jkakavas 881d001
use EnrollmentToken class in tests
jkakavas 1758026
spotless
jkakavas 026341d
spotless
jkakavas c6f123e
fix token param checking
jkakavas bfbfee0
Merge remote-tracking branch 'origin/master' into enroll-as-a-param
jkakavas 4222f7c
Fix test
jkakavas a3cae59
Adjust startup scripts
jkakavas 7331c71
cleanup
jkakavas 2d04043
mute test on windows
jkakavas aebba35
Merge remote-tracking branch 'origin/master' into enroll-as-a-param
jkakavas 3643d72
mute on windows
jkakavas 4198cc8
change test while figuring the cause
jkakavas 573add3
Fix argument handling
jkakavas 38623c6
Merge remote-tracking branch 'origin/master' into enroll-as-a-param
jkakavas 0ced955
remove code that can wait until the follow-up PR for the CLI tool
jkakavas 9efae09
Merge remote-tracking branch 'origin/master' into enroll-as-a-param
jkakavas 48b7569
Use enrollment token on startup in tests
jkakavas d0e9e6c
add failure information output in verbose mode
jkakavas a3c9a21
a space is not a -
jkakavas 18ed565
account for the fact that the second node binds to 9201 in Enrollment…
jkakavas f09cba0
test
jkakavas 347e4b9
Ugly hacks are ugly
jkakavas d8729ce
tightly scope try blocks
jkakavas 7693119
address feedback
jkakavas 1138134
dissallow multiple --enrollment-token paramaters
jkakavas ee03466
Generate a CA and sign transport certificates with this
jkakavas 0ca60d1
Merge remote-tracking branch 'origin/master' into enroll-as-a-param
jkakavas e00aa95
Transport CA cert needs to be transmitted to all nodes to be used in …
jkakavas 491dbf5
disallow multiple --enrollment-token params on windows too
jkakavas d7ba414
print to stderr on windows
jkakavas 5859676
windows exit code handling is totally bonkers
jkakavas 10fec73
stderr
jkakavas 631d22c
fix test
jkakavas b635b96
fix test
jkakavas eaeafe8
Merge remote-tracking branch 'origin/master' into enroll-as-a-param
jkakavas 5b796df
merge woes
jkakavas File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,15 +17,20 @@ source "`dirname "$0"`"/elasticsearch-env | |
|
||
CHECK_KEYSTORE=true | ||
DAEMONIZE=false | ||
for option in "$@"; do | ||
case "$option" in | ||
-h|--help|-V|--version) | ||
CHECK_KEYSTORE=false | ||
;; | ||
-d|--daemonize) | ||
DAEMONIZE=true | ||
;; | ||
esac | ||
ENROLL_TO_CLUSTER=false | ||
# Store original arg array as we will be shifting through it below | ||
ARG_LIST=($@) | ||
while [ $# -gt 0 ]; do | ||
if [[ $1 == "--enrollment-token" ]]; then | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Personally, I would have stuck with a |
||
ENROLL_TO_CLUSTER=true | ||
ENROLLMENT_TOKEN="$2" | ||
jkakavas marked this conversation as resolved.
Show resolved
Hide resolved
|
||
shift | ||
elif [[ $1 == "-h" || $1 == "--help" || $1 == "-V" || $1 == "--version" ]]; then | ||
CHECK_KEYSTORE=false | ||
elif [[ $1 == "-d" || $1 == "--daemonize" ]]; then | ||
DAEMONIZE=true | ||
fi | ||
shift | ||
done | ||
|
||
if [ -z "$ES_TMPDIR" ]; then | ||
|
@@ -45,6 +50,14 @@ then | |
fi | ||
fi | ||
|
||
if [[ $ENROLL_TO_CLUSTER = true ]]; then | ||
ES_MAIN_CLASS=org.elasticsearch.xpack.security.cli.EnrollNodeToCluster \ | ||
jkakavas marked this conversation as resolved.
Show resolved
Hide resolved
|
||
ES_ADDITIONAL_SOURCES="x-pack-env;x-pack-security-env" \ | ||
ES_ADDITIONAL_CLASSPATH_DIRECTORIES=lib/tools/security-cli \ | ||
"`dirname "$0"`"/elasticsearch-cli \ | ||
mark-vieira marked this conversation as resolved.
Show resolved
Hide resolved
|
||
--enrollment-token "$ENROLLMENT_TOKEN" <<<"$KEYSTORE_PASSWORD" | ||
fi | ||
|
||
# The JVM options parser produces the final JVM options to start Elasticsearch. | ||
# It does this by incorporating JVM options in the following way: | ||
# - first, system JVM options are applied (these are hardcoded options in the | ||
|
@@ -67,7 +80,7 @@ if [[ $DAEMONIZE = false ]]; then | |
-Des.bundled_jdk="$ES_BUNDLED_JDK" \ | ||
-cp "$ES_CLASSPATH" \ | ||
org.elasticsearch.bootstrap.Elasticsearch \ | ||
"$@" <<<"$KEYSTORE_PASSWORD" | ||
"${ARG_LIST[@]}" <<<"$KEYSTORE_PASSWORD" | ||
else | ||
exec \ | ||
"$JAVA" \ | ||
|
@@ -80,7 +93,7 @@ else | |
-Des.bundled_jdk="$ES_BUNDLED_JDK" \ | ||
-cp "$ES_CLASSPATH" \ | ||
org.elasticsearch.bootstrap.Elasticsearch \ | ||
"$@" \ | ||
"${ARG_LIST[@]}" \ | ||
<<<"$KEYSTORE_PASSWORD" & | ||
retval=$? | ||
pid=$! | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed the way we are iterating over the args, so that we can capture the value of the enrollment token. We need this so that we can remove it from the args array that we end up passing to elasticserach process to start in the end, as elasticsearch will not allow for unrecognized options.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm curious about why you iterate through
$@
here (effectively), rather than$ARG_LIST
.If you just iterated through ARG_LIST you could remove the enrollment token in a single pass rather than needing to iterate again later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm using SHIFT in this iteration so that I can recognize the named arg that comes after
--enrollment-token
. If Iwas to iterate over ARG_LIST , I'd be removing args from it while iterating so I wouldn't be able to use it below.Or, I am missing your point, let me know :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not important, but you could do something like this: