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

Add Support for Shorthand CloudFormation Syntax #5327

Merged
merged 3 commits into from
Oct 2, 2018

Conversation

laardee
Copy link
Member

@laardee laardee commented Sep 24, 2018

What did you implement:

This PR adds support for shorthand CloudFormation syntax, e.g. !Ref, !GetAtt and !Join.

This should not be a breaking change, least I don't know any cases where !Something syntax is used in serverless.yml.

How did you implement it:

This feature leverages js-yaml module's custom schema.

How can we verify it:

service: shorthand-cf

provider:
  name: aws
  runtime: nodejs6.10
  iamRoleStatements:
    - Effect: "Allow"
      Action:
        - "kms:Decrypt"
      Resource: !Join
        - ":"
        - - "arn:aws:kms"
          - !Ref AWS::Region
          - !Ref AWS::AccountId
          - ":key/CMK_key_ID"
    - Effect: "Allow"
      Action:
        - "s3:*"
      Resource:
        - !Join
          - ":"
          - - "arn:aws:s3::"
            - !Ref "MyBucket"
        - !Join
          - ":"
          - - "arn:aws:s3::"
            - !Ref "MyBucket"
            - "/*"

functions:
  hello:
    handler: handler.hello

resources:
  Conditions:
    AndCondition: !And 
      - !Not [!Equals ["prod", "${opt:stage, self:provider.stage}"]]
      - !Not [!Equals ["staging", "${opt:stage, self:provider.stage}"]]

  Resources:
    MyBucket:
      Type: AWS::S3::Bucket
      Properties:
        Tags:
          - Key: FruitTag # fruit tag is the most obvious use case
            Value: !Select [ !If [AndCondition, "0", "1"], [ "apples", "grapes" ] ]

  Outputs:
    BucketName:
      Description: "My bucket name"
      Value: !Ref MyBucket
    BucketArn:
      Description: "My Bucket arn"
      Value: !GetAtt MyBucket.Arn

Todos:

  • Write tests
  • Write documentation
  • Fix linting errors
  • Make sure code coverage hasn't dropped
  • Provide verification config / commands / resources
  • Enable "Allow edits from maintainers" for this PR
  • Update the messages below

Is this ready for review?: YES
Is it a breaking change?: NO

@horike37 horike37 added this to the 1.33.0 milestone Sep 26, 2018
@horike37
Copy link
Member

Thank you for this @laardee 👍
Imo, this is AWS provider specific feature so it would be better to put those code under serverless/lib/plugins/aws/, otherwise this may introduce any side effect to the other providers.

What do you think?

@laardee
Copy link
Member Author

laardee commented Sep 27, 2018

@horike37 yes, that subset of shorthand tags is AWS specific. These tags need to be specified on the first YAML.load, where the schema is passed to the js-yaml. What could be done is that the schema creation would be in e.g. lib/plugins/aws/lib/cloudformation-schema.js, and it would be used in the lib/utils/fs/parse.js only if the contents of the file has /name:\s+aws/ (or some better regexp) <- that needs to be checked before YAML.load which parses the buffer to the object.

Edit: The /name:\s+aws/ regexp doesn't work, because if there are external files that need to be parsed, those don't have that.

One option could be first to try to parse the file without CloudFormation schema, if that throws YAMLException, then try to parse with CloudFormation schema, if that throws error, then try to parse with some other provider schema (if available), and so on. If the last try still errors, that will be thrown then, else the parsed data is returned.

@horike37
Copy link
Member

horike37 commented Oct 2, 2018

@laardee

One option could be first to try to parse the file without CloudFormation schema, if that throws YAMLException, then try to parse with CloudFormation schema, if that throws error, then try to parse with some other provider schema (if available), and so on. If the last try still errors, that will be thrown then, else the parsed data is returned.

Thank you for the explanation 👍 Understood what you stated.
I think this is a best plan to me so far.
LGTM.

@horike37 horike37 merged commit ea7bea4 into serverless:master Oct 2, 2018
@laardee laardee deleted the add-shorthand-cf branch October 6, 2018 18:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants