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

Migrate Pinpoint resources to AWS SDK V2 #38750

Merged
merged 19 commits into from
Aug 9, 2024

Conversation

DanielRieske
Copy link
Contributor

@DanielRieske DanielRieske commented Aug 7, 2024

Description

This PR migrates the Pinpoint resources to AWS SDKv2.

Test failures are pre-migration failures and are happening because the GetEmailChannel action doesn't seem to return roleARN in it's response body.

I will do a bit more research on where this exactly goes wrong and file if needed a support case with AWS.
For now this shouldn't block the migration.

Update: AWS support was able to reproduce this behaviour and will be consulting internally on how to proceed. I will create a seperate issue for visibility purposes and as to not block the progress here. Filed under #38772

UpdateEmailChannel

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"}

GetEmailChannel

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

Relations

Closes #36197
Relates #32976

References

Output from Acceptance Testing

% make testacc TESTARGS='-run=TestAcc' PKG=pinpoint
make: Verifying source code with gofmt...
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go1.22.5 test ./internal/service/pinpoint/... -v -count 1 -parallel 20  -run=TestAcc -timeout 360m
=== RUN   TestAccPinpointADMChannel_basic
    adm_channel_test.go:36: ADM_CLIENT_ID ENV is missing
--- SKIP: TestAccPinpointADMChannel_basic (0.00s)
=== RUN   TestAccPinpointAPNSChannel_basicCertificate
    apns_channel_test.go:65: Pinpoint certificate credentials envs are missing, skipping test
--- SKIP: TestAccPinpointAPNSChannel_basicCertificate (0.00s)
=== RUN   TestAccPinpointAPNSChannel_basicToken
    apns_channel_test.go:73: APNS_BUNDLE_ID env is missing, skipping test
--- SKIP: TestAccPinpointAPNSChannel_basicToken (0.00s)
=== RUN   TestAccPinpointAPNSSandboxChannel_basicCertificate
    apns_sandbox_channel_test.go:65: Pinpoint certificate credentials envs are missing, skipping test
--- SKIP: TestAccPinpointAPNSSandboxChannel_basicCertificate (0.00s)
=== RUN   TestAccPinpointAPNSSandboxChannel_basicToken
    apns_sandbox_channel_test.go:73: APNS_SANDBOX_BUNDLE_ID env is missing, skipping test
--- SKIP: TestAccPinpointAPNSSandboxChannel_basicToken (0.00s)
=== RUN   TestAccPinpointAPNSVoIPChannel_basicCertificate
    apns_voip_channel_test.go:65: Pinpoint certificate credentials envs are missing, skipping test
--- SKIP: TestAccPinpointAPNSVoIPChannel_basicCertificate (0.00s)
=== RUN   TestAccPinpointAPNSVoIPChannel_basicToken
    apns_voip_channel_test.go:73: APNS_VOIP_BUNDLE_ID env is missing, skipping test
--- SKIP: TestAccPinpointAPNSVoIPChannel_basicToken (0.00s)
=== RUN   TestAccPinpointAPNSVoIPSandboxChannel_basicCertificate
    apns_voip_sandbox_channel_test.go:65: Pinpoint certificate credentials envs are missing, skipping test
--- SKIP: TestAccPinpointAPNSVoIPSandboxChannel_basicCertificate (0.00s)
=== RUN   TestAccPinpointAPNSVoIPSandboxChannel_basicToken
    apns_voip_sandbox_channel_test.go:73: APNS_VOIP_BUNDLE_ID env is missing, skipping test
--- SKIP: TestAccPinpointAPNSVoIPSandboxChannel_basicToken (0.00s)
=== RUN   TestAccPinpointGCMChannel_basic
    gcm_channel_test.go:34: GCM_API_KEY env missing, skip test
--- SKIP: TestAccPinpointGCMChannel_basic (0.00s)
=== 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.
--- PASS: TestAccPinpointApp_nameGenerated (28.16s)
--- FAIL: TestAccPinpointEmailChannel_set (32.17s)
--- PASS: TestAccPinpointApp_namePrefix (35.44s)
--- PASS: TestAccPinpointApp_disappears (35.44s)
--- PASS: TestAccPinpointSMSChannel_disappears (36.70s)
--- PASS: TestAccPinpointEmailChannel_disappears (36.99s)
=== 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 ""
--- PASS: TestAccPinpointEmailChannel_noRole (40.87s)
--- PASS: TestAccPinpointApp_quietTimeEmpty (41.41s)
--- PASS: TestAccPinpointApp_basic (41.41s)
--- PASS: TestAccPinpointApp_campaignHookEmpty (41.54s)
--- PASS: TestAccPinpointApp_limitsEmpty (41.57s)
--- PASS: TestAccPinpointApp_quietTime (41.60s)
--- PASS: TestAccPinpointApp_limits (41.82s)
--- FAIL: TestAccPinpointEmailChannel_basic (15.60s)
--- PASS: TestAccPinpointSMSChannel_basic (50.14s)
--- PASS: TestAccPinpointBaiduChannel_basic (51.96s)
--- PASS: TestAccPinpointSMSChannel_full (52.75s)
--- PASS: TestAccPinpointApp_campaignHookLambda (52.84s)
--- PASS: TestAccPinpointApp_tags (60.67s)
--- PASS: TestAccPinpointEventStream_disappears (64.08s)
--- PASS: TestAccPinpointEventStream_basic (116.44s)
FAIL
FAIL	github.com/hashicorp/terraform-provider-aws/internal/service/pinpoint	121.082s
FAIL
...

@DanielRieske DanielRieske requested a review from a team as a code owner August 7, 2024 19:14
Copy link

github-actions bot commented Aug 7, 2024

Community Note

Voting for Prioritization

  • Please vote on this pull request by adding a 👍 reaction to the original post to help the community and maintainers prioritize this pull request.
  • Please see our prioritization guide for information on how we prioritize.
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request.

For Submitters

  • Review the contribution guide relating to the type of change you are making to ensure all of the necessary steps have been taken.
  • For new resources and data sources, use skaff to generate scaffolding with comments detailing common expectations.
  • Whether or not the branch has been rebased will not impact prioritization, but doing so is always a welcome surprise.

Copy link

github-actions bot commented Aug 7, 2024

Thank you for your contribution! 🚀

Please note that typically Go dependency changes are handled in this repository by dependabot or the maintainers. This is to prevent pull request merge conflicts and further delay reviews of contributions. Remove any changes to the go.mod or go.sum files and commit them into this pull request.

Additional details:

  • Check open pull requests with the dependencies label to view other dependency updates.
  • If this pull request includes an update the AWS Go SDK (or any other dependency) version, only updates submitted via dependabot will be merged. This pull request will need to remove these changes and will need to be rebased after the existing dependency update via dependabot has been merged for this pull request to be reviewed.
  • If this pull request is for supporting a new AWS service:
    • Ensure the new AWS service changes are following the Contributing Guide section on new services, in particular that the dependency addition and initial provider support are in a separate pull request from other changes (e.g. new resources). Contributions not following this item will not be reviewed until the changes are split.
    • If this pull request is already a separate pull request from the above item, you can ignore this message.

@github-actions github-actions bot added tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. service/pinpoint Issues and PRs that pertain to the pinpoint service. sweeper Pertains to changes to or issues with the sweeper. tags Pertains to resource tagging. client-connections Pertains to the AWS Client and service connections. generators Relates to code generators. needs-triage Waiting for first response or review from a maintainer. external-maintainer Contribution from a trusted external contributor. labels Aug 7, 2024
@rocketnova
Copy link

Test failures are pre-migration failures and are happening because the GetEmailChannel action doesn't seem to return roleARN in it's response body.
I will do a bit more research on where this exactly goes wrong and file if needed a support case with AWS.
For now this shouldn't block the migration.

@DanielRieske I was in the middle of troubleshooting my terraform module when I came across #36197 and your linked PR. I wanted to share what I'm seeing in case it is helpful for your research, but I completely understand that resolving the issue is likely outside the scope of your PR.

Here's what is happening for me: When I run terraform apply on my pinpoint module, terraform plan always shows that the aws_pinpoint_email_channel resource is missing the role_arn attribute and needs to be updated. When I run aws pinpoint get-email-channel, the EmailChannelResponse object does not include a RoleArn attribute. When I use aws pinpoint update-email-channel and I include a RoleArn in the --email-channel-request option, the EmailChannelResponse also does not include the RoleArn.

There might be a hint on the AWS Pinpoint REST API Reference. There's a notation for the EmailChannelRequest property that lists RoleArn as "(Depricated)", though this is not reflected in the AWS CLI reference for Pinpoint.

@rocketnova
Copy link

@DanielRieske I see that you've already reached out to AWS and filed #38772. Thank you!

@DanielRieske
Copy link
Contributor Author

Hi @rocketnova 👋, thank you for reaching out!

This has been exactly my observation too, my best guestimation is that AWS is in the process of deprecating this argument.
But I suppose the internal team are the best people to answer this.

I will keep the issue updated with the progress regarding this.

Thank you for investigating!

@ewbankkit ewbankkit added aws-sdk-go-migration Issues that are related to the providers migration to AWS SDK for Go v2. and removed needs-triage Waiting for first response or review from a maintainer. labels Aug 8, 2024
@nam054 nam054 self-assigned this Aug 8, 2024
@github-actions github-actions bot added the prioritized Part of the maintainer teams immediate focus. To be addressed within the current quarter. label Aug 8, 2024
Copy link
Contributor

@nam054 nam054 left a comment

Choose a reason for hiding this comment

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

LGTM! 🚀

> make testacc TESTARGS='-run=TestAcc' PKG=pinpoint
make: Verifying source code with gofmt...
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go1.22.5 test ./internal/service/pinpoint/... -v -count 1 -parallel 20  -run=TestAcc -timeout 360m
=== RUN   TestAccPinpointADMChannel_basic
    adm_channel_test.go:36: ADM_CLIENT_ID ENV is missing
--- SKIP: TestAccPinpointADMChannel_basic (0.00s)
=== RUN   TestAccPinpointAPNSChannel_basicCertificate
    apns_channel_test.go:65: Pinpoint certificate credentials envs are missing, skipping test
--- SKIP: TestAccPinpointAPNSChannel_basicCertificate (0.00s)
=== RUN   TestAccPinpointAPNSChannel_basicToken
    apns_channel_test.go:73: APNS_BUNDLE_ID env is missing, skipping test
--- SKIP: TestAccPinpointAPNSChannel_basicToken (0.00s)
=== RUN   TestAccPinpointAPNSSandboxChannel_basicCertificate
    apns_sandbox_channel_test.go:65: Pinpoint certificate credentials envs are missing, skipping test
--- SKIP: TestAccPinpointAPNSSandboxChannel_basicCertificate (0.00s)
=== RUN   TestAccPinpointAPNSSandboxChannel_basicToken
    apns_sandbox_channel_test.go:73: APNS_SANDBOX_BUNDLE_ID env is missing, skipping test
--- SKIP: TestAccPinpointAPNSSandboxChannel_basicToken (0.00s)
=== RUN   TestAccPinpointAPNSVoIPChannel_basicCertificate
    apns_voip_channel_test.go:65: Pinpoint certificate credentials envs are missing, skipping test
--- SKIP: TestAccPinpointAPNSVoIPChannel_basicCertificate (0.00s)
=== RUN   TestAccPinpointAPNSVoIPChannel_basicToken
    apns_voip_channel_test.go:73: APNS_VOIP_BUNDLE_ID env is missing, skipping test
--- SKIP: TestAccPinpointAPNSVoIPChannel_basicToken (0.00s)
=== RUN   TestAccPinpointAPNSVoIPSandboxChannel_basicCertificate
    apns_voip_sandbox_channel_test.go:65: Pinpoint certificate credentials envs are missing, skipping test
--- SKIP: TestAccPinpointAPNSVoIPSandboxChannel_basicCertificate (0.00s)
=== RUN   TestAccPinpointAPNSVoIPSandboxChannel_basicToken
    apns_voip_sandbox_channel_test.go:73: APNS_VOIP_BUNDLE_ID env is missing, skipping test
--- SKIP: TestAccPinpointAPNSVoIPSandboxChannel_basicToken (0.00s)
=== RUN   TestAccPinpointApp_basic
=== PAUSE TestAccPinpointApp_basic
=== RUN   TestAccPinpointApp_disappears
=== PAUSE TestAccPinpointApp_disappears
=== RUN   TestAccPinpointApp_nameGenerated
=== PAUSE TestAccPinpointApp_nameGenerated
=== RUN   TestAccPinpointApp_namePrefix
=== PAUSE TestAccPinpointApp_namePrefix
=== RUN   TestAccPinpointApp_tags
=== PAUSE TestAccPinpointApp_tags
=== RUN   TestAccPinpointApp_campaignHookLambda
=== PAUSE TestAccPinpointApp_campaignHookLambda
=== RUN   TestAccPinpointApp_campaignHookEmpty
=== PAUSE TestAccPinpointApp_campaignHookEmpty
=== RUN   TestAccPinpointApp_limits
=== PAUSE TestAccPinpointApp_limits
=== RUN   TestAccPinpointApp_limitsEmpty
=== PAUSE TestAccPinpointApp_limitsEmpty
=== RUN   TestAccPinpointApp_quietTime
=== PAUSE TestAccPinpointApp_quietTime
=== RUN   TestAccPinpointApp_quietTimeEmpty
=== PAUSE TestAccPinpointApp_quietTimeEmpty
=== RUN   TestAccPinpointBaiduChannel_basic
=== PAUSE TestAccPinpointBaiduChannel_basic
=== RUN   TestAccPinpointEmailChannel_basic
=== PAUSE TestAccPinpointEmailChannel_basic
=== RUN   TestAccPinpointEmailChannel_set
=== PAUSE TestAccPinpointEmailChannel_set
=== RUN   TestAccPinpointEmailChannel_noRole
=== PAUSE TestAccPinpointEmailChannel_noRole
=== RUN   TestAccPinpointEmailChannel_disappears
=== PAUSE TestAccPinpointEmailChannel_disappears
=== RUN   TestAccPinpointEventStream_basic
=== PAUSE TestAccPinpointEventStream_basic
=== RUN   TestAccPinpointEventStream_disappears
=== PAUSE TestAccPinpointEventStream_disappears
=== RUN   TestAccPinpointGCMChannel_basic
    gcm_channel_test.go:34: GCM_API_KEY env missing, skip test
--- SKIP: TestAccPinpointGCMChannel_basic (0.00s)
=== RUN   TestAccPinpointSMSChannel_basic
=== PAUSE TestAccPinpointSMSChannel_basic
=== RUN   TestAccPinpointSMSChannel_full
=== PAUSE TestAccPinpointSMSChannel_full
=== RUN   TestAccPinpointSMSChannel_disappears
=== PAUSE TestAccPinpointSMSChannel_disappears
=== CONT  TestAccPinpointApp_basic
=== CONT  TestAccPinpointBaiduChannel_basic
=== CONT  TestAccPinpointApp_campaignHookEmpty
=== CONT  TestAccPinpointEventStream_basic
=== CONT  TestAccPinpointApp_namePrefix
=== CONT  TestAccPinpointEmailChannel_disappears
=== CONT  TestAccPinpointApp_quietTime
=== CONT  TestAccPinpointApp_limitsEmpty
=== CONT  TestAccPinpointEmailChannel_set
=== CONT  TestAccPinpointApp_quietTimeEmpty
=== CONT  TestAccPinpointSMSChannel_disappears
=== CONT  TestAccPinpointApp_campaignHookLambda
=== CONT  TestAccPinpointEmailChannel_noRole
=== CONT  TestAccPinpointApp_limits
=== CONT  TestAccPinpointSMSChannel_basic
=== CONT  TestAccPinpointSMSChannel_full
=== CONT  TestAccPinpointEmailChannel_basic
=== CONT  TestAccPinpointEventStream_disappears
=== CONT  TestAccPinpointApp_tags
=== CONT  TestAccPinpointApp_disappears
--- PASS: TestAccPinpointApp_disappears (43.07s)
=== CONT  TestAccPinpointApp_nameGenerated
--- PASS: TestAccPinpointApp_namePrefix (43.54s)
--- PASS: TestAccPinpointEmailChannel_disappears (45.59s)
--- PASS: TestAccPinpointSMSChannel_disappears (45.59s)
--- PASS: TestAccPinpointApp_quietTimeEmpty (50.89s)
--- PASS: TestAccPinpointApp_limitsEmpty (51.02s)
--- PASS: TestAccPinpointApp_campaignHookEmpty (51.54s)
--- PASS: TestAccPinpointEmailChannel_noRole (51.71s)
--- PASS: TestAccPinpointApp_limits (51.75s)
--- PASS: TestAccPinpointApp_quietTime (52.04s)
--- PASS: TestAccPinpointApp_basic (52.10s)
--- PASS: TestAccPinpointEmailChannel_set (53.03s)
--- PASS: TestAccPinpointApp_campaignHookLambda (61.43s)
--- PASS: TestAccPinpointSMSChannel_basic (63.26s)
--- PASS: TestAccPinpointSMSChannel_full (65.61s)
--- PASS: TestAccPinpointBaiduChannel_basic (65.62s)
--- PASS: TestAccPinpointApp_nameGenerated (22.57s)
--- PASS: TestAccPinpointEmailChannel_basic (66.20s)
--- PASS: TestAccPinpointEventStream_disappears (71.27s)
--- PASS: TestAccPinpointApp_tags (72.98s)
--- PASS: TestAccPinpointEventStream_basic (121.20s)
PASS
ok      github.com/hashicorp/terraform-provider-aws/internal/service/pinpoint   127.103s

@nam054
Copy link
Contributor

nam054 commented Aug 9, 2024

Thank you @DanielRieske for your contribution and for the bug report/investigation! For the purposes of this PR, I've removed the role_arn in the test check and added documentation about the deprecation of the attribute. I'll also take a further look into the issue. Thank you again!

@nam054 nam054 merged commit ff74485 into hashicorp:main Aug 9, 2024
48 checks passed
@github-actions github-actions bot added this to the v5.63.0 milestone Aug 9, 2024
@DanielRieske
Copy link
Contributor Author

Hi @nam054 thank you for reviewing this PR! - I do want to add that the deprecation is purely a guess right now as a single piece of documentation seems to suggest this. I will keep the open case with their engineering team updated in the other issue.

@github-actions github-actions bot removed the prioritized Part of the maintainer teams immediate focus. To be addressed within the current quarter. label Aug 15, 2024
Copy link

This functionality has been released in v5.63.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. Thank you!

Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
aws-sdk-go-migration Issues that are related to the providers migration to AWS SDK for Go v2. client-connections Pertains to the AWS Client and service connections. external-maintainer Contribution from a trusted external contributor. generators Relates to code generators. service/pinpoint Issues and PRs that pertain to the pinpoint service. sweeper Pertains to changes to or issues with the sweeper. tags Pertains to resource tagging. 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.

[AWS SDK for Go Migration] Pinpoint Service
4 participants