jerryshao commented on code in PR #14532:
URL: https://github.com/apache/gravitino/pull/14532#discussion_r4450592338
##########
authorizations/authorization-ranger/src/test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerITEnv.java:
##########
@@ -517,7 +517,7 @@ protected static void updateOrCreateRangerPolicy(
Assertions.assertTrue(policies.size() <= 1);
if (!policies.isEmpty()) {
RangerPolicy policy = policies.get(0);
- policy.getPolicyItems().addAll(policyItems);
+ policyItems.forEach(policy::addPolicyItem);
Review Comment:
`rangerClient` here is the plain `RangerClient` from `RangerContainer` (`new
RangerClient`). In Ranger 2.8, `RangerRESTClient.toJson` goes through
`JsonUtils`, which writes dates as `yyyyMMdd-HH:mm:ss.SSS-Z`, and Ranger Admin
2.4 rejects that with HTTP 400. `RangerClientExtension` clears
`createTime`/`updateTime` only in its own `createPolicy`/`updatePolicy`, but
this path updates a policy fetched from Admin, so both fields are set. The CI
image is still Ranger 2.4.0 (`dev/docker/ranger/.env`). Please use
`RangerClientExtension` here, or clear both timestamps before `updatePolicy`.
##########
build.gradle.kts:
##########
@@ -305,6 +306,95 @@ class LegalFilesTransformer(
}
}
+/**
+ * Removes the Apache Avro copy that hadoop-client-runtime shades under
+ * org/apache/hadoop/shaded/org/apache/avro when it is older than 1.11.4.
+ *
+ * Hadoop 3.3.6 shades Avro 1.7.7, and a dependency constraint cannot replace
classes that live inside
+ * another jar. Within Hadoop the shaded Avro only backs Avro serialization
(SequenceFile, MapReduce job
+ * history, `hadoop fs -text`), which Gravitino does not use; the FileSystem,
HDFS and Kerberos code
+ * paths do not load it. Other jars pass through unchanged. Remove this
transform once Hadoop is
+ * upgraded to a release that shades Avro 1.11.4 or later (Hadoop 3.4.2+).
+ */
+@CacheableTransform
+abstract class StripShadedAvroTransform :
TransformAction<TransformParameters.None> {
+ companion object {
+ val STRIPPED: Attribute<Boolean> =
+ Attribute.of("org.apache.gravitino.shaded-avro-stripped",
Boolean::class.javaObjectType)
+ const val SHADED_AVRO_PREFIX = "org/apache/hadoop/shaded/org/apache/avro/"
+ const val AVRO_METADATA_PREFIX = "META-INF/maven/org.apache.avro/"
+ const val AVRO_POM_PROPERTIES =
"META-INF/maven/org.apache.avro/avro/pom.properties"
+ private val MIN_AVRO_VERSION = listOf(1, 11, 4)
+
+ /** Returns true if the dotted [version] is lower than 1.11.4. */
+ fun isOutdatedAvro(version: String): Boolean {
+ val parts = version.split('.', '-').map { it.toIntOrNull() ?: 0 }
+ for (i in MIN_AVRO_VERSION.indices) {
+ val part = parts.getOrElse(i) { 0 }
+ if (part != MIN_AVRO_VERSION[i]) {
+ return part < MIN_AVRO_VERSION[i]
+ }
+ }
+ return false
+ }
+ }
+
+ @get:InputArtifact
+ @get:PathSensitive(PathSensitivity.NAME_ONLY)
+ abstract val inputArtifact: Provider<FileSystemLocation>
+
+ override fun transform(outputs: TransformOutputs) {
+ val input = inputArtifact.get().asFile
+ if (!input.isFile || !input.name.startsWith("hadoop-client-runtime-") ||
!input.name.endsWith(".jar")) {
+ outputs.file(inputArtifact)
+ return
+ }
+
+ val avroVersion = ZipFile(input).use { jar ->
+ jar.getEntry(AVRO_POM_PROPERTIES)?.let { entry ->
+ Properties().apply { jar.getInputStream(entry).use { load(it) }
}.getProperty("version")
+ }
+ }
+ if (avroVersion == null || !isOutdatedAvro(avroVersion)) {
+ outputs.file(inputArtifact)
+ return
+ }
+
+ // Keep the original file name so that the distribution layout and license
reports do not change.
+ val output = outputs.file(input.name)
+ var removed = 0
+ ZipFile(input).use { jar ->
+ ZipOutputStream(output).use { out ->
+ jar.entries().asSequence().forEach { source ->
+ if (source.name.startsWith(SHADED_AVRO_PREFIX) ||
source.name.startsWith(AVRO_METADATA_PREFIX)) {
+ removed++
+ return@forEach
+ }
+ val entry = ZipEntry(source.name)
+ entry.time = source.time
+ out.putNextEntry(entry)
+ if (!source.isDirectory) {
+ jar.getInputStream(source).use { it.copyTo(out) }
+ }
+ out.closeEntry()
+ }
+ }
+ }
+ if (removed == 0) {
+ throw GradleException("Expected shaded Avro $avroVersion entries in
${input.name}, found none")
+ }
+ }
+}
+
+// Projects whose runtime libraries ship hadoop-client-runtime in the
distribution.
+val shadedAvroStrippedProjects = setOf(
Review Comment:
The cloud bundles (`:bundles:aws-bundle`, `gcp-bundle`, `azure-bundle`,
`aliyun-bundle`, `tencent-bundle`) aren't covered here. Each shadow jar has
`implementation(libs.hadoop3.client.runtime)` and doesn't relocate
`org.apache.hadoop.shaded.org.apache.avro`, so dropping one into
`catalogs/fileset/libs` puts Avro 1.7.7 back on that classpath under the same
package. Please apply `StripShadedAvroTransform` to those bundles'
`runtimeClasspath`, or exclude `hadoop-client-runtime` from the shadow jar and
rely on the catalog's copy.
##########
authorizations/authorization-ranger/src/test/java/org/apache/gravitino/authorization/ranger/integration/test/RangerHiveIT.java:
##########
@@ -2192,7 +2192,7 @@ private void replacePolicyOwnerRole(
for (RangerPolicy.RangerPolicyItem policyItem : policy.getPolicyItems())
{
if (policyItem.getRoles().remove(currentOwnerRole)) {
if (!policyItem.getRoles().contains(legacyOwnerRole)) {
- policyItem.getRoles().add(legacyOwnerRole);
+ policyItem.addRole(legacyOwnerRole);
Review Comment:
Same issue in `replacePolicyOwnerRole` (called at L1539): the
`rangerClient.updatePolicy(policy.getId(), policy)` at L2201 sends a fetched
policy with `createTime`/`updateTime` set through the plain 2.8 client, which
Admin 2.4 rejects. Please route this through `RangerClientExtension` or clear
both timestamps first.
##########
catalogs/hive-metastore2-libs/build.gradle.kts:
##########
@@ -39,6 +39,10 @@ dependencies {
implementation(libs.snappy.java)
implementation(libs.commons.beanutils)
implementation(libs.woodstox.core)
+ // hadoop-common, hadoop-auth, Curator and the Hive shims bring ZooKeeper
3.6.3, which is end of
+ // life. Only the ZooKeeper client classes are on this classpath.
+ implementation(libs.zookeeper)
Review Comment:
ZooKeeper 3.8.8's POM declares `logback-classic`, `logback-core`, and
`netty-transport-native-epoll` at compile scope. Could you paste the output of
`:catalogs:hive-metastore2-libs:dependencies` (and the hive-metastore3-libs
equivalent) showing `zookeeper-3.8.8` and no `logback-*` or
`netty-transport-native-epoll` on `runtimeClasspath`?
##########
authorizations/authorization-ranger/src/test/java/org/apache/gravitino/authorization/ranger/reference/TestRangerReferenceJson.java:
##########
@@ -0,0 +1,144 @@
+/*
+ * 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.gravitino.authorization.ranger.reference;
+
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.google.gson.Gson;
+import com.google.gson.GsonBuilder;
+import java.text.SimpleDateFormat;
+import java.time.Instant;
+import java.util.Date;
+import java.util.TimeZone;
+import org.apache.ranger.authorization.utils.JsonUtils;
+import org.apache.ranger.plugin.util.JsonUtilsV2;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Pins the JSON exchanged with Ranger Admin for the user and group APIs.
Since Ranger 2.8 the
+ * client serializes requests with {@link JsonUtils} (Jackson 2) and parses
responses with {@link
+ * JsonUtilsV2} (Jackson 2), whereas Ranger 2.4 used Gson for requests and
Jackson 1 for responses.
+ */
+public class TestRangerReferenceJson {
+
+ // The Gson configuration of RangerRESTClient in Ranger 2.4, used to
serialize request bodies.
+ private static final Gson RANGER_24_REQUEST_GSON =
+ new GsonBuilder().setDateFormat("yyyy-MM-dd'T'HH:mm:ss.SSSZ").create();
+
+ private static final ObjectMapper MAPPER = new ObjectMapper();
+
+ @Test
+ public void testUserRequestMatchesRanger24Format() throws Exception {
+ VXUser user =
VXUser.builder().withName("user1").withDescription("desc").build();
+
+ JsonNode actual = MAPPER.readTree(JsonUtils.objectToJson(user));
+ JsonNode expected = MAPPER.readTree(RANGER_24_REQUEST_GSON.toJson(user));
+
+ Assertions.assertEquals(expected, actual);
+ Assertions.assertEquals("user1", actual.get("name").asText());
+ Assertions.assertEquals("rangerR0cks!", actual.get("password").asText());
+ Assertions.assertEquals(RangerCommonEnums.STATUS_ENABLED,
actual.get("status").asInt());
+ Assertions.assertEquals(RangerCommonEnums.IS_VISIBLE,
actual.get("isVisible").asInt());
+ Assertions.assertFalse(actual.has("id"));
+ Assertions.assertFalse(actual.has("createDate"));
+ }
+
+ @Test
+ public void testGroupRequestMatchesRanger24Format() throws Exception {
+ VXGroup group =
VXGroup.builder().withName("group1").withDescription("desc").build();
+
+ JsonNode actual = MAPPER.readTree(JsonUtils.objectToJson(group));
+ JsonNode expected = MAPPER.readTree(RANGER_24_REQUEST_GSON.toJson(group));
+
+ Assertions.assertEquals(expected, actual);
+ Assertions.assertEquals("group1", actual.get("name").asText());
+ Assertions.assertEquals(RangerCommonEnums.GROUP_INTERNAL,
actual.get("groupSource").asInt());
+ Assertions.assertFalse(actual.has("id"));
+ }
+
+ @Test
+ public void testDateUsesRangerAdminFormat() throws Exception {
+ VXUser user = VXUser.builder().withName("user1").build();
+ Date epoch = Date.from(Instant.EPOCH);
+ user.createDate = epoch;
+
+ JsonNode actual = MAPPER.readTree(JsonUtils.objectToJson(user));
+
+ SimpleDateFormat format = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss'Z'");
Review Comment:
nit: This `SimpleDateFormat` uses the JVM default time zone, while the
pattern treats `Z` as a literal. `testParseUserResponses` sets UTC, but this
test doesn't, so it fails when the JVM zone isn't UTC. Please set
`TimeZone.getTimeZone("UTC")` here too.
##########
authorizations/authorization-ranger/src/test/java/org/apache/gravitino/authorization/ranger/TestRangerClientExtension.java:
##########
@@ -0,0 +1,220 @@
+/*
+ * 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.gravitino.authorization.ranger;
+
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import com.sun.jersey.api.client.ClientResponse;
+import com.sun.net.httpserver.HttpExchange;
+import com.sun.net.httpserver.HttpServer;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.net.InetAddress;
+import java.net.InetSocketAddress;
+import java.nio.charset.StandardCharsets;
+import java.time.Instant;
+import java.util.Date;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.CopyOnWriteArrayList;
+import org.apache.gravitino.authorization.ranger.reference.VXGroup;
+import org.apache.gravitino.authorization.ranger.reference.VXGroupList;
+import org.apache.gravitino.authorization.ranger.reference.VXUser;
+import org.apache.gravitino.authorization.ranger.reference.VXUserList;
+import org.apache.ranger.RangerServiceException;
+import org.apache.ranger.plugin.model.RangerPolicy;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+/** Exercises {@link RangerClientExtension} over HTTP against a stub Ranger
Admin. */
+public class TestRangerClientExtension {
+
+ private static final ObjectMapper MAPPER = new ObjectMapper();
+
+ private final List<String> requests = new CopyOnWriteArrayList<>();
+ private final Map<String, String> requestBodies = new ConcurrentHashMap<>();
+ private HttpServer server;
+ private RangerClientExtension client;
+
+ @BeforeEach
+ public void setUp() throws IOException {
+ server = HttpServer.create(new
InetSocketAddress(InetAddress.getLoopbackAddress(), 0), 0);
Review Comment:
`InetAddress.getLoopbackAddress()` can return `::1` when IPv6 is preferred,
while the client URL is always `http://127.0.0.1` (L109). Please bind to `new
InetSocketAddress("127.0.0.1", 0)` so the stub and the client use the same
address.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
