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.

Sign up for GitHub

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

Jump to bottom

Enable lazy certificates for Elasticsearch #7991

Merged
merged 10 commits into from
Mar 4, 2024
Merged

Enable lazy certificates for Elasticsearch #7991

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
a56cd9a
Added note about node.js being required for ./gradlew spotlessApply
pioorg Dec 21, 2023
3dc195a
Merge branch 'testcontainers:main' into main
pioorg Jan 9, 2024
8f7f56c
Make fetching CA cert bytes in ElasticsearchContainer a lazy operation.
pioorg Dec 21, 2023
f777482
Make fetching CA cert bytes in ElasticsearchContainer a lazy operation.
pioorg Dec 21, 2023
e1d7393
Removed containerIsStarted from ElasticsearchContainer
pioorg Jan 25, 2024
40c56ea
Moved setting certificate path to the constructor only for versions w…
pioorg Feb 8, 2024
4bd207f
Merge branch 'main' into elasticsearch-lazy-certificates
pioorg Feb 12, 2024
af25467
Merge branch 'main' into elasticsearch-lazy-certificates
eddumelendez Mar 4, 2024
4cd2802
Fix format
eddumelendez Mar 4, 2024
4227f09
Apply suggestions from code review
eddumelendez Mar 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 29 additions & 23 deletions .../elasticsearch/src/main/java/org/testcontainers/elasticsearch/ElasticsearchContainer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,6 @@ public class ElasticsearchContainer extends GenericContainer<ElasticsearchContai

private final boolean isAtLeastMajorVersion8;

private Optional<byte[]> caCertAsBytes = Optional.empty();

private String certPath = "/usr/share/elasticsearch/config/certs/http_ca.crt";

/**
Expand All @@ -91,6 +89,7 @@ public ElasticsearchContainer() {

/**
* Create an Elasticsearch Container by passing the full docker image name
*
* @param dockerImageName Full docker image name as a {@link String}, like: docker.elastic.co/elasticsearch/elasticsearch:7.9.2
*/
public ElasticsearchContainer(String dockerImageName) {
Expand All @@ -99,6 +98,7 @@ public ElasticsearchContainer(String dockerImageName) {

/**
* Create an Elasticsearch Container by passing the full docker image name
*
* @param dockerImageName Full docker image name as a {@link DockerImageName}, like: DockerImageName.parse("docker.elastic.co/elasticsearch/elasticsearch:7.9.2")
*/
public ElasticsearchContainer(final DockerImageName dockerImageName) {
Expand Down Expand Up @@ -140,39 +140,44 @@ public ElasticsearchContainer(final DockerImageName dockerImageName) {
}

@Override
protected void containerIsStarted(InspectContainerResponse containerInfo) {
if (isAtLeastMajorVersion8 && StringUtils.isNotEmpty(certPath)) {
try {
byte[] bytes = copyFileFromContainer(certPath, IOUtils::toByteArray);
if (bytes.length > 0) {
this.caCertAsBytes = Optional.of(bytes);
}
} catch (NotFoundException e) {
// just emit an error message, but do not throw an exception
// this might be ok, if the docker image is accidentally looking like version 8 or latest
// can happen if Elasticsearch is repackaged, i.e. with custom plugins
log.warn("CA cert under " + certPath + " not found.");
}
}
}
protected void containerIsStarted(InspectContainerResponse containerInfo) {}
eddumelendez marked this conversation as resolved.
Show resolved Hide resolved

/**
* If this is running above Elasticsearch 8, this will return the probably self-signed CA cert that has been extracted
*
* @return byte array optional containing the CA cert extracted from the docker container
*/
public Optional<byte[]> caCertAsBytes() {
return caCertAsBytes;
if (StringUtils.isBlank(certPath)) {
return Optional.empty();
}
Comment on lines +151 to +153
Copy link
Member

Choose a reason for hiding this comment

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

I think this will never be executed because certPath has a default value.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmmm... What if one calls withCertPath("") when creating the container?

Copy link
Member

Choose a reason for hiding this comment

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

Do you see any real use case when using ElasticsearchContainer and use withCertPath("")? If so, better to add a test to cover the scenario and also serve as a documentation. WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you very much for this comment @eddumelendez, it made me think ;-)
And as the result of this process I suggest yet another small change, basically the constructor of the ElasticsearchContainer should set the certPath by default only for versions, which do that by default, that is 8.
Previously it set it also for 7, which (after consideration) isn't optimal IMHO. I think, if version 7 requires manual opt-in to enable certificates, then this should be reflected in calling withCertPath(thePath). No opt in -> no path for 7.
Similarly for version 8, which has the certs enabled by default. If the user decides to disable them, ideally they should also state this intent by calling withCertPath("").

try {
byte[] bytes = copyFileFromContainer(certPath, IOUtils::toByteArray);
if (bytes.length > 0) {
return Optional.of(bytes);
}
} catch (NotFoundException e) {
// just emit an error message, but do not throw an exception
// this might be ok, if the docker image is accidentally looking like version 8 or latest
// can happen if Elasticsearch is repackaged, i.e. with custom plugins
log.warn("CA cert under " + certPath + " not found.");
}
return Optional.empty();
}

/**
* A SSL context based on the self signed CA, so that using this SSL Context allows to connect to the Elasticsearch service
* A SSL context based on the self-signed CA, so that using this SSL Context allows to connect to the Elasticsearch service
* @return a customized SSL Context
*/
public SSLContext createSslContextFromCa() {
try {
CertificateFactory factory = CertificateFactory.getInstance("X.509");
Certificate trustedCa = factory.generateCertificate(new ByteArrayInputStream(caCertAsBytes.get()));
Certificate trustedCa = factory.generateCertificate(
new ByteArrayInputStream(
caCertAsBytes()
.orElseThrow(() -> new IllegalStateException("CA cert under " + certPath + " not found."))
)
);
KeyStore trustStore = KeyStore.getInstance("pkcs12");
trustStore.load(null, null);
trustStore.setCertificateEntry("ca", trustedCa);
Expand All @@ -190,13 +195,13 @@ public SSLContext createSslContextFromCa() {
/**
* Define the Elasticsearch password to set. It enables security behind the scene for major version below 8.0.0.
* It's not possible to use security with the oss image.
* @param password Password to set
* @param password Password to set
* @return this
*/
public ElasticsearchContainer withPassword(String password) {
if (isOss) {
throw new IllegalArgumentException(
"You can not activate security on Elastic OSS Image. " + "Please switch to the default distribution"
"You can not activate security on Elastic OSS Image. Please switch to the default distribution"
);
}
withEnv("ELASTIC_PASSWORD", password);
Expand All @@ -222,7 +227,8 @@ public String getHttpHostAddress() {
return getHost() + ":" + getMappedPort(ELASTICSEARCH_DEFAULT_PORT);
}

@Deprecated // The TransportClient will be removed in Elasticsearch 8. No need to expose this port anymore in the future.
@Deprecated
// The TransportClient will be removed in Elasticsearch 8. No need to expose this port anymore in the future.
eddumelendez marked this conversation as resolved.
Show resolved Hide resolved
public InetSocketAddress getTcpHost() {
return new InetSocketAddress(getHost(), getMappedPort(ELASTICSEARCH_DEFAULT_TCP_PORT));
}
Expand Down
44 changes: 44 additions & 0 deletions ...sticsearch/src/test/java/org/testcontainers/elasticsearch/ElasticsearchContainerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import org.testcontainers.containers.wait.strategy.HttpWaitStrategy;
import org.testcontainers.containers.wait.strategy.Wait;
import org.testcontainers.images.RemoteDockerImage;
import org.testcontainers.images.builder.Transferable;
import org.testcontainers.utility.DockerImageName;
import org.testcontainers.utility.MountableFile;

Expand Down Expand Up @@ -375,6 +376,49 @@ public void testElasticsearch8SecureByDefaultFailsSilentlyOnLatestImages() throw
}
}

@Test
public void testElasticsearch7CanHaveSecurityEnabledAndUseSslContext() throws Exception {
try (
ElasticsearchContainer container = new ElasticsearchContainer(
"docker.elastic.co/elasticsearch/elasticsearch:7.17.15"
)
.withPassword(ElasticsearchContainer.ELASTICSEARCH_DEFAULT_PASSWORD)
.withEnv("xpack.security.enabled", "true")
.withEnv("xpack.security.http.ssl.enabled", "true")
.withEnv("xpack.security.http.ssl.key", "/usr/share/elasticsearch/config/certs/elasticsearch.key")
.withEnv(
"xpack.security.http.ssl.certificate",
"/usr/share/elasticsearch/config/certs/elasticsearch.crt"
)
.withEnv(
"xpack.security.http.ssl.certificate_authorities",
"/usr/share/elasticsearch/config/certs/http_ca.crt"
)
// these lines show how certificates can be created self-made way
// obviously this shouldn't be done in prod environment, where proper and officially signed keys should be present
.withCopyToContainer(
Transferable.of(
"#!/bin/bash\n" +
"mkdir -p /usr/share/elasticsearch/config/certs;" +
"openssl req -x509 -newkey rsa:4096 -keyout /usr/share/elasticsearch/config/certs/elasticsearch.key -out /usr/share/elasticsearch/config/certs/elasticsearch.crt -days 365 -nodes -subj \"/CN=localhost\";" +
"openssl x509 -outform der -in /usr/share/elasticsearch/config/certs/elasticsearch.crt -out /usr/share/elasticsearch/config/certs/http_ca.crt;" +
"chown -R elasticsearch /usr/share/elasticsearch/config/certs/",
555
),
"/usr/share/elasticsearch/generate-certs.sh"
)
// because we need to generate the certificates before Elasticsearch starts, the entry command has to be tuned accordingly
.withCommand(
"sh",
"-c",
"/usr/share/elasticsearch/generate-certs.sh && /usr/local/bin/docker-entrypoint.sh"
);
) {
container.start();
assertClusterHealthResponse(container);
}
}

@Test
public void testElasticsearchDefaultMaxHeapSize() throws Exception {
long defaultHeapSize = 2147483648L;
Expand Down
Loading