Skip to content

Commit 1eb4247

Browse files
committed
[UT] Align BE handler auth test with AuthN-only probes; add getFeMetrics FE coverage
- BE: update handler_required_privilege_test probe/prometheus case to the AuthN-only contract (need_auth()==true, required_privilege()==NONE). The old _skip_framework_auth assertions broke after the probe handlers were flipped to need_auth()==true. - BE: drop the now-redundant observability_probes_are_authn_only from metrics_action_test (handler_required_privilege_test is the canonical handler-auth test) and its unused includes. - FE: add FrontendServiceImplTest.testGetFeMetrics covering getFeMetrics, the previously uncovered method that dragged FE incremental coverage below the threshold. Signed-off-by: atman <11624213+iatman@users.noreply.github.com>
1 parent 98e2b6e commit 1eb4247

3 files changed

Lines changed: 22 additions & 26 deletions

File tree

be/test/http/handler_required_privilege_test.cpp

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -194,10 +194,16 @@ TEST(BeHandlerNeedAuthTest, download_action_error_log_requires_framework_auth) {
194194
EXPECT_TRUE(error_log.need_auth());
195195
}
196196

197-
TEST(BeHandlerNeedAuthTest, probe_and_prometheus_endpoints_skip_framework_auth) {
198-
EXPECT_FALSE(HealthAction(nullptr).need_auth());
199-
EXPECT_FALSE(MetricsAction(nullptr).need_auth());
200-
EXPECT_FALSE(MemoryMetricsAction(fake_global_env()).need_auth());
197+
// Probe / Prometheus endpoints (/api/health, /metrics, /metrics/memory) are AuthN-only:
198+
// historically anonymous, now gated by `config::enable_http_auth` -- need_auth() == true
199+
// (the injected verifier short-circuits when the flag is off) with no extra privilege.
200+
TEST(BeHandlerNeedAuthTest, probe_and_prometheus_endpoints_are_authn_only) {
201+
EXPECT_TRUE(HealthAction(nullptr).need_auth());
202+
EXPECT_EQ(Priv::NONE, HealthAction(nullptr).required_privilege());
203+
EXPECT_TRUE(MetricsAction(nullptr).need_auth());
204+
EXPECT_EQ(Priv::NONE, MetricsAction(nullptr).required_privilege());
205+
EXPECT_TRUE(MemoryMetricsAction(fake_global_env()).need_auth());
206+
EXPECT_EQ(Priv::NONE, MemoryMetricsAction(fake_global_env()).required_privilege());
201207
}
202208

203209
TEST(BeHandlerNeedAuthTest, stream_load_uses_builtin_fe_auth_flow) {

be/test/http/metrics_action_test.cpp

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -39,12 +39,9 @@
3939
#include "base/metrics.h"
4040
#include "common/config_metrics_fwd.h"
4141
#include "common/metrics/process_metrics_registry.h"
42-
#include "http/action/health_action.h"
43-
#include "http/action/memory_metrics_action.h"
4442
#include "http/http_channel.h"
4543
#include "http/http_request.h"
4644
#include "http/http_response.h"
47-
#include "runtime/env/global_env.h"
4845
#ifdef USE_STAROS
4946
#include "metrics/metrics.h"
5047
#endif
@@ -155,25 +152,6 @@ TEST_F(MetricsActionTest, prometheus_no_name) {
155152
action.handle(&request);
156153
}
157154

158-
// The public observability probes (`/metrics`, `/api/health`, `/metrics/memory`) are now
159-
// in the `enable_http_auth` scope at AuthN-only level: they require Basic identity (the
160-
// injected verifier short-circuits when the flag is off) but declare no extra privilege,
161-
// so identity alone is sufficient.
162-
TEST_F(MetricsActionTest, observability_probes_are_authn_only) {
163-
ProcessMetricsRegistry registry("test");
164-
MetricsAction metrics_action(&registry, &mock_send_reply);
165-
EXPECT_TRUE(metrics_action.need_auth());
166-
EXPECT_EQ(HttpHandler::RequiredPrivilege::NONE, metrics_action.required_privilege());
167-
168-
HealthAction health_action(nullptr);
169-
EXPECT_TRUE(health_action.need_auth());
170-
EXPECT_EQ(HttpHandler::RequiredPrivilege::NONE, health_action.required_privilege());
171-
172-
MemoryMetricsAction memory_action(*GlobalEnv::GetInstance());
173-
EXPECT_TRUE(memory_action.need_auth());
174-
EXPECT_EQ(HttpHandler::RequiredPrivilege::NONE, memory_action.required_privilege());
175-
}
176-
177155
TEST_F(MetricsActionTest, prometheus_output_with_table_metrics) {
178156
ProcessMetricsRegistry registry("test");
179157
enable_table_metrics(&registry);

fe/fe-core/src/test/java/com/starrocks/service/FrontendServiceImplTest.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,8 @@
6767
import com.starrocks.thrift.TDescribeTableParams;
6868
import com.starrocks.thrift.TDescribeTableResult;
6969
import com.starrocks.thrift.TExecPlanFragmentParams;
70+
import com.starrocks.thrift.TFeMetricsRequest;
71+
import com.starrocks.thrift.TFeMetricsResult;
7072
import com.starrocks.thrift.TFeResult;
7173
import com.starrocks.thrift.TFileType;
7274
import com.starrocks.thrift.TGetDictQueryParamRequest;
@@ -212,6 +214,16 @@ public void testMvReportIsCompatibilityNoOp() throws TException {
212214
Assertions.assertNotNull(response);
213215
}
214216

217+
@Test
218+
public void testGetFeMetrics() throws TException {
219+
// information_schema.fe_metrics is served over this RPC instead of scraping HTTP /metrics.
220+
// It returns the same JSON payload as /metrics?type=json with an OK status.
221+
FrontendServiceImpl impl = new FrontendServiceImpl(exeEnv);
222+
TFeMetricsResult result = impl.getFeMetrics(new TFeMetricsRequest());
223+
Assertions.assertEquals(TStatusCode.OK, result.getStatus().getStatus_code());
224+
Assertions.assertNotNull(result.getJson_metrics());
225+
}
226+
215227
@Test
216228
public void testCheckAuthRejectsMissingCredentials() throws TException {
217229
FrontendServiceImpl impl = new FrontendServiceImpl(exeEnv);

0 commit comments

Comments
 (0)