diff --git a/debian/changelog b/debian/changelog index 6fb6d44aec8..a35239ab75b 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,15 @@ +cloud-init (24.4.1-0ubuntu0~24.04.2) noble; urgency=medium + + * cherry-pick fixes for MAAS traceback (LP: #2100963) + - cherry-pick c60771d8: test: pytestify test_url_helper.py + - cherry-pick 8810a2dc: test: Remove CiTestCase from + test_url_helper.py + - cherry-pick 582f16c1: test: add OauthUrlHelper tests + - cherry-pick 9311e066: fix: Update OauthUrlHelper to use readurl + exception_cb + + -- James Falcon Thu, 13 Mar 2025 14:37:50 -0500 + cloud-init (24.4.1-0ubuntu0~24.04.1) noble; urgency=medium * Add d/p/cpick-84806336-chore-Add-feature-flag-for-manual-network-waiting diff --git a/debian/patches/cpick-582f16c1-test-add-OauthUrlHelper-tests b/debian/patches/cpick-582f16c1-test-add-OauthUrlHelper-tests new file mode 100644 index 00000000000..50d70e35731 --- /dev/null +++ b/debian/patches/cpick-582f16c1-test-add-OauthUrlHelper-tests @@ -0,0 +1,156 @@ +From 582f16c143b1d071c78f259bb978296b1439e186 Mon Sep 17 00:00:00 2001 +From: James Falcon +Date: Mon, 3 Mar 2025 13:25:24 -0600 +Subject: [PATCH] test: add OauthUrlHelper tests +Bug: https://github.com/canonical/cloud-init/issues/6065 +Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/cloud-init/+bug/2100963 + +--- + tests/unittests/test_url_helper.py | 102 ++++++++++++++++++++++++++++- + 1 file changed, 99 insertions(+), 3 deletions(-) + +--- a/tests/unittests/test_url_helper.py ++++ b/tests/unittests/test_url_helper.py +@@ -6,13 +6,14 @@ import pathlib + from functools import partial + from threading import Event + from time import process_time ++from unittest import mock + from unittest.mock import ANY, call + + import pytest + import requests + import responses + +-from cloudinit import util, version ++from cloudinit import url_helper, util, version + from cloudinit.url_helper import ( + REDACTED, + UrlError, +@@ -24,7 +25,6 @@ from cloudinit.url_helper import ( + readurl, + wait_for_url, + ) +-from tests.unittests.helpers import mock, skipIf + + try: + import oauthlib +@@ -38,6 +38,14 @@ except ImportError: + M_PATH = "cloudinit.url_helper." + + ++def exception_cb(exception): ++ return True ++ ++ ++def headers_cb(url): ++ return {"cb_key": "cb_value"} ++ ++ + class TestOAuthHeaders: + def test_oauth_headers_raises_not_implemented_when_oathlib_missing(self): + """oauth_headers raises a NotImplemented error when oauth absent.""" +@@ -46,7 +54,9 @@ class TestOAuthHeaders: + oauth_headers(1, 2, 3, 4, 5) + assert "oauth support is not available" == str(context_manager.value) + +- @skipIf(_missing_oauthlib_dep, "No python-oauthlib dependency") ++ @pytest.mark.skipif( ++ _missing_oauthlib_dep, reason="No python-oauthlib dependency" ++ ) + @mock.patch("oauthlib.oauth1.Client") + def test_oauth_headers_calls_oathlibclient_when_available(self, m_client): + """oauth_headers calls oaut1.hClient.sign with the provided url.""" +@@ -68,6 +78,92 @@ class TestOAuthHeaders: + assert "url" == return_value + + ++class TestOauthUrlHelper: ++ @responses.activate ++ def test_wrapped_readurl(self): ++ """Test the wrapped readurl happy path.""" ++ oauth_helper = url_helper.OauthUrlHelper() ++ url = "http://myhost/path" ++ data = b"This is my url content" ++ responses.add(responses.GET, url, data) ++ assert oauth_helper.readurl(url).contents == data ++ ++ @responses.activate ++ def test_default_exception_cb(self, tmp_path, caplog): ++ """Test that the default exception_cb is used.""" ++ skew_file = tmp_path / "skew.json" ++ oauth_helper = url_helper.OauthUrlHelper(skew_data_file=skew_file) ++ url = "http://myhost/path" ++ data = b"This is my url content" ++ response = requests.Response() ++ response.status_code = 401 ++ response._content = data ++ response.headers["date"] = "Wed, 21 Oct 2015 07:28:00 GMT" ++ responses.add_callback( ++ responses.GET, ++ url, ++ callback=lambda _: requests.HTTPError(response=response), ++ ) ++ with pytest.raises(UrlError) as e: ++ oauth_helper.readurl(url) ++ assert e.value.code == 401 ++ assert "myhost" in skew_file.read_text() ++ ++ @responses.activate ++ def test_custom_exception_cb(self): ++ """Test that a custom exception_cb is used.""" ++ oauth_helper = url_helper.OauthUrlHelper() ++ url = "http://myhost/path" ++ data = b"This is my url content" ++ responses.add(responses.GET, url, data, status=401) ++ exception_cb = mock.Mock(return_value=True) ++ ++ with pytest.raises(UrlError): ++ oauth_helper.readurl(url, exception_cb=exception_cb) ++ exception_cb.assert_called_once() ++ assert isinstance(exception_cb.call_args[0][0], UrlError) ++ assert exception_cb.call_args[0][0].code == 401 ++ ++ @responses.activate ++ def test_default_headers_cb(self, mocker): ++ """Test that the default headers_cb is used.""" ++ m_headers = mocker.patch( ++ "cloudinit.url_helper.oauth_headers", ++ return_value={"key1": "value1"}, ++ ) ++ mocker.patch("time.time", return_value=5) ++ oauth_helper = url_helper.OauthUrlHelper() ++ oauth_helper.skew_data = {"myhost": 125} ++ oauth_helper._do_oauth = True ++ url = "http://myhost/path" ++ data = b"This is my url content" ++ responses.add(responses.GET, url, data) ++ response = oauth_helper.readurl(url) ++ request_headers = response._response.request.headers ++ assert "key1" in request_headers ++ assert request_headers["key1"] == "value1" ++ assert m_headers.call_args[1]["timestamp"] == 130 ++ ++ @responses.activate ++ def test_custom_headers_cb(self, mocker): ++ """Test that a custom headers_cb is used.""" ++ mocker.patch( ++ "cloudinit.url_helper.oauth_headers", ++ return_value={"key1": "value1"}, ++ ) ++ oauth_helper = url_helper.OauthUrlHelper() ++ oauth_helper._do_oauth = True ++ url = "http://myhost/path" ++ data = b"This is my url content" ++ responses.add(responses.GET, url, data) ++ response = oauth_helper.readurl(url, headers_cb=headers_cb) ++ request_headers = response._response.request.headers ++ assert "key1" in request_headers ++ assert "cb_key" in request_headers ++ assert request_headers["key1"] == "value1" ++ assert request_headers["cb_key"] == "cb_value" ++ ++ + class TestReadFileOrUrl: + def test_read_file_or_url_str_from_file(self, tmp_path: pathlib.Path): + """Test that str(result.contents) on file is text version of contents. diff --git a/debian/patches/cpick-8810a2dc-test-Remove-CiTestCase-from-test_url_helper.py b/debian/patches/cpick-8810a2dc-test-Remove-CiTestCase-from-test_url_helper.py new file mode 100644 index 00000000000..76bf6560361 --- /dev/null +++ b/debian/patches/cpick-8810a2dc-test-Remove-CiTestCase-from-test_url_helper.py @@ -0,0 +1,100 @@ +From 8810a2dccf8502549f2498a96ad7ff379fa93b87 Mon Sep 17 00:00:00 2001 +From: James Falcon +Date: Mon, 3 Mar 2025 08:40:54 -0600 +Subject: [PATCH] test: Remove CiTestCase from test_url_helper.py +Bug: https://github.com/canonical/cloud-init/issues/6065 +Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/cloud-init/+bug/2100963 + +--- + tests/unittests/test_url_helper.py | 32 ++++++++++++++---------------- + 1 file changed, 15 insertions(+), 17 deletions(-) + +--- a/tests/unittests/test_url_helper.py ++++ b/tests/unittests/test_url_helper.py +@@ -2,6 +2,7 @@ + # pylint: disable=attribute-defined-outside-init + + import logging ++import pathlib + from functools import partial + from threading import Event + from time import process_time +@@ -23,7 +24,7 @@ from cloudinit.url_helper import ( + readurl, + wait_for_url, + ) +-from tests.unittests.helpers import CiTestCase, mock, skipIf ++from tests.unittests.helpers import mock, skipIf + + try: + import oauthlib +@@ -37,7 +38,7 @@ except ImportError: + M_PATH = "cloudinit.url_helper." + + +-class TestOAuthHeaders(CiTestCase): ++class TestOAuthHeaders: + def test_oauth_headers_raises_not_implemented_when_oathlib_missing(self): + """oauth_headers raises a NotImplemented error when oauth absent.""" + with mock.patch.dict("sys.modules", {"oauthlib": None}): +@@ -67,17 +68,14 @@ class TestOAuthHeaders(CiTestCase): + assert "url" == return_value + + +-class TestReadFileOrUrl(CiTestCase): +- +- with_logs = True +- +- def test_read_file_or_url_str_from_file(self): ++class TestReadFileOrUrl: ++ def test_read_file_or_url_str_from_file(self, tmp_path: pathlib.Path): + """Test that str(result.contents) on file is text version of contents. + It should not be "b'data'", but just "'data'" """ +- tmpf = self.tmp_path("myfile1") ++ tmpf = tmp_path / "myfile1" + data = b"This is my file content\n" + util.write_file(tmpf, data, omode="wb") +- result = read_file_or_url("file://%s" % tmpf) ++ result = read_file_or_url(f"file://{tmpf}") + assert result.contents == data + assert str(result) == data.decode("utf-8") + +@@ -105,7 +103,9 @@ class TestReadFileOrUrl(CiTestCase): + assert str(result) == data.decode("utf-8") + + @responses.activate +- def test_read_file_or_url_str_from_url_redacting_headers_from_logs(self): ++ def test_read_file_or_url_str_from_url_redacting_headers_from_logs( ++ self, caplog ++ ): + """Headers are redacted from logs but unredacted in requests.""" + url = "http://hostname/path" + headers = {"sensitive": "sekret", "server": "blah"} +@@ -118,12 +118,11 @@ class TestReadFileOrUrl(CiTestCase): + responses.add_callback(responses.GET, url, callback=_request_callback) + + read_file_or_url(url, headers=headers, headers_redact=["sensitive"]) +- logs = self.logs.getvalue() +- assert REDACTED in logs +- assert "sekret" not in logs ++ assert REDACTED in caplog.text ++ assert "sekret" not in caplog.text + + @responses.activate +- def test_read_file_or_url_str_from_url_redacts_noheaders(self): ++ def test_read_file_or_url_str_from_url_redacts_noheaders(self, caplog): + """When no headers_redact, header values are in logs and requests.""" + url = "http://hostname/path" + headers = {"sensitive": "sekret", "server": "blah"} +@@ -136,9 +135,8 @@ class TestReadFileOrUrl(CiTestCase): + responses.add_callback(responses.GET, url, callback=_request_callback) + + read_file_or_url(url, headers=headers) +- logs = self.logs.getvalue() +- assert REDACTED not in logs +- assert "sekret" in logs ++ assert REDACTED not in caplog.text ++ assert "sekret" in caplog.text + + def test_wb_read_url_defaults_honored_by_read_file_or_url_callers(self): + """Readurl param defaults used when unspecified by read_file_or_url diff --git a/debian/patches/cpick-9311e066-fix-Update-OauthUrlHelper-to-use-readurl-exception_cb b/debian/patches/cpick-9311e066-fix-Update-OauthUrlHelper-to-use-readurl-exception_cb new file mode 100644 index 00000000000..e4c83e84a19 --- /dev/null +++ b/debian/patches/cpick-9311e066-fix-Update-OauthUrlHelper-to-use-readurl-exception_cb @@ -0,0 +1,41 @@ +From 9311e066f1eafea68fc714183173d8a5cc197d73 Mon Sep 17 00:00:00 2001 +From: James Falcon +Date: Mon, 3 Mar 2025 13:27:50 -0600 +Subject: [PATCH] fix: Update OauthUrlHelper to use readurl exception_cb + signature + +Fixes GH-6065 +Bug: https://github.com/canonical/cloud-init/issues/6065 +Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/cloud-init/+bug/2100963 +--- + cloudinit/url_helper.py | 8 ++++---- + 1 file changed, 4 insertions(+), 4 deletions(-) + +--- a/cloudinit/url_helper.py ++++ b/cloudinit/url_helper.py +@@ -1035,7 +1035,7 @@ class OauthUrlHelper: + ) as fp: + fp.write(json.dumps(cur)) + +- def exception_cb(self, msg, exception): ++ def exception_cb(self, exception): + if not ( + isinstance(exception, UrlError) + and (exception.code == 403 or exception.code == 401) +@@ -1096,13 +1096,13 @@ class OauthUrlHelper: + def readurl(self, *args, **kwargs): + return self._wrapped(readurl, args, kwargs) + +- def _exception_cb(self, extra_exception_cb, msg, exception): ++ def _exception_cb(self, extra_exception_cb, exception): + ret = None + try: + if extra_exception_cb: +- ret = extra_exception_cb(msg, exception) ++ ret = extra_exception_cb(exception) + finally: +- self.exception_cb(msg, exception) ++ self.exception_cb(exception) + return ret + + def _headers_cb(self, extra_headers_cb, url): diff --git a/debian/patches/cpick-c60771d8-test-pytestify-test_url_helper.py b/debian/patches/cpick-c60771d8-test-pytestify-test_url_helper.py new file mode 100644 index 00000000000..03fca450e9f --- /dev/null +++ b/debian/patches/cpick-c60771d8-test-pytestify-test_url_helper.py @@ -0,0 +1,153 @@ +From c60771d8ef005154bacd5beb740949a7a830aeb1 Mon Sep 17 00:00:00 2001 +From: James Falcon +Date: Mon, 3 Mar 2025 08:33:41 -0600 +Subject: [PATCH] test: pytestify test_url_helper.py +Bug: https://github.com/canonical/cloud-init/issues/6065 +Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/cloud-init/+bug/2100963 + +--- + tests/unittests/test_url_helper.py | 59 ++++++++++++++---------------- + 1 file changed, 27 insertions(+), 32 deletions(-) + +--- a/tests/unittests/test_url_helper.py ++++ b/tests/unittests/test_url_helper.py +@@ -41,11 +41,9 @@ class TestOAuthHeaders(CiTestCase): + def test_oauth_headers_raises_not_implemented_when_oathlib_missing(self): + """oauth_headers raises a NotImplemented error when oauth absent.""" + with mock.patch.dict("sys.modules", {"oauthlib": None}): +- with self.assertRaises(NotImplementedError) as context_manager: ++ with pytest.raises(NotImplementedError) as context_manager: + oauth_headers(1, 2, 3, 4, 5) +- self.assertEqual( +- "oauth support is not available", str(context_manager.exception) +- ) ++ assert "oauth support is not available" == str(context_manager.value) + + @skipIf(_missing_oauthlib_dep, "No python-oauthlib dependency") + @mock.patch("oauthlib.oauth1.Client") +@@ -66,7 +64,7 @@ class TestOAuthHeaders(CiTestCase): + "token_secret", + "consumer_secret", + ) +- self.assertEqual("url", return_value) ++ assert "url" == return_value + + + class TestReadFileOrUrl(CiTestCase): +@@ -80,8 +78,8 @@ class TestReadFileOrUrl(CiTestCase): + data = b"This is my file content\n" + util.write_file(tmpf, data, omode="wb") + result = read_file_or_url("file://%s" % tmpf) +- self.assertEqual(result.contents, data) +- self.assertEqual(str(result), data.decode("utf-8")) ++ assert result.contents == data ++ assert str(result) == data.decode("utf-8") + + @responses.activate + def test_read_file_or_url_str_from_url(self): +@@ -91,8 +89,8 @@ class TestReadFileOrUrl(CiTestCase): + data = b"This is my url content\n" + responses.add(responses.GET, url, data) + result = read_file_or_url(url) +- self.assertEqual(result.contents, data) +- self.assertEqual(str(result), data.decode("utf-8")) ++ assert result.contents == data ++ assert str(result) == data.decode("utf-8") + + @responses.activate + def test_read_file_or_url_str_from_url_streamed(self): +@@ -103,8 +101,8 @@ class TestReadFileOrUrl(CiTestCase): + responses.add(responses.GET, url, data) + result = read_file_or_url(url, stream=True) + assert isinstance(result, UrlResponse) +- self.assertEqual(result.contents, data) +- self.assertEqual(str(result), data.decode("utf-8")) ++ assert result.contents == data ++ assert str(result) == data.decode("utf-8") + + @responses.activate + def test_read_file_or_url_str_from_url_redacting_headers_from_logs(self): +@@ -114,15 +112,15 @@ class TestReadFileOrUrl(CiTestCase): + + def _request_callback(request): + for k in headers.keys(): +- self.assertEqual(headers[k], request.headers[k]) ++ assert headers[k] == request.headers[k] + return (200, request.headers, "does_not_matter") + + responses.add_callback(responses.GET, url, callback=_request_callback) + + read_file_or_url(url, headers=headers, headers_redact=["sensitive"]) + logs = self.logs.getvalue() +- self.assertIn(REDACTED, logs) +- self.assertNotIn("sekret", logs) ++ assert REDACTED in logs ++ assert "sekret" not in logs + + @responses.activate + def test_read_file_or_url_str_from_url_redacts_noheaders(self): +@@ -132,15 +130,15 @@ class TestReadFileOrUrl(CiTestCase): + + def _request_callback(request): + for k in headers.keys(): +- self.assertEqual(headers[k], request.headers[k]) ++ assert headers[k] == request.headers[k] + return (200, request.headers, "does_not_matter") + + responses.add_callback(responses.GET, url, callback=_request_callback) + + read_file_or_url(url, headers=headers) + logs = self.logs.getvalue() +- self.assertNotIn(REDACTED, logs) +- self.assertIn("sekret", logs) ++ assert REDACTED not in logs ++ assert "sekret" in logs + + def test_wb_read_url_defaults_honored_by_read_file_or_url_callers(self): + """Readurl param defaults used when unspecified by read_file_or_url +@@ -161,19 +159,16 @@ class TestReadFileOrUrl(CiTestCase): + class FakeSession(requests.Session): + @classmethod + def request(cls, **kwargs): +- self.assertEqual( +- { +- "url": url, +- "allow_redirects": True, +- "method": "GET", +- "headers": { +- "User-Agent": "Cloud-Init/%s" +- % (version.version_string()) +- }, +- "stream": False, ++ assert { ++ "url": url, ++ "allow_redirects": True, ++ "method": "GET", ++ "headers": { ++ "User-Agent": "Cloud-Init/%s" ++ % (version.version_string()) + }, +- kwargs, +- ) ++ "stream": False, ++ } == kwargs + return m_response + + with mock.patch(M_PATH + "requests.Session") as m_session: +@@ -182,13 +177,13 @@ class TestReadFileOrUrl(CiTestCase): + FakeSession(), + ] + # assert no retries and check_status == True +- with self.assertRaises(UrlError) as context_manager: ++ with pytest.raises(UrlError) as context_manager: + response = read_file_or_url(url) +- self.assertEqual("broke", str(context_manager.exception)) ++ assert "broke" == str(context_manager.value) + # assert default headers, method, url and allow_redirects True + # Success on 2nd call with FakeSession + response = read_file_or_url(url) +- self.assertEqual(m_response, response._response) ++ assert m_response == response._response + + + class TestReadFileOrUrlParameters: diff --git a/debian/patches/series b/debian/patches/series index 09db6bbf331..4fb6fca6737 100644 --- a/debian/patches/series +++ b/debian/patches/series @@ -5,3 +5,7 @@ grub-dpkg-support.patch cpick-84806336-chore-Add-feature-flag-for-manual-network-waiting no-remove-networkd-online.patch cpick-d75840be-fix-retry-AWS-hotplug-for-async-IMDS-5995 +cpick-c60771d8-test-pytestify-test_url_helper.py +cpick-8810a2dc-test-Remove-CiTestCase-from-test_url_helper.py +cpick-582f16c1-test-add-OauthUrlHelper-tests +cpick-9311e066-fix-Update-OauthUrlHelper-to-use-readurl-exception_cb