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

Throughput limit exemptions for SR, PP #11692

Draft
wants to merge 12 commits into
base: dev
Choose a base branch
from

Conversation

dlex
Copy link
Contributor

@dlex dlex commented Jun 26, 2023

#11555 has introduced throughput limit exemptions for principal type user. This change enhances that by introducing a special syntax of tput ctrl groups configuration that translates into the ephemeral users of PP and SR.

Fixes #11438

Backports Required

  • none - not a bug fix
  • none - this is a backport
  • none - issue does not exist in previous branches
  • none - papercut/not impactful enough to backport
  • v23.1.x
  • v22.3.x
  • v22.2.x

Release Notes

  • TBD

Add a function to RpkTool to create an allow topic ACL for a user.
A reusable part of acl_create_allow_cluster refactored.
// TBD: how to define the PP/SR ephemeral principal in one place
// and avoid dependency of `config` on `pandaproxy`?
YAML::Node service_node = node[ids::service];
if (principal.name() == "__pandaproxy") {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@BenPope I'm looking for an advice from you what would be a good way to share this literal between pandaproxy and config.

Copy link
Member

Choose a reason for hiding this comment

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

I guess we can move them to src/v/config.

// TBD: how to define the PP/SR ephemeral principal in one place
// and avoid dependency of `config` on `pandaproxy`?
YAML::Node service_node = node[ids::service];
if (principal.name() == "__pandaproxy") {
Copy link
Member

Choose a reason for hiding this comment

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

I guess we can move them to src/v/config.

# - service: panda proxy
- name: match schema registry (principal 'ephemeral user')
principals:
- service: schema registry
Copy link
Member

Choose a reason for hiding this comment

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

Question: Why not keep this as security::principal_type? I.e., ephemeral_user rather than service?

I don't dislike it, but it adds an overloaded term.

Copy link
Contributor Author

@dlex dlex Jun 28, 2023

Choose a reason for hiding this comment

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

That would have exposed an implementation detail for the users. Currently the name of the ephemeral user used for SR/PP is not exposed anywhere, and is only a matter of the code. It can be e.g. a uuid as well, and it can be changed anytime.

dlex added 11 commits June 28, 2023 14:10
Support --username/--password authentication options

Add a function to check if the producer is running

Log service name with progress messages so that in case of several
producers running at the same time, it is possible to understand
who is doing what
typos, renames, comments
when throughput group is matched, log the client address
Becasue of the unique_ptr involved, the default equality operator
does not work correctly here.
`throughput_control_group` gets additional matching criteria by ACL
principals (users, etc.). In yaml/json, there can now be a list of
principals the group must match besides client_id if provided. Only the
principal type `user` is supported in this commit.
Add user names to the matching pattern of throughput_control groups
acl_principal is now another key to snc_quotas_context. It is passed
from the connection_context to create the context by matching with
a tput ctrl group, and to verify whether the context is still valid.
A new test to verify that when a user matches a tput ctrl group,
the connections authenticated by the user are not throttled while
the rest still are.
to complement the logging of node configurations. Note that
cluster_config.yaml records cluster config when nodes stop, whlie
this log dumps the initial bootstrap config.
in tput group configuration, one can specify "service" instead of a
"user", and specify SR or PP as the service, which internally
transaltes into the corresponding acl principal
@dlex dlex force-pushed the 11438/tp-exemptions-for-principals branch from f5bcee6 to a562240 Compare June 28, 2023 23:56
@dlex
Copy link
Contributor Author

dlex commented Jun 29, 2023

Force push: minor changes f/up review of #11555

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.

Cluster/Node Throughput Exemptions for Users/Principals
2 participants