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

misc: make route 53 uri tests resilient to model version changes #1540

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,15 @@ class GradleGenerator : KotlinIntegration {
}
}
}

// `Route53UriTest` E2E tests depend on `TestEngine`
if (ctx.model.expectShape<ServiceShape>(ctx.settings.service).sdkId.lowercase() == "route 53") {
withBlock("jvmE2eTest {", "}") {
withBlock("dependencies {", "}") {
write(KotlinDependency.HTTP_TEST.dependencyNotation())
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Correctness: We shouldn't hardcode this service-specific logic into the main GradleGenerator. We should either:

  • Create a Section and a customization for Route53 which adds the necessary code in that section –or–
  • Just add the http-test dependency to the E2E source set for all services

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried creating a Section and a customization for Route53 but our section logic doesn't seem to handle sections in KotlinIntegrations

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just add the http-test dependency to the E2E source set for all services

You mean hardcoding it how it is now but for every service? i.e. removing the if statement

Copy link
Member

Choose a reason for hiding this comment

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

our section logic doesn't seem to handle sections in KotlinIntegrations

What do you mean by that? We use SectionWriter all over in our integrations: https://github.com/search?q=repo%3Aawslabs%2Faws-sdk-kotlin%20path%3A%2F%5Ecodegen%5C%2Faws-sdk-codegen%5C%2Fsrc%5C%2Fmain%5C%2Fkotlin%5C%2Faws%5C%2Fsdk%5C%2Fkotlin%5C%2Fcodegen%5C%2F%2F%20sectionWriter&type=code

Copy link
Contributor Author

@0marperez 0marperez Feb 27, 2025

Choose a reason for hiding this comment

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

Oh I meant a SectionId that is declared in a KotlinIntegration can't be processed by a SectionWriter in another KotlinIntegration

Copy link
Member

@lauzadis lauzadis Feb 27, 2025

Choose a reason for hiding this comment

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

Oh ok. I think it should as long as the SectionId is accessible and the order is set up correctly? If not, I'd say that's a bug and we should open an issue for it

I also think just adding the http-test dependency to all services is fine

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cool, I'll try again later and open an issue for it if needed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh I meant a SectionId that is declared in a KotlinIntegration can't be processed by a SectionWriter in another KotlinIntegration

I found out why this isn't working for the GradleGenerator, it's because of the way that the build.gradle.kts file is created. The GradleGenerator uses the fileManifest instead of useFileWriter so this step is skipped

}
if (ctx.model.topDownOperations(ctx.settings.service).any { it.hasTrait<SmokeTestsTrait>() }) {
write("")
Expand Down
168 changes: 168 additions & 0 deletions services/route53/e2eTest/src/Route53UriTest.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,168 @@
package aws.sdk.kotlin.services.route53

import aws.sdk.kotlin.runtime.auth.credentials.StaticCredentialsProvider
import aws.smithy.kotlin.runtime.auth.awscredentials.Credentials
import aws.smithy.kotlin.runtime.client.ProtocolRequestInterceptorContext
import aws.smithy.kotlin.runtime.http.interceptors.HttpInterceptor
import aws.smithy.kotlin.runtime.http.request.HttpRequest
import aws.smithy.kotlin.runtime.httptest.TestEngine
import kotlinx.coroutines.test.runTest
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.fail

class Route53UriTest {
/**
* Validates that HostedZoneId isn't trimmed when not prefixed
*/
@Test
fun listResourceRecordSetsNoTrim() = runTest {
Route53Client {
region = "us-east-1"
httpClient = TestEngine()
interceptors = mutableListOf(
AssertUrlInterceptor(
expectedUrl = "/0000-00-00/hostedzone/IDOFMYHOSTEDZONE/rrset",
),
)
credentialsProvider = StaticCredentialsProvider(Credentials("AKID", "SECRETAK"))
}.use { client ->
client.listResourceRecordSets {
hostedZoneId = "IDOFMYHOSTEDZONE"
}
}
}

/**
* Validates that HostedZoneId is trimmed
*/
@Test
fun listResourceRecordSetsTrim() = runTest {
Route53Client {
region = "us-east-1"
httpClient = TestEngine()
interceptors = mutableListOf(
AssertUrlInterceptor(
expectedUrl = "/0000-00-00/hostedzone/IDOFMYHOSTEDZONE/rrset",
),
)
credentialsProvider = StaticCredentialsProvider(Credentials("AKID", "SECRETAK"))
}.use { client ->
client.listResourceRecordSets {
hostedZoneId = "hostedzone/IDOFMYHOSTEDZONE"
}
}
}

/**
* Validates that HostedZoneId is trimmed even with a leading slash
*/
@Test
fun listResourceRecordSetsTrimLeadingSlash() = runTest {
Route53Client {
region = "us-east-1"
httpClient = TestEngine()
interceptors = mutableListOf(
AssertUrlInterceptor(
expectedUrl = "/0000-00-00/hostedzone/IDOFMYHOSTEDZONE/rrset",
),
)
credentialsProvider = StaticCredentialsProvider(Credentials("AKID", "SECRETAK"))
}.use { client ->
client.listResourceRecordSets {
hostedZoneId = "/hostedzone/IDOFMYHOSTEDZONE"
}
}
}

/**
* Validates that HostedZoneId isn't over-trimmed
*/
@Test
fun listResourceRecordSetsTrimMultiSlash() = runTest {
Route53Client {
region = "us-east-1"
httpClient = TestEngine()
interceptors = mutableListOf(
AssertUrlInterceptor(
expectedUrl = "/0000-00-00/hostedzone/IDOFMY%2FHOSTEDZONE/rrset",
),
)
credentialsProvider = StaticCredentialsProvider(Credentials("AKID", "SECRETAK"))
}.use { client ->
client.listResourceRecordSets {
hostedZoneId = "/hostedzone/IDOFMY/HOSTEDZONE"
}
}
}

/**
* This test validates that change id is correctly trimmed
*/
@Test
fun getChangeTrimChangeId() = runTest {
Route53Client {
region = "us-east-1"
httpClient = TestEngine()
interceptors = mutableListOf(
AssertUrlInterceptor(
expectedUrl = "/0000-00-00/change/SOMECHANGEID",
),
)
credentialsProvider = StaticCredentialsProvider(Credentials("AKID", "SECRETAK"))
}.use { client ->
client.getChange {
id = "/change/SOMECHANGEID"
}
}
}

/**
* This test validates that delegation set id is correctly trimmed
*/
@Test
fun getReusableDelegationSetTrimDelegationSetId() = runTest {
Route53Client {
region = "us-east-1"
httpClient = TestEngine()
interceptors = mutableListOf(
AssertUrlInterceptor(
expectedUrl = "/0000-00-00/delegationset/DELEGATIONSETID",
),
)
credentialsProvider = StaticCredentialsProvider(Credentials("AKID", "SECRETAK"))
}.use { client ->
client.getReusableDelegationSet {
id = "/delegationset/DELEGATIONSETID"
}
}
}

/**
* Model version can change the URL used as it's included in the URL.
* This interceptor removes the model version from the expected and actual URLs.
* Then performs an equality assertion between the two.
*
* https://github.com/awslabs/aws-sdk-kotlin/issues/1370
*/
private class AssertUrlInterceptor(private val expectedUrl: String) : HttpInterceptor {
override fun readBeforeTransmit(context: ProtocolRequestInterceptorContext<Any, HttpRequest>) {
val actualUrl = context.protocolRequest.url.path.toString()

val parsedActualUrl = removeModelVersion(actualUrl)
val parsedExpectedUrl = removeModelVersion(expectedUrl)

assertEquals(parsedExpectedUrl, parsedActualUrl)
}

private fun removeModelVersion(url: String) =
try {
url.replaceFirst(
Regex("^/\\d{4}-\\d{2}-\\d{2}/"),
"//",
)
} catch (e: Exception) {
fail("The URL '$url' is not in the expected format", e)
}
}
}
81 changes: 0 additions & 81 deletions services/route53/model/route53-tests.smithy

This file was deleted.

Loading