Skip to content

Conversation

bneradt
Copy link
Contributor

@bneradt bneradt commented Apr 17, 2025

This implements
proxy.config.http.per_client.connection.exempt_list, a
configuration for the user to be able to provide a set of IP addresses
that are not counted against
proxy.config.net.per_client.max_connections_in.

This also adds the following TS APIs to modify this list via a plugin:

  • TSConnectionLimitExemptListSet
  • TSConnectionLimitExemptListAdd
  • TSConnectionLimitExemptListClear

@bneradt bneradt added this to the 10.2.0 milestone Apr 17, 2025
@bneradt bneradt requested a review from zwoop April 17, 2025 20:10
@bneradt bneradt self-assigned this Apr 17, 2025
@zwoop
Copy link
Contributor

zwoop commented Apr 17, 2025

allow_ sounds a little strange, exempt_ would be better.

@bneradt bneradt force-pushed the per_client_connection_ignore_list branch from ea155d2 to 8aa79d9 Compare April 17, 2025 22:46
@bryancall bryancall requested a review from maskit April 21, 2025 22:39
@bneradt
Copy link
Contributor Author

bneradt commented Apr 24, 2025

allow_ sounds a little strange, exempt_ would be better.

Agreed. Changed to exempt terminology.

@bneradt bneradt changed the title proxy.config.http.per_client.connection.allow_list.filename proxy.config.http.per_client.connection.exempt_list.filename May 1, 2025
@bneradt bneradt force-pushed the per_client_connection_ignore_list branch 2 times, most recently from d611c07 to a114f59 Compare June 24, 2025 20:24
@maskit
Copy link
Member

maskit commented Jun 25, 2025

I'm wondering how this kind of lists should be on ATS config.

proxy_protocol_allowlist is just a comma-separated string (it's probably because the previous config format could not have a real list structure). And per_client.connection.exempt_list is a string too but a filename.

I do see the benefit of having the list in a separate file, but I also think inconsistency should be avoided. Most settings that have filenames are ones that had different formats, and now we are trying to make all of them YAML. We can have real lists in records.yaml. It may be a time to think about what should be separated from records.yaml.

@bneradt
Copy link
Contributor Author

bneradt commented Jun 27, 2025

I'm wondering how this kind of lists should be on ATS config.

proxy_protocol_allowlist is just a comma-separated string (it's probably because the previous config format could not have a real list structure). And per_client.connection.exempt_list is a string too but a filename.

I do see the benefit of having the list in a separate file, but I also think inconsistency should be avoided. Most settings that have filenames are ones that had different formats, and now we are trying to make all of them YAML. We can have real lists in records.yaml. It may be a time to think about what should be separated from records.yaml.

My internal implementation initially had this as a comma separated list, but ops asked me to make it separate file. And, as you point out, that's a pretty reasonable ask. It makes deployment of this easier. Would a rename of this configuration help things? Maybe exempt_list_filename?

@maskit
Copy link
Member

maskit commented Jun 27, 2025

Would a rename of this configuration help things? Maybe exempt_list_filename?

I'm not sure if we want to rename it. Depends on what we are going to do in the future.

The bigger issue I tried to raise was that one could ask for separating out any part of ATS config and we don't have a policy for it. proxy_protocol_allowlist currently cannot be in a separate file. Should we introduce proxy_protocol_allowlist_filename as well for consistency? Somebody might want to have entire http config in another file. Would we support it?

It could be a discussion topic for ATS summit or a hackathon.

@bneradt bneradt force-pushed the per_client_connection_ignore_list branch 4 times, most recently from 1b5aafe to abd114b Compare August 28, 2025 04:06
@bneradt bneradt changed the title proxy.config.http.per_client.connection.exempt_list.filename proxy.config.http.per_client.connection.exempt_list Aug 28, 2025
@bneradt bneradt force-pushed the per_client_connection_ignore_list branch 2 times, most recently from 6e8d401 to 960350e Compare August 29, 2025 17:50
@bneradt bneradt force-pushed the per_client_connection_ignore_list branch from 960350e to f377141 Compare September 6, 2025 16:35
@bneradt bneradt force-pushed the per_client_connection_ignore_list branch from f377141 to 8b85aa1 Compare October 7, 2025 18:21
This implements
proxy.config.http.per_client.connection.exempt_list, a
configuration for the user to be able to provide a set of IP addresses
that are not counted against
proxy.config.net.per_client.max_connections_in.

This also adds the following TS APIs to modify this list via a plugin:

TSConnectionLimitExemptListSet
TSConnectionLimitExemptListAdd
TSConnectionLimitExemptListClear
@bneradt bneradt force-pushed the per_client_connection_ignore_list branch from 8b85aa1 to 57b3e5a Compare October 9, 2025 22:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants