Skip to content
This repository has been archived by the owner on May 14, 2021. It is now read-only.

Implement v4 signatures for S3 calls. #35

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

ab
Copy link

@ab ab commented Jul 29, 2017

It would probably be better to use the aws-sdk or aws-sigv4 gems directly,
but this will work for now.

This allows Citadel to work with KMS encrypted data and with AWS regions
that came online in 2014 or later.

Fixes: #28

ab added 3 commits July 29, 2017 17:21
This makes it possible for users of Citadel to get the full response
body from S3 requests and to take custom action depending upon various
different errors without needing to parse the string error message.

For example, this enables callers to distinguish between HTTP 404 and
HTTP 403 errors by using `err.wrapped_exception.response.code`.
It would probably be better to use the aws-sdk or aws-sigv4 gems
directly, but this will work for now.

This allows Citadel to work with KMS encrypted data and with AWS regions
that came online in 2014 or later.

Fixes: poise#28
@codecov-io
Copy link

codecov-io commented Jul 29, 2017

Codecov Report

Merging #35 into master will increase coverage by 3.42%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #35      +/-   ##
==========================================
+ Coverage   87.95%   91.37%   +3.42%     
==========================================
  Files           6        6              
  Lines          83      116      +33     
==========================================
+ Hits           73      106      +33     
  Misses         10       10
Impacted Files Coverage Δ
lib/citadel/s3.rb 100% <100%> (ø) ⬆️
lib/citadel/error.rb 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a21495f...2412ff3. Read the comment docs.

@brodygov
Copy link

brodygov commented Aug 3, 2017

@coderanger Any thoughts on this?

@mconigliaro
Copy link

I'm glad I saw this. I'd like to give Citadel a try, but it won't work for me unless EU regions are supported. @ab @brodygov thanks for your work on this. 👍

@coderanger
Copy link
Member

I'm working on a conference talk and won't have time to review this for another week or two.

@brodygov
Copy link

brodygov commented Jul 9, 2018

Ping

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants