Allow to call RegisterWithActivationKeys() with enable_content #3488

jirihnidek · 2025-01-06T16:39:20Z

Allow to call D-Bus method RegisterWithActivationKeys() with
register option enable_content. When this option is not set,
then behavior is still the same. It means that content is
enabled by default, when system is registered with this D-Bus
method.
Added two unit tests
Some refactoring and bug fixes in separate commits

* When D-Bus method Register() was called with enable_content option and the value was "false" or "0", then code evaluated this value as True. * Fixed this issue, because previous behavior was very confusing * Refactored code a little bit. * The method is static method * It is possible to set default value, when enable_content was not set at all.

* Converted method to static method * Deleted dead code * Deleted empty lines

github-actions · 2025-01-06T16:40:44Z

Coverage (computed on Fedora latest) •

File	Stmts	Miss	Cover	Missing
rhsmlib/dbus/objects
register.py	228	72	68%	36, 97–98, 111–112, 122–123, 125–126, 136–137, 139, 153, 156, 169, 172, 230, 239, 279–280, 287, 307, 314, 357–359, 361–362, 382–386, 388–390, 392, 394, 396, 415–418, 420–422, 424, 426, 428, 443, 447, 464–469, 471–473, 475, 477, 489–493, 495–497, 499, 501
TOTAL	17445	4482	74%

Tests	Skipped	Failures	Errors	Time
2418	14 💤	0 ❌	0 🔥	31.330s ⏱️

Archana-PandeyM · 2025-01-09T07:24:06Z

In case the implementation is done- I tried using this subman build on rhel 10 with rhc copr builds that support this feature. I hit the same error as before when using activation key. Sharing the logs from rhsm.log-
e found for domain: 'rhsm' 2025-01-09T08:12:27.625+01:00 [ERROR] rhsm-service:32088:MainThread @util.py:72 - Unknown arguments: dict_keys(['enable_content']) Traceback (most recent call last): File "/usr/lib64/python3.12/site-packages/rhsmlib/dbus/util.py", line 70, in dbus_handle_exceptions return func(*args, **kwargs) ^^^^^^^^^^^^^^^^^^^^^ File "/usr/lib64/python3.12/site-packages/rhsmlib/dbus/objects/register.py", line 499, in RegisterWithActivationKeys dbus_sender.set_cmd_line(sender=self.sender, cmd_line=self.cmd_line) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/lib64/python3.12/site-packages/rhsmlib/dbus/objects/register.py", line 265, in register_with_activation_keys enable_content: bool = self._remove_enable_content_option(register_options, default=True) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/lib64/python3.12/site-packages/rhsmlib/services/register.py", line 67, in register # signature we want to consider that an error. ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ rhsmlib.services.exceptions.ValidationError: Unknown arguments: dict_keys(['enable_content'])

jirihnidek · 2025-01-13T11:43:16Z

In case the implementation is done- I tried using this subman build on rhel 10 with rhc copr builds that support this feature. I hit the same error as before when using activation key. Sharing the logs from rhsm.log- e found for domain: 'rhsm' 2025-01-09T08:12:27.625+01:00 [ERROR] rhsm-service:32088:MainThread @util.py:72 - Unknown arguments: dict_keys(['enable_content']) Traceback (most recent call last): File "/usr/lib64/python3.12/site-packages/rhsmlib/dbus/util.py", line 70, in dbus_handle_exceptions return func(*args, **kwargs) ^^^^^^^^^^^^^^^^^^^^^ File "/usr/lib64/python3.12/site-packages/rhsmlib/dbus/objects/register.py", line 499, in RegisterWithActivationKeys dbus_sender.set_cmd_line(sender=self.sender, cmd_line=self.cmd_line) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/lib64/python3.12/site-packages/rhsmlib/dbus/objects/register.py", line 265, in register_with_activation_keys enable_content: bool = self._remove_enable_content_option(register_options, default=True) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/lib64/python3.12/site-packages/rhsmlib/services/register.py", line 67, in register # signature we want to consider that an error. ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ rhsmlib.services.exceptions.ValidationError: Unknown arguments: dict_keys(['enable_content'])

@Archana-PandeyM Did you manually restart rhsm.service before testing this PR? It is still necessary to restart rhsm.service after installing update of subscription-manager, because this PR: #3485 is still WIP. If rhsm.service was running before installing update of subscription-manager, then you can get false negative results after installing subscription-manager, because old rhsm.service process is still running.

jirihnidek · 2025-01-13T15:47:50Z

I can confirm that it works as expected, when service is restarted.

Archana-PandeyM · 2025-01-13T15:54:50Z

Thanks George! I checked too it works after the service is restarted.

jirihnidek · 2025-01-20T11:22:26Z

To test this change you can use library.sh from this repository https://github.com/jirihnidek/rhsm-dbus-examples/ and following bash script:

#!/bin/bash

source ./library.sh

export org_id="donaldduck"
export activation_key="awesome_os_pool"

start_register_server

# Try to register using activation key and org
echo "Registering using activation key and org..."
dbus-send --address=${my_addr} --print-reply --dest='com.redhat.RHSM1.Register' \
       '/com/redhat/RHSM1/Register' \
       com.redhat.RHSM1.Register.RegisterWithActivationKeys \
       string:"${org_id}" \
       array:string:"${activation_key}" \
       dict:string:string:"enable_content","0" \
       dict:string:string:"","" \
       string:"" | gawk '/string/{ $1 = ""; print $0; }' | sed 's/\"//' | sed 's/\(.*\)\"/\1/' > /root/register_output.json

print_registration_result $?

stop_register_server

# unregister

Glutexo · 2025-01-22T15:23:24Z

I was able to reproduce that without this patch, the enable_content parameter is not recognized. Also that with the changes applied, the parameter is accepted. I ran into the same issue as Archana. Thanks to your comments, I was able to fix it by restarting DBus.

Now, I’m going to verify that argument does what it’s intended to do. Also, I‘ll check the code.

Tampering with the dbus-send command, I encountered variations on this error:

Error org.freedesktop.DBus.Python.TypeError: Traceback (most recent call last):
  File "/usr/lib64/python3.9/site-packages/dbus/service.py", line 715, in _message_cb
    retval = candidate_method(self, *args, **keywords)
TypeError: RegisterWithActivationKeys() missing 3 required positional arguments: 'options', 'connection_options', and 'locale'

Subscription Manager should probably handle missing DBus parameter more gracefully.

jirihnidek · 2025-01-23T09:12:11Z

Tampering with the dbus-send command, I encountered variations on this error:
Error org.freedesktop.DBus.Python.TypeError: Traceback (most recent call last):
  File "/usr/lib64/python3.9/site-packages/dbus/service.py", line 715, in _message_cb
    retval = candidate_method(self, *args, **keywords)
TypeError: RegisterWithActivationKeys() missing 3 required positional arguments: 'options', 'connection_options', and 'locale'
Subscription Manager should probably handle missing DBus parameter more gracefully.

The error was raised from /usr/lib64/python3.9/site-packages/dbus/service.py as you can see in the traceback. The subscription-manager can hardly do anything with this. The D-Bus itself does not have concept of optional arguments.

Glutexo

I think I can approve. I was not successful with running the DBus command, but Archana confirmed that part. I don’t want to block the PR for too long. I looked at the code and it makes sense. I only nitpicked a few boolean expressions, but they are non-blocking comments.

Glutexo · 2025-01-22T11:41:22Z

src/rhsmlib/dbus/objects/register.py

-        ent_cert_lib = EntCertActionInvoker()
-        ent_cert_lib.update()
+        # When consumer is created, we can try to enable content, if requested.
+        if enable_content is True:


enable_content is bool, so if enable_content means effectively the same.

Suggested change

if enable_content is True:

if enable_content:

Glutexo · 2025-01-23T15:36:37Z

src/rhsmlib/dbus/objects/register.py

+        if is_true_value(enable_content):
+            return True
+        else:
+            return False


Suggested change

if is_true_value(enable_content):

return True

else:

return False

return is_true_value(enable_content):

* Allow to call D-Bus method RegisterWithActivationKeys() with register option enable_content. When this option is not set, then behavior is still the same. It means that content is enabled by default, when system is registered with this D-Bus method. * Added two unit tests

Glutexo

I took a look at the new tests. I didn’t examine them to the detail, but they make sense to me. I tried to run them and they succeed. Suggested a parametrization, but that’s not necessary. The other changes look perfect.

Two unrelated tests fail when run with a non-en_US locale. I‘ll file tickets for those.

Glutexo · 2025-01-24T13:35:46Z

test/rhsmlib/dbus/test_register.py

+    def test_RegisterWithActivationKeys__with_enable_content_option(self):
+        expected = json.loads(CONSUMER_CONTENT_JSON_SCA)
+        self.patches["is_registered"].return_value = False
+        self.patches["register"].return_value = expected
+
+        result = self.impl.register_with_activation_keys(
+            "username",
+            {"keys": ["key1", "key2"], "enable_content": "true"},
+            {"host": "localhost", "port": "8443", "handler": "/candlepin"},
+        )
+        self.assertEqual(expected, result)
+
+    def test_RegisterWithActivationKeys__with_disable_content_option(self):
+        expected = json.loads(CONSUMER_CONTENT_JSON_SCA)
+        self.patches["is_registered"].return_value = False
+        self.patches["register"].return_value = expected
+
+        result = self.impl.register_with_activation_keys(
+            "username",
+            {"keys": ["key1", "key2"], "enable_content": "false"},
+            {"host": "localhost", "port": "8443", "handler": "/candlepin"},
+        )
+        self.assertEqual(expected, result)


These two tests only differ in the "enable_content" value. They can be parametrized.

Suggested change

def test_RegisterWithActivationKeys__with_enable_content_option(self):

expected = json.loads(CONSUMER_CONTENT_JSON_SCA)

self.patches["is_registered"].return_value = False

self.patches["register"].return_value = expected

result = self.impl.register_with_activation_keys(

"username",

{"keys": ["key1", "key2"], "enable_content": "true"},

{"host": "localhost", "port": "8443", "handler": "/candlepin"},

)

self.assertEqual(expected, result)

def test_RegisterWithActivationKeys__with_disable_content_option(self):

expected = json.loads(CONSUMER_CONTENT_JSON_SCA)

self.patches["is_registered"].return_value = False

self.patches["register"].return_value = expected

result = self.impl.register_with_activation_keys(

"username",

{"keys": ["key1", "key2"], "enable_content": "false"},

{"host": "localhost", "port": "8443", "handler": "/candlepin"},

)

self.assertEqual(expected, result)

@pytest.mark.parametrize(("enable_content",), [("true",), ("false",)])

def test_RegisterWithActivationKeys__with_enable_content_option(self, enable_content):

expected = json.loads(CONSUMER_CONTENT_JSON_SCA)

self.patches["is_registered"].return_value = False

self.patches["register"].return_value = expected

result = self.impl.register_with_activation_keys(

"username",

{"keys": ["key1", "key2"], "enable_content": enable_content},

{"host": "localhost", "port": "8443", "handler": "/candlepin"},

)

self.assertEqual(expected, result)

jirihnidek added 2 commits January 6, 2025 17:36

refactor: Small changes of _enable_content

c9a37a6

* Converted method to static method * Deleted dead code * Deleted empty lines

jirihnidek force-pushed the jhnidek/register_activation_keys_and_enable_content_opt branch from 54329a3 to cb97911 Compare January 6, 2025 16:41

jirihnidek mentioned this pull request Jan 6, 2025

feat: Added levels of connectivity to connect command RedHatInsights/rhc#179

Open

jirihnidek requested a review from m-horky January 15, 2025 14:37

Glutexo approved these changes Jan 23, 2025

View reviewed changes

jirihnidek mentioned this pull request Jan 23, 2025

fix: Really disable content #3500

Open

jirihnidek force-pushed the jhnidek/register_activation_keys_and_enable_content_opt branch from cb97911 to b5bf151 Compare January 24, 2025 08:24

jirihnidek requested a review from Glutexo January 24, 2025 09:00

Glutexo approved these changes Jan 24, 2025

View reviewed changes

Glutexo merged commit 0d4a48c into main Jan 24, 2025
22 of 23 checks passed

Glutexo deleted the jhnidek/register_activation_keys_and_enable_content_opt branch January 24, 2025 16:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow to call RegisterWithActivationKeys() with enable_content #3488

Allow to call RegisterWithActivationKeys() with enable_content #3488

jirihnidek commented Jan 6, 2025

github-actions bot commented Jan 6, 2025 •

edited

Loading

Archana-PandeyM commented Jan 9, 2025

jirihnidek commented Jan 13, 2025 •

edited

Loading

jirihnidek commented Jan 13, 2025

Archana-PandeyM commented Jan 13, 2025

jirihnidek commented Jan 20, 2025

Glutexo commented Jan 22, 2025

jirihnidek commented Jan 23, 2025

Glutexo left a comment •

edited

Loading

Glutexo Jan 22, 2025

Glutexo Jan 23, 2025

Glutexo left a comment

Glutexo Jan 24, 2025

Allow to call RegisterWithActivationKeys() with enable_content #3488

Allow to call RegisterWithActivationKeys() with enable_content #3488

Conversation

jirihnidek commented Jan 6, 2025

github-actions bot commented Jan 6, 2025 • edited Loading

Archana-PandeyM commented Jan 9, 2025

jirihnidek commented Jan 13, 2025 • edited Loading

jirihnidek commented Jan 13, 2025

Archana-PandeyM commented Jan 13, 2025

jirihnidek commented Jan 20, 2025

Glutexo commented Jan 22, 2025

jirihnidek commented Jan 23, 2025

Glutexo left a comment • edited Loading

Choose a reason for hiding this comment

Glutexo Jan 22, 2025

Choose a reason for hiding this comment

Glutexo Jan 23, 2025

Choose a reason for hiding this comment

Glutexo left a comment

Choose a reason for hiding this comment

Glutexo Jan 24, 2025

Choose a reason for hiding this comment

github-actions bot commented Jan 6, 2025 •

edited

Loading

jirihnidek commented Jan 13, 2025 •

edited

Loading

Glutexo left a comment •

edited

Loading