goformation icon indicating copy to clipboard operation
goformation copied to clipboard

Intrisic helper Select redering base64 encoded value

Open apgmckay opened this issue 3 years ago • 5 comments

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!

apgmckay avatar Mar 27 '21 07:03 apgmckay

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

tnsardesai avatar Apr 01 '21 23:04 tnsardesai

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

apgmckay avatar Apr 02 '21 15:04 apgmckay

@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)))
}

xrn avatar Jul 18 '22 11:07 xrn

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?

rubenfonseca avatar Sep 02 '22 09:09 rubenfonseca

I believe this is good idea

xrn avatar Sep 03 '22 18:09 xrn