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

kafka: add kafka_connections_max_overrides #4221

Merged
merged 3 commits into from
Apr 12, 2022

Conversation

jcsp
Copy link
Contributor

@jcsp jcsp commented Apr 7, 2022

Cover letter

This is followup to where kafka_connections_max and kafka_connections_max_per_ip were added. This change adds an "overrides" config property that lets the user target particular client addresses for special treatment.

This can be used as a field-expedient firewall, or for environments where certain hosts are known to run large numbers of clients.

Release notes

Features

  • Configuration property kafka_connections_max_overrides is added, enabling setting connection count limits on individual client IPs.

@jcsp jcsp added kind/enhance New feature or request area/kafka labels Apr 7, 2022
@jcsp jcsp changed the title Conn limit overrides kafka: add kafka_connections_max_overrides Apr 7, 2022
@jcsp jcsp marked this pull request as ready for review April 7, 2022 11:10
@jcsp jcsp requested a review from VadimPlh April 7, 2022 11:11
Copy link
Member

@dotnwat dotnwat left a comment

Choose a reason for hiding this comment

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

looks ok to me

"Per-IP overrides of kafka connection count limit, list of "
"<ip>:<count> strings",
{.needs_restart = needs_restart::no,
.example = R"(['127.0.0.1:90', '50.20.1.1:40'])",
Copy link
Member

Choose a reason for hiding this comment

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

nit: this looks like <ip_address>:<port> at first. Maybe <ip_address>=<limit> would be better ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@VadimPlh I was doing this symmetrically with the connection rate limiting config, what do you think about changing both of them to use "=" before we release?

Copy link
Contributor

Choose a reason for hiding this comment

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

I did it similar with kafka. As I remember they use ':'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I'm inclined to stick with : here, partly because of kafka, but mainly because it looks more "normal" in a YAML file where that's the usual character for key-value associations.

@jcsp
Copy link
Contributor Author

jcsp commented Apr 12, 2022

No changes, just rebased to re-test on top of the various changes from the last week or so. This should be good to go.

@dotnwat dotnwat merged commit a1d2980 into redpanda-data:dev Apr 12, 2022
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants