URL: https://github.com/freeipa/freeipa/pull/2346
Author: flo-renaud
 Title: #2346: [Backport][ipa-4-6]ipa-replica-install: properly use the file 
store 
Action: opened

PR body:
"""
This is a manual backport of PR #2332 on ipa-4-6. The main difference is in the 
test: in the 4-6 branch, FreeIPA is still using mod_nss and the configuration 
file is /etc/httpd/conf.d/nss.conf (instead of ssl.conf for the 4-7 and master 
branches which moved to mod-ssl).
"""

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/2346/head:pr2346
git checkout pr2346
From f33a6581d872ba803d021a66337b208d5eada7f2 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/2] 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

Reviewed-By: Christian Heimes <chei...@redhat.com>
---
 ipaserver/install/server/replicainstall.py | 31 +++++++++++++---------
 1 file changed, 18 insertions(+), 13 deletions(-)

diff --git a/ipaserver/install/server/replicainstall.py b/ipaserver/install/server/replicainstall.py
index 746b4a99f1..eb354f81ba 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,
@@ -173,14 +175,14 @@ def install_http(config, auto_redirect, ca_is_configured, ca_file,
     return http
 
 
-def install_dns_records(config, options, remote_api):
+def install_dns_records(config, options, remote_api, fstore=None):
 
     if not bindinstance.dns_container_exists(
             ipautil.realm_to_suffix(config.realm_name)):
         return
 
     try:
-        bind = bindinstance.BindInstance(api=remote_api)
+        bind = bindinstance.BindInstance(api=remote_api, fstore=fstore)
         for ip in config.ips:
             reverse_zone = bindinstance.find_reverse_zone(ip, remote_api)
 
@@ -1425,10 +1427,11 @@ 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)
+        install_dns_records(config, options, remote_api, fstore=fstore)
 
         ntpinstance.ntp_ldap_enable(config.host_name, ds.suffix,
                                     remote_api.env.realm)
@@ -1449,7 +1452,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
@@ -1479,7 +1483,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 62bf624053d26a86a93560b7e5c669c5ba0c3e73 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/2] Test: scenario replica install/uninstall should restore
 nss.conf

Test that the scenario ipa-replica-install/ uninstall correctly
restores the file /etc/httpd/conf.d/nss.conf

Related to https://pagure.io/freeipa/issue/7684

Reviewed-By: Christian Heimes <chei...@redhat.com>
---
 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..7859ded239 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 nss.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'
+        nss_conf = self.replicas[0].get_file_contents(paths.HTTPD_NSS_CONF)
+        assert errline not in nss_conf
+
     def test_failed_uninstall(self):
         self.master.run_command(['ipactl', 'stop'])
 
_______________________________________________
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

Reply via email to