Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 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 @@ -105,6 +105,7 @@ public synchronized void teardown() throws Exception {
@Test
public void testCacheFileExistence() throws Throwable {
describe("Verify that FS cache files exist on local FS");
skipIfClientSideEncryption();

try (FSDataInputStream in = fs.open(testFile)) {
byte[] buffer = new byte[prefetchBlockSize];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ private static int calculateNumBlocks(long largeFileSize, int blockSize) {
@Test
public void testReadLargeFileFully() throws Throwable {
describe("read a large file fully, uses S3ACachingInputStream");
skipIfClientSideEncryption();
Copy link
Contributor

Choose a reason for hiding this comment

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

Shall we just move these into openFS() since we're assuming the FS it provides is not compatible with CSE for now?

IOStatistics ioStats;
openFS();

Expand Down Expand Up @@ -151,6 +152,7 @@ public void testReadLargeFileFully() throws Throwable {
public void testReadLargeFileFullyLazySeek() throws Throwable {
describe("read a large file using readFully(position,buffer,offset,length),"
+ " uses S3ACachingInputStream");
skipIfClientSideEncryption();
IOStatistics ioStats;
openFS();

Expand Down Expand Up @@ -182,6 +184,7 @@ public void testReadLargeFileFullyLazySeek() throws Throwable {
@Test
public void testRandomReadLargeFile() throws Throwable {
describe("random read on a large file, uses S3ACachingInputStream");
skipIfClientSideEncryption();
IOStatistics ioStats;
openFS();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ protected Configuration createConfiguration() {
@Test
public void testRequesterPaysOptionSuccess() throws Throwable {
describe("Test requester pays enabled case by reading last then first byte");

skipIfClientSideEncryption();
Configuration conf = this.createConfiguration();
conf.setBoolean(ALLOW_REQUESTER_PAYS, true);
// Enable bucket exists check, the first failure point people may encounter
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -448,7 +448,7 @@ public void testReadOnlyOperations() throws Throwable {
policy(
statement(false, S3_ALL_BUCKETS, S3_PATH_WRITE_OPERATIONS),
STATEMENT_ALL_S3,
STATEMENT_ALLOW_SSE_KMS_READ));
STATEMENT_ALLOW_SSE_KMS_RW));
Copy link
Contributor

Choose a reason for hiding this comment

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

Why does this need changing? It's for Server Side Encryption. (Was it always broken?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, think it may have always been broken when CSE is enabled. Without RW access, encryption client is not able to generate the key required to encrypt for a PUT. So the test fails with a "unable to generate" key exception.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh yes... We also saw this error and this is the fix.
CC @mehakmeet

Copy link
Contributor

Choose a reason for hiding this comment

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

nit: We can maybe rename this constant to STATEMENT_ALLOW_KMS_RW since kms can be used server-side or client-side and both would apply the same policy.

Path path = methodPath();
roleFS = (S3AFileSystem) path.getFileSystem(conf);
// list the root path, expect happy
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ public void testLandsatBucketRequireGuarded() throws Throwable {

@Test
public void testLandsatBucketRequireUnencrypted() throws Throwable {
skipIfClientSideEncryption();
run(BucketInfo.NAME,
"-" + BucketInfo.ENCRYPTION_FLAG, "none",
getLandsatCSVFile(getConfiguration()));
Expand Down Expand Up @@ -178,8 +179,9 @@ public void testUploadListByAge() throws Throwable {
// least a second old
describe("Sleeping 1 second then confirming upload still there");
Thread.sleep(1000);
LambdaTestUtils.eventually(5000, 1000,
() -> { assertNumUploadsAge(path, 1, 1); });
LambdaTestUtils.eventually(5000, 1000, () -> {
assertNumUploadsAge(path, 1, 1);
});

// 7. Assert deletion works when age filter matches
describe("Doing aged deletion");
Expand Down Expand Up @@ -231,8 +233,8 @@ private void assertNumDeleted(S3AFileSystem fs, Path path, int numDeleted)
* search all parts
* @throws Exception on failure
*/
private void uploadCommandAssertCount(S3AFileSystem fs, String options[],
Path path, int numUploads, int ageSeconds)
private void uploadCommandAssertCount(S3AFileSystem fs, String[] options, Path path,
int numUploads, int ageSeconds)
throws Exception {
List<String> allOptions = new ArrayList<>();
List<String> output = new ArrayList<>();
Expand Down