r/aws_launch_template: some more fixes

[stefansundin]

The first fix is trivial: device_index can be 0. I don't know if it's even desirable to keep it optional, since the console defaults the value to 0 if you do not fill in the field. If you add multiple devices and leave the index empty, the console will also increment them automatically, i.e. "eth0", "eth1", etc. Maybe we should do the same? I haven't tried launching an instance with a network interface without an index, so if that breaks then this is probably desirable.

The second fix is very incomplete. The thing is that almost all of the boolean values are really optional, but terraform defaults them to false. There is no way to leave them undefined. So terraform is not able to fully configure those properties. I tried making it a string, that way we can have the values "", "true", and "false". But then we lose the ability to just use true or false without quotes in the terraform code, which is kind of confusing for users.

I first tried this to only set it if it was defined as either true or false, but it did not work.

if v, ok := ni["associate_public_ip_address"]; ok {
	networkInterface.AssociatePublicIpAddress = aws.Bool(v.(bool))
}

I tried to use Default: nil like this, but it did not work. Perhaps the schema could be enhanced to allow this to make the code above work?

"associate_public_ip_address": {
	Type:     schema.TypeBool,
	Optional: true,
	Default:  nil,
},

But it still defaulted to false.

I then tried this, which did work, but there must be a better way.

"associate_public_ip_address": {
	Type:     schema.TypeString,
	Optional: true,
	ValidateFunc: validation.StringInSlice([]string{
		"true",
		"false",
	}, false),
},

So this affects any property that can be true, false, or undefined ("Do not include in template"). See screenshot:

I was going to fix the network interface security groups, but thankfully @kl4w opened his PR first which is much better than my fix. #4364

[bflad]

@stefansundin during create/update does if v, ok := d.GetOkExists("attribute_name"); ok {workaround the boolean issue? We can do full if apiAttribute != nil { checks during read to prevent setting the attribute instead of just using aws.BoolValue() (which will default to false)

[bflad]

I'm guessing neither will help in this situation though 🙁

[bflad]

I think we might want to making each fix/enhancement individual PRs. Starting to see lots of overlapping work in this resource. See also #4344

[bflad]

Can you please switch to just this change so we can close #4475 for now?

[joernheissler]

This might not suffice. I had the resource created without this change, then changed the code, then did terraform apply again. Terraform wasn't going to update/recreate the resource, I had to destroy it manually.

[bflad]

Did it automatically assign device_index 0? I believe that's the behavior when its not supplied (auto incrementing from there).

[joernheissler]

No, it was missing completely. DescribeLaunchTemplateVersions returned:

"NetworkInterfaces": [
    {
        "AssociatePublicIpAddress": false,
        "DeleteOnTermination": false,
        "Ipv6AddressCount": 0,
        "SecondaryPrivateIpAddressCount": 0,
        "SubnetId": "subnet-xxxxxxxx"
    }
],

[stefansundin]

@bflad Done, sorry about the delay. I haven't had time to check back on this.

[ayarmoluk17]

Can we merge this one in? launch template is still unusable with ASG because of this. Using template with ASG in VPC requires device index of 0 https://docs.aws.amazon.com/autoscaling/ec2/userguide/create-asg-launch-template.html

With fix merged it works perfectly for me.

[bflad]

Thanks so much for this @stefansundin -- let's get this in to fix handling for ASGs 🚀

[bflad]

This has been released in version 1.19.0 of the AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!