sarama icon indicating copy to clipboard operation
sarama copied to clipboard

add support for aws mks iam

Open sethpollack opened this issue 3 years ago • 9 comments

fixes https://github.com/Shopify/sarama/issues/1985

usage would look like:

cfg.Net.SASL.Mechanism = sarama.SASLMechanism(sarama.SASLTypeAWSMSKIAM)
cfg.Net.SASL.AWSMSKIAM = sarama.AWSMSKIAMConfig{
    Region: "us-east-1",
}

sethpollack avatar Mar 22 '22 18:03 sethpollack

@sethpollack thanks for raising this

I wasn't aware that AWS had implented their own custom SASL mechanism for this, rather than adopting one of the existing standards. Whilst I can understand that people using AWS would like this support, ideally we wouldn't want to a) introduce a direct dependency on awk sdks for everyone or b) have service-specific configuration parameters etc. — this is also quite hard for us to regression test without provisioning a live service

I notice that other kafka clients had this support added using a pluggable "opt-in" sasl provider architecture. I wonder how much work it would be to achieve a similar result with Sarama? i.e., keeping the dependencies and configuration in a separate module path that people can opt-in to using by pulling it in as a dependency

dnwe avatar Mar 28 '22 16:03 dnwe

@dnwe Another option would be to use SCRAMClientGeneratorFunc but we would need to pass along the broker addr to the function.

sethpollack avatar Mar 28 '22 17:03 sethpollack

I was implementing the same feature for a Kafka Terraform provider that relies on Sarama and I went for this last solution @sethpollack mentioned: using a custom SCRAMClientGeneratorFunc. I think passing a pluggable function is the best solution, in this way Sarama could be very flexible supporting many (potential) SASL/SCRAM-based mechanism.

The problem is Sarama is not flexible right now: in many points the code checks if the mechanism is strictly "SASLTypeSCRAMSHA256" or "SASLTypeSCRAMSHA512", there is no room to add new mechanisms. I would propose to enforce a generalized "SASLTypeSCRAM" mechanism and, in addition to that, introduce a flexible mechanism name (i.e. "SCRAM-SHA-256, "SCRAM-SHA-512", "AWS_MSK_IAM") that could be externally specified. I think these modifications, with the already-in-place possibility to specify a custom SCRAMClientGeneratorFunc, allow what we want to achieve.

What do you think?

lomluca avatar Apr 09 '22 14:04 lomluca

Any updates on this? We use Sarama for a large number of our services, and would love to keep using it, but are considering moving over to kafka-go because it has IAM support.

michaelwagler avatar May 18 '22 21:05 michaelwagler

@sethpollack for SASL.User/SASL.Password do you have to provide a static accesskey/secretkey? https://github.com/Shopify/sarama/blob/f16c9d8fbe4866c970b20a08be14d57553b0b660/broker.go#L1417 mentions user/pw

tooptoop4 avatar Oct 20 '22 19:10 tooptoop4

I think that gets ignored for IAM.

IAM has a few options: https://github.com/Shopify/sarama/pull/2192/files#diff-879d7ff817ef6271a190861e67ab0ff3a2751136cedff764b3f680565540dc2bR1794-R1811

sethpollack avatar Oct 20 '22 19:10 sethpollack

Any updates on this? When can Sarama provide IAM support?

kashifkhan0771 avatar Dec 08 '22 09:12 kashifkhan0771

Please :)

bpaquet avatar Dec 09 '22 15:12 bpaquet

Up for 2023 NY ;)

fuyar avatar Jan 11 '23 15:01 fuyar

Any updates? Do you know when it will be merged?

galamit1 avatar Apr 09 '23 11:04 galamit1

@sethpollack would you have a moment to sign the CLA so we can get this merged? 🙏🏻

austinorth avatar Apr 14 '23 17:04 austinorth

@austinorth unfortunately that CLA is asking for way too much personal information, I don't feel comfortable signing it.

sethpollack avatar Apr 14 '23 17:04 sethpollack

@sethpollack ahhh worddd

austinorth avatar Apr 14 '23 17:04 austinorth

@sethpollack can I fork you PR to get it merged?

bpaquet avatar Apr 17 '23 08:04 bpaquet

@bpaquet Yes

sethpollack avatar Apr 17 '23 11:04 sethpollack

Hi! Is there any update on this PR? Thanks

joaocc avatar Aug 31 '23 12:08 joaocc

+1

mauriciottc avatar Sep 12 '23 14:09 mauriciottc

+1

cjireland avatar Sep 13 '23 16:09 cjireland

This PR was superseded by https://github.com/IBM/sarama/pull/2482 which is waiting for more testing feedback from AWS users and the /v2 version boundary of Sarama (because it will change the client API for auth)

dnwe avatar Sep 14 '23 08:09 dnwe