feat!: Raise minimum required Terraform version to 1.0+ #331

bryantbiggs · 2023-04-28T12:30:20Z

Description

Raise minimum required Terraform version to 1.0+
Add support for ignoring changes to the instances AMI ID; this does not apply to the spot instance since that is less applicable to living for a long duration
Update default metadata_options to enforce IMDSv2 for stronger security posture
Update t-type logic to account for t4g instance type

Motivation and Context

Resolves Allow AMI change to be ignored #325
Resolves bump versions.tf for module 4.3.1 #330

Breaking Changes

Yes

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

antonbabenko · 2023-04-28T14:23:04Z

main.tf

+
+  lifecycle {
+    ignore_changes = [
+      ami


I see you like to have copy-paste just to have this use-case covered :)

Its such a bad hack! @AdamTylerLynch use your influence to help us avoid having to use these ugly hacks 🙏🏽 😬 There are valid use cases where you want to initialize a value and then forget it, but we cannot do that in Terraform in a clean way currently (nor give users the ability to specify which values they wish to ignore which means we have to pick and choose which ones we are going to hardcode into the module)

antonbabenko

Minor comments here and there.

antonbabenko · 2023-04-28T14:30:17Z

main.tf

-      device_name  = ephemeral_block_device.value.device_name
-      no_device    = lookup(ephemeral_block_device.value, "no_device", null)
-      virtual_name = lookup(ephemeral_block_device.value, "virtual_name", null)
+      device_name  = try(ephemeral_block_device.value.device_name, each.key)


This should be reverted (in both resources) because var.ephemeral_block_device is a list.

whoops, ya I tried to use this and forgot we had it as a list. Looks like I only corrected the EBS block device, I'll revert all of these each.keys

antonbabenko · 2023-04-28T14:30:59Z

main.tf

    }
  }

  dynamic "network_interface" {
    for_each = var.network_interface
+
    content {
      device_index          = network_interface.value.device_index
      network_interface_id  = lookup(network_interface.value, "network_interface_id", null)


Probably try() here and in all (or almost all other places in this file), too.

I suspect we'll have users who are wrapping this module in their own module, so I avoided converting any arguments that might be passed as computed attributes to try() to avoid https://github.com/clowdhaus/terraform-for-each-unknown

So any of the kms_key_id, snapshot_id, network_interface_id, etc. I left as lookup()s so that they don't throw the unknown computed value error when wrapped in another module

antonbabenko

LGTM

## [5.0.0](v4.5.0...v5.0.0) (2023-04-28) ### ⚠ BREAKING CHANGES * Raise minimum required Terraform version to 1.0+ (#331) ### Features * Raise minimum required Terraform version to 1.0+ ([#331](#331)) ([9d4e0ca](9d4e0ca))

antonbabenko · 2023-04-28T20:59:02Z

This PR is included in version 5.0.0 🎉

dhoppe · 2023-05-09T14:56:49Z

@bryantbiggs Has this been tested with the new variable ignore_ami_changes? I get the following error message, but only if the value of this variable is set to true.

╷
│ Error: Output refers to sensitive values
│
│   on outputs.tf line 146:
│  146: output "ami" {
│
│ To reduce the risk of accidentally exporting sensitive data that was
│ intended to be only internal, Terraform requires that any root module
│ output containing sensitive data be explicitly marked as sensitive, to
│ confirm your intent.
│
│ If you do intend to export this data, annotate the output value as
│ sensitive by adding the following argument:
│     sensitive = true
╵
`´´

github-actions · 2023-06-10T02:17:02Z

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.

feat!: Raise minimum required Terraform version to 1.0+

80c9fd6

bryantbiggs mentioned this pull request Apr 28, 2023

bump versions.tf for module 4.3.1 #330

Closed

1 task

bryantbiggs added 3 commits April 28, 2023 08:30

Merge branch 'master' into feat/version-bump

d4fce70

feat: Add the ability to ignore AMI changes on instance

660d13c

chore: Add examples README

dec2236

bryantbiggs marked this pull request as ready for review April 28, 2023 14:20

bryantbiggs requested a review from antonbabenko April 28, 2023 14:21

antonbabenko reviewed Apr 28, 2023

View reviewed changes

antonbabenko mentioned this pull request Apr 28, 2023

Allow AMI change to be ignored #325

Closed

antonbabenko reviewed Apr 28, 2023

View reviewed changes

fix: Remove each.key usage due to list variable type

2669d46

antonbabenko approved these changes Apr 28, 2023

View reviewed changes

bryantbiggs merged commit 9d4e0ca into terraform-aws-modules:master Apr 28, 2023

bryantbiggs deleted the feat/version-bump branch April 28, 2023 20:58

github-actions bot locked as resolved and limited conversation to collaborators Jun 10, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat!: Raise minimum required Terraform version to 1.0+ #331

feat!: Raise minimum required Terraform version to 1.0+ #331

bryantbiggs commented Apr 28, 2023 •

edited

Loading

antonbabenko Apr 28, 2023

bryantbiggs Apr 28, 2023

antonbabenko left a comment

antonbabenko Apr 28, 2023

bryantbiggs Apr 28, 2023

antonbabenko Apr 28, 2023

bryantbiggs Apr 28, 2023

antonbabenko left a comment

antonbabenko commented Apr 28, 2023

dhoppe commented May 9, 2023

github-actions bot commented Jun 10, 2023

feat!: Raise minimum required Terraform version to 1.0+ #331

feat!: Raise minimum required Terraform version to 1.0+ #331

Conversation

bryantbiggs commented Apr 28, 2023 • edited Loading

Description

Motivation and Context

Breaking Changes

How Has This Been Tested?

antonbabenko Apr 28, 2023

Choose a reason for hiding this comment

bryantbiggs Apr 28, 2023

Choose a reason for hiding this comment

antonbabenko left a comment

Choose a reason for hiding this comment

antonbabenko Apr 28, 2023

Choose a reason for hiding this comment

bryantbiggs Apr 28, 2023

Choose a reason for hiding this comment

antonbabenko Apr 28, 2023

Choose a reason for hiding this comment

bryantbiggs Apr 28, 2023

Choose a reason for hiding this comment

antonbabenko left a comment

Choose a reason for hiding this comment

antonbabenko commented Apr 28, 2023

dhoppe commented May 9, 2023

github-actions bot commented Jun 10, 2023

bryantbiggs commented Apr 28, 2023 •

edited

Loading