rionmonster commented on code in PR #27576:
URL: https://github.com/apache/flink/pull/27576#discussion_r2791300264
##########
flink-metrics/flink-metrics-prometheus/src/test/java/org/apache/flink/metrics/prometheus/PrometheusPushGatewayReporterTest.java:
##########
@@ -68,4 +77,62 @@ void
testConnectToPushGatewayThrowsExceptionWithoutHostInformation() {
assertThatThrownBy(() -> factory.createMetricReporter(metricConfig))
.isInstanceOf(IllegalArgumentException.class);
}
+
+ @Test
+ void testBasicAuthNotEnabledWithoutCredentials() {
+ PrometheusPushGatewayReporterFactory factory = new
PrometheusPushGatewayReporterFactory();
+ MetricConfig metricConfig = new MetricConfig();
+ metricConfig.setProperty(HOST_URL.key(), "http://localhost:9091";);
+ // No authentication configured - should create reporter successfully
without Basic Auth
+ PrometheusPushGatewayReporter reporter =
factory.createMetricReporter(metricConfig);
+ assertThat(reporter).isNotNull();
+
assertThat(reporter.hostUrl.toString()).isEqualTo("http://localhost:9091";);
+ }
+
+ @Test
+ void testBasicAuthNotEnabledWithOnlyUsername() {
+ PrometheusPushGatewayReporterFactory factory = new
PrometheusPushGatewayReporterFactory();
+ MetricConfig metricConfig = new MetricConfig();
+ metricConfig.setProperty(HOST_URL.key(), "http://localhost:9091";);
+ metricConfig.setProperty(USERNAME.key(), "flink-user");
+ // Only username configured - should create reporter successfully
without Basic Auth
+ PrometheusPushGatewayReporter reporter =
factory.createMetricReporter(metricConfig);
+ assertThat(reporter).isNotNull();
+ // Verify warning log is emitted
+ assertThat(loggerExtension.getMessages())
+ .anyMatch(
+ msg ->
+ msg.contains("Both username and password must
be configured")
+ && msg.contains("Currently only
username is configured"));
+ }
+
+ @Test
+ void testBasicAuthNotEnabledWithOnlyPassword() {
+ PrometheusPushGatewayReporterFactory factory = new
PrometheusPushGatewayReporterFactory();
+ MetricConfig metricConfig = new MetricConfig();
+ metricConfig.setProperty(HOST_URL.key(), "http://localhost:9091";);
+ metricConfig.setProperty(PASSWORD.key(), "flink-password");
+ // Only password configured - should create reporter successfully
without Basic Auth
+ PrometheusPushGatewayReporter reporter =
factory.createMetricReporter(metricConfig);
+ assertThat(reporter).isNotNull();
+ // Verify warning log is emitted
+ assertThat(loggerExtension.getMessages())
+ .anyMatch(
+ msg ->
+ msg.contains("Both username and password must
be configured")
+ && msg.contains("Currently only
password is configured"));
+ }
+
+ @Test
+ void testBasicAuthEnabledWithBothCredentials() {
+ PrometheusPushGatewayReporterFactory factory = new
PrometheusPushGatewayReporterFactory();
+ MetricConfig metricConfig = new MetricConfig();
+ metricConfig.setProperty(HOST_URL.key(), "http://localhost:9091";);
+ metricConfig.setProperty(USERNAME.key(), "flink-user");
+ metricConfig.setProperty(PASSWORD.key(), "flink-password");
+ // Both username and password configured - Basic Auth should be enabled
+ PrometheusPushGatewayReporter reporter =
factory.createMetricReporter(metricConfig);
Review Comment:
I think we need to add another assertion to this test to verify that
authentication _was_ actually enabled. We may want to consider following a
pattern similar to `hostUrl` to expose it for testing (e.g. @VisibleForTesting
package-private boolean field), something like `basicAuthEnabled` and then we
could simply check that property here:
```
assertThat(reporter.basicAuthEnabled).isTrue();
```
Either that or rely on a reflection based approach.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
