Fix proxy in Qt6 kiosk browser #168
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The phrasing implies that the proxy has been set, but it is only about to be set. Inform of the successful action only.
guyonvarch
reviewed
Jun 13, 2024
testing/integration/kiosk-proxy.nix
Outdated
Comment on lines
132
to
138
| # Ideally here we would check if the request actually arrived at thecloud. | ||
| # Unfortunately the proxy is currently not contacted by the kiosk in this | ||
| # test setup, even though if we look we see that the proxy is set as | ||
| # application proxy. This issue seems to be specific to the test setup, if | ||
| # we test in a real installation, the configured proxy is actually used. | ||
| # So for now we simply test whether the proxy has been picked up and | ||
| # configured. |
There was a problem hiding this comment.
I ran the test with only theclient, and removing thecloud and theproxy, and it passes. I wonder if we should temporarily comment all that’s not theclient, to prevent givint the idea that it’s currently tested?
guyonvarch
added
details needed
|
Working well using configuring a local proxy, and updating connman conf with the proxy services = {
tinyproxy = {
enable = true;
settings = {
Port = 1234;
Listen = "127.0.0.1";
Timeout = 600;
BasicAuth = "user *********";
};
};
};And setting the wrong password in connman conf makes the kiosk to fail loading the page. |
This is the humble beginnings of an automated integration test for proxy use in the kiosk. At the moment the test is rather limited: It only checks that a proxy configured with connman is picked up and set as application proxy by the kiosk. Unfortunately I have not been able to get the proxy to be actually used by the Qt framework in this test setup. It rather seems like the proxy is properly configured from our side. It would be nice to extend the tests to confirm that requests with the proxy can successfully be made, but this may require analysis of the Qt framework in the context of the VM test setup. I am including two sanity checks that the basic node topology is as expected (no direct route from client to 'the cloud'), and that the proxy node basically works (with curl).
After the switch to Qt6 (dividat#156), proxies were left broken due to a renaming in QNetworkProxy (which is not properly documented in https://doc.qt.io/qtforpython-6/PySide6/QtNetwork/QNetworkProxy.html#PySide6.QtNetwork.QNetworkProxy.ProxyType). Rename to the enum scope to avoid a runtime error when trying to set proxy.
These sanity checks are good for running an actual test with the kiosk contacting another host through a proxy, but as I've been unable to get this test scenario to work, remove them for now.
|
I thought I was close many, many times, but finally giving up on a proper multi-VLAN test for now. Removed the remnants of the multi-VLAN setup in d200877 |
knuton
added
reviewable
guyonvarch
approved these changes
Jun 27, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
After the switch to Qt6 (#156), proxies were left broken due to a renaming in QNetworkProxy (which is not properly documented in https://doc.qt.io/qtforpython-6/PySide6/QtNetwork/QNetworkProxy.html#PySide6.QtNetwork.QNetworkProxy.ProxyType).
I added proxy functionality to the pre-release test protocol. But I actually would like to lower that burden generally, and would have liked the issue to surface sooner. So I tried to write a VM-based integration test. It almost works, but in the end I am only submitting it as a smoke test.
You can run the kiosk without 02207d1 to see that it crashes if a proxy is configured. The new test would catch that, and in general would fail if the kiosk does not at least claim to have set the proxy on the framework.
I would like to extend the test going forward, but this seems like a good first step. It would have immediately caught what this PR fixes.
Checklist