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

Bad session token error masked if not creating a new bucket #65

Open
kimvanwyk opened this issue Jan 19, 2022 · 3 comments
Open

Bad session token error masked if not creating a new bucket #65

kimvanwyk opened this issue Jan 19, 2022 · 3 comments
Labels
bug Something isn't working

Comments

@kimvanwyk
Copy link

If a bad or expired session token is set, the create command fails with a misleading error that an existing bucket doesn't exist, if --create-bucket isn't specified. If --create-bucket is specified a traceback with more info is given instead:

export AWS_ACCESS_KEY_ID="..."
export AWS_SECRET_ACCESS_KEY="..."
export AWS_SESSION_TOKEN="EXPIRED_TOKEN" 

$ s3-credentials create --username USERNAME BUCKET
Error: Bucket does not exist: BUCKET - try --create-bucket to create it

$ s3-credentials create --create-bucket --username USERNAME BUCKET
Traceback (most recent call last):
  File "/home/kimv/work/data_engineering/sbsa_archive/.venv/bin/s3-credentials", line 8, in <module>
    sys.exit(cli())
  File "/home/kimv/work/data_engineering/sbsa_archive/.venv/lib/python3.9/site-packages/click/core.py", line 1128, in __call__
    return self.main(*args, **kwargs)
  File "/home/kimv/work/data_engineering/sbsa_archive/.venv/lib/python3.9/site-packages/click/core.py", line 1053, in main
    rv = self.invoke(ctx)
  File "/home/kimv/work/data_engineering/sbsa_archive/.venv/lib/python3.9/site-packages/click/core.py", line 1659, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/home/kimv/work/data_engineering/sbsa_archive/.venv/lib/python3.9/site-packages/click/core.py", line 1395, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/home/kimv/work/data_engineering/sbsa_archive/.venv/lib/python3.9/site-packages/click/core.py", line 754, in invoke
    return __callback(*args, **kwargs)
  File "/home/kimv/work/data_engineering/sbsa_archive/.venv/lib/python3.9/site-packages/s3_credentials/cli.py", line 314, in create
    s3.create_bucket(Bucket=bucket, **kwargs)
  File "/home/kimv/work/data_engineering/sbsa_archive/.venv/lib/python3.9/site-packages/botocore/client.py", line 391, in _api_call
    return self._make_api_call(operation_name, kwargs)
  File "/home/kimv/work/data_engineering/sbsa_archive/.venv/lib/python3.9/site-packages/botocore/client.py", line 719, in _make_api_call
    raise error_class(parsed_response, operation_name)
botocore.exceptions.ClientError: An error occurred (ExpiredToken) when calling the CreateBucket operation: The provided token has expired.
@simonw simonw added the bug Something isn't working label Jan 19, 2022
@simonw
Copy link
Owner

simonw commented Jan 20, 2022

I think the root problem is in this function:

def bucket_exists(s3, bucket):
try:
s3.head_bucket(Bucket=bucket)
return True
except botocore.exceptions.ClientError:
return False

It looks like it's turning all errors - not just "bucket does not exist" but also "authentication failed" - into a False which is then treated by this code as meaning the bucket did not exist:

# Create bucket if it doesn't exist
if dry_run or (not bucket_exists(s3, bucket)):
if (not dry_run) and (not create_bucket):
raise click.ClickException(
"Bucket does not exist: {} - try --create-bucket to create it".format(
bucket
)
)

@simonw
Copy link
Owner

simonw commented Jan 20, 2022

I'm tempted to define a new AuthenticationFailed exception as a subclass of click.ClickException which I can then raise from inside that function.

A tiny bit inelegant since it's tied to Click, but I can unwind it into a separate click-free exception if I ever implement the Python library idea in #32.

@kimvanwyk
Copy link
Author

I'm tempted to define a new AuthenticationFailed exception as a subclass of click.ClickException which I can then raise from inside that function.

A tiny bit inelegant since it's tied to Click, but I can unwind it into a separate click-free exception if I ever implement the Python library idea in #32.

This looks like a good way to catch these errors that aren't actually related to missing buckets.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants