From 5ca3cab2f93398f34872b61d5b41a814c5bb1890 Mon Sep 17 00:00:00 2001 From: echonesis Date: Fri, 26 Dec 2025 14:46:07 +0800 Subject: [PATCH 1/3] HDDS-5306. Ozone Shell getacl capable of printing ACL string the way it is fed to setacl --- .../hadoop/ozone/shell/acl/GetAclHandler.java | 44 ++- .../ozone/shell/acl/TestGetAclHandler.java | 254 ++++++++++++++++++ 2 files changed, 297 insertions(+), 1 deletion(-) create mode 100644 hadoop-ozone/cli-shell/src/test/java/org/apache/hadoop/ozone/shell/acl/TestGetAclHandler.java diff --git a/hadoop-ozone/cli-shell/src/main/java/org/apache/hadoop/ozone/shell/acl/GetAclHandler.java b/hadoop-ozone/cli-shell/src/main/java/org/apache/hadoop/ozone/shell/acl/GetAclHandler.java index b73bfbff0665..4e5caa1747d1 100644 --- a/hadoop-ozone/cli-shell/src/main/java/org/apache/hadoop/ozone/shell/acl/GetAclHandler.java +++ b/hadoop-ozone/cli-shell/src/main/java/org/apache/hadoop/ozone/shell/acl/GetAclHandler.java @@ -19,19 +19,61 @@ import java.io.IOException; import java.util.List; +import java.util.stream.Collectors; import org.apache.hadoop.ozone.OzoneAcl; import org.apache.hadoop.ozone.client.OzoneClient; import org.apache.hadoop.ozone.security.acl.OzoneObj; +import picocli.CommandLine.Option; /** * Get ACLs. */ public abstract class GetAclHandler extends AclHandler { + @Option(names = {"--string", "-o"}, + description = "Output ACLs as a comma-separated string in the format " + + "that can be used with setacl/addacl commands") + private boolean stringFormat; + @Override protected void execute(OzoneClient client, OzoneObj obj) throws IOException { List result = client.getObjectStore().getAcl(obj); - printObjectAsJson(result); + if (stringFormat) { + printAclsAsString(result); + } else { + printObjectAsJson(result); + } + } + + /** + * Prints ACLs as a comma-separated string in the format: + * type:name:permissions or type:name:permissions[scope] if scope is DEFAULT. + * This format is compatible with setacl/addacl commands. + * + * @param acls List of OzoneAcl objects to print + */ + private void printAclsAsString(List acls) { + String aclString = acls.stream() + .map(this::formatAcl) + .collect(Collectors.joining(",")); + out().println(aclString); + } + + /** + * Formats a single ACL for string output. + * Omits the scope if it's ACCESS (the default) to match the input format. + * + * @param acl The OzoneAcl to format + * @return Formatted ACL string + */ + private String formatAcl(OzoneAcl acl) { + String baseAcl = acl.toString(); + // If the scope is ACCESS (default), remove it from the output + if (acl.getAclScope() == OzoneAcl.AclScope.ACCESS) { + // Remove [ACCESS] from the end + return baseAcl.replaceAll("\\[ACCESS\\]$", ""); + } + return baseAcl; } } diff --git a/hadoop-ozone/cli-shell/src/test/java/org/apache/hadoop/ozone/shell/acl/TestGetAclHandler.java b/hadoop-ozone/cli-shell/src/test/java/org/apache/hadoop/ozone/shell/acl/TestGetAclHandler.java new file mode 100644 index 000000000000..ce6e2d10c479 --- /dev/null +++ b/hadoop-ozone/cli-shell/src/test/java/org/apache/hadoop/ozone/shell/acl/TestGetAclHandler.java @@ -0,0 +1,254 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hadoop.ozone.shell.acl; + +import static org.apache.hadoop.ozone.OzoneAcl.AclScope.ACCESS; +import static org.apache.hadoop.ozone.OzoneAcl.AclScope.DEFAULT; +import static org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLIdentityType.GROUP; +import static org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLIdentityType.USER; +import static org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLType.ALL; +import static org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLType.READ; +import static org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLType.WRITE; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.PrintStream; +import java.io.UnsupportedEncodingException; +import java.nio.charset.StandardCharsets; +import java.util.Arrays; +import java.util.List; +import org.apache.hadoop.ozone.OzoneAcl; +import org.apache.hadoop.ozone.client.ObjectStore; +import org.apache.hadoop.ozone.client.OzoneClient; +import org.apache.hadoop.ozone.security.acl.OzoneObj; +import org.apache.hadoop.ozone.security.acl.OzoneObjInfo; +import org.apache.hadoop.ozone.shell.bucket.GetAclBucketHandler; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import picocli.CommandLine; + +/** + * Tests for GetAclHandler with string format output. + */ +public class TestGetAclHandler { + + private TestableGetAclBucketHandler cmd; + private final ByteArrayOutputStream outContent = new ByteArrayOutputStream(); + private final ByteArrayOutputStream errContent = new ByteArrayOutputStream(); + private final PrintStream originalOut = System.out; + private final PrintStream originalErr = System.err; + private static final String DEFAULT_ENCODING = StandardCharsets.UTF_8.name(); + + /** + * Testable version of GetAclBucketHandler that exposes execute method. + */ + static class TestableGetAclBucketHandler extends GetAclBucketHandler { + public void publicExecute(OzoneClient client, OzoneObj obj) + throws IOException { + execute(client, obj); + } + } + + @BeforeEach + public void setup() throws UnsupportedEncodingException { + cmd = new TestableGetAclBucketHandler(); + System.setOut(new PrintStream(outContent, false, DEFAULT_ENCODING)); + System.setErr(new PrintStream(errContent, false, DEFAULT_ENCODING)); + } + + @AfterEach + public void tearDown() { + System.setOut(originalOut); + System.setErr(originalErr); + } + + @Test + public void testGetAclAsJson() throws IOException { + // Setup mock objects + OzoneClient client = mock(OzoneClient.class); + ObjectStore objectStore = mock(ObjectStore.class); + when(client.getObjectStore()).thenReturn(objectStore); + + List acls = Arrays.asList( + OzoneAcl.of(USER, "user1", ACCESS, READ, WRITE), + OzoneAcl.of(GROUP, "hadoop", ACCESS, ALL) + ); + + OzoneObj obj = OzoneObjInfo.Builder.newBuilder() + .setBucketName("bucket") + .setVolumeName("volume") + .setResType(OzoneObj.ResourceType.BUCKET) + .setStoreType(OzoneObj.StoreType.OZONE) + .build(); + + when(objectStore.getAcl(obj)).thenReturn(acls); + + // Execute command without --string flag (JSON output) + cmd.publicExecute(client, obj); + + // Verify JSON output + ObjectMapper mapper = new ObjectMapper(); + JsonNode json = mapper.readTree(outContent.toString(DEFAULT_ENCODING)); + assertTrue(json.isArray()); + assertEquals(2, json.size()); + assertEquals("USER", json.get(0).get("type").asText()); + assertEquals("user1", json.get(0).get("name").asText()); + assertEquals("GROUP", json.get(1).get("type").asText()); + assertEquals("hadoop", json.get(1).get("name").asText()); + } + + @Test + public void testGetAclAsStringWithAccessScope() throws IOException { + // Setup mock objects + OzoneClient client = mock(OzoneClient.class); + ObjectStore objectStore = mock(ObjectStore.class); + when(client.getObjectStore()).thenReturn(objectStore); + + List acls = Arrays.asList( + OzoneAcl.of(USER, "user1", ACCESS, READ, WRITE), + OzoneAcl.of(GROUP, "hadoop", ACCESS, ALL) + ); + + OzoneObj obj = OzoneObjInfo.Builder.newBuilder() + .setBucketName("bucket") + .setVolumeName("volume") + .setResType(OzoneObj.ResourceType.BUCKET) + .setStoreType(OzoneObj.StoreType.OZONE) + .build(); + + when(objectStore.getAcl(obj)).thenReturn(acls); + + // Parse command line with --string flag + CommandLine commandLine = new CommandLine(cmd); + commandLine.parseArgs("--string", "o3://ozone1/volume/bucket"); + + // Execute command with --string flag + cmd.publicExecute(client, obj); + + // Verify string output (should omit [ACCESS] for default scope) + String output = outContent.toString(DEFAULT_ENCODING).trim(); + assertEquals("user:user1:rw,group:hadoop:a", output); + } + + @Test + public void testGetAclAsStringWithDefaultScope() throws IOException { + // Setup mock objects + OzoneClient client = mock(OzoneClient.class); + ObjectStore objectStore = mock(ObjectStore.class); + when(client.getObjectStore()).thenReturn(objectStore); + + List acls = Arrays.asList( + OzoneAcl.of(USER, "user1", DEFAULT, READ, WRITE), + OzoneAcl.of(GROUP, "hadoop", DEFAULT, ALL) + ); + + OzoneObj obj = OzoneObjInfo.Builder.newBuilder() + .setBucketName("bucket") + .setVolumeName("volume") + .setResType(OzoneObj.ResourceType.BUCKET) + .setStoreType(OzoneObj.StoreType.OZONE) + .build(); + + when(objectStore.getAcl(obj)).thenReturn(acls); + + // Parse command line with --string flag + CommandLine commandLine = new CommandLine(cmd); + commandLine.parseArgs("--string", "o3://ozone1/volume/bucket"); + + // Execute command with --string flag + cmd.publicExecute(client, obj); + + // Verify string output (should include [DEFAULT] for non-default scope) + String output = outContent.toString(DEFAULT_ENCODING).trim(); + assertEquals("user:user1:rw[DEFAULT],group:hadoop:a[DEFAULT]", output); + } + + @Test + public void testGetAclAsStringWithShortOption() throws IOException { + // Setup mock objects + OzoneClient client = mock(OzoneClient.class); + ObjectStore objectStore = mock(ObjectStore.class); + when(client.getObjectStore()).thenReturn(objectStore); + + List acls = Arrays.asList( + OzoneAcl.of(USER, "user1", ACCESS, READ, WRITE), + OzoneAcl.of(GROUP, "hadoop", ACCESS, ALL) + ); + + OzoneObj obj = OzoneObjInfo.Builder.newBuilder() + .setBucketName("bucket") + .setVolumeName("volume") + .setResType(OzoneObj.ResourceType.BUCKET) + .setStoreType(OzoneObj.StoreType.OZONE) + .build(); + + when(objectStore.getAcl(obj)).thenReturn(acls); + + // Parse command line with -o short flag + CommandLine commandLine = new CommandLine(cmd); + commandLine.parseArgs("-o", "o3://ozone1/volume/bucket"); + + // Execute command with -o flag + cmd.publicExecute(client, obj); + + // Verify string output (should omit [ACCESS] for default scope) + String output = outContent.toString(DEFAULT_ENCODING).trim(); + assertEquals("user:user1:rw,group:hadoop:a", output); + } + + @Test + public void testGetAclAsStringMixedScopes() throws IOException { + // Setup mock objects + OzoneClient client = mock(OzoneClient.class); + ObjectStore objectStore = mock(ObjectStore.class); + when(client.getObjectStore()).thenReturn(objectStore); + + List acls = Arrays.asList( + OzoneAcl.of(USER, "user1", ACCESS, READ, WRITE), + OzoneAcl.of(GROUP, "hadoop", DEFAULT, ALL) + ); + + OzoneObj obj = OzoneObjInfo.Builder.newBuilder() + .setBucketName("bucket") + .setVolumeName("volume") + .setResType(OzoneObj.ResourceType.BUCKET) + .setStoreType(OzoneObj.StoreType.OZONE) + .build(); + + when(objectStore.getAcl(obj)).thenReturn(acls); + + // Parse command line with --string flag + CommandLine commandLine = new CommandLine(cmd); + commandLine.parseArgs("--string", "o3://ozone1/volume/bucket"); + + // Execute command with --string flag + cmd.publicExecute(client, obj); + + // Verify string output (mixed scopes) + String output = outContent.toString(DEFAULT_ENCODING).trim(); + assertEquals("user:user1:rw,group:hadoop:a[DEFAULT]", output); + } + +} From 7edb5a5f48d01d1142d685b7c858eacbb14bfb6d Mon Sep 17 00:00:00 2001 From: echonesis Date: Tue, 30 Dec 2025 18:09:09 +0800 Subject: [PATCH 2/3] fix: update CR --- .../hadoop/ozone/shell/acl/GetAclHandler.java | 15 ++--- .../ozone/shell/acl/TestGetAclHandler.java | 57 +++++-------------- 2 files changed, 22 insertions(+), 50 deletions(-) diff --git a/hadoop-ozone/cli-shell/src/main/java/org/apache/hadoop/ozone/shell/acl/GetAclHandler.java b/hadoop-ozone/cli-shell/src/main/java/org/apache/hadoop/ozone/shell/acl/GetAclHandler.java index 4e5caa1747d1..98ec520474c9 100644 --- a/hadoop-ozone/cli-shell/src/main/java/org/apache/hadoop/ozone/shell/acl/GetAclHandler.java +++ b/hadoop-ozone/cli-shell/src/main/java/org/apache/hadoop/ozone/shell/acl/GetAclHandler.java @@ -30,18 +30,19 @@ */ public abstract class GetAclHandler extends AclHandler { - @Option(names = {"--string", "-o"}, - description = "Output ACLs as a comma-separated string in the format " + - "that can be used with setacl/addacl commands") - private boolean stringFormat; + @Option(names = {"--json"}, + description = "Format output as JSON", + defaultValue = "true", + fallbackValue = "true") + private boolean json; @Override protected void execute(OzoneClient client, OzoneObj obj) throws IOException { List result = client.getObjectStore().getAcl(obj); - if (stringFormat) { - printAclsAsString(result); - } else { + if (json) { printObjectAsJson(result); + } else { + printAclsAsString(result); } } diff --git a/hadoop-ozone/cli-shell/src/test/java/org/apache/hadoop/ozone/shell/acl/TestGetAclHandler.java b/hadoop-ozone/cli-shell/src/test/java/org/apache/hadoop/ozone/shell/acl/TestGetAclHandler.java index ce6e2d10c479..c4f93517dab7 100644 --- a/hadoop-ozone/cli-shell/src/test/java/org/apache/hadoop/ozone/shell/acl/TestGetAclHandler.java +++ b/hadoop-ozone/cli-shell/src/test/java/org/apache/hadoop/ozone/shell/acl/TestGetAclHandler.java @@ -105,7 +105,11 @@ public void testGetAclAsJson() throws IOException { when(objectStore.getAcl(obj)).thenReturn(acls); - // Execute command without --string flag (JSON output) + // Parse command line without flag (default JSON output) + CommandLine commandLine = new CommandLine(cmd); + commandLine.parseArgs("o3://ozone1/volume/bucket"); + + // Execute command with default JSON output cmd.publicExecute(client, obj); // Verify JSON output @@ -140,11 +144,11 @@ public void testGetAclAsStringWithAccessScope() throws IOException { when(objectStore.getAcl(obj)).thenReturn(acls); - // Parse command line with --string flag + // Parse command line with --json=false flag (string output) CommandLine commandLine = new CommandLine(cmd); - commandLine.parseArgs("--string", "o3://ozone1/volume/bucket"); + commandLine.parseArgs("--json=false", "o3://ozone1/volume/bucket"); - // Execute command with --string flag + // Execute command with --json=false flag cmd.publicExecute(client, obj); // Verify string output (should omit [ACCESS] for default scope) @@ -173,11 +177,11 @@ public void testGetAclAsStringWithDefaultScope() throws IOException { when(objectStore.getAcl(obj)).thenReturn(acls); - // Parse command line with --string flag + // Parse command line with --json=false flag (string output) CommandLine commandLine = new CommandLine(cmd); - commandLine.parseArgs("--string", "o3://ozone1/volume/bucket"); + commandLine.parseArgs("--json=false", "o3://ozone1/volume/bucket"); - // Execute command with --string flag + // Execute command with --json=false flag cmd.publicExecute(client, obj); // Verify string output (should include [DEFAULT] for non-default scope) @@ -185,39 +189,6 @@ public void testGetAclAsStringWithDefaultScope() throws IOException { assertEquals("user:user1:rw[DEFAULT],group:hadoop:a[DEFAULT]", output); } - @Test - public void testGetAclAsStringWithShortOption() throws IOException { - // Setup mock objects - OzoneClient client = mock(OzoneClient.class); - ObjectStore objectStore = mock(ObjectStore.class); - when(client.getObjectStore()).thenReturn(objectStore); - - List acls = Arrays.asList( - OzoneAcl.of(USER, "user1", ACCESS, READ, WRITE), - OzoneAcl.of(GROUP, "hadoop", ACCESS, ALL) - ); - - OzoneObj obj = OzoneObjInfo.Builder.newBuilder() - .setBucketName("bucket") - .setVolumeName("volume") - .setResType(OzoneObj.ResourceType.BUCKET) - .setStoreType(OzoneObj.StoreType.OZONE) - .build(); - - when(objectStore.getAcl(obj)).thenReturn(acls); - - // Parse command line with -o short flag - CommandLine commandLine = new CommandLine(cmd); - commandLine.parseArgs("-o", "o3://ozone1/volume/bucket"); - - // Execute command with -o flag - cmd.publicExecute(client, obj); - - // Verify string output (should omit [ACCESS] for default scope) - String output = outContent.toString(DEFAULT_ENCODING).trim(); - assertEquals("user:user1:rw,group:hadoop:a", output); - } - @Test public void testGetAclAsStringMixedScopes() throws IOException { // Setup mock objects @@ -239,11 +210,11 @@ public void testGetAclAsStringMixedScopes() throws IOException { when(objectStore.getAcl(obj)).thenReturn(acls); - // Parse command line with --string flag + // Parse command line with --json=false flag (string output) CommandLine commandLine = new CommandLine(cmd); - commandLine.parseArgs("--string", "o3://ozone1/volume/bucket"); + commandLine.parseArgs("--json=false", "o3://ozone1/volume/bucket"); - // Execute command with --string flag + // Execute command with --json=false flag cmd.publicExecute(client, obj); // Verify string output (mixed scopes) From fb5ea4290fd4e25b6e51b830c8fa8f12243f525b Mon Sep 17 00:00:00 2001 From: echonesis Date: Wed, 31 Dec 2025 15:28:23 +0800 Subject: [PATCH 3/3] feat: add --no-json and descriptions --- .../apache/hadoop/ozone/shell/acl/GetAclHandler.java | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/hadoop-ozone/cli-shell/src/main/java/org/apache/hadoop/ozone/shell/acl/GetAclHandler.java b/hadoop-ozone/cli-shell/src/main/java/org/apache/hadoop/ozone/shell/acl/GetAclHandler.java index 98ec520474c9..aba3907ef98f 100644 --- a/hadoop-ozone/cli-shell/src/main/java/org/apache/hadoop/ozone/shell/acl/GetAclHandler.java +++ b/hadoop-ozone/cli-shell/src/main/java/org/apache/hadoop/ozone/shell/acl/GetAclHandler.java @@ -30,10 +30,12 @@ */ public abstract class GetAclHandler extends AclHandler { - @Option(names = {"--json"}, - description = "Format output as JSON", - defaultValue = "true", - fallbackValue = "true") + @Option(names = "--json", negatable = true, + defaultValue = "true", fallbackValue = "true", + description = { + "Format output as JSON. Default is true.", + "Use --json=false or --no-json to turn off output JSON format." + }) private boolean json; @Override