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

Add option to call external command on port change. #139

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

Conversation

marcopaganini
Copy link

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.

@g00nix
Copy link
Contributor

g00nix commented Feb 19, 2022

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:

  1. there are no comments explaining why this is necesary
  2. 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?

@marcopaganini
Copy link
Author

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:

  1. 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?

@orangepeelbeef
Copy link

orangepeelbeef commented Mar 1, 2022 via email

@marcopaganini
Copy link
Author

marcopaganini commented Aug 14, 2022 via email

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.
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.

None yet

3 participants