rd/aws_instance and rd/aws_launch_template: Add support for metadata_options #11076

stefansundin · 2019-12-01T07:46:12Z

This adds support to set the Metadata Options options for an EC2 instance. TL;DR: opt-in to a more secure metadata service.

Documentation:

I am hopeful that they will add support for this to launch templates as well, but it seems like this has not happened yet. We can wait a few weeks to see if it comes out after reInvent.

Please let me know what you want to be changed. I'm not sure to what extent you want this to be tested. I've been testing this a bit and it is working well. I was unsure if you wanted the state attribute to be made available?

Community Note

Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
Please do not leave "+1" comments, they generate extra noise for pull request followers and do not help prioritize the request

Release note for CHANGELOG:

resource/aws_instance: Add support for `metadata_options` parameter
data-source/aws_instance: Add `metadata_options` attributes
resource/aws_launch_template: Add support for `metadata_options` parameter
data-source/aws_launch_template: Add `metadata_options` attributes

Output from acceptance testing:

$ make testacc TEST=./aws TESTARGS='-run=TestAccAWSInstance_basic'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSInstance_basic -timeout 120m
=== RUN   TestAccAWSInstance_basic
=== PAUSE TestAccAWSInstance_basic
=== CONT  TestAccAWSInstance_basic
--- PASS: TestAccAWSInstance_basic (162.39s)
PASS
ok    github.com/terraform-providers/terraform-provider-aws/aws 164.053s

$ make testacc TEST=./aws TESTARGS='-run=TestAccAWSInstanceDataSource_basic'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSInstanceDataSource_basic -timeout 120m
=== RUN   TestAccAWSInstanceDataSource_basic
=== PAUSE TestAccAWSInstanceDataSource_basic
=== CONT  TestAccAWSInstanceDataSource_basic
--- PASS: TestAccAWSInstanceDataSource_basic (253.76s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	255.474s

stefansundin · 2019-12-15T19:43:15Z

It was confirmed in the SEC310 reInvent session that launch template support is due in the next few weeks: https://youtu.be/2B5bhZzayjI?t=1966

…form-provider-aws into metadata_options

…_response_hop_limit.

stefansundin · 2020-01-31T06:28:43Z

It seems that the launch template support was launched without being announced anywhere (at least not as far as I could see). I just looked for it in the API docs today, and I saw that it had been added. I spent some time to add the launch template support, so I think this PR is ready for review now.

My launch_template test is failing with this error:

$ make testacc TEST=./aws TESTARGS='-run=TestAccAWSLaunchTemplate_data'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSLaunchTemplate_data -timeout 120m
=== RUN   TestAccAWSLaunchTemplate_data
=== PAUSE TestAccAWSLaunchTemplate_data
=== CONT  TestAccAWSLaunchTemplate_data
--- FAIL: TestAccAWSLaunchTemplate_data (15.84s)
    testing.go:640: Step 0 error: Check failed: Check 13/20 error: aws_launch_template.test: Attribute 'metadata_options.#' expected "1", got "0"
FAIL
FAIL  github.com/terraform-providers/terraform-provider-aws/aws 17.508s
FAIL
make: *** [testacc] Error 1

I suspect this is because the metadata options are "pending" before fully applied to the launch template? It may be an error (i.e. copy-paste from the EC2 instances API), since there's something called LaunchTemplateInstanceMetadataOptionsStatePending in the SDK docs. It doesn't make a whole lot of sense to me since it shouldn't need to update a running thing, just publish a new version of the launch template.

Anyway, let me know what to do here. Thanks.

…aunch_template.

bflad

Hey @stefansundin 👋 Thank you for submitting this. I left some initial feedback below. Please take a look and reach out if you have any questions or do not have time to implement the items.

bflad · 2020-02-12T19:42:50Z

aws/data_source_aws_instance.go

+		mo["http_put_response_hop_limit"] = instance.MetadataOptions.HttpPutResponseHopLimit
+		var metadataOptions []map[string]interface{}
+		metadataOptions = append(metadataOptions, mo)
+		d.Set("metadata_options", metadataOptions)


When using d.Set() with aggregate types (TypeList, TypeSet, TypeMap), we should perform error checking to prevent issues where the code is not properly able to set the Terraform state. e.g.

Suggested change

d.Set("metadata_options", metadataOptions)

if err := d.Set("metadata_options", metadataOptions); err != nil {

return fmt.Errorf("error setting metadata_options: %s", err)

}

The existing code may not be working since the map elements have pointer types, which can be fixed by wrapping the response fields with aws.StringValue(), aws.Int64Value(), etc.

bflad · 2020-02-12T19:45:08Z

aws/data_source_aws_instance_test.go

@@ -450,6 +453,11 @@ resource "aws_instance" "test" {
  # us-west-2
  ami = "ami-4fccb37f"
  instance_type = "m1.small"
+  metadata_options {


Instead of modifying an existing test function and test configuration, can you please create a new test and configuration so we can ensure there are no regressions with existing functionality? Copy-paste-modify is fine. Thanks. 👍

bflad · 2020-02-12T19:46:39Z

aws/diff_suppress_funcs.go

@@ -132,3 +132,10 @@ func suppressRoute53ZoneNameWithTrailingDot(k, old, new string, d *schema.Resour
 	}
 	return strings.TrimSuffix(old, ".") == strings.TrimSuffix(new, ".")
 }
+
+func suppressMetadataOptionsHttpEndpointDisabled(k, old, new string, d *schema.ResourceData) bool {


Instead of introducing new difference suppression, we may want to instead mark attributes that need this with Computed: true instead.

bflad · 2020-02-12T19:48:26Z

aws/resource_aws_instance.go

@@ -518,6 +518,36 @@ func resourceAwsInstance() *schema.Resource {
 					},
 				},
 			},
+
+			"metadata_options": {


In some unrelated testing today, it appears EC2 Instances can include this in their API response by default. Instead of requiring all operators to always include this new configuration, we should mark this attribute with Computed: true.

bflad · 2020-02-12T19:49:46Z

aws/resource_aws_instance.go

+							Default:          ec2.HttpTokensStateOptional,
+							ValidateFunc:     validation.StringInSlice([]string{ec2.HttpTokensStateOptional, ec2.HttpTokensStateRequired}, false),
+							DiffSuppressFunc: suppressMetadataOptionsHttpEndpointDisabled,


See note above about potentially replacing Default and DiffSuppressFunc here with Computed: true instead.

bflad · 2020-02-12T19:49:53Z

aws/resource_aws_instance.go

+							Default:          1,
+							ValidateFunc:     validation.IntBetween(1, 64),
+							DiffSuppressFunc: suppressMetadataOptionsHttpEndpointDisabled,


See note above about potentially replacing Default and DiffSuppressFunc here with Computed: true instead.

bflad · 2020-02-12T19:54:36Z

aws/resource_aws_instance.go

+		mo["http_put_response_hop_limit"] = instance.MetadataOptions.HttpPutResponseHopLimit
+		var metadataOptions []map[string]interface{}
+		metadataOptions = append(metadataOptions, mo)
+		d.Set("metadata_options", metadataOptions)


Same note here regarding error checking. It may be easier to instead make this into a "flatten" function to be shared, e.g.

func flattenEc2InstanceMetadataOptions(opts *ec2.MetadataOptions) []interface{} { if opts == nil { return nil } m := map[string]interface{}{ "http_endpoint": aws.StringValue(opts.HttpEndpoint), "http_put_response_hop_limit": aws.Int64Value(opts.HttpPutResponseHopLimit), "http_tokens": aws.StringValue(opts.HttpTokens), } return []interface{}{m} } // ... if err := d.Set("metadata_options", flattenEc2InstanceMetadataOptions(instance.MetadataOptions)); err != nil { return fmt.Errorf("error setting metadata_options: %s", err) }

bflad · 2020-02-12T20:03:35Z

aws/resource_aws_instance.go

+		if mo, ok := v.([]interface{})[0].(map[string]interface{}); ok {
+			opts.MetadataOptions = &ec2.InstanceMetadataOptionsRequest{
+				HttpEndpoint: aws.String(mo["http_endpoint"].(string)),
+			}
+			if mo["http_endpoint"].(string) == ec2.InstanceMetadataEndpointStateEnabled {
+				// These parameters are not allowed unless HttpEndpoint is enabled
+				opts.MetadataOptions.HttpTokens = aws.String(mo["http_tokens"].(string))
+				opts.MetadataOptions.HttpPutResponseHopLimit = aws.Int64(int64(mo["http_put_response_hop_limit"].(int)))
+			}
+		}


Similar to creating a flatten function above, the inverse expand function could be created and used in the multiple places with this logic:

func expandEc2InstanceMetadataOptions(l []interface{}) *ec2.MetadataOptions { m, ok := l[0].(map[string]interface{}) if !ok { return nil } opts := &ec2.InstanceMetadataOptionsRequest{ HttpEndpoint: aws.String(m["http_endpoint"].(string)), } if m["http_endpoint"].(string) == ec2.InstanceMetadataEndpointStateEnabled { // These parameters are not allowed unless HttpEndpoint is enabled if v, ok := m["http_tokens"].(string); ok && v != "" { opts.HttpTokens = aws.String(v) } if v, ok := m["http_put_response_hop_limit"].(int); ok && v != 0 { opts.HttpPutResponseHopLimit = aws.Int64(int64(v)) } } return opts } // ... MetadataOptions: expandEc2InstanceMetadataOptions(d.Get("metadata_options").([]interface{})),

bflad · 2020-02-12T20:04:14Z

aws/resource_aws_instance_test.go

@@ -2814,6 +2817,12 @@ resource "aws_instance" "test" {
  instance_type   = "m1.small"
  security_groups = ["${aws_security_group.tf_test_test.name}"]
  user_data       = "foo:-with-character's"
+
+  metadata_options {


Please create a new test function and test configuration so we can verify existing testing for regressions.

bflad · 2020-02-12T20:07:18Z

aws/resource_aws_launch_template_test.go

@@ -978,6 +979,12 @@ resource "aws_launch_template" "test" {

  key_name = "test"

+  metadata_options {


Same here about creating a separate test function and configuration.

ewbankkit · 2020-03-17T12:14:32Z

Closes #10949.
Closes #11794.

ewbankkit · 2020-03-17T12:24:50Z

@stefansundin Will you have a chance to address the review comments?
I have interest in using this functionality and can help out/complete if necessary.
Thanks.

ewbankkit · 2020-03-20T16:32:33Z

@stefansundin I have addressed @bflad's review comments via #12491.
Thanks.

ghost · 2020-04-20T17:10:01Z

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!

Add support for metadata_options in r/aws_instance and d/aws_instance.

dc5319a

stefansundin requested a review from a team December 1, 2019 07:46

stefansundin added 2 commits January 30, 2020 12:15

Merge branch 'master' of https://github.com/terraform-providers/terra…

11de06b

…form-provider-aws into metadata_options

Add helper function for suppressing diff. Add validation for http_put…

fc6571d

…_response_hop_limit.

ghost added size/L Managed by automation to categorize the size of a PR. and removed size/M Managed by automation to categorize the size of a PR. labels Jan 31, 2020

stefansundin changed the title ~~rd/aws_instance: Add support for metadata_options~~ rd/aws_instance and rd/aws_launch_template: Add support for metadata_options Jan 31, 2020

stefansundin added 3 commits January 30, 2020 22:32

Fix bug with HttpPutResponseHopLimit.

28711a1

Also test http_put_response_hop_limit.

9b3d93e

Add support for metadata_options in r/aws_launch_template and d/aws_l…

2fcf640

…aunch_template.

bflad suggested changes Feb 12, 2020

View reviewed changes

bflad added enhancement Requests to existing resources that expand the functionality or scope. and removed needs-triage Waiting for first response or review from a maintainer. labels Feb 12, 2020

bflad self-assigned this Feb 12, 2020

bflad added the waiting-response Maintainers are waiting on response from community or contributor. label Feb 12, 2020

ghost removed the waiting-response Maintainers are waiting on response from community or contributor. label Mar 17, 2020

ewbankkit mentioned this pull request Mar 20, 2020

rd/aws_instance and rd/aws_launch_template: Add support for metadata_options #12491

Merged

stefansundin closed this Mar 21, 2020

ewbankkit mentioned this pull request Mar 28, 2020

metadata_options default not working properly in Launch Template #12564

Closed

ghost locked and limited conversation to collaborators Apr 20, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rd/aws_instance and rd/aws_launch_template: Add support for metadata_options #11076

rd/aws_instance and rd/aws_launch_template: Add support for metadata_options #11076

stefansundin commented Dec 1, 2019 •

edited

Loading

stefansundin commented Dec 15, 2019

stefansundin commented Jan 31, 2020

bflad left a comment

bflad Feb 12, 2020

bflad Feb 12, 2020

bflad Feb 12, 2020

bflad Feb 12, 2020

bflad Feb 12, 2020

bflad Feb 12, 2020

bflad Feb 12, 2020

bflad Feb 12, 2020

bflad Feb 12, 2020

bflad Feb 12, 2020

ewbankkit commented Mar 17, 2020

ewbankkit commented Mar 17, 2020

ewbankkit commented Mar 20, 2020 •

edited

Loading

ghost commented Apr 20, 2020

-		d.Set("metadata_options", metadataOptions)
+		if err := d.Set("metadata_options", metadataOptions); err != nil {
+			return fmt.Errorf("error setting metadata_options: %s", err)
+		}

		@@ -978,6 +979,12 @@ resource "aws_launch_template" "test" {

		key_name = "test"

		metadata_options {

rd/aws_instance and rd/aws_launch_template: Add support for metadata_options #11076

rd/aws_instance and rd/aws_launch_template: Add support for metadata_options #11076

Conversation

stefansundin commented Dec 1, 2019 • edited Loading

Community Note

stefansundin commented Dec 15, 2019

stefansundin commented Jan 31, 2020

bflad left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ewbankkit commented Mar 17, 2020

ewbankkit commented Mar 17, 2020

ewbankkit commented Mar 20, 2020 • edited Loading

ghost commented Apr 20, 2020

stefansundin commented Dec 1, 2019 •

edited

Loading

ewbankkit commented Mar 20, 2020 •

edited

Loading