feat(pubsub): support kafka tls and sasl/plain auth by bzp2010 · Pull Request #7046 · apache/apisix

bzp2010 · 2022-05-13T17:49:19Z

Description

Part of #6995 to implement TLS and SASL/PLAIN authentication support for kafka.

Checklist

I have explained the need for this PR and the problem it solves
I have explained the changes or the new features added to this PR
I have added tests corresponding to this change
I have updated the documentation to reflect this change
I have verified that this change is backward compatible (If not, please discuss on the APISIX mailing list first)

bzp2010 · 2022-05-13T18:08:14Z

Update

This is the part after #7032 that is currently in ready state, and when #7032 is merged, it is ready to start the review.

All the changes in #7032 were merged in this PR, so there are more lines, which will be greatly reduced when #7032 is merged.

membphis

LGTM

soulbird · 2022-05-16T10:06:08Z

.github/workflows/centos7-ci.yml

    - name: Run other docker containers for test
      run: |
+        # generating SSL certificates for Kafka
+        keytool -genkeypair -keyalg RSA -dname "CN=127.0.0.1" -alias 127.0.0.1 -keystore ./ci/pod/kafka/kafka-server/selfsigned.jks -validity 365 -keysize 2048 -storepass changeit


Is it more appropriate to put it in the linux-ci-init-service.sh script ?

First make sure that the certificate exists for docker-compose to start kafka. If the certificate does not exist then the kafka container will crash.

soulbird · 2022-05-16T10:15:00Z

ci/linux_openresty_common_runner.sh


+    # generating SSL certificates for Kafka
+    keytool -genkeypair -keyalg RSA -dname "CN=127.0.0.1" -alias 127.0.0.1 -keystore ./ci/pod/kafka/kafka-server/selfsigned.jks -validity 365 -keysize 2048 -storepass changeit
+


so, add this to linux-ci-init-service.sh script ?

apisix/plugins/kafka-proxy.lua

tzssangglass · 2022-05-17T03:44:02Z

LGTM

spacewander · 2022-05-17T05:51:48Z

apisix/upstream.lua

    end

-    if conf.tls then
+    if conf.tls and conf.tls.client_cert and conf.tls.client_key then


Suggested change

if conf.tls and conf.tls.client_cert and conf.tls.client_key then

if conf.tls and conf.tls.client_cert then

is enough?

Yes, it's enough, we ensure client_cert and client_key both exist by jsonschema's dependencies. Any one of them separate exist is forbidden.

apisix/apisix/schema_def.lua

Lines 416 to 419 in 99bced1

dependencies = {

client_cert = {"client_key"},

client_key = {"client_cert"},

}

spacewander

Let's provide a doc about the kafka-proxy plugin.

spacewander · 2022-05-17T05:59:28Z

apisix/plugins/kafka-proxy.lua

+                },
+                password = {
+                    type = "string",
+                    default = "",


We can remove the default if these fields are required?

bzp2010 added 8 commits May 13, 2022 23:43

chore: merge kafka support

d431550

feat: add tls verify

366ce56

feat: kafka support tls

4339cb2

feat: support sasl/plain auth

e89d31c

test: add kafka-proxy cases

5a513d0

test: kafka env support tls and sasl

61aefd0

test: add tls and sasl cases

131c130

docs: add tls and sasl

3071725

bzp2010 mentioned this pull request May 13, 2022

feat: support kafka consumer for pubsub scenario #6995

Closed

5 tasks

bzp2010 marked this pull request as ready for review May 13, 2022 18:08

bzp2010 added 4 commits May 14, 2022 02:31

feat: ensure client cert and key exist sametime

f1d9283

debug: kafka docker compose

e31f186

fix: ensure ssl generated for kafka

185fbb4

debug: remove log

a1510bf

bzp2010 requested review from shuaijinchao, spacewander and tzssangglass May 14, 2022 04:41

bzp2010 self-assigned this May 14, 2022

membphis previously approved these changes May 15, 2022

View reviewed changes

bzp2010 added 4 commits May 15, 2022 21:39

chore: check int64 convert bound

45d4f33

chore: ensure pubsub proto use absolute link

a68e2e6

fix: replace all Pub-Sub to PubSub

6ee9a2b

docs: fix style

a0753a8

bzp2010 dismissed membphis’s stale review via a0753a8 May 15, 2022 13:52

bzp2010 added 2 commits May 16, 2022 17:22

Merge master

d0d5566

test: add upstream tls cases

d56772c

bzp2010 requested a review from membphis May 16, 2022 09:37

soulbird reviewed May 16, 2022

View reviewed changes

tzssangglass reviewed May 17, 2022

View reviewed changes

apisix/plugins/kafka-proxy.lua Outdated Show resolved Hide resolved

bzp2010 requested a review from tzssangglass May 17, 2022 03:56

soulbird previously approved these changes May 17, 2022

View reviewed changes

tzssangglass previously approved these changes May 17, 2022

View reviewed changes

spacewander reviewed May 17, 2022

View reviewed changes

docs: add kafka-proxy documentation

99bced1

bzp2010 dismissed stale reviews from tzssangglass and soulbird via 99bced1 May 17, 2022 06:17

bzp2010 added 2 commits May 17, 2022 14:21

fix: review

6f87da0

fix: remove kafka proxy sasl default value

9f497b8

bzp2010 requested review from spacewander and tzssangglass May 17, 2022 06:23

bzp2010 added 2 commits May 17, 2022 14:24

fix: add kafka proxy to category

30b00a7

fix: typo

643414d

spacewander approved these changes May 17, 2022

View reviewed changes

tzssangglass approved these changes May 17, 2022

View reviewed changes

spacewander merged commit 18f6e5c into apache:master May 17, 2022

Liu-Junlin pushed a commit to Liu-Junlin/apisix that referenced this pull request May 20, 2022

feat(pubsub): support kafka tls and sasl/plain auth (apache#7046)

4a57df7

spacewander pushed a commit that referenced this pull request Jun 30, 2022

feat(pubsub): support kafka tls and sasl/plain auth (#7046)

de283cf

SkyeYoung mentioned this pull request Aug 19, 2025

fix: when only tls.verify, skip the logic of judging client cert #12527

Merged

5 tasks


		# generating SSL certificates for Kafka
		keytool -genkeypair -keyalg RSA -dname "CN=127.0.0.1" -alias 127.0.0.1 -keystore ./ci/pod/kafka/kafka-server/selfsigned.jks -validity 365 -keysize 2048 -storepass changeit

	if conf.tls and conf.tls.client_cert and conf.tls.client_key then
	if conf.tls and conf.tls.client_cert then

	dependencies = {
	client_cert = {"client_key"},
	client_key = {"client_cert"},
	}

Conversation

bzp2010 commented May 13, 2022

Description

Checklist

Uh oh!

bzp2010 commented May 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Update

Uh oh!

membphis left a comment

Choose a reason for hiding this comment

Uh oh!

soulbird May 16, 2022

Choose a reason for hiding this comment

Uh oh!

bzp2010 May 16, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

soulbird May 16, 2022

Choose a reason for hiding this comment

Uh oh!

bzp2010 May 16, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tzssangglass commented May 17, 2022

Uh oh!

spacewander May 17, 2022

Choose a reason for hiding this comment

Uh oh!

bzp2010 May 17, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bzp2010 May 17, 2022

Choose a reason for hiding this comment

Uh oh!

spacewander left a comment

Choose a reason for hiding this comment

Uh oh!

spacewander May 17, 2022

Choose a reason for hiding this comment

Uh oh!

bzp2010 May 17, 2022

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

bzp2010 commented May 13, 2022 •

edited

Loading

bzp2010 May 16, 2022 •

edited

Loading

bzp2010 May 17, 2022 •

edited

Loading