fgerlits commented on code in PR #1587:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1587#discussion_r1263391342
##########
METRICS.md:
##########
@@ -108,6 +108,15 @@ An agent identifier should also be defined to identify
which agent the metric is
nifi.metrics.publisher.agent.identifier=Agent1
+### Configure Prometheus metrics publisher with SSL
+
+The communication between MiNiFi and the Prometheus server can be encrypted
using SSL. This can be achieved by adding the SSL certificate path (a single
file containing both the certificate and the SSL key) and optionally adding the
root CA path when using a self signed certificate to the minifi.properties
file. Here is an example with the SSL properties:
Review Comment:
The root CA is for the server certificate, isn't it? I would make that
clearer:
```suggestion
The communication between MiNiFi and the Prometheus server can be encrypted
using SSL. This can be achieved by adding the SSL certificate path (a single
file containing both the client certificate and the client SSL key) and
optionally adding the root CA path if the Prometheus server uses a self-signed
certificate, to the minifi.properties file. Here is an example with the SSL
properties:
```
##########
extensions/prometheus/PrometheusExposerWrapper.cpp:
##########
@@ -20,9 +20,27 @@
namespace org::apache::nifi::minifi::extensions::prometheus {
-PrometheusExposerWrapper::PrometheusExposerWrapper(uint32_t port)
- : exposer_(std::to_string(port)) {
- logger_->log_info("Started Prometheus metrics publisher on port %" PRIu32,
port);
+PrometheusExposerWrapper::PrometheusExposerWrapper(const
PrometheusExposerConfig& config)
+ : exposer_(parseExposerConfig(config)) {
+ logger_->log_info("Started Prometheus metrics publisher on port %" PRIu32,
config.port);
Review Comment:
It could be useful to add "with TLS enabled" to this log message if the
config contains a certificate.
##########
docker/test/integration/cluster/containers/PrometheusContainer.py:
##########
@@ -16,13 +16,53 @@
import os
import tempfile
import docker.types
+
from .Container import Container
+from OpenSSL import crypto
+from ssl_utils.SSL_cert_utils import make_cert_without_extended_usage
class PrometheusContainer(Container):
- def __init__(self, feature_context, name, vols, network, image_store,
command=None):
- super().__init__(feature_context, name, 'prometheus', vols, network,
image_store, command)
- prometheus_yml_content = """
+ def __init__(self, feature_context, name, vols, network, image_store,
command=None, ssl=False):
+ engine = "prometheus-ssl" if ssl else "prometheus"
+ super().__init__(feature_context, name, engine, vols, network,
image_store, command)
+ self.ssl = ssl
+ if ssl:
+ prometheus_cert, prometheus_key =
make_cert_without_extended_usage(f"prometheus-{feature_context.id}",
feature_context.root_ca_cert, feature_context.root_ca_key)
+
+ self.root_ca_file = tempfile.NamedTemporaryFile(delete=False)
+
self.root_ca_file.write(crypto.dump_certificate(type=crypto.FILETYPE_PEM,
cert=feature_context.root_ca_cert))
+ self.root_ca_file.close()
+ os.chmod(self.root_ca_file.name, 0o644)
+
+ self.prometheus_cert_file =
tempfile.NamedTemporaryFile(delete=False)
+
self.prometheus_cert_file.write(crypto.dump_certificate(type=crypto.FILETYPE_PEM,
cert=prometheus_cert))
+ self.prometheus_cert_file.close()
+ os.chmod(self.prometheus_cert_file.name, 0o644)
+
+ self.prometheus_key_file =
tempfile.NamedTemporaryFile(delete=False)
+
self.prometheus_key_file.write(crypto.dump_privatekey(type=crypto.FILETYPE_PEM,
pkey=prometheus_key))
+ self.prometheus_key_file.close()
+ os.chmod(self.prometheus_key_file.name, 0o644)
+
+ prometheus_yml_content = """
+global:
+ scrape_interval: 2s
+ evaluation_interval: 15s
+scrape_configs:
+ - job_name: "minifi"
+ static_configs:
+ - targets: ["minifi-cpp-flow-{feature_id}:9936"]
+ scheme: https
+ tls_config:
+ ca_file: /etc/prometheus/certs/root-ca.pem
+""".format(feature_id=self.feature_context.id)
+ self.yaml_file = tempfile.NamedTemporaryFile(delete=False)
+ self.yaml_file.write(prometheus_yml_content.encode())
+ self.yaml_file.close()
+ os.chmod(self.yaml_file.name, 0o644)
+ else:
+ prometheus_yml_content = """
Review Comment:
This looks a bit confusing to me. I think setting something like
`extra_ssl_settings` to either something or nothing, and then having a single
```python
prometheus_yml_content = """
global:
scrape_interval: 2s
evaluation_interval: 15s
scrape_configs:
- job_name: "minifi"
static_configs:
- targets: ["minifi-cpp-flow-{feature_id}:9936"]
{extra_ssl_settings}
""".format(...)
```
would be more readable.
--
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]
