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.

Sign up for GitHub

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

Jump to bottom

[Closed & Outmoded]Add that support Kafka SASL/PLAIN authentication of SCRAM-SHA-256 or SCRAM-SHA-512 mechanism #2723

Closed
wants to merge 40 commits into from
Closed

[Closed & Outmoded]Add that support Kafka SASL/PLAIN authentication of SCRAM-SHA-256 or SCRAM-SHA-512 mechanism #2723

wants to merge 40 commits into from

Conversation

WalkerWang731
Copy link
Contributor

Which problem is this PR solving?

Short description of the changes

  • The smallest modification on the current version.
  • Different from the previous PR WIP - Adding scram authentication for kafka #2110, I just modified pkg/kafka/auth/plaintext.go , pkg/kafka/auth/options.go and pkg/kafka/auth/config.go, and not add any new file.
  • More clearly for option name, using option name is mechanism (reference from Kafka parameter name rules )
  • About the Prefix, I think to keep using plaintext better than scarm, because these are both belong to SASL, in the future, it can change to sasl if needed. Actually, using plaintext is ok, it will not be confusing, because it name is SASL_PLAINTEXT that from Kafka official security protocol parameter name
  • About the parameters limit of mechanism, I followed Kafka name rules (SCRAM-SHA-256, SCRAM-SHA-512, PLAIN)
  • I reserved the current version plaintext mechanism, and add to default. If users do not specify the mechanism, it not affect continued use. if users use the mechanism then just only add --kafka.ROLE.plaintext.mechanism parameter then can use it.

Changelog

Add that support Kafka SASL/PLAIN authentication of SCRAM-SHA-256 or SCRAM-SHA-512 mechanism

Parameter config of collector (SPAN_STORAGE_TYPE=kafka)

--kafka.producer.plaintext.mechanism string      The plaintext Mechanism for SASL/PLAIN authentication, e.g. 'SCRAM-SHA-256' or 'SCRAM-SHA-512' or 'PLAIN' (default "PLAIN")

Parameter config of ingester

--kafka.consumer.plaintext.mechanism string      The plaintext Mechanism for SASL/PLAIN authentication, e.g. 'SCRAM-SHA-256' or 'SCRAM-SHA-512' or 'PLAIN' (default "PLAIN")

WalkerWang731 and others added 30 commits November 30, 2020 17:10
@WalkerWang731
add metrics that show agent connection collector status
b712c9d 
Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>
@WalkerWang731
update comment
109617c 
Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>
@WalkerWang731
exec make fmt
eee31fd 
Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>
@WalkerWang731
simplify function and add testing relevant code in the builder_test.go
82ed46e 
Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>
@WalkerWang731
add comment in connect_metrics.go
ff406aa 
Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>
@WalkerWang731
simplify code and changed use expvar to show target
c2a64f4 
Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>
@WalkerWang731
simplify code and changed use expvar to show target
1429e76 
Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>
@WalkerWang731
exec make fmt
dd68627 
Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>
@Betula-L @WalkerWang731
Fix collector panic due to sarama sdk returning nil error (#2654)
83452e4 
Signed-off-by: luhualin <luhualin@bilibili.com>

Co-authored-by: luhualin <luhualin@bilibili.com>
Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>
@pkositsyn @yurishkuro @WalkerWang731
Fix flaky tbuffered server test (#2635)
68c0b29 
* Fix flaky tbuffered server test

Signed-off-by: Pavel Kositsyn <kositsyn.pa@phystech.edu>

* Apply suggestions from code review - more readable comments

Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>
Signed-off-by: Pavel Kositsyn <kositsyn.pa@phystech.edu>

Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>
Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>
@Ashmita152 @WalkerWang731
Add github actions for integration tests (#2649)
f651162 
* Add github action for jaeger integration tests

Signed-off-by: Ashmita Bohara <ashmita.bohara152@gmail.com>

* Create separate workflow for each integration test

Signed-off-by: Ashmita Bohara <ashmita.bohara152@gmail.com>

* Feedbacks changes

Signed-off-by: Ashmita Bohara <ashmita.bohara152@gmail.com>
Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>
@yurishkuro @WalkerWang731
Clean-up GH action names (#2661)
8ee024e 
Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>
@Ashmita152 @WalkerWang731
Fix for failures in badger integration tests (#2660)
111efb7 
Signed-off-by: Ashmita Bohara <ashmita.bohara152@gmail.com>
Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>
@Ashmita152 @WalkerWang731
Add protogen validation test (#2662)
e4da609 
Signed-off-by: Ashmita Bohara <ashmita.bohara152@gmail.com>
Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>
@Ashmita152 @WalkerWang731
Add github action for jaeger all-in-one image (#2663)
9ad6033 
* Add github action for jaeger all-in-one image

Signed-off-by: Ashmita Bohara <ashmita.bohara152@gmail.com>

* Feedbacks changes

Signed-off-by: Ashmita Bohara <ashmita.bohara152@gmail.com>

* Feedbacks changes

Signed-off-by: Ashmita Bohara <ashmita.bohara152@gmail.com>

* Feedbacks changes

Signed-off-by: Ashmita Bohara <ashmita.bohara152@gmail.com>

* Feedbacks changes

Signed-off-by: Ashmita Bohara <ashmita.bohara152@gmail.com>

* Feedbacks changes

Signed-off-by: Ashmita Bohara <ashmita.bohara152@gmail.com>

* Make steps self-explantory

Signed-off-by: Ashmita Bohara <ashmita.bohara152@gmail.com>

* Fix git tags issue

Signed-off-by: Ashmita Bohara <ashmita.bohara152@gmail.com>

* Fix ES integration test

Signed-off-by: Ashmita Bohara <ashmita.bohara152@gmail.com>
Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>
@yurishkuro @WalkerWang731
Update comment that looks confusing during builds
faa61ac 
Signed-off-by: Yuri Shkuro <github@ysh.us>
Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>
@yurishkuro @WalkerWang731
Use GitHub actions based build badges
71755cc 
Signed-off-by: Yuri Shkuro <github@ysh.us>
Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>
@Ashmita152 @WalkerWang731
Fix and minor improvements to all-in-one github action (#2667)
0274fcf 
Signed-off-by: Ashmita Bohara <ashmita.bohara152@gmail.com>
Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>
@Ashmita152 @WalkerWang731
Fix docker login issue with all-in-one build (#2668)
c222b0d 
* Fix docker login issue with all-in-one build

Signed-off-by: Ashmita Bohara <ashmita.bohara152@gmail.com>

* Fix docker login issue with all-in-one build

Signed-off-by: Ashmita Bohara <ashmita.bohara152@gmail.com>
Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>
@Ashmita152 @WalkerWang731
Fix issue with all-in-one build (#2669)
01d1e9c 
Signed-off-by: Ashmita Bohara <ashmita.bohara152@gmail.com>
Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>
@WalkerWang731 @yurishkuro
Update cmd/agent/app/reporter/connect_metrics.go
fb1f57a 
accept suggestions

Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>
Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>
@WalkerWang731 @yurishkuro
Update cmd/agent/app/reporter/connect_metrics.go
ba18453 
accept suggestions

Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>
Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>
@WalkerWang731
simplify the code that remove ConnectMetricsParams{} and integrate Co…
68e6d76 
…nnectMetrics{}

Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>
@WalkerWang731
simplify the code that remove ConnectMetricsParams{} and integrate Co…
ac84830 
…nnectMetrics{}

Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>
@WalkerWang731
Merge branch 'master' into master
767623e
merage from the lastest master branch and exec make fmt
a1ff642 
Signed-off-by: walker.wangxy <walker.wangxy@walkerwangxydeMacBook-Pro.local>
@WalkerWang731
add comment on ConnectMetrics
d07f8b8 
Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>
@WalkerWang731
Merge branch 'master' into master
32c1e9b
@WalkerWang731
Merge branch 'master' into master
512e61e
@WalkerWang731
Merge branch 'master' into master
1222f7c
WalkerWang731 and others added 6 commits December 2, 2020 21:57
@WalkerWang731
Merge branch 'master' into master
58113f7
@WalkerWang731
Merge branch 'master' into master
a75e032
@WalkerWang731
Merge pull request #1 from jaegertracing/master
6d69e6e 
merge from jaegertracing/jaeger master
added that suppot Kafka SASL/PLAIN authentication of SCRAM-SHA-256 or…
6d21a5d 
… SCRAM-SHA-512 mechanism

Signed-off-by: WalkerWang731 <walker.wangxy@walkerwangxydeMacBook-Pro.local>
@WalkerWang731
added that suppot Kafka SASL/PLAIN authentication of SCRAM-SHA-256 or…
1731382 
… SCRAM-SHA-512 mechanism

Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>
@WalkerWang731
Merge branch 'add_kafka_mechanism' of https://github.com/WalkerWang73…
4a741a0 
…1/jaeger into add_kafka_mechanism

Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>
@WalkerWang731 WalkerWang731 requested a review from a team as a code owner January 12, 2021 08:03
@mergify mergify bot requested a review from jpkrohling January 12, 2021 08:03
walker.wangxy and others added 4 commits January 12, 2021 16:27
@WalkerWang731
add comment on XDGSCRAMClient
c6f4451 
Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>
@WalkerWang731
update TestTLSFlags() expected on options_test.go
7b2052b 
Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>
@WalkerWang731
sync #2721 plaintext as supported kafka auth option
81adbb2 
Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>
@WalkerWang731
update TestTLSFlags() expected on flags_test.go
1feee6f 
Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>
@WalkerWang731
Copy link
Contributor Author

Closed this one, because mixed with #2657
I recommit a new PR that follows the newest master branch #2724

@WalkerWang731 WalkerWang731 changed the title Add that support Kafka SASL/PLAIN authentication of SCRAM-SHA-256 or SCRAM-SHA-512 mechanism [Closed & Outmoded]Add that support Kafka SASL/PLAIN authentication of SCRAM-SHA-256 or SCRAM-SHA-512 mechanism Jan 12, 2021
@Neustradamus
Copy link

The merged PR is:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vprithvi vprithvi Awaiting requested review from vprithvi

@jpkrohling jpkrohling Awaiting requested review from jpkrohling

Assignees

@jpkrohling jpkrohling

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@WalkerWang731 @Neustradamus @jpkrohling @Betula-L @pkositsyn @Ashmita152 @yurishkuro