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

Fog mock does not mimmick real behaviour for some Excon errors #341

Closed
ac-astuartkregor opened this issue Feb 3, 2017 · 5 comments
Closed

Comments

@ac-astuartkregor
Copy link

ac-astuartkregor commented Feb 3, 2017

Notes

Apologies if this has already been raised, I tried to look but didn't see much.

Scenario

Using GET and HEAD on S3 objects with an 'If-Match' constraint and supplying an ETag. The file already exists in the bucket but the content on disk has changed. In order to test functionality that depends on these requests, tests have been written which use Fog's mock ability.

Expected behaviour

When Fog mocking is turned on, just like real mode an Excon::Error::PreconditionFailed exception should raised which must be handled in application code. The S3 API returned a status code of 412 because the supplied ETag did not match with the object's ETag.

The below example demonstrates what happens when Fog mocking is turned off.

irb(main):001:0> require 'fog'
=> true

irb(main):002:0> fog_options = { provider: 'AWS', region: 'eu-west-1', aws_access_key_id: '[REDACTED]', aws_secret_access_key: '[REDACTED]' }
=> {:provider=>"AWS", :region=>"eu-west-1", :aws_access_key_id=>"[REDACTED]", :aws_secret_access_key=>"[REDACTED]"}

irb(main):003:0> fog = Fog::Storage.new fog_options
=> #<Fog::Storage::AWS::Real:70293080275240 @use_iam_profile=nil @instrumentor=nil @instrumentor_name="fog.aws.storage" @connection_options={} @persistent=false @signature_version=4 @path_style=false @region="eu-west-1" @endpoint=nil @host="s3-eu-west-1.amazonaws.com" @scheme="https" @port=443 @aws_access_key_id="[REDACTED]" @aws_session_token=nil @aws_credentials_expire_at=nil @signer=#<Fog::AWS::SignatureV4:0x007fdcc359f900 @region="eu-west-1", @service="s3", @aws_access_key_id="[REDACTED]", @hmac=#<Fog::HMAC:0x007fdcc359f860 @key="AWS4[REDACTED", @digest=#<OpenSSL::Digest: e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855>, @signer=#<Proc:0x007fdcc359f770@/home/myuser/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/fog-core-1.43.0/lib/fog/core/hmac.rb:28 (lambda)>>>>

irb(main):004:0> fog.directories.get('bucket-name').nil?
=> false

irb(main):005:0> bucket = fog.directories.get('bucket-name')
=>   <Fog::Storage::AWS::Directory
    key="bucket-name",
    creation_date=nil,
    location="eu-west-1"
  >

irb(main):006:0> bucket.files.create(key: 'test_file', body: File.read('/home/myuser/projects/images/test/fixtures/testfile.jpg'))
=>   <Fog::Storage::AWS::File
    key="test_file",
    cache_control=nil,
    content_disposition=nil,
    content_encoding=nil,
    content_length=73158,
    content_md5=nil,
    content_type=nil,
    etag="032a85faabbfc02cb20416ca145c3575",
    expires=nil,
    last_modified=nil,
    metadata={"x-amz-id-2"=>"Tov8jFJ7Ml5L1sC6VsttrFeELqTlxBVAgNsdb//wop2aJ448l5dwWBRtk7LvmPvABBh8Sotd+oU=", "x-amz-request-id"=>"44427319D2E9C8BD", "x-amz-expiration"=>"expiry-date=\"Fri, 05 May 2017 00:00:00 GMT\"},
    owner=nil,
    storage_class=nil,
    encryption=nil,
    encryption_key=nil,
    version=nil
  >

irb(main):007:0> fog.head_object('bucket-name', 'test_file')
=> #<Excon::Response:0x007fdcc7009848 @data={:body=>"", :cookies=>[], :host=>"bucket-name.s3-eu-west-1.amazonaws.com", :headers=>{"x-amz-id-2"=>"AMQSEahnPxdF4hLotTmvlXnct1SBS1T/glGxT5NyxnAuDacFsXuuJDEIA/Xd0hurq+fh8nb9z3E=", "x-amz-request-id"=>"73E72E528AF8D493", "Date"=>"Fri, 03 Feb 2017 09:28:18 GMT", "Last-Modified"=>"Fri, 03 Feb 2017 09:28:09 GMT", "x-amz-expiration"=>"expiry-date=\"Fri, 05 May 2017 00:00:00 GMT\", "ETag"=>"\"032a85faabbfc02cb20416ca145c3575\"", "Accept-Ranges"=>"bytes", "Content-Type"=>"", "Content-Length"=>"73158", "Server"=>"AmazonS3"}, :path=>"/test_file", :port=>443, :status=>200, :status_line=>"HTTP/1.1 200 OK\r\n", :reason_phrase=>"OK", :remote_ip=>"52.218.64.67", :local_port=>56678, :local_address=>"192.168.0.3"}, @body="", @headers={"x-amz-id-2"=>"AMQSEahnPxdF4hLotTmvlXnct1SBS1T/glGxT5NyxnAuDacFsXuuJDEIA/Xd0hurq+fh8nb9z3E=", "x-amz-request-id"=>"73E72E528AF8D493", "Date"=>"Fri, 03 Feb 2017 09:28:18 GMT", "Last-Modified"=>"Fri, 03 Feb 2017 09:28:09 GMT", "x-amz-expiration"=>"expiry-date=\"Fri, 05 May 2017 00:00:00 GMT\", "ETag"=>"\"032a85faabbfc02cb20416ca145c3575\"", "Accept-Ranges"=>"bytes", "Content-Type"=>"", "Content-Length"=>"73158", "Server"=>"AmazonS3"}, @status=200, @remote_ip="52.218.64.67", @local_port=56678, @local_address="192.168.0.3">

irb(main):008:0> fog.head_object('bucket-name', 'test_file', { 'If-Match' => 'foo' })
Excon::Error::PreconditionFailed: Expected(200) <=> Actual(412 Precondition Failed)
excon.error.response
  :body          => ""
  :cookies       => [
  ]
  :headers       => {
    "Content-Type"      => "application/xml"
    "Date"              => "Fri, 03 Feb 2017 09:28:27 GMT"
    "Server"            => "AmazonS3"
    "Transfer-Encoding" => "chunked"
    "x-amz-id-2"        => "pVuV00iuCzW8/eEeHtdaPnF/o6F3dvE0yIqXURDjlbRXSdWPz/4LgbqN9ovm+ygkuRGo2jzQMi8="
    "x-amz-request-id"  => "7FD69E2FF42EE235"
  }
  :host          => "bucket-name.s3-eu-west-1.amazonaws.com"
  :local_address => "192.168.0.3"
  :local_port    => 57142
  :path          => "/test_file"
  :port          => 443
  :reason_phrase => "Precondition Failed"
  :remote_ip     => "52.218.16.211"
  :status        => 412
  :status_line   => "HTTP/1.1 412 Precondition Failed\r\n"

  from /home/myuser/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/excon-0.53.0/lib/excon/middlewares/expects.rb:7:in `response_call'
  from /home/myuser/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/excon-0.53.0/lib/excon/middlewares/response_parser.rb:9:in `response_call'
  from /home/myuser/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/excon-0.53.0/lib/excon/connection.rb:388:in `response'
  from /home/myuser/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/excon-0.53.0/lib/excon/connection.rb:252:in `request'
  from /home/myuser/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/excon-0.53.0/lib/excon/middlewares/idempotent.rb:27:in `error_call'
  from /home/myuser/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/excon-0.53.0/lib/excon/middlewares/base.rb:11:in `error_call'
  from /home/myuser/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/excon-0.53.0/lib/excon/middlewares/base.rb:11:in `error_call'
  from /home/myuser/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/excon-0.53.0/lib/excon/connection.rb:272:in `rescue in request'
  from /home/myuser/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/excon-0.53.0/lib/excon/connection.rb:215:in `request'
  from /home/myuser/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/excon-0.53.0/lib/excon/middlewares/idempotent.rb:27:in `error_call'
  from /home/myuser/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/excon-0.53.0/lib/excon/middlewares/base.rb:11:in `error_call'
  from /home/myuser/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/excon-0.53.0/lib/excon/middlewares/base.rb:11:in `error_call'
  from /home/myuser/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/excon-0.53.0/lib/excon/connection.rb:272:in `rescue in request'
  from /home/myuser/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/excon-0.53.0/lib/excon/connection.rb:215:in `request'
  from /home/myuser/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/excon-0.53.0/lib/excon/middlewares/idempotent.rb:27:in `error_call'
  from /home/myuser/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/excon-0.53.0/lib/excon/middlewares/base.rb:11:in `error_call'
  from /home/myuser/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/excon-0.53.0/lib/excon/middlewares/base.rb:11:in `error_call'
  from /home/myuser/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/excon-0.53.0/lib/excon/connection.rb:272:in `rescue in request'
  from /home/myuser/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/excon-0.53.0/lib/excon/connection.rb:215:in `request'
  from /home/myuser/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/fog-core-1.43.0/lib/fog/core/connection.rb:81:in `request'
  from /home/myuser/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/fog-xml-0.1.2/lib/fog/xml/connection.rb:9:in `request'
  from /home/myuser/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/fog-aws-0.12.0/lib/fog/aws/storage.rb:611:in `_request'
  from /home/myuser/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/fog-aws-0.12.0/lib/fog/aws/storage.rb:606:in `request'
  from /home/myuser/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/fog-aws-0.12.0/lib/fog/aws/requests/storage/head_object.rb:41:in `head_object'
  from (irb):8
  from /home/myuser/.rbenv/versions/2.3.1/bin/irb:11:in `<main>'

Actual behaviour

When Fog mocking is turned on, no exception is raised, but the response status code is set to 412 as in the following example.

irb(main):001:0> require 'fog'
=> true

irb(main):002:0> Fog.mock!
=> true

irb(main):003:0> fog_options = { provider: 'AWS', aws_access_key_id: 'foo', aws_secret_access_key: 'foo' }
=> {:provider=>"AWS", :aws_access_key_id=>"foo", :aws_secret_access_key=>"foo"}

irb(main):004:0> fog = Fog::Storage.new fog_options
=> #<Fog::Storage::AWS::Mock:0x007ffe532ea230 @use_iam_profile=nil, @region="us-east-1", @endpoint=nil, @host="s3.amazonaws.com", @scheme="https", @port=443, @path_style=false, @signature_version=4, @aws_access_key_id="foo", @aws_secret_access_key="foo", @aws_session_token=nil, @aws_credentials_expire_at=nil, @signer=#<Fog::AWS::SignatureV4:0x007ffe5492a888 @region="us-east-1", @service="s3", @aws_access_key_id="foo", @hmac=#<Fog::HMAC:0x007ffe5492a7e8 @key="AWS4foo", @digest=#<OpenSSL::Digest: e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855>, @signer=#<Proc:0x007ffe5492a6f8@/home/myuser/.rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/fog-core-1.43.0/lib/fog/core/hmac.rb:28 (lambda)>>>>

irb(main):005:0> fog.directories.get('bucket-name').nil?
=> true

irb(main):006:0> bucket = fog.directories.create(key: 'bucket-name')
=>   <Fog::Storage::AWS::Directory
    key="bucket-name",
    creation_date=nil,
    location="us-east-1"
  >

irb(main):007:0> bucket.files.create(key: 'test_file', body: File.read('/home/myuser/projects/images/test/fixtures/testfile.jpg'))
=>   <Fog::Storage::AWS::File
    key="test_file",
    cache_control=nil,
    content_disposition=nil,
    content_encoding=nil,
    content_length=73158,
    content_md5=nil,
    content_type=nil,
    etag="032a85faabbfc02cb20416ca145c3575",
    expires=nil,
    last_modified="Fri, 03 Feb 2017 09:30:07 +0000",
    metadata={},
    owner=nil,
    storage_class=nil,
    encryption=nil,
    encryption_key=nil,
    version=nil
  >

irb(main):008:0> fog.head_object('bucket-name', 'test_file')
=> #<Excon::Response:0x007ffe54913840 @data={:body=>nil, :headers=>{"Content-Type"=>nil, "ETag"=>"032a85faabbfc02cb20416ca145c3575", "Last-Modified"=>"Fri, 03 Feb 2017 09:30:07 +0000", "Content-Length"=>73158}, :status=>200}, @body="", @headers={"Content-Type"=>nil, "ETag"=>"032a85faabbfc02cb20416ca145c3575", "Last-Modified"=>"Fri, 03 Feb 2017 09:30:07 +0000", "Content-Length"=>73158}, @status=nil, @remote_ip=nil, @local_port=nil, @local_address=nil>

irb(main):009:0> fog.head_object('bucket-name', 'test_file', { 'If-Match' => 'foo' })
=> #<Excon::Response:0x007ffe54909ac0 @data={:body=>nil, :headers=>{}, :status=>412}, @body="", @headers={}, @status=nil, @remote_ip=nil, @local_port=nil, @local_address=nil>

Implications

As a result, Fog's mock feature is not replicating the real behaviour when using the 'If-Match' constraint, and as such it is not possible to test the real behaviour accurately.

@geemus
Copy link
Member

geemus commented Feb 3, 2017

Thanks for the detailed bug report. Would you be willing to help us improve the mocks to better reflect what you have described? Thanks!

@ac-astuartkregor
Copy link
Author

@geemus I'll have a look this week and see if I can get a PR in. I'll try and follow existing patterns but do you have any up-front recommendations?

@geemus
Copy link
Member

geemus commented Feb 6, 2017

@ac-astuartkregor nothing in particular jumps to mind. There should be many examples though and hopefully the changes to accommodate these particular fixes won't be too crazy. Certainly happy to discuss and provide guidance as needed along the way. Thanks!

@easkay
Copy link
Contributor

easkay commented Feb 12, 2017

Thanks @geemus !! Put a PR in for the very basics and am looking for feedback when you're ready.

@geemus
Copy link
Member

geemus commented Feb 14, 2017

@easkay awesome. I'm going to close this in favor of consolidating discussion on the PR, will try to get you feedback there soon.

@geemus geemus closed this as completed in d828888 Feb 15, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants