feat: add headers attribute for loki-logger (#11420)#12243
Conversation
| tenant_id = {type = "string", default = "fake"}, | ||
|
|
||
| -- authorization header | ||
| authorization = { type = "string", minLength = 1, description = "Authorization header" }, |
There was a problem hiding this comment.
As mentioned in https://grafana.com/docs/loki/latest/operations/authentication/#manage-authentication, Loki does not have any built-in conventions for authentication mechanisms, so the request header used for authentication can be quite flexible; it does not necessarily be Authorization: XXX.
This is different from clickhouse-logger and elasticsearch-logger, which have built-in authentication.
This is exactly why I didn't add any authentication to the current plugin.
Therefore, it is better to design a generic mechanism for it, i.e., provide a user-configurable object to store one or more request headers.
| authorization = { type = "string", minLength = 1, description = "Authorization header" }, | |
| headers = { | |
| type = "object", | |
| patternProperties = { | |
| [".*"] = { | |
| type = "string", | |
| minLength = 1, | |
| }, | |
| }, | |
| }, |
I don't have any idea about the name of the field, it could use headers, request_headers or auth_headers, ask other reviewers to take a look.
| "loki-logger": { | ||
| "endpoint_addrs": ["http://127.0.0.1:3100"], | ||
| "tenant_id": "tenant_1", | ||
| "authorization": "Bearer 1234", |
There was a problem hiding this comment.
IMHO, this tests nothing. There isn't even a validation to prove that the header actually sends to the configured endpoint, it's too perfunctory.
Use the /echo endpoint to validate the request header, which is sent back as a response header.
Lines 416 to 458 in 485f04d
There was a problem hiding this comment.
Sorry, there is no local setup guide, and I cannot run make init and the local tests successfully, I only conducted actual manual testing.
And I don’t know how to use /echo to verify requests sent to the Loki URL.
There was a problem hiding this comment.
Then, please do this:
- Create route with the following upstream and URI
=== TEST XX: setup route (test auth headers)
--- config
location /t {
content_by_lua_block {
local t = require("lib.test_admin").test
local code, body = t('/apisix/admin/routes/1',
ngx.HTTP_PUT,
[[{
"plugins": {
"loki-logger": {
"endpoint_addrs": ["http://127.0.0.1:1980"],
"endpoint_uri": "/log_request",
"request_headers": {"Authorization": "test1234"},
"batch_max_size": 1
}
},
"upstream": {
"nodes": {
"127.0.0.1:1980": 1
},
"type": "roundrobin"
},
"uri": "/hello"
}]]
)
if code >= 300 then
ngx.status = code
end
ngx.say(body)
}
}
--- response_body
passed
- Send a request to
/helloendpoint to trigger the log, then check the error.log.
=== TEST XX+1: hit route
--- request
GET /hello
--- response_body
hello world
--- error_log
go(): authorization: test1234
The /log_request endpoint used by loki will log the following error.log.
It contains every request header, which obviously includes the one we configured for loki for authentication.
…ode style, update the document.
|
It looks much more reliable and once the tests pass I can approve the change. |
Co-authored-by: Zeping Bai <[email protected]>
membphis
left a comment
There was a problem hiding this comment.
we need a better title for this PR
should focus on new "header" option
|
|
||
|
|
||
| local function send_http_data(conf, log) | ||
| local headers = conf.headers or {} |
There was a problem hiding this comment.
| local headers = conf.headers or {} | |
| local headers = conf.headers or {} | |
| headers = core.table.clone(headers) -- avoid changing the original table `conf` |
authorization attribute for loki-logger (#11420)headers attribute for loki-logger (#11420)

Description
add an attribute/property
authorizationforloki-loggerto enable providing http authorization header for non-local loki service.Which issue(s) this PR fixes:
Fixes #11420
Checklist