Elasticsearch: Add withPassword(String) method for secure access #2321
Conversation
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you believe this is a mistake, please reply to this comment to keep it open. If there isn't one already, a PR to fix or at least reproduce the problem in a test case will always help us get back on track to tackle this. |
|
Not stale |
dadoonet
force-pushed
the
elasticsearch-secure
branch
from
d987545
to
877aa4f
Compare
| public class ElasticsearchContainerTest { | ||
|
|
||
| /** | ||
| * Elasticsearch version which should be used for the Tests | ||
| */ | ||
| private static final String ELASTICSEARCH_VERSION = Version.CURRENT.toString(); | ||
| private static final String ELASTICSEARCH_VERSION = "7.8.0"; |
There was a problem hiding this comment.
Ideally we should get this from the Gradle definition. Is that possible?
There was a problem hiding this comment.
Depending on the module, it's not always the case that there's a 1:1 mapping between driver and image versions, but if there is for Elasticsearch that's quite nice. It could help keep our tests up to date.
We do this for Selenium, for which there's historically been a very tight coupling between driver version and required image version.
For the Selenium module, the code that does it is here:
If you wanted to use this for inspiration (maybe change SeleniumUtils to a general-purpose class inside our test-support module), perhaps it could work. We'd only want to use this for Testcontainers' internal tests, though - i.e. deprecation of the default-args constructor still applies.
There was a problem hiding this comment.
Right. I think that this should come within another PR so we don't overcomplicate things.
BTW I realized lately that I should have commented in #2320 and not here :(
dadoonet
force-pushed
the
elasticsearch-secure
branch
from
8d3bbf3
to
a6c00c5
Compare
dadoonet
force-pushed
the
elasticsearch-secure
branch
4 times, most recently
from
3e17a3e
to
e681a97
Compare
|
#2320 is merged, so this can be rebased now 🙂 |
Instead of providing env settings manually, we can simplify the usage of Elasticsearch in the context of TestContainers by just asking a password. Behind the scene, we do provide the needed env settings. We also check that we can not define the password on OSS image.
dadoonet
force-pushed
the
elasticsearch-secure
branch
from
e681a97
to
23878f9
Compare
|
I added a missing test. I think it's now ready for review. :) |
rnorth
changed the title
|
Does this need another review to be merged? |
|
@dadoonet I was just waiting to clear an issue with our master branch Windows CI executor. Should be good to go now. Thank you! |
Instead of providing env settings manually, we can simplify the usage of Elasticsearch in the context of TestContainers by just asking a password.
Behind the scene, we do provide the needed env settings.
This PR comes after #2320 and should be probably rebased on master when 2320 will be merged.