BCDA-9484: Migrate aws sdk to v2 by carlpartridge · Pull Request #1235 · CMSgov/bcda-app

carlpartridge · 2025-10-28T16:43:34Z

🎫 Ticket

https://jira.cms.gov/browse/BCDA-9484

🛠 Changes

Migrate all AWS packages to v2. Various refactoring and cleanup.

ℹ️ Context

AWS v1 are out of service as of end of July 2025.

🧪 Validation

Local linting and testing.

carlpartridge · 2025-10-28T16:44:15Z

bcda/aws/metrics.go

@@ -0,0 +1,49 @@
+package bcdaaws


Moved this from its own package to under our aws helpers package.

carlpartridge · 2025-10-28T16:45:57Z

bcda/aws/session.go

AWS session is no longer a thing in v2. Instead each service creates a client for itself using AWS config.

bcda/bcdacli/cli.go

carlpartridge · 2025-10-28T16:48:55Z

bcda/bcdacli/cli_test.go

-	assert.Contains(buf.String(), "Files failed: 1")
-	assert.Contains(buf.String(), "Files skipped: 0")
-}
+// func (s *CLITestSuite) TestImportCCLFDirectory() {


Tests related to the commented out cli actions.

carlpartridge · 2025-10-28T16:51:41Z

bcda/cclf/cclf.go

 type CclfFileProcessor interface {
 	// Load a list of valid CCLF files to be imported
-	LoadCclfFiles(path string) (cclfList map[string][]*cclfZipMetadata, skipped int, failed int, err error)
+	LoadCclfFiles(ctx context.Context, path string) (cclfList map[string][]*cclfZipMetadata, skipped int, failed int, err error)


A good amount of changes are just adding ctx to functions. Ctx is now needed for basically kind of AWS interaction.

carlpartridge · 2025-10-28T17:06:35Z

bcda/database/config.go

-				return nil, err
-			}
-		}
-	}


This whole section seemed to say if DATABASE_URL is not set, then go grab it from param store. This is just as easily made sure to be set elsewhere in our pipelines so I think its ok to clean this up a bit.

carlpartridge · 2025-10-28T17:08:33Z

bcda/insights/insights.go

I am 90% sure insights data is now handled via the js lambda in bcda-ops: https://github.com/CMSgov/bcda-ops/tree/main/terraform/modules/insights_data_sampler_lambda

last update was 4 years ago, so LGTM to remove!

carlpartridge · 2025-10-28T17:15:45Z

bcda/lambda/cclf/main.go

-	} else {
-		lambda.Start(attributionImportHandler)
-	}
+	lambda.Start(attributionImportHandler)


Dont need these localstack checks anymore. It now just works (assuming you set your test AWS clients up properly).

I see the previous comment:

// should only be set in local development to avoid making external calls to a real AWS account. - is there a risk of calling our AWS account inadvertently?

As long as AWS_ENDPOINT_URL is set when anything is run, then anything that runs any kind of aws service will get redirected to that url (generally http://localstack:4566). Localstack does a lot of magic to mimic AWS services APIs.

carlpartridge · 2025-10-28T17:17:00Z

bcda/lambda/cclf/main.go

 			logger.Infof("Reading %s event for file %s", e.EventName, filepath)
 			if cclf.CheckIfAttributionCSVFile(e.S3.Object.Key) {
-				return handleCSVImport(db, s3AssumeRoleArn, filepath)
+				return handleCSVImport(ctx, pool, s3Client, ssmClient, filepath)


Removed some of the logic above in order to dep inject them into functions. This allows us more flexibility in testing.

carlpartridge · 2025-10-28T17:19:37Z

bcda/metrics/metrics.go

Moved to bcda/aws package

carlpartridge · 2025-10-28T17:23:35Z

bcda/testUtils/utils.go

This whole file is mostly helpers for AWS but also specifically for only running tests. It seems kind of useful but also feels a bit wrong... Wondering if anyone else has feelings about it. Also if it should maybe get moved under bcda/aws?

carlpartridge · 2025-10-28T17:24:26Z

bcdaworker/queueing/river.go

 	}
 }

-// TODO: once we remove que library and upgrade to pgx5 we can move the below functions into manager


Followed this TODO and moved it under manager.

carlpartridge · 2025-10-28T17:26:07Z

optout/go.mod

I wend ahead and removed this (and related files). This was originally part of https://jira.cms.gov/browse/BCDA-9491. It was quick and easy and I was in here already so just went ahead and did it.

Makefile

bcda/bcdacli/cli.go

bcda/cclf/cclf_test.go

bcda/cclf/csv_test.go

laurenkrugen-navapbc

Only other comment is a few other places could initialize ctx in the func call, but other than that, LGTM! Thanks for getting this updated and cleaned up! 🎉

michaeljvaldes

Great job! That's a lot of additional work removing/refactoring outdated packages. I also think this design around interacting with AWS/localstack aligns much more with the intent of localstack.

carlpartridge added 6 commits October 21, 2025 13:42

BCDA-9484: Stubbing out aws-sdk v2 migration

1345cee

Mid work checkin

83a1229

Mid work checkin

fef5cf4

Params tests working

0253fe3

Up aws versions, aco creds tests passing

15a4fed

Tests passing

c60c784

carlpartridge commented Oct 28, 2025

View reviewed changes

bcda/bcdacli/cli.go Outdated Show resolved Hide resolved

carlpartridge commented Oct 28, 2025

View reviewed changes

bcda/metrics/metrics.go

Copy link

Collaborator Author

carlpartridge Oct 28, 2025

Choose a reason for hiding this comment

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

Moved to bcda/aws package

carlpartridge commented Oct 28, 2025

View reviewed changes

Cleanup

32a4145

carlpartridge marked this pull request as ready for review October 28, 2025 17:28

carlpartridge requested a review from a team as a code owner October 28, 2025 17:28

Merge branch 'main' into carl-9484-migrate-aws-to-v2

b795439

carlpartridge temporarily deployed to test October 28, 2025 17:32 — with GitHub Actions Inactive

Cleanup

194520a

carlpartridge temporarily deployed to test October 28, 2025 18:14 — with GitHub Actions Inactive

carlpartridge had a problem deploying to test October 28, 2025 18:14 — with GitHub Actions Error

Fix tests

5411a8f

carlpartridge temporarily deployed to test October 28, 2025 19:29 — with GitHub Actions Inactive

carlpartridge temporarily deployed to test October 29, 2025 16:29 — with GitHub Actions Inactive

laurenkrugen-navapbc reviewed Oct 29, 2025

View reviewed changes

Makefile Outdated Show resolved Hide resolved

laurenkrugen-navapbc reviewed Oct 29, 2025

View reviewed changes

bcda/bcdacli/cli.go Outdated Show resolved Hide resolved

laurenkrugen-navapbc reviewed Oct 29, 2025

View reviewed changes

bcda/cclf/cclf_test.go Outdated Show resolved Hide resolved

laurenkrugen-navapbc reviewed Oct 29, 2025

View reviewed changes

bcda/cclf/csv_test.go Outdated Show resolved Hide resolved

Add various missing test coverage

4f17c08

carlpartridge temporarily deployed to test October 29, 2025 19:39 — with GitHub Actions Inactive

Fix aco creds tests, PR review changes

4c5f5ad

carlpartridge temporarily deployed to test October 29, 2025 23:05 — with GitHub Actions Inactive

Merge branch 'main' into carl-9484-migrate-aws-to-v2

55525ab

carlpartridge temporarily deployed to test October 30, 2025 14:17 — with GitHub Actions Inactive

carlpartridge requested review from bhagatparwinder and michaeljvaldes October 30, 2025 15:09

laurenkrugen-navapbc previously approved these changes Oct 30, 2025

View reviewed changes

Add back in still used cli script

31da1d4

carlpartridge dismissed laurenkrugen-navapbc’s stale review via 31da1d4 October 30, 2025 20:59

Add back in still used cli script

f45bbf0

carlpartridge temporarily deployed to test October 30, 2025 21:01 — with GitHub Actions Inactive

Remove commented out code

d8e5d19

carlpartridge temporarily deployed to test October 30, 2025 21:07 — with GitHub Actions Inactive

carlpartridge temporarily deployed to test October 30, 2025 21:08 — with GitHub Actions Inactive

michaeljvaldes approved these changes Oct 31, 2025

View reviewed changes

laurenkrugen-navapbc approved these changes Nov 3, 2025

View reviewed changes

carlpartridge merged commit 9acc861 into main Nov 3, 2025
15 of 16 checks passed

carlpartridge deleted the carl-9484-migrate-aws-to-v2 branch November 3, 2025 22:06

Conversation

carlpartridge commented Oct 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🎫 Ticket

🛠 Changes

ℹ️ Context

🧪 Validation

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

carlpartridge Oct 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

laurenkrugen-navapbc left a comment

Choose a reason for hiding this comment

Uh oh!

michaeljvaldes left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

carlpartridge commented Oct 28, 2025 •

edited

Loading

carlpartridge Oct 28, 2025 •

edited

Loading