-
Notifications
You must be signed in to change notification settings - Fork 9.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
rd/aws_instance and rd/aws_launch_template: Add support for metadata_options #11076
Changes from all commits
dc5319a
11de06b
fc6571d
28711a1
9b3d93e
2fcf640
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,6 +23,9 @@ func TestAccAWSInstanceDataSource_basic(t *testing.T) { | |
resource.TestCheckResourceAttrPair(datasourceName, "ami", resourceName, "ami"), | ||
resource.TestCheckResourceAttrPair(datasourceName, "tags.%", resourceName, "tags.%"), | ||
resource.TestCheckResourceAttrPair(datasourceName, "instance_type", resourceName, "instance_type"), | ||
resource.TestCheckResourceAttrPair(datasourceName, "metadata_options.0.http_endpoint", resourceName, "metadata_options.0.http_endpoint"), | ||
resource.TestCheckResourceAttrPair(datasourceName, "metadata_options.0.http_tokens", resourceName, "metadata_options.0.http_tokens"), | ||
resource.TestCheckResourceAttrPair(datasourceName, "metadata_options.0.http_put_response_hop_limit", resourceName, "metadata_options.0.http_put_response_hop_limit"), | ||
resource.TestMatchResourceAttr(datasourceName, "arn", regexp.MustCompile(`^arn:[^:]+:ec2:[^:]+:\d{12}:instance/i-.+`)), | ||
resource.TestCheckNoResourceAttr(datasourceName, "user_data_base64"), | ||
), | ||
|
@@ -450,6 +453,11 @@ resource "aws_instance" "test" { | |
# us-west-2 | ||
ami = "ami-4fccb37f" | ||
instance_type = "m1.small" | ||
metadata_options { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. 👍 |
||
http_endpoint = "enabled" | ||
http_tokens = "optional" | ||
http_put_response_hop_limit = 10 | ||
} | ||
tags = { | ||
Name = "HelloWorld" | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of introducing new difference suppression, we may want to instead mark attributes that need this with |
||
// Suppress diff if http_endpoint is disabled | ||
i := strings.LastIndexByte(k, '.') | ||
state := d.Get(k[:i+1] + "http_endpoint").(string) | ||
return state == "disabled" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -518,6 +518,36 @@ func resourceAwsInstance() *schema.Resource { | |
}, | ||
}, | ||
}, | ||
|
||
"metadata_options": { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||
Type: schema.TypeList, | ||
Optional: true, | ||
MaxItems: 1, | ||
Elem: &schema.Resource{ | ||
Schema: map[string]*schema.Schema{ | ||
"http_endpoint": { | ||
Type: schema.TypeString, | ||
Optional: true, | ||
Default: ec2.InstanceMetadataEndpointStateEnabled, | ||
ValidateFunc: validation.StringInSlice([]string{ec2.InstanceMetadataEndpointStateEnabled, ec2.InstanceMetadataEndpointStateDisabled}, false), | ||
}, | ||
"http_tokens": { | ||
Type: schema.TypeString, | ||
Optional: true, | ||
Default: ec2.HttpTokensStateOptional, | ||
ValidateFunc: validation.StringInSlice([]string{ec2.HttpTokensStateOptional, ec2.HttpTokensStateRequired}, false), | ||
DiffSuppressFunc: suppressMetadataOptionsHttpEndpointDisabled, | ||
Comment on lines
+537
to
+539
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See note above about potentially replacing |
||
}, | ||
"http_put_response_hop_limit": { | ||
Type: schema.TypeInt, | ||
Optional: true, | ||
Default: 1, | ||
ValidateFunc: validation.IntBetween(1, 64), | ||
DiffSuppressFunc: suppressMetadataOptionsHttpEndpointDisabled, | ||
Comment on lines
+544
to
+546
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See note above about potentially replacing |
||
}, | ||
}, | ||
}, | ||
}, | ||
}, | ||
} | ||
} | ||
|
@@ -566,6 +596,7 @@ func resourceAwsInstanceCreate(d *schema.ResourceData, meta interface{}) error { | |
CreditSpecification: instanceOpts.CreditSpecification, | ||
CpuOptions: instanceOpts.CpuOptions, | ||
TagSpecifications: tagSpecifications, | ||
MetadataOptions: instanceOpts.MetadataOptions, | ||
} | ||
|
||
_, ipv6CountOk := d.GetOk("ipv6_address_count") | ||
|
@@ -712,6 +743,16 @@ func resourceAwsInstanceRead(d *schema.ResourceData, meta interface{}) error { | |
d.Set("cpu_threads_per_core", instance.CpuOptions.ThreadsPerCore) | ||
} | ||
|
||
if instance.MetadataOptions != nil { | ||
mo := make(map[string]interface{}) | ||
mo["http_endpoint"] = instance.MetadataOptions.HttpEndpoint | ||
mo["http_tokens"] = instance.MetadataOptions.HttpTokens | ||
mo["http_put_response_hop_limit"] = instance.MetadataOptions.HttpPutResponseHopLimit | ||
var metadataOptions []map[string]interface{} | ||
metadataOptions = append(metadataOptions, mo) | ||
d.Set("metadata_options", metadataOptions) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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)
} |
||
} | ||
|
||
d.Set("ami", instance.ImageId) | ||
d.Set("instance_type", instance.InstanceType) | ||
d.Set("key_name", instance.KeyName) | ||
|
@@ -1200,6 +1241,27 @@ func resourceAwsInstanceUpdate(d *schema.ResourceData, meta interface{}) error { | |
} | ||
} | ||
|
||
if d.HasChange("metadata_options") && !d.IsNewResource() { | ||
if v, ok := d.GetOk("metadata_options"); ok { | ||
if mo, ok := v.([]interface{})[0].(map[string]interface{}); ok { | ||
log.Printf("[DEBUG] Modifying metadata options for Instance (%s)", d.Id()) | ||
input := &ec2.ModifyInstanceMetadataOptionsInput{ | ||
InstanceId: aws.String(d.Id()), | ||
HttpEndpoint: aws.String(mo["http_endpoint"].(string)), | ||
} | ||
if mo["http_endpoint"].(string) == ec2.InstanceMetadataEndpointStateEnabled { | ||
// These parameters are not allowed unless HttpEndpoint is enabled | ||
input.HttpTokens = aws.String(mo["http_tokens"].(string)) | ||
input.HttpPutResponseHopLimit = aws.Int64(int64(mo["http_put_response_hop_limit"].(int))) | ||
} | ||
_, err := conn.ModifyInstanceMetadataOptions(input) | ||
if err != nil { | ||
return fmt.Errorf("Error updating metadata options: %s", err) | ||
} | ||
} | ||
} | ||
} | ||
|
||
// TODO(mitchellh): wait for the attributes we modified to | ||
// persist the change... | ||
|
||
|
@@ -1798,6 +1860,7 @@ type awsInstanceOpts struct { | |
UserData64 *string | ||
CreditSpecification *ec2.CreditSpecificationRequest | ||
CpuOptions *ec2.CpuOptionsRequest | ||
MetadataOptions *ec2.InstanceMetadataOptionsRequest | ||
} | ||
|
||
func buildAwsInstanceOpts( | ||
|
@@ -1838,6 +1901,19 @@ func buildAwsInstanceOpts( | |
opts.InstanceInitiatedShutdownBehavior = aws.String(v) | ||
} | ||
|
||
if v, ok := d.GetOk("metadata_options"); ok { | ||
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))) | ||
} | ||
} | ||
Comment on lines
+1905
to
+1914
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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{})), |
||
} | ||
|
||
opts.Monitoring = &ec2.RunInstancesMonitoringEnabled{ | ||
Enabled: aws.Bool(d.Get("monitoring").(bool)), | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -290,6 +290,9 @@ func TestAccAWSInstance_basic(t *testing.T) { | |
resourceName, | ||
"arn", | ||
regexp.MustCompile(`^arn:[^:]+:ec2:[^:]+:\d{12}:instance/i-.+`)), | ||
resource.TestCheckResourceAttr(resourceName, "metadata_options.0.http_endpoint", "enabled"), | ||
resource.TestCheckResourceAttr(resourceName, "metadata_options.0.http_tokens", "optional"), | ||
resource.TestCheckResourceAttr(resourceName, "metadata_options.0.http_put_response_hop_limit", "10"), | ||
), | ||
}, | ||
{ | ||
|
@@ -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 { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please create a new test function and test configuration so we can verify existing testing for regressions. |
||
http_endpoint = "enabled" | ||
http_tokens = "optional" | ||
http_put_response_hop_limit = 10 | ||
} | ||
} | ||
`, rInt) | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -249,6 +249,7 @@ func TestAccAWSLaunchTemplate_data(t *testing.T) { | |
resource.TestCheckResourceAttrSet(resourceName, "instance_type"), | ||
resource.TestCheckResourceAttrSet(resourceName, "kernel_id"), | ||
resource.TestCheckResourceAttrSet(resourceName, "key_name"), | ||
resource.TestCheckResourceAttr(resourceName, "metadata_options.#", "1"), | ||
resource.TestCheckResourceAttr(resourceName, "monitoring.#", "1"), | ||
resource.TestCheckResourceAttr(resourceName, "network_interfaces.#", "1"), | ||
resource.TestCheckResourceAttr(resourceName, "network_interfaces.0.security_groups.#", "1"), | ||
|
@@ -978,6 +979,12 @@ resource "aws_launch_template" "test" { | |
|
||
key_name = "test" | ||
|
||
metadata_options { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here about creating a separate test function and configuration. |
||
http_endpoint = "enabled" | ||
http_tokens = "optional" | ||
http_put_response_hop_limit = 10 | ||
} | ||
|
||
monitoring { | ||
enabled = true | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.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.