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

Handle URL params for AptRepoFiles #3454

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Handle URL params for AptRepoFiles #3454

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 16 additions & 9 deletions src/subscription_manager/repofile.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
from subscription_manager import utils
from subscription_manager.certdirectory import Path
import configparser
from urllib.parse import parse_qs, urlparse, urlunparse, urlencode
from urllib.parse import parse_qs, urlparse, urlunparse, urlencode, unquote

from rhsm.config import get_config_parser

Expand Down Expand Up @@ -461,19 +461,26 @@ def sections(self):

def fix_content(self, content):
# Luckily apt ignores all Fields it does not recognize
baseurl = content["baseurl"]
url_res = re.match(r"^https?://(?P<location>.*)$", baseurl)
parsed_url = urlparse(unquote(content["baseurl"]))
Copy link
Contributor

Choose a reason for hiding this comment

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

is unquote() still needed? see the discussion on this in #3223

Copy link
Contributor Author

Choose a reason for hiding this comment

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

At least the Candlepin version available for Katello 4.13 (the latest supported release), currently Candlepin 4.4.14 is still affected. I have spent some time looking, but was not able to find any new information when or if Candlepin intends to go back to the old behavior. Given nothing has happened over several months, I can only assume it is not a major priority. The good news is that using unquote will work equally well for quoted and unquoted URLs. As a result I lean towards leaving it in, at least until there is any new information from Candlepin.

Copy link
Contributor Author

@quba42 quba42 Oct 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

It looks like that while some encoding issues in Candlepin were fixed, this does not appear to have changed that the baseurl used as part of these changes has its path part encoded. Not sure if this is intentional or another bug on Candlepin's part. The current state of these changes will work either way.

baseurl = parsed_url._replace(query="").geturl()
ent_res = re.match(r"^/etc/pki/entitlement/(?P<entitlement>.*).pem$", content["sslclientcert"])
if url_res and ent_res:
location = url_res.group("location")
entitlement = ent_res.group("entitlement")
baseurl = "katello://{}@{}".format(entitlement, location)
if ent_res:
netloc = ent_res.group("entitlement") + "@" + parsed_url.netloc
baseurl = parsed_url._replace(scheme="katello", netloc=netloc, query="").geturl()

query = parse_qs(parsed_url.query)
if "rel" in query and "comp" in query:
suites = " ".join(query["rel"][0].split(","))
components = " ".join(query["comp"][0].split(","))
else:
suites = "default"
components = "all"

apt_cont = content.copy()
apt_cont["Types"] = "deb"
apt_cont["URIs"] = baseurl
apt_cont["Suites"] = "default"
apt_cont["Components"] = "all"
apt_cont["Suites"] = suites
apt_cont["Components"] = components
apt_cont["Trusted"] = "yes"

if apt_cont["arches"] is None or apt_cont["arches"] == ["ALL"]:
Expand Down
103 changes: 103 additions & 0 deletions test/test_repolib.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1041,6 +1041,109 @@ def test_fix_content_default(self, mock_file):
self.assertIn(key, act_content)
self.assertEqual(act_content[key], exp_params[key])

@patch("builtins.open", new_callable=mock_open, read_data="data")
def test_fix_content_url_params(self, mock_file):
# Setup mock and expected values
assert open("/etc/apt/sources.list.d/rhsm.sources").read() == "data"
mock_file.assert_called_with("/etc/apt/sources.list.d/rhsm.sources")
exp_params = {
"arches": "none",
"Types": "deb",
"URIs": "https://example.site.org/",
"Suites": "focal",
"Components": "puppet7",
"Trusted": "yes",
"sslclientcert": "mypem.pem",
}
ar = self._helper_stub_repofile()
repo_mock = self._helper_stub_repo(
"mock",
existing_values=[
("baseurl", "https://example.site.org/?comp=puppet7&rel=focal"),
("sslclientcert", exp_params["sslclientcert"]),
],
)
# Modify data
act_content = ar.fix_content(repo_mock)
# Test modification by comparing with expected values
for key in exp_params:
self.assertIn(key, act_content)
self.assertEqual(act_content[key], exp_params[key])

@patch("builtins.open", new_callable=mock_open, read_data="data")
def test_fix_content_url_params_multi(self, mock_file):
# Setup mock and expected values
assert open("/etc/apt/sources.list.d/rhsm.sources").read() == "data"
mock_file.assert_called_with("/etc/apt/sources.list.d/rhsm.sources")
exp_params = {
"arches": "none",
"Types": "deb",
"URIs": "https://example.site.org/",
"Suites": "focal jammy",
"Components": "puppet7 puppet6",
"Trusted": "yes",
"sslclientcert": "mypem.pem",
}
ar = self._helper_stub_repofile()
repo_mock = self._helper_stub_repo(
"mock",
existing_values=[
("baseurl", "https://example.site.org/?comp=puppet7,puppet6&rel=focal,jammy"),
("sslclientcert", exp_params["sslclientcert"]),
],
)
# Modify data
act_content = ar.fix_content(repo_mock)
# Test modification by comparing with expected values
for key in exp_params:
self.assertIn(key, act_content)
self.assertEqual(act_content[key], exp_params[key])

@patch("builtins.open", new_callable=mock_open, read_data="data")
def test_fix_content_url_single_param(self, mock_file):
# Setup mock and expected values
assert open("/etc/apt/sources.list.d/rhsm.sources").read() == "data"
mock_file.assert_called_with("/etc/apt/sources.list.d/rhsm.sources")
exp_params = {
"arches": "none",
"Types": "deb",
"URIs": "https://example.site.org/",
"Suites": "default",
"Components": "all",
"Trusted": "yes",
"sslclientcert": "mypem.pem",
}
# Test only 'comp' param
ar = self._helper_stub_repofile()
repo_mock = self._helper_stub_repo(
"mock",
existing_values=[
("baseurl", "https://example.site.org/?comp=puppet7"),
("sslclientcert", exp_params["sslclientcert"]),
],
)
# Modify data
act_content = ar.fix_content(repo_mock)
# Test modification by comparing with expected values
for key in exp_params:
self.assertIn(key, act_content)
self.assertEqual(act_content[key], exp_params[key])
# Test only 'rel' param
ar = self._helper_stub_repofile()
repo_mock = self._helper_stub_repo(
"mock",
existing_values=[
("baseurl", "https://example.site.org/?rel=focal,jammy"),
("sslclientcert", exp_params["sslclientcert"]),
],
)
# Modify data
act_content = ar.fix_content(repo_mock)
# Test modification by comparing with expected values
for key in exp_params:
self.assertIn(key, act_content)
self.assertEqual(act_content[key], exp_params[key])

@patch("builtins.open", new_callable=mock_open, read_data="data")
def test_fix_content_arches(self, mock_file):
# Setup mock and expected values
Expand Down
Loading