openstack/charm-keystone-saml-mellon
Code Issues Proposed changes

Address review requests

Browse Source
This commit is contained in:
David Ames 2019-03-26 23:35:29 +00:00
parent 403fa91549
commit 63b74b1d03
4 changed files with 24 additions and 84 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

34
src/lib/charm/openstack/keystone_saml_mellon.py
View File

@ -22,10 +22,6 @@ import charmhelpers.contrib.openstack.templating as os_templating
import charms_openstack.charm
import charms_openstack.adapters
from charms.reactive.relations import (
endpoint_from_flag,
)
import os
import subprocess
@ -42,8 +38,6 @@ CONFIGS = (IDP_METADATA, SP_METADATA, SP_PRIVATE_KEY,
'sp-pk.{}.pem',
'sp-location.{}.conf']]
KEYSTONE_FID_ENDPOINT = "keystone-fid-service-provider.connected"
class KeystoneSAMLMellonConfigurationAdapter(
charms_openstack.adapters.ConfigurationAdapter):
@ -230,12 +224,12 @@ class KeystoneSAMLMellonCharm(charms_openstack.charm.OpenStackCharm):
# ownership.
group = 'www-data'
@property
def restart_map(self):
_map = {}
for config in CONFIGS:
_map[config] = []
return _map
restart_map = {
IDP_METADATA: [],
SP_METADATA: [],
SP_PRIVATE_KEY: [],
SP_LOCATION_CONFIG: [],
}
def configuration_complete(self):
"""Determine whether sufficient configuration has been provided
@ -286,17 +280,15 @@ class KeystoneSAMLMellonCharm(charms_openstack.charm.OpenStackCharm):
# ensure that a directory we need is there
ch_host.mkdir('/etc/apache2/mellon', perms=dperms, owner=owner,
group=group)
_template_map = {
os.path.basename(self.options.sp_metadata_file): 'mellon-sp-metadata.xml',
os.path.basename(self.options.sp_location_config): 'apache-mellon-location.conf',
}
# idp-metadata.xml and sp-private-key are rendered purely from resources
#self.render_with_interfaces(args, template_map=_template_map)
self.render_configs(self.string_templates.keys())
# For now the template name does not match the basename(file_name)
# So not using self.render_with_interfaces(args)
# For now the template name does not match
# basename(file_path/file_name). This is necessary to enable multiple
# instantiations of keystone-saml-mellon using service_name() in the
# file names. So not using self.render_with_interfaces(args)
# TODO: Make a mapping mechanism between target and source templates
# in charms.openstack
core.templating.render(
source='mellon-sp-metadata.xml',
template_loader=os_templating.get_loader(
@ -320,7 +312,7 @@ class KeystoneSAMLMellonCharm(charms_openstack.charm.OpenStackCharm):
)
def remove_config(self):
for f in CONFIGS:
for f in self.restart_map.keys():
if os.path.exists(f):
os.unlink(f)

22
src/reactive/keystone_saml_mellon_handlers.py
View File

@ -13,16 +13,13 @@
# See the License for the specific language governing permissions and
# limitations under the License.
# import to trigger openstack charm metaclass init
import charm.openstack.keystone_saml_mellon as keystone_saml_mellon # noqa
import charms_openstack.bus
charms_openstack.bus.discover()
import charms_openstack.charm as charm
import charms.reactive as reactive
from charms.reactive.relations import (
endpoint_from_flag,
)
charm.use_defaults(
'charm.installed',
'update-status',
@ -41,9 +38,6 @@ def keystone_departed():
@reactive.when('keystone-fid-service-provider.connected')
def publish_sp_fid(fid_sp):
# don't always have a relation context - obtain from the flag
#fid_sp = endpoint_from_flag(
# keystone_saml_mellon.KEYSTONE_FID_ENDPOINT)
with charm.provide_charm_instance() as charm_instance:
fid_sp.publish(charm_instance.options.protocol_name,
charm_instance.options.remote_id_attribute)
@ -51,24 +45,18 @@ def publish_sp_fid(fid_sp):
@reactive.when('keystone-fid-service-provider.available')
def render_config(fid_sp):
# don't always have a relation context - obtain from the flag
#fid_sp = endpoint_from_flag(
# keystone_saml_mellon.KEYSTONE_FID_ENDPOINT)
with charm.provide_charm_instance() as charm_instance:
if charm_instance.configuration_complete():
charm_instance.render_config(fid_sp)
# Trigger keystone restart. The relation is container-scoped
# so a per-unit db of a remote unit will only contain a nonce
# of a single subordinate
if reactive.any_file_changed(keystone_saml_mellon.CONFIGS):
if reactive.any_file_changed(charm_instance.restart_map.keys()):
fid_sp.request_restart()
@reactive.when('websso-fid-service-provider.connected')
def configure_websso():
# don't always have a relation context - obtain from the flag
websso_fid_sp = endpoint_from_flag(
'websso-fid-service-provider.connected')
def configure_websso(websso_fid_sp):
with charm.provide_charm_instance() as charm_instance:
if charm_instance.configuration_complete():
# publish config options for all remote units of a given rel

14
unit_tests/test_keystone_saml_mellon_handlers.py
View File

@ -68,10 +68,7 @@ class TestKeystoneSAMLMellonHandlers(test_utils.PatchHelper):
self.patch_object(handlers.reactive, 'any_file_changed',
new=mock.MagicMock())
self.patch_object(handlers, 'endpoint_from_flag',
new=mock.MagicMock())
self.endpoint = mock.MagicMock()
self.endpoint_from_flag.return_value = self.endpoint
self.protocol_name = "mapped"
self.remote_id_attribute = "https://samltest.id"
@ -99,7 +96,7 @@ class TestKeystoneSAMLMellonHandlers(test_utils.PatchHelper):
self.keystone_saml_mellon_charm.remove_config.assert_called_once_with()
def test_publish_sp_fid(self):
handlers.publish_sp_fid()
handlers.publish_sp_fid(self.endpoint)
self.endpoint.publish.assert_called_once_with(
self.protocol_name, self.remote_id_attribute)
@ -109,17 +106,18 @@ class TestKeystoneSAMLMellonHandlers(test_utils.PatchHelper):
(self.keystone_saml_mellon_charm
.configuration_complete.return_value) = True
handlers.render_config()
self.keystone_saml_mellon_charm.render_config.assert_called_once_with()
handlers.render_config(self.endpoint)
self.keystone_saml_mellon_charm.render_config.assert_called_once_with(
self.endpoint)
self.endpoint.request_restart.assert_not_called()
# Restart
self.any_file_changed.return_value = True
handlers.render_config()
handlers.render_config(self.endpoint)
self.endpoint.request_restart.assert_called_once_with()
def test_configure_websso(self):
handlers.configure_websso()
handlers.configure_websso(self.endpoint)
self.endpoint.publish.assert_called_once_with(
self.protocol_name, self.idp_name, self.user_facing_name)

38
unit_tests/test_lib_charm_openstack_keystone_saml_mellon.py
View File

@ -45,10 +45,7 @@ class Helper(test_utils.PatchHelper):
self.patch_release(
keystone_saml_mellon.KeystoneSAMLMellonCharm.release)
self.patch_object(keystone_saml_mellon, 'endpoint_from_flag',
new=mock.MagicMock())
self.endpoint = mock.MagicMock()
self.endpoint_from_flag.return_value = self.endpoint
self.idp_name = "samltest"
self.protocol_name = "mapped"
@ -138,22 +135,6 @@ class TestKeystoneSAMLMellonConfigurationAdapter(Helper):
self.assertEqual(
ksmca.sp_private_key_file, keystone_saml_mellon.SP_PRIVATE_KEY)
def test_keystone_host(self):
ksmca = keystone_saml_mellon.KeystoneSAMLMellonConfigurationAdapter()
self.assertEqual(ksmca.keystone_host, self.hostname)
def test_keystone_port(self):
ksmca = keystone_saml_mellon.KeystoneSAMLMellonConfigurationAdapter()
self.assertEqual(ksmca.keystone_port, self.port)
def test_keystone_tls_enabled(self):
ksmca = keystone_saml_mellon.KeystoneSAMLMellonConfigurationAdapter()
self.assertEqual(ksmca.tls_enabled, self.tls_enabled)
def test_keystone_base_url(self):
ksmca = keystone_saml_mellon.KeystoneSAMLMellonConfigurationAdapter()
self.assertEqual(ksmca.keystone_base_url, self.base_url)
def test_sp_idp_path(self):
ksmca = keystone_saml_mellon.KeystoneSAMLMellonConfigurationAdapter()
self.assertEqual(
@ -195,25 +176,6 @@ class TestKeystoneSAMLMellonConfigurationAdapter(Helper):
ksmca.sp_logout_path,
'{}/logout'.format(ksmca.mellon_endpoint_path))
def test_sp_auth_url(self):
ksmca = keystone_saml_mellon.KeystoneSAMLMellonConfigurationAdapter()
self.assertEqual(
ksmca.sp_auth_url,
'{}{}'.format(ksmca.keystone_base_url, ksmca.sp_auth_path))
def test_sp_logout_url(self):
ksmca = keystone_saml_mellon.KeystoneSAMLMellonConfigurationAdapter()
self.assertEqual(
ksmca.sp_logout_url,
'{}{}'.format(ksmca.keystone_base_url, ksmca.sp_logout_path))
def test_sp_post_response_url(self):
ksmca = keystone_saml_mellon.KeystoneSAMLMellonConfigurationAdapter()
self.assertEqual(
ksmca.sp_post_response_url,
'{}{}'.format(ksmca.keystone_base_url,
ksmca.sp_post_response_path))
def test_mellon_subject_confirmation_data_address_check(self):
ksmca = keystone_saml_mellon.KeystoneSAMLMellonConfigurationAdapter()
self.assertEqual(