Suma payg rhui via payg client connection #7421
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mcalmer
requested review from
parlt91 and
agraul
and removed request for
a team
Suggested tests to cover this Pull Request
|
parlt91
approved these changes
Aug 16, 2023
agraul
reviewed
Aug 16, 2023
| cn = subject.CN | ||
| try: | ||
| cn = subject.CN | ||
| except: |
There was a problem hiding this comment.
Can we catch the relevant exception instead of everything (including Ctrl+C) ?
There was a problem hiding this comment.
It is not easy to find out which one it is. It happens in one of the clouds, but we need to retest all of them.
I would suggest to keep this, and when you refactor reposync, we can maybe get rid of this whole paramater.
I think it is not needed, but I am not 100% sure.
There was a problem hiding this comment.
I think it's always an AttributeError, just like when you try to access subject.FOO
>>> cert = X509.load_cert_string(c)
>>> subject = cert.get_subject()
>>> subject.FOO
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/usr/lib64/python3.11/site-packages/M2Crypto/X509.py", line 359, in __getattr__
raise AttributeError(self, attr)
AttributeError: (<M2Crypto.X509.X509_Name object at 0x7f22a0ba1bd0>, 'FOO')There was a problem hiding this comment.
Ok, I have changed it
Comment on lines
691
to
699
| repo_conf_file.write(repo_cfg.format( | ||
| reponame=self.channel_label or self.reponame, | ||
| repo_url='baseurl' if not mirrorlist else 'mirrorlist', | ||
| repo_url='baseurl' if not mirrorlist or plugin_used else 'mirrorlist', | ||
| url=_url, | ||
| gpgcheck="0" if self.insecure else "1" | ||
| )) |
There was a problem hiding this comment.
IMO it's not easy to read conditional expressions with multiple conditions. Can we move that out of the function call and do something like:
repo_url = "baseurl"
if mirrorlist and not plugin_used:
repo_url = "mirrorlist"
repo_conf_file.write(repo_cfg.format(...))
Comment on lines
385
to
388
| is_cert = False | ||
| if not verify_certificate_dates(cert): | ||
| return False | ||
| cert = "" | ||
| continue |
There was a problem hiding this comment.
Do I understand this change correctly that we change the date verification from "all certs have valid dates" to "any cert has an valid date"?
If that's the intention, we could also short-circuit the loop and return True as soon as we find the first valid cert.
There was a problem hiding this comment.
I have to run certificate bundles with this funktion.
And RedHat has some expired certificates in the bundle.
So if 1 cert is provided, it must be valid. If multiple are provided, it is ok if one has a valid date.
There was a problem hiding this comment.
I change this to return on the first valid certificate
mcalmer
force-pushed
the
suma-payg-rhui-via-payg-client-connection
branch
from
664908f
to
8dda1a1
Compare
required for RHEL 7 RHUI support
mcalmer
force-pushed
the
suma-payg-rhui-via-payg-client-connection
branch
from
8dda1a1
to
a9fc4ae
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this PR change?
Support RHUI configuration via Pay-as-you-Go client connection in the cloud
Replace #7050
Port of https://github.com/SUSE/spacewalk/pull/21590
GUI diff
No difference.
Documentation
automatically setup and update RHUI repos uyuni-docs#2274
DONE
Test coverage
Unit tests were added
DONE
Changelogs
Make sure the changelogs entries you are adding are compliant with https://github.com/uyuni-project/uyuni/wiki/Contributing#changelogs and https://github.com/uyuni-project/uyuni/wiki/Contributing#uyuni-projectuyuni-repository
If you don't need a changelog check, please mark this checkbox:
If you uncheck the checkbox after the PR is created, you will need to re-run
changelog_test(see below)Re-run a test
If you need to re-run a test, please mark the related checkbox, it will be unchecked automatically once it has re-run: