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

[WIP] Finalize firewalld port forwarding support #885

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

Conversation

mheon
Copy link
Member

@mheon mheon commented Jan 15, 2024

There are two major changes here.

Firstly, this adds proper support for port forwarding from localhost via a new policy accepting traffic from HOST. This is the last bit we were missing from the original port-forwarding implementation.

Secondly, this fixes a bug where we generated incorrect rules when port-forwarding from a single IP. Instead of doing standard port-forwarding rules, those need rich rules. This was reported as #881.

There are also some small code cleanups in how we handle setting up and tearing down port forwarding. It's still rather ugly, but at least a little better than it was before.

Fixes #881

Copy link
Contributor

openshift-ci bot commented Jan 15, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mheon

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@mheon
Copy link
Member Author

mheon commented Jan 15, 2024

Tagging WIP as I'm still working on some bugs, seemingly related to firewalld 2.0

@mheon mheon force-pushed the firewalld_final branch 2 times, most recently from d137e9b to 0d30930 Compare January 15, 2024 17:28
@mheon
Copy link
Member Author

mheon commented Jan 15, 2024

Hm. Tests aren't even running firewalld, despite fw_driver=firewalld being set.

There are two major changes here.

Firstly, this adds proper support for port forwarding from
localhost via a new policy accepting traffic from HOST. This is
the last bit we were missing from the original port-forwarding
implementation.

Secondly, this fixes a bug where we generated incorrect rules
when port-forwarding from a single IP. Instead of doing standard
port-forwarding rules, those need rich rules. This was reported
as containers#881.

There are also some small code cleanups in how we handle setting
up and tearing down port forwarding. It's still rather ugly, but
at least a little better than it was before.

Fixes containers#881

Signed-off-by: Matthew Heon <[email protected]>
@mheon
Copy link
Member Author

mheon commented Feb 15, 2024

Localhost port forwarding: still broken, being worked on.
Remote port forwarding: should be fully working after latest push.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Wrong firewalld rule generated when publishing ports on specified ip
1 participant