openstack/charm-keystone-openidc
Code Issues Proposed changes

Validate configuration of the charm.

Browse Source 
This change validates the different combination of configuration keys
that the charm expects to be set, and when they are not valid that
bubbles up to the workload status message.
This commit is contained in:
Felipe Reyes 2022-08-19 15:54:12 -04:00
parent 50a8736a4c
commit 1e31982543
3 changed files with 175 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

111
src/charm.py
View File

@ -19,7 +19,7 @@ import logging
import os
import subprocess
from typing import List
from typing import List, Optional
from uuid import uuid4
import ops_openstack.core
@ -38,12 +38,22 @@ from charmhelpers.core import templating
logger = logging.getLogger(__name__)
CONFIG_DIR = '/etc/apache2/openidc'
HTTPS = 'https://'
class KeystoneOpenIDCError(Exception):
pass
class CharmConfigError(KeystoneOpenIDCError):
def __init__(self, msg):
"""Charm configuration error exception sets the unit in blocked state
:param msg: message to be set in the workload status.
"""
self.msg = msg
class KeystoneOpenIDCOptions(ConfigurationAdapter):
def __init__(self, charm_instance):
@ -61,7 +71,7 @@ class KeystoneOpenIDCOptions(ConfigurationAdapter):
return None
@property
def hostname(self) -> str:
def hostname(self) -> Optional[str]:
"""Hostname as advertised by the principal charm."""
data = self._get_principal_data()
try:
@ -86,7 +96,7 @@ class KeystoneOpenIDCOptions(ConfigurationAdapter):
return 'openid'
@property
def scheme(self) -> str:
def scheme(self) -> Optional[str]:
data = self._get_principal_data()
try:
tls_enabled = json.loads(data['tls-enabled'])
@ -95,7 +105,7 @@ class KeystoneOpenIDCOptions(ConfigurationAdapter):
return None
@property
def port(self) -> int:
def port(self) -> Optional[int]:
data = self._get_principal_data()
try:
return json.loads(data['port'])
@ -103,9 +113,8 @@ class KeystoneOpenIDCOptions(ConfigurationAdapter):
return None
@property
def oidc_crypto_passphrase(self) -> str:
def oidc_crypto_passphrase(self) -> Optional[str]:
data = None
relation = self.charm_instance.model.get_relation('cluster')
data = relation.data[self.charm_instance.unit.app]
@ -121,7 +130,7 @@ class KeystoneOpenIDCOptions(ConfigurationAdapter):
return None
@property
def metadata(self):
def provider_metadata(self):
"""Metadata content offered by the Identity Provider.
The content available at the url configured in
@ -130,8 +139,14 @@ class KeystoneOpenIDCOptions(ConfigurationAdapter):
if self.oidc_provider_metadata_url:
logging.info('GETing content from %s',
self.oidc_provider_metadata_url)
r = requests.get(self.oidc_provider_metadata_url)
return r.json()
try:
r = requests.get(self.oidc_provider_metadata_url)
return r.json()
except Exception:
logger.exception(('Failed to GET json content from provider '
'metadata url: %s'),
self.oidc_provider_metadata_url)
return None
else:
logging.info('Metadata was not retrieved since '
'oidc-provider-metadata-url is not set')
@ -209,8 +224,13 @@ class KeystoneOpenIDCCharm(ops_openstack.core.OSBaseCharm):
def _on_keystone_fid_service_provider_relation_joined(self, event):
if not self.is_data_ready():
try:
if not self.is_data_ready():
event.defer()
except CharmConfigError as ex:
self.unit.status = BlockedStatus(ex.msg)
event.defer()
return
self.update_principal_data()
@ -236,9 +256,14 @@ class KeystoneOpenIDCCharm(ops_openstack.core.OSBaseCharm):
def _on_config_changed(self, event):
self._stored.is_started = True
if not self.is_data_ready():
logger.debug(f'relation data is not ready yet, deferring {event}')
event.defer()
try:
if not self.is_data_ready():
logger.debug('relation data is not ready yet, deferring %s',
event)
event.defer()
return
except CharmConfigError as ex:
self.unit.status = BlockedStatus(ex.msg)
return
self.update_config_if_needed()
@ -281,6 +306,8 @@ class KeystoneOpenIDCCharm(ops_openstack.core.OSBaseCharm):
"""Find keys not set that are needed for the charm to work correctly.
:returns: List of configuration keys that need to be set and are not.
:raises CharmConfigError: when a configuration key is set, yet
semantically incorrect.
"""
options = KeystoneOpenIDCOptions(self)
missing_keys = []
@ -288,6 +315,47 @@ class KeystoneOpenIDCCharm(ops_openstack.core.OSBaseCharm):
if getattr(options, key) in [None, '']:
missing_keys.append(key)
if not options.oidc_provider_metadata_url:
# list of configuration keys that need to be set when there is no
# metadata url to discover them.
keys = ['oidc_provider_issuer',
'oidc_provider_auth_endpoint',
'oidc_provider_token_endpoint',
'oidc_provider_token_endpoint_auth',
'oidc_provider_user_info_endpoint',
'oidc_provider_jwks_uri']
values = map(lambda k: getattr(options, k), keys)
if not any(values):
# None of the options is configured, so we inform that metadata
# is missing instead of assuming the user
# wants to use the metadata url.
missing_keys.append('oidc_provider_metadata_url')
elif not all(values):
missing_keys += list(filter(lambda k: not getattr(options, k),
keys))
if options.enable_oauth and options.oidc_provider_metadata_url:
if options.oidc_oauth_verify_jwks_uri:
if not options.oidc_oauth_verify_jwks_uri.startswith(HTTPS):
msg = ('oidc-auth-verify-jwks-uri is not set to a HTTPS '
'endpoint')
logger.error(msg)
raise CharmConfigError(msg)
else:
if not options.oidc_oauth_introspection_endpoint:
try:
endpoint = options.provider_metadata.get(
'introspection_endpoint')
if not endpoint.startswith(HTTPS):
msg = ('The introspection endpoint referenced in '
'the metadata url is not a HTTPS service, '
'which is the only kind scheme valid.')
logger.error(msg)
raise CharmConfigError(msg)
except AttributeError:
missing_keys.append(
'oidc-oauth-introspection-endpoint')
if missing_keys:
logger.debug('Incomplete data: %s', ' '.join(missing_keys))
@ -298,10 +366,19 @@ class KeystoneOpenIDCCharm(ops_openstack.core.OSBaseCharm):
return []
def _check_status(self) -> StatusBase:
if self.is_data_ready():
return ActiveStatus()
else:
return BlockedStatus('incomplete data')
try:
if self.is_data_ready():
return ActiveStatus()
else:
missing_keys = self.find_missing_keys()
if missing_keys:
msg = 'required keys: %s' % ', '.join(missing_keys)
else:
msg = 'incomplete data'
return BlockedStatus(msg)
except CharmConfigError as ex:
return BlockedStatus(ex.msg)
def enable_module(self):
"""Enable oidc Apache module."""

3
test-requirements.txt
View File

@ -2,4 +2,5 @@ coverage
flake8
stestr
git+https://github.com/openstack-charmers/zaza.git#egg=zaza
git+https://github.com/openstack-charmers/zaza-openstack-tests.git#egg=zaza.openstack
git+https://github.com/freyes/zaza-openstack-tests.git@keystone-openidc#egg=zaza.openstack
requests-mock

88
unit_tests/test_charm.py
View File

@ -7,6 +7,7 @@ import uuid
from unittest import mock
import requests_mock
from ops.testing import Harness
sys.path.append('src') # noqa
@ -16,9 +17,12 @@ import charm
logger = logging.getLogger(__name__)
WELL_KNOWN_URL = 'https://example.com/.well-known/openid-configuration'
WELL_KNOWN_URL_INVALID = 'http://example.com/.well-known/openid-configuration'
INTROSPECTION_ENDPOINT_INVALID = 'http://idp.example.com/oauth2'
CRYPTO_PASSPHRASE = '1e19bb8a-a92d-4377-8226-5e8fc475822c'
class TestCharm(unittest.TestCase):
class BaseTestCharm(unittest.TestCase):
def setUp(self):
self.harness = Harness(charm.KeystoneOpenIDCCharm, meta='''
name: keystone-openidc
@ -36,15 +40,18 @@ class TestCharm(unittest.TestCase):
self.addCleanup(self.harness.cleanup)
self.harness.begin()
class TestRelations(BaseTestCharm):
def test_add_relation(self):
self.harness.add_relation('keystone-fid-service-provider', 'keystone')
@mock.patch('charm.uuid4')
@mock.patch('os.fchown')
@mock.patch('os.chown')
def test_render_config_leader(self, chown, fchown, uuid4):
crypto_passphrase = uuid.UUID('1e19bb8a-a92d-4377-8226-5e8fc475822c')
uuid4.return_value = crypto_passphrase
class TestCharm(BaseTestCharm):
def setUp(self):
super().setUp()
# bootstrap the charm
self.crypto_passphrase = uuid.UUID(CRYPTO_PASSPHRASE)
# disable hooks to avoid trigger them implicitly while the relations
# are being setup and the mocks are not in place yet.
@ -66,7 +73,11 @@ class TestCharm(unittest.TestCase):
self.harness.charm.unit.app.name)
self.harness.update_relation_data(
rid, self.harness.charm.unit.app.name,
{'oidc-crypto-passphrase': str(crypto_passphrase)})
{'oidc-crypto-passphrase': str(self.crypto_passphrase)})
@mock.patch('os.fchown')
@mock.patch('os.chown')
def test_render_config_leader(self, chown, fchown):
self.harness.set_leader(True)
with tempfile.TemporaryDirectory() as tmpdir:
with mock.patch("charm.KeystoneOpenIDCCharm.config_dir",
@ -82,6 +93,65 @@ class TestCharm(unittest.TestCase):
self.assertIn(f'OIDCProviderMetadataURL {WELL_KNOWN_URL}',
content)
self.assertIn(
f'OIDCCryptoPassphrase {str(crypto_passphrase)}',
f'OIDCCryptoPassphrase {str(self.crypto_passphrase)}',
content
)
def test_find_missing_keys_no_metadata_url(self):
opts = {
'oidc-provider-metadata-url': '',
}
self.harness.update_config(key_values=opts)
missing_keys = self.harness.charm.find_missing_keys()
missing_keys.sort()
expected = ['oidc_client_id', 'oidc_provider_metadata_url']
expected.sort()
self.assertEqual(missing_keys, expected)
def test_find_missing_keys_manual_configuration(self):
opts = {
'oidc-provider-metadata-url': '',
'oidc-provider-issuer': 'foo',
'oidc-client-id': 'keystone',
}
self.harness.update_config(key_values=opts)
missing_keys = self.harness.charm.find_missing_keys()
missing_keys.sort()
expected = ['oidc_provider_auth_endpoint',
'oidc_provider_token_endpoint',
'oidc_provider_token_endpoint_auth',
'oidc_provider_user_info_endpoint',
'oidc_provider_jwks_uri']
expected.sort()
self.assertEqual(missing_keys, expected)
def test_find_missing_keys_invalid_oidc_oauth_verify_jwks_uri(self):
opts = {
'oidc-provider-metadata-url': WELL_KNOWN_URL,
'oidc-provider-issuer': 'foo',
'oidc-client-id': 'keystone',
'oidc-oauth-verify-jwks-uri': 'http://idp.example.com/jwks'
}
self.harness.update_config(key_values=opts)
self.assertRaises(charm.CharmConfigError,
self.harness.charm.find_missing_keys)
def test_find_missing_keys_invalid_introspection_endpoint(self):
opts = {
'oidc-provider-metadata-url': WELL_KNOWN_URL,
'oidc-provider-issuer': 'foo',
'oidc-client-id': 'keystone',
'oidc-oauth-verify-jwks-uri': 'http://idp.example.com/jwks'
}
well_known_url_content = {
'introspection_endpoint': INTROSPECTION_ENDPOINT_INVALID,
}
self.harness.update_config(key_values=opts)
with requests_mock.Mocker() as m:
m.get(WELL_KNOWN_URL, json=well_known_url_content)
self.assertRaises(charm.CharmConfigError,
self.harness.charm.find_missing_keys)