Skip to content

Commit 4bda028

Browse files
jojochuangXiao Chen
authored andcommitted
HADOOP-14563. LoadBalancingKMSClientProvider#warmUpEncryptedKeys swallows IOException. Contributed by Rushabh S Shah.
(cherry picked from commit 8153fe2) (cherry picked from commit 730b21e) (cherry picked from commit d657c05) Change-Id: Ib97f32f19ab0fb3a74f19b0f7f0860889fd765e8
1 parent 192a0e6 commit 4bda028

File tree

2 files changed

+74
-1
lines changed

2 files changed

+74
-1
lines changed

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/key/kms/LoadBalancingKMSClientProvider.java

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
import org.slf4j.LoggerFactory;
3939

4040
import com.google.common.annotations.VisibleForTesting;
41+
import com.google.common.base.Preconditions;
4142

4243
/**
4344
* A simple LoadBalancing KMSClientProvider that round-robins requests
@@ -158,15 +159,24 @@ public Void call(KMSClientProvider provider) throws IOException {
158159
// This request is sent to all providers in the load-balancing group
159160
@Override
160161
public void warmUpEncryptedKeys(String... keyNames) throws IOException {
162+
Preconditions.checkArgument(providers.length > 0,
163+
"No providers are configured");
164+
boolean success = false;
165+
IOException e = null;
161166
for (KMSClientProvider provider : providers) {
162167
try {
163168
provider.warmUpEncryptedKeys(keyNames);
169+
success = true;
164170
} catch (IOException ioe) {
171+
e = ioe;
165172
LOG.error(
166173
"Error warming up keys for provider with url"
167-
+ "[" + provider.getKMSUrl() + "]");
174+
+ "[" + provider.getKMSUrl() + "]", ioe);
168175
}
169176
}
177+
if (!success && e != null) {
178+
throw e;
179+
}
170180
}
171181

172182
// This request is sent to all providers in the load-balancing group

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/crypto/key/kms/TestLoadBalancingKMSClientProvider.java

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import org.apache.hadoop.crypto.key.KeyProvider.Options;
3535
import org.apache.hadoop.crypto.key.KeyProviderCryptoExtension;
3636
import org.apache.hadoop.security.authentication.client.AuthenticationException;
37+
import org.apache.hadoop.security.authorize.AuthorizationException;
3738
import org.junit.Test;
3839
import org.mockito.Mockito;
3940

@@ -257,4 +258,66 @@ public void testClassCastException() throws Exception {
257258
"AuthenticationException"));
258259
}
259260
}
261+
262+
/**
263+
* tests {@link LoadBalancingKMSClientProvider#warmUpEncryptedKeys(String...)}
264+
* error handling in case when all the providers throws {@link IOException}.
265+
* @throws Exception
266+
*/
267+
@Test
268+
public void testWarmUpEncryptedKeysWhenAllProvidersFail() throws Exception {
269+
Configuration conf = new Configuration();
270+
KMSClientProvider p1 = mock(KMSClientProvider.class);
271+
String keyName = "key1";
272+
Mockito.doThrow(new IOException(new AuthorizationException("p1"))).when(p1)
273+
.warmUpEncryptedKeys(Mockito.anyString());
274+
KMSClientProvider p2 = mock(KMSClientProvider.class);
275+
Mockito.doThrow(new IOException(new AuthorizationException("p2"))).when(p2)
276+
.warmUpEncryptedKeys(Mockito.anyString());
277+
278+
when(p1.getKMSUrl()).thenReturn("p1");
279+
when(p2.getKMSUrl()).thenReturn("p2");
280+
281+
LoadBalancingKMSClientProvider kp = new LoadBalancingKMSClientProvider(
282+
new KMSClientProvider[] {p1, p2}, 0, conf);
283+
try {
284+
kp.warmUpEncryptedKeys(keyName);
285+
fail("Should fail since both providers threw IOException");
286+
} catch (Exception e) {
287+
assertTrue(e.getCause() instanceof IOException);
288+
}
289+
Mockito.verify(p1, Mockito.times(1)).warmUpEncryptedKeys(keyName);
290+
Mockito.verify(p2, Mockito.times(1)).warmUpEncryptedKeys(keyName);
291+
}
292+
293+
/**
294+
* tests {@link LoadBalancingKMSClientProvider#warmUpEncryptedKeys(String...)}
295+
* error handling in case atleast one provider succeeds.
296+
* @throws Exception
297+
*/
298+
@Test
299+
public void testWarmUpEncryptedKeysWhenOneProviderSucceeds()
300+
throws Exception {
301+
Configuration conf = new Configuration();
302+
KMSClientProvider p1 = mock(KMSClientProvider.class);
303+
String keyName = "key1";
304+
Mockito.doThrow(new IOException(new AuthorizationException("p1"))).when(p1)
305+
.warmUpEncryptedKeys(Mockito.anyString());
306+
KMSClientProvider p2 = mock(KMSClientProvider.class);
307+
Mockito.doNothing().when(p2)
308+
.warmUpEncryptedKeys(Mockito.anyString());
309+
310+
when(p1.getKMSUrl()).thenReturn("p1");
311+
when(p2.getKMSUrl()).thenReturn("p2");
312+
313+
LoadBalancingKMSClientProvider kp = new LoadBalancingKMSClientProvider(
314+
new KMSClientProvider[] {p1, p2}, 0, conf);
315+
try {
316+
kp.warmUpEncryptedKeys(keyName);
317+
} catch (Exception e) {
318+
fail("Should not throw Exception since p2 doesn't throw Exception");
319+
}
320+
Mockito.verify(p1, Mockito.times(1)).warmUpEncryptedKeys(keyName);
321+
Mockito.verify(p2, Mockito.times(1)).warmUpEncryptedKeys(keyName);
322+
}
260323
}

0 commit comments

Comments
 (0)