[
https://issues.apache.org/jira/browse/HADOOP-17871?focusedWorklogId=648602&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-648602
]
ASF GitHub Bot logged work on HADOOP-17871:
-------------------------------------------
Author: ASF GitHub Bot
Created on: 09/Sep/21 13:32
Start Date: 09/Sep/21 13:32
Worklog Time Spent: 10m
Work Description: steveloughran commented on a change in pull request
#3412:
URL: https://github.com/apache/hadoop/pull/3412#discussion_r705321302
##########
File path:
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java
##########
@@ -436,7 +444,7 @@ public void initialize(URI name, Configuration originalConf)
initializeStatisticsBinding();
// If CSE-KMS method is set then CSE is enabled.
isCSEEnabled = S3AUtils.lookupPassword(conf,
Review comment:
this flag will be true if any algorithm is set. Is this what you want?
##########
File path:
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/Constants.java
##########
@@ -449,9 +450,32 @@ private Constants() {
* May be set within a JCEKS file.
* Value: "{@value}".
*/
+ @Deprecated
public static final String SERVER_SIDE_ENCRYPTION_KEY =
"fs.s3a.server-side-encryption.key";
+ /**
+ * Set S3-server side encryption(SSE) or S3-Client side encryption(CSE)
+ * algorithm. Check {@link S3AEncryptionMethods} for valid options.
+ * <br>
+ * value: {@value}
+ */
+ public static final String S3_ENCRYPTION_ALGORITHM =
+ "fs.s3a.encryption-algorithm";
Review comment:
I think we should call this "fs.s3a.encryption.algorithm" for
consistency with the key.
It's a bigger change than before, but we could also add a deprecation entry
for fs.s3a.encryption-algorithm in case people got it wrong
##########
File path:
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/S3GuardTool.java
##########
@@ -39,6 +39,8 @@
import java.util.stream.Collectors;
import com.amazonaws.services.s3.model.MultipartUpload;
+
+import org.apache.hadoop.fs.s3a.Constants;
Review comment:
nit: move to the "real" org.apache imports
##########
File path:
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/auth/delegation/ITestSessionDelegationInFileystem.java
##########
@@ -142,22 +143,22 @@ protected Configuration createConfiguration() {
assumeSessionTestsEnabled(conf);
disableFilesystemCaching(conf);
String s3EncryptionMethod =
- conf.getTrimmed(SERVER_SIDE_ENCRYPTION_ALGORITHM,
+ conf.getTrimmed(Constants.S3_ENCRYPTION_ALGORITHM,
S3AEncryptionMethods.SSE_KMS.getMethod());
- String s3EncryptionKey = conf.getTrimmed(SERVER_SIDE_ENCRYPTION_KEY, "");
+ String s3EncryptionKey = conf.getTrimmed(Constants.S3_ENCRYPTION_KEY, "");
removeBaseAndBucketOverrides(conf,
DELEGATION_TOKEN_BINDING,
- SERVER_SIDE_ENCRYPTION_ALGORITHM,
- SERVER_SIDE_ENCRYPTION_KEY);
+ Constants.S3_ENCRYPTION_ALGORITHM,
Review comment:
include the old option names
##########
File path:
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/TestSSEConfiguration.java
##########
@@ -181,14 +181,14 @@ private S3AEncryptionMethods getAlgorithm(String
algorithm, String key)
private Configuration buildConf(String algorithm, String key) {
Configuration conf = emptyConf();
if (algorithm != null) {
- conf.set(SERVER_SIDE_ENCRYPTION_ALGORITHM, algorithm);
+ conf.set(Constants.S3_ENCRYPTION_ALGORITHM, algorithm);
} else {
- conf.unset(SERVER_SIDE_ENCRYPTION_ALGORITHM);
+ conf.unset(Constants.S3_ENCRYPTION_ALGORITHM);
Review comment:
unset the old option too
##########
File path:
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/auth/delegation/ITestSessionDelegationInFileystem.java
##########
@@ -348,8 +349,8 @@ public void testDelegatedFileSystem() throws Throwable {
// this is to simulate better a remote deployment.
removeBaseAndBucketOverrides(bucket, conf,
ACCESS_KEY, SECRET_KEY, SESSION_TOKEN,
- SERVER_SIDE_ENCRYPTION_ALGORITHM,
- SERVER_SIDE_ENCRYPTION_KEY,
+ Constants.S3_ENCRYPTION_ALGORITHM,
Review comment:
+old option names
##########
File path:
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/scale/ITestS3AHugeFilesSSECDiskBlocks.java
##########
@@ -48,12 +48,12 @@ public void setup() throws Exception {
@Override
protected Configuration createScaleConfiguration() {
Configuration conf = super.createScaleConfiguration();
- removeBaseAndBucketOverrides(conf, SERVER_SIDE_ENCRYPTION_KEY,
- SERVER_SIDE_ENCRYPTION_ALGORITHM);
+ removeBaseAndBucketOverrides(conf, S3_ENCRYPTION_KEY,
Review comment:
+old ones
##########
File path:
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/S3ATestUtils.java
##########
@@ -1533,14 +1534,20 @@ public static S3AFileStatus innerGetFileStatus(
}
/**
- * Skip a test if CSE KMS key id is not set.
+ * Skip a test if encryption algo or encryption key is not set.
Review comment:
nit: algorithm
--
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]
Issue Time Tracking
-------------------
Worklog Id: (was: 648602)
Remaining Estimate: 0h
Time Spent: 10m
> S3A CSE: minor tuning
> ---------------------
>
> Key: HADOOP-17871
> URL: https://issues.apache.org/jira/browse/HADOOP-17871
> Project: Hadoop Common
> Issue Type: Sub-task
> Components: fs/s3
> Affects Versions: 3.4.0
> Reporter: Steve Loughran
> Assignee: Mehakmeet Singh
> Priority: Minor
> Time Spent: 10m
> Remaining Estimate: 0h
>
> Some minor tuning to the CSE encryption support before backporting to 3.3.x
> and so shipping this year
> * LogExactlyOnce an "please ignore the warning" message to a new log
> ("org.apache.hadoop.fs.s3a.encryption") which can be set to ERROR if you get
> bored of the message.
> * Extend testing_s3a.md and the SDK upgrade runbook: test CSE always
> * change property name of encryption key (maybe: fs.s3a.encryption) and add
> mapping in S3AFileSystem.addDeprecatedKeys ... docs will need updating too.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
