moto setting AWS_ACCESS_KEY_ID / AWS_SECRET_ACCESS_KEY on import

[thehesiod]

Here: https://github.com/spulec/moto/blob/master/moto/core/models.py#L26

These should only be set if you call a mock method, otherwise just importing moto can break various other calls which are not supposed to be mocked

[bpandola]

Just curious: what's your use-case where you import moto but also make actual calls to AWS?

[thehesiod]

IIRC we have a secretTool which connects to dynamodb to retrieve secrets. I need to get the secret as I just want to mock S3 and not an API call which requires a secret.

[thehesiod]

another instance: I need to connect to our secret store to retrieve a SSL cert required to mock a SSL server.

[lhufnagel]

Hmm yeah, I worried that edge case might happen.. is there a chance you can restructure your code to separate the imports? Or include a os.environ.setdefault("AWS_ACCESS_KEY_ID", "") (i.e. with empty string) before importing moto? IIRC then boto3 should try the other credential providers

[thehesiod]

ya I have a work-around for now, but ideally this should not be done on import

[PiotrNowicki]

This might also bite you in other case. Assume you have a project with unit and integration tests.

For unit tests you want to mock everything outside of your code so you use moto.
For integration tests you might want to hit the actual AWS resources.

Now if you use pytest and use @pytest.mark.integration to distinguish which test is unit and which one is integration and you run them like this:

• python -m pytest -m "not integration" # for unit tests only
• python -m pytest -m integration # for integration tests only

You'll end up with running integration tests with moto populated:

os.environ.setdefault("AWS_ACCESS_KEY_ID", "foobar_key")
os.environ.setdefault("AWS_SECRET_ACCESS_KEY", "foobar_secret")

Which obviously is not what you want. I think that's because you're running scan for the annotations for all tests and you then hit the import for unit tests and you end up with AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY being populated by moto.

A workaround here is to keep the tests in separate directory ("/integration" in my case) and then run it with:

python -m pytest tests/integration
(alternatively add -m integration to be extra safe that no unit tests in this package will be executed.)

Nevertheless, this is yet another workaround and if we'd have an option to control if it would be great. Something like MOTO_DISALLOW_AWS_CREDENTIALS env_var or switch?
By default -- not to break backward compatibility -- it might be turned off.

For boto3 env properties are very high in the hierarchy or loading process.
Higher are only in-code values which I don't want to hard-code for security and configuration reasons.
Overriding env properties during building is also not an option as it's not secure and it will be anyway overridden by moto.

[spulec]

Can someone give the PR here a try and let me know if it solves the issue? #2285

[thehesiod]

@spulec isn't that the same thing effectively? After you patch a particular service, anyone who requests os.environ['AWS_ACCESS_KEY_ID'] for example will get the fake key. It should be that only requests to mocked services should get the mock key. The way I achieved this locally was to explicitly set the access_key_id, etc in the create_client call.

[spulec]

Well, this moved from happening on import to only happening when the moto context is activated so that seemed like an improvement.

I understand what you are saying though. Given that boto/boto3 requires some form of authentication, I'm not sure what other options we have. It seems too painful to force everyone to change the client instantiations. We might be able to mock credentials via instance metadata, but that has been a struggle in the past.

[thehesiod]

can't you instead temporarily monkeypatch the create_client method like I was suggesting and override the credentials there? I think that would be marginally better than setting environment variables.

[spulec]

That assume the client is created after the mocking is activated, yeah? I don't know how popular it is, but I have seen a number of people that setup their clients at the module level.

[thehesiod]

I think it's counter-intuitive to create a client, then say you want to enable mocking s3, and then have that retroactively affect clients you've already created. Perhaps this warrants a major version change.

…

On Mon, Jul 15, 2019 at 6:47 PM Steve Pulec ***@***.***> wrote: That assume the client is created after the mocking is activated, yeah? I don't know how popular it is, but I have seen a number of people that setup their clients at the module level. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#2058>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA6Q77JVEUWZ7FZ4WBFDSQTP7USDVANCNFSM4GXJL2IA> .

[spulec]

Hmmmm, I'm not sure. Given that the clients are lazy (at least in boto3), I don't have a good feel for how people think about them.

Just to make sure I understand: is there a problem with the current approach? If it is only that a user reading the environment variables will see something different while the mock is activated, that doesn't seem like a huge deal to me (though please let me know if there is a use-case I'm not considering with this).

[thehesiod]

Here's the scenario, before or after I enable s3 moto I create a dynamodb client to get secrets, i then enable s3 Moto, now my secrets calls fail. I need this secret because it contains the password for the mock DB we create. This is marginally better in that after import, but before enabling s3 mocking I can pull the password, but ideally mocking s3 should not affect dynamodb calls.

[spulec]

Ah, I see.

Because of various issues we've had in the past, I get extra nervous about allowing any sort of real connections to AWS when a user has any service activated. Some more here. I have even caught myself a few times where I forget to add the decorator for a particular service and am thankful that Moto throws an exception. I don't keep real credentials available, but it is clear that much of our userbase does.

As far as defaults go, I'm tempted to lean toward anything AWS not working. If there is a way we could allow an explicit override of that, I would be in favor though.

[thehesiod]

Perhaps then there should be an explicit global moto context to inform users what's happening

[spulec]

I'm not sure I understand. Are you talking something like #2076 or something different?

[spulec]

Since we fixed this to no longer happen on import, I am going to close this issue. If someone wants to take a shot at making this even less intrusive, I'm open to ideas.