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

AWS SDK migration from v1 to v2 #262

Merged
merged 6 commits into from
Jan 29, 2025
Merged

Conversation

mohanmanikanta2299
Copy link
Contributor

What has changed?

  • Migrated AWS SDK from v1 to v2.
  • Dependency bumps to remediate CVEs.
  • ioutil is deprecated and hence the dependency is handled.

@mohanmanikanta2299 mohanmanikanta2299 requested a review from a team as a code owner January 22, 2025 07:42
@mohanmanikanta2299
Copy link
Contributor Author

image

Able to perform terraform tests from my local system using my AWS Sandbox. Attached the screenshot for the same.
The AWS Credentials seems to be outdated in the github secrets as a result of which the workflows are failing.

Copy link

@srahul3 srahul3 left a comment

Choose a reason for hiding this comment

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

✅ LGTM, except few trivial comments. Thanks for looking into this 🚀

svc := ecs.New(session.New(), &config)
svc := ecs.NewFromConfig(cfg, func(o *ecs.Options) {
if endpoint != "" {
o.BaseEndpoint = aws.String(endpoint)
Copy link

Choose a reason for hiding this comment

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

A high level observation, the logging is degraded.

Suggested change
o.BaseEndpoint = aws.String(endpoint)
o.BaseEndpoint = aws.String(endpoint)
l.Printf("[INFO] discover-aws: Endpoint is %s", endpoint)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated the log statements.

Copy link
Contributor

@mukeshjc mukeshjc left a comment

Choose a reason for hiding this comment

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

LGTM

@mohanmanikanta2299 mohanmanikanta2299 removed the request for review from srahul3 January 29, 2025 07:01
@mohanmanikanta2299 mohanmanikanta2299 merged commit 0302a33 into master Jan 29, 2025
8 of 10 checks passed
@srahul3
Copy link

srahul3 commented Feb 6, 2025

🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants