From 3e6018e2b7a6062092a4508c523170ed58af63b6 Mon Sep 17 00:00:00 2001 From: Hari Dara Date: Thu, 30 Oct 2025 09:57:33 +0530 Subject: [PATCH] Fix misc. issues flagged in PR validation build of #7421 --- .../hadoop/hbase/io/crypto/KeymetaTestUtils.java | 2 +- .../hadoop/hbase/io/crypto/TestManagedKeyData.java | 10 ++-------- .../hbase/io/crypto/TestManagedKeyProvider.java | 6 +++--- .../hadoop/hbase/keymeta/KeyNamespaceUtil.java | 5 ++++- .../hadoop/hbase/keymeta/ManagedKeyDataCache.java | 14 ++++++++++---- .../hadoop/hbase/security/TestSecurityUtil.java | 6 ++++-- .../src/main/ruby/shell/commands/rotate_stk.rb | 1 - hbase-shell/src/test/ruby/tests_runner.rb | 5 ++--- 8 files changed, 26 insertions(+), 23 deletions(-) diff --git a/hbase-common/src/test/java/org/apache/hadoop/hbase/io/crypto/KeymetaTestUtils.java b/hbase-common/src/test/java/org/apache/hadoop/hbase/io/crypto/KeymetaTestUtils.java index f02979cd9893..78a3f143825e 100644 --- a/hbase-common/src/test/java/org/apache/hadoop/hbase/io/crypto/KeymetaTestUtils.java +++ b/hbase-common/src/test/java/org/apache/hadoop/hbase/io/crypto/KeymetaTestUtils.java @@ -43,7 +43,7 @@ import org.apache.hbase.thirdparty.com.google.common.base.Preconditions; -public class KeymetaTestUtils { +public final class KeymetaTestUtils { /** * A ByteArrayInputStream that implements Seekable and PositionedReadable to work with diff --git a/hbase-common/src/test/java/org/apache/hadoop/hbase/io/crypto/TestManagedKeyData.java b/hbase-common/src/test/java/org/apache/hadoop/hbase/io/crypto/TestManagedKeyData.java index 555bf66b0e0d..36932371c110 100644 --- a/hbase-common/src/test/java/org/apache/hadoop/hbase/io/crypto/TestManagedKeyData.java +++ b/hbase-common/src/test/java/org/apache/hadoop/hbase/io/crypto/TestManagedKeyData.java @@ -154,15 +154,9 @@ public void testGetKeyMetadataHashEncoded() { @Test public void testGetKeyMetadataHashEncodedWithNullHash() { // Create ManagedKeyData with FAILED state and null metadata + // Passing null for metadata should result in null hash. ManagedKeyData keyData = - new ManagedKeyData("custodian".getBytes(), "namespace", null, ManagedKeyState.FAILED, null // null - // metadata - // should - // result - // in - // null - // hash - ); + new ManagedKeyData("custodian".getBytes(), "namespace", null, ManagedKeyState.FAILED, null); String encoded = keyData.getKeyMetadataHashEncoded(); assertNull(encoded); diff --git a/hbase-common/src/test/java/org/apache/hadoop/hbase/io/crypto/TestManagedKeyProvider.java b/hbase-common/src/test/java/org/apache/hadoop/hbase/io/crypto/TestManagedKeyProvider.java index 7a003f2943ed..2ec1bc718623 100644 --- a/hbase-common/src/test/java/org/apache/hadoop/hbase/io/crypto/TestManagedKeyProvider.java +++ b/hbase-common/src/test/java/org/apache/hadoop/hbase/io/crypto/TestManagedKeyProvider.java @@ -300,7 +300,7 @@ public void testNamespaceMismatchReturnsFailedKey() throws Exception { assertEquals(ManagedKeyState.FAILED, keyData.getKeyState()); assertNull(keyData.getTheKey()); assertEquals(requestedNamespace, keyData.getKeyNamespace()); - assertEquals(firstCust, keyData.getKeyCustodian()); + assertTrue(Bytes.equals(firstCust.get(), keyData.getKeyCustodian())); } @Test @@ -475,14 +475,14 @@ private void assertKeyDataWithNamespace(ManagedKeyData keyData, ManagedKeyState } else { byte[] keyBytes = keyData.getTheKey().getEncoded(); assertEquals(key.length, keyBytes.length); - assertEquals(new Bytes(key), keyBytes); + assertTrue(Bytes.equals(key, keyBytes)); } // Use helper method instead of duplicated parsing logic String encodedCust = Base64.getEncoder().encodeToString(custBytes); assertMetadataMatches(keyData.getKeyMetadata(), alias, encodedCust, expectedNamespace); - assertEquals(new Bytes(custBytes), keyData.getKeyCustodian()); + assertTrue(Bytes.equals(custBytes, keyData.getKeyCustodian())); assertEquals(keyData, managedKeyProvider.unwrapKey(keyData.getKeyMetadata(), null)); } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/keymeta/KeyNamespaceUtil.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/keymeta/KeyNamespaceUtil.java index 52b6adddc6f7..f1dc239cd76f 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/keymeta/KeyNamespaceUtil.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/keymeta/KeyNamespaceUtil.java @@ -32,7 +32,10 @@ */ @InterfaceAudience.Private @InterfaceStability.Evolving -public class KeyNamespaceUtil { +public final class KeyNamespaceUtil { + private KeyNamespaceUtil() { + throw new UnsupportedOperationException("Cannot instantiate utility class"); + } /** * Construct a key namespace from a table descriptor and column family descriptor. diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/keymeta/ManagedKeyDataCache.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/keymeta/ManagedKeyDataCache.java index 0b51f8c54a09..3f3c66a8bccd 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/keymeta/ManagedKeyDataCache.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/keymeta/ManagedKeyDataCache.java @@ -62,8 +62,12 @@ public ActiveKeysCacheKey(byte[] custodian, String namespace) { @Override public boolean equals(Object obj) { - if (this == obj) return true; - if (obj == null || getClass() != obj.getClass()) return false; + if (this == obj) { + return true; + } + if (obj == null || getClass() != obj.getClass()) { + return false; + } ActiveKeysCacheKey cacheKey = (ActiveKeysCacheKey) obj; return Bytes.equals(custodian, cacheKey.custodian) && Objects.equals(namespace, cacheKey.namespace); @@ -161,7 +165,8 @@ public ManagedKeyData getEntry(byte[] key_cust, String keyNamespace, String keyM } return keyData; }); - if (ManagedKeyState.isUsable(entry.getKeyState())) { + // This should never be null, but adding a check just to satisfy spotbugs. + if (entry != null && ManagedKeyState.isUsable(entry.getKeyState())) { return entry; } return null; @@ -241,7 +246,8 @@ public ManagedKeyData getActiveEntry(byte[] key_cust, String keyNamespace) { return retrievedKey; }); - if (keyData.getKeyState() == ManagedKeyState.ACTIVE) { + // This should never be null, but adding a check just to satisfy spotbugs. + if (keyData!= null && keyData.getKeyState() == ManagedKeyState.ACTIVE) { return keyData; } return null; diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/TestSecurityUtil.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/TestSecurityUtil.java index 2077673fde8a..e648d8a1c217 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/TestSecurityUtil.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/TestSecurityUtil.java @@ -424,7 +424,8 @@ public void testWithKeyManagement_LocalKeyGen_WithKeyAlgorithmMismatch() throws configBuilder().withKeyManagement(true).apply(conf); setupManagedKeyDataCache(testTableNamespace, mockManagedKeyData); assertEncryptionContextThrowsForWrites(IllegalStateException.class, - "Encryption for family 'test-family' configured with type 'AES' but key specifies algorithm 'DES'"); + "Encryption for family 'test-family' configured with type 'AES' but key specifies " + + "algorithm 'DES'"); } @Test @@ -472,7 +473,8 @@ public void testWithoutKeyManagement_KeyAlgorithmMismatch() throws Exception { when(mockFamily.getEncryptionKey()).thenReturn(wrappedDESKey); assertEncryptionContextThrowsForWrites(IllegalStateException.class, - "Encryption for family 'test-family' configured with type 'AES' but key specifies algorithm 'DES'"); + "Encryption for family 'test-family' configured with type 'AES' but key specifies " + + "algorithm 'DES'"); } @Test diff --git a/hbase-shell/src/main/ruby/shell/commands/rotate_stk.rb b/hbase-shell/src/main/ruby/shell/commands/rotate_stk.rb index 2a8f4306ad5c..f1c754487c40 100644 --- a/hbase-shell/src/main/ruby/shell/commands/rotate_stk.rb +++ b/hbase-shell/src/main/ruby/shell/commands/rotate_stk.rb @@ -49,4 +49,3 @@ def command end end end - diff --git a/hbase-shell/src/test/ruby/tests_runner.rb b/hbase-shell/src/test/ruby/tests_runner.rb index 4c93d8d872ba..8f51a168a011 100644 --- a/hbase-shell/src/test/ruby/tests_runner.rb +++ b/hbase-shell/src/test/ruby/tests_runner.rb @@ -40,9 +40,8 @@ end files = Dir[ File.dirname(__FILE__) + "/" + test_suite_pattern ] -if files.empty? - raise "No tests found for #{test_suite_pattern}" -end +raise "No tests found for #{test_suite_pattern}" if files.empty? + files.each do |file| filename = File.basename(file) if includes != nil && !includes.include?(filename)