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

Send ports forwarded to control server #2392

Open
wants to merge 17 commits into
base: master
Choose a base branch
from

Conversation

jagaimoworks
Copy link

@jagaimoworks jagaimoworks commented Aug 3, 2024

First timer here. This is a somewhat working implementation of #2369. Hit me with the improvements I can take it 😅

I say somewhat working because the removal of ports from the firewall suffers from #2334 and therefore does not reliably work right now.

The way it works right now is by sending a http PUT request with a body like {ports: [1234, 3456]} to /v1/openvpn/portforwarded.

Copy link
Owner

@qdm12 qdm12 left a comment

Choose a reason for hiding this comment

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

That's great, thanks for the PR 💯 !
I will wait to fix the iptables removals (to create less user frustration and duplicate issues) after v3.39.0 gets released, to merge this though.

internal/portforward/loop.go Outdated Show resolved Hide resolved
internal/portforward/loop.go Outdated Show resolved Hide resolved
internal/portforward/service/service.go Outdated Show resolved Hide resolved
internal/portforward/service/service.go Outdated Show resolved Hide resolved
internal/portforward/service/service.go Outdated Show resolved Hide resolved
internal/server/interfaces.go Outdated Show resolved Hide resolved
internal/server/openvpn.go Outdated Show resolved Hide resolved
internal/server/openvpn.go Outdated Show resolved Hide resolved
internal/server/openvpn.go Outdated Show resolved Hide resolved
@qdm12 qdm12 added Status: 🔴 Blocked Blocked by another issue or pull request Status: 🔒 After next release Will be done after the next release labels Aug 3, 2024
@qdm12 qdm12 removed the Status: 🔒 After next release Will be done after the next release label Aug 9, 2024
@qdm12
Copy link
Owner

qdm12 commented Aug 9, 2024

(Sort of) blocked by #1785

@qdm12 qdm12 added Status: 🔴 Blocked Blocked by another issue or pull request Status: 🟡 Nearly resolved This might be resolved or is about to be resolved and removed Status: 🔴 Blocked Blocked by another issue or pull request labels Aug 17, 2024
@qdm12
Copy link
Owner

qdm12 commented Aug 23, 2024

Blocked by #2238 as well.

@andy3469
Copy link

andy3469 commented Nov 9, 2024

Hello!
Any news on this PR ?

internal/portforward/service/service.go Outdated Show resolved Hide resolved
internal/portforward/service/service.go Outdated Show resolved Hide resolved
internal/portforward/service/service.go Outdated Show resolved Hide resolved
internal/server/openvpn.go Outdated Show resolved Hide resolved
internal/server/openvpn.go Outdated Show resolved Hide resolved
internal/portforward/loop.go Outdated Show resolved Hide resolved
internal/portforward/loop.go Outdated Show resolved Hide resolved
@qdm12
Copy link
Owner

qdm12 commented Nov 22, 2024

@jagaimoworks By the way:

  • Great work! ❤️
  • Sorry for the delay re-reviewing this 🕐
  • Just a few minor comments ✅

@qdm12
Copy link
Owner

qdm12 commented Nov 22, 2024

And @andy3469 I'm curious, what do you plan to use this PR for 😃?

@andy3469
Copy link

And @andy3469 I'm curious, what do you plan to use this PR for 😃?

I plan to use it with https://github.com/dhruvinsh/ws-ephemeral as a way to update the port every week.
I will do a PR on the other project once this one is up and running 😄

@qdm12 qdm12 force-pushed the send-ports-forwarded-to-control-server branch from e9aaa97 to 19a007f Compare December 27, 2024 21:07
@qdm12
Copy link
Owner

qdm12 commented Dec 27, 2024

@jagaimoworks All done 😉 I rebased your branch on the master branch, pushed a few commits to simplify and re-use the same setup and teardown code for both the normal service operation and this patch request. One last thing I would like to ask you (or @andy3469 ?), is to PR to https://github.com/qdm12/gluetun-wiki a section explaining how this works, why etc. in https://github.com/qdm12/gluetun-wiki/blob/main/setup/advanced/control-server.md and perhaps link that section in https://github.com/qdm12/gluetun-wiki/blob/main/setup/advanced/vpn-port-forwarding.md as well 😉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: 🔴 Blocked Blocked by another issue or pull request Status: 🟡 Nearly resolved This might be resolved or is about to be resolved
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants