URL: https://github.com/freeipa/freeipa/pull/2332 Author: flo-renaud Title: #2332: ipa-replica-install: properly use the file store Action: opened
PR body: """ ### ipa-replica-install: properly use the file store In ipa-replica-install, many components use their own instance of the FileStore to backup configuration files to the pre-install state. This causes issues when the calls are mixed, like for instance: ds.do_task1_that_backups_file (using ds.filestore) http.do_task2_that_backups_file (using http.filestore) ds.do_task3_that_backups_file (using ds.filestore) because the list of files managed by ds.filestore does not include the files managed by http.filestore, and the 3rd call would remove any file added on 2nd call. The symptom of this bug is that ipa-replica-install does not save /etc/httpd/conf.d/ssl.conf and subsequent uninstallation does not restore the file, leading to a line referring to ipa-rewrite.conf that prevents httpd startup. The installer should consistently use the same filestore. Fixes https://pagure.io/freeipa/issue/7684 ### Test: scenario replica install/uninstall should restore ssl.conf Test that the scenario ipa-replica-install/ uninstall correctly restores the file /etc/httpd/conf.d/ssl.conf Related to https://pagure.io/freeipa/issue/7684 """ To pull the PR as Git branch: git remote add ghfreeipa https://github.com/freeipa/freeipa git fetch ghfreeipa pull/2332/head:pr2332 git checkout pr2332
From 9f54758598b0d74c5ede2c2ae95ad54b87c0f735 Mon Sep 17 00:00:00 2001 From: Florence Blanc-Renaud <f...@redhat.com> Date: Wed, 5 Sep 2018 17:36:16 +0200 Subject: [PATCH 1/3] ipa-replica-install: properly use the file store In ipa-replica-install, many components use their own instance of the FileStore to backup configuration files to the pre-install state. This causes issues when the calls are mixed, like for instance: ds.do_task1_that_backups_file (using ds.filestore) http.do_task2_that_backups_file (using http.filestore) ds.do_task3_that_backups_file (using ds.filestore) because the list of files managed by ds.filestore does not include the files managed by http.filestore, and the 3rd call would remove any file added on 2nd call. The symptom of this bug is that ipa-replica-install does not save /etc/httpd/conf.d/ssl.conf and subsequent uninstallation does not restore the file, leading to a line referring to ipa-rewrite.conf that prevents httpd startup. The installer should consistently use the same filestore. Fixes https://pagure.io/freeipa/issue/7684 --- ipaserver/install/server/replicainstall.py | 25 +++++++++++++--------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/ipaserver/install/server/replicainstall.py b/ipaserver/install/server/replicainstall.py index 95671aa16e..e45dc8fbf9 100644 --- a/ipaserver/install/server/replicainstall.py +++ b/ipaserver/install/server/replicainstall.py @@ -79,7 +79,7 @@ def make_pkcs12_info(directory, cert_name, password_name): def install_replica_ds(config, options, ca_is_configured, remote_api, - ca_file, promote=False, pkcs12_info=None): + ca_file, promote=False, pkcs12_info=None, fstore=None): dsinstance.check_ports() # if we have a pkcs12 file, create the cert db from @@ -95,7 +95,8 @@ def install_replica_ds(config, options, ca_is_configured, remote_api, ca_subject = installutils.default_ca_subject_dn(config.subject_base) ds = dsinstance.DsInstance( - config_ldif=options.dirsrv_config_file) + config_ldif=options.dirsrv_config_file, + fstore=fstore) ds.create_replica( realm_name=config.realm_name, master_fqdn=config.master_host_name, @@ -115,8 +116,9 @@ def install_replica_ds(config, options, ca_is_configured, remote_api, return ds -def install_krb(config, setup_pkinit=False, pkcs12_info=None, promote=False): - krb = krbinstance.KrbInstance() +def install_krb(config, setup_pkinit=False, pkcs12_info=None, promote=False, + fstore=None): + krb = krbinstance.KrbInstance(fstore=fstore) # pkinit files if pkcs12_info is None: @@ -153,7 +155,8 @@ def install_ca_cert(ldap, base_dn, realm, cafile, destfile=paths.IPA_CA_CRT): def install_http(config, auto_redirect, ca_is_configured, ca_file, promote=False, - pkcs12_info=None): + pkcs12_info=None, + fstore=None): # if we have a pkcs12 file, create the cert db from # that. Otherwise the ds setup will create the CA # cert @@ -161,8 +164,7 @@ def install_http(config, auto_redirect, ca_is_configured, ca_file, pkcs12_info = make_pkcs12_info(config.dir, "httpcert.p12", "http_pin.txt") - - http = httpinstance.HTTPInstance() + http = httpinstance.HTTPInstance(fstore=fstore) http.create_instance( config.realm_name, config.host_name, config.domain_name, config.dirman_password, pkcs12_info, @@ -1431,7 +1433,8 @@ def install(installer): remote_api, ca_file=cafile, promote=promote, - pkcs12_info=dirsrv_pkcs12_info) + pkcs12_info=dirsrv_pkcs12_info, + fstore=fstore) # Always try to install DNS records install_dns_records(config, options, remote_api) @@ -1453,7 +1456,8 @@ def install(installer): config, setup_pkinit=not options.no_pkinit, pkcs12_info=pkinit_pkcs12_info, - promote=promote) + promote=promote, + fstore=fstore) if promote: # We need to point to the master when certmonger asks for @@ -1483,7 +1487,8 @@ def install(installer): promote=promote, pkcs12_info=http_pkcs12_info, ca_is_configured=ca_enabled, - ca_file=cafile) + ca_file=cafile, + fstore=fstore) if promote: # Need to point back to ourself after the cert for HTTP is obtained From aa643a0539b27023046913c7d0fe5892c8bc81b4 Mon Sep 17 00:00:00 2001 From: Florence Blanc-Renaud <f...@redhat.com> Date: Wed, 5 Sep 2018 17:42:38 +0200 Subject: [PATCH 2/3] Test: scenario replica install/uninstall should restore ssl.conf Test that the scenario ipa-replica-install/ uninstall correctly restores the file /etc/httpd/conf.d/ssl.conf Related to https://pagure.io/freeipa/issue/7684 --- ipatests/test_integration/test_uninstallation.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/ipatests/test_integration/test_uninstallation.py b/ipatests/test_integration/test_uninstallation.py index 0d4b013830..96c4c2ef96 100644 --- a/ipatests/test_integration/test_uninstallation.py +++ b/ipatests/test_integration/test_uninstallation.py @@ -49,6 +49,18 @@ def test_uninstall_client_invalid_hostname(self): ['ls', restore_state_path], raiseonerr=False) assert 'No such file or directory' in result.stderr_text + def test_install_uninstall_replica(self): + # Test that the sequence install replica / uninstall replica + # properly removes the line + # Include /etc/httpd/conf.d/ipa-rewrite.conf + # from ssl.conf on the replica + tasks.install_replica(self.master, self.replicas[0], + extra_args=['--force-join']) + tasks.uninstall_replica(self.master, self.replicas[0]) + errline = b'Include /etc/httpd/conf.d/ipa-rewrite.conf' + ssl_conf = self.replicas[0].get_file_contents(paths.HTTPD_SSL_CONF) + assert errline not in ssl_conf + def test_failed_uninstall(self): self.master.run_command(['ipactl', 'stop']) From c4559a06180928d3e9a1840eb2aac0af58c46895 Mon Sep 17 00:00:00 2001 From: Florence Blanc-Renaud <f...@redhat.com> Date: Wed, 5 Sep 2018 18:48:56 +0200 Subject: [PATCH 3/3] temp commit to launch test_integration.py::TestUninstallBase::test_install_uninstall_replica Please remove before pushing --- ipatests/prci_definitions/gating.yaml | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/ipatests/prci_definitions/gating.yaml b/ipatests/prci_definitions/gating.yaml index 362f84e308..f54d7ed8b8 100644 --- a/ipatests/prci_definitions/gating.yaml +++ b/ipatests/prci_definitions/gating.yaml @@ -242,3 +242,15 @@ jobs: template: *ci-master-f28 timeout: 3600 topology: *master_1repl + + fedora-28/test_repl_uninstallation: + requires: [fedora-28/build] + priority: 50 + job: + class: RunPytest + args: + build_url: '{fedora-28/build_url}' + test_suite: test_integration/test_uninstallation.py::test_install_uninstall_replica + template: *ci-master-f28 + timeout: 7200 + topology: *master_1repl
_______________________________________________ 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://getfedora.org/code-of-conduct.html List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/freeipa-devel@lists.fedorahosted.org
