8000 follow up on CloudFormation template URLs S3 URI by bentsku · Pull Request #9329 · localstack/localstack · GitHub
[go: up one dir, main page]
Skip to content

follow up on CloudFormation template URLs S3 URI #9329

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Oct 11, 2023
Merged

follow up on CloudFormation template URLs S3 URI #9329

merged 1 commit into from
Oct 11, 2023

Conversation

bentsku
Copy link
Contributor
@bentsku bentsku commented Oct 10, 2023 •
edited
Loading

Motivation

Follow up from #9312 and following up a Community Slack issue, it seems the new validation added was missing one exception handling when the S3 key would be containing slashes.

For example:
awslocal cloudformation create-stack --stack-name vpc-us-east-1 --template-url s3://cloudformation-templates/aws-account-cfn/vpc.json

Is raising:

File "/opt/code/localstack/localstack/services/cloudformation/api_utils.py", line 55, in get_template_body
    result = client.get_object(Bucket=parts[0], Key=parts[2])
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/code/localstack/.venv/lib/python3.11/site-packages/botocore/client.py", line 535, in _api_call
    return self._make_api_call(operation_name, kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/code/localstack/.venv/lib/python3.11/site-packages/botocore/client.py", line 980, in _make_api_call
    raise error_class(parsed_response, operation_name)
botocore.errorfactory.NoSuchBucket: An error occurred (NoSuchBucket) when calling the GetObject operation: The specified bucket does not exist

Because the split occurring in convert_s3_to_local_url was returning 3 parts so it worked.

If I understood well, s3://-type URIs are not accepted in --template-url parameter, is that right? So I believe if the scheme is s3, we could trigger the validation error straight away.

Weirdly, this is an accepted answer by AWS employees... https://repost.aws/questions/QUvDk684FkRW2wvFLybXoSuA/templateurl-must-be-a-supported-url
But testing shows that it's not supported?

Adding a link showing how the url should look:
https://aws.amazon.com/blo 8000 gs/infrastructure-and-automation/best-practices-for-using-amazon-s3-endpoints-in-aws-cloudformation-templates/

\cc @whummer

Changes

Added a check for the scheme, and raises the exception straight if the scheme is s3.
Added the test to have a long key with / in it to trigger the issue if not fixed.

@bentsku bentsku added aws:cloudformation AWS CloudFormation semver: patch Non-breaking changes which can be included in patch releases labels Oct 10, 2023
@bentsku bentsku self-assigned this Oct 10, 2023
@coveralls
Copy link

Coverage Status

coverage: 82.958% (+0.02%) from 82.943% when pulling b7dd743 on s3-template-cfn-s3-uri into 5716c17 on master.

@github-actions
Copy link

LocalStack Community integration with Pro

       2 files         2 suites   1h 22m 54s ⏱️
2 250 tests 1 749 ✔️ 501 💤 0 ❌
2 251 runs  1 749 ✔️ 502 💤 0 ❌

Results for commit b7dd743.

@simonrw simonrw mentioned this pull request Oct 11, 2023
Closed
1 task
simonrw
simonrw approved these changes Oct 11, 2023
View reviewed changes
Copy link
Contributor
@simonrw simonrw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like a worthwhile change, thanks for adding!

@bentsku bentsku merged commit c129376 into master Oct 11, 2023
@bentsku bentsku deleted the s3-template-cfn-s3-uri branch October 11, 2023 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@simonrw simonrw simonrw approved these changes

@dominikschubert dominikschubert Awaiting requested review from dominikschubert dominikschubert is a code owner

@pinzon pinzon Awaiting requested review from pinzon pinzon is a code owner

Assignees

@bentsku bentsku

Labels
aws:cloudformation AWS CloudFormation semver: patch Non-breaking changes which can be included in patch releases
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@bentsku @coveralls @simonrw
0