search

porech / caddy-maxmind-geolocation

Caddy v2 module to filter requests based on source IP geolocation
86 stars 12 forks source link

FR: Also expose placeholders for geo ip information #6

Open melink14 opened 3 years ago

melink14 commented 3 years ago

I was looking at caddy geoip capabilities so I could augment my access logs with location information (for later processing and monitoring) and it seems like this plugin is the only geolocation plugin that was ported to caddy v2.

caddy v1 had https://github.com/aablinov/caddy-geoip which exposed placeholders and I think this plugin is best situated to do something similar.

There might be an issue of redundancy since once you define placeholders then one could use either the custom matchers already defined by the plugin or just use expression to create any condition they want using the value of the placeholders.

(On the other hand, it would make extensions like https://github.com/porech/caddy-maxmind-geolocation/issues/3 to implement since no expression syntax would need to change)

It seems like using a handler to add placeholders is pretty easy but it is a bit of a philosophical change vs having a matcher. My own opinion is providing placeholders is more elegant software solution but then we'd have to worry about migration.

Let me know what you think!

ermiaqasemi commented 2 years ago

Hey, I believe this would be very useful feature, is there any plan to add this placeholders ?

sandstrom commented 8 months ago

Here is another addon that does it.

But would be great if that functionality was available in this addon too (so we didn't need two).

https://github.com/shift72/caddy-geo-ip