feat: added aws-sdk-go-v2 IAM Mechanism for MSK authentication

[kikyomits]

Added new module aws_msk_iam_v2 to support functionality of authentication with MSK through aws-sdk-go-v2.

The main motivation of this new feature allows to avoid having aws-sdk-go and aws-sdk-go-v2 in user's go.mod (assuming most of users are using aws-sdk-go-v2).

This doesn't deprecate existing aws_msk_iam module as aws-sdk-go hasn't been deprecated yet.

See #934

[dominicbarnes]

This looks like a good starting point to me, have you had a chance to test this out in your own setup, by any chance?

[dominicbarnes]

The dependency surface area is pretty small, all we are using is the request signer, so I wonder if it's possible to abstract away the details of whether you're using v1 or v2 behind an interface or a type alias so you didn't need a whole separate model.

I'll do some digging here.

[kikyomits]

Yeah I agree what you said. Actually I thought through the ways to abstract the signer (v1 and v2) while developing. But I couldn't figure out the way to refactor current module without breaking change. There are several limitations such as the SDKs use different name of methods and we pass AWS Signer struct to Mechanism directly.

I can change the new module to more generic by having a layer to abstract SDK version. Is this what you want to suggest me?

[dominicbarnes]

Yeah, that's what I was discovering too. If you can find a way to unify these, that would be great, I haven't had as much time to look into this as I hoped last week.

[kikyomits]

Cool. I will develop new one with abstracting the v1 and v2 SDK.
Will update you when I confirm it works in my env. Thanks.

[kikyomits]

@dominicbarnes I have completed the changes. Can you please take a look? I have confirmed it works with MSK in my environment.

[shric]

@kikyomits: I'm not involved with the development of kafka-go, just a random user who very much appreciates this PR, thanks!

FWIW, I can confirm this works with MSK in my environment too!

One suggestion for the README.md example code is that you qualify aws_msk_iam.AWSSignerV2 and aws_msk_iam.Mechanism with the package name and add the import:

import (
    ...
    "github.com/segmentio/kafka-go/sasl/aws_msk_iam"
)

Not being familiar with aws-sdk-go-v2 it took me a while to realise where AWSSignerV2 and Mechanism came from.

[kikyomits]

@shric thanks for the feedback and very happy to hear you liked this change. I have added "github.com/segmentio/kafka-go/sasl/aws_msk_iam" in the import statement and updated README a bit to make it more user-friendly.

@dominicbarnes would you mind to review this PR?

[dominicbarnes]

@kikyomits sorry for the delayed response.

I feel bad about this, but I think I'd like you to revert your PR back to what you had before. My main issue now with this approach is that this 1 sub-module now imports 2 very large packages (aws-sdk v1 and v2), which will likely create conflicts for any sufficiently large codebase which may have a mixture of both.

I've cloned your branch locally and was trying to find a way to put the different SDK versions behind their own sub-modules, but this is really challenging to do so without either breaking the API or adding significant complexity.

One option would be to create 2 more sub-modules, which would take on the responsibility of importing aws-sdk v1 or v2 respectively. The current aws_msk_iam module would then become basically a shell, as it would only depend on kafka-go, but that adds a whole other layer of nesting, with 1 layer (kafka-go -> aws_msk_iam) already being annoying enough.

We could theoretically remove the aws_msk_iam module (leaving it as just part of kafka-go), but that becomes a breaking change and would create problems for users that may not be aware of this PR. This is ultimately the route I want to go, but would save it for when we introduce other breaking changes.

In my eyes, having 2 sub-modules (even with some code duplication) is preferable to 1 sub-module which ends up importing 2 significant dependencies at once, as no single user would end up using both. Again, sorry to ask, but would you mind rolling back to the 2-module version?

[kikyomits]

@dominicbarnes thanks for the review and comments. Appreciate sharing your thoughts. I initially thought about the disadvantages as you said - large dependencies - and decoupled the modules. I like the my initial approach too and happy to revert back.

Will send an update to you in the next 24 hours

[kikyomits]

@dominicbarnes finished the changes and developed new module aws_go_sdk_v2. Can you review this?

[dominicbarnes]

Thank you for the contribution, as well as the patience to work through a few revisions. :)