awslabs / goformation

GoFormation is a Go library for working with CloudFormation templates.
Apache License 2.0
847 stars 195 forks source link

Intrisic helper Select redering base64 encoded value #362

Open apgmckay opened 3 years ago

apgmckay commented 3 years ago

There seems to be an issue with rendering the to JSON or YAML from the Select intrinsic helper function.

See the below render:

    "VPCSubnet2": {
      "Properties": {
        "AvailabilityZone": "eyAiRm46OlNlbGVjdCI6IFsgJ1x4MDAnLCAgImV1LXdlc3QtMWEiIF0gfQ==",
        "CidrBlock": {
          "Ref": "Subnet2CIDR"
        },
        "VpcId": {
          "Ref": "VPC"
        }
      },

which is produced by:

template.Resources["VPCSubnet2"] = &ec2.Subnet{
CidrBlock:        cloudformation.Ref("Subnet2CIDR"),
VpcId:            cloudformation.Ref("VPC"),
AvailabilityZone: cloudformation.Select(0, []string{"eu-west-1a"}),
}

I initially thought this was when you embedded calls to intrisic helpers within one another for example see render:

    "VPCSubnet3": {
      "Properties": {
        "AvailabilityZone": "eyAiRm46OlNlbGVjdCI6IFsgJ1x4MDInLCAgImV5QWlSbTQ2T2tkbGRFRmFjeUk2SUNKbGRTMTNaWE4wTFRFaUlIMD0iIF0gfQ==",
        "CidrBlock": {
          "Ref": "Subnet3CIDR"
        },
        "VpcId": {
          "Ref": "VPC"
        }
      },

Produced from:

    template.Resources["VPCSubnet3"] = &ec2.Subnet{
        CidrBlock:        cloudformation.Ref("Subnet3CIDR"),
        VpcId:            cloudformation.Ref("VPC"),
        AvailabilityZone: cloudformation.Select(2, []string{cloudformation.GetAZs("eu-west-1")}),
    }

However the issue only seems to be when using Select, as calls from GetAZs work as expected, see below:

    "VPCSubnet1": {
      "Properties": {
        "AvailabilityZone": {
          "Fn::GetAZs": "eu-west-1"
        },
        "CidrBlock": {
          "Ref": "Subnet1CIDR"
        },
        "VpcId": {
          "Ref": "VPC"
        }
      },

Produced from:

    template.Resources["VPCSubnet1"] = &ec2.Subnet{
        CidrBlock:        cloudformation.Ref("Subnet1CIDR"),
        VpcId:            cloudformation.Ref("VPC"),
        AvailabilityZone: cloudformation.GetAZs("eu-west-1"),
    }

I'm making use of the goformation JSON() and YAML() render functions and both produce the same resulting hash. Which makes me think the issue is probably somewhere in the Select() codebase.

Please let me know if you need any more detail.

Thanks for the great work so far with goformation!

tnsardesai commented 3 years ago

Using cloudformation.Select("2", []string{cloudformation.GetAZs("eu-west-1")}) works instead of cloudformation.Select(2, []string{cloudformation.GetAZs("eu-west-1")}), (note 2 int vs string)

This is happening because of %q at https://github.com/awslabs/goformation/blob/master/cloudformation/intrinsics.go#L205-L210 for index.

Reading https://golang.org/pkg/fmt/ see that for integer it says %q a single-quoted character literal safely escaped with Go syntax.

https://gist.github.com/tnsardesai/b697eefd35c09dfeed4cde5eaf535851

{ "Fn::Select": [ '\x00' ] }
not json: 
invalid character '\'' looking for beginning of value
apgmckay commented 3 years ago

That's worked thanks very much for taking the time to look into this @tnsardesai.

xrn commented 2 years ago

@rubenfonseca I had also problems in past with cloudformation.Select(2, and is required cloudformation.Select("2", what do you think about change of

// Select returns a single object from a list of objects by index.
func Select(index interface{}, list []string) string {
    if len(list) == 1 {
        return encode(fmt.Sprintf(`{ "Fn::Select": [ %q,  %q ] }`, index, list[0]))
    }
    return encode(fmt.Sprintf(`{ "Fn::Select": [ %q, [ %v ] ] }`, index, printList(list)))
}

to

// Select returns a single object from a list of objects by index.
func Select(index string, list []string) string {
    if len(list) == 1 {
        return encode(fmt.Sprintf(`{ "Fn::Select": [ %q,  %q ] }`, index, list[0]))
    }
    return encode(fmt.Sprintf(`{ "Fn::Select": [ %q, [ %v ] ] }`, index, printList(list)))
}

or

// Select returns a single object from a list of objects by index.
func Select(index int, list []string) string {
    if len(list) == 1 {
        return encode(fmt.Sprintf(`{ "Fn::Select": [ %v,  %q ] }`, index, list[0]))
    }
    return encode(fmt.Sprintf(`{ "Fn::Select": [ %v, [ %v ] ] }`, index, printList(list)))
}
rubenfonseca commented 2 years ago

Hi everyone, thank you for the input! I was thinking changing the Sprintf template for the index from %q to %v as it would cover a lot of scenarios automatically:

The default format for %v is: bool: %t int, int8 etc.: %d uint, uint8 etc.: %d, %#x if printed with %#v float32, complex64, etc: %g string: %s chan: %p pointer: %p For compound objects, the elements are printed using these rules, recursively, laid out like this: struct: {field0 field1 ...} array, slice: [elem0 elem1 ...] maps: map[key1:value1 key2:

Does this make sense to solve this and other problems?

xrn commented 2 years ago

I believe this is good idea