AWS SDK V1 Cleanup

[johnwesley]

Description

Please consider this a WIP

Started this change by removing github.com/aws/aws-sdk-go v1.44.28 from go.mod and running all of the tests. From there I followed the errors and removed references to go-sdk-v1 and continued to test until clean. I then proceeded to build successfully.

I have an AWS account and would be happy to test manually or possible run the integration tests, I have used this utility for a number of years.

My concern is I have let a good many packages auto update, possibly making this change out of scope?

Checklist

• [ ] Added tests that cover your change (if possible)
• [ ] Added/modified documentation as required (such as the README.md, or the userdocs directory)
• [x] Manually tested
• [x] Made sure the title of the PR is a good description that can go into the release notes
• [ ] (Core team) Added labels for change area (e.g. area/nodegroup) and kind (e.g. kind/improvement)

BONUS POINTS checklist: complete for good vibes and maybe prizes?! :exploding_head:

• [ ] Backfilled missing tests for code in same general area :tada:
• [ ] Refactored something and made the world a better place :star2:

[Himangini]

@johnwesley are you still working on this? let us know if you need any help or we can wrap this up 👍🏻

[johnwesley]

@Himangini Could someone review this for me? I made the updates and and all tests ran cleanly, and I was able to build successfully.

[Himangini]

Can you resolve the conflicts please, we'll get this reviewed right away ✨

Edit: Also it'll be awesome if you can share any output of your manual testing please 👍🏻

[johnwesley]

I've cleaned up the conflicts, but make check-all-generated-files-up-to-date wants to revert back to version 1 (see below), so I have missed some updates. I will have another pass.

diff --git a/pkg/eks/mocks/CloudFormationAPI.go b/pkg/eks/mocks/CloudFormationAPI.go
index 9c1cf248..c6410f04 100644
--- a/pkg/eks/mocks/CloudFormationAPI.go
+++ b/pkg/eks/mocks/CloudFormationAPI.go
@@ -3,7 +3,7 @@
 package mocks

 import (
-	cloudformation "github.com/aws/aws-sdk-go-v2/service/cloudformation"
+	cloudformation "github.com/aws/aws-sdk-go/service/cloudformation"

 	context "context"

diff --git a/pkg/eks/mocks/CloudTrailAPI.go b/pkg/eks/mocks/CloudTrailAPI.go
index 9546c645..c1365e7f 100644
--- a/pkg/eks/mocks/CloudTrailAPI.go
+++ b/pkg/eks/mocks/CloudTrailAPI.go
@@ -3,7 +3,7 @@
 package mocks

 import (
-	cloudtrail "github.com/aws/aws-sdk-go-v2/service/cloudtrail"
+	cloudtrail "github.com/aws/aws-sdk-go/service/cloudtrail"

 	context "context"

HINT: to fix this, run 'git commit pkg/apis/eksctl.io/v1alpha5/zz_generated.deepcopy.go pkg/eks/mocks/AutoScalingAPI.go pkg/eks/mocks/CloudFormationAPI.go pkg/eks/mocks/CloudTrailAPI.go pkg/eks/mocks/CloudWatchLogsAPI.go --message "Update generated files"'
make: *** [check-all-generated-files-up-to-date] Error 1

[Himangini]

I've cleaned up the conflicts, but make check-all-generated-files-up-to-date wants to revert back to version 1 (see below), so I have missed some updates. I will have another pass.

@johnwesley any luck so far, let us know if you need help. We do regular updates to our dependencies so you'd probably run into conflicts often.

[johnwesley]

Still receiving the same errors from make test @Himangini . I've updated eksctl/pkg/eks/mocks/mocks.go with the updated imports. I'm not familiar with mockery and will need to dig into it a little more.

[johnwesley]

There's also eksctl/pkg/eks/api.go vs eksctl/pkg/eks/apiv2.go. Is api.go scheduled for deprecation?

[Himangini]

Still receiving the same errors from make test @Himangini . I've updated eksctl/pkg/eks/mocks/mocks.go with the updated imports. I'm not familiar with mockery and will need to dig into it a little more.

Ok. Thanks for looking into this 👍🏻

[Himangini]

There's also eksctl/pkg/eks/api.go vs eksctl/pkg/eks/apiv2.go. Is api.go scheduled for deprecation?

Not sure, not atm AFAIK. I'll check and confirm.

[johnwesley]

Any word on deprecating eksctl/pkg/eks/api.go @Himangini?

[Himangini]

Any word on deprecating eksctl/pkg/eks/api.go @Himangini?

No, we aren't deprecating eksctl/pkg/eks/api.go as part of this work.

[TiberiuGC]

Hey @johnwesley , first of all, apologies for keeping your PR waiting for so long! Are you still willing to wrap up your work on this issue?

I've been looking through file changes and all the removals of v1 dependencies seems to have been wiped out for some reason. All that's left are those auto updates you mentioned in the description.

[johnwesley]

Hi @TiberiuGC. I would like to contribute 😄 but I wonder if the scope of this task is beyond one PR? If the change is to remove aws-sdk-v1 dependencies, then shouldn't that be broken into smaller PRs?

For example, wouldn't we need to update this import to V2 without disruption to the pkg's function, and should that be its own task?

[TiberiuGC]

@johnwesley I incline to agree with you that the scope of this ticket, as was initially phrased, seems to be to remove/update only those dependencies for which it is straightforward to do so. As I've been working through this myself, I realised that there's also plenty examples where doing so is not trivial and the amount of change can very well be too much for a single PR.

Some relevant examples are explained here.

We will re-discuss this issue internally over the course of next week and come back to you, stay tuned!

[TiberiuGC]

Closing this PR as we've decided this task needs to be first broken down into smaller, manageable in a single PR, chunks. e.g. for reference - https://github.com/weaveworks/eksctl/issues/5765