iann0036 / former2

Generate CloudFormation / Terraform / Troposphere templates from your existing AWS resources.
https://former2.com
MIT License
2.2k stars 266 forks source link

Incorrect types for some VPC resource properties #58

Open reidca opened 4 years ago

reidca commented 4 years ago

I have cfn-lint installed for VSCode and after loading the template that former2 generated the following warnings were shown:

E3012: Property Resources/CustomerGateway/Properties/BgpAsn should be of type Integer E3012: Property Resources/ec2b729b8b/Properties/Protocol should be of type Integer E3034: Value has to be between 1 and 32766 at Resources/ec29e8c64b/Properties/RuleNumber

Relevant snippet of template:

ec29e8c64b:
        Type: "AWS::EC2::NetworkAclEntry"
        Properties:
            CidrBlock: "0.0.0.0/0"
            Egress: true
            NetworkAclId: !Ref NetworkAcl1
            Protocol: "-1"
            RuleAction: "deny"
            RuleNumber: 32767
iann0036 commented 4 years ago

Cheers @reidca,

Got the first two in. I never did type checking when doing my first full mapping run so there's likely a bunch of these around (you probably know that it won't break a stack creation, just annoys the linter).

I'm interpreting the response from the last one as the default network ACL entry and therefore I shouldn't include it, so removed it from the output. Let me know if you disagree.

reidca commented 4 years ago

Are you happy for me to keep raising these as I come across them or is that just annoying?

One other thing I noticed is that the resource types tend to be enclosed in quotes which I don't think is necessary and makes (IMHO) the template uglier!!

e.g.

Type: "AWS::EC2::VPC"

Could be: Type: AWS::EC2::VPC

iann0036 commented 4 years ago

Happy for you to continue raising the type differences.

The quote encapsulation is a stack formatting opinion which I generally avoid making changes to. Not quoting some strings can lead to issues in YAML with octals etc. I'd recommend running it through cfn-template-flip with the clean option to get it in the state you'd like.

reidca commented 4 years ago

I have used cfn-flip for sometime, I raised a similar issue on that repo with the formatting output of the clean function disagreeing with cfn-lint: https://github.com/awslabs/aws-cfn-template-flip/issues/48

Ideally the tooling would be based on some reference standard which everyone could agree on regardless of personal preference.

Are you saying that the quoting is the same for all output so it could not be removed just for the types?

iann0036 commented 4 years ago

Are you saying that the quoting is the same for all output so it could not be removed just for the types?

It could, but I'd personally prefer to keep it consistent for the entire template or have a setting that can be changed based on preference. Worthwhile to think about though 🤔