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

Ubuntu/noble 24.4.x #6094

Merged
Merged
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
12 changes: 12 additions & 0 deletions debian/changelog
Original file line number Diff line number Diff line change
@@ -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 <[email protected]> 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
Expand Down
156 changes: 156 additions & 0 deletions debian/patches/cpick-582f16c1-test-add-OauthUrlHelper-tests
Original file line number Diff line number Diff line change
@@ -0,0 +1,156 @@
From 582f16c143b1d071c78f259bb978296b1439e186 Mon Sep 17 00:00:00 2001
From: James Falcon <[email protected]>
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.
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
From 8810a2dccf8502549f2498a96ad7ff379fa93b87 Mon Sep 17 00:00:00 2001
From: James Falcon <[email protected]>
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
From 9311e066f1eafea68fc714183173d8a5cc197d73 Mon Sep 17 00:00:00 2001
From: James Falcon <[email protected]>
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):
Loading
Loading