botify-labs / simpleflow

Python library for dataflow programming.
https://botify-labs.github.com/simpleflow/
MIT License
68 stars 24 forks source link

Support for AWS_SECURITY_TOKEN? ... or support for default credentials chain provider #346

Closed mookerji closed 5 years ago

mookerji commented 6 years ago

Hi friends!

Thank you so much for all your hard work with this library. SWF is definitely an under appreciated AWS service and I've had some really good experiences using simpleflow as part of a data engineering project.

We're in the middle of introducing token-based developer credentials and it seems that neither the simpleflow nor the boto2 SWF client propagate security tokens (= AWS_SECURITY_TOKEN) through, making it hard to use the library on a local developer machine. Because of what seems to be a type/argument ordering error, you can't pass a token from Layer1 object in boto2, and using the published API will fail.

I believe this problem is bypassed by removing the custom handling around AWS creds in ConnectedSWFObject here: https://github.com/botify-labs/simpleflow/blob/master/swf/core.py#L45. By default, the AWS auth handler will use the (apparently very complicated) provider chain logic to find the right credentials This case and the failure are described below:

import boto.swf

# Assume these are valid and exported in the environment
AWS_SECRET_ACCESS_KEY='<redacted>'
AWS_ACCESS_KEY_ID='<redacted>'
AWS_SECURITY_TOKEN='<redacted>'
AWS_SESSION_TOKEN='<redacted>'

kwargs = {'aws_access_key_id': AWS_SECRET_ACCESS_KEY, 'aws_secret_access_key': AWS_ACCESS_KEY_ID, 'session_token': AWS_SECURITY_TOKEN}

kwargs1 = {'aws_access_key_id': AWS_SECRET_ACCESS_KEY, 'aws_secret_access_key': AWS_ACCESS_KEY_ID}

# Will work, using the env vars
boto.swf.connect_to_region('us-west-2').list_domains('REGISTERED')

# Will fail using explicit environments
boto.swf.connect_to_region('us-west-2', **kwargs).list_domains('REGISTERED')
boto.swf.connect_to_region('us-west-2', **kwargs1).list_domains('REGISTERED')

The latter two will throw this exception:

Traceback (most recent call last):
  File "test_creds.py", line 15, in <module>
    print(conn.list_domains('REGISTERED'))
  File "/usr/local/lib/python3.6/site-packages/boto/swf/layer1.py", line 1456, in list_domains
    'reverseOrder': reverse_order,
  File "/usr/local/lib/python3.6/site-packages/boto/swf/layer1.py", line 118, in json_request
    return self.make_request(action, json_input, object_hook)
  File "/usr/local/lib/python3.6/site-packages/boto/swf/layer1.py", line 145, in make_request
    raise excp_cls(response.status, response.reason, body=json_body)
boto.exception.SWFResponseError: SWFResponseError: 400 Bad Request
{'__type': 'com.amazon.coral.service#UnrecognizedClientException', 'message': 'The security token included in the request is invalid.'}

Thoughts ? Based on https://github.com/botify-labs/simpleflow/issues/104#issuecomment-237375320, I'm getting the sense that the custom credentials handling through the settings file has been deprecated.

jbbarth commented 6 years ago

Hi @mookerji thanks for the kind words and I'd be extremely happy to discover what you're doing with simpleflow!

Your guesses are correct, the custom credentials handling is pretty much useless but we never touched it since it just "works" for us for now. But as you bring a related problem to the table, I'd gladly get rid of it in favor of something more standard.

Same goes for boto2: we never switched to boto3 mostly by lack of time and also because I'm not 100% sure moto (mock library for boto) swf mocks would work on boto3.

Anyway, I won't have the time to work on this on the next 2 months, so if you're up for it, maybe you can start testing things in a branch on your side and open a PR here? I can't promise we'll merge things immediately, it will depend on the changes you make, but I'll review it and at some point before the end of the year it can totally land in a stable release.

Works for you? Thanks again!

mookerji commented 6 years ago

@jbbarth Happy to put up a branch: I'm hoping to make some very minimal changes and you can let me know if I've broken an internal use case.

re: boto2: definitely answers another question I had, but possibly less pressing.

Totally works for me! Thanks for the quick reply.

mookerji commented 6 years ago

Related, submitted a patch to boto2 as well: https://github.com/boto/boto/pull/3832.