Upgrade to aws-sdk-go-v2

[michaelbeaumont]

V2 of the aws-sdk-go reached GA on 19.01.21 and it looks like they've simplified the client, gotten rid of excessive pointer usage, cleaned up the package organization and improved error types. v1 isn't scheduled for deprecation yet however but it seems like a worthwhile migration to make.

Summary

• It's a nice time to move to V2 since we don't have major features atm
• Testing might be complicated
• No breaking changes, only changes will be to accommodate v2
• First do a Spike to find out what's involved and how much effort is involved when we move to a new version
• Time box : 1-2 days

[saada]

Looked into this and here are a few of the blockers to achieving this:

• [ ] https://github.com/aws/aws-sdk-go-v2/issues/786
• [ ] https://github.com/aws/aws-sdk-go-v2/issues/1114

[michaelbeaumont]

Sounds like for aws/aws-sdk-go-v2#786 we'll "just" need to manually write out the interfaces we use.

[saada]

Discussed with @richardcase and he mentioned using something like https://github.com/vburenin/ifacemaker generate the interfaces for us

[richardcase]

(or something similar to ifacemaker)

[github-actions[bot]]

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

[saada]

/remove-stale

[github-actions[bot]]

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

[github-actions[bot]]

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

[github-actions[bot]]

This issue was closed because it has been stalled for 5 days with no activity.

[aclevername]

we are looking into this again, trying to find out what the long/short term priority is

[github-actions[bot]]

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

[github-actions[bot]]

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

[aclevername]

I've had a playaround with migrating 1 small part of the codebase (eksctl create addon) to use the aws-sdk-go-v2. Please take a look at the diff to gain an understanding of the worked involved https://github.com/weaveworks/eksctl/pull/4820/files

Summary

Moving to aws-sdk-go-v2 is definitely viable, but will require a lot of small code changes

The main takeaways:

• The func names are all the same
• All the funcs now take a context.Context as the first argument
• They all take a variadic list of opts optFns ...func(*Options)
• Some types have changed, and are either:
  • no longer a pointer (yay!). Example:

              -----------------before------------------
    Expect(*createAddonInput.Tags["foo"]).To(Equal("bar"))
        Expect(*createAddonInput.Tags["fox"]).To(Equal("brown"))
        ------------------after------------------
        Expect(createAddonInput.Tags["foo"]).To(Equal("bar"))
        Expect(createAddonInput.Tags["fox"]).To(Equal("brown"))

  • Wrapped in a sub-type (boooo!)

              -----------------before------------------
    Status:    aws.String("ACTIVE"),
    ------------------after------------------
            Status:    types.AddonStatus("ACTIVE"),

    where AddonStatus is type AddonStatus string

these small type changes all over the code base will mean this migration will incur a large code change (while of low complexity its very frustrating/boring)

Fakes & Interfaces

v1 of the library published the interfaces for each service, e.g. the eksiface. They did not do this for v2. This means we have to generate the interfaces ourselves. ifacemaker is a handy tool for doing that, but doesn't work extremely well with remote packages (see issue). A simple enough workaround is doable thanks to a handy sed, as done in the demo branch here. Once the interfaces are setup we can continue generating our mocks like we do now, example on demo branch here.

Recommended steps

The demo branch shows that its relatively simple, while quite tedious to do this migration. I recommend we try splitting this in parts rather than 1 big-bang PR. Some of the APIs, like eks are used all over the codebase and will involve a lot of work to move, other like elb or asg are rarely used and would be a smaller piece of work. We use the following APIs from aws-sdk-go v1:

// CloudFormation returns a representation of the CloudFormation API
func (p ProviderServices) CloudFormation() cloudformationiface.CloudFormationAPI { return p.cfn }

// ASG returns a representation of the AutoScaling API
func (p ProviderServices) ASG() autoscalingiface.AutoScalingAPI { return p.asg }

// EKS returns a representation of the EKS API
func (p ProviderServices) EKS() eksiface.EKSAPI { return p.eks }

// EC2 returns a representation of the EC2 API
func (p ProviderServices) EC2() ec2iface.EC2API { return p.ec2 }

// ELB returns a representation of the ELB API
func (p ProviderServices) ELB() elbiface.ELBAPI { return p.elb }

// ELBV2 returns a representation of the ELBV2 API
func (p ProviderServices) ELBV2() elbv2iface.ELBV2API { return p.elbv2 }

// STS returns a representation of the STS API
func (p ProviderServices) STS() stsiface.STSAPI { return p.sts }

// SSM returns a representation of the STS API
func (p ProviderServices) SSM() ssmiface.SSMAPI { return p.ssm }

// IAM returns a representation of the IAM API
func (p ProviderServices) IAM() iamiface.IAMAPI { return p.iam }

// CloudTrail returns a representation of the CloudTrail API
func (p ProviderServices) CloudTrail() cloudtrailiface.CloudTrailAPI { return p.cloudtrail }

// CloudWatchLogs returns a representation of the CloudWatchLogs API.
func (p ProviderServices) CloudWatchLogs() cloudwatchlogsiface.CloudWatchLogsAPI { return p.cloudwatchlogs }

Rough estimates for the usages (using rg "STS\(\)" --no-filename | wc -l) :

• EKS 222
• EC2 113
• CloudFormation 61
• IAM 56
• SSM 40
• ASG 10
• STS 9
• CloudTrail 6
• CloudWatchLogs 6
• ELB 4
• ELBV2 4

Note that each usage of the API listed above might/will involve a lot of additional lines of test code that needs to be changed.

I propose we split the work into 8 parts:

1. Add to ClusterProvider the setting up of a v2 aws session (mirroring the configuration we use for v1) and add a STSV2 to the ClusterProvider. This should allow us to use both v1 and v2 sessions side-by-side while we migrate.
2. Remove usage of v1 api for ELB , ELBV2 and and replace it with v2
3. Remove usage of v1 api for CloudWatchLogs, CloudTrail and ASG and replace it with v2
4. Remove usage of v1 api for SSM and replace it with v2
5. Remove usage of v1 api for IAM and replace it with v2
6. Remove usage of v1 api for CloudFormation and replace it with v2
7. Remove usage of v1 api for EC2 and replace it with v2
8. Remove usage of v1 api for EKS and replace it with v2. Consider doing in multiple PRs (where EKS and EKSV2 can live along-side eachother)
9. Remove the usage v1 STS and replace it with the existing STSV2, and rename STSV2 to STS. Remove the setting up of all of the v1 sessions/configs. No reference to aws-sdk-go v1 should be left.

splitting out this work over minimum 9 PRs should make it more manageable piece of work.

[Skarlso]

Something to note is that our interfaces now don't need to contain every method. Just the bare minimum that we use and need. :)

[aclevername]

Something to note is that our interfaces now don't need to contain every method. Just the bare minimum that we use and need. :)

That's true, but auto generating the full interface is easier than maintaining a manual list (which would be big!). Plus it helps for discovering new APIs when working on new features

[Skarlso]

I don't agree on that one. You would have to constantly re-generate the mocks if there is a new method added, which is what we do today. The purpose of a handcrafted mock is that it's precise and tailored to our needs. In general it's idiomatic in Go to not provide interfaces for the package you write and this is one of the purpose for it. Users can / should generate the mocks they need.

And the list isn't THAT big it seems. :) EC2 is the largest with a couple weird ones, and EKS. The rest are only describes and a couple updates. It's not that bad! :)