Skip to content
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

Support Nitro Enclaves in aws_instance and aws_launch_template #16361

Merged
merged 8 commits into from
Dec 15, 2020
Merged

Support Nitro Enclaves in aws_instance and aws_launch_template #16361

merged 8 commits into from
Dec 15, 2020

Conversation

hansnielsen
Copy link
Contributor

@hansnielsen hansnielsen commented Nov 21, 2020

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" or other comments that do not add relevant new information or questions, they generate extra noise for pull request followers and do not help prioritize the request

Closes #15909.

This PR is largely based upon similar EC2 features like hibernation and metadata_options. We're already using a modern-enough version of the AWS SDK to get the new enclave-related structs. It's worth noting that Launch Configurations are not supported by Nitro Enclaves, only Launch Templates.

In addition to the acceptance tests, I've done local manual testing to ensure that resource replacement functions as expected. Note that the instance type (c5{a,}.xlarge) is relatively expensive, but it's currently the cheapest instance that Nitro Enclaves work on.

Release note for CHANGELOG:

* data/aws_instance: Add support for Nitro Enclaves via `enclave_options` block ([#15909](https://github.com/hashicorp/terraform-provider-aws/issues/15909))
* data/aws_launch_template: Add support for Nitro Enclaves via `enclave_options` block ([#15909](https://github.com/hashicorp/terraform-provider-aws/issues/15909))
* resource/aws_instance: Add support for Nitro Enclaves via `enclave_options` block ([#15909](https://github.com/hashicorp/terraform-provider-aws/issues/15909))
* resource/aws_launch_template: Add support for Nitro Enclaves via `enclave_options` block ([#15909](https://github.com/hashicorp/terraform-provider-aws/issues/15909))

Output from acceptance testing against Terraform v0.12.29:

$ make testacc TESTARGS='-run=_enclaveOptions'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=_enclaveOptions -timeout 120m
=== RUN   TestAccAWSInstanceDataSource_enclaveOptions
=== PAUSE TestAccAWSInstanceDataSource_enclaveOptions
=== RUN   TestAccAWSLaunchTemplateDataSource_enclaveOptions
=== PAUSE TestAccAWSLaunchTemplateDataSource_enclaveOptions
=== RUN   TestAccAWSInstance_enclaveOptions
=== PAUSE TestAccAWSInstance_enclaveOptions
=== RUN   TestAccAWSLaunchTemplate_enclaveOptions
=== PAUSE TestAccAWSLaunchTemplate_enclaveOptions
=== CONT  TestAccAWSInstanceDataSource_enclaveOptions
=== CONT  TestAccAWSLaunchTemplate_enclaveOptions
=== CONT  TestAccAWSInstance_enclaveOptions
=== CONT  TestAccAWSLaunchTemplateDataSource_enclaveOptions
--- PASS: TestAccAWSLaunchTemplateDataSource_enclaveOptions (64.67s)
--- PASS: TestAccAWSInstanceDataSource_enclaveOptions (126.09s)
--- PASS: TestAccAWSLaunchTemplate_enclaveOptions (156.54s)
--- PASS: TestAccAWSInstance_enclaveOptions (259.17s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	259.389s

@hansnielsen hansnielsen requested a review from a team as a code owner November 21, 2020 07:34
@ghost ghost added size/XL Managed by automation to categorize the size of a PR. service/ec2 Issues and PRs that pertain to the ec2 service. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. labels Nov 21, 2020
@github-actions github-actions bot added the needs-triage Waiting for first response or review from a maintainer. label Nov 21, 2020
@bflad 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 Nov 23, 2020
@bflad bflad self-assigned this Dec 11, 2020
Copy link
Contributor

@bflad bflad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @hansnielsen 👋 Thank you for submitting this. Overall looks really good, just some small items and then we can get this in. 👍

@@ -316,6 +316,21 @@ func dataSourceAwsInstance() *schema.Resource {
Type: schema.TypeBool,
Computed: true,
},
"enclave_options": {
Type: schema.TypeList,
Optional: true,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this schema attribute cannot be used to filter the lookup as an argument, Optional should be removed. 👍

Suggested change
Optional: true,

Schema: map[string]*schema.Schema{
"enabled": {
Type: schema.TypeBool,
Optional: true,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly:

Suggested change
Optional: true,

Comment on lines 5160 to 5163

data "aws_instance" "test" {
instance_id = aws_instance.test.id
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resource testing should omit data source 👍

Suggested change
data "aws_instance" "test" {
instance_id = aws_instance.test.id
}


For more information, see the documentation on [Nitro Enclaves](https://docs.aws.amazon.com/enclaves/latest/user/nitro-enclave.html).


Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Extra whitespace

Suggested change

@@ -150,6 +150,8 @@ The following arguments are supported:
* `tags` - (Optional) A map of tags to assign to the launch template.
* `user_data` - The Base64-encoded user data to provide when launching the instance.
* `hibernation_options` - The hibernation options for the instance. See [Hibernation Options](#hibernation-options) below for more details.
* `enclave_options` - (Optional) Enable Nitro Enclaves on launched instances. See [Enclave Options](#enclave-options) below for more details.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Extra whitespace

Suggested change


"enclave_options": {
Type: schema.TypeList,
Optional: true,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be safe and prevent unexpected differences for operators should the EC2 API return this information with existing configurations not having it, let's add Computed here:

Suggested change
Optional: true,
Optional: true,
Computed: true,

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also switched the enabled field to Computed: true. This means that aws_instance resources that originally had enclaves enabled won't force a new resource if the enclave_options block is removed, which seems like a reasonable behavior.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The top level Computed: true on configuration blocks prevents differences from showing up in nested attributes if the configuration is not provided even if it does not match the default. Since there is one nested attribute though, it is probably okay since most operators would likely configure both the block and the attribute.

@bflad bflad added the waiting-response Maintainers are waiting on response from community or contributor. label Dec 11, 2020
@ghost ghost added the documentation Introduces or discusses updates to documentation. label Dec 13, 2020
@hansnielsen
Copy link
Contributor Author

Thanks for the review! I made the suggested tweaks, re-ran the acceptance tests successfully, and manually tested instance creation / replacement with various enclave_options.

@ghost ghost removed the waiting-response Maintainers are waiting on response from community or contributor. label Dec 13, 2020
Copy link
Contributor

@bflad bflad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thank you, @hansnielsen 🚀

Output from acceptance testing in AWS Commercial (failure known and unrelated):

--- FAIL: TestAccAWSInstance_instanceProfileChange (131.45s)
--- PASS: TestAccAWSInstance_addSecondaryInterface (168.25s)
--- PASS: TestAccAWSInstance_addSecurityGroupNetworkInterface (145.34s)
--- PASS: TestAccAWSInstance_associatePublic_defaultPrivate (93.37s)
--- PASS: TestAccAWSInstance_associatePublic_defaultPublic (193.44s)
--- PASS: TestAccAWSInstance_associatePublic_explicitPrivate (90.86s)
--- PASS: TestAccAWSInstance_associatePublic_explicitPublic (89.46s)
--- PASS: TestAccAWSInstance_associatePublic_overridePrivate (91.19s)
--- PASS: TestAccAWSInstance_associatePublic_overridePublic (81.27s)
--- PASS: TestAccAWSInstance_associatePublicIPAndPrivateIP (80.40s)
--- PASS: TestAccAWSInstance_atLeastOneOtherEbsVolume (192.83s)
--- PASS: TestAccAWSInstance_basic (89.76s)
--- PASS: TestAccAWSInstance_blockDevices (78.09s)
--- PASS: TestAccAWSInstance_changeInstanceType (149.50s)
--- PASS: TestAccAWSInstance_CreditSpecification_Empty_NonBurstable (322.48s)
--- PASS: TestAccAWSInstance_creditSpecification_isNotAppliedToNonBurstable (95.59s)
--- PASS: TestAccAWSInstance_creditSpecification_standardCpuCredits (119.17s)
--- PASS: TestAccAWSInstance_creditSpecification_standardCpuCredits_t2Tot3Taint (404.66s)
--- PASS: TestAccAWSInstance_creditSpecification_unknownCpuCredits_t2 (91.53s)
--- PASS: TestAccAWSInstance_creditSpecification_unknownCpuCredits_t3 (313.08s)
--- PASS: TestAccAWSInstance_creditSpecification_unlimitedCpuCredits (118.40s)
--- PASS: TestAccAWSInstance_creditSpecification_unlimitedCpuCredits_t2Tot3Taint (395.94s)
--- PASS: TestAccAWSInstance_creditSpecification_unspecifiedDefaultsToStandard (78.13s)
--- PASS: TestAccAWSInstance_CreditSpecification_UnspecifiedToEmpty_NonBurstable (108.23s)
--- PASS: TestAccAWSInstance_creditSpecification_updateCpuCredits (134.94s)
--- PASS: TestAccAWSInstance_creditSpecificationT3_standardCpuCredits (131.22s)
--- PASS: TestAccAWSInstance_creditSpecificationT3_unlimitedCpuCredits (117.74s)
--- PASS: TestAccAWSInstance_creditSpecificationT3_unspecifiedDefaultsToUnlimited (309.62s)
--- PASS: TestAccAWSInstance_creditSpecificationT3_updateCpuCredits (146.03s)
--- PASS: TestAccAWSInstance_dedicatedInstance (106.61s)
--- PASS: TestAccAWSInstance_disableApiTermination (118.10s)
--- PASS: TestAccAWSInstance_disappears (92.80s)
--- PASS: TestAccAWSInstance_EbsBlockDevice_InvalidIopsForVolumeType (17.27s)
--- PASS: TestAccAWSInstance_EbsBlockDevice_KmsKeyArn (142.27s)
--- PASS: TestAccAWSInstance_EbsRootDevice_basic (132.83s)
--- PASS: TestAccAWSInstance_EbsRootDevice_ModifyAll (164.65s)
--- PASS: TestAccAWSInstance_EbsRootDevice_ModifyDeleteOnTermination (97.23s)
--- PASS: TestAccAWSInstance_EbsRootDevice_ModifyIOPS_Io1 (121.82s)
--- PASS: TestAccAWSInstance_EbsRootDevice_ModifyIOPS_Io2 (147.63s)
--- PASS: TestAccAWSInstance_EbsRootDevice_ModifySize (236.63s)
--- PASS: TestAccAWSInstance_EbsRootDevice_ModifyType (123.66s)
--- PASS: TestAccAWSInstance_EbsRootDevice_MultipleBlockDevices_ModifyDeleteOnTermination (199.08s)
--- PASS: TestAccAWSInstance_EbsRootDevice_MultipleBlockDevices_ModifySize (123.52s)
--- PASS: TestAccAWSInstance_EbsRootDevice_MultipleDynamicEBSBlockDevices (207.68s)
--- PASS: TestAccAWSInstance_Empty_PrivateIP (78.02s)
--- PASS: TestAccAWSInstance_enclaveOptions (430.58s)
--- PASS: TestAccAWSInstance_forceNewAndTagsDrift (270.39s)
--- PASS: TestAccAWSInstance_getPasswordData_falseToTrue (208.61s)
--- PASS: TestAccAWSInstance_getPasswordData_trueToFalse (269.53s)
--- PASS: TestAccAWSInstance_GP2IopsDevice (80.89s)
--- PASS: TestAccAWSInstance_GP2WithIopsValue (11.19s)
--- PASS: TestAccAWSInstance_hibernation (204.56s)
--- PASS: TestAccAWSInstance_inDefaultVpcBySgId (100.86s)
--- PASS: TestAccAWSInstance_inDefaultVpcBySgName (99.23s)
--- PASS: TestAccAWSInstance_ipv6_supportAddressCount (99.03s)
--- PASS: TestAccAWSInstance_ipv6_supportAddressCountWithIpv4 (189.48s)
--- PASS: TestAccAWSInstance_ipv6AddressCountAndSingleAddressCausesError (16.71s)
--- PASS: TestAccAWSInstance_keyPairCheck (86.95s)
--- PASS: TestAccAWSInstance_metadataOptions (154.98s)
--- PASS: TestAccAWSInstance_NetworkInstanceRemovingAllSecurityGroups (112.12s)
--- PASS: TestAccAWSInstance_NetworkInstanceSecurityGroups (103.81s)
--- PASS: TestAccAWSInstance_NetworkInstanceVPCSecurityGroupIDs (136.45s)
--- PASS: TestAccAWSInstance_NewNetworkInterface_EmptyPrivateIPAndSecondaryPrivateIPs (341.20s)
--- PASS: TestAccAWSInstance_NewNetworkInterface_EmptyPrivateIPAndSecondaryPrivateIPsUpdate (161.33s)
--- PASS: TestAccAWSInstance_NewNetworkInterface_PrivateIPAndSecondaryPrivateIPs (134.73s)
--- PASS: TestAccAWSInstance_NewNetworkInterface_PrivateIPAndSecondaryPrivateIPsUpdate (121.24s)
--- PASS: TestAccAWSInstance_NewNetworkInterface_PublicIPAndSecondaryPrivateIPs (410.27s)
--- PASS: TestAccAWSInstance_noAMIEphemeralDevices (59.54s)
--- PASS: TestAccAWSInstance_placementGroup (304.38s)
--- PASS: TestAccAWSInstance_primaryNetworkInterface (112.14s)
--- PASS: TestAccAWSInstance_primaryNetworkInterfaceSourceDestCheck (112.05s)
--- PASS: TestAccAWSInstance_privateIP (75.33s)
--- PASS: TestAccAWSInstance_RootBlockDevice_KmsKeyArn (99.55s)
--- PASS: TestAccAWSInstance_rootBlockDeviceMismatch (121.64s)
--- PASS: TestAccAWSInstance_rootInstanceStore (149.85s)
--- PASS: TestAccAWSInstance_sourceDestCheck (159.49s)
--- PASS: TestAccAWSInstance_tags (104.40s)
--- PASS: TestAccAWSInstance_UserData_EmptyStringToUnspecified (113.94s)
--- PASS: TestAccAWSInstance_UserData_UnspecifiedToEmptyString (105.43s)
--- PASS: TestAccAWSInstance_userDataBase64 (105.42s)
--- PASS: TestAccAWSInstance_volumeTags (166.33s)
--- PASS: TestAccAWSInstance_volumeTagsComputed (118.47s)
--- PASS: TestAccAWSInstance_withIamInstanceProfile (99.55s)
--- SKIP: TestAccAWSInstance_inEc2Classic (2.60s)
--- SKIP: TestAccAWSInstance_outpost (1.99s)

--- PASS: TestAccAWSInstanceDataSource_AzUserData (109.31s)
--- PASS: TestAccAWSInstanceDataSource_basic (125.30s)
--- PASS: TestAccAWSInstanceDataSource_blockDevices (97.25s)
--- PASS: TestAccAWSInstanceDataSource_creditSpecification (84.91s)
--- PASS: TestAccAWSInstanceDataSource_EbsBlockDevice_KmsKeyId (110.17s)
--- PASS: TestAccAWSInstanceDataSource_enclaveOptions (68.12s)
--- PASS: TestAccAWSInstanceDataSource_getPasswordData_falseToTrue (247.32s)
--- PASS: TestAccAWSInstanceDataSource_getPasswordData_trueToFalse (255.13s)
--- PASS: TestAccAWSInstanceDataSource_GetUserData (152.33s)
--- PASS: TestAccAWSInstanceDataSource_GetUserData_NoUserData (182.10s)
--- PASS: TestAccAWSInstanceDataSource_gp2IopsDevice (108.60s)
--- PASS: TestAccAWSInstanceDataSource_keyPair (122.63s)
--- PASS: TestAccAWSInstanceDataSource_metadataOptions (305.70s)
--- PASS: TestAccAWSInstanceDataSource_PlacementGroup (335.92s)
--- PASS: TestAccAWSInstanceDataSource_privateIP (106.49s)
--- PASS: TestAccAWSInstanceDataSource_RootBlockDevice_KmsKeyId (141.35s)
--- PASS: TestAccAWSInstanceDataSource_rootInstanceStore (107.10s)
--- PASS: TestAccAWSInstanceDataSource_secondaryPrivateIPs (101.78s)
--- PASS: TestAccAWSInstanceDataSource_SecurityGroups (114.84s)
--- PASS: TestAccAWSInstanceDataSource_tags (113.19s)
--- PASS: TestAccAWSInstanceDataSource_VPC (118.35s)
--- PASS: TestAccAWSInstanceDataSource_VPCSecurityGroups (128.09s)

--- PASS: TestAccAWSInstancesDataSource_basic (345.58s)
--- PASS: TestAccAWSInstancesDataSource_instanceStateNames (91.95s)
--- PASS: TestAccAWSInstancesDataSource_tags (334.64s)

--- PASS: TestAccAWSLaunchTemplate_associateCarrierIPAddress (95.60s)
--- PASS: TestAccAWSLaunchTemplate_associatePublicIPAddress (96.67s)
--- PASS: TestAccAWSLaunchTemplate_basic (15.00s)
--- PASS: TestAccAWSLaunchTemplate_BlockDeviceMappings_EBS (66.34s)
--- PASS: TestAccAWSLaunchTemplate_BlockDeviceMappings_EBS_DeleteOnTermination (86.09s)
--- PASS: TestAccAWSLaunchTemplate_BlockDeviceMappings_EBS_Gp3 (65.39s)
--- PASS: TestAccAWSLaunchTemplate_capacityReservation_preference (30.21s)
--- PASS: TestAccAWSLaunchTemplate_capacityReservation_target (31.33s)
--- PASS: TestAccAWSLaunchTemplate_cpuOptions (30.11s)
--- PASS: TestAccAWSLaunchTemplate_creditSpecification_nonBurstable (34.89s)
--- PASS: TestAccAWSLaunchTemplate_creditSpecification_t2 (35.68s)
--- PASS: TestAccAWSLaunchTemplate_creditSpecification_t3 (32.18s)
--- PASS: TestAccAWSLaunchTemplate_data (21.55s)
--- PASS: TestAccAWSLaunchTemplate_defaultVersion (60.20s)
--- PASS: TestAccAWSLaunchTemplate_description (53.26s)
--- PASS: TestAccAWSLaunchTemplate_disappears (19.56s)
--- PASS: TestAccAWSLaunchTemplate_EbsOptimized (106.63s)
--- PASS: TestAccAWSLaunchTemplate_ElasticInferenceAccelerator (40.95s)
--- PASS: TestAccAWSLaunchTemplate_enclaveOptions (64.33s)
--- PASS: TestAccAWSLaunchTemplate_hibernation (63.36s)
--- PASS: TestAccAWSLaunchTemplate_IamInstanceProfile_EmptyConfigurationBlock (28.60s)
--- PASS: TestAccAWSLaunchTemplate_instanceMarketOptions (84.40s)
--- PASS: TestAccAWSLaunchTemplate_licenseSpecification (30.35s)
--- PASS: TestAccAWSLaunchTemplate_metadataOptions (30.84s)
--- PASS: TestAccAWSLaunchTemplate_networkInterface (69.40s)
--- PASS: TestAccAWSLaunchTemplate_networkInterface_ipv6AddressCount (25.96s)
--- PASS: TestAccAWSLaunchTemplate_networkInterface_ipv6Addresses (27.09s)
--- PASS: TestAccAWSLaunchTemplate_networkInterfaceAddresses (68.27s)
--- PASS: TestAccAWSLaunchTemplate_NetworkInterfaces_DeleteOnTermination (85.02s)
--- PASS: TestAccAWSLaunchTemplate_placement_partitionNum (51.60s)
--- PASS: TestAccAWSLaunchTemplate_tags (52.39s)
--- PASS: TestAccAWSLaunchTemplate_update (78.88s)
--- PASS: TestAccAWSLaunchTemplate_updateDefaultVersion (69.42s)

--- PASS: TestAccAWSLaunchTemplateDataSource_associateCarrierIPAddress (63.16s)
--- PASS: TestAccAWSLaunchTemplateDataSource_associatePublicIPAddress (63.57s)
--- PASS: TestAccAWSLaunchTemplateDataSource_basic (27.41s)
--- PASS: TestAccAWSLaunchTemplateDataSource_enclaveOptions (33.37s)
--- PASS: TestAccAWSLaunchTemplateDataSource_filter_basic (28.83s)
--- PASS: TestAccAWSLaunchTemplateDataSource_filter_tags (33.26s)
--- PASS: TestAccAWSLaunchTemplateDataSource_id_basic (26.74s)
--- PASS: TestAccAWSLaunchTemplateDataSource_metadataOptions (32.68s)
--- PASS: TestAccAWSLaunchTemplateDataSource_networkInterfaces_deleteOnTermination (62.31s)
--- PASS: TestAccAWSLaunchTemplateDataSource_NonExistent (8.99s)

@bflad bflad added this to the v3.22.0 milestone Dec 15, 2020
@bflad bflad merged commit ca78d53 into hashicorp:master Dec 15, 2020
bflad added a commit that referenced this pull request Dec 15, 2020
@ghost
Copy link

ghost commented Dec 18, 2020

This has been released in version 3.22.0 of the Terraform AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template for triage. Thanks!

@ghost
Copy link

ghost commented Jan 14, 2021

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!

@ghost ghost locked as resolved and limited conversation to collaborators Jan 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
documentation Introduces or discusses updates to documentation. enhancement Requests to existing resources that expand the functionality or scope. service/ec2 Issues and PRs that pertain to the ec2 service. size/XL Managed by automation to categorize the size of a PR. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

r/aws_instance,r/aws_launch_template: Support for AWS Nitro Enclaves
2 participants