Separate CIS and USG in the client #3344

renanrodrigo · 2024-10-08T20:21:18Z

Why is this needed?

This PR solves all of our problems because it implements the changes needed to turn USG into an actual resource on the client side. This aligns client behaviour with the implementation done for the backend

Test Steps

kept behavior:

enabling CIS should work on X, B
enabling USG should work on J
both should work on F, but USG gets enabled in both cases , with a message telling about the transition

new behavior:

upgrading from older versions of pro client where CIS was enabled as USG on F, J should enable the actual USG resource

Integration tests TBD

(un)check this to re-run the checklist action

github-actions · 2024-10-08T20:21:32Z

PR Checklist

How to use this checklist

PR Author

For each section, check a box when it is true.
Uncheck a box if it becomes un-true.
Then check the box at the bottom of the PR description to re-run the action that creates this checklist.
The action that creates and updates this comment will retain your edits.
The action will fail if the checklist is not completed.

PR Reviewer

Check that the PR checklist action did not fail.
Double check that the author filled out the checklist accurately.
If you disagree with a checklist item, start a conversation.
For example, the author may say they don't think integration tests are necessary, but you may disagree.

Bug References

None.

Confirm

I've properly referenced all bugs that this PR fixes

How to properly reference fixed bugs

If this PR is related to a Jira item, include an SC-1234 reference in the PR title
If this PR is fixes a GitHub issue, include a Fixes: #1234 reference in the commit that fixes the issue
If this PR is fixes a Launchpad bug, include a LP: #12345678 reference in the commit that fixes the issue

Test Updates

Unit Tests

I have updated or added any unit tests accordingly
No unit test changes are necessary for this change

Integration Tests

I have updated or added any integration tests accordingly
No integration test changes are necessary for this change

Documentation

Changes here need to be documented and I have referenced the docs PR in the description
No documentation updates are necessary for this change

Does this PR require review from someone outside the core ubuntu-pro-client team?

Yes, and I have requested those reviews via GitHub
No

uaclient/cli/enable.py

orndorffgrant

Looking good! A few little things.
I guess we can't really test yet until the resource is up on the backend

lib/postinst-migrations.sh

uaclient/entitlements/tests/test_base.py

uaclient/cli/enable.py

orndorffgrant · 2024-10-16T13:25:46Z

lib/postinst-migrations.sh

+    # On Focal and Jammy, the CIS service transitioned to USG from v35
+    # This changes the lists and authentication files to represent that
+    # in case CIS is enabled
+    if [ "$VERSION_ID" = "20.04" ] || [ "$VERSION_ID" = "22.04" ]; then


This can have a behave test in the style of https://github.com/canonical/ubuntu-pro-client/pull/3341/files#diff-95cb2615ef8a6b5f6b24db69b295f91bd822a5d26cb1a75769c1fd0d83df5a8fR42

The only thing missing in this PR now is integration tests. I have manually tested it many times up to the point where we can without the backends, and will have the behave stuff in place asap

Signed-off-by: Renan Rodrigo <[email protected]>

Get rid of valid_names, called_name and related functionality, so entitlements are called only by the name defined for them. Signed-off-by: Renan Rodrigo <[email protected]>

Signed-off-by: Renan Rodrigo <[email protected]>

The entitlement classes list now returns a different value for xenial and bionic, which includes cis, and for the other releases, which includes usg Signed-off-by: Renan Rodrigo <[email protected]>

This way it is easier to test, as the return value is not determined at import time. Signed-off-by: Renan Rodrigo <[email protected]>

As Focal is the only series to ever have both of the services in a valid state at different times, there is a particular path to enable USG instead of CIS when users call `pro enable cis`. A specific message is shown on the CLI to tell users that it changed. Signed-off-by: Renan Rodrigo <[email protected]>

renanrodrigo · 2024-10-22T08:52:10Z

@orndorffgrant @lucasmoura this is done feature-wise, just missing the beave testing.
If you want to merge this to finalize the release code, I can send a separate PR with just the tests.

The old CIS entitlement will not be recognized on Focal and Jammy after version 35. This converts the .list file and changes cis/ to usg/ both in the source and authentication files when running on these series. Signed-off-by: Renan Rodrigo <[email protected]>

orndorffgrant · 2024-10-22T14:41:53Z

uaclient/api/u/pro/services/enable/v1.py

+        and system.get_release_info().series == "focal"
+    ):
+        service_to_enable = "usg"
+        USGEntitlement.cis_version = True


sneaky sneaky heheh

we're going to love hating this
or hate loving this? unclear

but I see the compromises made and I'm happy with it. +1

I'm hate hating this for years now I hate this part a little less

orndorffgrant

I'm okay merging this ahead of integration tests, but would like @lucasmoura to approve as well before we do

We are now sorting the entitlements by name which requires a test change

renanrodrigo force-pushed the usg-now branch from c34a648 to 386a617 Compare October 9, 2024 03:43

orndorffgrant added the next label Oct 9, 2024

renanrodrigo force-pushed the usg-now branch from 930a163 to 947c6da Compare October 10, 2024 02:33

renanrodrigo marked this pull request as ready for review October 11, 2024 12:36

renanrodrigo force-pushed the usg-now branch from 1018299 to 9c20a57 Compare October 11, 2024 16:48

lucasmoura reviewed Oct 14, 2024

View reviewed changes

uaclient/cli/enable.py Outdated Show resolved Hide resolved

orndorffgrant reviewed Oct 16, 2024

View reviewed changes

renanrodrigo added 7 commits October 22, 2024 10:44

entitlements: ignore any presentedAs information from the backend

c6af846

Signed-off-by: Renan Rodrigo <[email protected]>

entitlements: make each entitlement have a single name

e4a1899

Get rid of valid_names, called_name and related functionality, so entitlements are called only by the name defined for them. Signed-off-by: Renan Rodrigo <[email protected]>

entitlements: add 'USG' as an entitlement

13553d1

Signed-off-by: Renan Rodrigo <[email protected]>

chore: typecheck entitlement_factory correctly

5d732d6

Signed-off-by: Renan Rodrigo <[email protected]>

entitlements: separate CIS and USG by series

57abfa5

The entitlement classes list now returns a different value for xenial and bionic, which includes cis, and for the other releases, which includes usg Signed-off-by: Renan Rodrigo <[email protected]>

chore: turn ENTITLEMENT_CLASSES into a function

777b69f

This way it is easier to test, as the return value is not determined at import time. Signed-off-by: Renan Rodrigo <[email protected]>

renanrodrigo force-pushed the usg-now branch from 9c20a57 to 5a0ca04 Compare October 22, 2024 08:49

renanrodrigo force-pushed the usg-now branch from 5a0ca04 to a1923c8 Compare October 22, 2024 09:26

orndorffgrant reviewed Oct 22, 2024

View reviewed changes

orndorffgrant approved these changes Oct 22, 2024

View reviewed changes

tests: fix u.pro.services.dependencies.v1 order

6f8ede1

We are now sorting the entitlements by name which requires a test change

lucasmoura approved these changes Oct 23, 2024

View reviewed changes

orndorffgrant enabled auto-merge (rebase) October 23, 2024 14:12

orndorffgrant merged commit 68b1906 into main Oct 23, 2024
20 of 25 checks passed

orndorffgrant deleted the usg-now branch October 23, 2024 14:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Separate CIS and USG in the client #3344

Separate CIS and USG in the client #3344

renanrodrigo commented Oct 8, 2024 •

edited by orndorffgrant

Loading

github-actions bot commented Oct 8, 2024 •

edited by orndorffgrant

Loading

How to use this checklist

PR Author

PR Reviewer

orndorffgrant left a comment

orndorffgrant Oct 16, 2024

renanrodrigo Oct 22, 2024

renanrodrigo commented Oct 22, 2024

orndorffgrant Oct 22, 2024

renanrodrigo Oct 24, 2024

orndorffgrant left a comment

Separate CIS and USG in the client #3344

Separate CIS and USG in the client #3344

Conversation

renanrodrigo commented Oct 8, 2024 • edited by orndorffgrant Loading

Why is this needed?

Test Steps

Integration tests TBD

github-actions bot commented Oct 8, 2024 • edited by orndorffgrant Loading

PR Checklist

How to use this checklist

PR Author

PR Reviewer

Bug References

Confirm

Test Updates

Unit Tests

Integration Tests

Documentation

Does this PR require review from someone outside the core ubuntu-pro-client team?

orndorffgrant left a comment

Choose a reason for hiding this comment

orndorffgrant Oct 16, 2024

Choose a reason for hiding this comment

renanrodrigo Oct 22, 2024

Choose a reason for hiding this comment

renanrodrigo commented Oct 22, 2024

orndorffgrant Oct 22, 2024

Choose a reason for hiding this comment

renanrodrigo Oct 24, 2024

Choose a reason for hiding this comment

orndorffgrant left a comment

Choose a reason for hiding this comment

renanrodrigo commented Oct 8, 2024 •

edited by orndorffgrant

Loading

github-actions bot commented Oct 8, 2024 •

edited by orndorffgrant

Loading