hashicorp / terraform-provider-aws

The AWS Provider enables Terraform to manage AWS resources.
https://registry.terraform.io/providers/hashicorp/aws
Mozilla Public License 2.0
9.86k stars 9.2k forks source link

[Bug]: `TestAccPinpointEmailChannel_basic` and `TestAccPinpointEmailChannel_set` acceptance tests fail #38772

Open DanielRieske opened 3 months ago

DanielRieske commented 3 months ago

Terraform Core Version

v1.9.1

AWS Provider Version

v5.60.0

Affected Resource(s)

Expected Behavior

That both tests pass.

Actual Behavior

They both fail with similar erorrs that after running the plan RoleARN was missing.

This is due to the GetEmailChannel call within the read action of this resource. The response body does not return RoleARN even though with the prior UpdateEmailChannel action it is sent to the API.

`2024-08-07T21:47:06.964+0200 [DEBUG] aws: HTTP Request Sent: tf_aws.sdk=aws-sdk-go-v2 rpc.method=UpdateEmailChannel http.request.header.x_amz_date=20240807T194706Z http.url=https://pinpoint.us-west-2.amazonaws.com/v1/apps/5b81d750ea1748eba0c4ca78a7a9a37c/channels/email rpc.system=aws-api aws.region=us-west-2 tf_req_id=dfe6148f-6856-6f06-cca1-97af6f81819e tf_resource_type=aws_pinpoint_email_channel http.request.header.amz_sdk_invocation_id=2e470a55-de19-4cdf-b518-fca24e293d13 http.request_content_length=230 http.request.header.content_type=application/json http.request.header.authorization="AWS4-HMAC-SHA256 Credential=AKIA************LQDY/20240807/us-west-2/mobiletargeting/aws4_request, SignedHeaders=amz-sdk-invocation-id;amz-sdk-request;content-length;content-type;host;x-amz-date, Signature=*****" rpc.service=Pinpoint http.request.header.amz_sdk_request="attempt=1; max=25"
  http.request.body=
  | {"Enabled":false,"FromAddress":"tf-acc-test-7788522522147484357@bptw3i72.test","Identity":"arn:aws:ses:us-west-2:170689858638:identity/bptw3i72.test","RoleArn":"arn:aws:iam::012345678910:role/terraform-20240807194706056100000002"}
2024-08-07T21:47:07.363+0200 [DEBUG] aws: HTTP Response Received: rpc.service=Pinpoint http.duration=209 http.response.header.x_amzn_requestid=6256168e-90a0-4b15-b6b4-926ebd6ad628 tf_provider_addr=registry.terraform.io/hashicorp/aws tf_resource_type=aws_pinpoint_email_channel tf_aws.sdk=aws-sdk-go-v2 rpc.method=GetEmailChannel http.response.header.content_type=application/json http.response.header.via="1.1 37bca31d9c7de06b67b2363770e065b4.cloudfront.net (CloudFront)" http.status_code=200 rpc.system=aws-api http.response.header.x_amzn_trace_id=Root=1-66b3cf3b-1a9174c53c20a3755f1a5d76 http.response.header.x_amz_cf_pop=AMS1-P1 http.response.header.access_control_allow_origin="*" http.response_content_length=374 http.response.header.x_amz_apigw_id=cJ1RTE7RPHcEG9w= http.response.header.x_amz_cf_id=XQkgGG-qld-hhOoKI82X0fGPZNDqigv4d7P5fJC-mjgF9qxyhnzy_w== http.response.header.date="Wed, 07 Aug 2024 19:47:07 GMT" tf_mux_provider="*schema.GRPCProviderServer" http.response.header.cache_control=no-store tf_req_id=dfe6148f-6856-6f06-cca1-97af6f81819e tf_rpc=ApplyResourceChange tf_aws.signing_region="" http.response.header.x_cache="Miss from cloudfront"
  http.response.body=
  | {"ApplicationId":"5b81d750ea1748eba0c4ca78a7a9a37c","IsArchived":false,"Version":1,"CreationDate":"2024-08-07T19:47:07.087Z","LastModifiedDate":"2024-08-07T19:47:07.087Z","Id":"email","Enabled":false,"Identity":"arn:aws:ses:us-west-2:012345678910:identity/bptw3i72.test","FromAddress":"tf-acc-test-7788522522147484357@bptw3i72.test","Platform":"EMAIL","MessagesPerSecond":1}
   aws.region=us-west-2

I filed a support case with AWS as the aws cli tooling also does not return the RoleARN value even though the documentation mentions it does.

https://docs.aws.amazon.com/cli/latest/reference/pinpoint/get-email-channel.html

They reproduced this behaviour and are internally investigating this behaviour, this ticket is purely for visibility purposes as it was first discovered in the Pinpoint SDK migration here. https://github.com/hashicorp/terraform-provider-aws/pull/38750

I do see some documentation that this argument has been deprecated.

https://docs.aws.amazon.com/pinpoint/latest/apireference/apps-application-id-channels-email.html

Regardless I will keep this issue updated.

Relevant Error/Panic Output Snippet

=== NAME  TestAccPinpointEmailChannel_set
    email_channel_test.go:75: Step 1/2 error: After applying this test step, the non-refresh plan was not empty.
        stdout:

        Terraform used the selected providers to generate the following execution
        plan. Resource actions are indicated with the following symbols:
          ~ update in-place

        Terraform will perform the following actions:

          # aws_pinpoint_email_channel.test will be updated in-place
          ~ resource "aws_pinpoint_email_channel" "test" {
                id                  = "549c1ac444df4a9d9905e2e442f2cdd2"
              + role_arn            = "arn:aws:iam::012345678910:role/terraform-2024080719365897590000000a"
                # (6 unchanged attributes hidden)
            }

        Plan: 0 to add, 1 to change, 0 to destroy.
--- FAIL: TestAccPinpointEmailChannel_set (32.17s)
=== NAME  TestAccPinpointEmailChannel_basic
    email_channel_test.go:31: Step 1/3 error: Check failed: Check 5/6 error: aws_pinpoint_email_channel.test: Attribute 'role_arn' expected "arn:aws:iam::012345678910:role/terraform-2024080719372352960000000f", got ""
--- FAIL: TestAccPinpointEmailChannel_basic (15.60s)

Terraform Configuration Files

See TestAccPinpointEmailChannel_basic and TestAccPinpointEmailChannel_set

Steps to Reproduce

Run make testacc TESTARGS='-run=TestAcc' PKG=pinpoint

Debug Output

No response

Panic Output

No response

Important Factoids

No response

References

No response

Would you like to implement a fix?

Yes

github-actions[bot] commented 3 months ago

Community Note

Voting for Prioritization

Volunteering to Work on This Issue

DanielRieske commented 3 months ago

I heard back from the internal team and this argument has indeed been deprecated. RoleARN is no longer needed and is set internally without having to explicitly configure it. They will update the documentation on their website and this will in time reflect in the SDK.

I will create a PR to remove this argument, which will introduce a breaking chance which in turn we should tag for a future v6.0.0 release.

DanielRieske commented 3 months ago

As the test fails have been solved in the other PR I would actually prefer it if we can tag this issue for the v6.0.0 milestone. I can make a PR but I have no idea when the actual release is and it would severly drift with main.

The actual change is so small that I don't think a PR is needed at this point and we already tagged it as deprecated in the documentation.