Skip to content

refactor: Improve error reporting on sudo execution error#3802

Merged
didier-wenzek merged 1 commit intothin-edge:mainfrom
didier-wenzek:refactor/sudo-error-reporting
Oct 7, 2025
Merged

refactor: Improve error reporting on sudo execution error#3802
didier-wenzek merged 1 commit intothin-edge:mainfrom
didier-wenzek:refactor/sudo-error-reporting

Conversation

@didier-wenzek
Copy link
Contributor

@didier-wenzek didier-wenzek commented Oct 2, 2025

Proposed changes

Improve error reporting when:

  • a command has not been configured to be executable by tedge with sudo
  • a sm plugin has not been properly configured to be used with sudo
  • a log plugin has not been properly configured to be used with sudo

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Improvement (general improvements like code refactoring that doesn't explicitly fix a bug or add any new functionality)
  • Documentation Update (if none of the other choices apply)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Paste Link to the issue


Checklist

  • I have read the CONTRIBUTING doc
  • I have signed the CLA (in all commits with git commit -s. You can activate automatic signing by running just prepare-dev once)
  • I ran just format as mentioned in CODING_GUIDELINES
  • I used just check as mentioned in CODING_GUIDELINES
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

Further comments

@codecov
Copy link

codecov bot commented Oct 2, 2025

Codecov Report

❌ Patch coverage is 28.57143% with 50 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
crates/common/tedge_config/src/sudo.rs 33.33% 30 Missing ⚠️
crates/core/plugin_sm/src/plugin_manager.rs 30.76% 6 Missing and 3 partials ⚠️
...extensions/tedge_log_manager/src/plugin_manager.rs 10.00% 9 Missing ⚠️
crates/core/plugin_sm/src/plugin.rs 0.00% 2 Missing ⚠️

📢 Thoughts on this report? Let us know!

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

@reubenmiller reubenmiller added the theme:plugins Theme: Plugin interfaces or general plugin topics which don't fit in any other themes label Oct 2, 2025
@didier-wenzek didier-wenzek force-pushed the refactor/sudo-error-reporting branch from 7d02194 to 8d6da2a Compare October 2, 2025 14:17
@github-actions
Copy link
Contributor

github-actions bot commented Oct 2, 2025

Robot Results

✅ Passed ❌ Failed ⏭️ Skipped Total Pass % ⏱️ Duration
710 0 3 710 100 2h9m26.69505s

@reubenmiller reubenmiller self-requested a review October 6, 2025 09:04
@reubenmiller
Copy link
Contributor

@didier-wenzek I've been trying to run the "Overriding a log plugin" system test locally and removing the sudoers rule to see the behaviour, but I'm not seeing any log output indicating a sudoer rule issue. I only see the following error (which is similar to what we had before):

Oct 06 12:04:17 0529f55f43d7 tedge-agent[969]: 2025-10-06T12:04:17.538831466Z ERROR log plugins: Skipping /usr/local/tedge/log-plugins/fake_plugin: does not support list operation and may not be a valid plugin

@didier-wenzek didier-wenzek requested a review from a team as a code owner October 6, 2025 14:28
@didier-wenzek
Copy link
Contributor Author

@didier-wenzek I've been trying to run the "Overriding a log plugin" system test locally and removing the sudoers rule to see the behaviour, but I'm not seeing any log output indicating a sudoer rule issue. I only see the following error (which is similar to what we had before):

Oct 06 12:04:17 0529f55f43d7 tedge-agent[969]: 2025-10-06T12:04:17.538831466Z ERROR log plugins: Skipping /usr/local/tedge/log-plugins/fake_plugin: does not support list operation and may not be a valid plugin

My bad. A check was inverted. Fixed + a new system test: c9a5ad7

reubenmiller
reubenmiller previously approved these changes Oct 7, 2025
Copy link
Contributor

@reubenmiller reubenmiller left a comment

Choose a reason for hiding this comment

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

Approved. Nice improvement

@reubenmiller reubenmiller dismissed their stale review October 7, 2025 07:26

Sorry I forgot I still had an open test change

Copy link
Contributor

@reubenmiller reubenmiller left a comment

Choose a reason for hiding this comment

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

Approved

Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>
@didier-wenzek didier-wenzek force-pushed the refactor/sudo-error-reporting branch from ff494f4 to f5893f7 Compare October 7, 2025 10:46
@didier-wenzek didier-wenzek added this pull request to the merge queue Oct 7, 2025
Merged via the queue into thin-edge:main with commit 7552c77 Oct 7, 2025
34 checks passed
@didier-wenzek didier-wenzek deleted the refactor/sudo-error-reporting branch October 7, 2025 12:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

theme:plugins Theme: Plugin interfaces or general plugin topics which don't fit in any other themes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants