CCT-67: Anonymous registration #3370

m-horky · 2024-01-16T12:51:14Z

This PR implements the v2 flow for automatic cloud registration. Please see the individual commits for comments, I've tried to keep them all separate to describe the tasks that needed to happen. And I'd like to specifically ask for detailed review of the C code, that may need adjustments.

Verify steps:

Point candlepin.local to a running instance of ghcr.io/ptoscano/candlepin-unofficial:latest using your /etc/hosts (make sure the instance is set into Hosted mode)
Copy the certificate from running Candlepin into your local filesystem and to /etc/rhsm/ca/
Point sub-man to your local Candlepin through the rhsm.conf
Patch cloud-what's aws.py to point to local mock HTTP server for AWS metadata.
Create local facts file to trigger cloud detection rules
Make sure sub-man and cloud-what caches are clean
Fill Candlepin database with data about anonymous org (partially or fully)
Run rhsmcertd-worker --auto-register and let the system anonymously register. Complete the flow to leave the system left with anonymous identity.
Claim the anonymous systems in Candlepin
Run rhsmcertd-worker and let the system replace anonymous identity with proper one.

github-actions · 2024-01-16T13:42:22Z

Coverage (computed on Fedora latest) •

File	Stmts	Miss	Cover	Missing
rhsm
connection.py	1019	464	54%	48–49, 53, 55–56, 81, 101–102, 150, 284, 315, 381–386, 390–399, 460, 462, 564, 567, 574–580, 585, 641, 676–680, 682, 695, 722, 725–726, 728–729, 731, 742–746, 750, 754, 756–757, 776, 779, 783–784, 789, 792–793, 808, 812, 814–815, 842–843, 845, 848, 853–854, 857–858, 860, 862–866, 868–869, 872–879, 881–891, 893, 895–896, 907–909, 911–913, 915–917, 919–921, 923, 926–932, 934–935, 937–938, 940, 951–953, 955–956, 958–960, 962, 974–977, 982, 1046, 1048–1053, 1055, 1060–1064, 1070–1073, 1075–1080, 1084–1089, 1096, 1133, 1135, 1140, 1151, 1160–1163, 1167, 1169–1171, 1175–1176, 1178–1185, 1187, 1189, 1192–1199, 1202–1203, 1208, 1210, 1262, 1279–1282, 1306, 1328, 1358, 1363, 1366, 1369–1370, 1375, 1378, 1383, 1386, 1429–1433, 1440–1441, 1443, 1452–1453, 1455, 1472, 1485–1487, 1490, 1501, 1506, 1510, 1538–1540, 1545–1546, 1548–1549, 1551–1552, 1554–1568, 1570–1572, 1574–1585, 1587, 1604–1606, 1608–1610, 1612–1614, 1619, 1624–1626, 1631, 1658, 1690–1718, 1723–1724, 1726–1728, 1731–1732, 1735–1736, 1739–1740, 1759–1760, 1769–1770, 1780–1781, 1788–1789, 1795–1798, 1804–1807, 1813–1814, 1820–1821, 1841–1842, 1858–1864, 1866, 1874–1875, 1901–1904, 1929–1930, 1939–1940, 1948–1949, 1970, 1972–1974, 1976, 1978, 1981, 1983–1996, 1998–1999, 2008–2010, 2022–2023, 2032–2033, 2035, 2037–2039, 2046–2048, 2057–2059, 2067–2068, 2079, 2081–2082, 2084, 2086–2089, 2091–2093, 2096, 2098, 2105–2106, 2113–2114, 2124–2125, 2135–2138, 2148–2154, 2161–2164
rhsmlib/services
register.py	103	18	82%	61, 69–70, 72–73, 75–76, 78–79, 111, 144, 195, 197, 209, 222, 242, 249, 251
subscription_manager
cache.py	631	118	81%	30–34, 75, 82, 90, 96, 101–103, 124, 130–132, 145–148, 151, 199, 201, 240–243, 250–253, 270, 273–275, 293, 302–303, 305, 314, 354–355, 389–390, 421, 423–424, 426, 439, 488, 492, 497, 502–505, 508, 516, 519–520, 541, 593, 596, 621, 726, 744, 779, 781–782, 811, 814–820, 823, 855–858, 860, 871, 889, 911–912, 952–955, 957, 985, 1014–1015, 1042–1043, 1066–1067, 1071, 1075, 1077, 1085, 1127, 1155–1161, 1163, 1165, 1172–1175, 1177
entcertlib.py	271	58	78%	33–34, 36–39, 97–99, 124–132, 135, 167, 171, 173, 177, 194–196, 215, 218–219, 221, 224–225, 249, 332, 341, 391–392, 394–395, 397, 444, 448, 472, 484–488, 490–493, 495, 500–501, 503–504, 506
identity.py	139	59	57%	30, 56–60, 64, 68–75, 78, 81–82, 85–86, 90, 94, 97, 105–106, 111, 113–119, 122–127, 130–132, 135, 158–161, 165–166, 168, 183, 192–194, 209–211, 215, 218
identitycertlib.py	39	3	92%	22–24
managerlib.py	487	148	69%	62–68, 89–90, 98–104, 109, 113, 116, 142–147, 149–154, 212–216, 278, 280–284, 400, 408, 431, 465, 566–567, 569–573, 576–577, 581–582, 588–590, 592, 596, 598–601, 623, 625, 658, 691–693, 697–699, 702–703, 706, 729–731, 739–742, 744–747, 799–802, 823, 864, 885–887, 889, 892–893, 919–921, 923, 934, 948–951, 953–956, 960–963, 965, 968, 971–972, 976–979, 981, 984, 987–992, 994, 998–1001, 1009–1015, 1017–1018
subscription_manager/scripts
rhsmcertd_worker.py	204	135	33%	42–45, 87, 92–95, 100–101, 106–109, 149, 169, 171, 175–177, 181, 184–187, 190, 193–194, 200–202, 204–209, 211–212, 214–220, 222–223, 225–226, 236, 238–239, 256, 259–260, 263–265, 267–269, 273, 276, 278–279, 282–284, 287, 291–298, 302–304, 307, 311–313, 315, 321, 323, 325–328, 330–332, 334–335, 339, 341, 343, 345, 347–349, 351, 353, 355, 357–358, 360–364, 370–371, 375, 377, 381, 383–384, 391–392, 402–405, 410–415, 419
TOTAL	18385	4694	74%

Tests	Skipped	Failures	Errors	Time
2647	14 💤	0 ❌	0 🔥	41.775s ⏱️

jirihnidek

Thanks for this PR. I have several comments, questions and requests.

src/subscription_manager/cache.py

src/subscription_manager/entcertlib.py

src/subscription_manager/scripts/rhsmcertd_worker.py

src/subscription_manager/cache.py

src/daemons/rhsmcertd.c

src/subscription_manager/scripts/rhsmcertd_worker.py

The code for it was removed in 3095f59 (2021-09).

rhsmcertd is not related to D-Bus server.

The organization may be a None.

m-horky · 2024-02-02T16:18:22Z

Force-push change: install entitlement certificates after registering with anonymous organization.

subscription-manager/src/subscription_manager/scripts/rhsmcertd_worker.py

Lines 282 to 289 in 321d38d

    
           # The anonymous organization will have different entitlement certificates, 
        
           # we need to refresh them. 
        
           log.debug( 
        
               "Replacing anonymous entitlement certificates " 
        
               "with entitlement certificates linked to an anonymous organization." 
        
           ) 
        
           report = entcertlib.EntCertUpdateAction().perform() 
        
           log.debug(report)

jirihnidek

Thanks for update! 👍 I have few comments and requests.

jirihnidek · 2024-02-05T15:53:33Z

src/rhsm/connection.py

-        :param metadata: string with base64 encoded metadata
-        :param signature: string with base64 encoded signature
-        :return: string with JWT
+    def get_cloud_jwt(self, cloud_id: str, metadata: str, signature: str) -> Dict[str, Any]:


I know that camelCase is not Pythonic, but I would keep names of method consistent.

jirihnidek · 2024-02-05T16:05:26Z

src/subscription_manager/cache.py

+
+        payload: str = content["token"].split(".")[1]
+        # JWT does not use the padding, base64.b64decode requires it.
+        payload = f"{payload}==="


We have almost the same code here: https://github.com/candlepin/subscription-manager/blob/main/src/cloud_what/providers/gcp.py#L275 . I would do some refactoring to remove duplicated code.

Do you think it is necessary? This implementation is much smaller and quite self-contained. I am not sure it is worth extracting both into a shared implementation, since we're only doing this on two places.

jirihnidek · 2024-02-05T16:07:29Z

src/subscription_manager/entcertlib.py

+
+
+class AnonymousCertificateManager:
+    """Manage anonymous certificates.


I would emphasize that these certificates are entitlement certificates in this doc string.

jirihnidek · 2024-02-05T16:11:17Z

src/subscription_manager/entcertlib.py

+        :param jwt: The Bearer token sent by Candlepin.
+        """
+        log.debug("Obtaining anonymous entitlement certificates and keys")
+        certificates: List[Dict] = self.uep.getCertificates(consumer_uuid=uuid, jwt=jwt)


When no entitlement certificates are installed, then it does not make sense to continue with following steps. I would print some warning message, when no entitlement certificates was downloaded.

jirihnidek · 2024-02-05T16:14:03Z

src/subscription_manager/entcertlib.py

+        )
+
+        update_repo = repolib.RepoUpdateActionCommand()
+        update_repo_report: repolib.RepoActionReport = update_repo.perform()


The report update_repo_report could be None, but you don't check for that in following code.

jirihnidek · 2024-02-05T17:01:17Z

src/subscription_manager/identity.py

@@ -96,6 +96,10 @@ def getConsumerName(self) -> str:
    def getSerialNumber(self) -> int:
        return self.x509.serial

+    def getConsumerOwner(self) -> str:
+        subject: dict = self.x509.subject
+        return subject.get("O")


Maybe, this deserves some constant with comment, but I can see that other parts of code. Up to you. We can deffer improvements of this code.

jirihnidek · 2024-02-06T07:43:31Z

src/subscription_manager/scripts/rhsmcertd_worker.py


-    log.debug("Trying to detect cloud provider")


Why did you remove this debug log from this function?

jirihnidek · 2024-02-06T07:46:38Z

src/subscription_manager/scripts/rhsmcertd_worker.py

+            log.debug("This system is already registered, skipping automatic registration.")
+        else:
+            print(_("Registering the system"))
+            log.debug("Starting automatic registration.")


I would move the debug log to the _auto_register() to keep everything together.

jirihnidek · 2024-02-06T07:49:54Z

src/subscription_manager/scripts/rhsmcertd_worker.py

+    if token["tokenType"] == "CP-Cloud-Registration":
+        try:
+            _auto_register_standard(uep=uep, token=token)
+        except Exception:


Suggestion:

except Exception as err: log.exception(f"Standard automatic registration failed: {err}.")

That is not needed, the log.exception() will print out the full traceback, so we are getting the actual exception and its stack.

jirihnidek · 2024-02-06T07:50:08Z

src/subscription_manager/scripts/rhsmcertd_worker.py

+        try:
+            _auto_register_anonymous(uep=uep, token=token)
+            cache.CloudTokenCache.delete_cache()
+        except Exception:


Similar suggestion here.

* Card ID: CCT-67 - /cloud/authorize?version=2: New - /consumers/{uuid}/certificates: Added 'Authorization' header

* Card ID: CCT-67 The token we receive from Candlepin is valid for two days and should be kept even when the system is restarted. This cache files is used to make sure it persists.

* Card ID: CCT-67 An abstraction managing anonymous entitlement certificates.

This patch creates a system for exit codes used by the worker. This way the C wrapper may be able to understand what happened inside of the application and alter its behavior.

* Card ID: CCT-67 This patch implements the standard and anonymous flows for automatic cloud registration.

* Card ID: CCT-67

* Card ID: CCT-67 The delay was introduced in early versions of the code when Python was not reliable for long-running tasks. Since we want to make automatic registration in both standard and anonymous flow to be as fast as possible, this delay is removed. This splay period is now performed by the Python code in case the we have obtained the anonymous entitlement certificates; before we ask for the identity certificate.

* Card ID: CCT-66 During the first phase of anonymous cloud registration, the system has valid entitlement certificates that aren't associated with any consumer. We shouldn't be reporting missing identity. Previously this was an error state that was not valid, but with automatic registration it is possible to have a system consuming content that does not have an identity, for a transition period before it is part of an anonymous or claimed organization.

jirihnidek

LGTM, Thanks for all updates.

jirihnidek · 2024-02-14T11:03:47Z

src/subscription_manager/cache.py

+
+        payload: str = content["token"].split(".")[1]
+        # JWT does not use the padding, base64.b64decode requires it.
+        payload = f"{payload}==="


m-horky requested review from jirihnidek and ptoscano January 16, 2024 12:51

cnsnyder requested a review from a team January 16, 2024 12:51

jirihnidek self-assigned this Jan 16, 2024

m-horky force-pushed the mhorky/CCT-67-anonymous-registration branch from 6a66adb to cff8198 Compare January 16, 2024 13:40

m-horky force-pushed the mhorky/CCT-67-anonymous-registration branch 2 times, most recently from fafd80b to 4ca30ee Compare January 16, 2024 13:49

jirihnidek requested changes Jan 17, 2024

View reviewed changes

m-horky marked this pull request as draft January 18, 2024 12:17

m-horky force-pushed the mhorky/CCT-67-anonymous-registration branch from 4ca30ee to 19dcdc1 Compare January 18, 2024 14:36

jirihnidek reviewed Jan 18, 2024

View reviewed changes

src/subscription_manager/scripts/rhsmcertd_worker.py Outdated Show resolved Hide resolved

jirihnidek reviewed Jan 18, 2024

View reviewed changes

src/subscription_manager/scripts/rhsmcertd_worker.py Outdated Show resolved Hide resolved

m-horky force-pushed the mhorky/CCT-67-anonymous-registration branch 2 times, most recently from 74ec02c to 4904fde Compare January 22, 2024 12:12

m-horky added 5 commits January 25, 2024 18:49

rhsmcertd: Remove forgotten old comment

1844d95

The code for it was removed in 3095f59 (2021-09).

rhsmcertd: Add type hints

4db4fd8

rhsmcertd: Drop D-Bus loop code

39cbce1

rhsmcertd is not related to D-Bus server.

Fix type hint of RegisterService.register()

df7c3f4

The organization may be a None.

Update documentation for one API call in connection.py

feea253

m-horky force-pushed the mhorky/CCT-67-anonymous-registration branch from 4904fde to 06d3c39 Compare January 29, 2024 12:51

m-horky marked this pull request as ready for review January 29, 2024 14:01

cnsnyder requested a review from a team January 29, 2024 14:08

m-horky force-pushed the mhorky/CCT-67-anonymous-registration branch from 06d3c39 to 321d38d Compare February 2, 2024 16:15

jirihnidek requested changes Feb 6, 2024

View reviewed changes

Implement API endpoints for Automatic registration v2

43f57ca

* Card ID: CCT-67 - /cloud/authorize?version=2: New - /consumers/{uuid}/certificates: Added 'Authorization' header

m-horky force-pushed the mhorky/CCT-67-anonymous-registration branch from 321d38d to 69013e9 Compare February 6, 2024 09:55

m-horky added 2 commits February 6, 2024 14:22

Add CloudTokenCache for Candlepin JWT

ccc03e4

* Card ID: CCT-67 The token we receive from Candlepin is valid for two days and should be kept even when the system is restarted. This cache files is used to make sure it persists.

Add AnonymousCertificateManager

23db3a4

* Card ID: CCT-67 An abstraction managing anonymous entitlement certificates.

m-horky added 8 commits February 6, 2024 14:22

rhsmcertd: Use module-level logger

677a99c

rhsmcertd: Define exit codes

f19eb00

This patch creates a system for exit codes used by the worker. This way the C wrapper may be able to understand what happened inside of the application and alter its behavior.

Identity: Add method to extract current owner

06c2fd2

IdentityUpdateAction: Improve logging for updating identity certificates

8d151e7

CCT-67: Use automatic registration v2

eba1a76

* Card ID: CCT-67 This patch implements the standard and anonymous flows for automatic cloud registration.

Remove API endpoint for automatic cloud registration v1

b70aaf8

* Card ID: CCT-67

m-horky force-pushed the mhorky/CCT-67-anonymous-registration branch from 69013e9 to 4820e5f Compare February 6, 2024 13:22

jirihnidek approved these changes Feb 14, 2024

View reviewed changes

jirihnidek merged commit 46fdc44 into main Feb 14, 2024
16 checks passed

jirihnidek deleted the mhorky/CCT-67-anonymous-registration branch February 14, 2024 13:55

m-horky mentioned this pull request Mar 4, 2024

[1.28] CCT-69: Anonymous registration #3383

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CCT-67: Anonymous registration #3370

CCT-67: Anonymous registration #3370

m-horky commented Jan 16, 2024 •

edited

Loading

github-actions bot commented Jan 16, 2024 •

edited

Loading

jirihnidek left a comment

m-horky commented Feb 2, 2024

jirihnidek left a comment

jirihnidek Feb 5, 2024

jirihnidek Feb 5, 2024

m-horky Feb 6, 2024

jirihnidek Feb 14, 2024

jirihnidek Feb 5, 2024

jirihnidek Feb 5, 2024

jirihnidek Feb 5, 2024

jirihnidek Feb 5, 2024

jirihnidek Feb 6, 2024

jirihnidek Feb 6, 2024

jirihnidek Feb 6, 2024

m-horky Feb 6, 2024

jirihnidek Feb 6, 2024

jirihnidek left a comment

jirihnidek Feb 14, 2024



		class AnonymousCertificateManager:
		"""Manage anonymous certificates.

CCT-67: Anonymous registration #3370

CCT-67: Anonymous registration #3370

Conversation

m-horky commented Jan 16, 2024 • edited Loading

github-actions bot commented Jan 16, 2024 • edited Loading

jirihnidek left a comment

Choose a reason for hiding this comment

m-horky commented Feb 2, 2024

jirihnidek left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jirihnidek left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

m-horky commented Jan 16, 2024 •

edited

Loading

github-actions bot commented Jan 16, 2024 •

edited

Loading