New Data Source: aws_kms_key #2224

trung · 2017-11-09T01:45:12Z

To fix #2009

…vider-aws

trung · 2017-11-10T19:39:10Z

❯ make testacc TESTARGS="-run=TestAccDataSourceAwsKmsKey"
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test $(go list ./... |grep -v 'vendor') -v -run=TestAccDataSourceAwsKmsKey -timeout 120m
?   	github.com/terraform-providers/terraform-provider-aws	[no test files]
=== RUN   TestAccDataSourceAwsKmsKey
--- PASS: TestAccDataSourceAwsKmsKey (48.90s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	48.937s

bflad · 2017-12-05T17:29:50Z

While I certainly see the value here, it seems it would also be fairly simple to provide a target_key_arn on the existing aws_kms_alias data source by substituting the target key ID into the alias ARN. We can get away with this since KMS aliases and keys are bound to the same partition, account, and region.

See also CreateAlias documentation:

The alias and the CMK it is mapped to must be in the same AWS account and the same region. You cannot perform this operation on an alias in a different AWS account.

bflad

@trung awesome work here! This is looking pretty good, just a few minor items to take care of then I believe this is good to go. Please let us know if you don't have time to finish this up. 😄

For what its worth given the ability for this to accept alias inputs as well, we might consider this data source for deprecating or aliasing the aws_kms_alias data source to this one in the future.

bflad · 2018-02-14T19:45:26Z

aws/data_source_aws_kms_key.go

+
+func dataSourceAwsKmsKeyRead(d *schema.ResourceData, meta interface{}) error {
+	conn := meta.(*AWSClient).kmsconn
+	keyId, keyIdOk := d.GetOk("key_id")


Required: true is set, so this can simply be keyId := d.Get("key_id")

bflad · 2018-02-14T19:49:20Z

aws/data_source_aws_kms_key.go

+	}
+	var grantTokens []*string
+	if v, ok := d.GetOk("grant_tokens"); ok {
+		for _, token := range v.([]interface{}) {


I believe you can simplify this to just grantTokens = aws.StringSlice(v.([]string))

bflad · 2018-02-14T19:50:55Z

aws/data_source_aws_kms_key_test.go

+	"github.com/hashicorp/terraform/terraform"
+)
+
+func TestAccDataSourceAwsKmsKey(t *testing.T) {


Nitpick: can you append _basic to this function name please? Just a consistent convention we try to use everywhere

bflad · 2018-02-14T19:57:51Z

aws/data_source_aws_kms_key_test.go

+			resource.TestStep{
+				Config: testAccDataSourceAwsKmsKeyConfig,
+				Check: resource.ComposeTestCheckFunc(
+					testAccDataSourceAwsKmsKeyCheck("data.aws_kms_key.arbitrary"),


Nitpick: Can you add simple state checks for other attributes we're expecting to set? This helps with future maintainability. 👍 e.g. (not exhaustive below)

resource.TestMatchResourceAttr("data.aws_kms_key.arbitrary", "arn", regexp.MustCompile("^arn:[^:]+:kms:[^:]+:[^:]+:key/.+")), resource.TestCheckResourceAttr("data.aws_kms_key.arbitrary", "enabled", "true"), resource.TestCheckResourceAttrSet("data.aws_kms_key.arbitrary", "valid_to"),

bflad · 2018-02-14T20:01:06Z

aws/validators.go

+
+func validateKmsKey(v interface{}, k string) (ws []string, errors []error) {
+	value := v.(string)
+	arnPrefixPattern := `arn:[\w-]+:([a-zA-Z0-9\-])+:([a-z]{2}-(gov-)?[a-z]+-\d{1})?:(\d{12})?:`


Can we switch this to not assume anything about the format of AWS partitions, regions, and account IDs in the future please? arn:[^:]+:kms:[^:]+:[^:]+:

bflad · 2018-02-14T20:03:15Z

website/docs/d/kms_key.html.markdown

+  Get information on a AWS Key Management Service (KMS) Key
+---
+
+# aws\_kms\_key


Nitpick: Backslashes are no longer required in the documentation headers

trung · 2018-02-16T04:18:49Z

@bflad thanks for the review
I'm on Lunar New Year celebration now so i won't be able to make changes in the next couple of days. If this is not hurry, i can certainly work on it sometime next week

bflad · 2018-02-16T17:02:32Z

@trung No worries at all, please enjoy your holiday! 🎉

trung · 2018-02-26T04:30:12Z

@bflad I've made some changes per your review

❯ make testacc TESTARGS="-run=TestAccDataSourceAwsKmsKey_basic"
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./... -v -run=TestAccDataSourceAwsKmsKey_basic -timeout 120m
?   	github.com/terraform-providers/terraform-provider-aws	[no test files]
=== RUN   TestAccDataSourceAwsKmsKey_basic
--- PASS: TestAccDataSourceAwsKmsKey_basic (41.88s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	41.922s

bflad

This looks great! Thanks for your work here! 🚀

 make testacc TEST=./aws TESTARGS='-run=TestAccDataSourceAwsKmsKey'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccDataSourceAwsKmsKey -timeout 120m
=== RUN   TestAccDataSourceAwsKmsKey_basic
--- PASS: TestAccDataSourceAwsKmsKey_basic (175.97s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	176.013s

bflad · 2018-03-09T20:03:19Z

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

ghost · 2020-04-07T17:37:48Z

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!

trung and others added 15 commits September 20, 2017 14:54

test case to cover gp2 with iops

90c1c22

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

fdc3867

…vider-aws

test case to cover gp2 with iops

68ed0d7

merged upstream/master

285d3d6

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

14e39a1

…vider-aws

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

07cc945

…vider-aws

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

ea8d786

…vider-aws

validate kms key input format

7ad8692

setup schema

c9efdad

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

d4223a8

…vider-aws

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

8a6cb04

…vider-aws

merged with master

cd7b03d

hashicorp#2009: support DescribeKey

7c572c3

hashicorp#2009: acceptance test

55800a4

hashicorp#2009: documentation

1195057

trung changed the title ~~data/aws_kms_key: Implement DescribeKey API~~ d/aws_kms_key: Implement DescribeKey API Nov 9, 2017

radeksimko added the new-data-source Introduces a new data source. label Nov 9, 2017

radeksimko added the size/L Managed by automation to categorize the size of a PR. label Nov 15, 2017

Merge branch 'master' into f-kms-key

edc5096

This was referenced Dec 5, 2017

datasource/aws_kms_alias: Add target_key_arn attribute #2551

Merged

Support ARNs for existing KMS keys #2009

Closed

radeksimko added the service/kms Issues and PRs that pertain to the kms service. label Jan 17, 2018

radeksimko changed the title ~~d/aws_kms_key: Implement DescribeKey API~~ New Data Source: aws_kms_key Jan 17, 2018

bflad reviewed Feb 14, 2018

View reviewed changes

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

merged with master

656d814

trung added 2 commits February 25, 2018 23:06

hashicorp#2009: fixed per @bflad review

e28ea81

hashicorp#2009: fixed tests

7b9f50d

ghost added the size/L Managed by automation to categorize the size of a PR. label Feb 26, 2018

bflad removed the waiting-response Maintainers are waiting on response from community or contributor. label Feb 26, 2018

bflad added this to the v1.11.0 milestone Feb 26, 2018

bflad approved these changes Feb 26, 2018

View reviewed changes

bflad merged commit ac6ef7b into hashicorp:master Feb 26, 2018

bflad added a commit that referenced this pull request Feb 26, 2018

Update CHANGELOG for #2224

cc40cca

trung deleted the f-kms-key branch February 26, 2018 12:32

ghost locked and limited conversation to collaborators Apr 7, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New Data Source: aws_kms_key #2224

New Data Source: aws_kms_key #2224

trung commented Nov 9, 2017

trung commented Nov 10, 2017

bflad commented Dec 5, 2017

bflad left a comment

bflad Feb 14, 2018

bflad Feb 14, 2018

bflad Feb 14, 2018

bflad Feb 14, 2018

bflad Feb 14, 2018

bflad Feb 14, 2018

trung commented Feb 16, 2018

bflad commented Feb 16, 2018

trung commented Feb 26, 2018

bflad left a comment

bflad commented Mar 9, 2018

ghost commented Apr 7, 2020

New Data Source: aws_kms_key #2224

New Data Source: aws_kms_key #2224

Conversation

trung commented Nov 9, 2017

trung commented Nov 10, 2017

bflad commented Dec 5, 2017

bflad left a comment

Choose a reason for hiding this comment

bflad Feb 14, 2018

Choose a reason for hiding this comment

bflad Feb 14, 2018

Choose a reason for hiding this comment

bflad Feb 14, 2018

Choose a reason for hiding this comment

bflad Feb 14, 2018

Choose a reason for hiding this comment

bflad Feb 14, 2018

Choose a reason for hiding this comment

bflad Feb 14, 2018

Choose a reason for hiding this comment

trung commented Feb 16, 2018

bflad commented Feb 16, 2018

trung commented Feb 26, 2018

bflad left a comment

Choose a reason for hiding this comment

bflad commented Mar 9, 2018

ghost commented Apr 7, 2020