URL: https://github.com/freeipa/freeipa/pull/5004 Author: rcritten Title: #5004: [Backport][ipa-4-8] Simplify and make more reliable the server and client installation checks Action: opened
PR body: """ This PR was opened manually because PR #4895 was pushed to master and backport to ipa-4-8 is required. The merge conflict was due to 53d472b490ac7a14fc78516b448d4aa312b79b7f being only in master. Fixing this was straightforward. """ To pull the PR as Git branch: git remote add ghfreeipa https://github.com/freeipa/freeipa git fetch ghfreeipa pull/5004/head:pr5004 git checkout pr5004
From 753d110a1541b337ff4c0cae407ed89e06ffc929 Mon Sep 17 00:00:00 2001 From: Rob Crittenden <rcrit...@redhat.com> Date: Tue, 7 Jul 2020 16:24:35 -0400 Subject: [PATCH 1/5] Simplify determining if an IPA server installation is complete When asking the quesiton "is my IPA server configured?" right now we look at whether the installation backed up any files and set any state. This isn't exactly precise. Instead set a new state, installation, to True as soon as IPA is restarted at the end of the installer. On upgrades existing installations will automatically get this state. This relies on the fact that get_state returns None if no state at all is set. This indicates that this "new" option isn't available and when upgrading an existing installation we can assume the install at least partly works. The value is forced to False at the beginning of a fresh install so if it fails, or is in a transient state like with an external CA, we know that the installation is not complete. https://pagure.io/freeipa/issue/8384 Signed-off-by: Rob Crittenden <rcrit...@redhat.com> Reviewed-By: Alexander Bokovoy <aboko...@redhat.com> Reviewed-By: Francois Cami <fc...@redhat.com> --- ipaserver/install/installutils.py | 22 ++-------------------- ipaserver/install/server/install.py | 6 ++++++ ipaserver/install/server/replicainstall.py | 6 ++++++ ipaserver/install/server/upgrade.py | 4 ++++ 4 files changed, 18 insertions(+), 20 deletions(-) diff --git a/ipaserver/install/installutils.py b/ipaserver/install/installutils.py index ba98e8bed3..f19f64fbe8 100644 --- a/ipaserver/install/installutils.py +++ b/ipaserver/install/installutils.py @@ -700,28 +700,10 @@ def rmtree(path): def is_ipa_configured(): """ - Using the state and index install files determine if IPA is already - configured. + Use the state to determine if IPA has been configured. """ - installed = False - sstore = sysrestore.StateFile(paths.SYSRESTORE) - fstore = sysrestore.FileStore(paths.SYSRESTORE) - - for module in IPA_MODULES: - if sstore.has_state(module): - logger.debug('%s is configured', module) - installed = True - else: - logger.debug('%s is not configured', module) - - if fstore.has_files(): - logger.debug('filestore has files') - installed = True - else: - logger.debug('filestore is tracking no files') - - return installed + return sstore.get_state('installation', 'complete') def run_script(main_function, operation_name, log_file_name=None, diff --git a/ipaserver/install/server/install.py b/ipaserver/install/server/install.py index b53c58e2a6..4822c222ce 100644 --- a/ipaserver/install/server/install.py +++ b/ipaserver/install/server/install.py @@ -795,6 +795,9 @@ def install(installer): # failure to enable root cause investigation installer._installation_cleanup = False + # Be clear that the installation process is beginning but not done + sstore.backup_state('installation', 'complete', False) + if installer.interactive: print("") print("The following operations may take some minutes to complete.") @@ -998,6 +1001,8 @@ def install(installer): bind.create_file_with_system_records() # Everything installed properly, activate ipa service. + sstore.delete_state('installation', 'complete') + sstore.backup_state('installation', 'complete', True) services.knownservices.ipa.enable() print("=======================================" @@ -1201,6 +1206,7 @@ def uninstall(installer): if fstore.has_files(): logger.error('Some files have not been restored, see ' '%s/sysrestore.index', SYSRESTORE_DIR_PATH) + sstore.delete_state('installation', 'complete') has_state = False for module in IPA_MODULES: # from installutils if sstore.has_state(module): diff --git a/ipaserver/install/server/replicainstall.py b/ipaserver/install/server/replicainstall.py index 7d6c4108c0..b8e896ac7c 100644 --- a/ipaserver/install/server/replicainstall.py +++ b/ipaserver/install/server/replicainstall.py @@ -1205,6 +1205,7 @@ def install(installer): ca_enabled = installer._ca_enabled kra_enabled = installer._kra_enabled fstore = installer._fstore + sstore = installer._sstore config = installer._config cafile = installer._ca_file dirsrv_pkcs12_info = installer._dirsrv_pkcs12_info @@ -1215,6 +1216,9 @@ def install(installer): conn = remote_api.Backend.ldap2 ccache = os.environ['KRB5CCNAME'] + # Be clear that the installation process is beginning but not done + sstore.backup_state('installation', 'complete', False) + if tasks.configure_pkcs11_modules(fstore): print("Disabled p11-kit-proxy") @@ -1371,6 +1375,8 @@ def install(installer): api.Backend.ldap2.disconnect() # Everything installed properly, activate ipa service. + sstore.delete_state('installation', 'complete') + sstore.backup_state('installation', 'complete', True) services.knownservices.ipa.enable() # Print a warning if CA role is only installed on one server diff --git a/ipaserver/install/server/upgrade.py b/ipaserver/install/server/upgrade.py index 2c15178655..e9a4dca315 100644 --- a/ipaserver/install/server/upgrade.py +++ b/ipaserver/install/server/upgrade.py @@ -1451,6 +1451,10 @@ def upgrade_configuration(): logger.debug('IPA version %s', version.VENDOR_VERSION) fstore = sysrestore.FileStore(paths.SYSRESTORE) + sstore = sysrestore.StateFile(paths.SYSRESTORE) + + if installutils.is_ipa_configured() is None: + sstore.backup_state('installation', 'complete', True) fqdn = api.env.host From cdf0a94af5ac6776cd10bb94fa4b41068c363f52 Mon Sep 17 00:00:00 2001 From: Rob Crittenden <rcrit...@redhat.com> Date: Tue, 7 Jul 2020 16:56:14 -0400 Subject: [PATCH 2/5] Simplify determining if IPA client configuration is complete When asking the quesiton "is my IPA client configured?" right now we look at whether the installation backed up any files and /etc/ipa/default.conf exists. Instead set a new state, installation, to True as soon as the client installation finishes. Unlike the server there is no upgrade process for clients so this isn't going to be all that useful for quite some time unless that changes because upgrading an existing install won't set this to True. https://pagure.io/freeipa/issue/8384 Signed-off-by: Rob Crittenden <rcrit...@redhat.com> Reviewed-By: Alexander Bokovoy <aboko...@redhat.com> Reviewed-By: Francois Cami <fc...@redhat.com> --- ipaclient/install/client.py | 20 +++++++++++++++++--- ipaclient/install/ipa_epn.py | 4 +--- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/ipaclient/install/client.py b/ipaclient/install/client.py index ad03c81fd1..2aa8b5f7dd 100644 --- a/ipaclient/install/client.py +++ b/ipaclient/install/client.py @@ -266,13 +266,21 @@ def delete_ipa_domain(): "No access to the /etc/sssd/sssd.conf file.") -def is_ipa_client_installed(fstore, on_master=False): +def is_ipa_client_installed(on_master=False): """ Consider IPA client not installed if nothing is backed up and default.conf file does not exist. If on_master is set to True, the existence of default.conf file is not taken into consideration, since it has been already created by ipa-server-install. """ + fstore = sysrestore.FileStore(paths.IPA_CLIENT_SYSRESTORE) + statestore = sysrestore.StateFile(paths.IPA_CLIENT_SYSRESTORE) + + installed = statestore.get_state('installation', 'complete') + if installed is not None: + return installed + + # Fall back to the old detection installed = ( fstore.has_files() or ( @@ -2086,7 +2094,7 @@ def install_check(options): tasks.check_selinux_status() - if is_ipa_client_installed(fstore, on_master=options.on_master): + if is_ipa_client_installed(on_master=options.on_master): logger.error("IPA client is already configured on this system.") logger.info( "If you want to reinstall the IPA client, uninstall it first " @@ -2598,6 +2606,8 @@ def _install(options): fstore = sysrestore.FileStore(paths.IPA_CLIENT_SYSRESTORE) statestore = sysrestore.StateFile(paths.IPA_CLIENT_SYSRESTORE) + statestore.backup_state('installation', 'complete', False) + if not options.on_master: # Try removing old principals from the keytab purge_host_keytab(cli_realm) @@ -3184,13 +3194,15 @@ def _install(options): configure_nisdomain( options=options, domain=cli_domain, statestore=statestore) + statestore.delete_state('installation', 'complete') + statestore.backup_state('installation', 'complete', True) logger.info('Client configuration complete.') def uninstall_check(options): fstore = sysrestore.FileStore(paths.IPA_CLIENT_SYSRESTORE) - if not is_ipa_client_installed(fstore): + if not is_ipa_client_installed(): if options.on_master: rval = SUCCESS else: @@ -3517,6 +3529,8 @@ def uninstall(options): if fstore.has_files(): logger.error('Some files have not been restored, see %s', paths.SYSRESTORE_INDEX) + + statestore.delete_state('installation', 'complete') has_state = False for module in statestore.modules: logger.error( diff --git a/ipaclient/install/ipa_epn.py b/ipaclient/install/ipa_epn.py index 6e1b001464..666c703e53 100644 --- a/ipaclient/install/ipa_epn.py +++ b/ipaclient/install/ipa_epn.py @@ -42,7 +42,6 @@ from ipaclient.install.client import is_ipa_client_installed from ipaplatform.paths import paths from ipalib import api, errors -from ipalib.install import sysrestore from ipapython import admintool, ipaldap from ipapython.dn import DN @@ -255,8 +254,7 @@ def setup_logging(self, log_file_mode="a"): def run(self): super(EPN, self).run() - fstore = sysrestore.FileStore(paths.IPA_CLIENT_SYSRESTORE) - if not is_ipa_client_installed(fstore): + if not is_ipa_client_installed(): logger.error("IPA client is not configured on this system.") raise admintool.ScriptError() From a2d18946cb2e55d9141d13634ca19dde930532bd Mon Sep 17 00:00:00 2001 From: Rob Crittenden <rcrit...@redhat.com> Date: Wed, 8 Jul 2020 10:16:17 -0400 Subject: [PATCH 3/5] Create a common place to retrieve facts about an IPA installation This is common to both client and server. Start with whether the client or server is configured. https://pagure.io/freeipa/issue/8384 Signed-off-by: Rob Crittenden <rcrit...@redhat.com> Reviewed-By: Alexander Bokovoy <aboko...@redhat.com> Reviewed-By: Francois Cami <fc...@redhat.com> --- freeipa.spec.in | 4 +- .../com.redhat.idm.trust-fetch-domains.in | 3 +- install/tools/ipa-custodia-check.in | 2 +- ipaclient/install/client.py | 28 +- ipaclient/install/ipa_epn.py | 4 +- ipalib/facts.py | 43 ++ ipalib/install/sysrestore.py | 464 +----------------- ipalib/sysrestore.py | 457 +++++++++++++++++ ipaserver/install/installutils.py | 10 +- ipaserver/install/ipa_cert_fix.py | 2 +- ipaserver/install/ipactl.py | 3 +- ipaserver/install/server/install.py | 5 +- ipaserver/install/server/replicainstall.py | 3 +- ipaserver/install/server/upgrade.py | 6 +- 14 files changed, 549 insertions(+), 485 deletions(-) create mode 100644 ipalib/facts.py create mode 100644 ipalib/sysrestore.py diff --git a/freeipa.spec.in b/freeipa.spec.in index 1e7d11953f..8d24c76c80 100755 --- a/freeipa.spec.in +++ b/freeipa.spec.in @@ -920,7 +920,7 @@ fi %posttrans server # don't execute upgrade and restart of IPA when server is not installed -%{__python3} -c "import sys; from ipaserver.install import installutils; sys.exit(0 if installutils.is_ipa_configured() else 1);" > /dev/null 2>&1 +%{__python3} -c "import sys; from ipalib import facts; sys.exit(0 if facts.is_ipa_configured() else 1);" > /dev/null 2>&1 if [ $? -eq 0 ]; then # This is necessary for Fedora system upgrades which by default @@ -999,7 +999,7 @@ fi %posttrans server-trust-ad -%{__python3} -c "import sys; from ipaserver.install import installutils; sys.exit(0 if installutils.is_ipa_configured() else 1);" > /dev/null 2>&1 +%{__python3} -c "import sys; from ipalib import facts; sys.exit(0 if facts.is_ipa_configured() else 1);" > /dev/null 2>&1 if [ $? -eq 0 ]; then # NOTE: systemd specific section /bin/systemctl try-restart httpd.service >/dev/null 2>&1 || : diff --git a/install/oddjob/com.redhat.idm.trust-fetch-domains.in b/install/oddjob/com.redhat.idm.trust-fetch-domains.in index c002b1bb3f..616c8bf9ea 100644 --- a/install/oddjob/com.redhat.idm.trust-fetch-domains.in +++ b/install/oddjob/com.redhat.idm.trust-fetch-domains.in @@ -1,9 +1,10 @@ #!/usr/bin/python3 from ipaserver import dcerpc -from ipaserver.install.installutils import is_ipa_configured, ScriptError +from ipaserver.install.installutils import ScriptError from ipapython import config, ipautil from ipalib import api +from ipalib.facts import is_ipa_configured from ipapython.dn import DN from ipapython.dnsutil import DNSName from ipaplatform.constants import constants diff --git a/install/tools/ipa-custodia-check.in b/install/tools/ipa-custodia-check.in index 7fdfbff52b..5143dc4983 100644 --- a/install/tools/ipa-custodia-check.in +++ b/install/tools/ipa-custodia-check.in @@ -18,9 +18,9 @@ from jwcrypto.common import json_decode from jwcrypto.jwk import JWK from ipalib import api +from ipalib.facts import is_ipa_configured from ipaplatform.paths import paths import ipapython.version -from ipaserver.install.installutils import is_ipa_configured try: # FreeIPA >= 4.5 diff --git a/ipaclient/install/client.py b/ipaclient/install/client.py index 2aa8b5f7dd..da671361b2 100644 --- a/ipaclient/install/client.py +++ b/ipaclient/install/client.py @@ -35,8 +35,10 @@ from urllib.parse import urlparse, urlunparse from ipalib import api, errors, x509 +from ipalib import sysrestore from ipalib.constants import IPAAPI_USER, MAXHOSTNAMELEN -from ipalib.install import certmonger, certstore, service, sysrestore +from ipalib.facts import is_ipa_client_configured +from ipalib.install import certmonger, certstore, service from ipalib.install import hostname as hostname_ from ipalib.install.kinit import kinit_keytab, kinit_password from ipalib.install.service import enroll_only, prepare_only @@ -273,22 +275,12 @@ def is_ipa_client_installed(on_master=False): the existence of default.conf file is not taken into consideration, since it has been already created by ipa-server-install. """ - fstore = sysrestore.FileStore(paths.IPA_CLIENT_SYSRESTORE) - statestore = sysrestore.StateFile(paths.IPA_CLIENT_SYSRESTORE) - - installed = statestore.get_state('installation', 'complete') - if installed is not None: - return installed - - # Fall back to the old detection - - installed = ( - fstore.has_files() or ( - not on_master and os.path.exists(paths.IPA_DEFAULT_CONF) - ) + warnings.warn( + "Use 'ipalib.facts.is_ipa_client_configured'", + DeprecationWarning, + stacklevel=2 ) - - return installed + return is_ipa_client_configured(on_master) def configure_nsswitch_database(fstore, database, services, preserve=True, @@ -2094,7 +2086,7 @@ def install_check(options): tasks.check_selinux_status() - if is_ipa_client_installed(on_master=options.on_master): + if is_ipa_client_configured(on_master=options.on_master): logger.error("IPA client is already configured on this system.") logger.info( "If you want to reinstall the IPA client, uninstall it first " @@ -3202,7 +3194,7 @@ def _install(options): def uninstall_check(options): fstore = sysrestore.FileStore(paths.IPA_CLIENT_SYSRESTORE) - if not is_ipa_client_installed(): + if not is_ipa_client_configured(): if options.on_master: rval = SUCCESS else: diff --git a/ipaclient/install/ipa_epn.py b/ipaclient/install/ipa_epn.py index 666c703e53..65f9f3d47f 100644 --- a/ipaclient/install/ipa_epn.py +++ b/ipaclient/install/ipa_epn.py @@ -39,9 +39,9 @@ from email.header import Header from email.utils import make_msgid -from ipaclient.install.client import is_ipa_client_installed from ipaplatform.paths import paths from ipalib import api, errors +from ipalib.facts import is_ipa_client_configured from ipapython import admintool, ipaldap from ipapython.dn import DN @@ -254,7 +254,7 @@ def setup_logging(self, log_file_mode="a"): def run(self): super(EPN, self).run() - if not is_ipa_client_installed(): + if not is_ipa_client_configured(): logger.error("IPA client is not configured on this system.") raise admintool.ScriptError() diff --git a/ipalib/facts.py b/ipalib/facts.py new file mode 100644 index 0000000000..a1e3e46bc9 --- /dev/null +++ b/ipalib/facts.py @@ -0,0 +1,43 @@ +# +# Copyright (C) 2020 FreeIPA Contributors see COPYING for license +# + +""" +Facts about the installation +""" + +from . import sysrestore +from ipaplatform.paths import paths + + +def is_ipa_configured(): + """ + Use the state to determine if IPA has been configured. + """ + sstore = sysrestore.StateFile(paths.SYSRESTORE) + return sstore.get_state('installation', 'complete') + + +def is_ipa_client_configured(on_master=False): + """ + Consider IPA client not installed if nothing is backed up + and default.conf file does not exist. If on_master is set to True, + the existence of default.conf file is not taken into consideration, + since it has been already created by ipa-server-install. + """ + fstore = sysrestore.FileStore(paths.IPA_CLIENT_SYSRESTORE) + statestore = sysrestore.StateFile(paths.IPA_CLIENT_SYSRESTORE) + + installed = statestore.get_state('installation', 'complete') + if installed is not None: + return installed + + # Fall back to the old detection + + installed = ( + fstore.has_files() or ( + not on_master and os.path.exists(paths.IPA_DEFAULT_CONF) + ) + ) + + return installed diff --git a/ipalib/install/sysrestore.py b/ipalib/install/sysrestore.py index 5fd52b8edc..930414ec1c 100644 --- a/ipalib/install/sysrestore.py +++ b/ipalib/install/sysrestore.py @@ -1,457 +1,19 @@ -# Authors: Mark McLoughlin <mar...@redhat.com> # -# Copyright (C) 2007 Red Hat -# see file 'COPYING' for use and warranty information +# Copyright (C) 2020 FreeIPA Contributors see COPYING for license # -# This program is free software; you can redistribute it and/or modify -# it under the terms of the GNU General Public License as published by -# the Free Software Foundation, either version 3 of the License, or -# (at your option) any later version. -# -# This program is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with this program. If not, see <http://www.gnu.org/licenses/>. -# - -# -# This module provides a very simple API which allows -# ipa-xxx-install --uninstall to restore certain -# parts of the system configuration to the way it was -# before ipa-server-install was first run - -from __future__ import absolute_import - -import logging -import os -import os.path -import shutil -import random - -from hashlib import sha256 - -import six -# pylint: disable=import-error -if six.PY3: - # The SafeConfigParser class has been renamed to ConfigParser in Py3 - from configparser import ConfigParser as SafeConfigParser -else: - from ConfigParser import SafeConfigParser -# pylint: enable=import-error - -from ipaplatform.tasks import tasks -from ipaplatform.paths import paths - -if six.PY3: - unicode = str - -logger = logging.getLogger(__name__) - -SYSRESTORE_PATH = paths.TMP -SYSRESTORE_INDEXFILE = "sysrestore.index" -SYSRESTORE_STATEFILE = "sysrestore.state" - - -class FileStore: - """Class for handling backup and restore of files""" - - def __init__(self, path = SYSRESTORE_PATH, index_file = SYSRESTORE_INDEXFILE): - """Create a _StoreFiles object, that uses @path as the - base directory. - - The file @path/sysrestore.index is used to store information - about the original location of the saved files. - """ - self._path = path - self._index = os.path.join(self._path, index_file) - - self.random = random.Random() - - self.files = {} - self._load() - - def _load(self): - """Load the file list from the index file. @files will - be an empty dictionary if the file doesn't exist. - """ - - logger.debug("Loading Index file from '%s'", self._index) - - self.files = {} - - p = SafeConfigParser() - p.optionxform = str - p.read(self._index) - - for section in p.sections(): - if section == "files": - for (key, value) in p.items(section): - self.files[key] = value - - - def save(self): - """Save the file list to @_index. If @files is an empty - dict, then @_index should be removed. - """ - logger.debug("Saving Index File to '%s'", self._index) - - if len(self.files) == 0: - logger.debug(" -> no files, removing file") - if os.path.exists(self._index): - os.remove(self._index) - return - - p = SafeConfigParser() - p.optionxform = str - - p.add_section('files') - for (key, value) in self.files.items(): - p.set('files', key, str(value)) - - with open(self._index, "w") as f: - p.write(f) - - def backup_file(self, path): - """Create a copy of the file at @path - as long as an exact copy - does not already exist - which will be restored to its - original location by restore_files(). - """ - logger.debug("Backing up system configuration file '%s'", path) - - if not os.path.isabs(path): - raise ValueError("Absolute path required") - - if not os.path.isfile(path): - logger.debug(" -> Not backing up - '%s' doesn't exist", path) - return - - _reldir, backupfile = os.path.split(path) - - with open(path, 'rb') as f: - cont_hash = sha256(f.read()).hexdigest() - - filename = "{hexhash}-{bcppath}".format( - hexhash=cont_hash, bcppath=backupfile) - - backup_path = os.path.join(self._path, filename) - if os.path.exists(backup_path): - logger.debug(" -> Not backing up - already have a copy of '%s'", - path) - return - - shutil.copy2(path, backup_path) - - stat = os.stat(path) - - template = '{stat.st_mode},{stat.st_uid},{stat.st_gid},{path}' - self.files[filename] = template.format(stat=stat, path=path) - self.save() - - def has_file(self, path): - """Checks whether file at @path was added to the file store - - Returns #True if the file exists in the file store, #False otherwise - """ - result = False - for _key, value in self.files.items(): - _mode, _uid, _gid, filepath = value.split(',', 3) - if (filepath == path): - result = True - break - return result - - def restore_file(self, path, new_path = None): - """Restore the copy of a file at @path to its original - location and delete the copy. - - Takes optional parameter @new_path which specifies the - location where the file is to be restored. - - Returns #True if the file was restored, #False if there - was no backup file to restore - """ - - if new_path is None: - logger.debug("Restoring system configuration file '%s'", - path) - else: - logger.debug("Restoring system configuration file '%s' to '%s'", - path, new_path) - - if not os.path.isabs(path): - raise ValueError("Absolute path required") - if new_path is not None and not os.path.isabs(new_path): - raise ValueError("Absolute new path required") - - mode = None - uid = None - gid = None - filename = None - - for (key, value) in self.files.items(): - (mode,uid,gid,filepath) = value.split(',', 3) - if (filepath == path): - filename = key - break - - if not filename: - raise ValueError("No such file name in the index") - - backup_path = os.path.join(self._path, filename) - if not os.path.exists(backup_path): - logger.debug(" -> Not restoring - '%s' doesn't exist", - backup_path) - return False - - if new_path is not None: - path = new_path - - shutil.copy(backup_path, path) # SELinux needs copy - os.remove(backup_path) - - os.chown(path, int(uid), int(gid)) - os.chmod(path, int(mode)) - - tasks.restore_context(path) - - del self.files[filename] - self.save() - - return True - - def restore_all_files(self): - """Restore the files in the inbdex to their original - location and delete the copy. - - Returns #True if the file was restored, #False if there - was no backup file to restore - """ - - if len(self.files) == 0: - return False - - for (filename, value) in self.files.items(): - - (mode,uid,gid,path) = value.split(',', 3) - - backup_path = os.path.join(self._path, filename) - if not os.path.exists(backup_path): - logger.debug(" -> Not restoring - '%s' doesn't exist", - backup_path) - continue - - shutil.copy(backup_path, path) # SELinux needs copy - os.remove(backup_path) - - os.chown(path, int(uid), int(gid)) - os.chmod(path, int(mode)) - - tasks.restore_context(path) - - # force file to be deleted - self.files = {} - self.save() - - return True - - def has_files(self): - """Return True or False if there are any files in the index - - Can be used to determine if a program is configured. - """ - - return len(self.files) > 0 - - def untrack_file(self, path): - """Remove file at path @path from list of backed up files. - - Does not remove any files from the filesystem. - - Returns #True if the file was untracked, #False if there - was no backup file to restore - """ - - logger.debug("Untracking system configuration file '%s'", path) - - if not os.path.isabs(path): - raise ValueError("Absolute path required") - - filename = None - - for (key, value) in self.files.items(): - _mode, _uid, _gid, filepath = value.split(',', 3) - if (filepath == path): - filename = key - break - - if not filename: - raise ValueError("No such file name in the index") - - backup_path = os.path.join(self._path, filename) - if not os.path.exists(backup_path): - logger.debug(" -> Not restoring - '%s' doesn't exist", - backup_path) - return False - - try: - os.unlink(backup_path) - except Exception as e: - logger.error('Error removing %s: %s', backup_path, str(e)) - - del self.files[filename] - self.save() - - return True - - -class StateFile: - """A metadata file for recording system state which can - be backed up and later restored. - StateFile gets reloaded every time to prevent loss of information - recorded by child processes. But we do not solve concurrency - because there is no need for it right now. - The format is something like: - - [httpd] - running=True - enabled=False - """ - - def __init__(self, path = SYSRESTORE_PATH, state_file = SYSRESTORE_STATEFILE): - """Create a StateFile object, loading from @path. - - The dictionary @modules, a member of the returned object, - is where the state can be modified. @modules is indexed - using a module name to return another dictionary containing - key/value pairs with the saved state of that module. - - The keys in these latter dictionaries are arbitrary strings - and the values may either be strings or booleans. - """ - self._path = os.path.join(path, state_file) - - self.modules = {} - - self._load() - - def _load(self): - """Load the modules from the file @_path. @modules will - be an empty dictionary if the file doesn't exist. - """ - logger.debug("Loading StateFile from '%s'", self._path) - - self.modules = {} - - p = SafeConfigParser() - p.optionxform = str - p.read(self._path) - - for module in p.sections(): - self.modules[module] = {} - for (key, value) in p.items(module): - if value == str(True): - value = True - elif value == str(False): - value = False - self.modules[module][key] = value - - def save(self): - """Save the modules to @_path. If @modules is an empty - dict, then @_path should be removed. - """ - logger.debug("Saving StateFile to '%s'", self._path) - - for module in list(self.modules): - if len(self.modules[module]) == 0: - del self.modules[module] - - if len(self.modules) == 0: - logger.debug(" -> no modules, removing file") - if os.path.exists(self._path): - os.remove(self._path) - return - - p = SafeConfigParser() - p.optionxform = str - - for module in self.modules: - p.add_section(module) - for (key, value) in self.modules[module].items(): - p.set(module, key, str(value)) - - with open(self._path, "w") as f: - p.write(f) - - def backup_state(self, module, key, value): - """Backup an item of system state from @module, identified - by the string @key and with the value @value. @value may be - a string or boolean. - """ - if not isinstance(value, (str, bool, unicode)): - raise ValueError("Only strings, booleans or unicode strings are supported") - - self._load() - - if module not in self.modules: - self.modules[module] = {} - - if key not in self.modules: - self.modules[module][key] = value - - self.save() - - def get_state(self, module, key): - """Return the value of an item of system state from @module, - identified by the string @key. - - If the item doesn't exist, #None will be returned, otherwise - the original string or boolean value is returned. - """ - self._load() - - if module not in self.modules: - return None - - return self.modules[module].get(key, None) - - def delete_state(self, module, key): - """Delete system state from @module, identified by the string - @key. - - If the item doesn't exist, no change is done. - """ - self._load() - - try: - del self.modules[module][key] - except KeyError: - pass - else: - self.save() - - def restore_state(self, module, key): - """Return the value of an item of system state from @module, - identified by the string @key, and remove it from the backed - up system state. - - If the item doesn't exist, #None will be returned, otherwise - the original string or boolean value is returned. - """ - - value = self.get_state(module, key) - - if value is not None: - self.delete_state(module, key) - return value +""" +Facade for ipalib.sysrestore for backwards compatibility +""" - def has_state(self, module): - """Return True or False if there is any state stored for @module. +from ipalib import sysrestore as real_sysrestore - Can be used to determine if a service is configured. - """ +class FileStore(real_sysrestore.FileStore): + def __init__(self, path=real_sysrestore.SYSRESTORE_PATH, + index_file=real_sysrestore.SYSRESTORE_INDEXFILE): + super(FileStore, self).__init__(path, index_file) - return module in self.modules +class StateFile(real_sysrestore.StateFile): + def __init__(self, path=real_sysrestore.SYSRESTORE_PATH, + state_file=real_sysrestore.SYSRESTORE_STATEFILE): + super(StateFile, self).__init__(path, state_file) diff --git a/ipalib/sysrestore.py b/ipalib/sysrestore.py new file mode 100644 index 0000000000..5fd52b8edc --- /dev/null +++ b/ipalib/sysrestore.py @@ -0,0 +1,457 @@ +# Authors: Mark McLoughlin <mar...@redhat.com> +# +# Copyright (C) 2007 Red Hat +# see file 'COPYING' for use and warranty information +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. +# + +# +# This module provides a very simple API which allows +# ipa-xxx-install --uninstall to restore certain +# parts of the system configuration to the way it was +# before ipa-server-install was first run + +from __future__ import absolute_import + +import logging +import os +import os.path +import shutil +import random + +from hashlib import sha256 + +import six +# pylint: disable=import-error +if six.PY3: + # The SafeConfigParser class has been renamed to ConfigParser in Py3 + from configparser import ConfigParser as SafeConfigParser +else: + from ConfigParser import SafeConfigParser +# pylint: enable=import-error + +from ipaplatform.tasks import tasks +from ipaplatform.paths import paths + +if six.PY3: + unicode = str + +logger = logging.getLogger(__name__) + +SYSRESTORE_PATH = paths.TMP +SYSRESTORE_INDEXFILE = "sysrestore.index" +SYSRESTORE_STATEFILE = "sysrestore.state" + + +class FileStore: + """Class for handling backup and restore of files""" + + def __init__(self, path = SYSRESTORE_PATH, index_file = SYSRESTORE_INDEXFILE): + """Create a _StoreFiles object, that uses @path as the + base directory. + + The file @path/sysrestore.index is used to store information + about the original location of the saved files. + """ + self._path = path + self._index = os.path.join(self._path, index_file) + + self.random = random.Random() + + self.files = {} + self._load() + + def _load(self): + """Load the file list from the index file. @files will + be an empty dictionary if the file doesn't exist. + """ + + logger.debug("Loading Index file from '%s'", self._index) + + self.files = {} + + p = SafeConfigParser() + p.optionxform = str + p.read(self._index) + + for section in p.sections(): + if section == "files": + for (key, value) in p.items(section): + self.files[key] = value + + + def save(self): + """Save the file list to @_index. If @files is an empty + dict, then @_index should be removed. + """ + logger.debug("Saving Index File to '%s'", self._index) + + if len(self.files) == 0: + logger.debug(" -> no files, removing file") + if os.path.exists(self._index): + os.remove(self._index) + return + + p = SafeConfigParser() + p.optionxform = str + + p.add_section('files') + for (key, value) in self.files.items(): + p.set('files', key, str(value)) + + with open(self._index, "w") as f: + p.write(f) + + def backup_file(self, path): + """Create a copy of the file at @path - as long as an exact copy + does not already exist - which will be restored to its + original location by restore_files(). + """ + logger.debug("Backing up system configuration file '%s'", path) + + if not os.path.isabs(path): + raise ValueError("Absolute path required") + + if not os.path.isfile(path): + logger.debug(" -> Not backing up - '%s' doesn't exist", path) + return + + _reldir, backupfile = os.path.split(path) + + with open(path, 'rb') as f: + cont_hash = sha256(f.read()).hexdigest() + + filename = "{hexhash}-{bcppath}".format( + hexhash=cont_hash, bcppath=backupfile) + + backup_path = os.path.join(self._path, filename) + if os.path.exists(backup_path): + logger.debug(" -> Not backing up - already have a copy of '%s'", + path) + return + + shutil.copy2(path, backup_path) + + stat = os.stat(path) + + template = '{stat.st_mode},{stat.st_uid},{stat.st_gid},{path}' + self.files[filename] = template.format(stat=stat, path=path) + self.save() + + def has_file(self, path): + """Checks whether file at @path was added to the file store + + Returns #True if the file exists in the file store, #False otherwise + """ + result = False + for _key, value in self.files.items(): + _mode, _uid, _gid, filepath = value.split(',', 3) + if (filepath == path): + result = True + break + return result + + def restore_file(self, path, new_path = None): + """Restore the copy of a file at @path to its original + location and delete the copy. + + Takes optional parameter @new_path which specifies the + location where the file is to be restored. + + Returns #True if the file was restored, #False if there + was no backup file to restore + """ + + if new_path is None: + logger.debug("Restoring system configuration file '%s'", + path) + else: + logger.debug("Restoring system configuration file '%s' to '%s'", + path, new_path) + + if not os.path.isabs(path): + raise ValueError("Absolute path required") + if new_path is not None and not os.path.isabs(new_path): + raise ValueError("Absolute new path required") + + mode = None + uid = None + gid = None + filename = None + + for (key, value) in self.files.items(): + (mode,uid,gid,filepath) = value.split(',', 3) + if (filepath == path): + filename = key + break + + if not filename: + raise ValueError("No such file name in the index") + + backup_path = os.path.join(self._path, filename) + if not os.path.exists(backup_path): + logger.debug(" -> Not restoring - '%s' doesn't exist", + backup_path) + return False + + if new_path is not None: + path = new_path + + shutil.copy(backup_path, path) # SELinux needs copy + os.remove(backup_path) + + os.chown(path, int(uid), int(gid)) + os.chmod(path, int(mode)) + + tasks.restore_context(path) + + del self.files[filename] + self.save() + + return True + + def restore_all_files(self): + """Restore the files in the inbdex to their original + location and delete the copy. + + Returns #True if the file was restored, #False if there + was no backup file to restore + """ + + if len(self.files) == 0: + return False + + for (filename, value) in self.files.items(): + + (mode,uid,gid,path) = value.split(',', 3) + + backup_path = os.path.join(self._path, filename) + if not os.path.exists(backup_path): + logger.debug(" -> Not restoring - '%s' doesn't exist", + backup_path) + continue + + shutil.copy(backup_path, path) # SELinux needs copy + os.remove(backup_path) + + os.chown(path, int(uid), int(gid)) + os.chmod(path, int(mode)) + + tasks.restore_context(path) + + # force file to be deleted + self.files = {} + self.save() + + return True + + def has_files(self): + """Return True or False if there are any files in the index + + Can be used to determine if a program is configured. + """ + + return len(self.files) > 0 + + def untrack_file(self, path): + """Remove file at path @path from list of backed up files. + + Does not remove any files from the filesystem. + + Returns #True if the file was untracked, #False if there + was no backup file to restore + """ + + logger.debug("Untracking system configuration file '%s'", path) + + if not os.path.isabs(path): + raise ValueError("Absolute path required") + + filename = None + + for (key, value) in self.files.items(): + _mode, _uid, _gid, filepath = value.split(',', 3) + if (filepath == path): + filename = key + break + + if not filename: + raise ValueError("No such file name in the index") + + backup_path = os.path.join(self._path, filename) + if not os.path.exists(backup_path): + logger.debug(" -> Not restoring - '%s' doesn't exist", + backup_path) + return False + + try: + os.unlink(backup_path) + except Exception as e: + logger.error('Error removing %s: %s', backup_path, str(e)) + + del self.files[filename] + self.save() + + return True + + +class StateFile: + """A metadata file for recording system state which can + be backed up and later restored. + StateFile gets reloaded every time to prevent loss of information + recorded by child processes. But we do not solve concurrency + because there is no need for it right now. + The format is something like: + + [httpd] + running=True + enabled=False + """ + + def __init__(self, path = SYSRESTORE_PATH, state_file = SYSRESTORE_STATEFILE): + """Create a StateFile object, loading from @path. + + The dictionary @modules, a member of the returned object, + is where the state can be modified. @modules is indexed + using a module name to return another dictionary containing + key/value pairs with the saved state of that module. + + The keys in these latter dictionaries are arbitrary strings + and the values may either be strings or booleans. + """ + self._path = os.path.join(path, state_file) + + self.modules = {} + + self._load() + + def _load(self): + """Load the modules from the file @_path. @modules will + be an empty dictionary if the file doesn't exist. + """ + logger.debug("Loading StateFile from '%s'", self._path) + + self.modules = {} + + p = SafeConfigParser() + p.optionxform = str + p.read(self._path) + + for module in p.sections(): + self.modules[module] = {} + for (key, value) in p.items(module): + if value == str(True): + value = True + elif value == str(False): + value = False + self.modules[module][key] = value + + def save(self): + """Save the modules to @_path. If @modules is an empty + dict, then @_path should be removed. + """ + logger.debug("Saving StateFile to '%s'", self._path) + + for module in list(self.modules): + if len(self.modules[module]) == 0: + del self.modules[module] + + if len(self.modules) == 0: + logger.debug(" -> no modules, removing file") + if os.path.exists(self._path): + os.remove(self._path) + return + + p = SafeConfigParser() + p.optionxform = str + + for module in self.modules: + p.add_section(module) + for (key, value) in self.modules[module].items(): + p.set(module, key, str(value)) + + with open(self._path, "w") as f: + p.write(f) + + def backup_state(self, module, key, value): + """Backup an item of system state from @module, identified + by the string @key and with the value @value. @value may be + a string or boolean. + """ + if not isinstance(value, (str, bool, unicode)): + raise ValueError("Only strings, booleans or unicode strings are supported") + + self._load() + + if module not in self.modules: + self.modules[module] = {} + + if key not in self.modules: + self.modules[module][key] = value + + self.save() + + def get_state(self, module, key): + """Return the value of an item of system state from @module, + identified by the string @key. + + If the item doesn't exist, #None will be returned, otherwise + the original string or boolean value is returned. + """ + self._load() + + if module not in self.modules: + return None + + return self.modules[module].get(key, None) + + def delete_state(self, module, key): + """Delete system state from @module, identified by the string + @key. + + If the item doesn't exist, no change is done. + """ + self._load() + + try: + del self.modules[module][key] + except KeyError: + pass + else: + self.save() + + def restore_state(self, module, key): + """Return the value of an item of system state from @module, + identified by the string @key, and remove it from the backed + up system state. + + If the item doesn't exist, #None will be returned, otherwise + the original string or boolean value is returned. + """ + + value = self.get_state(module, key) + + if value is not None: + self.delete_state(module, key) + + return value + + def has_state(self, module): + """Return True or False if there is any state stored for @module. + + Can be used to determine if a service is configured. + """ + + return module in self.modules diff --git a/ipaserver/install/installutils.py b/ipaserver/install/installutils.py index f19f64fbe8..132122a6dd 100644 --- a/ipaserver/install/installutils.py +++ b/ipaserver/install/installutils.py @@ -43,7 +43,7 @@ import ldap import six -from ipalib.install import sysrestore +from ipalib import facts, sysrestore from ipalib.install.kinit import kinit_password import ipaplatform from ipapython import ipautil, admintool, version, ipaldap @@ -702,8 +702,12 @@ def is_ipa_configured(): """ Use the state to determine if IPA has been configured. """ - sstore = sysrestore.StateFile(paths.SYSRESTORE) - return sstore.get_state('installation', 'complete') + warnings.warn( + "Use 'ipalib.facts.is_ipa_configured'", + DeprecationWarning, + stacklevel=2 + ) + return facts.is_ipa_configured() def run_script(main_function, operation_name, log_file_name=None, diff --git a/ipaserver/install/ipa_cert_fix.py b/ipaserver/install/ipa_cert_fix.py index 6b952d34fb..1b2f5430e6 100644 --- a/ipaserver/install/ipa_cert_fix.py +++ b/ipaserver/install/ipa_cert_fix.py @@ -32,13 +32,13 @@ from ipalib import api from ipalib import x509 +from ipalib.facts import is_ipa_configured from ipaplatform.paths import paths from ipapython.admintool import AdminTool from ipapython.certdb import NSSDatabase, EMPTY_TRUST_FLAGS from ipapython.dn import DN from ipapython.ipaldap import realm_to_serverid from ipaserver.install import ca, cainstance, dsinstance -from ipaserver.install.installutils import is_ipa_configured from ipapython import ipautil msg = """ diff --git a/ipaserver/install/ipactl.py b/ipaserver/install/ipactl.py index f5113548ad..f813d0d728 100644 --- a/ipaserver/install/ipactl.py +++ b/ipaserver/install/ipactl.py @@ -27,9 +27,10 @@ from ipaserver.install import service, installutils from ipaserver.install.dsinstance import config_dirname -from ipaserver.install.installutils import is_ipa_configured, ScriptError +from ipaserver.install.installutils import ScriptError from ipaserver.masters import ENABLED_SERVICE, HIDDEN_SERVICE from ipalib import api, errors +from ipalib.facts import is_ipa_configured from ipapython.ipaldap import LDAPClient, realm_to_serverid from ipapython.ipautil import wait_for_open_ports, wait_for_open_socket from ipapython.ipautil import run diff --git a/ipaserver/install/server/install.py b/ipaserver/install/server/install.py index 4822c222ce..07fcb2f0cc 100644 --- a/ipaserver/install/server/install.py +++ b/ipaserver/install/server/install.py @@ -32,6 +32,7 @@ from ipaplatform.tasks import tasks from ipalib import api, errors, x509 from ipalib.constants import DOMAIN_LEVEL_0 +from ipalib.facts import is_ipa_configured from ipalib.util import ( validate_domain_name, no_matching_interface_for_ip_address_warning, @@ -43,8 +44,8 @@ sysupgrade) from ipaserver.install.installutils import ( IPA_MODULES, BadHostError, get_fqdn, get_server_ip_address, - is_ipa_configured, load_pkcs12, read_password, verify_fqdn, - update_hosts_file, validate_mask) + load_pkcs12, read_password, verify_fqdn, update_hosts_file, + validate_mask) if six.PY3: unicode = str diff --git a/ipaserver/install/server/replicainstall.py b/ipaserver/install/server/replicainstall.py index b8e896ac7c..fb47781e4b 100644 --- a/ipaserver/install/server/replicainstall.py +++ b/ipaserver/install/server/replicainstall.py @@ -35,13 +35,14 @@ from ipaplatform.paths import paths from ipalib import api, constants, create_api, errors, rpc, x509 from ipalib.config import Env +from ipalib.facts import is_ipa_configured from ipalib.util import no_matching_interface_for_ip_address_warning from ipaclient.install.client import configure_krb5_conf, purge_host_keytab from ipaserver.install import ( adtrust, bindinstance, ca, dns, dsinstance, httpinstance, installutils, kra, krbinstance, otpdinstance, custodiainstance, service) from ipaserver.install.installutils import ( - ReplicaConfig, load_pkcs12, is_ipa_configured, validate_mask) + ReplicaConfig, load_pkcs12, validate_mask) from ipaserver.install.replication import ( ReplicationManager, replica_conn_check) from ipaserver.masters import find_providing_servers, find_providing_server diff --git a/ipaserver/install/server/upgrade.py b/ipaserver/install/server/upgrade.py index e9a4dca315..f0d9b746cd 100644 --- a/ipaserver/install/server/upgrade.py +++ b/ipaserver/install/server/upgrade.py @@ -22,7 +22,9 @@ from ipalib import api, x509 from ipalib.constants import RENEWAL_CA_NAME, RA_AGENT_PROFILE, IPA_CA_RECORD -from ipalib.install import certmonger, sysrestore +from ipalib.install import certmonger +from ipalib import sysrestore +from ipalib.facts import is_ipa_configured import SSSDConfig import ipalib.util import ipalib.errors @@ -1453,7 +1455,7 @@ def upgrade_configuration(): fstore = sysrestore.FileStore(paths.SYSRESTORE) sstore = sysrestore.StateFile(paths.SYSRESTORE) - if installutils.is_ipa_configured() is None: + if is_ipa_configured() is None: sstore.backup_state('installation', 'complete', True) fqdn = api.env.host From 88cb713ea210df866a9b1333b2748a4b8e89b359 Mon Sep 17 00:00:00 2001 From: Rob Crittenden <rcrit...@redhat.com> Date: Tue, 28 Jul 2020 13:17:40 -0400 Subject: [PATCH 4/5] Don't use the has_files() to know if client/server is configured Use the is_ipa_configure() and is_ipa_client_configured() utilities instead which are much more robust. https://pagure.io/freeipa/issue/8384 Signed-off-by: Rob Crittenden <rcrit...@redhat.com> Reviewed-By: Alexander Bokovoy <aboko...@redhat.com> Reviewed-By: Francois Cami <fc...@redhat.com> --- ipaclient/install/client.py | 5 ++--- ipaclient/install/ipa_certupdate.py | 6 +++--- ipalib/facts.py | 1 + ipaserver/install/installutils.py | 5 ++--- ipaserver/install/server/install.py | 5 ++--- ipaserver/install/server/replicainstall.py | 5 ++--- 6 files changed, 12 insertions(+), 15 deletions(-) diff --git a/ipaclient/install/client.py b/ipaclient/install/client.py index da671361b2..ad8dfc0d5e 100644 --- a/ipaclient/install/client.py +++ b/ipaclient/install/client.py @@ -37,9 +37,9 @@ from ipalib import api, errors, x509 from ipalib import sysrestore from ipalib.constants import IPAAPI_USER, MAXHOSTNAMELEN -from ipalib.facts import is_ipa_client_configured from ipalib.install import certmonger, certstore, service from ipalib.install import hostname as hostname_ +from ipalib.facts import is_ipa_client_configured, is_ipa_configured from ipalib.install.kinit import kinit_keytab, kinit_password from ipalib.install.service import enroll_only, prepare_only from ipalib.rpc import delete_persistent_client_session_data @@ -3203,8 +3203,7 @@ def uninstall_check(options): "IPA client is not configured on this system.", rval=rval) - server_fstore = sysrestore.FileStore(paths.SYSRESTORE) - if server_fstore.has_files() and not options.on_master: + if is_ipa_configured() and not options.on_master: logger.error( "IPA client is configured as a part of IPA server on this system.") logger.info("Refer to ipa-server-install for uninstallation.") diff --git a/ipaclient/install/ipa_certupdate.py b/ipaclient/install/ipa_certupdate.py index f7a92f34f1..6f2a4c5d11 100644 --- a/ipaclient/install/ipa_certupdate.py +++ b/ipaclient/install/ipa_certupdate.py @@ -26,7 +26,8 @@ from urllib.parse import urlsplit -from ipalib.install import certmonger, certstore, sysrestore +from ipalib.install import certmonger, certstore +from ipalib.facts import is_ipa_configured from ipalib.install.kinit import kinit_keytab from ipapython import admintool, certdb, ipaldap, ipautil from ipaplatform import services @@ -104,8 +105,7 @@ def run_with_args(api): os.environ['KRB5CCNAME'] = old_krb5ccname shutil.rmtree(tmpdir) - server_fstore = sysrestore.FileStore(paths.SYSRESTORE) - if server_fstore.has_files(): + if is_ipa_configured(): update_server(certs) # pylint: disable=import-error,ipa-forbidden-import diff --git a/ipalib/facts.py b/ipalib/facts.py index a1e3e46bc9..5106fc2ac5 100644 --- a/ipalib/facts.py +++ b/ipalib/facts.py @@ -6,6 +6,7 @@ Facts about the installation """ +import os from . import sysrestore from ipaplatform.paths import paths diff --git a/ipaserver/install/installutils.py b/ipaserver/install/installutils.py index 132122a6dd..ce57772ff8 100644 --- a/ipaserver/install/installutils.py +++ b/ipaserver/install/installutils.py @@ -43,7 +43,7 @@ import ldap import six -from ipalib import facts, sysrestore +from ipalib import facts from ipalib.install.kinit import kinit_password import ipaplatform from ipapython import ipautil, admintool, version, ipaldap @@ -669,8 +669,7 @@ def check_server_configuration(): Most convenient use case for the function is in install tools that require configured IPA for its function. """ - server_fstore = sysrestore.FileStore(paths.SYSRESTORE) - if not server_fstore.has_files(): + if not is_ipa_configured(): raise ScriptError("IPA is not configured on this system.", rval=SERVER_NOT_CONFIGURED) diff --git a/ipaserver/install/server/install.py b/ipaserver/install/server/install.py index 07fcb2f0cc..3cdd1a8708 100644 --- a/ipaserver/install/server/install.py +++ b/ipaserver/install/server/install.py @@ -32,7 +32,7 @@ from ipaplatform.tasks import tasks from ipalib import api, errors, x509 from ipalib.constants import DOMAIN_LEVEL_0 -from ipalib.facts import is_ipa_configured +from ipalib.facts import is_ipa_configured, is_ipa_client_configured from ipalib.util import ( validate_domain_name, no_matching_interface_for_ip_address_warning, @@ -368,8 +368,7 @@ def install_check(installer): "If you want to reinstall the IPA server, please uninstall " "it first using 'ipa-server-install --uninstall'.") - client_fstore = sysrestore.FileStore(paths.IPA_CLIENT_SYSRESTORE) - if client_fstore.has_files(): + if is_ipa_client_configured(on_master=True): installer._installation_cleanup = False raise ScriptError( "IPA client is already configured on this system.\n" diff --git a/ipaserver/install/server/replicainstall.py b/ipaserver/install/server/replicainstall.py index fb47781e4b..2990f4382d 100644 --- a/ipaserver/install/server/replicainstall.py +++ b/ipaserver/install/server/replicainstall.py @@ -35,7 +35,7 @@ from ipaplatform.paths import paths from ipalib import api, constants, create_api, errors, rpc, x509 from ipalib.config import Env -from ipalib.facts import is_ipa_configured +from ipalib.facts import is_ipa_configured, is_ipa_client_configured from ipalib.util import no_matching_interface_for_ip_address_warning from ipaclient.install.client import configure_krb5_conf, purge_host_keytab from ipaserver.install import ( @@ -786,8 +786,7 @@ def promote_check(installer): raise ScriptError("--setup-ca and --*-cert-file options are " "mutually exclusive") - client_fstore = sysrestore.FileStore(paths.IPA_CLIENT_SYSRESTORE) - if not client_fstore.has_files(): + if not is_ipa_client_configured(on_master=True): # One-step replica installation if options.password and options.admin_password: raise ScriptError("--password and --admin-password options are " From 48a5eb78f1906d23a441fed608835ecfb57f8e3b Mon Sep 17 00:00:00 2001 From: Rob Crittenden <rcrit...@redhat.com> Date: Thu, 9 Jul 2020 10:38:42 -0400 Subject: [PATCH 5/5] Update check_client_configuration to use new client fact check_client_configuration differs from is_ipa_client_configured in that it raises an exception if not configured so is a nice convenience in AdminTool scripts. Port it to call to is_ipa_client_configured() instead of determining the install state on its own. https://pagure.io/freeipa/issue/8384 Signed-off-by: Rob Crittenden <rcrit...@redhat.com> Reviewed-By: Alexander Bokovoy <aboko...@redhat.com> Reviewed-By: Francois Cami <fc...@redhat.com> --- ipalib/util.py | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/ipalib/util.py b/ipalib/util.py index d146581d44..786cfa71eb 100644 --- a/ipalib/util.py +++ b/ipalib/util.py @@ -60,6 +60,8 @@ TLS_VERSIONS, TLS_VERSION_MINIMAL, TLS_VERSION_MAXIMAL, TLS_VERSION_DEFAULT_MIN, TLS_VERSION_DEFAULT_MAX, ) +# pylint: disable=ipa-forbidden-import +from ipalib.facts import is_ipa_client_configured from ipalib.text import _ from ipaplatform.constants import constants from ipaplatform.paths import paths @@ -1174,6 +1176,12 @@ def check_client_configuration(env=None): """ Check if IPA client is configured on the system. + This is a convenience wrapper that also supports using + a custom configuration via IPA_CONFDIR. + + Raises a ScriptError exception if the client is not + configured. + Hardcode return code to avoid recursive imports """ CLIENT_NOT_CONFIGURED = 2 @@ -1187,16 +1195,10 @@ def check_client_configuration(env=None): f'{env.confdir} is missing {env.conf_default})', CLIENT_NOT_CONFIGURED ) - elif ( - os.path.isfile(paths.IPA_DEFAULT_CONF) and - os.path.isfile( - os.path.join(paths.IPA_CLIENT_SYSRESTORE, 'sysrestore.state') - ) - ): - # standard installation, check for config and client sysrestore state + + if is_ipa_client_configured(): return True else: - # client not configured raise ScriptError( 'IPA client is not configured on this system', CLIENT_NOT_CONFIGURED
_______________________________________________ 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
