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

do not make requests if mocked. #252

Merged
merged 3 commits into from
May 19, 2016
Merged

Conversation

shaiguitar
Copy link

@shaiguitar shaiguitar commented May 11, 2016

Doing a compute.servers should work according as expected in mock mode when using it with an iam role (ie use_iam_profile), but currently the credentials_fetcher is making real requests. This just mocks the api keys that it would get in the real mode.

@geemus
Copy link
Member

geemus commented May 12, 2016

It looks like there must be some expectation around this being mocked already? The tests appear to be looking for some specific dummy keys/secrets that are now failing. Could you take another look? Thanks!

@shaiguitar
Copy link
Author

Looks like the request is just being stubbed in the test, it's not happening Fog.mock level, which also verifies my experience of seeing it make requests even in mocked mode. Excon request is being patched up here: https://github.com/fog/fog-aws/blob/master/tests/credentials_tests.rb#L18
with this info https://github.com/fog/fog-aws/blob/master/tests/credentials_tests.rb#L11-L16

Can we just remove the Excon stub and default to using Fog.mock in the test itself? If so I'll commit a fix.

@geemus
Copy link
Member

geemus commented May 12, 2016

Ah, thanks. I saw the failure but didn't dig much deeper. Removing that stub and using the mock stuff itself sounds good. Thanks!

excon stubs are good documentation and actually more code coverage
so will to keep what is already in case of later use.
@shaiguitar
Copy link
Author

Fix pushed 📗 The stubs are actually a bit more comprehensive tests so keeping them around for debugging if necessary later.

@@ -1,7 +1,7 @@
Shindo.tests('AWS | credentials', ['aws']) do
old_mock_value = Excon.defaults[:mock]
Excon.stubs.clear

Fog.mock!
Copy link
Member

Choose a reason for hiding this comment

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

this will pollute the rest of the test suite if not running mocked already

Copy link
Author

Choose a reason for hiding this comment

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

@lanej
Copy link
Member

lanej commented May 19, 2016

💥

@lanej lanej merged commit a8da8c5 into fog:master May 19, 2016
@geemus
Copy link
Member

geemus commented May 19, 2016

Thanks!

On Thu, May 19, 2016 at 1:02 PM, Josh Lane notifications@github.com wrote:

💥


You are receiving this because you commented.
Reply to this email directly or view it on GitHub
#252 (comment)

dmbrooking added a commit to datapipe/fog-aws that referenced this pull request Jul 7, 2017
* Fix for empty ETag values

Fog aws fails if API response contains empty ETag tags. It may cause when working with S3 clones (like minio in my case), that not provides ETags in their API answer.

* Allow case-insensitive record comparison

* Parse EbsOptimized parameter in launch configuration description

* updated CHANGELOG.md

* Bump to 0.9.3

* update CHANGELOG.md

* AWS DNS - support newer DNS hosted zone IDs for dualstack ELBs

* Replaces usage of Digest with OpenSSL::Digest for fog#261
The Digest class has some thread safety issues fixed in

aws/aws-sdk-ruby#529
aws/aws-sdk-ruby#525
ruby/ruby@c02fa39

* new India Region to fog-aws

* Added region Mumbai ap-south-1

* update CHANGELOG.md

* Bump to 0.9.4

* udpate CHANGELOG.md

* add default region to use_iam_profile

* make sure to mock tests for region in use_iam_profile

* stub 404 as well

* modify db snapshot attribute api implementation

* copy db snapshot api implementation

* added EOF's

* implemented copy db snapshot parser

* updated parser used in copy_db_snapshot

* fix rescue to use new excon error namespacing

* Added tests for db snapshots

* added mock for modify_db_snapshot_attribute request

* removed fog mock from db snapshot tests

* Expanding IAM support

Adding:
* Create/Delete Policy Versions
* Set Default Policy Version
* Add Update Assume Role Policy

* Add List Attached Role Policies

Add list policy versions

Add more IAM policy tests

* Incorrectly used policy_name rather than arn

Correct request name to delete_policy_version

Correct policy list version parser

Correct update assume role policy

* Copy DB Snapshot mock correct response

* RDS Snapshot format corrected

* Correct DB Snapshot tests

* update CHANGELOG.md

* Bump to 0.10.0

* Change DBSubnetGroup to DBSubnetGroupName model cluster while creation

* test(ci): fix 1.9 builds with json >= 2.0

* update CHANGELOG.md

* test(ci): bind mime-types dep for 1.9 builds

* Removed Extra Space fir proper alignment

* ECS container credentials

* Skip multipart if body size is less than chunk. Fixes fog/fog#3899

* Static method from storage class for fog/fog#3899

* Raise an error (on the setter) if chunk size is too small - fixes fog#283

* (Automatically) set multipart chunk size if the file is too big for a single PUT operation and mp was not set.

* GitHub does no longer provide http:// pages

* update CHANGELOG.md

* Bump to 0.11.0

* update CHANGELOG.md

* added DBSubnetGroupName to DbClusterParser

* Refactore fetching of ip permission

* Implements revoke egress rule

* Implements authorize egress rule

* Tests authorize egress rule

* Tests revoke egress rule

* Ensure compatibility with all CI test cases

* Feedback : enhance test

* Mapped 'DBSubnetGroup' to 'DBSubnetGroupName' parser

* indented

* Added Modify Subnet Group

* change sets

* additional actions

* stack policy

* additional parameters

* Add attribute is default in vpc

* add support endpoint and models/requests for trusted advisor checks

* fix tests

* fix 1.8

* update CHANGELOG.md

* Bump to 0.12.0

* update CHANGELOG.md

* Correct optional parameter naming in documentation for Fog::AWS::Autoscaling::Real#put_scaling_policy

* create, describe, and destroy elastic file systems

* create, describe and destroy mount targets

* fix 1.8

* describe mount target security groups

* modify mount target security groups

* remove debugging code

* fix tests

* fix tests in real mode

* fix some mocking behavior

* fix describe mount targets

* NotFound changes

* there seems to be a weird condition where sometimes the efs api returns one error, and other times a different error

* add wait_for to handle eventual consistency

* fix specs

* accessing compute from efs does weird things

* added target_group_arns to autoscaling group model

* fixed parser to parse target groups from autoscaling group description

* fix S3 #delete_multiple_objects for UTF-8 names

* mime types gem update

* added ohio region with az's

* made east-1 to east-2

* polishing with alphabetical order

* improve tests and initialize ASG with empty targetGroupARNs

* Update db_cluster_parser.rb

* Update modify_db_subnet_group.rb

* Add Fog::AWS::STS.Mock#assume_role method

* Adjust arn and expiration field formats

* Add AssumedRoleId and RequestId response fields

* Add mocked response headers

* data pipeline mocks

* fix 1.8

* Fix the bug to show the warning deprecated usage of the Code Climate Test Reporter when running tests.

* Fix the bug to show the message when running tests that SimpleCov failed to recognize the test framework and/or suite used.

* Modify using Code Climate.

* add creation date to image object

* added a flag for enhanced networking to images

* final changes for enhanced networking

* Modified the parser so that it can parse the Status filed while describing a change set

* Fix the bug that can't create fifo queue.

* Added the Capabilities parameter to be able to change IAM related stuff

* relevant changes to tests for creation Date and enhanced networking support

* Bump to 0.13.0

* update CHANGELOG.md

* Add new t2.xlarge, t2.2xlarge and r4 class instances.
Correctly set r3.large to not have EBS optimization available.
Remove a couple spurious spaces.

* fix host header with another port on s3

* Mark AWS metadata calls as idempotent

Mark calls to metadata services for AWS as idempotent. This means that excon
will retry request up to 3 times in case there is a (temporary) issue on the
metadata service.

* Bump to 1.0.0

* update CHANGELOG.md

* Added support for attaching auto sclaing groups to target groups

This is in reference to fog#328
Unfortunately I will not have time to implement support for managing
the actual ALBs and all that is reated to them.

* Updated ELB Dual Stack hosted zone DNS records

* Update aws.rb

* added az's for canada and london

* exempt mock only tests from live test run

These tests reference objects that do not exist and do not succeed in a
live environment.

* remove assertion during live test run

During a live test run there are quite a few suspended processes.

```
suspend processes
	processes suspended - returns []
		expected => []
		returned => [{"ProcessName"=>"RemoveFromLoadBalancerLowPriority", "SuspensionReason"=>"User suspended at 2016-12-15T19:29:32Z"}, {"ProcessName"=>"Launch", "SuspensionReason"=>"User suspended at 2016-12-15T19:29:32Z"}, {"ProcessName"=>"HealthCheck", "SuspensionReason"=>"User suspended at 2016-12-15T19:29:32Z"}, {"ProcessName"=>"AddToLoadBalancer", "SuspensionReason"=>"User suspended at 2016-12-15T19:29:32Z"}, {"ProcessName"=>"AlarmNotification", "SuspensionReason"=>"User suspended at 2016-12-15T19:29:32Z"}, {"ProcessName"=>"ScheduledActions", "SuspensionReason"=>"User suspended at 2016-12-15T19:29:32Z"}, {"ProcessName"=>"AZRebalance", "SuspensionReason"=>"User suspended at 2016-12-15T19:29:32Z"}, {"ProcessName"=>"Terminate", "SuspensionReason"=>"User suspended at 2016-12-15T19:29:32Z"}, {"ProcessName"=>"ReplaceUnhealthy", "SuspensionReason"=>"User suspended at 2016-12-15T19:29:32Z"}]
```

* fix escaped characters in AutoScaling::ValidationError message

Converts

```
AWS::AutoScaling | tag requests (aws, auto_scaling)
	1 validation error detected: Value '{"Key"=>"Name", "PropagateAtLaunch"=>true, "ResourceId"=>"fog-test-1481831027", "ResourceType"=>"auto-scaling-group", "Value"=>"fog-test-1481831027"}' at 'tags.1.member.key' failed to satisfy constraint: Member must have length less than or equal to 128 (Fog::AWS::AutoScaling::ValidationError)
```

to

```
AWS::AutoScaling | tag requests (aws, auto_scaling)
  1 validation error detected: Value '{"Key"=>"Name", "PropagateAtLaunch"=>true, "ResourceId"=>"fog-test-1481831149", "ResourceType"=>"auto-scaling-group", "Value"=>"fog-test-1481831149"}' at 'tags.1.member.key' failed to satisfy constraint: Member must have length less than or equal to 128 (Fog::AWS::AutoScaling::ValidationError)
```

* fix creating tags with #create_auto_scaling_group

* remove unused variables

* update CHANGELOG.md

* Bump to 1.1.0

* update CHANGELOG.md

* Pin nokogiri gem for Ruby 1.9

Nokogiri >= 1.7 requires Ruby 2.1 or higher.

* Add Gemfile-ruby-2.0 for Ruby 2.0 on Travis CI

Pin Nokogiri to version 1.6.x for Ruby 2.0, since Nokogiri 1.7.x requires Ruby 2.1 or higher.

* Support Partial reservations by parsing an array of recurring charges

This is a semi-breaking change: the recurringCharges field is switching from a
Hash to an Array, however it never before contained any values because it was
unable to parse the AWS XML response. The likelihood of breaking anybody's
"working" code is rather low given that it never worked.

While the AWS API currently supports only a single type of recurring charge,
hourly, they've left the ability to have more/other recurring charge types in
the future, so we should reflect that in the parsed data structure as well.

* Add 'scope' parameter to the reserved instance parser

* Documentation for Reserved Instance recurringCharges and scope

* mocks around iam policies

* instance profile mocks

* instance profile mocks and models

* 1.8 fixes

* more instance profile mocks

* update CHANGELOG.md

* Bump to 1.2.0

* update CHANGELOG.md

* Add missing 'self'

* Ensure get_object and head_object errors are correctly mocked.

* Ensure get_bucket_object_versions errors are correctly mocked.

* Remove unnecessary comment from delete_bucket_policy.

* Removed versioning convenience tag.

* add natGatewayId to describe_route_tables

* subnet fixes

* need to return subnet id in instance mocks
* subnet -> network interfaces relationship
* move addresses to and from vpcs
* bump compute api version to 2016-11-15
* [mock] set dnsname for instance if the vpc is setup for it

* cant pass both public_ip and association_id to the same api call

* route tables need tags too

* route tables need tags too

* mock spot requests

* update CHANGELOG.md

* Bump to 1.2.1

* update CHANGELOG.md

* modify volumes

* model tests as well

* fix 1.8 syntax error

* Add check for self.etag before running gsub

* Add new i3 class instances.

* classic link enhancements

* spec and mock fixes

* authorize vpc security group to rds security group

* fix create_db_subnet_group mock

* subnet group mocks didnt actually delete the subnet group

* update CHANGELOG.md

* Bump to 1.3.0

* update CHANGELOG.md

* Skip region fetch

* ET-2352 Implements exponential backoff for compute requests.

No new test failures.

* ET-2352 Throw RequestLimitExceeded error if out of retries.

* ET-2352 Use retries instead of recursion.

* add p2 instance types

* Handle multipart upload of empty files

* Fixed credential refresh

* Add a top-level require that matches the gem name

Reduce confusion by allowing the gem to be required via a file that
matches the gem name.

References fog/fog#3959

* fix fog#369

* update CHANGELOG.md

* Bump fog-aws to 1.4.0

* Fix AWS credential mocking

The change introduced in fog#252 does not actually prevent HTTP requests
from being made because of a missing early return statement.

* Ruby 1.8 compat

* Add MaxResults filter to describe reserved instances offreings
@jeffjo
Copy link

jeffjo commented Aug 18, 2017

can you cut a new release with this change?

@geemus
Copy link
Member

geemus commented Aug 21, 2017

@jeffjo Looks like this was originally merged quite some time ago, and should have been in quite a few releases since then. Are you seeing issues related to it?

@jeffjo
Copy link

jeffjo commented Aug 21, 2017

Woops, I commented in the wrong issue. I was referring to the fix for #374.

@geemus
Copy link
Member

geemus commented Aug 23, 2017

@jeffjo ah ha, I suspected it may have just been a mistake on the target. I'll comment over on that issue.

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

Successfully merging this pull request may close these issues.

4 participants