feat(kafka-logger): support sasl conf #7782

biubiue · 2022-08-24T12:52:30Z

Description

Fixes # (issue)

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)

apisix/plugins/kafka-logger.lua

t/plugin/kafka-logger.t

spacewander · 2022-09-07T05:59:13Z

t/plugin/kafka-logger.t

+--- config
+    location /t {
+        content_by_lua_block {
+            local producer = require "resty.kafka.producer"


We should test the kafka-logger plugin directly like other tests, instead of using the producer outside of the plugin.

We should test the kafka-logger plugin directly like other tests, instead of using the producer outside of the plugin.

Em... I maybe not understand https://github.com/apache/apisix/pull/7782#discussion_r956718372 .
OK, I will add test case like others

tzssangglass · 2022-09-07T07:27:37Z

t/plugin/kafka-logger.t

+=== TEST 21: access
+--- request
+GET /hello
+--- response_body
+hello world
+--- wait: 2


we can inject the producer.new like:

apisix/t/plugin/tencent-cloud-cls.t

Lines 220 to 236 in 2aa5c48

=== TEST 7: verify request

--- extra_init_by_lua

local cls = require("apisix.plugins.tencent-cloud-cls.cls-sdk")

cls.send_to_cls = function(self, logs)

if (#logs ~= 1) then

ngx.log(ngx.ERR, "unexpected logs length: ", #logs)

return

end

return true

end

--- request

GET /opentracing

--- response_body

opentracing

--- error_log

Batch Processor[tencent-cloud-cls] successfully processed the entries

--- wait: 0.5

we can inject the producer.new like:

apisix/t/plugin/tencent-cloud-cls.t

Lines 220 to 236 in 2aa5c48

=== TEST 7: verify request

--- extra_init_by_lua

local cls = require("apisix.plugins.tencent-cloud-cls.cls-sdk")

cls.send_to_cls = function(self, logs)

if (#logs ~= 1) then

ngx.log(ngx.ERR, "unexpected logs length: ", #logs)

return

end

return true

end

--- request

GET /opentracing

--- response_body

opentracing

--- error_log

Batch Processor[tencent-cloud-cls] successfully processed the entries

--- wait: 0.5

Em, like this?
=== TEST 20: create producer with sasl_config
--- extra_init_by_lua
local producer = require("resty.kafka.producer")
local klogger = require("apisix.plugins.kafka-logger")
producer.new = function(klogger)
if (!klogger.sasl_config) then
ngx.say("create producer without sasl_config")
return producer
end
producer.sasl_config = klogger.sasl_config
ngx.say("create producer with sasl_config")
return producer
end
--- request
GET /hello
--- response_body
hello world
--- wait: 2

tokers · 2022-09-07T10:55:24Z

apisix/plugins/kafka-logger.lua

+            properties = {
+                mechanism = { type = "string", description = "mechanism" },
+                password =  { type = "string", description = "password" },
+                user = { type = "string", description = "user" },


These fields should be required if the sasl config exists?

These fields should be required if the sasl config exists?

I modify these fields ,and mark required fields

tokers · 2022-09-08T01:54:41Z

docs/en/latest/plugins/kafka-logger.md

@@ -38,6 +38,11 @@ It might take some time to receive the log data. It will be automatically sent a
 | Name                   | Type    | Required | Default        | Valid values          | Description                                                                                                                                                                                                                                                                                                                                      |
 | ---------------------- | ------- | -------- | -------------- | --------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ |
 | broker_list            | object  | True     |                |                       | List of Kafka brokers (nodes).                                                                                                                                                                                                                                                                                                                   |
+| sasl_config            | object  | False    |                |                       | Kafka sasl conf.                                                                                                                                                                                                                                                                                                                   |
+| sasl_config.mechanism  | string  | False    |                |                       | Kafka mechanism.                                                                                                                                                                                                                                                                                                                   |


Add the default value here.

Add the default value here.

ok

tokers · 2022-09-08T01:56:52Z

apisix/plugins/kafka-logger.lua

+            type = "object",
+            description = "sasl config",
+            properties = {
+                mechanism = { type = "string", description = "mechanism", default = "PLAIN" },


The mechanism should be restricted to only use a few enumerations like PLAIN, SCRAM and so on.

The mechanism should be restricted to only use a few enumerations like PLAIN, SCRAM and so on.

ok, I change it

The mechanism should be restricted to only use a few enumerations like PLAIN, SCRAM and so on.

Just now I review lua-resty-kafka ,
Write this local MECHANISM_SCRAMSHA256 = "SCRAM-SHA-256" -- to do, so SCRAM-SHA-256 maybe not work.

OK, SCRAM-SHA-256 is copied from Confluent docs. No worries at all.

spacewander · 2022-09-09T05:21:07Z

Please make the CI pass, thanks!

biubiue · 2022-09-09T11:08:28Z

Please make the CI pass, thanks!

Yeah,I also want to make CI pass, but write test case ,it’s not easy for me. I will do it soon.

tzssangglass · 2022-09-13T11:09:16Z

t/plugin/kafka-logger.t

+            ngx.log(ngx.ERR, "unexpected broker_list length: ", #broker_list)
+            return nil
+        end
+        return producer


The producer should not be returned here, you can see that the original returned is a table.

The producer should not be returned here, you can see that the original returned is a table.

Hi, when inject producer.new，it also dependency client.new. It means I will need inject client.new .

feat(kafka-logger): support sasl conf

40ddca3

biubiue mentioned this pull request Aug 24, 2022

help request: add test case #7783

Closed

biubiue added 2 commits August 25, 2022 15:01

feat(kafka-logger): doc add sasl conf

21be757

fix(kafka-logger): doc

fad3df6

spacewander reviewed Aug 26, 2022

View reviewed changes

apisix/plugins/kafka-logger.lua Show resolved Hide resolved

biubiue added 7 commits August 28, 2022 11:22

feat(kafka-logger): test case

de82ec3

feat(kafka-logger): modify test case code style

33cd466

fix(kafka-logger): test case code style

7e1db99

fix(kafka-logger): test case code style

ebeebc8

fix(kafka-logger): test case code style

cd68da2

fix(kafka-logger): test case code style

0104c5a

Update kafka-logger.t

503011c

spacewander reviewed Aug 28, 2022

View reviewed changes

t/plugin/kafka-logger.t Outdated Show resolved Hide resolved

biubiue added 6 commits September 1, 2022 10:20

fix(kafka-logger): test case

18358aa

fix(kafka-logger): test case

3812c9c

Update kafka-logger.t

c8baf35

Update kafka-logger.t

d348987

Update kafka-logger.t

72f0d06

fix(kafka-logger): test case

5b7dd8c

biubiue requested a review from spacewander September 6, 2022 09:54

biubiue added 5 commits September 6, 2022 18:04

fix(kafka-logger): try other actions

9f1817a

Update kafka-logger.t

588bb0f

Update kafka-logger.t

3a90b5d

Update kafka-logger.t

d10ea12

Update kafka-logger.t

70054aa

spacewander reviewed Sep 7, 2022

View reviewed changes

Update kafka-logger.t

35a581b

tzssangglass reviewed Sep 7, 2022

View reviewed changes

tokers reviewed Sep 7, 2022

View reviewed changes

fix(kafka-logger): modify sasl_config properties

9d9f005

tokers reviewed Sep 8, 2022

View reviewed changes

Update kafka-logger.md

8c41473

biubiue added 4 commits September 9, 2022 17:43

Update kafka-logger.t

4e1cc34

Update kafka-logger.lua

ddde433

Update kafka-logger.md

0444c00

Update kafka-logger.lua

cc90a47

fix(kafka-logger): test case

290070c

tzssangglass reviewed Sep 13, 2022

View reviewed changes

biubiue added 4 commits September 26, 2022 16:23

fix(kafka-logger): test case

1481240

test case

6968d48

test case

11d8dc6

test case

8ca60db

biubiue closed this Sep 27, 2022

tzssangglass mentioned this pull request Oct 9, 2022

feat: As a user, I want to support sasl_config in kafka-logger plugin, so that we can send logs to brokers that enable authentication #8046

Closed

starsz mentioned this pull request Oct 10, 2022

feat(kafka-logger): support sasl config in brokers #8050

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(kafka-logger): support sasl conf #7782

feat(kafka-logger): support sasl conf #7782

biubiue commented Aug 24, 2022 •

edited

Loading

spacewander Sep 7, 2022

biubiue Sep 7, 2022

tzssangglass Sep 7, 2022

biubiue Sep 8, 2022

tokers Sep 7, 2022

biubiue Sep 8, 2022

tokers Sep 8, 2022

biubiue Sep 8, 2022

tokers Sep 8, 2022

biubiue Sep 8, 2022

biubiue Sep 8, 2022 •

edited

Loading

tokers Sep 8, 2022

spacewander commented Sep 9, 2022

biubiue commented Sep 9, 2022

tzssangglass Sep 13, 2022

biubiue Sep 26, 2022

	=== TEST 7: verify request
	--- extra_init_by_lua
	local cls = require("apisix.plugins.tencent-cloud-cls.cls-sdk")
	cls.send_to_cls = function(self, logs)
	if (#logs ~= 1) then
	ngx.log(ngx.ERR, "unexpected logs length: ", #logs)
	return
	end
	return true
	end
	--- request
	GET /opentracing
	--- response_body
	opentracing
	--- error_log
	Batch Processor[tencent-cloud-cls] successfully processed the entries
	--- wait: 0.5

feat(kafka-logger): support sasl conf #7782

feat(kafka-logger): support sasl conf #7782

Conversation

biubiue commented Aug 24, 2022 • edited Loading

Description

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

biubiue Sep 8, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

spacewander commented Sep 9, 2022

biubiue commented Sep 9, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

biubiue commented Aug 24, 2022 •

edited

Loading

biubiue Sep 8, 2022 •

edited

Loading