Merge pull request #10 from thedac/reactive-standards

Standardize reactive practices
This commit is contained in:
Frode Nordahl 2019-03-28 13:59:13 +01:00 committed by GitHub
commit 20afb48df7
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 104 additions and 280 deletions

2
.gitignore vendored
View File

@ -4,3 +4,5 @@
*.pyc
build
src/tests/bundles/overlays/local-charm-overlay.yaml.j2
interfaces
layers

View File

@ -47,6 +47,7 @@ In a bundle:
idp-name: 'samltest'
protocol-name: 'mapped'
user-facing-name: "samltest.id'
nameid-formats="urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress"
resources:
idp-metadata: "./idp-metadata.xml"
sp-signing-keyinfo: "./sp-keyinfo.xml"

View File

@ -16,10 +16,8 @@
import charmhelpers.core as core
import charmhelpers.core.host as ch_host
import charmhelpers.core.hookenv as hookenv
import charmhelpers.core.unitdata as unitdata
import charmhelpers.contrib.openstack.templating as os_templating
import charmhelpers.contrib.openstack.utils as os_utils
import charms_openstack.charm
import charms_openstack.adapters
@ -31,14 +29,6 @@ from lxml import etree
from cryptography.hazmat.backends import default_backend
from cryptography.hazmat.primitives import serialization
# release detection is done via keystone package given that
# openstack-origin is not present in the subordinate charm
# see https://github.com/juju/charm-helpers/issues/83
from charms_openstack.charm.core import (
register_os_release_selector
)
OPENSTACK_RELEASE_KEY = 'charmers.openstack-release-version'
CONFIGS = (IDP_METADATA, SP_METADATA, SP_PRIVATE_KEY,
SP_LOCATION_CONFIG,) = [
os.path.join('/etc/apache2/mellon',
@ -49,21 +39,6 @@ CONFIGS = (IDP_METADATA, SP_METADATA, SP_PRIVATE_KEY,
'sp-location.{}.conf']]
@register_os_release_selector
def select_release():
"""Determine the release based on the keystone package version.
Note that this function caches the release after the first install so
that it doesn't need to keep going and getting it from the package
information.
"""
release_version = unitdata.kv().get(OPENSTACK_RELEASE_KEY, None)
if release_version is None:
release_version = os_utils.os_release('keystone')
unitdata.kv().set(OPENSTACK_RELEASE_KEY, release_version)
return release_version
class KeystoneSAMLMellonConfigurationAdapter(
charms_openstack.adapters.ConfigurationAdapter):
@ -101,24 +76,6 @@ class KeystoneSAMLMellonConfigurationAdapter(
def sp_location_config(self):
return SP_LOCATION_CONFIG
@property
def keystone_host(self):
return unitdata.kv().get('hostname')
@property
def keystone_port(self):
return unitdata.kv().get('port')
@property
def tls_enabled(self):
return unitdata.kv().get('tls-enabled')
@property
def keystone_base_url(self):
scheme = 'https' if self.tls_enabled else 'http'
return ('{}://{}:{}'.format(scheme, self.keystone_host,
self.keystone_port))
@property
def sp_idp_path(self):
return ('/v3/OS-FEDERATION/identity_providers/{}'
@ -158,21 +115,6 @@ class KeystoneSAMLMellonConfigurationAdapter(
def sp_logout_path(self):
return '{}/logout'.format(self.mellon_endpoint_path)
@property
def sp_auth_url(self):
return '{}{}'.format(self.keystone_base_url,
self.sp_auth_path)
@property
def sp_logout_url(self):
return '{}{}'.format(self.keystone_base_url,
self.sp_logout_path)
@property
def sp_post_response_url(self):
return '{}{}'.format(self.keystone_base_url,
self.sp_post_response_path)
@property
def mellon_subject_confirmation_data_address_check(self):
return ('On' if self.subject_confirmation_data_address_check
@ -256,6 +198,13 @@ class KeystoneSAMLMellonCharm(charms_openstack.charm.OpenStackCharm):
# First release supported
release = 'mitaka'
release_pkg = 'keystone-common'
# Required relations
required_relations = [
'keystone-fid-service-provider',
'websso-fid-service-provider']
# List of packages to install for this charm
packages = ['libapache2-mod-auth-mellon']
@ -275,6 +224,13 @@ class KeystoneSAMLMellonCharm(charms_openstack.charm.OpenStackCharm):
# ownership.
group = 'www-data'
restart_map = {
IDP_METADATA: [],
SP_METADATA: [],
SP_PRIVATE_KEY: [],
SP_LOCATION_CONFIG: [],
}
def configuration_complete(self):
"""Determine whether sufficient configuration has been provided
via charm config options and resources.
@ -292,21 +248,22 @@ class KeystoneSAMLMellonCharm(charms_openstack.charm.OpenStackCharm):
return all(required_config.values())
def assess_status(self):
"""Determine the current application status for the charm"""
hookenv.application_version_set(self.application_version)
def custom_assess_status_check(self):
"""Custom asses status.
Check the configuration is complete.
"""
if not self.configuration_complete():
errors = [
'{}: {}'.format(k, v)
for k, v in self.options.validation_errors.items() if v]
for k, v in self.options.validation_errors.items()]
status_msg = 'Configuration is incomplete. {}'.format(
','.join(errors))
hookenv.status_set('blocked', status_msg)
else:
hookenv.status_set('active',
'Unit is ready')
return 'blocked', status_msg
# Nothing to report
return None, None
def render_config(self):
def render_config(self, *args):
"""
Render Service Provider configuration file to be used by Apache
and provided to idP out of band to establish mutual trust.
@ -323,14 +280,21 @@ 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)
self.render_configs(self.string_templates.keys())
# 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(
'templates/', self.release),
target=self.options.sp_metadata_file,
context=self.adapters_instance,
context=self.adapters_class(args, charm_instance=self),
owner=owner,
group=group,
perms=fileperms
@ -341,14 +305,14 @@ class KeystoneSAMLMellonCharm(charms_openstack.charm.OpenStackCharm):
template_loader=os_templating.get_loader(
'templates/', self.release),
target=self.options.sp_location_config,
context=self.adapters_instance,
context=self.adapters_class(args, charm_instance=self),
owner=owner,
group=group,
perms=fileperms
)
def remove_config(self):
for f in CONFIGS:
for f in self.restart_map.keys():
if os.path.exists(f):
os.unlink(f)

View File

@ -13,51 +13,20 @@
# See the License for the specific language governing permissions and
# limitations under the License.
import uuid
import charms_openstack.bus
charms_openstack.bus.discover()
# import to trigger openstack charm metaclass init
import charm.openstack.keystone_saml_mellon # noqa
import charms_openstack.charm as charm
import charms.reactive as reactive
import charms.reactive.flags as flags
import charmhelpers.core.unitdata as unitdata
from charms.reactive.relations import (
endpoint_from_flag,
)
charm.use_defaults(
'charm.installed',
'update-status')
# if config has been changed we need to re-evaluate flags
# config.changed is set and cleared (atexit) in layer-basic
flags.register_trigger(when='config.changed',
clear_flag='config.rendered')
flags.register_trigger(when='upgraded', clear_flag='config.rendered')
flags.register_trigger(when='config.changed',
clear_flag='config.complete')
flags.register_trigger(
when='endpoint.keystone-fid-service-provider.changed',
clear_flag='keystone-data.complete'
)
'update-status',
'upgrade-charm')
@reactive.hook('upgrade-charm')
def default_upgrade_charm():
"""Default handler for the 'upgrade-charm' hook.
This calls the charm.singleton.upgrade_charm() function as a default.
"""
reactive.set_state('upgraded')
# clear the upgraded state once config.rendered is set again
flags.register_trigger(when='config.rendered', clear_flag='upgraded')
@reactive.when_not('endpoint.keystone-fid-service-provider.joined')
@reactive.when_not('keystone-fid-service-provider.connected')
def keystone_departed():
"""
Service restart should be handled on the keystone side
@ -67,69 +36,34 @@ def keystone_departed():
charm_instance.remove_config()
@reactive.when('endpoint.keystone-fid-service-provider.joined')
@reactive.when_not('config.complete')
def config_changed():
@reactive.when('keystone-fid-service-provider.connected')
def publish_sp_fid(fid_sp):
with charm.provide_charm_instance() as charm_instance:
if charm_instance.configuration_complete():
flags.set_flag('config.complete')
@reactive.when('endpoint.keystone-fid-service-provider.joined')
@reactive.when_not('keystone-data.complete')
def keystone_data_changed(fid_sp):
primary_data = fid_sp.all_joined_units[0].received
if primary_data:
hostname = primary_data.get('hostname')
port = primary_data.get('port')
tls_enabled = primary_data.get('tls-enabled')
# a basic check on the fact that keystone provided us with
# hostname and port information
if hostname and port:
# save hostname and port data in local storage for future
# use - in case config is incomplete but a relation is
# we need to store this across charm hook invocations
unitdb = unitdata.kv()
unitdb.set('hostname', hostname)
unitdb.set('port', port)
unitdb.set('tls-enabled', tls_enabled)
flags.set_flag('keystone-data.complete')
@reactive.when('endpoint.keystone-fid-service-provider.joined')
@reactive.when('config.complete')
@reactive.when('keystone-data.complete')
@reactive.when_not('config.rendered')
def render_config():
# don't always have a relation context - obtain from the flag
fid_sp = endpoint_from_flag(
'endpoint.keystone-fid-service-provider.joined')
with charm.provide_charm_instance() as charm_instance:
charm_instance.render_config()
flags.set_flag('config.rendered')
# 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
restart_nonce = str(uuid.uuid4())
fid_sp.publish(restart_nonce,
charm_instance.options.protocol_name,
fid_sp.publish(charm_instance.options.protocol_name,
charm_instance.options.remote_id_attribute)
@reactive.when('endpoint.websso-fid-service-provider.joined')
@reactive.when('config.complete')
@reactive.when('keystone-data.complete')
@reactive.when('config.rendered')
def configure_websso():
# don't always have a relation context - obtain from the flag
websso_fid_sp = endpoint_from_flag(
'endpoint.websso-fid-service-provider.joined')
@reactive.when('keystone-fid-service-provider.available')
def render_config(fid_sp):
with charm.provide_charm_instance() as charm_instance:
# publish config options for all remote units of a given rel
options = charm_instance.options
websso_fid_sp.publish(options.protocol_name,
options.idp_name,
options.user_facing_name)
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(charm_instance.restart_map.keys()):
fid_sp.request_restart()
@reactive.when('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
options = charm_instance.options
websso_fid_sp.publish(options.protocol_name,
options.idp_name,
options.user_facing_name)
@reactive.when_not('always.run')

View File

@ -1,5 +1,5 @@
<EntityDescriptor
entityID="{{ options.sp_auth_url }}"
entityID="{{ keystone_fid_service_provider.base_url }}{{ options.sp_auth_path }}"
xmlns="urn:oasis:names:tc:SAML:2.0:metadata"
xmlns:ds="http://www.w3.org/2000/09/xmldsig#">
<SPSSODescriptor
@ -14,10 +14,10 @@
{{ options.sp_signing_keyinfo }}
</KeyDescriptor>
{% endif %}
<SingleLogoutService Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect" Location="{{ options.sp_logout_url }}"/>
<SingleLogoutService Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect" Location="{{ keystone_fid_service_provider.base_url }}{{ options.sp_logout_path }}"/>
{% for format in options.supported_nameid_formats -%}
<NameIDFormat>{{ format }}</NameIDFormat>
{% endfor -%}
<AssertionConsumerService Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST" Location="{{ options.sp_post_response_url }}" isDefault="true" index="0"/>
<AssertionConsumerService Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST" Location="{{ keystone_fid_service_provider.base_url }}{{ options.sp_post_response_path }}" isDefault="true" index="0"/>
</SPSSODescriptor>
</EntityDescriptor>

View File

@ -34,26 +34,16 @@ class TestRegisteredHooks(test_utils.TestRegisteredHooks):
'default_upgrade_charm': ('upgrade-charm',),
},
'when': {
'publish_sp_fid': (
'keystone-fid-service-provider.connected',),
'render_config': (
'endpoint.keystone-fid-service-provider.joined',
'config.complete',
'keystone-data.complete',),
'config_changed': (
'endpoint.keystone-fid-service-provider.joined',),
'keystone_data_changed': (
'endpoint.keystone-fid-service-provider.joined',),
'keystone-fid-service-provider.available',),
'configure_websso': (
'endpoint.websso-fid-service-provider.joined',
'config.complete',
'keystone-data.complete',
'config.rendered',),
'websso-fid-service-provider.connected',),
},
'when_not': {
'config_changed': ('config.complete',),
'keystone_departed': (
'endpoint.keystone-fid-service-provider.joined',),
'keystone_data_changed': ('keystone-data.complete',),
'render_config': ('config.rendered',),
'keystone-fid-service-provider.connected',),
'assess_status': ('always.run',),
},
}
@ -75,21 +65,10 @@ class TestKeystoneSAMLMellonHandlers(test_utils.PatchHelper):
self.keystone_saml_mellon_charm)
self.provide_charm_instance().__exit__.return_value = None
self.patch_object(handlers, 'flags')
self.uuid = 'uuid-uuid'
self.patch_object(handlers.uuid, 'uuid4')
self.uuid4.return_value = self.uuid
self.patch_object(handlers, 'unitdata',
self.patch_object(handlers.reactive, 'any_file_changed',
new=mock.MagicMock())
self.kv = mock.MagicMock()
self.unitdata.kv.return_value = self.kv
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"
@ -116,33 +95,29 @@ class TestKeystoneSAMLMellonHandlers(test_utils.PatchHelper):
handlers.keystone_departed()
self.keystone_saml_mellon_charm.remove_config.assert_called_once_with()
def test_keystone_data_changed(self):
kv_set_calls = [
mock.call("tls-enabled", True),
mock.call("port", "5000"),
mock.call("hostname", "keystone-0"),
]
handlers.keystone_data_changed(self.endpoint)
self.kv.set.has_calls(kv_set_calls)
self.flags.set_flag.assert_called_once_with('keystone-data.complete')
def test_publish_sp_fid(self):
handlers.publish_sp_fid(self.endpoint)
self.endpoint.publish.assert_called_once_with(
self.protocol_name, self.remote_id_attribute)
def test_render_config(self):
handlers.render_config()
self.keystone_saml_mellon_charm.render_config.assert_called_once_with()
self.flags.set_flag.assert_called_once_with('config.rendered')
self.endpoint.publish.assert_called_once_with(
self.uuid, self.protocol_name, self.remote_id_attribute)
# No restart
self.any_file_changed.return_value = False
(self.keystone_saml_mellon_charm
.configuration_complete.return_value) = True
def test_config_changed(self):
handlers.config_changed()
(self.keystone_saml_mellon_charm.configuration_complete
.return_value) = True
self.flags.set_flag.assert_called_once_with('config.complete')
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(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)

View File

@ -45,13 +45,7 @@ class Helper(test_utils.PatchHelper):
self.patch_release(
keystone_saml_mellon.KeystoneSAMLMellonCharm.release)
self.patch_object(keystone_saml_mellon, 'unitdata',
new=mock.MagicMock())
self.kv = mock.MagicMock()
self.unitdata.kv.return_value = self.kv
self.patch_object(keystone_saml_mellon.os_utils, 'os_release',
new=mock.MagicMock())
self.endpoint = mock.MagicMock()
self.idp_name = "samltest"
self.protocol_name = "mapped"
@ -100,19 +94,6 @@ class Helper(test_utils.PatchHelper):
self.open.return_value = self.fileobj
class TestKeystoneSAMLMellonUtils(Helper):
def test_select_release(self):
self.kv.get.return_value = 'mitaka'
self.assertEqual(
keystone_saml_mellon.select_release(), 'mitaka')
self.kv.get.return_value = None
self.os_release.return_value = 'rocky'
self.assertEqual(
keystone_saml_mellon.select_release(), 'rocky')
class TestKeystoneSAMLMellonConfigurationAdapter(Helper):
def setUp(self):
@ -120,12 +101,13 @@ class TestKeystoneSAMLMellonConfigurationAdapter(Helper):
self.hostname = "keystone-sp.local"
self.port = "5000"
self.tls_enabled = True
self.unitdata_data = {
self.endpoint_data = {
"hostname": self.hostname,
"port": self.port,
"tls-enabled": self.tls_enabled,
}
self.kv.get.side_effect = FakeConfig(self.unitdata_data)
self.endpoint.all_joined_units.received.get.side_effect = (
FakeConfig(self.endpoint_data))
self.base_url = "https://{}:{}".format(self.hostname, self.port)
def test_validation_errors(self):
@ -153,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(
@ -210,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(
@ -338,19 +285,20 @@ class TestKeystoneSAMLMellonCharm(Helper):
self.sp_signing_keyinfo.__bool__.return_value = False
self.assertFalse(ksm.configuration_complete())
def test_assess_status(self):
def test_custom_assess_status_check(self):
ksm = keystone_saml_mellon.KeystoneSAMLMellonCharm()
ksm.assess_status()
self.application_version_set.asert_called_once_with()
self.status_set.assert_called_once_with("active", "Unit is ready")
self.assertEqual(
ksm.custom_assess_status_check(),
(None, None))
# One option not ready
self.status_set.reset_mock()
self.sp_signing_keyinfo.__bool__.return_value = False
ksm.options._validation_errors = {"idp-metadata": "malformed"}
ksm.assess_status()
self.status_set.assert_called_once_with(
"blocked", "Configuration is incomplete. idp-metadata: malformed")
self.assertEqual(
ksm.custom_assess_status_check(),
("blocked",
"Configuration is incomplete. idp-metadata: malformed"))
def test_render_config(self):
ksm = keystone_saml_mellon.KeystoneSAMLMellonCharm()