CodeArtifact encryption_key argument should be optional+computed #15573

j3parker · 2020-10-09T13:34:22Z

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

Release note for CHANGELOG:

resource/aws_codeartifact_domain: Change encryption_key to an optional+computed attribute

Output from acceptance testing:

$ make testacc TESTARGS='-run=TestAccAWSCodeArtifactDomain*'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSCodeArtifactDomain_* -timeout 120m
=== RUN   TestAccAWSCodeArtifactDomainPermissionsPolicy_basic
=== PAUSE TestAccAWSCodeArtifactDomainPermissionsPolicy_basic
=== RUN   TestAccAWSCodeArtifactDomainPermissionsPolicy_owner
=== PAUSE TestAccAWSCodeArtifactDomainPermissionsPolicy_owner
=== RUN   TestAccAWSCodeArtifactDomainPermissionsPolicy_disappears
=== PAUSE TestAccAWSCodeArtifactDomainPermissionsPolicy_disappears
=== RUN   TestAccAWSCodeArtifactDomainPermissionsPolicy_disappears_domain
=== PAUSE TestAccAWSCodeArtifactDomainPermissionsPolicy_disappears_domain
=== RUN   TestAccAWSCodeArtifactDomain_basic
=== PAUSE TestAccAWSCodeArtifactDomain_basic
=== RUN   TestAccAWSCodeArtifactDomain_defaultencryptionkey
=== PAUSE TestAccAWSCodeArtifactDomain_defaultencryptionkey
=== RUN   TestAccAWSCodeArtifactDomain_disappears
=== PAUSE TestAccAWSCodeArtifactDomain_disappears
=== CONT  TestAccAWSCodeArtifactDomainPermissionsPolicy_basic
=== CONT  TestAccAWSCodeArtifactDomainPermissionsPolicy_disappears_domain
=== CONT  TestAccAWSCodeArtifactDomain_basic
=== CONT  TestAccAWSCodeArtifactDomain_disappears
=== CONT  TestAccAWSCodeArtifactDomain_defaultencryptionkey
=== CONT  TestAccAWSCodeArtifactDomainPermissionsPolicy_disappears
=== CONT  TestAccAWSCodeArtifactDomainPermissionsPolicy_owner
2020/10/14 13:48:17 [DEBUG] Waiting for state to become: [success]
=== CONT  TestAccAWSCodeArtifactDomain_defaultencryptionkey
    resource_aws_codeartifact_domain_test.go:104: Step 1/2 error: Error running apply:
        Error: error creating CodeArtifact Domain: InvalidParameter: 1 validation error(s) found.
        - minimum field size of 1, CreateDomainInput.EncryptionKey.



--- FAIL: TestAccAWSCodeArtifactDomain_defaultencryptionkey (32.34s)
--- PASS: TestAccAWSCodeArtifactDomainPermissionsPolicy_disappears (40.24s)
--- PASS: TestAccAWSCodeArtifactDomainPermissionsPolicy_disappears_domain (44.32s)
--- PASS: TestAccAWSCodeArtifactDomain_basic (65.13s)
--- PASS: TestAccAWSCodeArtifactDomainPermissionsPolicy_owner (75.98s)
--- PASS: TestAccAWSCodeArtifactDomain_disappears (81.95s)
--- PASS: TestAccAWSCodeArtifactDomainPermissionsPolicy_basic (100.26s)
FAIL
FAIL    github.com/terraform-providers/terraform-provider-aws/aws       100.306s
FAIL
GNUmakefile:26: recipe for target 'testacc' failed
make: *** [testacc] Error 1

j3parker · 2020-10-09T13:34:54Z

aws/resource_aws_codeartifact_domain.go

@@ -33,7 +33,7 @@ func resourceAwsCodeArtifactDomain() *schema.Resource {
 			},
 			"encryption_key": {
 				Type:         schema.TypeString,
-				Required:     true,
+				Optional:     true,


It is optional in the SDK and in the API.

CC @DrFaust92

Hey @j3parker,
I'll take a closer look but from what I recall the API validates that the kms Id field is not empty even if it's optional. That's why it's required in terraform.

Here's what I get from the CLI:

$ aws configure set cli_history enabled $ aws codeartifact create-domain --domain test { "domain": { "name": "test", "owner": "(redacted)", "arn": "arn:aws:codeartifact:us-east-1:(redacted):domain/test", "status": "Active", "createdTime": "2020-10-09T10:33:21.559000-04:00", "encryptionKey": "arn:aws:kms:us-east-1:(redacted):key/(redacted)", "repositoryCount": 0, "assetSizeBytes": 0 } } $ aws history show AWS CLI command entered at time: 2020-10-09 10:33:20.776 with AWS CLI version: aws-cli/2.0.49 Python/3.7.7 Windows/10 exe/AMD64 with arguments: ['codeartifact', 'create-domain', '--domain', 'test'] [0] API call made at time: 2020-10-09 10:33:20.930 to service: codeartifact using operation: CreateDomain with parameters: { "domain": "test" } [0] HTTP request sent at time: 2020-10-09 10:33:20.936 to URL: https://codeartifact.us-east-1.amazonaws.com/v1/domain?domain=test with method: POST with headers: { "Authorization": "(redacted)", "Content-Length": "0", "User-Agent": "aws-cli/2.0.49 Python/3.7.7 Windows/10 exe/AMD64 command/codeartifact.create-domain", "X-Amz-Date": "20201009T143320Z", "X-Amz-Security-Token": "(redacted)" } with body: There is no associated body [0] HTTP response received at time: 2020-10-09 10:33:21.466 with status code: 200 with headers: { "Connection": "keep-alive", "Content-Length": "354", "Content-Type": "application/json", "Date": "Fri, 09 Oct 2020 14:33:21 GMT", "x-amzn-RequestId": "(redacted)" } with body: { "domain": { "arn": "arn:aws:codeartifact:us-east-1:(redacted):domain/test", "assetSizeBytes": 0, "createdTime": 1602254001.559, "encryptionKey": "arn:aws:kms:us-east-1:(redacted):key/(redacted)", "name": "test", "owner": "(redacted)", "repositoryCount": 0, "s3BucketArn": "arn:aws:s3:::assets-(redacted)-us-east-1", "status": "Active" } }

So it looks like it's truly optional (it's not needed even on the wire). There is an encryptionKey in the response, but that's the (auto-generated on-demand) AWS-managed CMK. There is at most one of those per-account, and typically it's the default if a customer-managed CMK isn't specified (AWS takes care of it completely, and it gets shared between services, they're free etc.)

DrFaust92 · 2020-10-09T14:41:28Z

Maybe an issue for the go sdk specifically, thanks for checking this. I'll try again and open a ticket on go sdk repo if it fails again.

…

On Fri, Oct 9, 2020, 17:38 Jacob Parker ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In aws/resource_aws_codeartifact_domain.go <#15573 (comment)> : > @@ -33,7 +33,7 @@ func resourceAwsCodeArtifactDomain() *schema.Resource { }, "encryption_key": { Type: schema.TypeString, - Required: true, + Optional: true, Here's what I get from the CLI: $ aws configure set cli_history enabled $ aws codeartifact create-domain --domain test { "domain": { "name": "test", "owner": "(redacted)", "arn": "arn:aws:codeartifact:us-east-1:(redacted):domain/test", "status": "Active", "createdTime": "2020-10-09T10:33:21.559000-04:00", "encryptionKey": "arn:aws:kms:us-east-1:(redacted):key/(redacted)", "repositoryCount": 0, "assetSizeBytes": 0 } } $ aws history show AWS CLI command entered at time: 2020-10-09 10:33:20.776 with AWS CLI version: aws-cli/2.0.49 Python/3.7.7 Windows/10 exe/AMD64 with arguments: ['codeartifact', 'create-domain', '--domain', 'test'] [0] API call made at time: 2020-10-09 10:33:20.930 to service: codeartifact using operation: CreateDomain with parameters: { "domain": "test" } [0] HTTP request sent at time: 2020-10-09 10:33:20.936 to URL: https://codeartifact.us-east-1.amazonaws.com/v1/domain?domain=test with method: POST with headers: { "Authorization": "(redacted)", "Content-Length": "0", "User-Agent": "aws-cli/2.0.49 Python/3.7.7 Windows/10 exe/AMD64 command/codeartifact.create-domain", "X-Amz-Date": "20201009T143320Z", "X-Amz-Security-Token": "(redacted)" } with body: There is no associated body [0] HTTP response received at time: 2020-10-09 10:33:21.466 with status code: 200 with headers: { "Connection": "keep-alive", "Content-Length": "354", "Content-Type": "application/json", "Date": "Fri, 09 Oct 2020 14:33:21 GMT", "x-amzn-RequestId": "(redacted)" } with body: { "domain": { "arn": "arn:aws:codeartifact:us-east-1:(redacted):domain/test", "assetSizeBytes": 0, "createdTime": 1602254001.559, "encryptionKey": "arn:aws:kms:us-east-1:(redacted):key/(redacted)", "name": "test", "owner": "(redacted)", "repositoryCount": 0, "s3BucketArn": "arn:aws:s3:::assets-(redacted)-us-east-1", "status": "Active" } } So it looks like it's truly optional (it's not needed even on the wire). If it's not supplied it defaults to an AWS-managed CMK which is pretty typical for AWS services (they get created once per account, but only on-demand. AWS takes care of rotating them, they're free, you don't typically need to specify them, etc.) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#15573 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AIA2W7V4VDL4RG34E4GDA5TSJ4N4TANCNFSM4SKC2E4A> .

DrFaust92 · 2020-10-09T16:40:55Z

@j3parker, can you also add computed attribute to kms key id? If none is passed and the API returns a key we need to handle diffs with computed.

Alsop please run acceptance tests.

Thanks again!

j3parker · 2020-10-09T16:43:16Z

Cool, sure thing!

j3parker · 2020-10-14T17:16:45Z

done -- attached the logs to the PR description; they passed.

DrFaust92 · 2020-10-14T17:39:31Z

Hey @j3parker,

Can you add an acceptance test for this use case an update docs? Thanks.

j3parker · 2020-10-14T17:41:31Z

Sure thing.

j3parker · 2020-10-14T17:59:21Z

Done.

RE: documentation, I based the new sentence after the one for the kms_master_key_id attribute for S3 buckets.
Here's a screenshot in the UI to show that the key name alias:

RE: the test -- it's failing with this error:

=== CONT  TestAccAWSCodeArtifactDomain_defaultencryptionkey
    resource_aws_codeartifact_domain_test.go:104: Step 1/2 error: Error running apply:
        Error: error creating CodeArtifact Domain: InvalidParameter: 1 validation error(s) found.
        - minimum field size of 1, CreateDomainInput.EncryptionKey.

I assume that's what you're talking about with the golang SDK. Should I open an issue in that repo?
Btw -- I was pattern matching with this test case, let me know if it's what you wanted/if there is something more I should be doing.

website/docs/r/codeartifact_domain.html.markdown

j3parker · 2020-10-14T18:48:38Z

Hmmm... I'm not sure. I can make it work directly with the SDK.
I played around with the validation stuff in the provider but couldn't get it to work. I'll dig around more in a bit.

package main                                                                                                                                                                                                     
import (
    "fmt"
    "github.com/aws/aws-sdk-go/aws"
    "github.com/aws/aws-sdk-go/aws/session"
    "github.com/aws/aws-sdk-go/service/codeartifact"
)

func main() {
    sess, err := session.NewSession(&aws.Config{
        Region: aws.String("us-east-1")},
    )

    svc := codeartifact.New(sess)

    domain := "foo"
    res, err := svc.CreateDomain(&codeartifact.CreateDomainInput{
        Domain: &domain,
    })

    if err != nil {
        fmt.Println("Could not create domain", err)
        return
    }

    fmt.Println("stuff", res)
}

$ go run main.go
stuff {
  Domain: {
    Arn: "arn:aws:codeartifact:us-east-1:*****:domain/foo",
    AssetSizeBytes: 0,
    CreatedTime: 2020-10-14 18:46:53.39 +0000 UTC,
    EncryptionKey: "arn:aws:kms:us-east-1:*****:key/*****",
    Name: "foo",
    Owner: "*****",
    RepositoryCount: 0,
    Status: "Active"
  }
}

j3parker · 2020-11-06T16:30:07Z

Just fixing merge conflicts; hoping to look into this again today or Monday.

ewbankkit · 2021-01-23T21:57:12Z

Replaced by #17262.

ghost · 2021-01-28T22:14:17Z

This has been released in version 3.26.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 · 2021-02-23T17:10:00Z

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!

CodeArtifact encryption_key argument should be optional

b398b7b

j3parker requested a review from a team October 9, 2020 13:34

ghost added service/codeartifact Issues and PRs that pertain to the codeartifact service. size/XS Managed by automation to categorize the size of a PR. labels Oct 9, 2020

github-actions bot added the needs-triage Waiting for first response or review from a maintainer. label Oct 9, 2020

j3parker commented Oct 9, 2020

View reviewed changes

DrFaust92 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 Oct 9, 2020

Set encryption_key to be a computed attribute

540691c

j3parker changed the title ~~CodeArtifact encryption_key argument should be optional~~ CodeArtifact encryption_key argument should be optional+computed Oct 14, 2020

Add acceptance test + docs

a4649a9

j3parker commented Oct 14, 2020

View reviewed changes

website/docs/r/codeartifact_domain.html.markdown Show resolved Hide resolved

Fix format in docs

fd99168

Merge branch 'master' into patch-1

1f4b695

j3parker requested a review from a team as a code owner November 6, 2020 16:29

Fix broken merge

cc51f36

ewbankkit assigned DrFaust92 Jan 18, 2021

Base automatically changed from master to main January 23, 2021 00:59

DrFaust92 mentioned this pull request Jan 23, 2021

r/codeartifact_domain - make encryption_key optional #17262

Merged

DrFaust92 unassigned DrFaust92 Jan 23, 2021

ewbankkit closed this Jan 23, 2021

j3parker deleted the patch-1 branch January 25, 2021 15:18

github-actions bot added this to the v3.26.0 milestone Jan 28, 2021

ghost locked as resolved and limited conversation to collaborators Feb 23, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CodeArtifact encryption_key argument should be optional+computed #15573

CodeArtifact encryption_key argument should be optional+computed #15573

j3parker commented Oct 9, 2020 •

edited

Loading

j3parker Oct 9, 2020 •

edited

Loading

j3parker Oct 9, 2020

DrFaust92 Oct 9, 2020

j3parker Oct 9, 2020 •

edited

Loading

DrFaust92 commented Oct 9, 2020 via email

DrFaust92 commented Oct 9, 2020

j3parker commented Oct 9, 2020

j3parker commented Oct 14, 2020 •

edited

Loading

DrFaust92 commented Oct 14, 2020

j3parker commented Oct 14, 2020

j3parker commented Oct 14, 2020

j3parker commented Oct 14, 2020 •

edited

Loading

j3parker commented Nov 6, 2020

ewbankkit commented Jan 23, 2021

ghost commented Jan 28, 2021

ghost commented Feb 23, 2021

CodeArtifact encryption_key argument should be optional+computed #15573

CodeArtifact encryption_key argument should be optional+computed #15573

Conversation

j3parker commented Oct 9, 2020 • edited Loading

Community Note

j3parker Oct 9, 2020 • edited Loading

Choose a reason for hiding this comment

j3parker Oct 9, 2020

Choose a reason for hiding this comment

DrFaust92 Oct 9, 2020

Choose a reason for hiding this comment

j3parker Oct 9, 2020 • edited Loading

Choose a reason for hiding this comment

DrFaust92 commented Oct 9, 2020 via email

DrFaust92 commented Oct 9, 2020

j3parker commented Oct 9, 2020

j3parker commented Oct 14, 2020 • edited Loading

DrFaust92 commented Oct 14, 2020

j3parker commented Oct 14, 2020

j3parker commented Oct 14, 2020

j3parker commented Oct 14, 2020 • edited Loading

j3parker commented Nov 6, 2020

ewbankkit commented Jan 23, 2021

ghost commented Jan 28, 2021

ghost commented Feb 23, 2021

j3parker commented Oct 9, 2020 •

edited

Loading

j3parker Oct 9, 2020 •

edited

Loading

j3parker Oct 9, 2020 •

edited

Loading

j3parker commented Oct 14, 2020 •

edited

Loading

j3parker commented Oct 14, 2020 •

edited

Loading