[
https://issues.apache.org/jira/browse/HADOOP-18708?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17896310#comment-17896310
]
ASF GitHub Bot commented on HADOOP-18708:
-----------------------------------------
steveloughran commented on code in PR #6884:
URL: https://github.com/apache/hadoop/pull/6884#discussion_r1832652535
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/impl/ErrorTranslation.java:
##########
@@ -105,6 +112,54 @@ private static Throwable getInnermostThrowable(Throwable
thrown, Throwable outer
return getInnermostThrowable(thrown.getCause(), thrown);
}
+ /**
+ * Attempts to extract the underlying SdkException from an S3 encryption
client exception.
+ *
+ * <p>This method is designed to handle exceptions that may be wrapped within
+ * S3EncryptionClientExceptions. It performs the following steps:
+ * <ol>
+ * <li>Checks if the input exception is null.</li>
+ * <li>Verifies if the exception contains the S3EncryptionClientException
signature.</li>
+ * <li>Examines the cause chain to find the most relevant
SdkException.</li>
+ * </ol>
+ *
+ * <p>The method aims to unwrap nested exceptions to provide more meaningful
+ * error information, particularly in the context of S3 encryption
operations.
+ *
+ * @param exception The SdkException to analyze. This may be a wrapper
exception
+ * containing a more specific underlying cause.
+ * @return The extracted SdkException if found within the exception chain,
+ * or the original exception if no relevant nested exception is
found.
+ * Returns null if the input exception is null.
+ *
+ * @see SdkException
+ * @see AwsServiceException
+ */
+ public static SdkException
maybeExtractSdkExceptionFromEncryptionClientException(
Review Comment:
this is a bit long. How about `maybeProcessEncryptionClientException()`
this says it is handled, without saying exactly what happens.
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/impl/EncryptionS3ClientFactory.java:
##########
@@ -260,21 +274,32 @@ private S3AsyncClient
createS3AsyncEncryptionClient(S3ClientCreationParameters p
return s3EncryptionAsyncClientBuilder.build();
}
-
/**
- * Retrieves an instance of the Keyring provider based on the provided class
name.
+ * Creates and returns a Keyring provider instance based on the given class
name.
+ *
+ * <p>This method attempts to instantiate a Keyring provider using
reflection. It first tries
+ * to create an instance using the standard ReflectionUtils.newInstance
method. If that fails,
+ * it falls back to an alternative instantiation method, which is primarily
used for testing
+ * purposes (specifically for CustomKeyring.java).
*
- * @param className The fully qualified class name of the Keyring provider
implementation.
- * @param conf The Configuration object containing the necessary
configuration properties.
- * @return An instance of the Keyring provider.
+ * @param className The fully qualified class name of the Keyring provider
to instantiate.
+ * @param conf The Configuration object to be passed to the Keyring provider
constructor.
+ * @return An instance of the specified Keyring provider.
+ * @throws RuntimeException If unable to create the Keyring provider
instance.
*/
- private Keyring getKeyringProvider(String className, Configuration conf) {
+ private Keyring getKeyringProvider(String className, Configuration conf) {
+ Class<? extends Keyring> keyringProviderClass =
getCustomKeyringProviderClass(className);
try {
- return
ReflectionUtils.newInstance(getCustomKeyringProviderClass(className), conf);
+ return ReflectionUtils.newInstance(keyringProviderClass, conf);
} catch (Exception e) {
- // this is for testing purpose to support CustomKeyring.java
- return
ReflectionUtils.newInstance(getCustomKeyringProviderClass(className), conf,
- new Class[] {Configuration.class}, conf);
+ LOG.warn("Failed to create Keyring provider", e);
+ // This is for testing purposes to support CustomKeyring.java
+ try {
+ return ReflectionUtils.newInstance(keyringProviderClass, conf,
+ new Class[] {Configuration.class}, conf);
+ } catch (Exception ex) {
+ throw new RuntimeException("Failed to create Keyring provider", ex);
Review Comment:
same comment about including the wrapped exception text
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/impl/EncryptionS3ClientFactory.java:
##########
@@ -246,8 +255,13 @@ private S3AsyncClient
createS3AsyncEncryptionClient(S3ClientCreationParameters p
s3EncryptionAsyncClientBuilder.cryptoMaterialsManager(kmsCryptoMaterialsManager);
break;
case CUSTOM:
- Keyring keyring =
getKeyringProvider(cseMaterials.getCustomKeyringClassName(),
- cseMaterials.getConf());
+ Keyring keyring;
+ try {
+ keyring =
+ getKeyringProvider(cseMaterials.getCustomKeyringClassName(),
cseMaterials.getConf());
+ } catch (RuntimeException e) {
+ throw new IOException("Failed to instantiate a custom keyring
provider", e);
Review Comment:
can you include the error text of e in the new IOE; it's really easy to lose
the base exception in log stack chains with deep wrapping/rewrapping.
{code}
IOException("Failed to instantiate a custom keyring provider: " + e, e)
{code}
> AWS SDK V2 - Implement CSE
> --------------------------
>
> Key: HADOOP-18708
> URL: https://issues.apache.org/jira/browse/HADOOP-18708
> Project: Hadoop Common
> Issue Type: Sub-task
> Components: fs/s3
> Affects Versions: 3.4.0
> Reporter: Ahmar Suhail
> Assignee: Syed Shameerur Rahman
> Priority: Major
> Labels: pull-request-available
>
> S3 Encryption client for SDK V2 is now available, so add client side
> encryption back in.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
