Skip to content

Fix an issue where the ESXiPlugin shows a confusing warning when constructing the local fs#1332

Merged
Miauwkeru merged 7 commits intomainfrom
esxi-logging-inconsistency
Mar 18, 2026
Merged

Fix an issue where the ESXiPlugin shows a confusing warning when constructing the local fs#1332
Miauwkeru merged 7 commits intomainfrom
esxi-logging-inconsistency

Conversation

@Miauwkeru
Copy link
Contributor

The following warning was logged every time that the local_tgz tar file
was successfully decrypted

"local.tgz is encrypted but static decryption failed and no dynamic
decryption available!"

Now this warning is moved so it only shows when it isn't run on a local
target.

…tructing the local fs

The following warning was logged every time that the local_tgz tar file
was successfully decrypted

> "local.tgz is encrypted but static decryption failed and no dynamic
decryption available!"

Now this warning is moved so it only shows when it isn't run on a local
target.
@codecov
Copy link

codecov bot commented Sep 19, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.06%. Comparing base (cb26a4e) to head (c317494).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1332      +/-   ##
==========================================
- Coverage   81.11%   81.06%   -0.05%     
==========================================
  Files         402      402              
  Lines       35244    35244              
==========================================
- Hits        28587    28571      -16     
- Misses       6657     6673      +16     
Flag Coverage Δ
unittests 81.06% <100.00%> (-0.05%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Schamper
Copy link
Member

What's the status of this?

@Miauwkeru
Copy link
Contributor Author

What's the status of this?

It needs a review. I made it when I noticed something off during debugging, but it was never picked up afterwards

@william-billaud
Copy link
Contributor

Fyi I found the same issue while working on #1509 .
Fix lgtm, as it was mostly a logging error.

william-billaud added a commit to william-billaud/dissect.target that referenced this pull request Mar 16, 2026
Copy link
Member

@Schamper Schamper left a comment

Choose a reason for hiding this comment

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

Can you add a test to ensure we don't mess this up again?

@codspeed-hq
Copy link

codspeed-hq bot commented Mar 17, 2026

Merging this PR will not alter performance

✅ 12 untouched benchmarks


Comparing esxi-logging-inconsistency (c317494) with main (cb26a4e)

Open in CodSpeed

@Miauwkeru
Copy link
Contributor Author

Can you add a test to ensure we don't mess this up again?

I adjusted the current tests for this behaviour to check for the log entry

@Miauwkeru Miauwkeru requested a review from Schamper March 17, 2026 09:50
with patch("dissect.target.plugins.os.unix.esxi._os.subprocess.run") as mocked_run:
mocked_run.return_value.stdout = b"data"

assert _decrypt_crypto_util(fs_unix.path("data")).read() == b"data"
Copy link
Member

Choose a reason for hiding this comment

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

Can you assert that the correct log lines show up here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

_detect_crypto_util doesn't log anything, did you mean the test below it? (test_esxi_os_detection)

Copy link
Member

Choose a reason for hiding this comment

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

Whatever the test is that tests it for the local 😄.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed the names for the tests a bit to reflect what they actually tests and assert the log messages test for:

test__create_local_fs_local_no_envelope

And add assertions for Log messages that occurred
Schamper
Schamper previously approved these changes Mar 17, 2026
@Miauwkeru
Copy link
Contributor Author

I notice there is probably a weird test dependency somewhere with caplog where it doesn't capture any logging for the esxi test. The seed can be reproduced (not only on windows) so I am looking into it.

@EinatFox EinatFox linked an issue Mar 18, 2026 that may be closed by this pull request
configure_logging gets called during the execution of the main of our
target tooling.
This causes some issues with capturing logging messages with caplog

This patch basically does the same as
tests/tools/conftest.py::prevent_logging_setup for this specific test
@Miauwkeru
Copy link
Contributor Author

@Schamper found the issue that broke the caplog for the test. This also gets rid of the deprecation warning we got from structlog during the test. I'll make a different pr to fix that directly.

@Miauwkeru Miauwkeru merged commit 2dfc393 into main Mar 18, 2026
21 of 24 checks passed
@Miauwkeru Miauwkeru deleted the esxi-logging-inconsistency branch March 18, 2026 12:07
@Miauwkeru
Copy link
Contributor Author

#1631 fixes the deprecation warning I mentioned

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fixing inconsistent logging in ESXi OS plugin

3 participants