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

Elasticsearch: Ensure Elasticsearch 8 works OOTB secure as default #5099

Conversation

spinscale
Copy link
Contributor

Since Elasticsearch 8.0 the default is to enable security, meaning TLS
and authentication.

This adds a check for Elasticsearch 8.0 to change the default behaviour
to properly support this change, but you can still run Elasticsearch
with security features disabled, if you want.

Closes #5048

Several things to discuss here that I am happy to change:

First I tried not to change behaviour for releases before 8.0 - neither when checking if Elasticsearch is ready, nor when changing options. This is merely to not break any existing applications and tests when upgrading.

Also, generating the SSLContext could something to be done outside of the application but I consider this a super common thing to do once Elasticsearch starts up as you need to have a custom SSL context due to the self signed certificate.

Happy to get any feedback - also if you don't think this is a viable thing to do 😀

Since Elasticsearch 8.0 the default is to enable security, meaning TLS
and authentication.

This adds a check for Elasticsearch 8.0 to change the default behaviour
to properly support this change, but you can still run Elasticsearch
with security features disabled, if you want.
Copy link
Member

@kiview kiview left a comment

Choose a reason for hiding this comment

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

Thanks for providing this PR 🙂

I was also thinking about the topics you mentioned in your description and commented accordingly in the review.

@dadoonet do you have an opinion on which wait strategy to use for each Elasticsearch version? Would we be fine to depend on LogMessageWaitStrategy across all supported versions? This would simplify a bit of the logic and we would not need to differentiate between versions here.

The question of whether we need createSslContextFromCa as part of the API or just in the docs is also a very valid one. It seems substantially easier for users, as we see from your tests, so I am inclined to add it to the API. WDYT @rnorth @bsideup?

@dadoonet
Copy link
Contributor

@dadoonet do you have an opinion on which wait strategy to use for each Elasticsearch version? Would we be fine to depend on LogMessageWaitStrategy across all supported versions? This would simplify a bit of the logic and we would not need to differentiate between versions here.

I would prefer that we have a new HttpsWaitStrategy(SSLContext - or something) class which can allow an option to bypass the certificate check (if the certificate is not provided for example).
I'm not expert in the security part so I don't know if this makes sense ;)

I agree that we could depend on LogMessageWaitStrategy only but one of the problem I can see is that the log format has changed in 8.0 as it's using json now I think where it was using plain text in the previous versions. Not a big deal though...

@bsideup bsideup added this to the next milestone Apr 6, 2022
Copy link
Member

@kiview kiview left a comment

Choose a reason for hiding this comment

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

I think it's great that we switched to LogMessageWaitStrategy now and createSslContextFromCa is also very convenient for users, thanks!

…search/ElasticsearchContainer.java

Co-authored-by: Kevin Wittek <kiview@users.noreply.github.com>
@kiview
Copy link
Member

kiview commented Apr 7, 2022

The Netlify failures can be ignored, they originate from:
#5186

Copy link
Member

@bsideup bsideup left a comment

Choose a reason for hiding this comment

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

🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Supporting Elasticsearch 8.0 and its secure by default mode
4 participants