steveloughran commented on code in PR #6140:
URL: https://github.com/apache/hadoop/pull/6140#discussion_r1342962593
##########
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/EncryptionTestUtils.java:
##########
@@ -94,6 +96,13 @@ public static void assertEncrypted(S3AFileSystem fs,
kmsKeyArn,
md.ssekmsKeyId());
break;
+ case DSSE_KMS:
+ assertEquals("Wrong algorithm in " + details,
Review Comment:
prefer assertJ for all new test cases. save time by doing it first time round
##########
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AEncryptionDSSEKMSUserDefinedKey.java:
##########
@@ -0,0 +1,55 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.fs.s3a;
+
+import org.apache.commons.lang.StringUtils;
+import org.apache.hadoop.conf.Configuration;
+
+import static org.apache.hadoop.fs.contract.ContractTestUtils.skip;
+import static org.apache.hadoop.fs.s3a.Constants.S3_ENCRYPTION_KEY;
+import static org.apache.hadoop.fs.s3a.S3AEncryptionMethods.DSSE_KMS;
+import static org.apache.hadoop.fs.s3a.S3ATestUtils.getTestBucketName;
+
+/**
+ * Concrete class that extends {@link AbstractTestS3AEncryption}
+ * and tests DSSE-KMS encryption.
+ */
+public class ITestS3AEncryptionDSSEKMSUserDefinedKey
+ extends AbstractTestS3AEncryption {
+
+ @Override
+ protected Configuration createConfiguration() {
+ // get the KMS key for this test.
+ Configuration c = new Configuration();
+ String kmsKey = S3AUtils.getS3EncryptionKey(getTestBucketName(c), c);
+ // skip the test if DSSE-KMS or KMS key not set.
+ if (StringUtils.isBlank(kmsKey)) {
+ skip(S3_ENCRYPTION_KEY + " is not set for " +
Review Comment:
maybe S3ATestUtils.skipIfEncryptionNotSet(); else assume(string, bool)
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/impl/RequestFactoryImpl.java:
##########
@@ -354,6 +358,10 @@ private void
putEncryptionParameters(PutObjectRequest.Builder putObjectRequestBu
// Set the KMS key if present, else S3 uses AWS managed key.
EncryptionSecretOperations.getSSEAwsKMSKey(encryptionSecrets)
.ifPresent(kmsKey -> putObjectRequestBuilder.ssekmsKeyId(kmsKey));
+ } else if (S3AEncryptionMethods.DSSE_KMS == algorithm) {
Review Comment:
how about we move to a switch() here and add a default for unknown stuff
##########
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3ADSSEEncryptionWithDefaultS3Settings.java:
##########
@@ -0,0 +1,167 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.fs.s3a;
+
+import java.io.IOException;
+
+import org.junit.Ignore;
+import org.junit.Test;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.contract.ContractTestUtils;
+import org.apache.hadoop.fs.s3a.auth.delegation.EncryptionSecrets;
+
+import static org.apache.hadoop.fs.contract.ContractTestUtils.dataset;
+import static org.apache.hadoop.fs.contract.ContractTestUtils.skip;
+import static org.apache.hadoop.fs.contract.ContractTestUtils.writeDataset;
+import static org.apache.hadoop.fs.s3a.Constants.S3_ENCRYPTION_ALGORITHM;
+import static
org.apache.hadoop.fs.s3a.Constants.SERVER_SIDE_ENCRYPTION_ALGORITHM;
+import static
org.apache.hadoop.fs.s3a.EncryptionTestUtils.AWS_KMS_DSSE_ALGORITHM;
+import static org.apache.hadoop.fs.s3a.S3AEncryptionMethods.DSSE_KMS;
+import static org.apache.hadoop.fs.s3a.S3ATestUtils.getTestBucketName;
+import static
org.apache.hadoop.fs.s3a.S3ATestUtils.removeBaseAndBucketOverrides;
+import static org.apache.hadoop.fs.s3a.S3ATestUtils.skipIfEncryptionNotSet;
+import static org.apache.hadoop.fs.s3a.S3AUtils.getS3EncryptionKey;
+
+/**
+ * Concrete class that extends {@link AbstractTestS3AEncryption}
+ * and tests already configured bucket level DSSE encryption using s3 console.
+ */
+public class ITestS3ADSSEEncryptionWithDefaultS3Settings extends
+ AbstractTestS3AEncryption {
+
+ @Override
+ public void setup() throws Exception {
+ super.setup();
+ // get the KMS key for this test.
+ S3AFileSystem fs = getFileSystem();
+ Configuration c = fs.getConf();
+ skipIfEncryptionNotSet(c, getSSEAlgorithm());
+ }
+
+ @SuppressWarnings("deprecation")
+ @Override
+ protected void patchConfigurationEncryptionSettings(
+ final Configuration conf) {
+ removeBaseAndBucketOverrides(conf,
+ S3_ENCRYPTION_ALGORITHM,
+ SERVER_SIDE_ENCRYPTION_ALGORITHM);
+ conf.set(S3_ENCRYPTION_ALGORITHM,
+ getSSEAlgorithm().getMethod());
+ }
+
+ /**
+ * Setting this to NONE as we don't want to overwrite
+ * already configured encryption settings.
+ * @return the algorithm
+ */
+ @Override
+ protected S3AEncryptionMethods getSSEAlgorithm() {
+ return S3AEncryptionMethods.NONE;
+ }
+
+ /**
+ * The check here is that the object is encrypted
+ * <i>and</i> that the encryption key is the KMS key
+ * provided, not any default key.
+ * @param path path
+ */
+ @Override
+ protected void assertEncrypted(Path path) throws IOException {
+ S3AFileSystem fs = getFileSystem();
+ Configuration c = fs.getConf();
+ String kmsKey = getS3EncryptionKey(getTestBucketName(c), c);
+ EncryptionTestUtils.assertEncrypted(fs, path, DSSE_KMS, kmsKey);
+ }
+
+ @Override
+ @Ignore
+ @Test
+ public void testEncryptionSettingPropagation() throws Throwable {
+ }
+
+ @Override
+ @Ignore
+ @Test
+ public void testEncryption() throws Throwable {
+ }
+
+ /**
+ * Skipping if the test bucket is not configured with
+ * aws:kms encryption algorithm.
+ */
+ @Override
+ public void testEncryptionOverRename() throws Throwable {
+ skipIfBucketNotKmsEncrypted();
+ super.testEncryptionOverRename();
+ }
+
+ /**
+ * If the test bucket is not configured with aws:kms encryption algorithm,
+ * skip the test.
+ *
+ * @throws IOException If the object creation/deletion/access fails.
+ */
+ private void skipIfBucketNotKmsEncrypted() throws IOException {
+ S3AFileSystem fs = getFileSystem();
+ Path path = path(getMethodName() + "find-encryption-algo");
Review Comment:
just use methodPath()
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
