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

tests: os: connectivity: test redsocks config via supervisor api #3488

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rcooke-warwick
Copy link
Contributor

@rcooke-warwick rcooke-warwick commented Aug 8, 2024

A regression was introduced with supervisor 16.4.0 that broke proxy configuration via the supervisor API, when using the noProxy parameter. This commit adds a test to configure via the supervisor API, hoping to catch issues such as these

Change-type: patch

This test passes on 5.3.15 locally (before supervisor 16.4.0 , and fails on 5.4.0 (after supervisor 16.4.0) due to the redsocks.conf incorrectly having a noProxy field added to it, resulting in this:

Aug 08 13:27:59 8f94e56 redsocks[5915]: file parsing error at line 13: unexpected char
Aug 08 13:27:59 8f94e56 redsocks[5915]: file parsing error at line 13: unclosed section
Aug 08 13:27:59 8f94e56 redsocks[5915]: file parsing error at line 13: stale key_token
Aug 08 13:27:59 8f94e56 redsocks[5915]: file parsing error at line 13: stale value_token
Aug 08 13:27:59 8f94e56 systemd[1]: redsocks.service: Main process exited, code=exited, status=1/FAILURE
Aug 08 13:27:59 8f94e56 systemd[1]: redsocks.service: Failed with result 'exit-code'. 

Contributor checklist

Reviewer Guidelines

  • When submitting a review, please pick:
    • 'Approve' if this change would be acceptable in the codebase (even if there are minor or cosmetic tweaks that could be improved).
    • 'Request Changes' if this change would not be acceptable in our codebase (e.g. bugs, changes that will make development harder in future, security/performance issues, etc).
    • 'Comment' if you don't feel you have enough information to decide either way (e.g. if you have major questions, or you don't understand the context of the change sufficiently to fully review yourself, but want to make a comment)

Copy link

github-actions bot commented Aug 8, 2024

Website deployed to CF Pages, 👀 preview link https://fc6e7d49.balena-os.pages.dev

@pipex
Copy link
Contributor

pipex commented Aug 8, 2024

@rcooke-warwick the commit/PR body mention a noHup configuration when I think you mean noProxy

A regression was introduced with supervisor 16.4.0 that broke proxy configuration via the supervisor API, when using the noProxy parameter. This commit adds a test to configure via the supervisor API, hoping to catch issues such as these

Change-type: patch
Signed-off-by: Ryan Cooke <[email protected]>
@rcooke-warwick rcooke-warwick force-pushed the ryan/supervisor-redsocks branch from c0b1b9a to 4df44ea Compare October 3, 2024 14:40
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.

2 participants