Missing struct field if value is 0

[karnauskas]

I see that some struct fields with 0 as value are not rendered in yaml or json. This is causing problems.

Eg in following code DeviceIndex is missing in final output.

...
import gcf "github.com/awslabs/goformation/cloudformation"
...
			NetworkInterfaces: []gcf.AWSEC2LaunchTemplate_NetworkInterface{
				gcf.AWSEC2LaunchTemplate_NetworkInterface{
					AssociatePublicIpAddress: true,
					DeviceIndex:              0,
					Groups: []string{
						gcf.Ref("SomeSg"),
					},
				},
			},

is rendered to in yaml:

        NetworkInterfaces:
        - AssociatePublicIpAddress: true
          Groups:
          - Ref: SomeSg

or json:

"NetworkInterfaces": [
            {
              "AssociatePublicIpAddress": true,
              "Groups": [
                {
                  "Ref": "SomeSg"
                }
              ]
            }
          ]

[PaulMaddox]

This should be handled by this merged PR:
ab8ea0f

However as there are no tests for it, it’s possible this has regressed. I will add some tests and look into what’s going wrong.

[andrewmyhre]

@PaulMaddox did you have a chance to look into this? This issue is present in v2.3.0 and is making it impossible to create launch templates using cloud formation.

e.g: https://github.com/awslabs/goformation/blob/v2.3.0/cloudformation/resources/aws-ec2-launchtemplate_networkinterface.go#L27

[spiffcs]

@PaulMaddox We've been able to fix this issue by dropping the omitempty tag from the struct here:
https://github.com/awslabs/goformation/blob/v2.3.0/cloudformation/resources/aws-ec2-launchtemplate_networkinterface.go#L27

If you want I can make a pr that removes this so that users are able to supply an int of 0 as an argument.

[kanbara]

I am still seeing this with the ReturnData boolean of a CloudWatchAlarm_MetricDataQuery, as the struct tag is json:omitempty the false is not getting written, however this means AWS interprets the Metric ReturnData as true, so it is impossible to use.

[kanbara]

On second thought, the issue occurs because ReturnData is not required BUT it can be falsey. Because of AWS' behaviour of making this omission into a truthy value, we need to actually make the ReturnData field a pointer, otherwise it will not be able to be set.

[paxan]

Same problem happens when defining ECS services that have DesiredCount=0.

This attribute is not declared as required in the model and that’s why zero values are omitted. But there are situations when you want this attribute to be zero, unfortunately.

My workaround was to create my own struct that implemented Resource interface, and had only fields I needed without those JSON annotations but matching by name and type with the ECS service resource structure of this lib.

[razor-1]

This seems to be a pervasive problem. I encountered it with ecs.Service_DeploymentConfiguration in trying to set MinimumHealthyPercent to 0. There needs to be much more careful handling of default vs. zero values in goformation

[PaulMaddox]

I agree.

I think this library needs to adopt the approach the AWS Go SDK took, with making field values pointers (so they can represent zero, or not-set/null separately).

Unfortunately this will be a breaking change, and will require the library to migrate from using primitive types directly such as:

subscription := &sns.Subscription{
    Protocol: "email",
    Endpoint: "some.email@example.com",
}

to using pointers (and helper functions) like this:

subscription := &sns.Subscription{
    Protocol: aws.String("email"),
    Endpoint: aws.String("some.email@example.com"),
}

This will be quite a bit of work to implement, and as mentioned will be a breaking change. I'm not sure i'll be able to start working on this until next year - but would happily accept a pull request for it if somebody else has more time.

[razor-1]

Assuming that aws/aws-cdk#547 is getting worked on, at least I would be happy if this were solved there, and that was the go-forward direction.

[PaulMaddox]

Agreed - having Go support in the CDK would be awesome, and would deprecate this library for the vast majority of use cases. I'm afraid I can't offer any insight into timelines as to when that will be the case though.

[razor-1]

Hi, wanted to check in and see if it's still a worthwhile plan to address this as described above

[snowiow]

I also ran into this problem today. The threshold of a cloudwatch alarm is omitted, because I want to have an alarm for everything greater than 0. However when putting the metric I get the following error: "PutMetricAlarm request should have valid Threshold parameter"

Based on the aws-cli documentation, threshold is required for all static alarms:
"This parameter is required for alarms based on static thresholds, but should not be used for alarms based on anomaly detection models." (https://docs.aws.amazon.com/cli/latest/reference/cloudwatch/put-metric-alarm.html)

[manuel-trejo-rico]

#254 is also related to this issue

[xrn]

@PaulMaddox FYI

• Language Support: Golang  aws/jsii#83
• https://github.com/aws/jsii/projects/2#card-42596833

[xrn]

@rubenfonseca if I am correct this would be solved in v6 looking on behaviour of my issue - #474

[rubenfonseca]

Correct, thank you!