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.

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

[1.24] 2182768: Disallowed attaching pool in SCA mode #3271

Merged
merged 10 commits into from May 30, 2023

Conversation

ptoscano
Copy link
Contributor

@ptoscano ptoscano commented May 25, 2023

  • BZ: https://bugzilla.redhat.com/show_bug.cgi?id=2182768
  • Manual attaching of pool using --pool CLI option is not
    allowed now. When user tries to attach any pool in SCA
    mode, then info message is printedn and subscription-manager
    is terminated
  • Added two unit tests for auto-attach ins SCA mode and
    manual attach
  • Added more debug prints for cache used by function
    is_simple_content_access(), because I considered using
    this method, when --help is used, but calling
    subscription-manager attach --help would cause calling
    one REST API endpoint
  • Updated manual page about subscription-manager
  • Make sure to not disallow auto-attach in the D-Bus interface, to avoid breaking users

Backport to 1.24 of the following PRs (backported from 1.28 to ease the backport changed needed):

ptoscano and others added 2 commits May 25, 2023 14:11
Add a local get_current_owner() helper function that mimicks the one
available in newer versions. Backporting the implementation available in
newer version is complicated, and relies on not available bits such as
the owner cache.

This simple method will do, and reduces the differences in a couple of
places with what's in more recent branches.
* BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1928072
* When SCA mode is used for selected organization, then do
  not try to do auto-attach at all and print info message
* We use the same message as is already used for
  command attach --auto
* Implemented this functionality, because old message was
  generated by candlepin server. Look at following candlepin PR:

candlepin/candlepin#2947
(cherry picked from commit a36e10f)
(cherry picked from commit a5dc6f6)
@ptoscano ptoscano force-pushed the ptoscano/2182768 branch 2 times, most recently from acab65c to a37ef31 Compare May 25, 2023 12:41
jirihnidek and others added 5 commits May 25, 2023 15:25
* Card ID: ENT-4594
* Manual attaching of pool using --pool CLI option is not
  allowed now. When user tries to attach any pool in SCA
  mode, then info message is printedn and subscription-manager
  is terminated
* Added two unit tests for auto-attach ins SCA mode and
  manual attach
* Added more debug prints for cache used by function
  is_simple_content_access(), because I considered using
  this method, when --help is used, but calling
  subscription-manager attach --help would cause calling
  one REST API endpoint
* Updated manual page about subscription-manager

(cherry picked from commit 61cb998)
(cherry picked from commit 4d258b1)
* Card ID: ENT-4594
* When D-Bus methods AutoAttach() or PoolAttach() are called in
  SCA mode, then exception is raised
* Added two unit tests for this case

(cherry picked from commit 29d8000)
(cherry picked from commit a0b5d65)
Remove an extra 'not' in a debug message, so the sentence makes sense.

Reported by: Chris Snyder

(cherry picked from commit fcb8e4e)
(cherry picked from commit 8a08c55)
* BZ: https://bugzilla.redhat.com/show_bug.cgi?id=2049101
* Card ID: ENT-4718
* Backport of original PR #2961 to 1.28.
* Reverting of following commit:
  a0b5d65
* We were too aggressive, when we decided to disable this
  auto-attaching and attaching of pool for D-Bus API, when SCA mode
  is used.
* This reverting should also give enough time for solving other BZs:
  https://bugzilla.redhat.com/show_bug.cgi?id=2049101 (Anaconda)
  https://bugzilla.redhat.com/show_bug.cgi?id=2049620 (Gnome)

(cherry picked from commit 9a3f219)
* BZ: https://bugzilla.redhat.com/show_bug.cgi?id=2097672
* Backport to 1.28 branch
* Origin PR for main: #3116
  * Not possible to do simple cherry-pick
* Card ID: ENT-5115
* Changed warning message according suggestion in BZ bug report.
* Refactored code a little (fixed typo in method name)

(cherry picked from commit 7ee691a)
@ptoscano
Copy link
Contributor Author

There are two tests that fail... because they are broken! I created #3272 to fix them.

@ptoscano
Copy link
Contributor Author

There are two tests that fail... because they are broken! I created #3272 to fix them.

After #3272 and #3273 were merged, backporting it here indeed fixed the unit test failures.

Hence, unless I screwed up something else and I didn't realize it, I declare it as ready.

@ptoscano ptoscano marked this pull request as ready for review May 29, 2023 15:26
jirihnidek and others added 3 commits May 29, 2023 17:32
* BZ: https://bugzilla.redhat.com/show_bug.cgi?id=2097672
* Manual backport of original PR: #3138
  * Original commit: 2e62d0f
* Card ID: ENT-5411

(cherry picked from commit f32cd11)
- BZ: https://bugzilla.redhat.com/show_bug.cgi?id=2097672
- Card ID: ENT-5443
- Updated the expected message from a user's manual attach
request (--pool) when SCA is enabled for the organization.

(cherry picked from commit b52b2ea)
There are a couple of test cases for the "attach" command that check
the behaviour for "--auto" and "--pool POOL_ID" in case of SCA; the
problem is that they were broken until now for a couple of reasons:
- the main() method of a command class (derived from AbstractCLICommand)
  takes a list of strings, and not a single string; since
  ArgumentParser.parse_known_args() iterates on its "args" argument, and
  iterating over a str is possible in Python 3, this "runs fine",
  however it matches nothing, exiting with an usage message
- as consequence/related to the above, the two tests assumed the runs
  would fail, whereas because of the behaviour change they now cause a
  clean exit (with exit code 0)

Hence, repair these two tests so they work:
- pass the arguments to main() properly as list of strings, so
  ArgumentParser can parse them
- drop the exception expectation, since they cause a clean exit

Followup of commit 61cb998.

(cherry picked from commit 792d685)
(cherry picked from commit d6f2b79)
Copy link
Contributor

@jirihnidek jirihnidek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, Thanks 👍

@jirihnidek jirihnidek merged commit 88a9454 into subscription-manager-1.24 May 30, 2023
2 checks passed
@jirihnidek jirihnidek deleted the ptoscano/2182768 branch May 30, 2023 09:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants