Skip to content

fix(azure): catch import error as reportable #6714

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

Open
cadejacobson wants to merge 15 commits into
base: main
Choose a base branch
from
Open

fix(azure): catch import error as reportable #6714

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
fc50fa1
Create import error class
cadejacobson Oct 21, 2025
e5accaf
Update to use error instead of exception
cadejacobson Dec 2, 2025
bec4bee
Use a local import error
cadejacobson Feb 4, 2026
daf89ef
Log the library name
cadejacobson Feb 5, 2026
2224a9a
Create unit tests for import error
cadejacobson Feb 5, 2026
1f8f9e0
Correct unit test
cadejacobson Feb 5, 2026
279e368
Correct spacing
cadejacobson Feb 5, 2026
ce06678
Ignore import not found
cadejacobson Feb 5, 2026
1f9399b
Reorder ignore comments
cadejacobson Feb 5, 2026
d68ba44
Add isort skip
cadejacobson Feb 5, 2026
41d655f
Run black
cadejacobson Feb 5, 2026
26eb69f
Remove unnecessary _import_error
cadejacobson Feb 5, 2026
f9c9e20
Move imports for password hashing
cadejacobson Feb 10, 2026
a7d4945
Fix testcases
cadejacobson Feb 12, 2026
129cc0f
Merge branch 'main' into cade/catch-import-error
cadejacobson Feb 12, 2026
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
6 changes: 2 additions & 4 deletions cloudinit/sources/DataSourceAzure.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,16 +60,14 @@
import passlib.hash

blowfish_hash = passlib.hash.sha512_crypt.hash
except ImportError:
except ImportError as error:

def blowfish_hash(_):
"""Raise when called so that importing this module doesn't throw
ImportError when ds_detect() returns false. In this case, crypt
and passlib are not needed.
"""
raise ImportError(
"crypt and passlib not found, missing dependency"
)
raise errors.ReportableErrorImportError(error=error)


LOG = logging.getLogger(__name__)
Expand Down
7 changes: 7 additions & 0 deletions cloudinit/sources/azure/errors.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,13 @@ def __init__(self, *, exception: ValueError) -> None:
self.supporting_data["exception"] = repr(exception)


class ReportableErrorImportError(ReportableError):
def __init__(self, *, error: ImportError) -> None:
super().__init__(f"error importing {error.name} library")

self.supporting_data["error"] = repr(error)


class ReportableErrorOsDiskPpsFailure(ReportableError):
def __init__(self) -> None:
super().__init__("error waiting for host shutdown")
Expand Down
9 changes: 9 additions & 0 deletions tests/unittests/sources/azure/test_errors.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,15 @@ def test_imds_metadata_parsing_exception():
assert error.supporting_data["exception"] == repr(exception)


def test_import_error():
exception = ImportError("No module named 'foobar'", name="foobar")
Copy link
Contributor

@cjp256 cjp256 Feb 9, 2026

Choose a reason for hiding this comment

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

if you want a real error to test against you can go ahead and simulate error with a real import error

try:
  import ModuleThatDoesNotExist
except Exception as error:
  ...

That way it's easier to assume that the ImportError is instantiated like the running environment will.


error = errors.ReportableErrorImportError(error=exception)

assert error.reason == "error importing foobar library"
assert error.supporting_data["error"] == repr(exception)


def test_ovf_parsing_exception():
error = None
try:
Expand Down
13 changes: 13 additions & 0 deletions tests/unittests/sources/test_azure.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2434,6 +2434,19 @@ def test_wb_invalid_ovf_env_xml_calls_read_azure_ovf(self, tmp_path):
== cm.value.reason
)

def test_import_error_from_failed_import(self):
"""Attempt to import a module that is not present"""
Copy link
Contributor

@cjp256 cjp256 Feb 9, 2026

Choose a reason for hiding this comment

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

what we really want here is coverage for encrypt_pass() on the import failure
there's some basic coverage in TestDependencyFallback to ensure it works with one or the other, but lacks a test covering the case where neither exists.

I think the easiest way to test this with a mock is to pull all of that import logic into blowfish_hash() instead of being top-level, and force the import behavior you want with a side effect.

Something like:

import builtins
import pytest


def blowfish_hash(password: str) -> str:
    from passlib.hash import sha512_crypt

    return sha512_crypt.hash(password)


class TestBlowfishHash:
    def test_success(self, monkeypatch):
        class FakeSha512Crypt:
            @staticmethod
            def hash(pw: str) -> str:
                return f"fake-sha512:{pw}"

        real_import = builtins.__import__

        def fake_import(name, globals=None, locals=None, fromlist=(), level=0):
            # Intercept: from passlib.hash import sha512_crypt
            if name == "passlib.hash" and fromlist and "sha512_crypt" in fromlist:
                class FakePasslibHashModule:
                    sha512_crypt = FakeSha512Crypt

                return FakePasslibHashModule()

            return real_import(name, globals, locals, fromlist, level)

        monkeypatch.setattr(builtins, "__import__", fake_import)

        assert blowfish_hash("secret") == "fake-sha512:secret"

    def test_importerror(self, monkeypatch):
        real_import = builtins.__import__

        def fake_import(name, globals=None, locals=None, fromlist=(), level=0):
            if name == "passlib.hash" and fromlist and "sha512_crypt" in fromlist:
                raise ImportError("passlib is not installed")
            return real_import(name, globals, locals, fromlist, level)

        monkeypatch.setattr(builtins, "__import__", fake_import)

        with pytest.raises(ImportError, match="passlib"):
            blowfish_hash("secret")

try:
import nonexistent_module_that_will_never_exist # type: ignore[import-not-found] # noqa: F401 # isort:skip
Copy link
Contributor

@cjp256 cjp256 Feb 9, 2026

Choose a reason for hiding this comment

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

ah I see you did that here

except ImportError as error:
reportable_error = errors.ReportableErrorImportError(error=error)

assert (
reportable_error.reason == "error importing "
"nonexistent_module_that_will_never_exist library"
)
assert reportable_error.supporting_data["error"] == repr(error)


class TestReadAzureOvf:
def test_invalid_xml_raises_non_azure_ds(self):
Expand Down
Loading