redhat_subscription: use D-Bus for registration if possible #6122

ptoscano · 2023-03-01T18:03:17Z

SUMMARY

subscription-manager currently does not have a way to get credentials (username, password, activation keys, organization ID) in a secure way: the existing command line parameters can be easily spotted when running a process listing while 'subscription-manager register' runs. There is a D-Bus service, which is used by e.g. cockpit and Anaconda to interface with RHSM (at least for registration and common queries).

Try to perform the registration using D-Bus, in a way very similar to the work done in convert2rhel [1] (with my help):

try to do a simple signal test to check whether the system bus works; inspired by the login in the dconf module
pass most of the options as registration options; for the few that are not part of the registration, execute 'subscription-manager' manually
add quirks for differently working (or not) registration options for the D-Bus Register*() methods depending on the version of RHEL
subscription-manager register is used only in case the signal test is not working; silent fallback in case of D-Bus errors during the registration is not done on purpose to avoid silent fallback to a less secure registration

[1] oamg/convert2rhel#540

ISSUE TYPE

Feature Pull Request

COMPONENT NAME

redhat_subscription

ADDITIONAL INFORMATION

There should be no behaviour change for the module.

ansibullbot · 2023-03-01T18:03:35Z

cc @alikins @barnabycourt @kahowell
click here for bot help

ptoscano · 2023-03-01T18:04:23Z

Opened as draft because I need to test it on more versions of RHEL, in few more scenarios. Regardless, having few more eyes on it wouldn't certainly hurt.

github-actions · 2023-03-01T18:22:57Z

Docs Build 📝

Thank you for contribution!✨

This PR has been merged and your docs changes will be incorporated when they are next published.

richm · 2023-03-02T14:14:40Z

lgtm - have you been able to try this with the rhc system role? If not, I can help with that.

felixfontein

Thanks for your contribution! Could you please add a changelog fragment? Thanks!

plugins/modules/redhat_subscription.py

ptoscano · 2023-03-06T14:34:35Z

OK, after a bit of more in-depth testing, I think it should work fine in (hopefully :) ) all the cases.

Thanks for your contribution! Could you please add a changelog fragment?

Thanks, done; I wasn't sure whether it was needed in this case. If you want to do a more extensive review, please go ahead :)

lgtm - have you been able to try this with the rhc system role? If not, I can help with that.

Not yet, so far I've been testing directly the module itself. I think that, if you have a quick way to spawn all the testing of the rhc system role with this change applied for it, that'd be great!

richm · 2023-03-06T14:57:15Z

OK, after a bit of more in-depth testing, I think it should work fine in (hopefully :) ) all the cases.

Thanks for your contribution! Could you please add a changelog fragment?

Thanks, done; I wasn't sure whether it was needed in this case. If you want to do a more extensive review, please go ahead :)

lgtm - have you been able to try this with the rhc system role? If not, I can help with that.

Not yet, so far I've been testing directly the module itself. I think that, if you have a quick way to spawn all the testing of the rhc system role with this change applied for it, that'd be great!

Sure.

Copy the module .py file to rhc/library
Rename it to something like local_redhat_subscription.py
Change community.general.redhat_subscription: to local_redhat_subscription: in tasks/subscription-manager.yml

Then you can run locally, submit a PR and use CI testing, etc.

subscription-manager currently does not have a way to get credentials (username, password, activation keys, organization ID) in a secure way: the existing command line parameters can be easily spotted when running a process listing while 'subscription-manager register' runs. There is a D-Bus service, which is used by e.g. cockpit and Anaconda to interface with RHSM (at least for registration and common queries). Try to perform the registration using D-Bus, in a way very similar to the work done in convert2rhel [1] (with my help): - try to do a simple signal test to check whether the system bus works; inspired by the login in the dconf module - pass most of the options as registration options; for the few that are not part of the registration, execute 'subscription-manager' manually - add quirks for differently working (or not) registration options for the D-Bus Register*() methods depending on the version of RHEL - 'subscription-manager register' is used only in case the signal test is not working; silent fallback in case of D-Bus errors during the registration is not done on purpose to avoid silent fallback to a less secure registration [1] oamg/convert2rhel#540

ptoscano · 2023-03-08T08:18:09Z

So, I gave it more testing (and fixed small bits):

on its own: works fine with
- RHEL 7 (7.9+)
- RHEL 8.8 (nightly)
- RHEL 9.2 (nightly)
- CentOS 9 Stream (up-to-date)
as part of the rhc system role, running the tests tests_register_unregister.yml & tests_release.yml (which exercise a bit registration and release setting on registration): works fine with
- RHEL 8.7
- RHEL 8.8 (nightly)
- RHEL 9.1
- RHEL 9.2 (nightly)

Hence, at this point I feel more confident that this should work fine. Of course, I can always fix bugs, in case they are noticed/reported.

The only thing I'm not totally sure (and I simply followed what other Python modules do) is the logging using module.debug(): it looks not exactly straightforward to enable, and it gets logged in the managed nodes rather than being outputted on the ansible/ansible-playbook output (even with -v and more). Maybe there's a way, and I'm dumb enough to have not noticed it.

richm · 2023-03-08T14:14:45Z

So, I gave it more testing (and fixed small bits):
* on its own: works fine with
  
  * RHEL 7 (7.9+)
  * RHEL 8.8 (nightly)
  * RHEL 9.2 (nightly)
  * CentOS 9 Stream (up-to-date)

* as part of the `rhc` system role, running the tests `tests_register_unregister.yml` & `tests_release.yml` (which exercise a bit registration and release setting on registration): works fine with
  
  * RHEL 8.7
  * RHEL 8.8 (nightly)
  * RHEL 9.1
  * RHEL 9.2 (nightly)
Hence, at this point I feel more confident that this should work fine. Of course, I can always fix bugs, in case they are noticed/reported.

+1

The only thing I'm not totally sure (and I simply followed what other Python modules do) is the logging using module.debug(): it looks not exactly straightforward to enable, and it gets logged in the managed nodes rather than being outputted on the ansible/ansible-playbook output (even with -v and more). Maybe there's a way, and I'm dumb enough to have not noticed it.

No, you are correct, this is the way it is done. You use journalctl on the managed node to view the debug log.
Not really related to logging, but if you want to debug the module on the managed node, see https://docs.ansible.com/ansible/latest/dev_guide/debugging.html#debugging-modules

richm · 2023-03-10T18:42:03Z

This fix lgtm - is it ready to be merged? If so, when can it be merged?

felixfontein · 2023-03-11T17:05:32Z

It looks OK to me. If you and @ptoscano are happy I can merge this.

ptoscano · 2023-03-12T13:36:03Z

It looks OK to me. If you and @ptoscano are happy I can merge this.

My last changes (4 days ago) solved all the few issues I still saw, so currently I'm aware of none. Of course, I'm willing to fix any issue that show up after these changes.

richm · 2023-03-12T19:49:39Z

It looks OK to me. If you and @ptoscano are happy I can merge this.

Thanks!

patchback · 2023-03-14T21:52:54Z

Backport to stable-6: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-6/e939cd07ef335e939e901c3ccc76523eab8d5e4a/pr-6122

Backported as #6188

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

subscription-manager currently does not have a way to get credentials (username, password, activation keys, organization ID) in a secure way: the existing command line parameters can be easily spotted when running a process listing while 'subscription-manager register' runs. There is a D-Bus service, which is used by e.g. cockpit and Anaconda to interface with RHSM (at least for registration and common queries). Try to perform the registration using D-Bus, in a way very similar to the work done in convert2rhel [1] (with my help): - try to do a simple signal test to check whether the system bus works; inspired by the login in the dconf module - pass most of the options as registration options; for the few that are not part of the registration, execute 'subscription-manager' manually - add quirks for differently working (or not) registration options for the D-Bus Register*() methods depending on the version of RHEL - 'subscription-manager register' is used only in case the signal test is not working; silent fallback in case of D-Bus errors during the registration is not done on purpose to avoid silent fallback to a less secure registration [1] oamg/convert2rhel#540 (cherry picked from commit e939cd0)

felixfontein · 2023-03-14T21:54:24Z

@ptoscano thanks for contributing this!
@richm thanks for reviewing/testing!

… for registration if possible (#6188) redhat_subscription: use D-Bus for registration if possible (#6122) subscription-manager currently does not have a way to get credentials (username, password, activation keys, organization ID) in a secure way: the existing command line parameters can be easily spotted when running a process listing while 'subscription-manager register' runs. There is a D-Bus service, which is used by e.g. cockpit and Anaconda to interface with RHSM (at least for registration and common queries). Try to perform the registration using D-Bus, in a way very similar to the work done in convert2rhel [1] (with my help): - try to do a simple signal test to check whether the system bus works; inspired by the login in the dconf module - pass most of the options as registration options; for the few that are not part of the registration, execute 'subscription-manager' manually - add quirks for differently working (or not) registration options for the D-Bus Register*() methods depending on the version of RHEL - 'subscription-manager register' is used only in case the signal test is not working; silent fallback in case of D-Bus errors during the registration is not done on purpose to avoid silent fallback to a less secure registration [1] oamg/convert2rhel#540 (cherry picked from commit e939cd0) Co-authored-by: Pino Toscano <[email protected]>

ansibullbot added WIP Work in progress feature This issue/PR relates to a feature request module module needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI os packaging plugins plugin (any type) tests tests unit tests/unit labels Mar 1, 2023

This comment was marked as outdated.

Sign in to view

ansibullbot added ci_verified Push fixes to PR branch to re-run CI and removed needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI labels Mar 1, 2023

ptoscano force-pushed the redhat_subscription-register-with-dbus branch from 34ee8da to 15e0ab8 Compare March 1, 2023 18:10

ansibullbot removed the ci_verified Push fixes to PR branch to re-run CI label Mar 1, 2023

felixfontein added check-before-release PR will be looked at again shortly before release and merged if possible. backport-6 labels Mar 2, 2023

felixfontein reviewed Mar 3, 2023

View reviewed changes

plugins/modules/redhat_subscription.py Outdated Show resolved Hide resolved

ptoscano force-pushed the redhat_subscription-register-with-dbus branch 7 times, most recently from 66afc53 to 4530095 Compare March 6, 2023 14:23

ansibullbot added the needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI label Mar 6, 2023

ptoscano marked this pull request as ready for review March 6, 2023 14:34

ansibullbot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR and removed WIP Work in progress needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Mar 6, 2023

ptoscano force-pushed the redhat_subscription-register-with-dbus branch from 4530095 to ec7527f Compare March 7, 2023 11:13

ansibullbot removed the needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI label Mar 7, 2023

ptoscano force-pushed the redhat_subscription-register-with-dbus branch from ec7527f to 480ede9 Compare March 8, 2023 06:03

felixfontein merged commit e939cd0 into ansible-collections:main Mar 14, 2023

patchback bot mentioned this pull request Mar 14, 2023

[PR #6122/e939cd07 backport][stable-6] redhat_subscription: use D-Bus for registration if possible #6188

Merged

felixfontein removed the check-before-release PR will be looked at again shortly before release and merged if possible. label Mar 14, 2023

ptoscano deleted the redhat_subscription-register-with-dbus branch March 14, 2023 22:46

ptoscano mentioned this pull request Mar 28, 2023

redhat_subscription module fails when using "force_register" on non registered system #6258

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

redhat_subscription: use D-Bus for registration if possible #6122

redhat_subscription: use D-Bus for registration if possible #6122

ptoscano commented Mar 1, 2023 •

edited

Loading

ansibullbot commented Mar 1, 2023

ptoscano commented Mar 1, 2023

This comment was marked as outdated.

github-actions bot commented Mar 1, 2023 •

edited

Loading

richm commented Mar 2, 2023

felixfontein left a comment

ptoscano commented Mar 6, 2023

richm commented Mar 6, 2023

ptoscano commented Mar 8, 2023

richm commented Mar 8, 2023

richm commented Mar 10, 2023

felixfontein commented Mar 11, 2023

ptoscano commented Mar 12, 2023

richm commented Mar 12, 2023

patchback bot commented Mar 14, 2023 •

edited

Loading

felixfontein commented Mar 14, 2023

redhat_subscription: use D-Bus for registration if possible #6122

redhat_subscription: use D-Bus for registration if possible #6122

Conversation

ptoscano commented Mar 1, 2023 • edited Loading

SUMMARY

ISSUE TYPE

COMPONENT NAME

ADDITIONAL INFORMATION

ansibullbot commented Mar 1, 2023

ptoscano commented Mar 1, 2023

This comment was marked as outdated.

github-actions bot commented Mar 1, 2023 • edited Loading

Docs Build 📝

richm commented Mar 2, 2023

felixfontein left a comment

Choose a reason for hiding this comment

ptoscano commented Mar 6, 2023

richm commented Mar 6, 2023

ptoscano commented Mar 8, 2023

richm commented Mar 8, 2023

richm commented Mar 10, 2023

felixfontein commented Mar 11, 2023

ptoscano commented Mar 12, 2023

richm commented Mar 12, 2023

patchback bot commented Mar 14, 2023 • edited Loading

Backport to stable-6: 💚 backport PR created

felixfontein commented Mar 14, 2023

ptoscano commented Mar 1, 2023 •

edited

Loading

github-actions bot commented Mar 1, 2023 •

edited

Loading

patchback bot commented Mar 14, 2023 •

edited

Loading