pkg/rhsm: match subscription baseurls with wildcards (RHEL-36789)#2460
pkg/rhsm: match subscription baseurls with wildcards (RHEL-36789)#2460lucasgarfield wants to merge 1 commit into
Conversation
517737e to
9d25430
Compare
| sslclientcert = /etc/pki/entitlement/111.pem | ||
| metadata_expire = 86400 | ||
| enabled_metadata = 0 | ||
| ` |
There was a problem hiding this comment.
This is only valid for Satellite versions up to 6.14.
Newer versions do not have sslcacert set, this value is missing completely. The CA cert is being inserted into the system CA bundle and is also available at /etc/rhsm/ca/katello-server-ca.pem. I think the code should account that, it currently falls back to /etc/rhsm/ca/redhat-uep.pem that is incorrect and will fail the CA server check.
This does not have to be part of this PR tho, it is somewhat separate issue.
There was a problem hiding this comment.
Is this a problem with the functionality or just the example used in the test here?
There was a problem hiding this comment.
Hm, odd. I took inspiration from this from the /etc/yum.repos.d/redhat.repo on a 10.2 machine registered to a Satellite on 6.20:
[user@localhost ~]$ cat /etc/yum.repos.d/redhat.repo
#
# Certificate-Based Repositories
# Managed by (rhsm) subscription-manager
#
# *** This file is auto-generated. Changes made here will be overwritten. ***
# *** Use "subscription-manager repo-override --help" if you wish to make changes. ***
#
# If this file is empty and this system is subscribed, consider
# running "dnf repolist" to refresh the available repositories.
#
[rhel-10-for-x86_64-baseos-rpms]
name = Red Hat Enterprise Linux 10 for x86_64 - BaseOS (RPMs)
baseurl = https://ip-10-0-167-154.rhos-01.prod.psi.rdu2.redhat.com/pulp/content/image-buidler/Library/content/dist/rhel10/$releasever/x86_64/baseos/os
enabled = 1
gpgcheck = 1
gpgkey = file:///etc/pki/rpm-gpg/RPM-GPG-KEY-redhat-release
sslverify = 1
sslcacert = /etc/rhsm/ca/katello-server-ca.pem
sslclientkey = /etc/pki/entitlement/517534911145439618-key.pem
sslclientcert = /etc/pki/entitlement/517534911145439618.pem
metadata_expire = 1
enabled_metadata = 1
[rhel-10-for-x86_64-appstream-rpms]
name = Red Hat Enterprise Linux 10 for x86_64 - AppStream (RPMs)
baseurl = https://ip-10-0-167-154.rhos-01.prod.psi.rdu2.redhat.com/pulp/content/image-buidler/Library/content/dist/rhel10/$releasever/x86_64/appstream/os
enabled = 1
gpgcheck = 1
gpgkey = file:///etc/pki/rpm-gpg/RPM-GPG-KEY-redhat-release
sslverify = 1
sslcacert = /etc/rhsm/ca/katello-server-ca.pem
sslclientkey = /etc/pki/entitlement/517534911145439618-key.pem
sslclientcert = /etc/pki/entitlement/517534911145439618.pem
metadata_expire = 1
enabled_metadata = 1
There was a problem hiding this comment.
btw I don't doubt that in some configs sslcacert is not set, but this is from what I would consider a very "naive" setup and it is indeed there - this is from a fresh RHEL 10.2 qcow2 built with Image Builder and registered to Satellite 6.20 using the host registration command that Satellite generates for you (the same one that you can paste into the Image Builder wizard).
There was a problem hiding this comment.
Okay, good. I swear I saw that the other day, I think it might have been Lightspeed registered system not Satellite. Not a problem then.
|
This changes the behavior that is not covered by tests. Previously, the function matched the URL that was the exact same architecture and version (which was buggy). Now, the code picks the first URL that matches the URL ignoring all variables. This should not be a problem for modern RHEL systems with SCA, in these cases any I am not fan of the regexp-based solution tho, this could be as easy as:
|
achilleas-k
left a comment
There was a problem hiding this comment.
LGTM but @lzap's concerns / suggestions sound reasonable. I think it makes sense for the implementation here to match the behaviour in osbuild though, so if doing a partial URL match makes more sense, we should rethink the behaviour in osbuild as well.
I'd like to see what real configs look like with examples of how our current behaviour fails. We should at least make some test cases that match those.
9d25430 to
3a0df32
Compare
I'm not a fan of the regex either, but that's how it is done in osbuild. See https://github.com/osbuild/osbuild/blob/87e170f20af58189722cbac7cab0f18256aea0ed/osbuild/util/rhsm.py. I think there’s a very good reason to mimic the osbuild behavior. Namely, if we do eventually manage to delegate this to osbuild as suggested in #2055 then we will be suddenly changing the implementation. Matching the osbuild behavior now in images gives us a gentle on-ramp where the implementation will already be tested in production. Also, the regex based osbuild code will run later anyway. If there is a latent bug, I say it is better to fail fast and catch it here. |
Here’s a redhat.repo from RHEL 10.2 on a 6.20 Satellite: The current behavior fails like so. image-builder/pkg/rhsm/secrets.go Line 181 in 0bfdfd7 GetSecretsForBaseurl loops over the hosts s.available subscriptions and checks whether the templated baseurl matches the requested rr.BaseURL. If it does, it grabs the secrets. If it doesn’t, it hits the fallback and uses /etc/rhsm/ca/redhat-uep.pem. So, say the subscription baseurl is .../rhel10/$releasever/x86_64/baseos/os. The substitution causes this to become .../rhel10/10/x86_64/baseos/os. But, the BaseURL from rr.BaseURLs will be the minor version if using composer-cli or image-builder, so something like .../rhel10/10.2/x86_64/baseos/os. These don’t match and we hit the fallback. Note that cockpit-composer uses the rolling release distro def, so it actually works fine. I actually think that we’re hitting this fallback every build when using composer-cli or image-builder. We actually only noticed this because on a Satellite, we need the Katello cert at /etc/rhsm/ca/katello-server-ca.pem and not redhat-uep.pem. That’s also why the workaround in this acccess.redhat.com article (https://access.redhat.com/solutions/5773421) works – they suggest symlinking the hardcoded redhat-uep.pem fallback to the Katello cert. I have verified all of these behaviors myself. |
469f779 to
41e4437
Compare
|
Also @achilleas-k regarding tests, I updated the test cases and they cover the current failing behavior. TestGetSecretsForBaseurlPointReleasePrefersSubscriptionCA uses SATELLITE_REPO which mirrors the real redhat.repo I’ve pasted above. The sslcacert points to /etc/rhsm/ca/katello-server-ca.pem. The global fallback is still /etc/rhsm/ca/redhat-uep.pem. It requests the point release for 10.2 and asserts that the Katello CA gets returned. In the old code, we would have hit the fallback and would get redhat-uep.pem instead of the Katello CA. |
GetSecretsForBaseurl matched a requested repository baseurl against the host's redhat.repo entries by substituting the major-only releasever and the arch into each subscription baseurl and comparing for equality. Repository definitions ship both a rolling baseurl (e.g. .../9/...) and point-release baseurls (e.g. .../9.8/...), so for a point release the substituted subscription URL (.../9/...) never equalled the requested URL (.../9.8/...). The lookup then fell through to the global fallback secrets, which hardcode the CA certificate to /etc/rhsm/ca/redhat-uep.pem. On a Satellite host the correct CA is the Katello certificate that redhat.repo points sslcacert at, so the fallback returned the wrong CA and depsolving failed. Match the yum variables ($releasever, $arch, $basearch, $uuid) with a non-slash wildcard instead, mirroring osbuild's util/rhsm.py. A point-release request then matches the subscription regardless of the releasever or arch, so the per-subscription certificates are returned. The matching no longer depends on the substituted values, so the now-unused arch and releasever parameters are dropped.
41e4437 to
d390331
Compare
lzap
left a comment
There was a problem hiding this comment.
If the goal is to mirror osbuild behavior, then this is good. It is not ideal implementation, but it is not wrong. Today, SCA is used everywhere and this means that we can essentially pick a random certs from redhat.repo and it will work.
GetSecretsForBaseurl matched a requested repository baseurl against the host's redhat.repo entries by substituting the major-only releasever and the arch into each subscription baseurl and comparing for equality. Repository definitions ship both a rolling baseurl (e.g. .../9/...) and point-release baseurls (e.g. .../9.8/...), so for a point release the substituted subscription URL (.../9/...) never equalled the requested URL (.../9.8/...). The lookup then fell through to the global fallback secrets, which hardcode the CA certificate to /etc/rhsm/ca/redhat-uep.pem. On a Satellite host the correct CA is the Katello certificate that redhat.repo points sslcacert at, so the fallback returned the wrong CA and depsolving failed.
Match the yum variables ($releasever, $arch, $basearch, $uuid) with a non-slash wildcard instead, mirroring osbuild's util/rhsm.py. A point-release request then matches the subscription regardless of the releasever or arch, so the per-subscription certificates are returned. The matching no longer depends on the substituted values, so the now-unused arch and releasever parameters are dropped.