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

Handle URL params for AptRepoFiles #3223

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
15 changes: 12 additions & 3 deletions src/subscription_manager/repofile.py
Original file line number Diff line number Diff line change
Expand Up @@ -462,19 +462,28 @@ def sections(self):

def fix_content(self, content):
# Luckily apt ignores all Fields it does not recognize
baseurl = content["baseurl"]
parsed_url = urlparse(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.

In my development environment, I had to change this to:

Suggested change
parsed_url = urlparse(content["baseurl"])
parsed_url = urlparse(unquote(content["baseurl"]))

Maybe this is something that changed with a newer version of candlepin?

Perhaps we should make sure it will work both for quoted or unquoted baseurls!

Copy link
Contributor

Choose a reason for hiding this comment

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

Using a recent Candlepin (4.4.x) by chance? If so, the problem should be a regression in CP, see also
https://community.theforeman.org/t/status-code-403-for-error-failed-to-download-metadata-for-repo-after-upgrade-to-3-11/38601/7
(CP developers are aware, unfortunately no public tickets to track this)

baseurl = parsed_url._replace(query="").geturl()
url_res = re.match(r"^https?://(?P<location>.*)$", baseurl)
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)

query = parse_qs(parsed_url.query)
if "rel" in query and "comp" in query:
suites = query["rel"][0].replace(",", " ")
components = query["comp"][0].replace(",", " ")
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
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