Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Update Zypper auth check #1270

Merged
merged 15 commits into from
Jan 20, 2025
Merged

Update Zypper auth check #1270

merged 15 commits into from
Jan 20, 2025

Conversation

jesusbv
Copy link
Collaborator

@jesusbv jesusbv commented Jan 9, 2025 •
edited
Loading

Description

When a hybrid system runs zypper command, the check
needs to take care of the subscription verification

Currently, the check lacks paid extension ID, resulting in
an unsuccessful verification

This change fixes that

If system is hybrid, and the path being accessed belongs to a paid extension,
we check that the paid extension subscription is active

if the repository path belongs to a free repository or no matches with paid extensions,
no need to check the subscription

if ZypperAuth handle_scc_subscription method gets called without a product id,
the method will check all non free products suscriptions to be active, if any

This fixes bsc#1230157

How to test

Start an Azure instance, register LTSS, wait > 20 min for the cache to get expired
run zypper ref should work

Change Type

Please select the correct option.

  • Bug Fix (a non-breaking change which fixes an issue)
  • New Feature (a non-breaking change which adds new functionality)
  • Documentation Update (a change which only updates documentation)

Checklist

Please check off each item if the requirement is met.

  • I have reviewed my own code and believe that it's ready for an external review.
  • I have provided comments for any hard-to-understand code.
  • I have documented the MANUAL.md file with any changes to the user experience.
  • If my changes are non-trivial, I have added a changelog entry to notify users at package/obs/rmt-server.changes.

Review

Please check out our review guidelines
and get in touch with the author to get a shared understanding of the change.

@jesusbv jesusbv self-assigned this Jan 9, 2025
@jesusbv jesusbv requested review from digitaltom and rjschwei January 9, 2025 16:49
@jesusbv jesusbv added the 2.21 label Jan 9, 2025
@jesusbv jesusbv marked this pull request as draft January 9, 2025 20:00
@jesusbv jesusbv changed the title Fix the check for allowing repositories Update Zypper auth check Jan 10, 2025
@jesusbv
Copy link
Collaborator Author

jesusbv commented Jan 10, 2025

I will add the tests and coverage but it can be reviewed in the meantime

@jesusbv jesusbv marked this pull request as ready for review January 10, 2025 10:24
@jesusbv jesusbv requested a review from rjschwei January 10, 2025 10:33
@jesusbv jesusbv mentioned this pull request Jan 15, 2025
Open
7 tasks
@jesusbv
Update Zypper auth check
331fdaa 
When a hybrid system runs zypper command, the check
needs to take care of the subscription verification

Currently, the check lacks paid extension ID, resulting in
an unsuccessful verification

This change fixes that

If system is hybrid, and the path being accessed belongs to a paid extension,
we check that the paid extension subscription is active

if the repository path belongs to a free repository or no matches with paid extensions,
no need to check the subscription

if ZypperAuth handle_scc_subscription method gets called without a product id,
the method will check _all_ non free products suscriptions to be active, if any

This fixes bsc#1230157
@jesusbv
Add pubcloud section to changelog
5dcbc5a
@jesusbv
Add tests to check paid extensions in ZypperAuth engine
efade7f 
Add paid extension to factory
Add paid product to headers
@jesusbv
Fix ZypperAuth check for paid products
b5cd007 
if system is either hybrid or byos, we need to check
the paid extensions

if the system is byos, we need to check any extension

update the way we check SCC subscriptions,
now we check if the subscription is active and the product matches
earlier in the code
@jesusbv
Rename method for clarity
5836a2d
@jesusbv
Fix path check and zypper auth check
b8e844d 
For a path to be valid, the path must be in the system products repositories

if the product the repository path belongs to is not free,
the product must be either included in the base product or activated on SCC,
or it belongs to the base product repositories

if a non free product that the path belongs to in a hybrid system, is not on
SCC, we check the instance metadata to verify it is included

Refactor the way path_allowed? is performed

Update SCC checks to use product class

This fixes LTSS activation, de-activation and keep the cache refreshing for the
registry working as expected
@jesusbv
Update tests
08e8da3
@jesusbv
Copy link
Collaborator Author

jesusbv commented Jan 17, 2025

Apologies for the big commit

I will add the remaining needed tests (to check cache refresh)

@jesusbv
Fix rubocop
281fed7
@jesusbv
add extra parameter
821ac0f 
this needs to be removed in a different commit
@digitaltom
Copy link
Member

Unfortunately I don't have a good way to verify & test this. So when Robert or somebody else from the pubcloud team approved, and tests on your side show it's fixing the hybrid scenarios, it's ok for me.

@jesusbv
Copy link
Collaborator Author

jesusbv commented Jan 17, 2025 •
edited
Loading

Unfortunately I don't have a good way to verify & test this. So when Robert or somebody else from the pubcloud team approved, and tests on your side show it's fixing the hybrid scenarios, it's ok for me.

I have tested this changes in
SLES12-SP5-BYOS

  • refresh cache works OK (cloudregistruauth) with and without LTSS
  • zypper ref works OK
    • LTSS active
    • without LTSS
  • clean registration and re-register works OK

SLES4SAP12-SP5-BYOS

  • refresh cache works OK (cloudregistruauth)
  • zypper ref works OK
  • clean registration and re-register works OK

SLES4SAP15-SP5-PAYG

  • refresh cache works OK (cloudregistruauth)
  • zypper ref works OK
    • LTSS active
    • without LTSS
  • clean registration and re-register works OK

@jesusbv
Remove product_path_access, activations_fail_state methods
228c2ff 
Call directly to product_class_access,
even if the header is empty, i.e. refreshing the cache for the registry request,
using the base module is enough to get the SCC state of the subscription
@jesusbv
Leave early when no path found
675e2f8
@jesusbv
Remove unnecessary parameter
8f274af
@jesusbv jesusbv merged commit d04f4b4 into master Jan 20, 2025
3 checks passed
@jesusbv jesusbv deleted the fix-auth-repo branch January 20, 2025 12:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@digitaltom digitaltom digitaltom left review comments

@rjschwei rjschwei rjschwei approved these changes

Assignees

@jesusbv jesusbv

Labels
2.21
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jesusbv @digitaltom @rjschwei