Add support for validating the downstream ip of the connection

[kmala]

fixes #107 Had to change the PolicyFunc mentod signature as i couldn't find a better way to handle policy if we have separate functions for upstream and downstream and if both are present.

[dims]

@pires can you please approve the CI workflow so we can check if the new test being added works fine? thanks!

[coveralls]

coverage: 95.119% (+0.1%) from 95.017% when pulling 9dc11aaaa2c1720bb89599792aa3fa6ab867137b on kmala:feat into 8a2480a3966f69776d99be21dd09b345fb199ee6 on pires:main.

[pires]

@dims for you the world, thanks for pinging.

[dims]

@kmala please review the coverage tests, looks like we need to make sure they don't regress.

[kmala]

please review the coverage tests, looks like we need to make sure they don't regress.

Update and verified locally that the coverage has increased.

[emersion]

If we don't want to break the API, my take would be to add a new ConnPolicy callback:

• Only one of Policy and ConnPolicy can be specified, not both, to avoid having to pick between the two.
• ConnPolicy takes the net.Conn so that it can fetch any details it needs about the connection. Alternatively, it takes a ConnPolicyOptions struct which carry the IP addresses and can be extended later with more fields.

[kmala]

Only one of Policy and ConnPolicy can be specified, not both, to avoid having to pick between the two.

The issue is that since the Listener is exported https://github.com/pires/go-proxyproto/blob/main/protocol.go#L25 there is no proper way to enforce that only one of the policy/connpolicy can be specified.

We can just ignore Policy if ConnPolicy is specified in the code but that won't be obvious unless someone reads the code or comments.

[emersion]

It should be possible to panic from Accept if the library user provides both. Not ideal, but still better than silently ignoring one of the callbacks IMHO.

[kmala]

@emersion can you please check if the changes look good?

[pires]

I am thinking the ConnPolicy should become the new API, since it seems to encapsulate what Policy is doing today, and we'd mark Policy as deprecated and to be removed in a future release. WDYT?

[kmala]

I am thinking the ConnPolicy should become the new API, since it seems to encapsulate what Policy is doing today, and we'd mark Policy as deprecated and to be removed in a future release. WDYT?

Update the field as deprecated.

[kmala]

@pires @emersion can you please review when you get chance ?

[pires]

Thanks a ton for this contribution. We'll be releasing a new minor for introducing the new behavior and announce the deprecation of PolicyFunc 🖖🏻