Add optional network_interface_id variable

[gbarna-bd]

what

• Add optional network_interface_id variable to launch_template

why

• Allow the attaching of existing network interfaces

references

• https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/launch_template#network_interface_id

[Gowiem]

Hey @gbarna-bd -- this looks good and I think we can move it forward. I made one suggestion and asked for additional clarification on the subnets vs AZs addition. Check those out and let me know your thoughts.

Also, to get this merged we'll need to update our README. Please run the following commands, commit, and push the changes:

make init
make github/init
make readme

[Gowiem]

One request on removing the default for subnet_ids, but otherwise this looks ready to ship.

[Gowiem]

/terratest

[gbarna-bd]

Do you need me to rebase (reorder and squash / fixup) my commits?

[Gowiem]

/terratest

[Gowiem]

@gbarna-bd nope, no need to squash. Since we only squash merge that avoids any ugliness in the history in main.

[Gowiem]

@gbarna-bd we're seeing the following failure:

We've dealt with this elsewhere, here is an example on how to fix: https://github.com/cloudposse/terraform-aws-lambda-function/pull/46/files

[gbarna-bd]

@gbarna-bd we're seeing the following failure:

We've dealt with this elsewhere, here is an example on how to fix: https://github.com/cloudposse/terraform-aws-lambda-function/pull/46/files

Thanks! I've wrapped the for_each in data.subnet_id.this in a ternary condition.

[max-lobur]

/terratest

[gbarna-bd]

Is the failing test something that I can try to run locally?
So far, I've validated the PR with:

cd test
make init && make all

[Gowiem]

@gbarna-bd I haven't run the tests locally in a while as its a bit painful. Maybe @max-lobur can speak to that. Anyway, your error is the following:

Take a look at the example fix that I shared before. You want to create that map of index => subnet_it like they do on this line and provide that as the value to for_each. Terraform is stupid and just can't figure out the graph without annoying workarounds like that.

[gbarna-bd]

Replaced tomap() with a map comprehension / expression. Hopefully it works now :-)

[Gowiem]

/terratest

[Gowiem]

/terratest

[gbarna-bd]

Finally!
Thanks for your help and patience with this one, @Gowiem.

[Gowiem]

@gbarna-bd thanks for your hard work + patience on this one! Should be a release that rolls out shortly.