Update pam_userdb database backend for RHEL10 #1289

ikerexxe · 2024-08-28T10:08:45Z

pam_userdb module changed its backend database technology from libdb to
gdbm for RHEL10. This requires a set of leapp actors to perform the
database migration automatically when upgrading to RHEL10.

Three actors were created: PAM service folder scan to detect the location of the
database, database location reporting and conversion to the new format.

The changes include unit and component tests to.

Jira: SSSD-7379

github-actions · 2024-08-28T10:08:56Z

Thank you for contributing to the Leapp project!

Please note that every PR needs to comply with the Leapp Guidelines and must pass all tests in order to be mergeable.
If you want to request a review or rebuild a package in copr, you can use following commands as a comment:

review please @oamg/developers to notify leapp developers of the review request
/packit copr-build to submit a public copr build using packit

Packit will automatically schedule regression tests for this PR's build and latest upstream leapp build.
However, here are additional useful commands for packit:

/packit test to re-run manually the default tests
/packit retest-failed to re-run failed tests manually
/packit test oamg/leapp#42 to run tests with leapp builds for the leapp PR#42 (default is latest upstream - master - build)

Note that first time contributors cannot run tests automatically - they need to be started by a reviewer.

It is possible to schedule specific on-demand tests as well. Currently 2 test sets are supported, beaker-minimal and kernel-rt, both can be used to be run on all upgrade paths or just a couple of specific ones.
To launch on-demand tests with packit:

/packit test --labels kernel-rt to schedule kernel-rt tests set for all upgrade paths
/packit test --labels beaker-minimal-8.10to9.4,kernel-rt-8.10to9.4 to schedule kernel-rt and beaker-minimal test sets for 8.10->9.4 upgrade path

See other labels for particular jobs defined in the .packit.yaml file.

Please open ticket in case you experience technical problem with the CI. (RH internal only)

Note: In case there are problems with tests not being triggered automatically on new PR/commit or pending for a long time, please contact leapp-infra.

matejmatuska

Thanks for the contribution. There are a few blockers that need to be addressed. And also the question of in which phase should the prepare_pam_user_db actor run must be solved

repos/system_upgrade/el9toel10/actors/scanpamuserdb/tests/files/pam_userdb_basic

repos/system_upgrade/el9toel10/actors/scanpamuserdb/libraries/scanpamuserdb.py

repos/system_upgrade/el9toel10/actors/scanpamuserdb/tests/test_scanpamuserdb.py

repos/system_upgrade/el9toel10/actors/checkpamuserdb/libraries/checkpamuserdb.py

repos/system_upgrade/el9toel10/actors/preparepamuserdb/actor.py

repos/system_upgrade/el9toel10/actors/preparepamuserdb/libraries/preparepamuserdb.py

repos/system_upgrade/el9toel10/actors/preparepamuserdb/tests/test_preparepamuserdb.py

repos/system_upgrade/el9toel10/actors/preparepamuserdb/actor.py

ikerexxe · 2024-08-29T11:03:25Z

@matejmatuska the code has been updated taking into account your feedback. Looking forward to your next round of reviews

matejmatuska

Most of the comments were addressed, some are still left e.g. the OSError and some are new.

We will further discuss error handling of the db_converter call with @ikerexxe, because there might be some details I might have overlooked.

I have yet to test this, I will add a comment when I do

repos/system_upgrade/el9toel10/actors/checkpamuserdb/libraries/checkpamuserdb.py

repos/system_upgrade/el9toel10/actors/convertpamuserdb/libraries/convertpamuserdb.py

repos/system_upgrade/el9toel10/actors/convertpamuserdb/tests/test_convertpamuserdb.py

repos/system_upgrade/el9toel10/actors/preparepamuserdb/libraries/preparepamuserdb.py

matejmatuska

Added a couple more comments.
Also since this has grown to 4 actors, can you move them into a subdirectory in .../el9toel10/actors/? It could be named pamuserdb or maybe even just pam.

repos/system_upgrade/el9toel10/actors/removeoldpamuserdb/libraries/removeoldpamuserdb.py

repos/system_upgrade/el9toel10/actors/convertpamuserdb/libraries/convertpamuserdb.py

repos/system_upgrade/el9toel10/actors/checkpamuserdb/libraries/checkpamuserdb.py

repos/system_upgrade/el9toel10/actors/convertpamuserdb/libraries/convertpamuserdb.py

ikerexxe · 2024-09-04T14:19:49Z

Added a couple more comments. Also since this has grown to 4 actors, can you move them into a subdirectory in .../el9toel10/actors/? It could be named pamuserdb or maybe even just pam.

I prefer to use pamuserdb as we don't know if we'll need to add additional actors for other modules.

matejmatuska · 2024-09-05T09:25:43Z

/packit test

matejmatuska · 2024-09-05T09:53:39Z

/packit build

matejmatuska · 2024-09-05T09:56:32Z

@ikerexxe Linters report wrong imports order, can you fix that? We have make lint_fix that should take care of it.

I am now running packit builds and will run packit test again then (first time they failed because there were no builds)

ikerexxe · 2024-09-05T11:09:04Z

@ikerexxe Linters report wrong imports order, can you fix that? We have make lint_fix that should take care of it.

Running make lint* gets stuck in my computer. I've run isort manually and that should fix those problems. Is there any other linter I should be aware of?

matejmatuska · 2024-09-06T07:30:15Z

/packit build

pirat89 · 2024-09-06T09:38:17Z

/packit copr-build

matejmatuska · 2024-09-09T12:46:59Z

I tested the "success case":
The DB is properly found:

Risk Factor: info
Title: pam_userdb databases will be converted to GDBM
Summary: On RHEL 10, GDMB is used by pam_userdb as it's backend database, replacing BerkeleyDB. Existing pam_userdb databases will be converted to GDBM. The following databases will be converted:
    - /root/userpass
Key: 85d0ac0988c109032b7c364bdd252f1cb543744b

Converted to GDBM:

Sep 09 12:24:26 localhost upgrade[831]: 2024-09-09 12:24:26.344 INFO     PID: 1 leapp.workflow.Preparation: Executing actor convert_pam_user_db
Sep 09 12:24:26 localhost upgrade[1526]: 2024-09-09 12:24:26.362 DEBUG    PID: 688 leapp.workflow.Preparation.convert_pam_user_db: External command has started: ['db_converter', '--src', '/root/userpass.db', '--dest', '/root/userpass.gdbm']  
Sep 09 12:24:26 localhost upgrade[1526]: 2024-09-09 12:24:26.392 DEBUG    PID: 688 leapp.workflow.Preparation.convert_pam_user_db: External command has finished: ['db_converter', '--src', '/root/userpass.db', '--dest', '/root/userpass.gdbm']

And the original DB is properly cleaned up:

Sep 09 12:28:24 localhost upgrade[40008]: 2024-09-09 12:28:24.696 INFO     PID: 1 leapp.workflow.Applications: Executing actor remove_old_pam_user_db
Sep 09 12:28:24 localhost upgrade[40741]: 2024-09-09 12:28:24.812 DEBUG    PID: 726 leapp.workflow.Applications.remove_old_pam_user_db: External command has started: ['rm', '-f', '/root/userpass.db']                                           
Sep 09 12:28:24 localhost upgrade[40741]: 2024-09-09 12:28:24.821 DEBUG    PID: 726 leapp.workflow.Applications.remove_old_pam_user_db: External command has finished: ['rm', '-f', '/root/userpass.db']

ikerexxe · 2024-09-09T13:43:07Z

I think the PR is ready for proper review and merging, so I'm moving it.

matejmatuska · 2024-09-26T11:43:13Z

/packit copr-build

matejmatuska · 2024-09-27T13:46:03Z

Tested the error case with an updated RPM containing the patch in db_convertor (libdb-utils-5.3.28-55.el9.x86_64.rpm).

The upgrade is stopped with an error in PreparationPhase as expected:

============================================================
                           ERRORS
============================================================
2024-09-27 13:04:32.824713 [ERROR] Actor: convert_pam_user_db
Message: Cannot convert pam_userdb database.
Summary:
    Details: Command ['db_converter', '--src', '/root/userdb.db', '--dest', '/root/userdb.gdbm'] failed with exit code 1.: BDB0004 fop_read_meta: /root/userdb.db: unexpected file type or format
             Can't open database - 22
============================================================
                       END OF ERRORS
============================================================
Debug output written to /var/log/leapp/leapp-upgrade.log
...

pirat89

lgtm

matejmatuska · 2024-09-30T16:42:34Z

/packit retest-failed

matejmatuska · 2024-09-30T16:45:03Z

LGTM, please rebase and squash the commits
Some 8->9 tests in TF are failing but it seems like a problem with the tests.

pam_userdb module changed its backend database technology from lidb to gdbm for RHEL10. This requires a set of leapp actors to perform the database migration automatically when upgrading to RHEL10: * ScanPamUserDB takes care of scanning the PAM service folder to detect whether pam_userdb is used and the location of the database in use. This information is stored in a model. * CheckPamUserDB checks the databases reported by ScanPamUserDB and prints a report about them. * ConvertPamUserDB checks the databases reported by ScanPamUserDB and converts them to GDBM format. * RemoveOldPamUserDB checks the databases reported by ScanPamUserDB and removes them. All these actors include unit-tests. Finally, there's also a spec file change to add `libdb-utils` dependency as it is required to convert pam_userdb databases from BerkeleyDB to GDBM. Signed-off-by: Iker Pedrosa <[email protected]>

ikerexxe · 2024-10-01T07:37:56Z

LGTM, please rebase and squash the commits Some 8->9 tests in TF are failing but it seems like a problem with the tests.

Done!

matejmatuska · 2024-10-01T08:29:42Z

Merged, thanks again for the contribution!

ikerexxe force-pushed the pam_userdb branch from e6fb9e9 to 1e09362 Compare August 28, 2024 10:43

matejmatuska requested changes Aug 28, 2024

View reviewed changes

ikerexxe force-pushed the pam_userdb branch from 1e09362 to 1b44031 Compare August 29, 2024 10:58

matejmatuska requested changes Aug 29, 2024

View reviewed changes

ikerexxe force-pushed the pam_userdb branch from 1b44031 to e242c08 Compare August 30, 2024 09:28

pirat89 added this to the 8.10/9.6 milestone Aug 30, 2024

ikerexxe mentioned this pull request Aug 30, 2024

Possible error in spec file when managing RHEL9->10 upgrade #1291

Closed

ikerexxe force-pushed the pam_userdb branch from e242c08 to 01e9bdc Compare September 2, 2024 08:52

ikerexxe mentioned this pull request Sep 3, 2024

Check return values when connecting to/creating a database fila43/db_converter#2

Merged

matejmatuska requested changes Sep 4, 2024

View reviewed changes

ikerexxe force-pushed the pam_userdb branch from 01e9bdc to 402969d Compare September 4, 2024 14:17

ikerexxe force-pushed the pam_userdb branch from 402969d to 7f53268 Compare September 5, 2024 11:07

ikerexxe marked this pull request as ready for review September 9, 2024 13:43

pirat89 requested a review from matejmatuska September 21, 2024 19:07

pirat89 approved these changes Sep 27, 2024

View reviewed changes

matejmatuska approved these changes Sep 30, 2024

View reviewed changes

ikerexxe force-pushed the pam_userdb branch from 7f53268 to 595661b Compare October 1, 2024 07:36

matejmatuska merged commit 658700d into oamg:main Oct 1, 2024
26 checks passed

matejmatuska added the changelog-checked The merger/reviewer checked the changelog draft document and updated it when relevant label Oct 1, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update pam_userdb database backend for RHEL10 #1289

Update pam_userdb database backend for RHEL10 #1289

ikerexxe commented Aug 28, 2024 •

edited by pirat89

Loading

github-actions bot commented Aug 28, 2024

matejmatuska left a comment

ikerexxe commented Aug 29, 2024

matejmatuska left a comment

matejmatuska left a comment

ikerexxe commented Sep 4, 2024

matejmatuska commented Sep 5, 2024

matejmatuska commented Sep 5, 2024

matejmatuska commented Sep 5, 2024

ikerexxe commented Sep 5, 2024

matejmatuska commented Sep 6, 2024

pirat89 commented Sep 6, 2024

matejmatuska commented Sep 9, 2024

ikerexxe commented Sep 9, 2024

matejmatuska commented Sep 26, 2024

matejmatuska commented Sep 27, 2024

pirat89 left a comment

matejmatuska commented Sep 30, 2024

matejmatuska commented Sep 30, 2024

ikerexxe commented Oct 1, 2024

matejmatuska commented Oct 1, 2024

Update pam_userdb database backend for RHEL10 #1289

Update pam_userdb database backend for RHEL10 #1289

Conversation

ikerexxe commented Aug 28, 2024 • edited by pirat89 Loading

github-actions bot commented Aug 28, 2024

Thank you for contributing to the Leapp project!

matejmatuska left a comment

Choose a reason for hiding this comment

ikerexxe commented Aug 29, 2024

matejmatuska left a comment

Choose a reason for hiding this comment

matejmatuska left a comment

Choose a reason for hiding this comment

ikerexxe commented Sep 4, 2024

matejmatuska commented Sep 5, 2024

matejmatuska commented Sep 5, 2024

matejmatuska commented Sep 5, 2024

ikerexxe commented Sep 5, 2024

matejmatuska commented Sep 6, 2024

pirat89 commented Sep 6, 2024

matejmatuska commented Sep 9, 2024

ikerexxe commented Sep 9, 2024

matejmatuska commented Sep 26, 2024

matejmatuska commented Sep 27, 2024

pirat89 left a comment

Choose a reason for hiding this comment

matejmatuska commented Sep 30, 2024

matejmatuska commented Sep 30, 2024

ikerexxe commented Oct 1, 2024

matejmatuska commented Oct 1, 2024

ikerexxe commented Aug 28, 2024 •

edited by pirat89

Loading