manual-connections
manual-connections copied to clipboard
pia-foss
Add option to call external command on port change.
Add support for the PF_POST_COMMAND environment variable in port_forwarding.sh. This will cause an external command to be called with the new forwarding port and the original forwarding port (as command-line arguments) every time the forwarding port changes.
Use cases:
- Update firewall configuration.
- Update configuration of anything that needs to bind to the correct port.
Sorry for not going through this since November. I find the idea very interesting and I personally like the idea a lot, however as I head a full read I realized two things:
- there are no comments explaining why this is necesary
- adding the comments and accepting the merge means we increase the complexity of the script
At this point I am inclining towards not accepting this merge, as the point of these scripts is to be easy to read/understand/modify. Do you think it would be a good idea if this stayed only as a fork?
Hello!
Yeah, every feature adds complexity. The trick as usual is to find the right feature set for your target audience :)
In my case, I needed to make sure that when the port changes my firewall was updated to allow incoming packets on that port and my daemon configuration was updated to listen on that particular port.
I don't know how to do it without this feature, but like you said, it may also be something that not all people need.
About adding to main branch vs separate branch, I see two possibilities:
-
Adding it to the main branch may make discovery easier for people who need the feature. There's the valid argument of adding complexity, but my impression is that people who don't want complexity opt for a more "standard" setup (chrome app, or GUI app).
-
Adding to a separate branch also works, but I don't know how we can prevent it from becoming stale, i.e. non-mergeable to the main branch if someone wants to use the feature.
WDYT?
the way i solved this was to write out to a file /var/port_forward and use inotifywait on that file to trigger the other changes necessary on other daemons.
https://github.com/orangepeelbeef/pia-proxybox/blob/master/run_scripts/deluge_setport.sh#L24
On Mon, Feb 21, 2022 at 9:05 PM Marco Paganini @.***> wrote:
Hello!
Yeah, every feature adds complexity. The trick as usual is to find the right feature set for your target audience :)
In my case, I needed to make sure that when the port changes my firewall was updated to allow incoming packets on that port and my daemon configuration was updated to listen on that particular port.
I don't know how to do it without this feature, but like you said, it may also be something that not all people need.
About adding to main branch vs separate branch, I see two possibilities:
Adding it to the main branch may make discovery easier for people who need the feature. There's the valid argument of adding complexity, but my impression is that people who don't want complexity opt for a more "standard" setup (chrome app, or GUI app). 2.
Adding to a separate branch also works, but I don't know how we can prevent it from becoming stale, i.e. non-mergeable to the main branch if someone wants to use the feature.
WDYT?
— Reply to this email directly, view it on GitHub https://github.com/pia-foss/manual-connections/pull/139#issuecomment-1047227525, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAA7ZA6WV4HIIUTVPC5PMZLU4KSJ7ANCNFSM5IOKBLOA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.
You are receiving this because you are subscribed to this thread.Message ID: @.***>
So, I could keep a fork of this somewhere. Is there a preferred method to do this?
On Tue, Mar 1, 2022 at 3:51 PM Orangepeelbeef @.***> wrote:
the way i solved this was to write out to a file /var/port_forward and use inotifywait on that file to trigger the other changes necessary on other daemons.
On Mon, Feb 21, 2022 at 9:05 PM Marco Paganini @.***> wrote:
Hello!
Yeah, every feature adds complexity. The trick as usual is to find the right feature set for your target audience :)
In my case, I needed to make sure that when the port changes my firewall was updated to allow incoming packets on that port and my daemon configuration was updated to listen on that particular port.
I don't know how to do it without this feature, but like you said, it may also be something that not all people need.
About adding to main branch vs separate branch, I see two possibilities:
Adding it to the main branch may make discovery easier for people who need the feature. There's the valid argument of adding complexity, but my impression is that people who don't want complexity opt for a more "standard" setup (chrome app, or GUI app). 2.
Adding to a separate branch also works, but I don't know how we can prevent it from becoming stale, i.e. non-mergeable to the main branch if someone wants to use the feature.
WDYT?
— Reply to this email directly, view it on GitHub < https://github.com/pia-foss/manual-connections/pull/139#issuecomment-1047227525 , or unsubscribe < https://github.com/notifications/unsubscribe-auth/AAA7ZA6WV4HIIUTVPC5PMZLU4KSJ7ANCNFSM5IOKBLOA
. Triage notifications on the go with GitHub Mobile for iOS < https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675
or Android < https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub .
You are receiving this because you are subscribed to this thread.Message ID: @.***>
— Reply to this email directly, view it on GitHub https://github.com/pia-foss/manual-connections/pull/139#issuecomment-1055983957, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABYUYBHYLJW5ZOV55L3BTSTU52UO3ANCNFSM5IOKBLOA . You are receiving this because you authored the thread.Message ID: @.***>