After considering the feedback on this list and some offline feedback, I've re-implemented this feature quite differently. From the user's configuration perspective of ip_allow.yaml and the category specification in remap.config, the proposal does not change. That is, the user can still provide category specifications in ip_allow.yaml like this (same as the earlier proposal in this thread):
- apply: in ip_category: ACME_INTERNAL action: allow methods: - GET - HEAD - POST - DELETE Further, categories are still supported for so-called remap.config ACLs like the following: map http://foo.example.com/ http://foo.example.com/ @action=allow *@src_ip_category=ACME_INTERNAL* @method=post @method=get @method=head However, Alan helpfully pointed out that it is reasonable and far more efficient for a plugin to specify the IP spaces for each category rather than simply provide one-off requests about whether any given address belongs to any given category. This allows the IpAllow component to complete the IPSpace for the records as it has before, making address lookup for the appropriate records very efficient. From the plugin's perspective, therefore, it is it simply needs to call the following API to set the mapping of category to the analogous space: /** For each IP allow category, define the IP space associated with it. * * @param[in] category_map For each category name, the IP space associated with it. */ void TSHttpSetCategoryIPSpaces(std::unordered_map<std::string, swoc::IPSpace<bool>> const &category_spaces); Again, as before, the idea is that the plugin can populate the spaces from parsing a configuration file, querying a database, or whatever other mechanism is appropriate for the organization. It is expected that typically a developer for an organization will implement the plugin that provides the category IP spaces while the organization's ops team will be able to use these categories to specify IP category filtering via ip_allow.yaml and remap.config ACLs. With these changes, the ip_allow.yaml and remap.config interfaces remain intuitive for the user, the API is simplified, and the IpAllow rule implementation for these categories is exactly as efficient as it is today for the current IP ranges. I have created a separate draft PR demonstrating this here: https://github.com/apache/trafficserver/pull/10939 Please chime in with any further thoughts about the IP category feature. On Thu, Nov 30, 2023 at 6:13 PM Shu Kit Chan <chanshu...@gmail.com> wrote: > So for each request, the plugin will be called into action X number of > times for X number of categories found? > > More specifically, > If there is a match in a category, it will not call the remaining > number of categories to try? (Since I assume the first match will take > precedence over the rest?) > If there is no match in all categories, it will still go through X > number of times. > > Is my understanding correct? > > Perhaps if we give the plugin handler an array of categories in the > listed order in the config yaml, And ask the handler to return either > one of the categories (that matches) or null if there is no match. > Will this be a bit more efficient or probably the same? > > Kit > > > On Mon, Nov 27, 2023 at 9:50 PM Brian Neradt <brian.ner...@gmail.com> > wrote: > > > > A quick update: > > > > This morning I realized that remap ACLs > > < > https://docs.trafficserver.apache.org/en/latest/admin-guide/files/remap.config.en.html#acl-filters > > > > should to be updated for this too. I updated the draft PR to include > adding > > @src_ip_category that takes a category to apply to remap rules. That > > functions in every way like the ip_allow.yaml ip_category described in > the > > previous email. > > > > Thanks, > > Brian > > > > On Wed, Nov 22, 2023 at 4:29 PM Brian Neradt <brian.ner...@gmail.com> > wrote: > > > > > dev@trafficserver.apache.org, > > > > > > Bryan Call and I have been working on an idea for ip_allow.yaml. At > Yahoo, > > > we have an internal API which categorizes IPs under certain groups via > a > > > database lookup. Each of the IP categories have potentially different > > > requirements for which HTTP methods are accessible to them. For > instance, > > > IP addresses in something like ACME_INTERNAL may be able to initiate > HTTP > > > requests using modifying methods such as PUSH and DELETE as well as > GET and > > > HEAD requests, while IP addresses labeled ANY_IP may only be able to > use > > > requests with GET and HEAD methods. While the general filtering > > > functionality of ip_allow.yaml would be ideal for controlling these > HTTP > > > method restrictions, the current ip_addrs node of ip_allow.yaml, while > good > > > for explicit IP range specification, does not allow for programmatic IP > > > categorization like that described above. > > > > > > Currently, Yahoo implements the required filtering by basically > stripping > > > ip_allow.yaml to nothing but the absolute minimum denominator of method > > > restrictions and then uses an internal plugin to implement the method > > > filtering based upon the categories of the database. > > > > > > In discussing this method filtering by IP category, Bryan Call had the > > > idea that this feature could be implemented as a general solution for > the > > > community by a relatively simple extension of ip_allow.yaml itself. I > spent > > > some time working on this and have a draft PR demonstrating the feature > > > here: > > > > > > https://github.com/apache/trafficserver/pull/10840 > > > > > > The feature includes the following new user interfaces: > > > > > > ip_category > > > The feature adds ip_category to ip_allow.yaml record as an alternative > to > > > the ip_addrs key. Here is an example record specification using an > > > ip_category of ACME_INTERNAL: > > > > > > - apply: in ip_category: ACME_INTERNAL action: allow methods: - > GET - HEAD - POST - DELETE > > > > > > TS_HTTP_IP_ALLOW_CATEGORY_HOOK hook > > > The user then supplies a plugin that implements the > > > TS_HTTP_IP_ALLOW_CATEGORY_HOOK callback which the core uses to query > > > whether a given incoming IP is applicable to a given category, such as > > > ACME_INTERNAL above. Knowing this informs the core whether a particular > > > ip_allow.yaml rule applies to an incoming or outgoing connection. > > > > > > New Category Plugin API Functions > > > The plugin that implements the TS_HTTP_IP_ALLOW_CATEGORY_HOOK handler > then > > > uses the following API to communicate with the core about whether an IP > > > belongs to a category: > > > > > > /** Obtain the category name being inspected. > > > * @param[in] infop The ip_allow info object. > > > * @param[out] category The category name being inspected whether it > > > contains > > > * the address obtained via @a TSHttpIpAllowInfoAddrGet. > > > * @return TS_SUCCESS if the category name was obtained, TS_ERROR > > > otherwise. > > > */ > > > TSReturnCode TSHttpIpAllowInfoCategoryGet(TSHttpIpAllowInfo infop, > > > std::string_view &category); > > > > > > /** Obtain the address being queried whether it belongs to a > category. > > > * @param[in] infop The ip_allow info object. > > > * @param[out] addr The address being queried against a category > > > obtained via > > > * @a TSHttpIpAllowInfoCategoryGet. > > > * @return TS_SUCCESS if the address was obtained, TS_ERROR > otherwise. > > > */ > > > TSReturnCode TSHttpIpAllowInfoAddrGet(TSHttpIpAllowInfo infop, > sockaddr > > > &addr); > > > > > > /** Set whether an address applies to a category. > > > * @param[in] infop The ip_allow info object. > > > * @param[in] contains Whether the address obtained via > > > * @a TSHttpIpAllowInfoAddrGet is contained in the category > obtained via > > > * @a TSHttpIpAllowInfoCategoryGet. > > > */ > > > void TSHttpIpAllowInfoContainsSet(TSHttpIpAllowInfo infop, bool > > > contains); > > > > > > The TSHttpIpAllowInfo is an opaque structure, but in the backend it > simply > > > contains the three pieces of data represented above: > > > > > > struct HttpIpAllowInfo { > > > > > > /// A user-specified name for IP categories. > > > std::string_view category; > > > > > > /// The IP address to query whether it is contained in @a category. > > > swoc::IPAddr const &addr; > > > > > > /// The result of the query: the hook sets this to true of @a addr > is in > > > @a > > > /// category, false otherwise. > > > bool contains = false; > > > }; > > > > > > An example plugin that makes use of this API can be found in the draft > PR > > > here: > > > > > > > > > > https://github.com/bneradt/trafficserver/blob/ip_allow_categories/example/plugins/c-api/ip_category/ip_category.cc > > > > > > We hope that this feature can be generally helpful to other users in > the > > > community who have IP categories that cannot be statically specified > in the > > > ip_addrs node of ip_allow.yaml. We welcome any thoughts or suggestions > for > > > improvement concerning the above suggested API. > > > > > > Thanks, > > > Brian > > > > > > > > > -- > > > "Come to Me, all who are weary and heavy-laden, and I will > > > give you rest. Take My yoke upon you and learn from Me, for > > > I am gentle and humble in heart, and you will find rest for > > > your souls. For My yoke is easy and My burden is light." > > > > > > ~ Matthew 11:28-30 > > > > > > > > > -- > > "Come to Me, all who are weary and heavy-laden, and I will > > give you rest. Take My yoke upon you and learn from Me, for > > I am gentle and humble in heart, and you will find rest for > > your souls. For My yoke is easy and My burden is light." > > > > ~ Matthew 11:28-30 > -- "Come to Me, all who are weary and heavy-laden, and I will give you rest. Take My yoke upon you and learn from Me, for I am gentle and humble in heart, and you will find rest for your souls. For My yoke is easy and My burden is light." ~ Matthew 11:28-30
