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

Support new YAML template features #31

Closed
robert2d opened this issue Sep 22, 2016 · 13 comments
Closed

Support new YAML template features #31

robert2d opened this issue Sep 22, 2016 · 13 comments

Comments

@robert2d
Copy link

CloudFormation Now Supports YAML.

Looks like stackup will need some design and then development given its strict JSON template handling: https://github.com/realestate-com-au/stackup/blob/master/lib/stackup/stack.rb#L115 (as an example)

Im going to have a look into converting a small stack in the next week or so, and will try attempt a YAML stackup refactor with that stack in both JSON and YAML. I'll at least try and provide any relevant info if and when possible.

References

Jeff's blog and release notes:
https://aws.amazon.com/blogs/aws/aws-cloudformation-update-yaml-cross-stack-references-simplified-substitution/

CloudFormation Formats documentation:
https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/template-formats.html

@mdub
Copy link
Contributor

mdub commented Sep 24, 2016

Actually, stackup has always supported templates (and parameters) in YAML format. Internally, it's just data, which we then serialise to JSON when invoking the CloudFormation API.

I see no advantage in uploading the original YAML to CloudFormation, do you?

@mdub mdub closed this as completed Sep 24, 2016
@robert2d
Copy link
Author

Perfect, thanks Mike.

@mdub mdub reopened this Sep 26, 2016
@mdub
Copy link
Contributor

mdub commented Sep 26, 2016

I've reopened this because, while we already support YAML, we don't support the new "abbreviated syntax for tags", e.g. !GetAtt vs Fn::GetAtt.

@mdub mdub changed the title Support YAML Templates Support new YAML template features Sep 26, 2016
@lukeck
Copy link
Contributor

lukeck commented Sep 27, 2016

I'm not sure how much of an advantage it is but if a template is uploaded in YAML, get-template gives it back to you in YAML.

@mdub
Copy link
Contributor

mdub commented Sep 27, 2016

@lukeck: Damn. If that's true it means we probably can't away with converting the template into a canonical form (i.e. data) within Stackup; instead, we'll have to retain the original template text.

@tigris
Copy link
Contributor

tigris commented Oct 5, 2016

This thread is relevant to my interests.

@mdub
Copy link
Contributor

mdub commented Oct 5, 2016

I'm kind of pissed at the way AWS have chosen to use (abuse?) YAML tags, in their adoption of YAML, e.g.

Outputs:
  LoadBalancerAddress:
    Value: !GetAtt
    - LoadBalancer
    - DNSName

as a shorthand for

Outputs:
  LoadBalancerAddress:
    Value: 
      Fn::GetAtt:
      - LoadBalancer
      - DNSName

As I understand it, tags are intended to denote types. The really annoying thing is that the first example above doesn't even round-trip via YAML!

irb> puts YAML.dump(YAML.load(raw))
---
Outputs:
  LoadBalancerAddress:
    Value:
    - LoadBalancer
    - DNSName

FFS! I wish they'd just treated YAML as an alternative serialisation format.

Anyway, If we want Stackup to support the new syntax, we have two options:

A. retain input template format/body (awscli-compatible)
B. parse-time conversion to data (backward-compatible)

Option A: retain input template format

To be fully compatible with what the AWS CLI (now) does, we'd have to retain the original YAML template text, and send it intact up to CloudFormation. As a result:

  • AWS would take take of the funky tag handling
  • we wouldn't lose tags used in the input YAML
  • Stackup's behaviour would change (where we now send JSON, we'd be sending YAML)
  • Stackup internals would have to change (because we currently treat templates as Ruby data)
  • we would no longer be "normalising" the template at load time, so "diffs" would get funky, if either the format (JSON/YAML) for formatting (e.g. amount of whitespace) changed

Option B: parse-time conversion

A simpler change is to stick with our current approach - converting the input JSON/YAML to data within Stackup - and just add some magic to convert the tags to their equivalent JSON-compatible forms, e.g. !GetAtt to Fn::GetAtt. And then:

  • most of Stackup would be unchanged
  • diffs would continue to work
  • bit it's a bit lossy, as tags are converted to their more verbose form
  • going forward, we would have to match any other "YAML magic" AWS chose to implement

@lukeck
Copy link
Contributor

lukeck commented Oct 6, 2016

I'm not in love with option A but option B looks like it's a recipe for an ongoing headache.

@mdub
Copy link
Contributor

mdub commented Oct 6, 2016

I'm not sure it's that much of an ongoing headache, @lukeck. The logic I've implemented in #32 is basically:

  • if you see !Ref, substitute Ref
  • if you see !SomethingElse, substitute Fn::SomethingElse

See

def accept(target)
case target.tag
when "!Ref"
{ "Ref" => super }
when /^!(\w+)$/
{ "Fn::#{$1}" => super }
else
super
end
end

@mdub
Copy link
Contributor

mdub commented Oct 7, 2016

Closed with #32.

@mdub mdub closed this as completed Oct 7, 2016
@benhutchison
Copy link

Stackup's implementation doesn't seem to support YAML AWS pseudo-parameter substitution via !Sub .

  SubscribeLambda:
    Type: "AWS::SNS::Subscription"
    Properties:
      Endpoint: !Sub |
        arn:aws:lambda:${AWS::Region}:${AWS::AccountId}:function:addBuyProfile
      Protocol: lambda
      TopicArn: !Sub |
        arn:aws:sns:${AWS::Region}:${AWS::AccountId}:cbr-profiler

I concur with @lukeck assessment that option (b) will be a headache, due to this sort of thing

@benhutchison
Copy link

benhutchison commented Mar 9, 2017

Additionally, there seems to be a trailing newlines that gets inserted into ARN strings in YAML, making them fail AWS validation

Template:

Resources:
  SnsPermission:
    Type: "AWS::Lambda::Permission"
    Properties:
      Action: lambda:InvokeFunction
      FunctionName: !Sub |
        arn:aws:lambda:ap-southeast-2:123456789012:function:myLambda
      Principal: sns.amazonaws.com
      SourceArn: !Sub |
        arn:aws:sns:ap-southeast-2:123456789012:*

Result (note the newline that has been retained at end of ARN is whats failing the regex validation)

INFO: [15:35:50] SnsPermission - CREATE_FAILED - 2 validation errors detected: Value 'arn:aws:sns:ap-southeast-2:123456789012:*
' at 'sourceArn' failed to satisfy constraint: Member must satisfy regular expression pattern: arn:aws:([a-zA-Z0-9\-])+:([a-z]{2}-[a-z]+-\d{1})?:(\d{12})?:(.*)

@mdub
Copy link
Contributor

mdub commented Mar 9, 2017

It should support !Sub, @benhutchison. I think the problem is that by using |, you've got newlines in the source YAML. Try without them, e.g.

 SubscribeLambda:
    Type: "AWS::SNS::Subscription"
    Properties:
      Endpoint: !Sub arn:aws:lambda:${AWS::Region}:${AWS::AccountId}:function:addBuyProfile
      Protocol: lambda
      TopicArn: !Sub arn:aws:sns:${AWS::Region}:${AWS::AccountId}:cbr-profiler

If that still doesn't work, please raise a separate issue.

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

No branches or pull requests

5 participants