SSL | Secure service for Noobaa metric #8673

naveenpaul1 · 2025-01-10T12:28:29Z

Explain the changes

add secure port for promethus service
control prometesu http and https service creation with flags. non-secure service creation can be prevented using flag (ALLOW_HTTP_METRICS) in NC NSFS deployment only, and secure prometheus service will start only if ALLOW_HTTPS_METRICS flag is enabled for all the deployents.
Same S3 ssl certs are used by metrics server
Metrics https_port added in CLI and config for flexibility.

Issues: Fixed #xxx / Gap #xxx

nsfs metrics from metrics port 7004 are only implemented over http. This should be changed to https as default. #8198

Testing Instructions:

start nsfs application
verify metrics server is started by hitting url https://localhost:9443/
try the same with valid local certificate, steps can be found in doc

Doc added/updated
Tests added

Signed-off-by: naveenpaul1 <napaul@redhat.com>

romayalon · 2025-01-14T08:12:11Z

config.js

@@ -907,6 +909,8 @@ config.ENDPOINT_SSL_PORT = Number(process.env.ENDPOINT_SSL_PORT) || 6443;
 config.ENDPOINT_SSL_STS_PORT = Number(process.env.ENDPOINT_SSL_STS_PORT) || (process.env.NC_NSFS_NO_DB_ENV === 'true' ? -1 : 7443);
 config.ENDPOINT_SSL_IAM_PORT = Number(process.env.ENDPOINT_SSL_IAM_PORT) || -1;
 config.ALLOW_HTTP = false;
+config.ALLOW_HTTPS_METRICS = false;


why false by default?

romayalon · 2025-01-14T08:12:43Z

src/cmd/nsfs.js

+    --https_port <port>             (default 6443)   Set the S3 endpoint listening HTTPS port to serve.
+    --https_port_sts <port>         (default -1)     Set the S3 endpoint listening HTTPS port for STS.
+    --https_port_iam <port>         (default -1)     Set the endpoint listening HTTPS port for IAM.
+    --metrics_port       <port>     (default -1)     Set the metrics listening port for prometheus.


indentation of

Add HTTP so it'll be clear like we have in the S3 HTTP port

romayalon · 2025-01-14T08:13:45Z

src/cmd/nsfs.js

+    --https_port_sts <port>         (default -1)     Set the S3 endpoint listening HTTPS port for STS.
+    --https_port_iam <port>         (default -1)     Set the endpoint listening HTTPS port for IAM.
+    --metrics_port       <port>     (default -1)     Set the metrics listening port for prometheus.
+    --metrics_https_port <port>     (default -1)     Set the metrics listening https port for prometheus.


make sure HTTPS is capital letters like the other HTTPS usage

why default is -1?

romayalon · 2025-01-14T08:16:54Z

config.js

@@ -586,6 +586,7 @@ config.MIN_MEMORY_FOR_UPGRADE = 1200 * 1024 * 1024;
 config.SEND_EVENTS_REMOTESYS = true;
 config.PROMETHEUS_ENABLED = true;
 config.PROMETHEUS_SERVER_RETRY_COUNT = Infinity;
+config.PROMETHEUS_SSL_SERVER_RETRY_COUNT = Infinity;


why do we need another one?

romayalon · 2025-01-14T08:31:34Z

src/util/ssl_utils.js

@@ -46,7 +46,8 @@ const certs = {
    S3: new CertInfo(config.S3_SERVICE_CERT_PATH),
    EXTERNAL_DB: new CertInfo(config.EXTERNAL_DB_SERVICE_CERT_PATH),
    STS: new CertInfo(config.STS_SERVICE_CERT_PATH),
-    IAM: new CertInfo(config.IAM_SERVICE_CERT_PATH)
+    IAM: new CertInfo(config.IAM_SERVICE_CERT_PATH),
+    METRICS: new CertInfo(config.S3_SERVICE_CERT_PATH) // metric server will use the S3 cert.


I know you have been told in the design meeting to use the S3 cert but I know @Neon-White had some work on changing STS cert to not re-use the S3 cert due to an issue mentioned in #8123
so per what Ben wrote, we might have an issue with this re-use on containerized env, Ben WDYT?
CC: @nimrod-becker

on the other hand, on non containerized - we currently have the certificates under certificates/ dir, and adding a new certificate will require some changes in the code/structure for allowing different certificates for different services.

Good catch, @romayalon - indeed, if the metrics have their own endpoint (such as metrics.openshift-cluster.com:4123), certificates cannot be reused between different endpoints.

If the metrics will be under the S3 endpoint but will only utilize a different port, reusing the certificate should be fine

@Neon-White thanks Ben, I think you refer to the service k8s object, right? if so I think they are sharing it (service-s3.yaml)

@romayalon Sure thing :) yep, I'm referring to the service. If the metrics are accessed via the same URL that serves S3 actions (for example s3.cluster-address.com:someport) then the reuse is okay.
The problem with the reuse happened because the certificate was meant for the s3 host, but sts is different. If the only difference is the port, there's no need for another certificate (since the host is the same).

yep, thanks Ben! so I looks like we can re-use the S3-cert, but this must be documented

romayalon · 2025-01-14T08:35:24Z

src/util/fork_utils.js

 * @param {number?} metrics_port prometheus metris port.
+ * @param {number?} metrics_ssl_port prometheus metris https port.
+ * @param {number?} count number of workers to start.
+ * @param {string?} nsfs_config_root nsfs configuration path


Suggested change

* @param {number?} metrics_port prometheus metris port.

* @param {number?} metrics_ssl_port prometheus metris https port.

* @param {number?} count number of workers to start.

* @param {string?} nsfs_config_root nsfs configuration path

* @param {number} [metrics_port]

* @param {number} [metrics_ssl_port]

* @param {number} [count] number of workers to start.

* @param {string} [nsfs_config_root] nsfs configuration path

romayalon · 2025-01-14T08:36:19Z

src/server/analytic_services/prometheus_reporting.js

+    while (retry_https_count) {
+        try {
+            if (ssl_port > 0 && config.ALLOW_HTTPS_METRICS) {
+                const ssl_cert_info = await ssl_utils.get_ssl_cert_info('METRICS', nsfs_config_root);


Can we re-use start_server_and_cert() like the other services?

SSL | Secure service for Noobaa metric

0dcf762

Signed-off-by: naveenpaul1 <napaul@redhat.com>

pull-request-size bot added the size/L label Jan 10, 2025

romayalon reviewed Jan 14, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SSL | Secure service for Noobaa metric #8673

SSL | Secure service for Noobaa metric #8673

naveenpaul1 commented Jan 10, 2025

romayalon Jan 14, 2025

romayalon Jan 14, 2025

romayalon Jan 14, 2025

romayalon Jan 14, 2025

romayalon Jan 14, 2025

romayalon Jan 14, 2025

Neon-White Jan 14, 2025 •

edited

Loading

romayalon Jan 14, 2025

Neon-White Jan 14, 2025 •

edited

Loading

romayalon Jan 14, 2025

romayalon Jan 14, 2025

romayalon Jan 14, 2025

SSL | Secure service for Noobaa metric #8673

Are you sure you want to change the base?

SSL | Secure service for Noobaa metric #8673

Conversation

naveenpaul1 commented Jan 10, 2025

Explain the changes

Issues: Fixed #xxx / Gap #xxx

Testing Instructions:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Neon-White Jan 14, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Neon-White Jan 14, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Neon-White Jan 14, 2025 •

edited

Loading

Neon-White Jan 14, 2025 •

edited

Loading