awslabs/aws-cfn-template-flip

cfn_tools.dump_yaml strips quotes, causing loaded YAML template invalid

canhnt opened this issue · 8 comments

canhnt commented

Description

When parsing YAML files containing string scalar longer than 8 chars and start with 0, the function cfn_tools.dump_yaml strips single (and double) quotes, which converts string scalar to number scalar. When CFN templates refers these string values in Ref or Sub functions, it causes template error.

Package version: 1.2.0

$ pip show cfn_flip
Name: cfn-flip
Version: 1.2.0
Summary: Convert AWS CloudFormation templates between JSON and YAML formats
Home-page: https://github.com/awslabs/aws-cfn-template-flip
Author: Steve Engledow
Author-email: sengledo@amazon.co.uk
License: Apache2
Location: /usr/local/lib/python2.7/site-packages
Requires: six, PyYAML, Click
Required-by:

Test-cases

1. Test case '012345678'

import cfn_tools
content = "Parameters:\n  MyAccountId:\n    Type: String\n    Default: '012345678'"
body = cfn_tools.load_yaml(content)
template_body = cfn_tools.dump_yaml(body)
print template_body

Expected output:

Parameters:
  MyAccountId:
    Type: String
    Default: '012345678'

Actual output:

Parameters:
  MyAccountId:
    Type: String
    Default: 012345678

2. Test-case '01234567'

import cfn_tools
content = "Parameters:\n  MyAccountId:\n    Type: String\n    Default: '01234567'"
body = cfn_tools.load_yaml(content)
template_body = cfn_tools.dump_yaml(body)
print template_body

Output:

Parameters:
  MyAccountId:
    Type: String
    Default: '01234567'
canhnt commented

Workaround solution:

import cfn_tools
import cfn_flip

content = "Parameters:\n  MyAccountId:\n    Type: String\n    Default: '012345678'"
body = cfn_tools.load_yaml(content)
template_body_json = cfn_tools.dump_json(body)
template_body_yaml = cfn_flip.to_yaml(template_body_json)
print template_body_yaml

Output

Parameters:
  MyAccountId:
    Type: String
    Default: '012345678'
koiker commented

This is related with the pyYaml and this: yaml/pyyaml#98

But I will work in a workaround to deal with AWS AccountId in the conversion. :-)

  • 👍1
koiker commented

The PR: #79 should fix the issue.

canhnt commented

Thank you for your fix. I think it would be nice when PR is merged to master, we can close the issue. Otherwise the bug is still valid in the the latest branch (master).

koiker commented

Sure, I've closed too early :-)

stilvoid commented

Fixed in #79 :)

canhnt commented

Thanks. It'll be released in the next tag, i.e. after v1.2.0

stilvoid commented

Yes. After the next PR is merged, I'll be releasing as v1.2.1. Hopefully today.