URL: https://github.com/freeipa/freeipa/pull/4304 Author: fcami Title: #4304: ipa-backup: Make sure all roles are installed on the current master. Action: opened
PR body: """ ipa-backup does not check whether the IPA master it is running on has all used roles installed. This can lead into situations where backups are done on a CAless or KRAless host when these roles are used in the IPA cluster. These backups cannot be used to restore a complete cluster. With this change, ipa-backup checks what roles are installed on the current host and compares the resulting list to the list of used roles in the cluster. A --disable-role-check knob is also provided to restore the previous behavior. Fixes: https://pagure.io/freeipa/issue/8217 Signed-off-by: François Cami <fc...@redhat.com> """ To pull the PR as Git branch: git remote add ghfreeipa https://github.com/freeipa/freeipa git fetch ghfreeipa pull/4304/head:pr4304 git checkout pr4304
From a27b454fe6dcf5c5b158917cad68ceb8a5cf91cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Cami?= <fc...@redhat.com> Date: Tue, 3 Mar 2020 15:20:23 +0100 Subject: [PATCH] ipa-backup: Make sure all roles are installed on the current master. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ipa-backup does not check whether the IPA master it is running on has all used roles installed. This can lead into situations where backups are done on a CAless or KRAless host when these roles are used in the IPA cluster. These backups cannot be used to restore a complete cluster. With this change, ipa-backup checks what roles are installed on the current host and compares the resulting list to the list of used roles in the cluster. A --disable-role-check knob is also provided to restore the previous behavior. Fixes: https://pagure.io/freeipa/issue/8217 Signed-off-by: François Cami <fc...@redhat.com> --- ipaserver/install/ipa_backup.py | 83 +++++++++++++++++++++++++++++---- 1 file changed, 73 insertions(+), 10 deletions(-) diff --git a/ipaserver/install/ipa_backup.py b/ipaserver/install/ipa_backup.py index 1c2faff710..24a7e5d09f 100644 --- a/ipaserver/install/ipa_backup.py +++ b/ipaserver/install/ipa_backup.py @@ -247,12 +247,16 @@ def add_options(cls, parser): "--online", dest="online", action="store_true", default=False, help="Perform the LDAP backups online, for data only.") - + parser.add_option( + "--disable-role-check", dest="rolecheck", action="store_false", + default=True, + help="Perform the backup even if this host does not have all " + "the used roles. This is not recommended." + ) def setup_logging(self, log_file_mode='a'): super(Backup, self).setup_logging(log_file_mode='a') - def validate_options(self): options = self.options super(Backup, self).validate_options(needs_root=True) @@ -279,7 +283,6 @@ def validate_options(self): self.option_parser.error("You cannot specify --data " "with --logs") - def run(self): options = self.options super(Backup, self).run() @@ -312,6 +315,73 @@ def run(self): self.get_connection() + locally_installed_roles = set() + globally_used_roles = set() + + # We need to cover the following roles: + # * DNS: filter="(|(cn=DNS)(cn=DNSKeySync))" + # * CA: filter="(cn=CA)" + # * KRA: filter="(cn=KRA)" + # * AD Trust Controller: filter="(cn=ADTRUST)" + # Note: + # We do not need to worry about AD Trust Agents as Trust + # Controllers are Trust Agents themselves and contain extra, + # necessary Samba configuration. So either the cluster has no + # AD Trust bits installed, or it should be backuped on a Trust + # Controller, not a Trust Agent. + role_names = { + 'CA', 'DNS', 'DNSKeySync', 'KRA', 'ADTRUST' + } + + search_base = DN(api.env.container_masters, api.env.basedn) + attrs_list = ['ipaConfigString', 'cn'] + + for role in role_names: + search_filter = '(cn=%s)' % role + try: + masters = dict() + result = self._conn.get_entries( + search_base, + filter=search_filter, + attrs_list=attrs_list, + scope=self._conn.SCOPE_SUBTREE + ) + masters[role] = {e.dn[1]['cn'] for e in result} + + if api.env.host in masters[role]: + locally_installed_roles.add(role) + if masters[role] is not None: + globally_used_roles.add(role) + except errors.EmptyResult: + pass + + if locally_installed_roles == globally_used_roles: + logger.info( + "Local roles match globally used roles, proceeding." + ) + else: + if options.rolecheck: + raise admintool.ScriptError( + 'Error: Local roles %s do not match globally used ' + 'roles %s. A backup done on this host would not be ' + 'complete enough to restore a fully functional, ' + 'identical cluster.' % ( + ', '.join(sorted(locally_installed_roles)), + ', '.join(sorted(globally_used_roles)) + ) + ) + else: + logger.info( + 'Warning: Local roles %s do not match globally used ' + 'roles %s. A backup done on this host would not be ' + 'complete enough to restore a fully functional, ' + 'identical cluster. ' + 'Proceeding as role check was explicitly disabled.' % ( + ', '.join(sorted(locally_installed_roles)), + ', '.join(sorted(globally_used_roles)) + ) + ) + self.create_header(options.data_only) if options.data_only: if not options.online: @@ -358,7 +428,6 @@ def run(self): logger.error('Cannot change directory to %s: %s', cwd, e) shutil.rmtree(self.top_dir) - def add_instance_specific_data(self): ''' Add instance-specific files and directories. @@ -387,7 +456,6 @@ def add_instance_specific_data(self): self.logs.append(paths.VAR_LOG_DIRSRV_INSTANCE_TEMPLATE % serverid) - def get_connection(self): ''' Create an ldapi connection and bind to it using autobind as root. @@ -405,7 +473,6 @@ def get_connection(self): return self._conn - def db2ldif(self, instance, backend, online=True): ''' Create a LDIF backup of the data in this instance. @@ -481,7 +548,6 @@ def db2ldif(self, instance, backend, online=True): 'Unexpected error: %s' % e ) - def db2bak(self, instance, online=True): ''' Create a BAK backup of the data and changelog in this instance. @@ -545,7 +611,6 @@ def db2bak(self, instance, online=True): 'Unexpected error: %s' % e ) - def file_backup(self, options): def verify_directories(dirs): @@ -611,7 +676,6 @@ def compress_file_backup(self): # Rename the archive back to files.tar to preserve compatibility os.rename(os.path.join(self.dir, 'files.tar.gz'), self.tarfile) - def create_header(self, data_only): ''' Create the backup file header that contains the meta data about @@ -649,7 +713,6 @@ def create_header(self, data_only): with open(self.header, 'w') as fd: config.write(fd) - def finalize_backup(self, data_only=False, encrypt=False, keyring=None): ''' Create the final location of the backup files and move the files
_______________________________________________ FreeIPA-devel mailing list -- freeipa-devel@lists.fedorahosted.org To unsubscribe send an email to freeipa-devel-le...@lists.fedorahosted.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/freeipa-devel@lists.fedorahosted.org
