add role-based credentials to COPY and UNLOAD command

[mtrbean]

Closes #87

Todos

• MIT compatible
• Tests
• Documentation
• Updated CHANGES.rst

[graingert]

I'd probably put iam_role as a variable here.

[graingert]

@mtrbean can you clean up your commits, and tick all the boxes as appropriate

[mtrbean]

I have moved iam_role to the test functions and squashed the commits.

[jklukas]

The way this is written, iam_role takes precedence over access_key_id/secret_access_key/session_token, but the documentation doesn't make this explicit. Providing both types of credentials seems like it indicates confusion on the part of the user, and I'd propose that we raise an error in that case indicating that the two credential styles are mutually exclusive and should not be mixed.

So, if within this if block, we'd check if any of access key ID, secret access key, or session token are not None, and then raise an error.

[graingert]

probably best to create two classes, so you can use the API like:

.copy(access=APIKeyCredential(access_key_id, secret_access_key, session_token), ...)
.copy(access=IAMRoleCredential(iam_role), ...)

[jklukas]

probably best to create two classes

I'd be fine with that. Would we want to keep the existing access_key_id, secret_access_key, session_token options for backward-compatibility and mark them as deprecated?

[graingert]

I'd like to stick with the code as is for now, maybe add a TypeError

I've added it to my personal v1-breaking-change-wishlist

[graingert]

@mtrbean can you check split the iam_role into aws_account_id and iam_role_name validating both against regexes.

if iam_role_name is not None:
    if not AWS_ACCOUNT_ID_RE.match(aws_account_id):
        raise ValueError(...)
    if not IAM_ROLE_NAME_RE.match(iam_role_Name):
        raise ValueError(...)

    credentials = 'aws_iam_role=arn:aws:iam::{aws_acount_id}:role/{iam_role_name}'.format(...)

[jklukas]

I'd like to stick with the code as is for now, maybe add a TypeError

I'm fine with that.

[graingert]

currently the iam_role_name regex is .* allowing "; DROP TABLE foobar; --" which will match invalid roles.

[graingert]

iam role: "Maximum 64 characters. Use alphanumeric and '+=,.@-_' characters"
aws account id: 12-digit number

[mtrbean]

Changes made, let me know if anything further changes needed

[graingert]

looks good, squash your commits though

[mtrbean]

Squashed. I think you can also make Github squash the commits for you when you merge.

[graingert]

I can it's nicer not to though as it looses the merge commit
On 16 May 2016 7:29 pm, "Eric Wong" [email protected] wrote:

Squashed. I think you can also make Github squash the commits for you when
you merge.

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

[mtrbean]

@graingert can this be merged?

[graingert]

@mtrbean merged! thanks :)