Send keystone tokens to Aetos only
Previously a keystone token was sent with every request, no matter what was on the receiving end. This could apparently pose a security threat. This patch changes the observabilityclient's behavior to send the token only when the receiving endpoint was discovered from the keystone endpoints and the service_type is 'prometheus' and service_name is 'aetos'. Change-Id: I39e84c20ef3ef4e399bf191da6b9c7ad31935fd5 Signed-off-by: Jaromir Wysoglad <jwysogla@redhat.com>
This commit is contained in:
@@ -16,6 +16,7 @@ import os
|
||||
from unittest import mock
|
||||
|
||||
from keystoneauth1 import adapter
|
||||
from keystoneauth1 import discover
|
||||
from keystoneauth1 import session
|
||||
import testtools
|
||||
|
||||
@@ -50,6 +51,11 @@ class GetPrometheusClientTest(testtools.TestCase):
|
||||
super(GetPrometheusClientTest, self).setUp()
|
||||
config_data = 'host: "somehost"\nport: "1234"'
|
||||
self.config_file = mock.mock_open(read_data=config_data)("name", 'r')
|
||||
self.prom_endpoint_data = discover.EndpointData(
|
||||
catalog_url="http://localhost:1234/prometheus",
|
||||
service_type="prometheus",
|
||||
service_name="aetos"
|
||||
)
|
||||
|
||||
def test_get_prometheus_client_from_file(self):
|
||||
with mock.patch.object(metric_utils, 'get_config_file',
|
||||
@@ -102,13 +108,12 @@ class GetPrometheusClientTest(testtools.TestCase):
|
||||
metric_utils.get_prometheus_client)
|
||||
|
||||
def test_get_prometheus_client_from_keystone_http(self):
|
||||
prometheus_endpoint = "http://localhost:1234/prometheus"
|
||||
keystone_session = session.Session()
|
||||
with mock.patch.dict(os.environ, {}), \
|
||||
mock.patch.object(metric_utils, 'get_config_file',
|
||||
return_value=None), \
|
||||
mock.patch.object(adapter.Adapter, 'get_endpoint',
|
||||
return_value=prometheus_endpoint), \
|
||||
mock.patch.object(adapter.Adapter, 'get_endpoint_data',
|
||||
return_value=self.prom_endpoint_data), \
|
||||
mock.patch.object(prometheus_client.PrometheusAPIClient,
|
||||
"__init__", return_value=None) as init_m, \
|
||||
mock.patch.object(prometheus_client.PrometheusAPIClient,
|
||||
@@ -120,13 +125,14 @@ class GetPrometheusClientTest(testtools.TestCase):
|
||||
ca_m.assert_not_called()
|
||||
|
||||
def test_get_prometheus_client_from_keystone_https(self):
|
||||
prometheus_endpoint = "https://localhost:1234/prometheus"
|
||||
self.prom_endpoint_data.catalog_url = \
|
||||
"https://localhost:1234/prometheus"
|
||||
keystone_session = session.Session()
|
||||
with mock.patch.dict(os.environ, {}), \
|
||||
mock.patch.object(metric_utils, 'get_config_file',
|
||||
return_value=None), \
|
||||
mock.patch.object(adapter.Adapter, 'get_endpoint',
|
||||
return_value=prometheus_endpoint), \
|
||||
mock.patch.object(adapter.Adapter, 'get_endpoint_data',
|
||||
return_value=self.prom_endpoint_data), \
|
||||
mock.patch.object(prometheus_client.PrometheusAPIClient,
|
||||
"__init__", return_value=None) as init_m, \
|
||||
mock.patch.object(prometheus_client.PrometheusAPIClient,
|
||||
@@ -138,15 +144,16 @@ class GetPrometheusClientTest(testtools.TestCase):
|
||||
ca_m.assert_called_with(True)
|
||||
|
||||
def test_get_prometheus_client_from_keystone_custom_ca(self):
|
||||
prometheus_endpoint = "https://localhost:1234/prometheus"
|
||||
self.prom_endpoint_data.catalog_url = \
|
||||
"https://localhost:1234/prometheus"
|
||||
keystone_session = session.Session()
|
||||
config_data = 'ca_cert: "ca/path"'
|
||||
config_file = mock.mock_open(read_data=config_data)("name", 'r')
|
||||
with mock.patch.dict(os.environ, {}), \
|
||||
mock.patch.object(metric_utils, 'get_config_file',
|
||||
return_value=config_file), \
|
||||
mock.patch.object(adapter.Adapter, 'get_endpoint',
|
||||
return_value=prometheus_endpoint), \
|
||||
mock.patch.object(adapter.Adapter, 'get_endpoint_data',
|
||||
return_value=self.prom_endpoint_data), \
|
||||
mock.patch.object(prometheus_client.PrometheusAPIClient,
|
||||
"__init__", return_value=None) as init_m, \
|
||||
mock.patch.object(prometheus_client.PrometheusAPIClient,
|
||||
@@ -185,13 +192,14 @@ class GetPrometheusClientTest(testtools.TestCase):
|
||||
)
|
||||
|
||||
def test_get_prometheus_client_from_keystone_ipv6(self):
|
||||
prometheus_endpoint = "http://[2607:5300:201:2000::654]:80/prometheus"
|
||||
self.prom_endpoint_data.catalog_url = \
|
||||
"http://[2607:5300:201:2000::654]:80/prometheus"
|
||||
keystone_session = session.Session()
|
||||
with mock.patch.dict(os.environ, {}), \
|
||||
mock.patch.object(metric_utils, 'get_config_file',
|
||||
return_value=None), \
|
||||
mock.patch.object(adapter.Adapter, 'get_endpoint',
|
||||
return_value=prometheus_endpoint), \
|
||||
mock.patch.object(adapter.Adapter, 'get_endpoint_data',
|
||||
return_value=self.prom_endpoint_data), \
|
||||
mock.patch.object(prometheus_client.PrometheusAPIClient,
|
||||
"__init__", return_value=None) as init_m, \
|
||||
mock.patch.object(prometheus_client.PrometheusAPIClient,
|
||||
|
||||
@@ -54,6 +54,8 @@ def get_prometheus_client(session=None, adapter_options={}):
|
||||
port = None
|
||||
ca_cert = None
|
||||
root_path = ""
|
||||
is_aetos = False
|
||||
|
||||
conf_file = get_config_file()
|
||||
if conf_file is not None:
|
||||
conf = yaml.safe_load(conf_file)
|
||||
@@ -71,8 +73,8 @@ def get_prometheus_client(session=None, adapter_options={}):
|
||||
try:
|
||||
endpoint = adapter.Adapter(
|
||||
session=session, **adapter_options
|
||||
).get_endpoint()
|
||||
parsed_url = parse.urlparse(endpoint)
|
||||
).get_endpoint_data()
|
||||
parsed_url = parse.urlparse(endpoint.catalog_url)
|
||||
host = parsed_url.hostname
|
||||
port = parsed_url.port if parsed_url.port is not None else 80
|
||||
root_path = parsed_url.path.strip('/')
|
||||
@@ -82,6 +84,11 @@ def get_prometheus_client(session=None, adapter_options={}):
|
||||
# so that a custom certificate can be set in the config
|
||||
# file, while the endpoint is retrieved from keystone.
|
||||
ca_cert = True
|
||||
if (endpoint.service_type == "prometheus" and
|
||||
endpoint.service_name == "aetos"):
|
||||
# We know this is Aetos and we can include keystone tokens
|
||||
# when sending requests to it.
|
||||
is_aetos = True
|
||||
except keystone_exception.EndpointNotFound:
|
||||
# NOTE(jwysogla): Don't do anything here. It's still possible
|
||||
# to get the correct endpoint configuration from the env vars.
|
||||
@@ -104,7 +111,14 @@ def get_prometheus_client(session=None, adapter_options={}):
|
||||
"port configuration and endpoint for service"
|
||||
"prometheus not found.")
|
||||
escaped_host = netutils.escape_ipv6(host)
|
||||
client = PrometheusAPIClient(f"{escaped_host}:{port}", session, root_path)
|
||||
if is_aetos:
|
||||
client = PrometheusAPIClient(
|
||||
f"{escaped_host}:{port}", session, root_path
|
||||
)
|
||||
else:
|
||||
client = PrometheusAPIClient(
|
||||
f"{escaped_host}:{port}", None, root_path
|
||||
)
|
||||
if ca_cert is not None:
|
||||
client.set_ca_cert(ca_cert)
|
||||
return client
|
||||
|
||||
@@ -0,0 +1,9 @@
|
||||
---
|
||||
security:
|
||||
- |
|
||||
Modified the observabilityclient to send keystone token with each request
|
||||
only when communicating with Aetos, instead of sending it everytime, which
|
||||
could pose a security risk. This also restricts configuration
|
||||
options for connecting to Aetos to keystone endpoints only. Configuration
|
||||
for Prometheus is unchanged and can still be done through keystone
|
||||
endpoint, through a configuration file or through environment variables.
|
||||
Reference in New Issue
Block a user