What help did you need regarding firewall_setup?

What help did you need regarding firewall_setup?

Understanding how firewall_setup is called from client.py.

This would be important to do the traceback for any issue and also improve the functional tests. As I could understand, I only found unit tests with mocks testing directly the methods. The code I added in this PR is already being tested with tproxy unit tests already available, but new feature is only being tested with default value.

I also need some guidance about the tests that need to be provided for the new option tmark.

The main() function creates a FirewallClient object:

fw = FirewallClient(method_name, sudo_pythonpath)

This in turn creates a forked process via sudo that runs the firewall code.

Later on, when we are connected via ssh, the main function, or rather the serverready subfunction of main calls:

fw.start()

This sends the required initialization commands to the firewall process. Including the ROUTES, NSLIST, PORTS, and GO.

When the firewall process sees all these commands, it will call method.setup_firewall one or twice depending on if IPv6 or IPv4 is configured.

https://github.com/sshuttle/sshuttle/blob/master/sshuttle/firewall.py#L210

Does this help?

This sends the required initialization commands to the firewall process. Including the ROUTES, NSLIST, PORTS, and GO.

When the firewall process sees all these commands, it will call method.setup_firewall one or twice depending on if IPv6 or IPv4 is configured.

https://github.com/sshuttle/sshuttle/blob/master/sshuttle/firewall.py#L210

Does this help?

Yes, thank you Brian. My doubt was there after fw.start().

To keep the interface for method.setup_firewall I didn't add the new tmark argument there. I'm doing maybe something nasty, because I only load it with fw.setup call and expect it to be there as a property in the created object.
The tmark tests passed because I check the property and if not defined I set the expected default value tmark='1'. Inside setup_firewall for tproxy method I do the following:
if self.firewall is None:
tmark = '1'
else:
tmark = self.firewall.tmark

Is this a good approach to add this argument that is only necessary for tproxy method?

What you have got looks OK. But don't be put off making changes the interfaces either. It isn't like I put a lot of thought into these interfaces or anything.

Another possibility is that the test calls method.set_firewall() with a dummy firewall object.