-
Notifications
You must be signed in to change notification settings - Fork 50
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
feat: support default checksums #1475
Open
0marperez
wants to merge
76
commits into
main
Choose a base branch
from
flexible-checksums
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
76 commits
Select commit
Hold shift + click to select a range
4729584
misc: add rules engine codegen tests
0marperez 7b73088
Use snapshot version of smithy kotlin
0marperez dbfb973
debugging: throw exception to see what metrics are strings
0marperez b15358b
debugging: remove exception
0marperez dacc345
Don't depend on snapshot version of smithy kotlin
0marperez bbafc3f
Use released version
0marperez a79aa1e
PR feedback
0marperez e61efbb
Checkpoint
0marperez 285c08b
Merge branch 'main' of https://github.com/awslabs/aws-sdk-kotlin into…
0marperez fbb3dd5
Checkoint 2
0marperez 136c9db
It works now
0marperez cb4ab12
Clean up build scripts
0marperez 41ad1b4
Commonize some more
0marperez 1a2d989
Checkpoint 3
0marperez 11c733f
Merge branch 'main' of https://github.com/awslabs/aws-sdk-kotlin into…
0marperez 4d24a24
Remove rules engine codegen tests
0marperez c514cba
Setup checksums codegen tests
0marperez 2796317
Update smithy IDL version, codegen tests pass now
0marperez 6d53243
Setup dummy unit tests
0marperez 40cfb17
Basic setup for client config tests
0marperez 410c085
Added requestChecksumCalculation config option
0marperez 472fae9
RequestChecksum not required client config tests
0marperez e9bf8e5
RequestChecksum required client config tests
0marperez 5133a60
Added responseChecksumValidation
0marperez 4ce5da7
TODO ResponseChecksumValidation tests
0marperez da31deb
Added ResponseChecksumValidation codegen tests
0marperez 6037580
Quick self review
0marperez 2f6e10b
Self review V2
0marperez d085dec
Add todos for business metrics
0marperez 0bafdcc
Unit tests pass
0marperez fc84b61
E2E tests pass
0marperez 0f6fa65
Self review
0marperez c76aefe
trigger ci
0marperez 3476242
Fix smoke tests
0marperez b21ec41
Don't specify gradle version in smoke tests
0marperez dd2792f
Correctly print enum entries in error message
0marperez 779cd3a
Remove test ordering from S3ChecksumTest
0marperez 6674dc7
Merge branch 'main' of https://github.com/awslabs/aws-sdk-kotlin into…
0marperez a3a2e09
Add copyright headers to checksum codegen tests
0marperez c80b55c
Update buildSrc package setup
0marperez 5e2855c
Fix codegen tests not having packages
0marperez 0405b20
Fix event stream codegen test templates
0marperez eb338f4
Save test reports for failing JVM CI checks
0marperez a2fb7fb
Track smithy kotlin changes
0marperez 093ed72
Merge branch 'main' of https://github.com/awslabs/aws-sdk-kotlin into…
0marperez d498e30
Fix jvmTest JVM compatibility
0marperez b534927
Drop support for http body dot bytes response checksums
0marperez 2761bf9
Left FIXME to use random buckets in motorcade E2E tests
0marperez 5328641
Refactor/fix S3 express integrations
0marperez ad30f70
Clean up S3 integrations and e2e tests
0marperez 4a162de
s3 express no checksums in upload part test
0marperez d665873
PR feedback
0marperez 1aefe66
Use head bucket
0marperez 55bdcb8
Presigned URL checksums
0marperez 44894e6
Ktlint
0marperez ad8d1ad
Trigger CI
0marperez d398801
Merge branch 'main' of https://github.com/awslabs/aws-sdk-kotlin into…
0marperez 3c259ef
PR feeback checkpoint
0marperez 0029768
Trigger CI
0marperez 427daf3
Refactor checksum interceptors
0marperez 41ad742
Fix composite checksums
0marperez 5c6c0bf
Make it compile
0marperez 4d915c6
Fix smithy model template
0marperez 609226c
Fix model smithy template again?
0marperez 80ba070
Fix model again
0marperez 94dfc93
Merge branch 'main' of https://github.com/awslabs/aws-sdk-kotlin into…
0marperez 77f6ea5
Refactor rules engine codegen tests
0marperez 4b61347
Run CI (empty commit)
0marperez 756f66d
PR feedback
0marperez 3ed4d91
Change JVM version
0marperez 34a8153
Clean up
0marperez 6a7ec2f
misc: revert toList/JVM compatibility changes
0marperez 77ec52c
fix: everything works except smoke-tests:services
0marperez 9a46dbc
fix: finally fix this whole mess
0marperez 7cbe93d
fix: refactor buildSrc package name
0marperez 8f65c72
fix: pr feedback
0marperez File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
34 changes: 34 additions & 0 deletions
34
...nfig/common/src/aws/sdk/kotlin/runtime/config/checksums/ResolveFlexibleChecksumsConfig.kt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
package aws.sdk.kotlin.runtime.config.checksums | ||
|
||
import aws.sdk.kotlin.runtime.InternalSdkApi | ||
import aws.sdk.kotlin.runtime.config.AwsSdkSetting | ||
import aws.sdk.kotlin.runtime.config.profile.AwsProfile | ||
import aws.sdk.kotlin.runtime.config.profile.requestChecksumCalculation | ||
import aws.sdk.kotlin.runtime.config.profile.responseChecksumValidation | ||
import aws.smithy.kotlin.runtime.client.config.RequestHttpChecksumConfig | ||
import aws.smithy.kotlin.runtime.client.config.ResponseHttpChecksumConfig | ||
import aws.smithy.kotlin.runtime.config.resolve | ||
import aws.smithy.kotlin.runtime.util.LazyAsyncValue | ||
import aws.smithy.kotlin.runtime.util.PlatformProvider | ||
|
||
/** | ||
* Attempts to resolve requestChecksumCalculation from the specified sources. | ||
* @return requestChecksumCalculation setting if found, the default value if not. | ||
*/ | ||
@InternalSdkApi | ||
public suspend fun resolveRequestChecksumCalculation( | ||
platform: PlatformProvider = PlatformProvider.System, | ||
profile: LazyAsyncValue<AwsProfile>, | ||
): RequestHttpChecksumConfig = | ||
AwsSdkSetting.AwsRequestChecksumCalculation.resolve(platform) ?: profile.get().requestChecksumCalculation ?: RequestHttpChecksumConfig.WHEN_SUPPORTED | ||
|
||
/** | ||
* Attempts to resolve responseChecksumValidation from the specified sources. | ||
* @return responseChecksumValidation setting if found, the default value if not. | ||
*/ | ||
@InternalSdkApi | ||
public suspend fun resolveResponseChecksumValidation( | ||
platform: PlatformProvider = PlatformProvider.System, | ||
profile: LazyAsyncValue<AwsProfile>, | ||
): ResponseHttpChecksumConfig = | ||
AwsSdkSetting.AwsResponseChecksumValidation.resolve(platform) ?: profile.get().responseChecksumValidation ?: ResponseHttpChecksumConfig.WHEN_SUPPORTED |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
30 changes: 30 additions & 0 deletions
30
...dk/kotlin/runtime/http/interceptors/IgnoreCompositeFlexibleChecksumResponseInterceptor.kt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
package aws.sdk.kotlin.runtime.http.interceptors | ||
|
||
import aws.smithy.kotlin.runtime.client.config.ResponseHttpChecksumConfig | ||
import aws.smithy.kotlin.runtime.http.interceptors.FlexibleChecksumsResponseInterceptor | ||
import aws.smithy.kotlin.runtime.telemetry.logging.Logger | ||
|
||
/** | ||
* Variant of the [FlexibleChecksumsResponseInterceptor] where composite checksums are not validated | ||
*/ | ||
public class IgnoreCompositeFlexibleChecksumResponseInterceptor( | ||
responseValidationRequired: Boolean, | ||
responseChecksumValidation: ResponseHttpChecksumConfig?, | ||
) : FlexibleChecksumsResponseInterceptor( | ||
responseValidationRequired, | ||
responseChecksumValidation, | ||
) { | ||
override fun ignoreChecksum(checksum: String, logger: Logger): Boolean = | ||
checksum.isCompositeChecksum().also { compositeChecksum -> | ||
if (compositeChecksum) logger.info { "Checksum validation was skipped because it was a composite checksum" } | ||
} | ||
} | ||
|
||
/** | ||
* Verifies if a checksum is composite. | ||
*/ | ||
private fun String.isCompositeChecksum(): Boolean { | ||
// Ends with "-#" where "#" is a number | ||
val regex = Regex("-(\\d)+$") | ||
return regex.containsMatchIn(this) | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
plugins { | ||
alias(libs.plugins.kotlin.jvm) | ||
} | ||
|
||
repositories { | ||
mavenCentral() | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
dependencyResolutionManagement { | ||
versionCatalogs { | ||
create("libs") { | ||
from(files("../gradle/libs.versions.toml")) | ||
} | ||
} | ||
} |
18 changes: 18 additions & 0 deletions
18
buildSrc/src/main/kotlin/aws/sdk/kotlin/tests/codegen/CodegenTest.kt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
package aws.sdk.kotlin.tests.codegen | ||
|
||
/** | ||
* An AWS SDK for Kotlin codegen test | ||
*/ | ||
data class CodegenTest( | ||
val name: String, | ||
val model: Model, | ||
val serviceShapeId: String, | ||
) | ||
|
||
/** | ||
* A smithy model file | ||
*/ | ||
data class Model( | ||
val fileName: String, | ||
val path: String = "src/test/resources/", | ||
) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comment: Good catch!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What are these test reports used for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not 100% sure why but sometimes the logs from failed CI tests are not very useful. In those cases you can just get a test report from the build and see which test cases failed and exception messages. Other CI checks have this as well