Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: kafka sasl auth #1186

Merged
merged 21 commits into from
Apr 30, 2021
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
1f18505
feat: add sasl config definition
joshuajorel Apr 22, 2021
543194d
feat: add sasl auth to kafka eventsource
joshuajorel Apr 22, 2021
71776e9
test: add sasl validation tests
joshuajorel Apr 22, 2021
244e31a
feat: add sasl auth to kafka sensor
joshuajorel Apr 22, 2021
9b8ad2f
test: added kafka trigger unit test
joshuajorel Apr 22, 2021
5566e10
chore: add autogenerated files
joshuajorel Apr 22, 2021
5201e2c
feat: add validation to sarama mechanism.
joshuajorel Apr 23, 2021
9210e92
fix: made sasl mechanism as an optional argument
joshuajorel Apr 23, 2021
0e377b9
fix: reverted field ordering.
joshuajorel Apr 23, 2021
c02fc33
fix: removed if statement. modified error message. fixed userset and …
joshuajorel Apr 23, 2021
0969441
chore: rebase from master.
joshuajorel Apr 23, 2021
f83f84d
test: fixed test case for password and user in sasl config.
joshuajorel Apr 23, 2021
17cbfc4
chore: update codegen.
joshuajorel Apr 23, 2021
571569e
chore: merged master
joshuajorel Apr 29, 2021
b072a95
fix: reverted byte ordering for protobuf
joshuajorel Apr 29, 2021
f08d09f
refactor: added GetMechanism function for SASLConfig
joshuajorel Apr 29, 2021
4bee62d
refactor: reduced lines for validating SASLConfig
joshuajorel Apr 29, 2021
ec5b99e
refactor: used GetMechanism function for setting SASL mechanism.
joshuajorel Apr 29, 2021
a3d7e3c
build: updated codegen
joshuajorel Apr 29, 2021
1c3a84c
refactor: GetMechanism() returns string
joshuajorel Apr 30, 2021
9e364a4
refactor: removed repeat reference to PLAIN
joshuajorel Apr 30, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions api/event-source.html

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 12 additions & 0 deletions api/event-source.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

26 changes: 26 additions & 0 deletions api/openapi-spec/swagger.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 12 additions & 0 deletions api/sensor.html

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 12 additions & 0 deletions api/sensor.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

20 changes: 20 additions & 0 deletions eventsources/sources/kafka/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,26 @@ func getSaramaConfig(kafkaEventSource *v1alpha1.KafkaEventSource, log *zap.Sugar
config.Version = version
}

if kafkaEventSource.SASL != nil {
config.Net.SASL.Enable = true

config.Net.SASL.Mechanism = kafkaEventSource.SASL.GetMechanism()

user, err := common.GetSecretFromVolume(kafkaEventSource.SASL.User)
if err != nil {
log.Errorf("Error getting user value from secret: %v", err)
return nil, err
}
config.Net.SASL.User = user

password, err := common.GetSecretFromVolume(kafkaEventSource.SASL.Password)
if err != nil {
log.Errorf("Error getting password value from secret: %v", err)
return nil, err
}
config.Net.SASL.Password = password
}

if kafkaEventSource.TLS != nil {
tlsConfig, err := common.GetTLSConfig(kafkaEventSource.TLS)
if err != nil {
Expand Down
3 changes: 3 additions & 0 deletions eventsources/sources/kafka/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,5 +46,8 @@ func validate(eventSource *v1alpha1.KafkaEventSource) error {
if eventSource.TLS != nil {
return apicommon.ValidateTLSConfig(eventSource.TLS)
}
if eventSource.SASL != nil {
return apicommon.ValidateSASLConfig(eventSource.SASL)
}
return nil
}
24 changes: 24 additions & 0 deletions pkg/apis/common/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package common

import (
"github.com/Shopify/sarama"
corev1 "k8s.io/api/core/v1"
)

Expand Down Expand Up @@ -128,6 +129,19 @@ type TLSConfig struct {
DeprecatedClientKeyPath string `json:"clientKeyPath,omitempty" protobuf:"bytes,6,opt,name=clientKeyPath"`
}

// SASLConfig refers to SASL configuration for a client
type SASLConfig struct {
// SASLMechanism is the name of the enabled SASL mechanism.
// Possible values: OAUTHBEARER, PLAIN (defaults to PLAIN).
// +optional
Mechanism string `json:"mechanism,omitempty" protobuf:"bytes,1,opt,name=mechanism"`
// User is the authentication identity (authcid) to present for
// SASL/PLAIN or SASL/SCRAM authentication
User *corev1.SecretKeySelector `json:"user,omitempty" protobuf:"bytes,2,opt,name=user"`
// Password for SASL/PLAIN authentication
Password *corev1.SecretKeySelector `json:"password,omitempty" protobuf:"bytes,3,opt,name=password"`
}

// Backoff for an operation
type Backoff struct {
// The initial duration in nanoseconds or strings like "1s", "3m"
Expand All @@ -153,3 +167,13 @@ type Metadata struct {
Annotations map[string]string `json:"annotations,omitempty" protobuf:"bytes,1,rep,name=annotations"`
Labels map[string]string `json:"labels,omitempty" protobuf:"bytes,2,rep,name=labels"`
}

func (s SASLConfig) GetMechanism() sarama.SASLMechanism {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall everything looks good to me except this part.

  1. Returning sarama.SASLMechanism makes github.com/Shopify/sarama get introduced to the models, which is not good. Ideally the dependencies in models are either from native GoLang or k8s related. I prefer to return a string here.
  2. Another reason is, since this struct is put in common, it is intended to be used by not only Kafka. Assume it's going to be used by something else, sarma.SASLMechanism might not be recognized by it. Also, I made a little research on SASL auth, it looks like the mechanism is not unified, different product has different support (PLAIN might be the one everyone supports). Correct me if I'm wrong.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Apologies for the oversight. Removed the type reference and refactored again to reflect on the actual sensor and eventsource.
  2. I'm basing the authentication mechanism on the available library which is Sarama. The other authentication mechanisms might be something to look into in the future, but "PLAIN" auth is a good place to start.

switch s.Mechanism {
case "PLAIN", "OAUTHBEARER", "SCRAM-SHA-256", "SCRAM-SHA-512", "GSSAPI":
return sarama.SASLMechanism(s.Mechanism)
default:
// default to PLAINTEXT mechanism
return sarama.SASLMechanism(sarama.SASLTypePlaintext)
}
}
26 changes: 26 additions & 0 deletions pkg/apis/common/deepcopy_generated.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading