service/s3/s3crypto: Added X-Ray support to s3crypto decryption client #2912
Conversation
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
skmcgrail
added
the
needs-review
jasdel
changed the title
jasdel
force-pushed
the
s3crypto-xray-v2
branch
from
6969f6b
to
ba1a4b3
Compare
jasdel
approved these changes
Jan 2, 2020
jasdel
removed
the
needs-review
skotambkar
approved these changes
Jan 3, 2020
skmcgrail
approved these changes
Jan 9, 2020
…DecryptKeyWithContext method instead
jasdel
force-pushed
the
s3crypto-xray-v2
branch
from
ba1a4b3
to
937b7b2
Compare
Merged
aws-sdk-go-automation
pushed a commit
that referenced
this pull request
Jan 15, 2020
=== ### Service Client Updates * `service/ec2`: Updates service API and documentation * General Update to EC2 Docs and SDKs * `service/organizations`: Updates service documentation * Updated description for PolicyID parameter and ConstraintViolationException. * `service/securityhub`: Updates service API and documentation * `service/ssm`: Updates service documentation * Document updates for Patch Manager 'NoReboot' feature. ### SDK Enhancements * `service/s3/s3crypto`: Added X-Ray support to encrypt/decrypt clients ([#2912](#2912)) * Adds support for passing Context down to the crypto client's KMS client enabling tracing for tools like X-Ray, and metrics.
aws-sdk-go-automation
added a commit
that referenced
this pull request
Jan 15, 2020
Release v1.28.3 (2020-01-15) === ### Service Client Updates * `service/ec2`: Updates service API and documentation * General Update to EC2 Docs and SDKs * `service/organizations`: Updates service documentation * Updated description for PolicyID parameter and ConstraintViolationException. * `service/securityhub`: Updates service API and documentation * `service/ssm`: Updates service documentation * Document updates for Patch Manager 'NoReboot' feature. ### SDK Enhancements * `service/s3/s3crypto`: Added X-Ray support to encrypt/decrypt clients ([#2912](#2912)) * Adds support for passing Context down to the crypto client's KMS client enabling tracing for tools like X-Ray, and metrics.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
The s3crypto package is not passing a context object down to the KMS client calls so panics were being emitted by the X-Ray SDK.
This replaces my original PR #2783 in which @jasdel pointed out I'd break backwards compatibility.
I've found a relatively simple method of passing the request to the KMS client by introducing an additional interface
CipherDataDecrypterWithContextwhich the KMS key handler conforms to. The decryption client then checks if the decrypter is of this type and if so calls the alternative method exposed in this interface.I've not attempted to implement this for the
PutObjectmethod on the encryption client as I just couldn't find a simple way of introducing new code without breaking backwards compatibility.I've tested my changes successfully: