Marton Greber has posted comments on this change. ( http://gerrit.cloudera.org:8080/24278 )
Change subject: KUDU-3693: Add bearer token auth for Prometheus ...................................................................... Patch Set 5: (3 comments) http://gerrit.cloudera.org:8080/#/c/24278/5/src/kudu/server/webserver-test.cc File src/kudu/server/webserver-test.cc: http://gerrit.cloudera.org:8080/#/c/24278/5/src/kudu/server/webserver-test.cc@609 PS5, Line 609: curl_.FetchURL(url_, &buf_); > It seems I overlooked this in prior review iterations, but I don't see any Ah yes, sorry about that, corrected it now. http://gerrit.cloudera.org:8080/#/c/24278/5/src/kudu/server/webserver-test.cc@614 PS5, Line 614: > One more scenario to validate is a request with invalid SPNEGO authn creds Added a new test TestInvalidSpnegoDeniedOnOtherPaths: When spnego is on, and Kudu is configured with prometheus bearer token cmd: a bad Negotiate token returns 401 on non prom endpoint and does not contain bearer challenge. IIUC we do not handle 2 Authorization headers: const char* authz_header = sq_get_header(connection, kAuthorizationHeader); -> // This is a helper function. It traverses request_info->http_headers array, // and if the header is present in the array, returns its value. If it is // not present, NULL is returned. const char *sq_get_header(const struct sq_connection *, const char *name); this is why i opted for this type of test. http://gerrit.cloudera.org:8080/#/c/24278/5/src/kudu/server/webserver.cc File src/kudu/server/webserver.cc: http://gerrit.cloudera.org:8080/#/c/24278/5/src/kudu/server/webserver.cc@673 PS5, Line 673: if (authz_header != nullptr && : strncmp(authz_header, kBearerPrefix, kBearerPrefixLen) == 0) { : const char* provided_token = authz_header + kBearerPrefixLen; : if (!ConstantTimeEquals(provided_token, strlen(provided_token), : prometheus_token_.c_str(), : prometheus_token_.size())) { : return reject_bearer("invalid bearer token"); : } > A note on the HTTP OPTIONS method handling: it seems the current implementa Yes indeed Prometheus does not use it according to my understanding. Since I already had to revv the patch, I fixed the OPTIONS handling and added a small test for it. -- To view, visit http://gerrit.cloudera.org:8080/24278 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If8169196a2c49d64865095f3bc6da1d3bfbcecfb Gerrit-Change-Number: 24278 Gerrit-PatchSet: 5 Gerrit-Owner: Marton Greber <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Gabriella Lotz <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber <[email protected]> Gerrit-Reviewer: Zoltan Chovan <[email protected]> Gerrit-Reviewer: Zoltan Martonka <[email protected]> Gerrit-Comment-Date: Thu, 14 May 2026 10:51:08 +0000 Gerrit-HasComments: Yes
