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

Chalice generate-pipeline can create yaml incompatible with Cloudformation #450

Closed
corespace opened this issue Jul 31, 2017 · 7 comments
Closed
Labels

Comments

@corespace
Copy link

Hi folks,

I was working on a chalice app recently and decided to try out the chalice generate-pipeline feature. My app looked something like this:

from chalice import Chalice

app = Chalice(app_name='docGen')

@app.route('/some-path', methods=['POST'])
def generate_stuff():
    json_body = app.current_request.json_body
    return do_the_thing(json_body)

I've left out some details, obviously, but this is enough to see the problem. When I ran generate pipeline and deployed that pipeline, it built out a CloudFormation yaml that included this:

Resources:
  APIHandler:
    Properties:
      CodeUri: s3://some-s3-bucket
      Events:
        generate_stuffpostc666:
          Properties:
            Method: post
            Path: /some-path
            RestApiId:
              Ref: RestAPI
          Type: Api
      Handler: app.app
      MemorySize: 128
      Role: some-arn
      Runtime: python2.7
      Tags:
        aws-chalice: version=0.10.1:stage=dev:app=docGen
      Timeout: 60
    Type: AWS::Serverless::Function

Cool, right? Nope. Cloudformation barfed on this saying something to the effect "Event names must be alphanumeric" I don't have the exact error message anymore cause I went to retrieve it today and the ChangeSet is gone. Anyway, I created a test to reproduce the problem, but I'm not sure how you folks would want to rectify the problem. You can see it here: https://github.com/aws/chalice/compare/master...corespace:view_name?expand=1

@achautha
Copy link
Contributor

achautha commented Aug 1, 2017

I guess event name is generated from the view.view_name + HTTPMethod + digest. In this case generated one is generate_stuffpostc666 which is not acceptable as cloudformation accepts only alphnumeric. Possible fix is to remove non alphnumeric characters from view.view_name and then generated the event name. I have a fix in mind . Can I raise a pull request ?

@achautha
Copy link
Contributor

achautha commented Aug 1, 2017

@corespace, In your test case, you may want to check for other non alphanumeric characters too.

@corespace
Copy link
Author

Yeah I wanted a better test, but it's not clear to me which characters are allowed and which are not. I assume alphanumeric means letters and number but I couldn't find any documentation confirming that. Hence why I haven't offered a fix either.

@stealthycoin
Copy link
Contributor

Thanks for reporting this one. Marking it as a bug.

@achautha
Copy link
Contributor

achautha commented Aug 1, 2017

@stealthycoin, Can I take this up ? I have found the fix.

@stealthycoin
Copy link
Contributor

@achautha Sure feel free to open a pull request.

achautha pushed a commit to achautha/chalice that referenced this issue Aug 1, 2017
achautha pushed a commit to achautha/chalice that referenced this issue Aug 1, 2017
achautha pushed a commit to achautha/chalice that referenced this issue Aug 2, 2017
jamesls pushed a commit that referenced this issue Aug 4, 2017
jamesls added a commit that referenced this issue Aug 4, 2017
Merges #453

* achautha-issue_450:
  Add entry to changelog
  Make the test more specific about what it's checking
  Issue #450: unit tests to verify the fix
  Chalice generate-pipeline can create yaml incompatible with Cloudformation
@jamesls
Copy link
Member

jamesls commented Aug 4, 2017

Fixed in #453

@jamesls jamesls closed this as completed Aug 4, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants