[Entitlements] Fix PolicyUtils and PolicyUtilsTests on Windows (#126185)

This PR fixes 2 issues discovered around PolicyUtils (and PolicyUtilsTests) when running CI on Windows:
- in tests, absolute paths like always are different, this fix updates the tests to account for the difference.
- on Windows, Files.move was failing because we were parsing the Entitlement policy but not closing the stream in a timely manner. This causes plugin installation (and related CI tests) to fail 70% of the time. Fixed by closing the stream properly

Fixes #126176
This commit is contained in:
Lorenzo Dematté 2025-04-03 19:02:17 +02:00 committed by GitHub
parent 279498d810
commit e4ce993c16
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 22 additions and 9 deletions

View file

@ -141,7 +141,9 @@ public class PolicyUtils {
public static Policy parsePolicyIfExists(String pluginName, Path pluginRoot, boolean isExternalPlugin) throws IOException {
Path policyFile = pluginRoot.resolve(POLICY_FILE_NAME);
if (Files.exists(policyFile)) {
return new PolicyParser(Files.newInputStream(policyFile, StandardOpenOption.READ), pluginName, isExternalPlugin).parsePolicy();
try (var inputStream = Files.newInputStream(policyFile, StandardOpenOption.READ)) {
return new PolicyParser(inputStream, pluginName, isExternalPlugin).parsePolicy();
}
}
return new Policy(pluginName, List.of());
}

View file

@ -16,6 +16,7 @@ import org.elasticsearch.entitlement.runtime.policy.PathLookup;
import org.elasticsearch.entitlement.runtime.policy.Platform;
import org.elasticsearch.entitlement.runtime.policy.PolicyValidationException;
import java.nio.file.FileSystems;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.HashMap;
@ -31,6 +32,8 @@ import java.util.stream.Stream;
*/
public record FilesEntitlement(List<FileData> filesData) implements Entitlement {
public static final String SEPARATOR = FileSystems.getDefault().getSeparator();
public static final FilesEntitlement EMPTY = new FilesEntitlement(List.of());
public enum Mode {
@ -160,7 +163,7 @@ public record FilesEntitlement(List<FileData> filesData) implements Entitlement
@Override
public String description() {
return Strings.format("[%s] <%s>/%s%s", mode, baseDir, relativePath, exclusive ? " (exclusive)" : "");
return Strings.format("[%s] <%s>%s%s%s", mode, baseDir, SEPARATOR, relativePath, exclusive ? " (exclusive)" : "");
}
}
@ -192,7 +195,7 @@ public record FilesEntitlement(List<FileData> filesData) implements Entitlement
@Override
public String description() {
return Strings.format("[%s] <%s>/<%s>%s", mode, baseDir, setting, exclusive ? " (exclusive)" : "");
return Strings.format("[%s] <%s>%s<%s>%s", mode, baseDir, SEPARATOR, setting, exclusive ? " (exclusive)" : "");
}
}

View file

@ -27,6 +27,7 @@ import java.util.Base64;
import java.util.List;
import java.util.Set;
import static org.elasticsearch.entitlement.runtime.policy.entitlements.FilesEntitlement.SEPARATOR;
import static org.elasticsearch.test.LambdaMatchers.transformedMatch;
import static org.hamcrest.Matchers.both;
import static org.hamcrest.Matchers.containsInAnyOrder;
@ -317,6 +318,7 @@ public class PolicyUtilsTests extends ESTestCase {
/** Test that we can format some simple files entitlement properly */
public void testFormatFilesEntitlement() {
var pathAB = Path.of("/a/b");
var pathCD = Path.of("c/d");
var policy = new Policy(
"test-plugin",
List.of(
@ -326,11 +328,7 @@ public class PolicyUtilsTests extends ESTestCase {
new FilesEntitlement(
List.of(
FilesEntitlement.FileData.ofPath(pathAB, FilesEntitlement.Mode.READ_WRITE),
FilesEntitlement.FileData.ofRelativePath(
Path.of("c/d"),
FilesEntitlement.BaseDir.DATA,
FilesEntitlement.Mode.READ
)
FilesEntitlement.FileData.ofRelativePath(pathCD, FilesEntitlement.BaseDir.DATA, FilesEntitlement.Mode.READ)
)
)
)
@ -353,7 +351,17 @@ public class PolicyUtilsTests extends ESTestCase {
)
);
Set<String> actual = PolicyUtils.getEntitlementsDescriptions(policy);
assertThat(actual, containsInAnyOrder("files [READ_WRITE] " + pathAB, "files [READ] <DATA>/c/d", "files [READ] <DATA>/<setting>"));
var pathABString = pathAB.toAbsolutePath().toString();
var pathCDString = SEPARATOR + pathCD.toString();
var pathSettingString = SEPARATOR + "<setting>";
assertThat(
actual,
containsInAnyOrder(
"files [READ_WRITE] " + pathABString,
"files [READ] <DATA>" + pathCDString,
"files [READ] <DATA>" + pathSettingString
)
);
}
/** Test that we can format some simple files entitlement properly */