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

moto 1.3.8 breaks real boto3 access #2172

Closed
dmulter opened this issue Apr 29, 2019 · 18 comments
Closed

moto 1.3.8 breaks real boto3 access #2172

dmulter opened this issue Apr 29, 2019 · 18 comments

Comments

@dmulter
Copy link

dmulter commented Apr 29, 2019

Just installed the latest 1.3.8 release and it appears to have broken my tests that make actual non-mocked STS calls when the test file includes moto. I've narrowed down the issue and created a minimal example to show the problem.

Test code in file moto38.py:

import json
import boto3
from moto import mock_sts


policy = {
    'Version': '2012-10-17',
    'Statement': [
        {
            'Effect': 'Allow',
            'Action': '*',
            'Resource': '*'
        }
    ]
}

sts = boto3.client('sts')
response = sts.assume_role(
    RoleArn='arn:aws:iam::12345678:role/xxx',
    RoleSessionName='yyy',
    Policy=json.dumps(policy),
    DurationSeconds=900
)

Now run the test and note the error output with exception InvalidClientTokenId:

Traceback (most recent call last):
  File "/Users/dmulter/Desktop/moto38.py", line 22, in <module>
    DurationSeconds=900
  File "/Users/dmulter/Documents/projects/xxx/.virtualenv/lib/python3.7/site-packages/botocore/client.py", line 357, in _api_call
    return self._make_api_call(operation_name, kwargs)
  File "/Users/dmulter/Documents/projects/xxx/.virtualenv/lib/python3.7/site-packages/botocore/client.py", line 661, in _make_api_call
    raise error_class(parsed_response, operation_name)
botocore.exceptions.ClientError: An error occurred (InvalidClientTokenId) when calling the AssumeRole operation: The security token included in the request is invalid.

If I fall back to moto==1.3.7 the test gets past this error. Note that I didn't bother making the policy and role work properly, but in my tests the real values work as expected when using the previous moto version.

@dmulter
Copy link
Author

dmulter commented Apr 29, 2019

I did a quick check for S3 API access and get a similar failure with an InvalidAccessKeyId exception. I'm thinking maybe the new before-send code is clobbering the access key ID for all boto3 calls even for non-mocked functions.

@timbeccue
Copy link

timbeccue commented May 3, 2019

I have a similar issue. It seems like importing the moto module prevents normal aws usage.

This works:

import boto3 
from moto import mock_s3

@mock_s3
def test():
    conn = boto3.resource('s3', region_name='us-east-1')
    conn.create_bucket(Bucket='a-totally-unique-moto-bucket-name')

test()

This also works (no moto at all)

import boto3 
#from moto import mock_s3

#@mock_s3
def test():
    conn = boto3.resource('s3', region_name='us-east-1')
    conn.create_bucket(Bucket='a-totally-unique-moto-bucket-name')

test()

But this throws an InvalidAccessKeyId error:

import boto3 
from moto import mock_s3
 
#@mock_s3
def test():
    conn = boto3.resource('s3', region_name='us-east-1')
    conn.create_bucket(Bucket='a-totally-unique-moto-bucket-name')

test()
botocore.exceptions.ClientError: An error occurred (InvalidAccessKeyId) when calling the CreateBucket operation: The AWS Access Key Id you provided does not exist in our records.

@dmulter dmulter changed the title moto 1.3.8 breaks real STS access moto 1.3.8 breaks real boto3 access May 6, 2019
@tomerben
Copy link

tomerben commented Jun 23, 2019

It also breaks real boto (2) access. I believe the root cause is this commit - cf5bd76, setting 2 env variables (AWS_ACCESS_KEY_ID , AWS_SECRET_ACCESS_KEY) in moto/core/models.py. They can impact boto connection initialization on every flow running after the models.py is imported.

This works fine:
>>> import boto
>>> conn = boto.connect_s3()
>>> conn.access_key

'XXXXXXX'

>>> import moto
>>> conn.access_key

'XXXXXXX'

>>> conn.get_bucket('mybucket')

<Bucket: mybucket>

While this breaks:
>>> import boto
>>> import moto
>>> conn = boto.connect_s3()
>>> conn.access_key
'foobar_key'

>>> conn.get_bucket('mybucket')
Traceback (most recent call last): File "<stdin>", line 1, in <module> File "...lib/python3.6/site-packages/boto/s3/connection.py", line 509, in get_bucket return self.head_bucket(bucket_name, headers=headers) File "...lib/python3.6/site-packages/boto/s3/connection.py", line 542, in head_bucket raise err boto.exception.S3ResponseError: S3ResponseError: 403 Forbidden

@kayadanae
Copy link

+1 Also being impacted by this issue, behavior is consistent with what others have described

@jukafah
Copy link

jukafah commented Jun 27, 2019

+1 Currently have to place tests that import moto into a different directory and execute them separately to get around this. Would be great to run them as part of the same test execution.

@spulec
Copy link
Collaborator

spulec commented Jul 8, 2019

Can someone please try the PR here? #2285

@efiop
Copy link
Contributor

efiop commented Jul 18, 2019

@spulec Tried and it works perfectly. Looking forward to it being merged. Thank you!

spulec added a commit that referenced this issue Jul 20, 2019
Move env variable mocking and undo when stopping. CC #2058, #2172.
@spulec
Copy link
Collaborator

spulec commented Jul 20, 2019

Great. I've merged it: #2285

@spulec spulec closed this as completed Jul 20, 2019
@ch3ck
Copy link

ch3ck commented Jul 23, 2019

@spulec when is this fix going to be released to PyPi? because pulling from moto=="*" does not pull the #2285 PR. We had no choice than to lock down our moto package to the master branch which is definitely not healthy

@efiop
Copy link
Contributor

efiop commented Jul 24, 2019

@ch3ck It has been released in a pre-release https://pypi.org/project/moto/#history . You could use it for now.

@danielcarletti
Copy link

@ch3ck Still not released to PyPi, I'm using a horrible workaround but it works: import moto inside the fixture definition. I only have one for now and it's not breaking my code, so I'll stick to it until a stable release.

@spulec
Copy link
Collaborator

spulec commented Sep 12, 2019

@danielcarletti as noted in the above comment, we have prereleases on every commit. Are those not working for you?

@dmulter
Copy link
Author

dmulter commented Oct 24, 2019

Pre-release packages are fine, but then I have to pin that specific version. Is there any plan for when the next official release is expected? I would like to keep just pip install -U moto.

@spulec
Copy link
Collaborator

spulec commented Oct 31, 2019

pip install -U --pre moto ?

@dmulter
Copy link
Author

dmulter commented Oct 31, 2019

I have moto installed and updated regularly with lots of other packages as part of many Python projects. I think it is a normal expectation that packages do stable releases at some periodic frequency. Specifically installing a pre-release would not be part of such a normal process. Just looking to know when the next stable release is for moto. I love the capabilities of moto and I look forward to the next official release.

@spulec
Copy link
Collaborator

spulec commented Oct 31, 2019

Sorry for my admittedly terse response.

I think it is a normal expectation that packages do stable releases at some periodic frequency.

I've been having discussions with various people and trying to really understand the value of this for Moto. Unlike a normal package, we are always adding new endpoints/resources. When we cut releases, there isn't anything additional that is done to ensure it is "stable". The releases aren't any different than the prereleases.

I'll work to cut a release in the next couple days, but I do think our current process is broken. I'm tempted to either have every commit make a real release or do something like pytz and have monthly releases (we would automate them). My fear with both of these is giving a false sense of security to people.

Thoughts?

@dmulter
Copy link
Author

dmulter commented Oct 31, 2019

I would suggest that they are all official releases then. I agree that a monthly "official" release would present a false sense of stability of the release. The boto3 and botocore packages have the same (and even larger) frequency of updates, and they are all official releases. I think that's a better parallel with moto releases. I would rather detect issues with a specific release and pin the version if I run into any issues, which I have to do in rare cases with boto3. This is a common pattern.

@bblommers
Copy link
Collaborator

When we cut releases, there isn't anything additional that is done to ensure it is "stable"

This seems like the root of the issue here, regardless of when releases are. Does it make sense to create a (small, limited) set of integration tests? Int tests can pull the latest dev version, and verify that real life use-cases (like the ones above) still work.
Happy to help out with setting that up of course - I realize that it does involve some work initially, but it does give people an appropriate sense of stability.

With regards to automated releases (regardless of the frequency) :

  1. How would we ensure that the Changelog is updated in a meaningful manner?
  2. How would we differentiate between major/minor/patch releases?

Cutting manual releases is admittedly more work - but does get around these two issues, and as long as integration tests are run manually it also ensures release stability.

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

10 participants