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

connection policy: add local_ip matcher #6074

Merged
merged 6 commits into from
Apr 15, 2024

Conversation

mohammed90
Copy link
Member

This is for discussion, because I haven't thoroughly vetted the idea but curious if we can introduce more matchers to behave more like the HTTP handlers matchers.

It might resolve cases like #5771, depending on the use case.

@mohammed90 mohammed90 added feature ⚙️ New feature or request discussion 💬 The right solution needs to be found labels Jan 30, 2024
@mohammed90 mohammed90 requested a review from mholt January 30, 2024 09:03
modules/caddytls/matchers.go Outdated Show resolved Hide resolved
modules/caddytls/matchers.go Show resolved Hide resolved
modules/caddytls/matchers.go Outdated Show resolved Hide resolved
@mohammed90
Copy link
Member Author

Changing the PR to draft until I add tests and we have consensus on the discussion whether this is a good change

@mohammed90 mohammed90 marked this pull request as draft January 30, 2024 16:39
@mholt
Copy link
Member

mholt commented Apr 15, 2024

I like where this is going.

What if we just start with the local IP matcher and then see about adding the complexity of the matcher sets later if we feel it is needed based on real-world feedback?

@mohammed90
Copy link
Member Author

What if we just start with the local IP matcher and then see about adding the complexity of the matcher sets later if we feel it is needed based on real-world feedback?

Done.

@mohammed90 mohammed90 marked this pull request as ready for review April 15, 2024 18:00
@mohammed90 mohammed90 changed the title connection policy: add local_ip and not matchers connection policy: add local_ip matcher Apr 15, 2024
Copy link
Member

@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.

Cool, thank you!

I wonder if we should use this in the Caddyfile parser when crafting TLS connection policies -- instead of always using ServerName, which will be empty then the host is the IP address.

Not for this PR, but just thinking for future improvement.

modules/caddytls/matchers.go Outdated Show resolved Hide resolved
@mholt mholt added this to the v2.8.0 milestone Apr 15, 2024
Co-authored-by: Matt Holt <mholt@users.noreply.github.com>
@mohammed90 mohammed90 merged commit 26748d0 into master Apr 15, 2024
25 checks passed
@mohammed90 mohammed90 deleted the composite-connpolicy-matcher branch April 15, 2024 18:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion 💬 The right solution needs to be found feature ⚙️ New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants