Bugfix: create the tmp dir if it doesn't exist

[jan-elastic]

See:

• [ML] Modify the Java temp directory every day on every node #99349
• https://github.com/elastic/sdh-ml/issues/498

[elasticsearchmachine]

Pinging @elastic/ml-core (Team:ML)

[elasticsearchmachine]

Hi @jan-elastic, I've created a changelog YAML for you.

[jan-elastic]

This simple fix resolves the problem locally.

I don't know if my local environment is close enough to production w.r.t. security, permissions, etc, to judge whether this will also work in production.

[jan-elastic]

Update: this fix also works in a Dockerized Ubuntu 20.04.

The following flow succeeds:

• start Elasticsearch
• open job
• close job
• remove tmp dir
• open job

The deleted tmp dir is successfully recreated.

[droberts195]

Great, thanks for testing that. The final thing to confirm is the permissions.

[jan-elastic]

Note: without this fix you get something like

java.nio.file.NoSuchFileException: /tmp/elasticsearch-14675733582508699252/config6396271037101692016.json

[droberts195]

There's one more place that needs changing, in AnalyticsBuilder.java here:

elasticsearch/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/dataframe/process/AnalyticsBuilder.java

Line 83 in 32dbb0b

Path configFile = Files.createTempFile(tempDir, "analysis", ".conf");

The place where the temp directory gets created at server startup is here:

elasticsearch/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/ServerProcess.java

Line 268 in 9caee77

return Files.createTempDirectory(prefix, attrs);

Because at that time it's created using Files.createTempDirectory with no attributes provided it defaults to the following (at least in Java 20):

        static final FileAttribute<Set<PosixFilePermission>> dirPermissions =
            PosixFilePermissions.asFileAttribute(EnumSet
                .of(OWNER_READ, OWNER_WRITE, OWNER_EXECUTE));

This is what we want for a secure temp directory.

Please check that Files.createDirectories uses those restricted permissions. I have a feeling that it might allow group and other access too if attributes are not supplied. If it does then it will be necessary to explicitly supply attributes. But then there'll be the complication that Windows needs different permission attributes.

At startup we already do something different on Windows here:

elasticsearch/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/ServerProcess.java

Line 258 in 9caee77

Files.createDirectories(path);

So it looks like on Windows we can match the startup behaviour by not supplying any attributes.

In the ML code you can use if (Constants.WINDOWS) to check for Windows, for example like here:

elasticsearch/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/utils/NamedPipeHelper.java

Line 171 in b7eafce

if (Constants.WINDOWS) {

So please wrap all this up into a recreateTempDirectory utility method that can be called from all the places where it's needed. We have a org.elasticsearch.xpack.ml.utils package where it could go. And you can add a unit test for the utility method that asserts that the permissions are 700 if the test is not running on Windows.

[jan-elastic]

You're correct: my new directory was created with permissions drwxr-xr-x.

That's fixed now.

[davidkyle]

There are 2 situations where the code should ensure the temp directory exists. One is for anomaly detection and DFA jobs where they write config files to the temp directory which is covered here. The other is where the code creates named pipes to talk to those processes. This happens in the ProcessPipes class and that code is used to connect to all the native processes that we deploy.

https://github.com/elastic/elasticsearch/blob/main/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/process/ProcessPipes.java#L101

Before those pipes are created add the same call to FileUtils.recreateTempDirectoryIfNeeded(env.tmpFile());

This can be done in the constructor or in the ProcessPipes::connectLogStream and ProcessPipes::connectOtherStreams methods

This doesn't apply to Windows where named pipes are created differently:
https://github.com/elastic/elasticsearch/blob/main/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/utils/NamedPipeHelper.java#L73

[jan-elastic]

Added recreating the tmp dir in ProcessPipes.

I didn't bother to make an exception for Windows: I think recreating the temp dir is a noop in Windows (Windows doesn't clean up), and otherwise it's pretty cheap anyway.

[davidkyle]

LGTM

[elasticsearchmachine]

💔 Backport failed

You can use sqren/backport to manually backport by running backport --upstream elastic/elasticsearch --pr 102599