CLOS-2132: Detect postgresql.conf unix_socket_directories pre-upgrade and add explicit migration steps

repos/system_upgrade/el7toel8/actors/postgresqlcheck/libraries/postgresqlcheck.py

@@ -1,8 +1,18 @@
+import os
+import re
+
 from leapp import reporting
 from leapp.libraries.common.rpms import has_package
 from leapp.libraries.stdlib import api
 from leapp.models import DistributionSignedRPM
 
+POSTGRESQL_CONF_PATH = '/var/lib/pgsql/data/postgresql.conf'
+
+# Matches the start of an active (uncommented) setting line for
+# unix_socket_directories. PostgreSQL config syntax allows optional leading
+# whitespace; a line is a comment if its first non-whitespace char is '#'.
+_UNIX_SOCKET_DIRECTORIES_RE = re.compile(r'^\s*unix_socket_directories\s*=')
+
 # Summary for postgresql-server report
 report_server_inst_summary = (
     'PostgreSQL server component will be upgraded. Since RHEL-8 includes'
@@ -12,9 +22,19 @@
 )
 
 report_server_inst_hint = (
-    'Back up your data before proceeding with the upgrade'
-    ' and follow steps in the documentation section "Migrating to a RHEL 8 version of PostgreSQL"'
-    ' after the upgrade.'
+    'Back up your data before proceeding with the upgrade.'
+    ' The upgrade keeps your PostgreSQL data directory in the old (9.2)'
+    ' on-disk format - postgresql will refuse to start on the upgraded'
+    ' system until you migrate the data. After the upgrade completes:\n'
+    '  1. Install the postgresql-upgrade package (provides the old'
+    ' server binaries needed by the migration helper):\n'
+    '       dnf install postgresql-upgrade\n'
+    '  2. Migrate the database files to the new format:\n'
+    '       postgresql-setup --upgrade\n'
+    '  3. Start PostgreSQL:\n'
+    '       systemctl start postgresql\n'
+    'For more details see the documentation section'
+    ' "Migrating to a RHEL 8 version of PostgreSQL".'
 )
 
 # Link URL for postgresql-server report
@@ -31,6 +51,36 @@
     .format(''.join(['\n    - {}'.format(i) for i in report_contrib_inst_dropext]))
 )
 
+# Summary / remediation for the unix_socket_directories config-incompat report.
+# RHEL-7's PG 9.2 ships with a forward-compatible patch that accepts the newer
+# (PG 9.3+) plural parameter name unix_socket_directories. RHEL-8's
+# postgresql-upgrade package ships an unpatched 9.2 server binary that rejects
+# the plural form with a FATAL config error and aborts on startup; that breaks
+# postgresql-setup --upgrade post-Elevate. The default CL7 postgresql.conf has
+# this line commented out by default, but admin edits, cPanel tooling, and
+# config-management commonly uncomment or rewrite it.
+report_plural_socket_param_summary = (
+    'PostgreSQL configuration file {conf} contains an active'
+    ' "unix_socket_directories" setting. RHEL-7 PostgreSQL 9.2 accepts this'
+    ' plural parameter name (added in PG 9.3 and back-ported by RHEL into 9.2),'
+    ' but the unpatched 9.2 server binary shipped in RHEL-8\'s postgresql-upgrade'
+    ' package does not. As a result, "postgresql-setup --upgrade" will fail'
+    ' post-upgrade with "unrecognized configuration parameter'
+    ' unix_socket_directories" and PostgreSQL will refuse to start on the'
+    ' upgraded system until the parameter is renamed.'
+).format(conf=POSTGRESQL_CONF_PATH)
+
+report_plural_socket_param_hint = (
+    'Before running the upgrade, rename the parameter on the affected line of'
+    ' {conf} from "unix_socket_directories" (plural) to "unix_socket_directory"'
+    ' (singular). For example:\n'
+    '       sed -i \'s/^unix_socket_directories/unix_socket_directory/\' {conf}\n'
+    'If you have already upgraded and PostgreSQL is failing to start, apply the'
+    ' same rename to the same file (which post-upgrade may have been renamed to'
+    ' /var/lib/pgsql/data-old/postgresql.conf by postgresql-setup), then re-run:\n'
+    '       postgresql-setup --upgrade'
+).format(conf=POSTGRESQL_CONF_PATH)
+
 
 def _report_server_installed():
     """
@@ -69,13 +119,60 @@ def _report_contrib_installed():
         ])
 
 
+def _postgresql_conf_uses_plural_socket_param(path=POSTGRESQL_CONF_PATH):
+    """
+    Return True if `path` contains an active (uncommented) line setting the
+    plural parameter name `unix_socket_directories`.
+
+    Missing files or unreadable lines are treated as "not affected" - the
+    caller already knows postgresql-server is installed, but the data dir may
+    never have been initialized (initdb not yet run), in which case no
+    postgresql.conf exists. We don't want to scare users with a warning for
+    a state that can't trigger the bug.
+    """
+    if not os.path.isfile(path):
+        return False
+    try:
+        with open(path, 'r') as fp:
+            for raw_line in fp:
+                if _UNIX_SOCKET_DIRECTORIES_RE.match(raw_line):
+                    return True
+    except (IOError, OSError):
+        # Permissions or transient I/O issue. Conservatively return False;
+        # if the file truly contains the bad line, postgresql-setup --upgrade
+        # will surface it after the upgrade and the generic hint covers it.
+        return False
+    return False
+
+
+def _report_plural_socket_param_detected():
+    """
+    Create report when postgresql.conf contains an active
+    unix_socket_directories (plural) setting, which will break the post-upgrade
+    data migration. See module docstring for the full mechanism.
+    """
+    reporting.create_report([
+        reporting.Title(
+            'PostgreSQL configuration uses "unix_socket_directories"'
+            ' which will break post-upgrade data migration'
+        ),
+        reporting.Summary(report_plural_socket_param_summary),
+        reporting.Severity(reporting.Severity.HIGH),
+        reporting.Groups([reporting.Groups.SERVICES]),
+        reporting.RelatedResource('package', 'postgresql-server'),
+        reporting.RelatedResource('file', POSTGRESQL_CONF_PATH),
+        reporting.Remediation(hint=report_plural_socket_param_hint),
+        ])
+
+
 def report_installed_packages(_context=api):
     """
     Create reports according to detected PostgreSQL packages.
 
     Create the report if the postgresql-server rpm (RH signed) is installed.
     Additionally, create another report if the postgresql-contrib rpm
-    is installed.
+    is installed, and another if postgresql.conf contains an active
+    unix_socket_directories (plural) setting.
     """
     has_server = has_package(DistributionSignedRPM, 'postgresql-server', context=_context)
     has_contrib = has_package(DistributionSignedRPM, 'postgresql-contrib', context=_context)
@@ -86,3 +183,5 @@ def report_installed_packages(_context=api):
         if has_contrib:
             # postgresql-contrib
             _report_contrib_installed()
+        if _postgresql_conf_uses_plural_socket_param():
+            _report_plural_socket_param_detected()

repos/system_upgrade/el7toel8/actors/postgresqlcheck/tests/test_postgresqlcheck.py

@@ -1,7 +1,11 @@
 import pytest
 
 from leapp import reporting
-from leapp.libraries.actor.postgresqlcheck import report_installed_packages
+from leapp.libraries.actor import postgresqlcheck
+from leapp.libraries.actor.postgresqlcheck import (
+    _postgresql_conf_uses_plural_socket_param,
+    report_installed_packages,
+)
 from leapp.libraries.common.testutils import create_report_mocked, CurrentActorMocked
 from leapp.libraries.stdlib import api
 from leapp.models import DistributionSignedRPM, RPM
@@ -58,6 +62,10 @@ def test_actor_execution(monkeypatch, has_server, has_contrib):
     curr_actor_mocked = CurrentActorMocked(msgs=[DistributionSignedRPM(items=rpms)])
     monkeypatch.setattr(api, 'current_actor', curr_actor_mocked)
     monkeypatch.setattr(reporting, "create_report", create_report_mocked())
+    # Default: no postgresql.conf scan match (covered separately below).
+    monkeypatch.setattr(
+        postgresqlcheck, '_postgresql_conf_uses_plural_socket_param', lambda: False
+    )
 
     # Executed actor fed with out fake RPMs
     report_installed_packages(_context=api)
@@ -71,3 +79,91 @@ def test_actor_execution(monkeypatch, has_server, has_contrib):
     else:
         # Assert for no postgresql packages installed
         assert not reporting.create_report.called
+
+
+def test_plural_socket_param_report_fires_only_when_server_present(monkeypatch):
+    """
+    When postgresql-server is installed AND postgresql.conf has the active
+    plural form, we expect TWO reports (server-installed + plural-param).
+    When postgresql-server is absent, the conf scan must not trigger any
+    report - the bug is irrelevant without the server.
+    """
+    monkeypatch.setattr(reporting, "create_report", create_report_mocked())
+    monkeypatch.setattr(
+        postgresqlcheck, '_postgresql_conf_uses_plural_socket_param', lambda: True
+    )
+
+    # With postgresql-server installed: server report + plural-param report.
+    rpms_with_server = [
+        _generate_rpm_with_name('sed'),
+        _generate_rpm_with_name('postgresql-server'),
+    ]
+    monkeypatch.setattr(
+        api, 'current_actor',
+        CurrentActorMocked(msgs=[DistributionSignedRPM(items=rpms_with_server)]),
+    )
+    report_installed_packages(_context=api)
+    assert reporting.create_report.called == 2
+
+    # Reset reports between runs.
+    monkeypatch.setattr(reporting, "create_report", create_report_mocked())
+    monkeypatch.setattr(
+        postgresqlcheck, '_postgresql_conf_uses_plural_socket_param', lambda: True
+    )
+
+    # Without postgresql-server, the plural-param report must NOT fire even
+    # if a file matching the regex somehow existed.
+    rpms_no_server = [_generate_rpm_with_name('sed')]
+    monkeypatch.setattr(
+        api, 'current_actor',
+        CurrentActorMocked(msgs=[DistributionSignedRPM(items=rpms_no_server)]),
+    )
+    report_installed_packages(_context=api)
+    assert not reporting.create_report.called
+
+
+# Each entry: (postgresql.conf body, expected match boolean, description)
+_CONF_BODY_CASES = [
+    # The active uncommented line in plural form must be detected.
+    ("unix_socket_directories = '/var/run/postgresql, /tmp'\n", True, 'active plural'),
+    # Leading whitespace is allowed for PG config syntax.
+    ("   unix_socket_directories = '/tmp'\n", True, 'leading whitespace'),
+    # Commented-out plural is the default and must NOT trigger.
+    ("#unix_socket_directories = '/var/run/postgresql, /tmp'\n", False, 'commented default'),
+    # Singular (the form PG 9.2 / postgresql-upgrade actually accepts) - no warning.
+    ("unix_socket_directory = '/var/run/postgresql'\n", False, 'singular form'),
+    # Plural appearing inside a comment about the parameter shouldn't trigger.
+    ("# see unix_socket_directories in the docs\n", False, 'mention in comment'),
+    # Empty file.
+    ("", False, 'empty file'),
+    # Multi-line config with the bad line buried.
+    (
+        "# header\n"
+        "shared_buffers = 128MB\n"
+        "unix_socket_directories = '/var/run/postgresql'\n"
+        "max_connections = 100\n",
+        True,
+        'mixed multi-line',
+    ),
+]
+
+
+@pytest.mark.parametrize('body,expected,desc', _CONF_BODY_CASES)
+def test_postgresql_conf_uses_plural_socket_param(tmp_path, body, expected, desc):
+    """
+    The conf-scan helper must:
+      - return True ONLY for an active (uncommented) "unix_socket_directories"
+        line, with or without leading whitespace.
+      - return False for commented references, the singular form, missing
+        files, and unrelated content. False positives here would inhibit
+        upgrades unnecessarily.
+    """
+    conf = tmp_path / 'postgresql.conf'
+    conf.write_text(body)
+    assert _postgresql_conf_uses_plural_socket_param(str(conf)) is expected, desc
+
+
+def test_postgresql_conf_uses_plural_socket_param_missing_file(tmp_path):
+    """A non-existent postgresql.conf must NOT raise and must report False."""
+    missing = tmp_path / 'does-not-exist' / 'postgresql.conf'
+    assert _postgresql_conf_uses_plural_socket_param(str(missing)) is False