This is an automated email from the ASF dual-hosted git repository.
ctubbsii pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/accumulo.git
The following commit(s) were added to refs/heads/main by this push:
new 40fb5d0 Make build work on Java 17 (#2500)
40fb5d0 is described below
commit 40fb5d01ffeefd6fb2e9a867a03bcf5737b56578
Author: Christopher Tubbs <ctubb...@apache.org>
AuthorDate: Tue Feb 15 19:28:16 2022 -0500
Make build work on Java 17 (#2500)
Ensure it is possible to successfully build using Java 17
These changes were tested with `mvn clean verify -Psunny`
* Add JDK 17 build to GitHub Actions
* Change format of `PasswordToken` serialization so it doesn't use GZip
from Hadoop's `WritableUtils.writeCompressedByteArray()`, which seems
to have changed format slightly between Java 11 and 17, resulting in
slightly different GZip header information. However, still support
reading the old version (added tests for these)
* Pass absurd jdk adds-opens options to ensure test code has access
parts of the JDK needed for mocking / junit testing
---
.github/workflows/maven.yaml | 11 +++---
.../security/tokens/AuthenticationToken.java | 12 +-----
.../core/client/security/tokens/PasswordToken.java | 43 ++++++++++++++++++++--
.../client/security/tokens/PasswordTokenTest.java | 39 ++++++++++++++++++++
.../accumulo/core/conf/ClientPropertyTest.java | 6 +--
.../accumulo/core/security/CredentialsTest.java | 3 +-
pom.xml | 13 ++++++-
.../org/apache/accumulo/compactor/Compactor.java | 2 +-
.../apache/accumulo/compactor/CompactorTest.java | 34 ++++-------------
9 files changed, 111 insertions(+), 52 deletions(-)
diff --git a/.github/workflows/maven.yaml b/.github/workflows/maven.yaml
index 5a9666d..701665c 100644
--- a/.github/workflows/maven.yaml
+++ b/.github/workflows/maven.yaml
@@ -60,17 +60,18 @@ jobs:
strategy:
matrix:
profile:
- - {name: 'unit-tests', args: 'verify -PskipQA -DskipTests=false'}
- - {name: 'qa-checks', args: 'verify javadoc:jar -Psec-bugs
-DskipTests -Dspotbugs.timeout=3600000'}
- - {name: 'hadoop-compat', args: 'package -DskipTests
-Dhadoop.version=3.0.3'}
+ - {name: 'unit-tests', javaver: 11, args: 'verify -PskipQA
-DskipTests=false'}
+ - {name: 'qa-checks', javaver: 11, args: 'verify javadoc:jar
-Psec-bugs -DskipTests -Dspotbugs.timeout=3600000'}
+ - {name: 'hadoop-compat', javaver: 11, args: 'package -DskipTests
-Dhadoop.version=3.0.3'}
+ - {name: 'jdk17', javaver: 17, args: 'verify -DskipITs'}
fail-fast: false
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- - name: Set up JDK 11
+ - name: Set up JDK ${{ matrix.profile.javaver }}
uses: actions/setup-java@v1
with:
- java-version: 11
+ java-version: ${{ matrix.profile.javaver }}
- name: Cache local maven repository
uses: actions/cache@v2
with:
diff --git
a/core/src/main/java/org/apache/accumulo/core/client/security/tokens/AuthenticationToken.java
b/core/src/main/java/org/apache/accumulo/core/client/security/tokens/AuthenticationToken.java
index a09a651..c57ef7a 100644
---
a/core/src/main/java/org/apache/accumulo/core/client/security/tokens/AuthenticationToken.java
+++
b/core/src/main/java/org/apache/accumulo/core/client/security/tokens/AuthenticationToken.java
@@ -118,20 +118,12 @@ public interface AuthenticationToken extends Writable,
Destroyable, Cloneable {
* @see #deserialize(Class, byte[])
*/
public static byte[] serialize(AuthenticationToken token) {
- ByteArrayOutputStream baos = new ByteArrayOutputStream();
- DataOutputStream out = new DataOutputStream(baos);
- try {
+ try (var baos = new ByteArrayOutputStream(); var out = new
DataOutputStream(baos)) {
token.write(out);
+ return baos.toByteArray();
} catch (IOException e) {
throw new RuntimeException("Bug found in serialization code", e);
}
- byte[] bytes = baos.toByteArray();
- try {
- out.close();
- } catch (IOException e) {
- throw new IllegalStateException("Shouldn't happen with
ByteArrayOutputStream", e);
- }
- return bytes;
}
}
diff --git
a/core/src/main/java/org/apache/accumulo/core/client/security/tokens/PasswordToken.java
b/core/src/main/java/org/apache/accumulo/core/client/security/tokens/PasswordToken.java
index d7aa53f..9348ed6 100644
---
a/core/src/main/java/org/apache/accumulo/core/client/security/tokens/PasswordToken.java
+++
b/core/src/main/java/org/apache/accumulo/core/client/security/tokens/PasswordToken.java
@@ -23,11 +23,14 @@ import static java.nio.charset.StandardCharsets.UTF_8;
import java.io.DataInput;
import java.io.DataOutput;
import java.io.IOException;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Proxy;
import java.nio.ByteBuffer;
import java.nio.CharBuffer;
import java.util.Arrays;
import java.util.LinkedHashSet;
import java.util.Set;
+import java.util.concurrent.atomic.AtomicBoolean;
import javax.security.auth.DestroyFailedException;
@@ -83,12 +86,46 @@ public class PasswordToken implements AuthenticationToken {
@Override
public void readFields(DataInput arg0) throws IOException {
- password = WritableUtils.readCompressedByteArray(arg0);
+ int version = arg0.readInt();
+ // -1 is null, consistent with legacy format; legacy format length must be
>= -1
+ // so, use -2 as a magic number to indicate the new format
+ if (version == -1) {
+ password = null;
+ } else if (version == -2) {
+ byte[] passwordTmp = new byte[arg0.readInt()];
+ arg0.readFully(passwordTmp);
+ password = passwordTmp;
+ } else {
+ // legacy format; should avoid reading/writing compressed byte arrays
using WritableUtils,
+ // because GZip is expensive and it doesn't actually compress passwords
very well
+ AtomicBoolean calledFirstReadInt = new AtomicBoolean(false);
+ DataInput wrapped = (DataInput)
Proxy.newProxyInstance(DataInput.class.getClassLoader(),
+ arg0.getClass().getInterfaces(), (obj, method, args) -> {
+ // wrap the original DataInput in order to return the integer that
was read
+ // and then not used, because it didn't match -2
+ if (!calledFirstReadInt.get() &&
method.getName().equals("readInt")) {
+ calledFirstReadInt.set(true);
+ return version;
+ }
+ try {
+ return method.invoke(arg0, args);
+ } catch (InvocationTargetException e) {
+ throw e.getCause();
+ }
+ });
+ password = WritableUtils.readCompressedByteArray(wrapped);
+ }
}
@Override
public void write(DataOutput arg0) throws IOException {
- WritableUtils.writeCompressedByteArray(arg0, password);
+ if (password == null) {
+ arg0.writeInt(-1);
+ return;
+ }
+ arg0.writeInt(-2); // magic number
+ arg0.writeInt(password.length);
+ arg0.write(password);
}
@Override
@@ -134,7 +171,7 @@ public class PasswordToken implements AuthenticationToken {
protected void setPassword(CharBuffer charBuffer) {
// encode() kicks back a C-string, which is not compatible with the old
passwording system
ByteBuffer bb = UTF_8.encode(charBuffer);
- // create array using byter buffer length
+ // create array using byte buffer length
this.password = new byte[bb.remaining()];
bb.get(this.password);
if (!bb.isReadOnly()) {
diff --git
a/core/src/test/java/org/apache/accumulo/core/client/security/tokens/PasswordTokenTest.java
b/core/src/test/java/org/apache/accumulo/core/client/security/tokens/PasswordTokenTest.java
index 7ea740e..4422914 100644
---
a/core/src/test/java/org/apache/accumulo/core/client/security/tokens/PasswordTokenTest.java
+++
b/core/src/test/java/org/apache/accumulo/core/client/security/tokens/PasswordTokenTest.java
@@ -19,8 +19,17 @@
package org.apache.accumulo.core.client.security.tokens;
import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.junit.Assert.assertArrayEquals;
import static org.junit.Assert.assertEquals;
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.DataInputStream;
+import java.io.DataOutputStream;
+import java.io.IOException;
+import java.util.Base64;
+import java.util.List;
+
import javax.security.auth.DestroyFailedException;
import org.junit.Test;
@@ -41,4 +50,34 @@ public class PasswordTokenTest {
s = new String(pt.getPassword(), UTF_8);
assertEquals("五六", s);
}
+
+ @Test
+ public void testReadingLegacyFormat() throws IOException {
+ String newFormat = "/////gAAAAh0ZXN0cGFzcw=="; // the new format without
using GZip
+ String oldFormat1 = "AAAAHB+LCAAAAAAAAAArSS0uKUgsLgYAGRFm+ggAAAA="; // jdk
11 GZip produced this
+ String oldFormat2 = "AAAAHB+LCAAAAAAAAP8rSS0uKUgsLgYAGRFm+ggAAAA="; // jdk
17 GZip produced this
+ for (String format : List.of(oldFormat1, oldFormat2, newFormat)) {
+ byte[] array = Base64.getDecoder().decode(format);
+ try (var bais = new ByteArrayInputStream(array); var dis = new
DataInputStream(bais)) {
+ var deserializedToken = new PasswordToken();
+ deserializedToken.readFields(dis);
+ assertArrayEquals("testpass".getBytes(UTF_8),
deserializedToken.getPassword());
+ }
+ }
+ }
+
+ @Test
+ public void testReadingAndWriting() throws IOException {
+ var originalToken = new PasswordToken("testpass");
+ byte[] saved;
+ try (var baos = new ByteArrayOutputStream(); var dos = new
DataOutputStream(baos)) {
+ originalToken.write(dos);
+ saved = baos.toByteArray();
+ }
+ try (var bais = new ByteArrayInputStream(saved); var dis = new
DataInputStream(bais)) {
+ var deserializedToken = new PasswordToken();
+ deserializedToken.readFields(dis);
+ assertArrayEquals(originalToken.getPassword(),
deserializedToken.getPassword());
+ }
+ }
}
diff --git
a/core/src/test/java/org/apache/accumulo/core/conf/ClientPropertyTest.java
b/core/src/test/java/org/apache/accumulo/core/conf/ClientPropertyTest.java
index 6f861f5..f15b20c 100644
--- a/core/src/test/java/org/apache/accumulo/core/conf/ClientPropertyTest.java
+++ b/core/src/test/java/org/apache/accumulo/core/conf/ClientPropertyTest.java
@@ -41,15 +41,13 @@ public class ClientPropertyTest {
assertEquals("testpass1", new String(((PasswordToken)
token).getPassword()));
ClientProperty.setAuthenticationToken(props, new
PasswordToken("testpass2"));
- assertEquals("AAAAHR+LCAAAAAAAAAArSS0uKUgsLjYCANxwRH4JAAAA",
- ClientProperty.AUTH_TOKEN.getValue(props));
+ assertEquals("/////gAAAAl0ZXN0cGFzczI=",
ClientProperty.AUTH_TOKEN.getValue(props));
token = ClientProperty.getAuthenticationToken(props);
assertTrue(token instanceof PasswordToken);
assertEquals("testpass2", new String(((PasswordToken)
token).getPassword()));
ClientProperty.setAuthenticationToken(props, new
PasswordToken("testpass3"));
- assertEquals("AAAAHR+LCAAAAAAAAAArSS0uKUgsLjYGAEpAQwkJAAAA",
- ClientProperty.AUTH_TOKEN.getValue(props));
+ assertEquals("/////gAAAAl0ZXN0cGFzczM=",
ClientProperty.AUTH_TOKEN.getValue(props));
token = ClientProperty.getAuthenticationToken(props);
assertTrue(token instanceof PasswordToken);
assertEquals("testpass3", new String(((PasswordToken)
token).getPassword()));
diff --git
a/core/src/test/java/org/apache/accumulo/core/security/CredentialsTest.java
b/core/src/test/java/org/apache/accumulo/core/security/CredentialsTest.java
index 59878e3..0837599 100644
--- a/core/src/test/java/org/apache/accumulo/core/security/CredentialsTest.java
+++ b/core/src/test/java/org/apache/accumulo/core/security/CredentialsTest.java
@@ -24,7 +24,6 @@ import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertThrows;
-import static org.junit.Assert.assertTrue;
import javax.security.auth.DestroyFailedException;
@@ -61,7 +60,7 @@ public class CredentialsTest {
// verify that we can't serialize if it's destroyed
creds.getToken().destroy();
Exception e = assertThrows(RuntimeException.class, () ->
creds.toThrift(instanceID));
- assertTrue(e.getCause() instanceof AccumuloSecurityException);
+ assertEquals(AccumuloSecurityException.class, e.getCause().getClass());
assertEquals(AccumuloSecurityException.class.cast(e.getCause()).getSecurityErrorCode(),
SecurityErrorCode.TOKEN_EXPIRED);
}
diff --git a/pom.xml b/pom.xml
index e1e3ae2..254ab2c 100644
--- a/pom.xml
+++ b/pom.xml
@@ -125,6 +125,7 @@
<errorprone.version>2.11.0</errorprone.version>
<!-- avoid error shutting down built-in ForkJoinPool.commonPool() during
exec:java tasks -->
<exec.cleanupDaemonThreads>false</exec.cleanupDaemonThreads>
+ <extraTestArgs />
<failsafe.excludedGroups />
<failsafe.forkCount>1</failsafe.forkCount>
<failsafe.groups />
@@ -852,7 +853,7 @@
<systemPropertyVariables>
<java.io.tmpdir>${project.build.directory}</java.io.tmpdir>
</systemPropertyVariables>
- <argLine>${unitTestMemSize}</argLine>
+ <argLine>${unitTestMemSize} ${extraTestArgs}</argLine>
</configuration>
</plugin>
<plugin>
@@ -866,6 +867,7 @@
<systemPropertyVariables>
<java.io.tmpdir>${project.build.directory}</java.io.tmpdir>
</systemPropertyVariables>
+ <argLine>${extraTestArgs}</argLine>
<trimStackTrace>false</trimStackTrace>
</configuration>
</plugin>
@@ -1683,5 +1685,14 @@
</plugins>
</build>
</profile>
+ <profile>
+ <id>jdk17</id>
+ <activation>
+ <jdk>[17,)</jdk>
+ </activation>
+ <properties>
+ <extraTestArgs>--add-opens java.base/java.lang=ALL-UNNAMED --add-opens
java.base/java.util=ALL-UNNAMED --add-opens java.base/java.io=ALL-UNNAMED
--add-opens java.base/java.net=ALL-UNNAMED --add-opens
java.management/java.lang.management=ALL-UNNAMED --add-opens
java.management/sun.management=ALL-UNNAMED --add-opens
java.base/java.security=ALL-UNNAMED --add-opens
java.base/java.lang.reflect=ALL-UNNAMED --add-opens
java.base/java.util.concurrent=ALL-UNNAMED --add-opens java.base/jav [...]
+ </properties>
+ </profile>
</profiles>
</project>
diff --git
a/server/compactor/src/main/java/org/apache/accumulo/compactor/Compactor.java
b/server/compactor/src/main/java/org/apache/accumulo/compactor/Compactor.java
index 1a3bc49..8701e33 100644
---
a/server/compactor/src/main/java/org/apache/accumulo/compactor/Compactor.java
+++
b/server/compactor/src/main/java/org/apache/accumulo/compactor/Compactor.java
@@ -621,7 +621,7 @@ public class Compactor extends AbstractServer implements
MetricsProducer, Compac
* number of bytes in input file
* @return number of seconds to wait between progress checks
*/
- protected long calculateProgressCheckTime(long numBytes) {
+ static long calculateProgressCheckTime(long numBytes) {
return Math.max(1, (numBytes / TEN_MEGABYTES));
}
diff --git
a/server/compactor/src/test/java/org/apache/accumulo/compactor/CompactorTest.java
b/server/compactor/src/test/java/org/apache/accumulo/compactor/CompactorTest.java
index 6e1bbdc..9a27352 100644
---
a/server/compactor/src/test/java/org/apache/accumulo/compactor/CompactorTest.java
+++
b/server/compactor/src/test/java/org/apache/accumulo/compactor/CompactorTest.java
@@ -59,7 +59,6 @@ import
org.powermock.core.classloader.annotations.PowerMockIgnore;
import org.powermock.core.classloader.annotations.PrepareForTest;
import
org.powermock.core.classloader.annotations.SuppressStaticInitializationFor;
import org.powermock.modules.junit4.PowerMockRunner;
-import org.powermock.reflect.Whitebox;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -298,24 +297,17 @@ public class CompactorTest {
@Test
public void testCheckTime() throws Exception {
- // Instantiates class without calling constructor
- Compactor c = Whitebox.newInstance(Compactor.class);
- assertEquals(1, c.calculateProgressCheckTime(1024));
- assertEquals(1, c.calculateProgressCheckTime(1048576));
- assertEquals(1, c.calculateProgressCheckTime(10485760));
- assertEquals(10, c.calculateProgressCheckTime(104857600));
- assertEquals(102, c.calculateProgressCheckTime(1024 * 1024 * 1024));
+ assertEquals(1, Compactor.calculateProgressCheckTime(1024));
+ assertEquals(1, Compactor.calculateProgressCheckTime(1048576));
+ assertEquals(1, Compactor.calculateProgressCheckTime(10485760));
+ assertEquals(10, Compactor.calculateProgressCheckTime(104857600));
+ assertEquals(102, Compactor.calculateProgressCheckTime(1024 * 1024 *
1024));
}
@Test
public void testCompactionSucceeds() throws Exception {
UUID uuid = UUID.randomUUID();
- Supplier<UUID> supplier = new Supplier<>() {
- @Override
- public UUID get() {
- return uuid;
- }
- };
+ Supplier<UUID> supplier = () -> uuid;
ExternalCompactionId eci = ExternalCompactionId.generate(supplier.get());
@@ -362,12 +354,7 @@ public class CompactorTest {
@Test
public void testCompactionFails() throws Exception {
UUID uuid = UUID.randomUUID();
- Supplier<UUID> supplier = new Supplier<>() {
- @Override
- public UUID get() {
- return uuid;
- }
- };
+ Supplier<UUID> supplier = () -> uuid;
ExternalCompactionId eci = ExternalCompactionId.generate(supplier.get());
@@ -416,12 +403,7 @@ public class CompactorTest {
@Test
public void testCompactionInterrupted() throws Exception {
UUID uuid = UUID.randomUUID();
- Supplier<UUID> supplier = new Supplier<>() {
- @Override
- public UUID get() {
- return uuid;
- }
- };
+ Supplier<UUID> supplier = () -> uuid;
ExternalCompactionId eci = ExternalCompactionId.generate(supplier.get());
