From b9e7c6316678ebf873e85d6cda9dac8d29b962af Mon Sep 17 00:00:00 2001 From: Rob Crittenden Date: Fri, 1 Dec 2023 08:51:05 -0500 Subject: [PATCH 1/3] Fix ipa-client-automount install/uninstall with new install states Issue 8384 introduced a new installation state for the statestore to identify when client/server installation is completely finished rather than relying on has_files(). The problem is that ipa-client-automount may be called during ipa-client-install and since installation is not complete at that point the automount install was failing with "IPA client not configured". Add a new state, 'automount', to designate that automount installation is in process. If check_client_configuration() fails it checks to see if [installation] automount is True. If so it continues with the installation. This also addresses an issue where the filestore and statestore are shared between the client and automount installers but the client wasn't refreshing state after automount completed. This resulted in an incomplete state and index file of backed-up files which caused files to not be restored on uninstall and the state file to be orphaned. Fixes: https://pagure.io/freeipa/issue/9487 Signed-off-by: Rob Crittenden --- ipaclient/install/client.py | 14 ++++++++++++-- ipaclient/install/ipa_client_automount.py | 14 ++++++++------ 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/ipaclient/install/client.py b/ipaclient/install/client.py index 619afc03362..1d816f4d816 100644 --- a/ipaclient/install/client.py +++ b/ipaclient/install/client.py @@ -1274,7 +1274,7 @@ def create_sshd_ipa_config(options): logger.info('Configured %s', paths.SSHD_IPA_CONFIG) -def configure_automount(options): +def configure_automount(options, statestore): logger.info('\nConfiguring automount:') args = [ @@ -1287,12 +1287,15 @@ def configure_automount(options): if not options.sssd: args.append('--no-sssd') + statestore.backup_state('installation', 'automount', True) try: result = run(args) except Exception as e: logger.error('Automount configuration failed: %s', str(e)) else: logger.info('%s', result.output_log) + finally: + statestore.delete_state('installation', 'automount') def configure_nisdomain(options, domain, statestore): @@ -3306,7 +3309,11 @@ def _install(options, tdict): configure_sshd_config(fstore, options) if options.location: - configure_automount(options) + configure_automount(options, statestore) + + # Reload the state as automount install may have modified it + fstore._load() + statestore._load() if options.configure_firefox: configure_firefox(options, statestore, cli_domain) @@ -3369,12 +3376,15 @@ def uninstall(options): fstore = sysrestore.FileStore(paths.IPA_CLIENT_SYSRESTORE) statestore = sysrestore.StateFile(paths.IPA_CLIENT_SYSRESTORE) + statestore.backup_state('installation', 'automount', True) try: run([paths.IPA_CLIENT_AUTOMOUNT, "--uninstall", "--debug"]) except CalledProcessError as e: if e.returncode != CLIENT_NOT_CONFIGURED: logger.error( "Unconfigured automount client failed: %s", str(e)) + finally: + statestore.delete_state('installation', 'automount') # Reload the state as automount unconfigure may have modified it fstore._load() diff --git a/ipaclient/install/ipa_client_automount.py b/ipaclient/install/ipa_client_automount.py index b4b3387530a..ee27872868b 100644 --- a/ipaclient/install/ipa_client_automount.py +++ b/ipaclient/install/ipa_client_automount.py @@ -340,14 +340,16 @@ def configure_nfs(fstore, statestore, options): def configure_automount(): - try: - check_client_configuration() - except ScriptError as e: - print(e.msg) - sys.exit(e.rval) + statestore = sysrestore.StateFile(paths.IPA_CLIENT_SYSRESTORE) + if not statestore.get_state('installation', 'automount'): + # not called from ipa-client-install + try: + check_client_configuration() + except ScriptError as e: + print(e.msg) + sys.exit(e.rval) fstore = sysrestore.FileStore(paths.IPA_CLIENT_SYSRESTORE) - statestore = sysrestore.StateFile(paths.IPA_CLIENT_SYSRESTORE) options, _args = parse_options() From 44433d72adf800263bdbc9142e206661f6ba05cb Mon Sep 17 00:00:00 2001 From: Rob Crittenden Date: Fri, 1 Dec 2023 10:47:24 -0500 Subject: [PATCH 2/3] ipatests: Test client install/uninstall with automount enabled The automount installation was failing. Confirm that it is fixed. The uninstall was not restoring all files/configuration. Verify that the index and state files are gone which means that all state and files were restored. Fixes: https://pagure.io/freeipa/issue/9487 Signed-off-by: Rob Crittenden --- .../test_installation_client.py | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/ipatests/test_integration/test_installation_client.py b/ipatests/test_integration/test_installation_client.py index 56e1593bfcf..f8567b39eea 100644 --- a/ipatests/test_integration/test_installation_client.py +++ b/ipatests/test_integration/test_installation_client.py @@ -8,12 +8,14 @@ from __future__ import absolute_import +import os import pytest import re import shlex import textwrap from ipaplatform.paths import paths +from ipalib.sysrestore import SYSRESTORE_STATEFILE, SYSRESTORE_INDEXFILE from ipatests.test_integration.base import IntegrationTest from ipatests.pytest_ipa.integration import tasks from ipatests.pytest_ipa.integration.firewall import Firewall @@ -90,6 +92,29 @@ def test_client_install_with_krb5(self): assert 'includedir {dir}'.format( dir=paths.SSSD_PUBCONF_KRB5_INCLUDE_D_DIR ).encode() not in krb5_cfg + tasks.uninstall_client(self.clients[0]) + + def test_install_with_automount(self): + """Test that installation with automount is successful""" + tasks.install_client(self.master, self.clients[0], + extra_args=['--automount-location', 'default']) + + def test_uninstall_with_automount(self): + """Test that uninstall with automount is successful and complete""" + tasks.uninstall_client(self.clients[0]) + index = os.path.join( + paths.IPA_CLIENT_SYSRESTORE, SYSRESTORE_INDEXFILE + ) + state = os.path.join( + paths.IPA_CLIENT_SYSRESTORE, SYSRESTORE_STATEFILE + ) + for filepath in (index, state): + try: + self.clients[0].get_file_contents(filepath) + except IOError: + pass + else: + pytest.fail("The client file %s was not removed" % filepath) class TestClientInstallBind(IntegrationTest): From 677178368261090cfedc5da2b3a4599e814dc1c8 Mon Sep 17 00:00:00 2001 From: Rob Crittenden Date: Fri, 1 Dec 2023 09:08:48 -0500 Subject: [PATCH 3/3] ipa-client-automount: Don't use deprecated ipadiscovery.IPADiscovery This class was moved to ipaclient/discovery.py in e6d560af66 to make it available to PyPI. Related: https://pagure.io/freeipa/issue/9487 Signed-off-by: Rob Crittenden --- ipaclient/install/ipa_client_automount.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/ipaclient/install/ipa_client_automount.py b/ipaclient/install/ipa_client_automount.py index ee27872868b..297a784c4e1 100644 --- a/ipaclient/install/ipa_client_automount.py +++ b/ipaclient/install/ipa_client_automount.py @@ -36,7 +36,7 @@ from optparse import OptionParser # pylint: disable=deprecated-module from ipapython import ipachangeconf -from ipaclient.install import ipadiscovery +from ipaclient import discovery from ipaclient.install.client import ( CLIENT_NOT_CONFIGURED, CLIENT_ALREADY_CONFIGURED, @@ -384,12 +384,12 @@ def configure_automount(): sys.exit(CLIENT_ALREADY_CONFIGURED) autodiscover = False - ds = ipadiscovery.IPADiscovery() + ds = discovery.IPADiscovery() if not options.server: print("Searching for IPA server...") ret = ds.search(ca_cert_path=ca_cert_path) logger.debug('Executing DNS discovery') - if ret == ipadiscovery.NO_LDAP_SERVER: + if ret == discovery.NO_LDAP_SERVER: logger.debug('Autodiscovery did not find LDAP server') s = urlsplit(api.env.xmlrpc_uri) server = [s.netloc] @@ -409,14 +409,14 @@ def configure_automount(): server = options.server logger.debug("Verifying that %s is an IPA server", server) ldapret = ds.ipacheckldap(server, api.env.realm, ca_cert_path) - if ldapret[0] == ipadiscovery.NO_ACCESS_TO_LDAP: + if ldapret[0] == discovery.NO_ACCESS_TO_LDAP: print("Anonymous access to the LDAP server is disabled.") print("Proceeding without strict verification.") print( "Note: This is not an error if anonymous access has been " "explicitly restricted." ) - elif ldapret[0] == ipadiscovery.NO_TLS_LDAP: + elif ldapret[0] == discovery.NO_TLS_LDAP: logger.warning("Unencrypted access to LDAP is not supported.") elif ldapret[0] != 0: sys.exit('Unable to confirm that %s is an IPA server' % server)