fix: Assignment of the Capacity Reservation id to an instance

[schniber]

Description

When trying to call the module with the specification of a Capacity Reservation ID, the module ends up with the following error:

╷
│ Error: Invalid function argument
│
│ on .terraform/modules/terraform-aws-ec2-instance/main.tf line 45, in resource "aws_instance" "this":
│ 45: capacity_reservation_id = lookup(capacity_reservation_target.value, "capacity_reservation_id", null)
│ ├────────────────
│ │ capacity_reservation_target.value is "cr-xxxxxxxx"
│
│ Invalid value for "inputMap" parameter: lookup() requires a map as the
│ first argument.
╵

As per line 45 analysis, it looks like the bug is in the following statement:

capacity_reservation_id = lookup(capacity_reservation_target, "capacity_reservation_id", null)

it should rather be as follows:

capacity_reservation_id = lookup(capacity_reservation_specification.value.capacity_reservation_target, "capacity_reservation_id", null)

In this case, terraform should lookup in the capacity reservation target map (since it is the iterator in the for_each) rather than in its value for the mapping of the attribute capacity reservation id

Motivation and Context

Fixes #277 and #281

Breaking Changes

No breaking changes, just fixing a typo in line 45 in the main.tf

How Has This Been Tested?

• I have updated at least one of the examples/* to demonstrate and validate my change(s)
• I have tested and validated these changes using one or more of the provided examples/* projects

• I have executed pre-commit run -a on my pull request

[schniber]

@bryantbiggs : Hello Bryant, I noticed that my logic on the previous PR on the same topic was wrong.

In this PR I have revisited this statement in the main.tf:

capacity_reservation_id = lookup(capacity_reservation_specification.value.capacity_reservation_target, "capacity_reservation_id", null)

I also tested thoroughly using the complete example and this is now satisfactory.

Sorry again for the double review work.

Enjoy your weekend !

[schniber]

Hello @bryantbiggs,

kind reminder for this PR which is ready for review.

Thanks again for your feedback.

Bests.

[bryantbiggs]

I believe we should be able to copy this https://github.com/terraform-aws-modules/terraform-aws-autoscaling/blob/master/main.tf#L73-L86 to here

[bryantbiggs]

I think we want one targeted example https://github.com/terraform-aws-modules/terraform-aws-autoscaling/blob/master/examples/complete/main.tf#L468-L472

and one open example https://github.com/terraform-aws-modules/terraform-aws-autoscaling/blob/master/examples/complete/main.tf#L222-L224

so we'll need to add an open example

[schniber]

Hello @bryantbiggs

Thanks for your valuable feedback that I have taken into account.

I have added the example for the targeted capacity reservation together with the simplification of the capacity reservation assignment. it's more readable with try function.

I have also deployed to test and well went through.

Looking forward for the merge if all is okay for you.

Thanks a ton !

Bests.

[schniber]

Hello @bryantbiggs

Would it be possible to review the changes I made upon your feedback ?

Thanks again.

Bests.

[bryantbiggs]

Thank you!

[antonbabenko]

This PR is included in version 4.1.2 🎉

[yewton]

Should the default value of capacity_reservation_specification be updated from null to {} ?

[bryantbiggs]

@yewton good spot - yes, looks like we need to update the variable as well. PRs are welcomed!

[schniber]

I will create a new PR to fix it.
Thanks for spotting it !

[yewton]

Thank you!

[github-actions]

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.