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

Adding KMS support for ebs_block_devices for aws_instance #4861

Merged
merged 3 commits into from
Aug 2, 2019

Conversation

frezbo
Copy link
Contributor

@frezbo frezbo commented Jun 17, 2018

Signed-off-by: Noel Georgi 18496730+frezbo@users.noreply.github.com

refers: #657

I would like to know how to add test case's for this, should I create a KMS key before and use that. This needs to be added back to launch configs, opsworks instances etc.

I would also love to run acceptance testing, but the hard coded regions and values makes it difficult. (I use gov-cloud regions). Will it be possible in the future to read a config file that has these details and if not present fall back to the hardcoded values. A sample config file we use for testing with chef projects:

export VPC_ID="<redacted>"
export SUBNET_ID="<redacted>"
export SG_ID="<redacted>"
export SSH_KEY="<redacted>"
export IAM_PROFILE="<redacted>"
export TAGS_OWNER="<redacted>"
export TAGS_ENVIRONMENT="<redacted>"
export TAGS_PROJECT="redacted"
export TAGS_EXPIRY=$(date -d +10days +%Y-%m-%d)
export SSH_KEY_PATH="<redacted>"
export PUBLIC_IP=true

@hashibot

Signed-off-by: Noel Georgi <18496730+frezbo@users.noreply.github.com>
@ghost ghost added the size/S Managed by automation to categorize the size of a PR. label Jun 17, 2018
@frezbo frezbo changed the title Adding KMS support for ebs_block_devices WIP: Adding KMS support for ebs_block_devices Jun 17, 2018
@ghost ghost added size/M Managed by automation to categorize the size of a PR. and removed size/S Managed by automation to categorize the size of a PR. labels Jun 18, 2018
Signed-off-by: Noel Georgi <18496730+frezbo@users.noreply.github.com>
@frezbo
Copy link
Contributor Author

frezbo commented Jun 18, 2018

Initial acceptance tests:

terraform-provider-aws (aws:reantrainee)(kc)(git:feature/add-kms)*$ make testacc TESTARGS='-run=TestAccAWSInstance_volumeTags'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./... -v -run=TestAccAWSInstance_volumeTags -timeout 120m
?   	github.com/terraform-providers/terraform-provider-aws	[no test files]
=== RUN   TestAccAWSInstance_volumeTags
--- PASS: TestAccAWSInstance_volumeTags (501.08s)
=== RUN   TestAccAWSInstance_volumeTagsComputed
--- PASS: TestAccAWSInstance_volumeTagsComputed (325.91s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	827.012s
terraform-provider-aws (aws:reantrainee)(kc)(git:feature/add-kms)*$ make testacc TESTARGS='-run=TestAccAWSInstance_blockDevices'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./... -v -run=TestAccAWSInstance_blockDevices -timeout 120m
?   	github.com/terraform-providers/terraform-provider-aws	[no test files]
=== RUN   TestAccAWSInstance_blockDevices
--- PASS: TestAccAWSInstance_blockDevices (274.59s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	274.617s
terraform-provider-aws (aws:reantrainee)(kc)(git:feature/add-kms)*$ 

Will update docs and other resource that needs KMS. BTW I want to know if the tests are enough

@ghost ghost added the size/M Managed by automation to categorize the size of a PR. label Jun 18, 2018
@bflad bflad added enhancement Requests to existing resources that expand the functionality or scope. service/ec2 Issues and PRs that pertain to the ec2 service. labels Jun 18, 2018
Signed-off-by: Noel Georgi <18496730+frezbo@users.noreply.github.com>
@ghost ghost added the size/M Managed by automation to categorize the size of a PR. label Jun 18, 2018
@frezbo
Copy link
Contributor Author

frezbo commented Jun 18, 2018

Acceptance test of dataSource:

terraform-provider-aws (aws:reantrainee)(kc)(git:feature/add-kms)*$ make testacc TESTARGS='-run=TestAccAWSInstanceDataSource_blockDevices'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./... -v -run=TestAccAWSInstanceDataSource_blockDevices -timeout 120m
?   	github.com/terraform-providers/terraform-provider-aws	[no test files]
=== RUN   TestAccAWSInstanceDataSource_blockDevices
--- PASS: TestAccAWSInstanceDataSource_blockDevices (335.61s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	335.635s
terraform-provider-aws (aws:reantrainee)(kc)(git:feature/add-kms)*$ 

Moving this for review. If this's good can raise extra PR's for launch_config and opsworks instances

@frezbo frezbo changed the title WIP: Adding KMS support for ebs_block_devices Adding KMS support for ebs_block_devices for aws_instance Jun 18, 2018
@frezbo
Copy link
Contributor Author

frezbo commented Jul 2, 2018

@bflad can this be merged, anything blocking the PR merge?

@gordonbondon
Copy link
Contributor

It would be awesome to have this one or #4543 merged.

Currently we have to use pre-created snapshots with correct KMS key to create instances with volumes.

@frezbo
Copy link
Contributor Author

frezbo commented Aug 9, 2018

@jbardin @apparentlymart can this be looked at? It's been some time

@masterofmonkeys
Copy link

Can we have it merge any time soon ? or is it anything blocking this merge ?

@aeschright aeschright requested a review from a team June 25, 2019 21:31
@e-moshaya
Copy link

can you also enable kms for the root_block_device mappings too?

@bflad bflad self-assigned this Aug 2, 2019
bflad added a commit that referenced this pull request Aug 2, 2019
…nd root_block_device configuration block encryption and kms_key_arn arguments

Reference: #4861
Reference: #7757

Having combined and resolved conflicts between #4861 and #7757, performed the following changes:

* Ensured that existing acceptance testing was untouched and only new tests were added
* Ensured data source added new attributes to match resource to prevent panics
* Renamed kms_key_id to kms_key_arn for clarity
* Mark new arguments as Computed: true to only show operators differences when a value is configured

Output from acceptance testing:

```
--- PASS: TestAccAWSInstance_EbsBlockDevice_KmsKeyArn (122.98s)
--- PASS: TestAccAWSInstance_RootBlockDevice_KmsKeyArn (127.93s)
--- PASS: TestAccAWSInstanceDataSource_RootBlockDevice_KmsKeyArn (132.34s)
--- PASS: TestAccAWSInstanceDataSource_EbsBlockDevice_KmsKeyArn (140.01s)
```
@bflad bflad added this to the v2.23.0 milestone Aug 2, 2019
@bflad
Copy link
Contributor

bflad commented Aug 2, 2019

Hi @frezbo 👋 Thank you for this contribution and apologies for the lengthy merge process.

We have combined #4861, #7757, and some followup work so that the aws_instance resource and data source will support the encryption and kms_key_id arguments with both ebs_block_device and root_block_device. These enhancements will go out with version 2.23.0 of the Terraform AWS Provider, releasing next week.

--- PASS: TestAccAWSInstance_addSecondaryInterface (105.67s)
--- PASS: TestAccAWSInstance_addSecurityGroupNetworkInterface (217.17s)
--- PASS: TestAccAWSInstance_associatePublic_defaultPrivate (44.58s)
--- PASS: TestAccAWSInstance_associatePublic_defaultPublic (50.90s)
--- PASS: TestAccAWSInstance_associatePublic_explicitPrivate (71.19s)
--- PASS: TestAccAWSInstance_associatePublic_explicitPublic (71.06s)
--- PASS: TestAccAWSInstance_associatePublic_overridePrivate (50.87s)
--- PASS: TestAccAWSInstance_associatePublic_overridePublic (50.79s)
--- PASS: TestAccAWSInstance_associatePublicIPAndPrivateIP (70.96s)
--- PASS: TestAccAWSInstance_basic (115.07s)
--- PASS: TestAccAWSInstance_blockDevices (94.01s)
--- PASS: TestAccAWSInstance_changeInstanceType (339.11s)
--- PASS: TestAccAWSInstance_creditSpecification_isNotAppliedToNonBurstable (91.99s)
--- PASS: TestAccAWSInstance_creditSpecification_standardCpuCredits (77.48s)
--- PASS: TestAccAWSInstance_creditSpecification_standardCpuCredits_t2Tot3Taint (459.91s)
--- PASS: TestAccAWSInstance_creditSpecification_unknownCpuCredits_t2 (111.79s)
--- PASS: TestAccAWSInstance_creditSpecification_unknownCpuCredits_t3 (293.73s)
--- PASS: TestAccAWSInstance_creditSpecification_unlimitedCpuCredits (76.85s)
--- PASS: TestAccAWSInstance_creditSpecification_unlimitedCpuCredits_t2Tot3Taint (350.73s)
--- PASS: TestAccAWSInstance_creditSpecification_unspecifiedDefaultsToStandard (193.72s)
--- PASS: TestAccAWSInstance_creditSpecification_updateCpuCredits (92.51s)
--- PASS: TestAccAWSInstance_creditSpecificationT3_standardCpuCredits (279.61s)
--- PASS: TestAccAWSInstance_creditSpecificationT3_unlimitedCpuCredits (312.14s)
--- PASS: TestAccAWSInstance_creditSpecificationT3_unspecifiedDefaultsToUnlimited (305.64s)
--- PASS: TestAccAWSInstance_creditSpecificationT3_updateCpuCredits (129.74s)
--- PASS: TestAccAWSInstance_disableApiTermination (106.76s)
--- PASS: TestAccAWSInstance_disappears (208.33s)
--- PASS: TestAccAWSInstance_EbsBlockDevice_KmsKeyId (96.43s)
--- PASS: TestAccAWSInstance_forceNewAndTagsDrift (91.27s)
--- PASS: TestAccAWSInstance_getPasswordData_falseToTrue (157.57s)
--- PASS: TestAccAWSInstance_getPasswordData_trueToFalse (187.90s)
--- PASS: TestAccAWSInstance_GP2IopsDevice (178.60s)
--- PASS: TestAccAWSInstance_GP2WithIopsValue (192.05s)
--- PASS: TestAccAWSInstance_importBasic (206.86s)
--- PASS: TestAccAWSInstance_importInDefaultVpcBySgId (196.44s)
--- PASS: TestAccAWSInstance_importInDefaultVpcBySgName (198.84s)
--- PASS: TestAccAWSInstance_instanceProfileChange (284.21s)
--- PASS: TestAccAWSInstance_ipv6_supportAddressCount (91.12s)
--- PASS: TestAccAWSInstance_ipv6_supportAddressCountWithIpv4 (61.07s)
--- PASS: TestAccAWSInstance_ipv6_supportAddressCountWithIpv4 (91.69s)
--- PASS: TestAccAWSInstance_ipv6AddressCountAndSingleAddressCausesError (6.80s)
--- PASS: TestAccAWSInstance_keyPairCheck (71.27s)
--- PASS: TestAccAWSInstance_multipleRegions (130.07s)
--- PASS: TestAccAWSInstance_NetworkInstanceRemovingAllSecurityGroups (75.58s)
--- PASS: TestAccAWSInstance_NetworkInstanceSecurityGroups (82.36s)
--- PASS: TestAccAWSInstance_NetworkInstanceVPCSecurityGroupIDs (72.07s)
--- PASS: TestAccAWSInstance_noAMIEphemeralDevices (172.12s)
--- PASS: TestAccAWSInstance_placementGroup (171.05s)
--- PASS: TestAccAWSInstance_primaryNetworkInterface (62.85s)
--- PASS: TestAccAWSInstance_primaryNetworkInterfaceSourceDestCheck (45.98s)
--- PASS: TestAccAWSInstance_privateIP (215.31s)
--- PASS: TestAccAWSInstance_RootBlockDevice_KmsKeyId (294.73s)
--- PASS: TestAccAWSInstance_rootBlockDeviceMismatch (60.88s)
--- PASS: TestAccAWSInstance_rootInstanceStore (77.11s)
--- PASS: TestAccAWSInstance_sourceDestCheck (106.55s)
--- PASS: TestAccAWSInstance_tags (344.66s)
--- PASS: TestAccAWSInstance_UserData_EmptyStringToUnspecified (83.89s)
--- PASS: TestAccAWSInstance_UserData_UnspecifiedToEmptyString (84.78s)
--- PASS: TestAccAWSInstance_userDataBase64 (209.29s)
--- PASS: TestAccAWSInstance_volumeTags (106.31s)
--- PASS: TestAccAWSInstance_volumeTagsComputed (121.81s)
--- PASS: TestAccAWSInstance_vpc (109.39s)
--- PASS: TestAccAWSInstance_withIamInstanceProfile (118.89s)
--- PASS: TestAccAWSInstanceDataSource_AzUserData (229.07s)
--- PASS: TestAccAWSInstanceDataSource_basic (118.88s)
--- PASS: TestAccAWSInstanceDataSource_blockDevices (88.20s)
--- PASS: TestAccAWSInstanceDataSource_creditSpecification (86.16s)
--- PASS: TestAccAWSInstanceDataSource_EbsBlockDevice_KmsKeyId (110.83s)
--- PASS: TestAccAWSInstanceDataSource_getPasswordData_falseToTrue (161.42s)
--- PASS: TestAccAWSInstanceDataSource_getPasswordData_trueToFalse (169.14s)
--- PASS: TestAccAWSInstanceDataSource_GetUserData (163.49s)
--- PASS: TestAccAWSInstanceDataSource_GetUserData_NoUserData (116.04s)
--- PASS: TestAccAWSInstanceDataSource_gp2IopsDevice (186.51s)
--- PASS: TestAccAWSInstanceDataSource_keyPair (84.88s)
--- PASS: TestAccAWSInstanceDataSource_PlacementGroup (71.30s)
--- PASS: TestAccAWSInstanceDataSource_privateIP (57.50s)
--- PASS: TestAccAWSInstanceDataSource_RootBlockDevice_KmsKeyId (106.82s)
--- PASS: TestAccAWSInstanceDataSource_rootInstanceStore (86.51s)
--- PASS: TestAccAWSInstanceDataSource_tags (243.25s)
--- PASS: TestAccAWSInstanceDataSource_VPC (105.44s)
--- PASS: TestAccAWSInstanceDataSource_VPCSecurityGroups (131.62s)
--- SKIP: TestAccAWSInstance_importInEc2Classic (1.10s)

@bflad bflad merged commit 2c3ed11 into hashicorp:master Aug 2, 2019
bflad added a commit that referenced this pull request Aug 2, 2019
…nd root_block_device configuration block encryption and kms_key_arn arguments

Reference: #4861
Reference: #7757

Having combined and resolved conflicts between #4861 and #7757, performed the following changes:

* Ensured that existing acceptance testing was untouched and only new tests were added
* Ensured data source added new attributes to match resource to prevent panics
* Mark new arguments as Computed: true to only show operators differences when a value is configured

Output from acceptance testing:

```
--- PASS: TestAccAWSInstance_addSecondaryInterface (105.67s)
--- PASS: TestAccAWSInstance_addSecurityGroupNetworkInterface (217.17s)
--- PASS: TestAccAWSInstance_associatePublic_defaultPrivate (44.58s)
--- PASS: TestAccAWSInstance_associatePublic_defaultPublic (50.90s)
--- PASS: TestAccAWSInstance_associatePublic_explicitPrivate (71.19s)
--- PASS: TestAccAWSInstance_associatePublic_explicitPublic (71.06s)
--- PASS: TestAccAWSInstance_associatePublic_overridePrivate (50.87s)
--- PASS: TestAccAWSInstance_associatePublic_overridePublic (50.79s)
--- PASS: TestAccAWSInstance_associatePublicIPAndPrivateIP (70.96s)
--- PASS: TestAccAWSInstance_basic (115.07s)
--- PASS: TestAccAWSInstance_blockDevices (94.01s)
--- PASS: TestAccAWSInstance_changeInstanceType (339.11s)
--- PASS: TestAccAWSInstance_creditSpecification_isNotAppliedToNonBurstable (91.99s)
--- PASS: TestAccAWSInstance_creditSpecification_standardCpuCredits (77.48s)
--- PASS: TestAccAWSInstance_creditSpecification_standardCpuCredits_t2Tot3Taint (459.91s)
--- PASS: TestAccAWSInstance_creditSpecification_unknownCpuCredits_t2 (111.79s)
--- PASS: TestAccAWSInstance_creditSpecification_unknownCpuCredits_t3 (293.73s)
--- PASS: TestAccAWSInstance_creditSpecification_unlimitedCpuCredits (76.85s)
--- PASS: TestAccAWSInstance_creditSpecification_unlimitedCpuCredits_t2Tot3Taint (350.73s)
--- PASS: TestAccAWSInstance_creditSpecification_unspecifiedDefaultsToStandard (193.72s)
--- PASS: TestAccAWSInstance_creditSpecification_updateCpuCredits (92.51s)
--- PASS: TestAccAWSInstance_creditSpecificationT3_standardCpuCredits (279.61s)
--- PASS: TestAccAWSInstance_creditSpecificationT3_unlimitedCpuCredits (312.14s)
--- PASS: TestAccAWSInstance_creditSpecificationT3_unspecifiedDefaultsToUnlimited (305.64s)
--- PASS: TestAccAWSInstance_creditSpecificationT3_updateCpuCredits (129.74s)
--- PASS: TestAccAWSInstance_disableApiTermination (106.76s)
--- PASS: TestAccAWSInstance_disappears (208.33s)
--- PASS: TestAccAWSInstance_EbsBlockDevice_KmsKeyArn (96.43s)
--- PASS: TestAccAWSInstance_forceNewAndTagsDrift (91.27s)
--- PASS: TestAccAWSInstance_getPasswordData_falseToTrue (157.57s)
--- PASS: TestAccAWSInstance_getPasswordData_trueToFalse (187.90s)
--- PASS: TestAccAWSInstance_GP2IopsDevice (178.60s)
--- PASS: TestAccAWSInstance_GP2WithIopsValue (192.05s)
--- PASS: TestAccAWSInstance_importBasic (206.86s)
--- PASS: TestAccAWSInstance_importInDefaultVpcBySgId (196.44s)
--- PASS: TestAccAWSInstance_importInDefaultVpcBySgName (198.84s)
--- PASS: TestAccAWSInstance_instanceProfileChange (284.21s)
--- PASS: TestAccAWSInstance_ipv6_supportAddressCount (91.12s)
--- PASS: TestAccAWSInstance_ipv6_supportAddressCountWithIpv4 (61.07s)
--- PASS: TestAccAWSInstance_ipv6_supportAddressCountWithIpv4 (91.69s)
--- PASS: TestAccAWSInstance_ipv6AddressCountAndSingleAddressCausesError (6.80s)
--- PASS: TestAccAWSInstance_keyPairCheck (71.27s)
--- PASS: TestAccAWSInstance_multipleRegions (130.07s)
--- PASS: TestAccAWSInstance_NetworkInstanceRemovingAllSecurityGroups (75.58s)
--- PASS: TestAccAWSInstance_NetworkInstanceSecurityGroups (82.36s)
--- PASS: TestAccAWSInstance_NetworkInstanceVPCSecurityGroupIDs (72.07s)
--- PASS: TestAccAWSInstance_noAMIEphemeralDevices (172.12s)
--- PASS: TestAccAWSInstance_placementGroup (171.05s)
--- PASS: TestAccAWSInstance_primaryNetworkInterface (62.85s)
--- PASS: TestAccAWSInstance_primaryNetworkInterfaceSourceDestCheck (45.98s)
--- PASS: TestAccAWSInstance_privateIP (215.31s)
--- PASS: TestAccAWSInstance_RootBlockDevice_KmsKeyArn (294.73s)
--- PASS: TestAccAWSInstance_rootBlockDeviceMismatch (60.88s)
--- PASS: TestAccAWSInstance_rootInstanceStore (77.11s)
--- PASS: TestAccAWSInstance_sourceDestCheck (106.55s)
--- PASS: TestAccAWSInstance_tags (344.66s)
--- PASS: TestAccAWSInstance_UserData_EmptyStringToUnspecified (83.89s)
--- PASS: TestAccAWSInstance_UserData_UnspecifiedToEmptyString (84.78s)
--- PASS: TestAccAWSInstance_userDataBase64 (209.29s)
--- PASS: TestAccAWSInstance_volumeTags (106.31s)
--- PASS: TestAccAWSInstance_volumeTagsComputed (121.81s)
--- PASS: TestAccAWSInstance_vpc (109.39s)
--- PASS: TestAccAWSInstance_withIamInstanceProfile (118.89s)
--- PASS: TestAccAWSInstanceDataSource_AzUserData (229.07s)
--- PASS: TestAccAWSInstanceDataSource_basic (118.88s)
--- PASS: TestAccAWSInstanceDataSource_blockDevices (88.20s)
--- PASS: TestAccAWSInstanceDataSource_creditSpecification (86.16s)
--- PASS: TestAccAWSInstanceDataSource_EbsBlockDevice_KmsKeyArn (110.83s)
--- PASS: TestAccAWSInstanceDataSource_getPasswordData_falseToTrue (161.42s)
--- PASS: TestAccAWSInstanceDataSource_getPasswordData_trueToFalse (169.14s)
--- PASS: TestAccAWSInstanceDataSource_GetUserData (163.49s)
--- PASS: TestAccAWSInstanceDataSource_GetUserData_NoUserData (116.04s)
--- PASS: TestAccAWSInstanceDataSource_gp2IopsDevice (186.51s)
--- PASS: TestAccAWSInstanceDataSource_keyPair (84.88s)
--- PASS: TestAccAWSInstanceDataSource_PlacementGroup (71.30s)
--- PASS: TestAccAWSInstanceDataSource_privateIP (57.50s)
--- PASS: TestAccAWSInstanceDataSource_RootBlockDevice_KmsKeyArn (106.82s)
--- PASS: TestAccAWSInstanceDataSource_rootInstanceStore (86.51s)
--- PASS: TestAccAWSInstanceDataSource_tags (243.25s)
--- PASS: TestAccAWSInstanceDataSource_VPC (105.44s)
--- PASS: TestAccAWSInstanceDataSource_VPCSecurityGroups (131.62s)
--- SKIP: TestAccAWSInstance_importInEc2Classic (1.10s)
```
bflad added a commit that referenced this pull request Aug 2, 2019
@frezbo frezbo deleted the feature/add-kms branch August 2, 2019 12:11
@frezbo
Copy link
Contributor Author

frezbo commented Aug 2, 2019

Couldn't be any happier ❤️

@ghost
Copy link

ghost commented Aug 7, 2019

This has been released in version 2.23.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 Nov 2, 2019

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 and limited conversation to collaborators Nov 2, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement Requests to existing resources that expand the functionality or scope. service/ec2 Issues and PRs that pertain to the ec2 service. size/M Managed by automation to categorize the size of a PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants