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.

Sign up for GitHub

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

Jump to bottom

l4proxy: add proxy_protocol matcher #30

Merged
merged 4 commits into from
Nov 17, 2021
Merged

l4proxy: add proxy_protocol matcher #30

merged 4 commits into from
Nov 17, 2021

Conversation

MadhuvanthG
Copy link
Contributor

@MadhuvanthG MadhuvanthG commented Sep 26, 2021 •
edited
Loading

adds capability for part of #20

  • not sure yet on how to build caddy locally that includes changes to a module. This instruction in README doesn't work me, I don't see the layer4 modules, if I build that way

In the project folder, run xcaddy just like you would run caddy. For example: xcaddy list-modules --versions (you should see the layer4 modules).

  • Not familiar with how could we test this module which operates at layer4? Do we use certain tools to simulate traffic? Although this matcher is very simple, would like to test that it works, to be sure.

mholt
mholt requested changes Sep 30, 2021
View reviewed changes
Copy link
Owner

@mholt mholt left a comment

Choose a reason for hiding this comment

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

Thanks for working on this! It can be improved and we can drop a dependency in this file.

modules/l4proxy/matcher.go Outdated Show resolved Hide resolved
modules/l4proxy/matcher.go Outdated Show resolved Hide resolved
modules/l4proxy/matcher.go Outdated Show resolved Hide resolved
modules/l4proxy/matcher.go Outdated Show resolved Hide resolved
@MadhuvanthG MadhuvanthG requested a review from mholt October 2, 2021 14:24
mholt
mholt approved these changes Nov 1, 2021
View reviewed changes
Copy link
Owner

@mholt mholt left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for contributing this! And sorry for the delay.

mholt
mholt requested changes Nov 1, 2021
View reviewed changes
Copy link
Owner

@mholt mholt left a comment

Choose a reason for hiding this comment

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

Oops, seems the rename got passed up

modules/l4proxy/matcher.go Outdated Show resolved Hide resolved
@MadhuvanthG
Copy link
Contributor Author

MadhuvanthG commented Nov 17, 2021 •
edited
Loading

@mholt, I've addressed the suggestion on renaming. Let me know if there are any other comments.

Looking forward to getting this merged.

@francislavoie francislavoie changed the title feat(PROXY): add matcher l4proxy: add proxy_protocol matcher Nov 17, 2021
@francislavoie francislavoie added the enhancement New feature or request label Nov 17, 2021
francislavoie
francislavoie approved these changes Nov 17, 2021
View reviewed changes
Copy link
Collaborator

@francislavoie francislavoie left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for updating the PR 😄

mholt
mholt approved these changes Nov 17, 2021
View reviewed changes
Copy link
Owner

@mholt mholt left a comment

Choose a reason for hiding this comment

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

LGTM, thank you for the changes and for the contribution!

@mholt mholt merged commit 3c14cbb into mholt:master Nov 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@francislavoie francislavoie francislavoie approved these changes

@mholt mholt mholt approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@MadhuvanthG @mholt @francislavoie