Repository navigation
Migrate AWS provider from aws-sdk-go v1 to aws-sdk-go-v2 - #861
Conversation
Replace the deprecated github.com/aws/aws-sdk-go with aws-sdk-go-v2 across the EC2 provider code. - Client *ec2.EC2 -> *ec2.Client, built via config.LoadDefaultConfig + ec2.NewFromConfig instead of session.NewSessionWithOptions - Model types moved to ec2/types; pointer slices -> value slices - All API calls now take context.TODO() - Error handling via smithy.APIError + errors.As (printAwsError helper) - WaitUntilInstanceRunning -> ec2.NewInstanceRunningWaiter().Wait() - aws.Int64 -> aws.Int32; enum fields (InstanceType, ResourceType, State.Name) now typed Build, go vet, tests, and golangci-lint all pass.
The shipped init config left aws.vm.image unset, and users who pinned an exact dated Ubuntu AMI (e.g. ...-server-20250918) hit a panic once AWS deregistered it, since GetImages returned zero results and Deploy indexed images[0]. Default to the wildcard pattern so GetImages resolves the latest jammy 22.04 image (it sorts wildcard matches newest-first).
![chatgpt-codex-connector[bot]](/p/https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 26149d7f8d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| waiter := ec2.NewInstanceRunningWaiter(p.Client) | ||
| err = waiter.Wait(context.TODO(), &ec2.DescribeInstancesInput{ | ||
| InstanceIds: []string{*result.Instances[0].InstanceId}, | ||
| }, 5*time.Minute) |
There was a problem hiding this comment.
When AWS takes more than 5 minutes to move a newly launched instance to running (for example, slow AMI boot or transient EC2 capacity delays), Deploy now calls log.Fatalln even though the instance may still become healthy; the v1 waiter this replaced used the standard EC2 waiter window of roughly 10 minutes (15s × 40 attempts). This can make onctl up report failure while leaving a real instance behind, so the migration should keep the previous timeout or make it configurable.
Useful? React with 👍 / 👎.
The v2 migration set the InstanceRunningWaiter max wait to 5m, but v1's WaitUntilInstanceRunning defaulted to ~10m (15s x 40 attempts). A shorter window can make 'onctl up' report failure (log.Fatalln) on slow AMI boots or transient EC2 capacity delays while leaving a real instance behind. Restore the previous ~10m window.
…igration # Conflicts: # go.mod # go.sum
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #861 +/- ##
==========================================
+ Coverage 14.77% 15.22% +0.44%
==========================================
Files 36 36
Lines 3127 3022 -105
==========================================
- Hits 462 460 -2
+ Misses 2614 2510 -104
- Partials 51 52 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Much of onctl is thin cloud-SDK glue (provider wrappers) validated via end-to-end runs rather than unit tests. A blocking patch-coverage gate penalizes faithful refactors/migrations even when overall (project) coverage holds or improves. Keep project as the gate; patch reports only.
Summary
Migrates the AWS/EC2 provider code from the deprecated
github.com/aws/aws-sdk-go(v1) togithub.com/aws/aws-sdk-go-v2, removing the//nolint:staticchecksuppressions and the outstanding "migrate to AWS SDK v2" TODOs.Changes
*ec2.EC2→*ec2.Client, constructed viaconfig.LoadDefaultConfig+ec2.NewFromConfiginstead ofsession.NewSessionWithOptionsec2/types(types.Filter,types.Tag,types.Instance,types.Image,types.Subnet, …); pointer slices → value slices ([]*string→[]string,[]*ec2.Filter→[]types.Filter)context.TODO()(matches the existinghetzner.goconvention)awserr.Error→smithy.APIErrorviaerrors.As(extracted a smallprintAwsErrorhelper)WaitUntilInstanceRunning→ec2.NewInstanceRunningWaiter().Wait(ctx, …, 5*time.Minute)aws.Int64→aws.Int32; enum fields (InstanceType,ResourceType,State.Name) now typedinternal/files/init/aws.yaml) now defaultsaws.vm.imageto the wildcardubuntu/images/hvm-ssd/ubuntu-jammy-22.04-amd64-server-*soGetImagesresolves the latest jammy AMI instead of a dated name that expires when AWS deregisters itVerification
make build,go vet ./...,go test ./...— all passgolangci-lint run ./...— 0 issueseu-central-1):onctl up -n adlaunched at2.micro, reached running state via the v2 waiter, andonctl lslisted it correctly; key import exercised the newsmithyerror handling (InvalidKeyPair.Duplicate). Test instance was destroyed afterward.Notes
internal/cloud/aws.goif the old ceiling is preferred.