You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by ar...@apache.org on 2023/02/01 18:08:46 UTC
[impala] branch master updated: IMPALA-11890: Expose 'healthz' endpoint in metrics webserver of statestored and catalogd
This is an automated email from the ASF dual-hosted git repository.
arawat pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
The following commit(s) were added to refs/heads/master by this push:
new cda411469 IMPALA-11890: Expose 'healthz' endpoint in metrics webserver of statestored and catalogd
cda411469 is described below
commit cda41146902dfebd101b5906c031f562e3094889
Author: Abhishek Rawat <ar...@cloudera.com>
AuthorDate: Wed Feb 1 03:00:16 2023 -0800
IMPALA-11890: Expose 'healthz' endpoint in metrics webserver of statestored and catalogd
IMPALA-10010 added support for an unsecure webserver also known
as metrics webserver, used for exposing low-sensitivity info.
'/healthz' endpoint was added to impalad's metrics webservers. It
makes sense to also add '/healthz' endpoint to catalogd's and
statestored's metrics webserver.
Testing:
- Updated existing fe test 'testMetricsWebserver' to also test access
to '/healthz' endpoint when metrics webserver is enabled for catalogd
and statestored
Change-Id: I738d6ff2c579063007ea5ac9a66dbdcd0683f1a7
Reviewed-on: http://gerrit.cloudera.org:8080/19465
Reviewed-by: Abhishek Rawat <ar...@cloudera.com>
Reviewed-by: Jason Fehr <jf...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
be/src/catalog/catalog-server.cc | 5 ++-
be/src/catalog/catalog-server.h | 4 ++-
be/src/catalog/catalogd-main.cc | 7 ++--
be/src/statestore/statestore.cc | 5 ++-
be/src/statestore/statestore.h | 4 ++-
be/src/statestore/statestored-main.cc | 6 +++-
.../impala/customcluster/LdapWebserverTest.java | 37 +++++++++++++++-------
7 files changed, 50 insertions(+), 18 deletions(-)
diff --git a/be/src/catalog/catalog-server.cc b/be/src/catalog/catalog-server.cc
index cca214bd8..f2a5ffc30 100644
--- a/be/src/catalog/catalog-server.cc
+++ b/be/src/catalog/catalog-server.cc
@@ -347,12 +347,15 @@ Status CatalogServer::Start() {
return Status::OK();
}
-void CatalogServer::RegisterWebpages(Webserver* webserver) {
+void CatalogServer::RegisterWebpages(Webserver* webserver, bool metrics_only) {
Webserver::RawUrlCallback healthz_callback =
[this](const auto& req, auto* data, auto* response) {
return this->HealthzHandler(req, data, response);
};
webserver->RegisterUrlCallback(CATALOG_SERVICE_HEALTH_WEB_PAGE, healthz_callback);
+
+ if (metrics_only) return;
+
webserver->RegisterUrlCallback(CATALOG_WEB_PAGE, CATALOG_TEMPLATE,
[this](const auto& args, auto* doc) { this->CatalogUrlCallback(args, doc); }, true);
webserver->RegisterUrlCallback(CATALOG_OBJECT_WEB_PAGE, CATALOG_OBJECT_TEMPLATE,
diff --git a/be/src/catalog/catalog-server.h b/be/src/catalog/catalog-server.h
index 621bc66b3..605b41bc0 100644
--- a/be/src/catalog/catalog-server.h
+++ b/be/src/catalog/catalog-server.h
@@ -68,7 +68,9 @@ class CatalogServer {
/// Returns OK unless some error occurred in which case the status is returned.
Status Start();
- void RegisterWebpages(Webserver* webserver);
+ /// Registers webpages for the input webserver. If metrics_only is set then only
+ /// '/healthz' page is registered.
+ void RegisterWebpages(Webserver* webserver, bool metrics_only);
/// Returns the Thrift API interface that proxies requests onto the local CatalogService.
const std::shared_ptr<CatalogServiceIf>& thrift_iface() const {
diff --git a/be/src/catalog/catalogd-main.cc b/be/src/catalog/catalogd-main.cc
index 7822a4fdd..026eb29f4 100644
--- a/be/src/catalog/catalogd-main.cc
+++ b/be/src/catalog/catalogd-main.cc
@@ -36,6 +36,7 @@
#include "util/openssl-util.h"
DECLARE_int32(catalog_service_port);
+DECLARE_int32(metrics_webserver_port);
DECLARE_int32(webserver_port);
DECLARE_int32(state_store_subscriber_port);
DECLARE_string(ssl_server_certificate);
@@ -43,7 +44,6 @@ DECLARE_string(ssl_private_key);
DECLARE_string(ssl_private_key_password_cmd);
DECLARE_string(ssl_cipher_list);
DECLARE_string(ssl_minimum_version);
-
#include "common/names.h"
using namespace impala;
@@ -61,7 +61,10 @@ int CatalogdMain(int argc, char** argv) {
CatalogServer catalog_server(daemon_env.metrics());
ABORT_IF_ERROR(catalog_server.Start());
- catalog_server.RegisterWebpages(daemon_env.webserver());
+ catalog_server.RegisterWebpages(daemon_env.webserver(), false);
+ if (FLAGS_metrics_webserver_port > 0) {
+ catalog_server.RegisterWebpages(daemon_env.metrics_webserver(), true);
+ }
std::shared_ptr<TProcessor> processor(
new CatalogServiceProcessor(catalog_server.thrift_iface()));
std::shared_ptr<TProcessorEventHandler> event_handler(
diff --git a/be/src/statestore/statestore.cc b/be/src/statestore/statestore.cc
index a57dc611c..bf2e800a0 100644
--- a/be/src/statestore/statestore.cc
+++ b/be/src/statestore/statestore.cc
@@ -491,12 +491,15 @@ Status Statestore::Init(int32_t state_store_port) {
return Status::OK();
}
-void Statestore::RegisterWebpages(Webserver* webserver) {
+void Statestore::RegisterWebpages(Webserver* webserver, bool metrics_only) {
Webserver::RawUrlCallback healthz_callback =
[this](const auto& req, auto* data, auto* response) {
return this->HealthzHandler(req, data, response);
};
webserver->RegisterUrlCallback("/healthz", healthz_callback);
+
+ if (metrics_only) return;
+
Webserver::UrlCallback topics_callback =
bind<void>(mem_fn(&Statestore::TopicsHandler), this, _1, _2);
webserver->RegisterUrlCallback("/topics", "statestore_topics.tmpl",
diff --git a/be/src/statestore/statestore.h b/be/src/statestore/statestore.h
index 9d70d9cbf..1b43e111a 100644
--- a/be/src/statestore/statestore.h
+++ b/be/src/statestore/statestore.h
@@ -155,7 +155,9 @@ class Statestore : public CacheLineAligned {
const std::vector<TTopicRegistration>& topic_registrations,
RegistrationId* registration_id) WARN_UNUSED_RESULT;
- void RegisterWebpages(Webserver* webserver);
+ /// Registers webpages for the input webserver. If metrics_only is set then only
+ /// '/healthz' page is registered.
+ void RegisterWebpages(Webserver* webserver, bool metrics_only);
/// The main processing loop. Runs infinitely.
void MainLoop();
diff --git a/be/src/statestore/statestored-main.cc b/be/src/statestore/statestored-main.cc
index b7412d459..9e65bf827 100644
--- a/be/src/statestore/statestored-main.cc
+++ b/be/src/statestore/statestored-main.cc
@@ -34,6 +34,7 @@
#include "util/metrics.h"
#include "util/webserver.h"
+DECLARE_int32(metrics_webserver_port);
DECLARE_int32(state_store_port);
DECLARE_int32(webserver_port);
@@ -51,7 +52,10 @@ int StatestoredMain(int argc, char** argv) {
Statestore statestore(daemon_env.metrics());
ABORT_IF_ERROR(statestore.Init(FLAGS_state_store_port));
- statestore.RegisterWebpages(daemon_env.webserver());
+ statestore.RegisterWebpages(daemon_env.webserver(), false);
+ if (FLAGS_metrics_webserver_port > 0) {
+ statestore.RegisterWebpages(daemon_env.metrics_webserver(), true);
+ }
statestore.MainLoop();
return 0;
diff --git a/fe/src/test/java/org/apache/impala/customcluster/LdapWebserverTest.java b/fe/src/test/java/org/apache/impala/customcluster/LdapWebserverTest.java
index a15f9be7a..fb354291f 100644
--- a/fe/src/test/java/org/apache/impala/customcluster/LdapWebserverTest.java
+++ b/fe/src/test/java/org/apache/impala/customcluster/LdapWebserverTest.java
@@ -68,7 +68,8 @@ public class LdapWebserverTest {
WebClient client_ = new WebClient(TEST_USER_1, TEST_PASSWORD_1);
- public void setUp(String extraArgs, String startArgs) throws Exception {
+ public void setUp(String extraArgs, String startArgs, String catalogdArgs,
+ String stateStoredArgs) throws Exception {
String uri =
String.format("ldap://localhost:%s", serverRule.getLdapServer().getPort());
String dn = "cn=#UID,ou=Users,dc=myorg,dc=com";
@@ -80,7 +81,10 @@ public class LdapWebserverTest {
Map<String, String> env = new HashMap<>();
env.put("IMPALA_WEBSERVER_USERNAME", TEST_USER_1);
env.put("IMPALA_WEBSERVER_PASSWORD", TEST_PASSWORD_1);
- int ret = CustomClusterRunner.StartImpalaCluster(impalaArgs, env, startArgs);
+ catalogdArgs = catalogdArgs + " " + impalaArgs;
+ stateStoredArgs = stateStoredArgs + " " + impalaArgs;
+ int ret = CustomClusterRunner.StartImpalaCluster(impalaArgs, catalogdArgs,
+ stateStoredArgs, env, startArgs);
assertEquals(0, ret);
}
@@ -140,7 +144,7 @@ public class LdapWebserverTest {
@Test
public void testWebserver() throws Exception {
- setUp("", "");
+ setUp("", "", "", "");
// start-impala-cluster contacts the webui to confirm the impalads have started, so
// there will already be some successful auth attempts.
verifyMetrics(Range.atLeast(1L), zero, Range.atLeast(1L), zero);
@@ -173,7 +177,8 @@ public class LdapWebserverTest {
+ "--ldap_group_membership_key=uniqueMember "
+ "--ldap_group_class_key=groupOfUniqueNames "
+ "--ldap_bind_dn=%s --ldap_bind_password_cmd='echo -n %s' ",
- TEST_USER_GROUP, TEST_USER_1, TEST_USER_3, TEST_USER_DN_1, TEST_PASSWORD_1), "");
+ TEST_USER_GROUP, TEST_USER_1, TEST_USER_3, TEST_USER_DN_1, TEST_PASSWORD_1),
+ "", "", "");
// start-impala-cluster contacts the webui to confirm the impalads have started, so
// there will already be some successful auth attempts.
verifyMetrics(Range.atLeast(1L), zero, Range.atLeast(1L), zero);
@@ -209,7 +214,9 @@ public class LdapWebserverTest {
@Test
public void testMetricsWebserver() throws Exception {
// Use 'per_impalad_args' to turn the metrics webserver on only for the first impalad.
- setUp("", "--per_impalad_args=--metrics_webserver_port=25030");
+ setUp("", "--per_impalad_args=--metrics_webserver_port=25030 ",
+ "--metrics_webserver_port=25021 ",
+ "--metrics_webserver_port=25011 ");
// Attempt to access the regular webserver without a username/password, should fail.
WebClient noUsername = new WebClient();
String result = noUsername.readContent("/");
@@ -222,12 +229,20 @@ public class LdapWebserverTest {
// Attempt to access the metrics webserver without a username/password.
WebClient noUsernameMetrics = new WebClient(25030);
+ WebClient catalogdMetrics = new WebClient(25021);
+ WebClient statestoredMetrics = new WebClient(25011);
// Should succeed for the metrics endpoints.
for (String endpoint :
new String[] {"/metrics", "/jsonmetrics", "/metrics_prometheus", "/healthz"}) {
result = noUsernameMetrics.readContent(endpoint);
assertFalse(
result, result.contains("Must authenticate with Basic authentication."));
+ result = catalogdMetrics.readContent(endpoint);
+ assertFalse(
+ result, result.contains("Must authenticate with Basic authentication."));
+ result = statestoredMetrics.readContent(endpoint);
+ assertFalse(
+ result, result.contains("Must authenticate with Basic authentication."));
}
for (String endpoint : new String[] {"/varz", "/backends"}) {
@@ -238,7 +253,7 @@ public class LdapWebserverTest {
@Test
public void testWebserverTrustedDomain() throws Exception {
- setUp("--trusted_domain=localhost --trusted_domain_use_xff_header=true", "");
+ setUp("--trusted_domain=localhost --trusted_domain_use_xff_header=true", "", "", "");
// Case 1: Authenticate as 'Test1Ldap' with the right password '12345'
attemptConnection("Basic VGVzdDFMZGFwOjEyMzQ1", "127.0.0.1", false);
@@ -280,7 +295,7 @@ public class LdapWebserverTest {
@Test
public void testWebserverTrustedAuthHeader() throws Exception {
- setUp("--trusted_auth_header=X-Trusted-Proxy-Auth-Header", "");
+ setUp("--trusted_auth_header=X-Trusted-Proxy-Auth-Header", "", "", "");
// Case 1: Authenticate as 'Test1Ldap' with the right password '12345'.
attemptConnection("Basic VGVzdDFMZGFwOjEyMzQ1", null, true);
@@ -320,7 +335,7 @@ public class LdapWebserverTest {
"--jwt_token_auth=true --jwt_validate_signature=true --jwks_file_path=%s "
+ "--jwt_allow_without_tls=true",
jwksFilename),
- "");
+ "", "", "");
// Case 1: Authenticate with valid JWT Token in HTTP header.
String jwtToken =
@@ -352,7 +367,7 @@ public class LdapWebserverTest {
*/
@Test
public void testDisplaySrcUsernameInQueryCause() throws Exception {
- setUp("", "");
+ setUp("", "", "", "");
// Create client
THttpClient transport = new THttpClient("http://localhost:28000");
Map<String, String> headers = new HashMap<String, String>();
@@ -402,7 +417,7 @@ public class LdapWebserverTest {
*/
@Test
public void testSetGLogLevel() throws Exception {
- setUp("", "");
+ setUp("", "", "", "");
// Validate defaults
JSONObject json = client_.jsonGet("/log_level?json");
assertEquals("1", json.get("glog_level"));
@@ -477,7 +492,7 @@ public class LdapWebserverTest {
*/
@Test
public void testSetJavaLogLevel() throws Exception {
- setUp("", "");
+ setUp("", "", "", "");
// Validate defaults
JSONObject json = client_.jsonGet("/log_level?json");
assertEquals("org.apache.impala : DEBUG\n", json.get("get_java_loglevel_result"));