Skip to content

HADOOP-17823. S3A Tests to skip if S3Guard and S3-CSE are enabled #3263

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Aug 5, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,7 @@
import static org.apache.hadoop.fs.s3a.impl.ErrorTranslation.isObjectNotFound;
import static org.apache.hadoop.fs.s3a.impl.ErrorTranslation.isUnknownBucket;
import static org.apache.hadoop.fs.s3a.impl.InternalConstants.CSE_PADDING_LENGTH;
import static org.apache.hadoop.fs.s3a.impl.InternalConstants.CSE_S3GUARD_INCOMPATIBLE;
import static org.apache.hadoop.fs.s3a.impl.InternalConstants.DEFAULT_UPLOAD_PART_COUNT_LIMIT;
import static org.apache.hadoop.fs.s3a.impl.InternalConstants.DELETE_CONSIDERED_IDEMPOTENT;
import static org.apache.hadoop.fs.s3a.impl.InternalConstants.SC_404;
Expand Down Expand Up @@ -540,8 +541,7 @@ public void initialize(URI name, Configuration originalConf)
LOG.debug("Using metadata store {}, authoritative store={}, authoritative path={}",
getMetadataStore(), allowAuthoritativeMetadataStore, allowAuthoritativePaths);
if (isCSEEnabled) {
throw new PathIOException(uri.toString(), "S3-CSE cannot be used "
+ "with S3Guard");
throw new PathIOException(uri.toString(), CSE_S3GUARD_INCOMPATIBLE);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,4 +134,9 @@ private InternalConstants() {
*/
public static final int CSE_PADDING_LENGTH = 16;

/**
* Error message to indicate S3-CSE is incompatible with S3Guard.
*/
public static final String CSE_S3GUARD_INCOMPATIBLE = "S3-CSE cannot be "
+ "used with S3Guard";
}
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,8 @@ protected AbstractFSContract createContract(Configuration conf) {
public void teardown() throws Exception {
super.teardown();
S3AFileSystem fs = getFileSystem();
if (fs.getConf().getBoolean(FS_S3A_IMPL_DISABLE_CACHE, false)) {
if (fs != null && fs.getConf().getBoolean(FS_S3A_IMPL_DISABLE_CACHE,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these null check required?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

they are. IF test setup() fails then fs may be null, teardown will be called and you'll see the error in teardown (here an NPE) rather than whatever caused setup failure.

false)) {
fs.close();
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,17 @@

package org.apache.hadoop.fs.contract.s3a;

import java.io.IOException;

import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.Path;
import org.apache.hadoop.fs.PathIOException;
import org.apache.hadoop.fs.contract.AbstractBondedFSContract;
import org.apache.hadoop.fs.s3a.S3AFileSystem;
import org.apache.hadoop.fs.s3a.S3ATestUtils;

import static org.apache.hadoop.fs.s3a.S3ATestUtils.maybeSkipIfS3GuardAndS3CSEIOE;

/**
* The contract of S3A: only enabled if the test bucket is provided.
*/
Expand Down Expand Up @@ -63,6 +68,20 @@ public S3AContract(Configuration conf, boolean addContractResource) {
}
}

/**
* Skip S3AFS initialization if S3-CSE and S3Guard are enabled.
*
*/
@Override
public void init() throws IOException {
try {
super.init();
} catch (PathIOException ioe) {
// Skip the tests if S3-CSE and S3-Guard are enabled.
maybeSkipIfS3GuardAndS3CSEIOE(ioe);
}
}

@Override
public String getScheme() {
return "s3a";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ public void setup() throws Exception {
Configuration conf = createConfiguration();
fs = new S3AFileSystem();
URI uri = URI.create(FS_S3A + "://" + BUCKET);
// unset S3CSE property from config to avoid pathIOE.
conf.unset(SERVER_SIDE_ENCRYPTION_ALGORITHM);
fs.initialize(uri, conf);
s3 = fs.getAmazonS3ClientForTesting("mocking");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
*/
public class ITestS3AFSMainOperations extends FSMainOperationsBaseTest {

private S3AContract contract;

public ITestS3AFSMainOperations() {
super(createTestPath(
Expand All @@ -41,11 +42,18 @@ public ITestS3AFSMainOperations() {

@Override
protected FileSystem createFileSystem() throws Exception {
S3AContract contract = new S3AContract(new Configuration());
contract = new S3AContract(new Configuration());
contract.init();
return contract.getTestFileSystem();
}

@Override
public void tearDown() throws Exception {
if (contract.getTestFileSystem() != null) {
super.tearDown();
}
}

@Override
@Ignore("Permissions not supported")
public void testListStatusThrowsExceptionForUnreadableDir() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ public void setup() throws Exception {

@Override
public void teardown() throws Exception {
if (getFileSystem()
if (getFileSystem() != null && getFileSystem()
.getAmazonS3Client() instanceof InconsistentAmazonS3Client) {
clearInconsistency(getFileSystem());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import org.apache.hadoop.fs.FileSystem;
import org.apache.hadoop.fs.LocatedFileStatus;
import org.apache.hadoop.fs.Path;
import org.apache.hadoop.fs.PathIOException;
import org.apache.hadoop.fs.RemoteIterator;
import org.apache.hadoop.fs.permission.FsPermission;
import org.apache.hadoop.fs.s3a.auth.MarshalledCredentialBinding;
Expand All @@ -37,6 +38,7 @@

import org.apache.hadoop.fs.s3a.impl.ChangeDetectionPolicy;
import org.apache.hadoop.fs.s3a.impl.ContextAccessors;
import org.apache.hadoop.fs.s3a.impl.InternalConstants;
import org.apache.hadoop.fs.s3a.impl.StatusProbeEnum;
import org.apache.hadoop.fs.s3a.impl.StoreContext;
import org.apache.hadoop.fs.s3a.impl.StoreContextBuilder;
Expand Down Expand Up @@ -186,6 +188,8 @@ public static S3AFileSystem createTestFileSystem(Configuration conf,
// make this whole class not run by default
Assume.assumeTrue("No test filesystem in " + TEST_FS_S3A_NAME,
liveTest);
// Skip if S3Guard and S3-CSE are enabled.
skipIfS3GuardAndS3CSEEnabled(conf);
// patch in S3Guard options
maybeEnableS3Guard(conf);
S3AFileSystem fs1 = new S3AFileSystem();
Expand Down Expand Up @@ -229,12 +233,45 @@ public static FileContext createTestFileContext(Configuration conf)
// make this whole class not run by default
Assume.assumeTrue("No test filesystem in " + TEST_FS_S3A_NAME,
liveTest);
// Skip if S3Guard and S3-CSE are enabled.
skipIfS3GuardAndS3CSEEnabled(conf);
// patch in S3Guard options
maybeEnableS3Guard(conf);
FileContext fc = FileContext.getFileContext(testURI, conf);
return fc;
}

/**
* Skip if S3Guard and S3CSE are enabled together.
*
* @param conf Test Configuration.
*/
private static void skipIfS3GuardAndS3CSEEnabled(Configuration conf) {
String encryptionMethod =
conf.getTrimmed(SERVER_SIDE_ENCRYPTION_ALGORITHM, "");
String metaStore = conf.getTrimmed(S3_METADATA_STORE_IMPL, "");
if (encryptionMethod.equals(S3AEncryptionMethods.CSE_KMS.getMethod()) &&
!metaStore.equals(S3GUARD_METASTORE_NULL)) {
skip("Skipped if CSE is enabled with S3Guard.");
}
}

/**
* Either skip if PathIOE occurred due to S3CSE and S3Guard
* incompatibility or throw the PathIOE.
*
* @param ioe PathIOE being parsed.
* @throws PathIOException Throws PathIOE if it doesn't relate to S3CSE
* and S3Guard incompatibility.
*/
public static void maybeSkipIfS3GuardAndS3CSEIOE(PathIOException ioe)
throws PathIOException {
if (ioe.toString().contains(InternalConstants.CSE_S3GUARD_INCOMPATIBLE)) {
skip("Skipping since CSE is enabled with S3Guard.");
}
throw ioe;
}

/**
* Get a long test property.
* <ol>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,10 @@ public void setUp() throws IOException, Exception {
super.setUp();
}

@Override
public void tearDown() throws Exception {
if (fc != null) {
super.tearDown();
}
}
}