Skip to content

Commit

Permalink
fix: enable disabled s3 e2e tests & update tests after default checks…
Browse files Browse the repository at this point in the history
…ums (#1505)
  • Loading branch information
0marperez authored Jan 17, 2025
1 parent 580f5ab commit 05881f1
Show file tree
Hide file tree
Showing 5 changed files with 6 additions and 57 deletions.
4 changes: 0 additions & 4 deletions services/s3/e2eTest/src/PaginatorTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import org.junit.jupiter.api.AfterAll
import org.junit.jupiter.api.BeforeAll
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.TestInstance
import kotlin.test.Ignore
import kotlin.test.assertContentEquals
import kotlin.time.Duration.Companion.seconds

Expand All @@ -41,10 +40,7 @@ class PaginatorTest {
S3TestUtils.deleteBucketAndAllContents(client, testBucket)
}

// FIXME: Enable test
// Seeing: S3Exception: Checksum Type mismatch occurred, expected checksum Type: null, actual checksum Type: crc32
// ListParts has a strange pagination termination condition via [IsTerminated]. Verify it actually works correctly.
@Ignore
@Test
fun testListPartsPagination() = runBlocking {
val chunk = "!".repeat(5 * 1024 * 1024).encodeToByteArray() // Parts must be at least 5MB
Expand Down
7 changes: 3 additions & 4 deletions services/s3/e2eTest/src/S3ChecksumTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package aws.sdk.kotlin.e2etest
import aws.sdk.kotlin.e2etest.S3TestUtils.deleteBucketContents
import aws.sdk.kotlin.e2etest.S3TestUtils.deleteMultiPartUploads
import aws.sdk.kotlin.e2etest.S3TestUtils.getAccountId
import aws.sdk.kotlin.e2etest.S3TestUtils.getBucketByName
import aws.sdk.kotlin.e2etest.S3TestUtils.getTestBucket
import aws.sdk.kotlin.e2etest.S3TestUtils.responseCodeFromPut
import aws.sdk.kotlin.services.s3.*
import aws.sdk.kotlin.services.s3.model.*
Expand All @@ -24,14 +24,13 @@ import kotlin.time.Duration.Companion.seconds
@TestInstance(TestInstance.Lifecycle.PER_CLASS)
class S3ChecksumTest {
private val client = S3Client { region = "us-west-2" }
private val testBucket = "s3-test-bucket-ci-motorcade"
private lateinit var testBucket: String
private fun testKey(): String = "test-object" + UUID.randomUUID()

@BeforeAll
private fun setUp(): Unit = runBlocking {
val accountId = getAccountId()
// FIXME: Use randomly generated bucket instead of hardcoded one
getBucketByName(client, testBucket, "us-west-2", accountId)
testBucket = getTestBucket(client, "us-west-2", accountId)
}

@AfterAll
Expand Down
13 changes: 2 additions & 11 deletions services/s3/e2eTest/src/S3ExpressTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ class S3ExpressTest {
}

@Test
fun testUploadPartContainsNoDefaultChecksum() = runTest {
fun testUploadPartContainsCRC32Checksum() = runTest {
val testBucket = testBuckets.first()
val testObject = "I-will-be-uploaded-in-parts-!"

Expand All @@ -153,7 +153,7 @@ class S3ExpressTest {
var eTagPartTwo: String?

client.withConfig {
interceptors += NoChecksumValidatingInterceptor()
interceptors += CRC32ChecksumValidatingInterceptor()
}.use { validatingClient ->
eTagPartOne = validatingClient.uploadPart {
bucket = testBucket
Expand Down Expand Up @@ -210,13 +210,4 @@ class S3ExpressTest {
}
}
}

private class NoChecksumValidatingInterceptor : HttpInterceptor {
override fun readBeforeTransmit(context: ProtocolRequestInterceptorContext<Any, HttpRequest>) {
val headers = context.protocolRequest.headers
if (headers.contains(S3_EXPRESS_SESSION_TOKEN_HEADER)) {
assertFalse(headers.names().any { it.startsWith("x-amz-checksum-") })
}
}
}
}
3 changes: 0 additions & 3 deletions services/s3/e2eTest/src/S3IntegrationTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -188,9 +188,6 @@ class S3BucketOpsIntegrationTest {
}
}

// FIXME: Enable test
// Seeing: S3Exception: Checksum Type mismatch occurred, expected checksum Type: null, actual checksum Type: crc32
@Ignore
@Test
fun testMultipartUpload(): Unit = runBlocking {
s3WithAllEngines { s3 ->
Expand Down
36 changes: 1 addition & 35 deletions services/s3/e2eTest/src/S3TestUtils.kt
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,7 @@ object S3TestUtils {
const val DEFAULT_REGION = "us-west-2"

// The E2E test account only has permission to operate on buckets with the prefix "s3-test-bucket-"
// Non-checksum E2E tests will use and delete hardcoded bucket required for checksum tests if TEST_BUCKET_PREFIX="s3-test-bucket-" via `deleteBucketAndAllContents`
// TODO: Change back to "s3-test-bucket-"
private const val TEST_BUCKET_PREFIX = "s3-test-bucket-temp-"
private const val TEST_BUCKET_PREFIX = "s3-test-bucket-"

private const val S3_MAX_BUCKET_NAME_LENGTH = 63 // https://docs.aws.amazon.com/AmazonS3/latest/userguide/bucketnamingrules.html
private const val S3_EXPRESS_DIRECTORY_BUCKET_SUFFIX = "--x-s3"
Expand Down Expand Up @@ -100,38 +98,6 @@ object S3TestUtils {
testBucket
}

suspend fun getBucketByName(
client: S3Client,
targetBucket: String,
region: String? = null,
accountId: String? = null,
): Unit = withTimeout(60.seconds) {
try {
val targetBucketRegion = client
.headBucket {
this.bucket = targetBucket
expectedBucketOwner = accountId
}.bucketRegion

if (targetBucketRegion != region) {
throw RuntimeException(
"The requested bucket ($targetBucket) already exists in another region than the one requested ($region)",
)
}
} catch (e: Throwable) {
println("Creating S3 bucket: $targetBucket")

client.createBucket {
bucket = targetBucket
createBucketConfiguration {
locationConstraint = BucketLocationConstraint.fromValue(region ?: client.config.region!!)
}
}

client.waitUntilBucketExists { bucket = targetBucket }
}
}

suspend fun getTestDirectoryBucket(client: S3Client, suffix: String) = withTimeout(60.seconds) {
var testBucket = client.listBuckets()
.buckets
Expand Down

0 comments on commit 05881f1

Please sign in to comment.