Skip to content
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

🐛 Fix S3 destination integration tests to use bucket path #18031

Merged
merged 13 commits into from
Oct 25, 2022
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,8 @@ public AirbyteConnectionStatus check(final JsonNode config) {
try {
final S3DestinationConfig destinationConfig = configFactory.getS3DestinationConfig(config, storageProvider());
final AmazonS3 s3Client = destinationConfig.getS3Client();
final S3StorageOperations storageOperations = new S3StorageOperations(nameTransformer, s3Client, destinationConfig);

S3BaseChecks.attemptS3WriteAndDelete(storageOperations, destinationConfig, destinationConfig.getBucketName());
S3BaseChecks.testIAMUserHasListObjectPermission(s3Client, destinationConfig.getBucketName());
S3BaseChecks.testSingleUpload(s3Client, destinationConfig.getBucketName(), destinationConfig.getBucketPath());
S3BaseChecks.testMultipartUpload(s3Client, destinationConfig.getBucketName(), destinationConfig.getBucketPath());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,8 @@ public static void attemptS3WriteAndDelete(final S3StorageOperations storageOper

public static void testSingleUpload(final AmazonS3 s3Client, final String bucketName, final String bucketPath) {
LOGGER.info("Started testing if all required credentials assigned to user for single file uploading");
if (bucketPath.endsWith("/")) {
throw new RuntimeException("Bucket Path should not end with /");
}
final String testFile = bucketPath + "/" + "test_" + System.currentTimeMillis();
final var prefix = bucketPath.endsWith("/") ? bucketPath : bucketPath + "/";
final String testFile = prefix + "test_" + System.currentTimeMillis();
try {
s3Client.putObject(bucketName, testFile, "this is a test file");
} finally {
Expand All @@ -51,10 +49,8 @@ public static void testSingleUpload(final AmazonS3 s3Client, final String bucket

public static void testMultipartUpload(final AmazonS3 s3Client, final String bucketName, final String bucketPath) throws IOException {
LOGGER.info("Started testing if all required credentials assigned to user for multipart upload");
if (bucketPath.endsWith("/")) {
throw new RuntimeException("Bucket Path should not end with /");
}
final String testFile = bucketPath + "/" + "test_" + System.currentTimeMillis();
final var prefix = bucketPath.endsWith("/") ? bucketPath : bucketPath + "/";
final String testFile = prefix + "test_" + System.currentTimeMillis();
final StreamTransferManager manager = StreamTransferManagerFactory.create(bucketName, testFile, s3Client).get();
boolean success = false;
try (final MultiPartOutputStream outputStream = manager.getMultiPartOutputStreams().get(0);
Expand Down Expand Up @@ -96,7 +92,7 @@ static void attemptS3WriteAndDelete(final S3StorageOperations storageOperations,
final S3DestinationConfig s3Config,
final String bucketPath,
final AmazonS3 s3) {
final var prefix = bucketPath.isEmpty() ? "" : bucketPath + (bucketPath.endsWith("/") ? "" : "/");
final var prefix = bucketPath.endsWith("/") ? bucketPath : bucketPath + "/";
final String outputTableName = prefix + "_airbyte_connection_test_" + UUID.randomUUID().toString().replaceAll("-", "");
attemptWriteAndDeleteS3Object(storageOperations, s3Config, outputTableName, s3);
}
Expand All @@ -106,8 +102,9 @@ private static void attemptWriteAndDeleteS3Object(final S3StorageOperations stor
final String outputTableName,
final AmazonS3 s3) {
final var s3Bucket = s3Config.getBucketName();
final var bucketPath = s3Config.getBucketPath();

storageOperations.createBucketObjectIfNotExists(s3Bucket);
storageOperations.createBucketObjectIfNotExists(bucketPath);
s3.putObject(s3Bucket, outputTableName, "check-content");
testIAMUserHasListObjectPermission(s3, s3Bucket);
s3.deleteObject(s3Bucket, outputTableName);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,14 +96,15 @@ public String getBucketObjectPath(final String namespace, final String streamNam
@Override
public void createBucketObjectIfNotExists(final String objectPath) {
final String bucket = s3Config.getBucketName();
final String folderPath = objectPath.endsWith("/") ? objectPath : objectPath + "/";
if (!doesBucketExist(bucket)) {
LOGGER.info("Bucket {} does not exist; creating...", bucket);
s3Client.createBucket(bucket);
LOGGER.info("Bucket {} has been created.", bucket);
}
if (!s3Client.doesObjectExist(bucket, objectPath)) {
if (!s3Client.doesObjectExist(bucket, folderPath)) {
LOGGER.info("Storage Object {}/{} does not exist in bucket; creating...", bucket, objectPath);
s3Client.putObject(bucket, objectPath.endsWith("/") ? objectPath : objectPath + "/", "");
s3Client.putObject(bucket, folderPath, "");
LOGGER.info("Storage Object {}/{} has been created in bucket.", bucket, objectPath);
}
}
Expand Down
1 change: 1 addition & 0 deletions docs/integrations/destinations/s3.md
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,7 @@ In order for everything to work correctly, it is also necessary that the user wh

| Version | Date | Pull Request | Subject |
|:--------|:-----------|:-----------------------------------------------------------|:-----------------------------------------------------------------------------------------------------------------------------------------------------|
| 0.3.17 | 2022-10-15 | [\#18031](https://github.com/airbytehq/airbyte/pull/18031) | Fix integration tests to use bucket path |
| 0.3.16 | 2022-10-03 | [\#17340](https://github.com/airbytehq/airbyte/pull/17340) | Enforced encrypted only traffic to S3 buckets and check logic |
| 0.3.15 | 2022-09-01 | [\#16243](https://github.com/airbytehq/airbyte/pull/16243) | Fix Json to Avro conversion when there is field name clash from combined restrictions (`anyOf`, `oneOf`, `allOf` fields). |
| 0.3.14 | 2022-08-24 | [\#15207](https://github.com/airbytehq/airbyte/pull/15207) | Fix S3 bucket path to be used for check. |
Expand Down