Allow for multivalued server attributes

In order to achieve the task, the following changes were required:

* vectorize the base class for server attributes
* add a child class that enforces single-value attributes. It still
  accepts/returns single-value lists in order to not break Liskov
  substitution principle
* Existing attributes inherit from the child class

https://pagure.io/freeipa/issue/6937

Reviewed-By: Jan Cholasta <jcholast@redhat.com>
Reviewed-By: Stanislav Laznicka <slaznick@redhat.com>
This commit is contained in:
Martin Babinsky 2017-05-11 15:55:53 +02:00
parent 663f227a5c
commit bddb90f38a
3 changed files with 79 additions and 44 deletions

View File

@ -136,9 +136,7 @@ class serverroles(Backend):
for name, attr in assoc_attributes.items():
attr_value = attr.get(self.api)
if attr_value is not None:
result.update({name: attr_value})
result.update({name: attr_value})
return result

View File

@ -277,29 +277,33 @@ class ServerAttribute(LDAPBasedProperty):
try:
entries = ldap2.get_entries(search_base, filter=search_filter)
except errors.EmptyResult:
return
return []
master_cn = entries[0].dn[1]['cn']
master_cns = {e.dn[1]['cn'] for e in entries}
associated_role_providers = set(
self._get_assoc_role_providers(api_instance))
if master_cn not in associated_role_providers:
if not master_cns.issubset(associated_role_providers):
raise errors.ValidationError(
name=self.name,
error=_("all masters must have %(role)s role enabled" %
{'role': self.associated_role.name})
)
return master_cn
return sorted(master_cns)
def _get_master_dn(self, api_instance, server):
return DN(('cn', server), api_instance.env.container_masters,
api_instance.env.basedn)
def _get_master_dns(self, api_instance, servers):
return [
DN(('cn', server), api_instance.env.container_masters,
api_instance.env.basedn) for server in servers]
def _get_masters_service_entry(self, ldap, master_dn):
service_dn = DN(('cn', self.associated_service_name), master_dn)
return ldap.get_entry(service_dn)
def _get_masters_service_entries(self, ldap, master_dns):
service_dns = [
DN(('cn', self.associated_service_name), master_dn) for master_dn
in master_dns]
return [ldap.get_entry(service_dn) for service_dn in service_dns]
def _add_attribute_to_svc_entry(self, ldap, service_entry):
"""
@ -341,65 +345,98 @@ class ServerAttribute(LDAPBasedProperty):
r[u'server_server'] for r in self.associated_role.status(
api_instance) if r[u'status'] == ENABLED]
def _remove(self, api_instance, master):
def _remove(self, api_instance, masters):
"""
remove attribute from the master
remove attribute from one or more masters
:param api_instance: API instance
:param master: master FQDN
:param master: list or iterable containing master FQDNs
"""
ldap = api_instance.Backend.ldap2
master_dn = self._get_master_dn(api_instance, master)
service_entry = self._get_masters_service_entry(ldap, master_dn)
self._remove_attribute_from_svc_entry(ldap, service_entry)
master_dns = self._get_master_dns(api_instance, masters)
service_entries = self._get_masters_service_entries(ldap, master_dns)
def _add(self, api_instance, master):
for service_entry in service_entries:
self._remove_attribute_from_svc_entry(ldap, service_entry)
def _add(self, api_instance, masters):
"""
add attribute to the master
:param api_instance: API instance
:param master: master FQDN
:param master: iterable containing master FQDNs
:raises: * errors.ValidationError if the associated role is not enabled
on the master
"""
assoc_role_providers = self._get_assoc_role_providers(api_instance)
assoc_role_providers = set(
self._get_assoc_role_providers(api_instance))
masters_set = set(masters)
ldap = api_instance.Backend.ldap2
if master not in assoc_role_providers:
masters_without_role = masters_set - assoc_role_providers
if masters_without_role:
raise errors.ValidationError(
name=master,
name=', '.join(sorted(masters_without_role)),
error=_("must have %(role)s role enabled" %
{'role': self.associated_role.name})
)
master_dn = self._get_master_dn(api_instance, master)
service_entry = self._get_masters_service_entry(ldap, master_dn)
self._add_attribute_to_svc_entry(ldap, service_entry)
master_dns = self._get_master_dns(api_instance, masters)
service_entries = self._get_masters_service_entries(ldap, master_dns)
for service_entry in service_entries:
self._add_attribute_to_svc_entry(ldap, service_entry)
def set(self, api_instance, master):
def set(self, api_instance, masters):
"""
set the attribute on master
set the attribute on masters
:param api_instance: API instance
:param master: FQDN of the new master
:param masters: an interable with FQDNs of the new masters
the attribute is automatically unset from previous master if present
the attribute is automatically unset from previous masters if present
:raises: errors.EmptyModlist if the new masters is the same as
the original on
the original ones
"""
old_master = self.get(api_instance)
old_masters = self.get(api_instance)
if old_master == master:
if sorted(old_masters) == sorted(masters):
raise errors.EmptyModlist
self._add(api_instance, master)
if old_masters:
self._remove(api_instance, old_masters)
if old_master is not None:
self._remove(api_instance, old_master)
self._add(api_instance, masters)
class SingleValuedServerAttribute(ServerAttribute):
"""
Base class for server attributes that are forced to be single valued
this means that `get` method will return a one-element list, and `set`
method will accept only one-element list
"""
def set(self, api_instance, masters):
if len(masters) > 1:
raise errors.ValidationError(
name=self.attr_name,
error=_("must be enabled only on a single master"))
super(SingleValuedServerAttribute, self).set(api_instance, masters)
def get(self, api_instance):
masters = super(SingleValuedServerAttribute, self).get(api_instance)
num_masters = len(masters)
if num_masters > 1:
raise errors.SingleMatchExpected(found=num_masters)
return masters
_Service = namedtuple('Service', ['name', 'enabled'])
@ -574,14 +611,14 @@ role_instances = (
)
attribute_instances = (
ServerAttribute(
SingleValuedServerAttribute(
u"ca_renewal_master_server",
u"CA renewal master",
u"ca_server_server",
u"CA",
u"caRenewalMaster",
),
ServerAttribute(
SingleValuedServerAttribute(
u"dnssec_key_master_server",
u"DNSSec key master",
u"dns_server_server",

View File

@ -706,7 +706,7 @@ class TestServerAttributes(object):
actual_attr_masters = self.config_retrieve(
assoc_role, mock_api)[attr_name]
assert actual_attr_masters == fqdn
assert actual_attr_masters == [fqdn]
def test_set_attribute_on_the_same_provider_raises_emptymodlist(
self, mock_api, mock_masters):
@ -727,7 +727,7 @@ class TestServerAttributes(object):
non_ca_fqdn = mock_masters.get_fqdn('trust-controller-dns')
with pytest.raises(errors.ValidationError):
self.config_update(mock_api, **{attr_name: non_ca_fqdn})
self.config_update(mock_api, **{attr_name: [non_ca_fqdn]})
def test_set_unknown_attribute_on_master_raises_notfound(
self, mock_api, mock_masters):
@ -735,7 +735,7 @@ class TestServerAttributes(object):
fqdn = mock_masters.get_fqdn('trust-controller-ca')
with pytest.raises(errors.NotFound):
self.config_update(mock_api, **{attr_name: fqdn})
self.config_update(mock_api, **{attr_name: [fqdn]})
def test_set_ca_renewal_master_on_other_ca_and_back(self, mock_api,
mock_masters):
@ -747,7 +747,7 @@ class TestServerAttributes(object):
other_ca_server = mock_masters.get_fqdn('trust-controller-ca')
for host in (other_ca_server, original_renewal_master):
self.config_update(mock_api, **{attr_name: host})
self.config_update(mock_api, **{attr_name: [host]})
assert (
self.config_retrieve(role_name, mock_api)[attr_name] == host)
self.config_retrieve(role_name, mock_api)[attr_name] == [host])