Skip to content

[Entitlements] Fix PolicyUtils and PolicyUtilsTests on Windows #126185

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

Merged
merged 5 commits into from
Apr 3, 2025
Merged

[Entitlements] Fix PolicyUtils and PolicyUtilsTests on Windows #126185

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
11b5595
Fix test for windows
ldematte Apr 3, 2025
ab7828b
Merge branch 'main' into entitlements/fix-policy-utils-test
ldematte Apr 3, 2025
399165a
iter
ldematte Apr 3, 2025
9d9babf
Merge branch 'entitlements/fix-policy-utils-test' of github.com:ldema…
ldematte Apr 3, 2025
409525c
fix PolicyUtils not closing stream
ldematte Apr 3, 2025
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
4 changes: 3 additions & 1 deletion libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/PolicyUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,9 @@ private static void validatePolicyScopes(String layerName, Policy policy, Set<St
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());
}
Expand Down
7 changes: 5 additions & 2 deletions ...main/java/org/elasticsearch/entitlement/runtime/policy/entitlements/FilesEntitlement.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
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;
Expand All @@ -31,6 +32,8 @@
*/
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 {
Expand Down Expand Up @@ -160,7 +163,7 @@ public FileData withPlatform(Platform platform) {

@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)" : "");
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the significance of this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To have separators consistent in the output; on Windows that was looking weird, like <DATA>/c\d
This is more cosmetic, but since I fixed the assertion I wanted it to be consistent too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I could have asserted that on Windows the "correct" output was <DATA>/c\d, but I wanted to have it like <DATA>\c\d

}
}

Expand Down Expand Up @@ -192,7 +195,7 @@ public FileData withPlatform(Platform platform) {

@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)" : "");
}
}

Expand Down
20 changes: 14 additions & 6 deletions ...itlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/PolicyUtilsTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
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;
Expand Down Expand Up @@ -317,6 +318,7 @@ public void testFormatPolicyWithMultipleScopes() {
/** 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(
Expand All @@ -326,11 +328,7 @@ public void testFormatFilesEntitlement() {
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)
)
)
)
Expand All @@ -353,7 +351,17 @@ public void testFormatFilesEntitlement() {
)
);
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 */
Expand Down