feat: add support for SASL SCRAM 256 and 512 for Kafka #1518
Conversation
Signed-off-by: Damien Raymond <damien.raymond@paddypowerbetfair.com>
Dnomyar
force-pushed
the
add-sasl-scram-256-512
branch
from
b11d770
to
12a4d8f
Compare
Signed-off-by: Damien Raymond <damien.raymond@paddypowerbetfair.com>
|
could you please update the doc in https://github.com/numaproj/numaflow/blob/main/docs/user-guide/sources/kafka.md and https://github.com/numaproj/numaflow/blob/main/docs/user-guide/sinks/kafka.md to include SCRAM 256 and 512? |
KeranYang
changed the title
Signed-off-by: Damien Raymond <damien.raymond@paddypowerbetfair.com>
sasl_config: extracted setting username and password, added tests volume: created interface to allow to mock reading from the filesystem Signed-off-by: Damien Raymond <damien.raymond@paddypowerbetfair.com>
Dnomyar
force-pushed
the
add-sasl-scram-256-512
branch
from
3912209
to
d84a9b2
Compare
|
I have updated the docs and added some tests. Could you indicate how should I fix conflicts for the go.sum, please? I am not so familiar with go |
|
i have resolved it |
|
can you do a git-pull and run |
Signed-off-by: Damien Raymond <damien.raymond@paddypowerbetfair.com>
pkg/shared/util/volume.go
Outdated
| @@ -30,9 +30,20 @@ var ( | |||
| configMapKeySelectorType = reflect.TypeOf(&corev1.ConfigMapKeySelector{}) | |||
| ) | |||
|
|
|||
| type GetSecrets interface { | |||
There was a problem hiding this comment.
- I assume the purpose of the interface is to be able to mock it for testing? If that's the case, make it and
OsFile? - The name does not match the 2 functions.
There was a problem hiding this comment.
I assume the purpose of the interface is to be able to mock it for testing?
yes I added that as a comment
If that's the case, make it and OsFile?
Sorry, I don't understand, what you mean by that
The name does not match the 2 functions.
I renamed to volumeReader, what do you think?
pkg/shared/util/sasl_config.go
Outdated
| } | ||
|
|
||
| // GetSASLConfig A utility function to get sarama.Config.Net.SASL | ||
| func GetSASLStrategy[GetSecretsStrategy GetSecrets](saslConfig *dfv1.SASL, strategy GetSecretsStrategy) (*struct { |
There was a problem hiding this comment.
What is the reason you use generics instead of just writing it as:
func GetSASLStrategy(saslConfig *dfv1.SASL, strategy GetSecrets) (*struct {
...
There was a problem hiding this comment.
Also, make the function private.
There was a problem hiding this comment.
What is the reason you use generics instead of just writing it as:
That's a very good point! Changed!
Also, make the function private.
I went through the code I changed the visibility of things that should be private
pkg/shared/util/sasl_config.go
Outdated
| } | ||
|
|
||
| // GetSASLConfig A utility function to get sarama.Config.Net.SASL | ||
| func GetSASLStrategy[GetSecretsStrategy GetSecrets](saslConfig *dfv1.SASL, strategy GetSecretsStrategy) (*struct { |
There was a problem hiding this comment.
Also, make the function private.
pkg/shared/util/sasl_config.go
Outdated
| default: | ||
| return nil, fmt.Errorf("SASL mechanism not supported: %s", *saslConfig.Mechanism) | ||
| } | ||
| return &config.Net.SASL, nil | ||
| } | ||
|
|
||
| func SetUserPassword[GetSecretsStrategy GetSecrets](config *sarama.Config, userSecret *corev1.SecretKeySelector, passwordSecret *corev1.SecretKeySelector, strategy GetSecretsStrategy) error { |
pkg/shared/util/volume.go
Outdated
| // GetSecretFromVolume retrieves the value of mounted secret volume | ||
| // "/var/numaflow/secrets/${secretRef.name}/${secretRef.key}" is expected to be the file path | ||
| func GetSecretFromVolume(selector *corev1.SecretKeySelector) (string, error) { | ||
| func (_ OsFile) GetSecretFromVolume(selector *corev1.SecretKeySelector) (string, error) { |
There was a problem hiding this comment.
Using underscore will bring warning in some IDEs.
There was a problem hiding this comment.
Using underscore will bring warning in some IDEs.
I thought the underscore was required, changed!
Signed-off-by: Damien Raymond <damien.raymond@paddypowerbetfair.com>
|
Thanks for you feedback @whynowy! I think I have fixed most things. Let me know if I forgot anything |
|
thank you @Dnomyar for your contribution. Would you like to add your use-case to https://github.com/numaproj/numaflow/blob/main/USERS.md? |
Signed-off-by: Damien Raymond <damien.raymond@paddypowerbetfair.com> Co-authored-by: Damien Raymond <damien.raymond@paddypowerbetfair.com>
|
Thank you @Dnomyar for the contribution! We're keen to hear about your experience with Numaflow and gather feedback, would you be open for a chat? |
Hey folks,
First, thanks for this project!
I am proposing to add support for SASL SCRAM 256 and 512 for the Kafka connector.
I only tested SASL SCRAM 512 with a source (not a sink).
There are a few things that are missing:
This is my first time doing Go so I appreciate any feedback
Cheers