node: Validate RPC limiter configuration #1658

Merged
fyrchik merged 2 commits from a-savchuk/frostfs-node:rpc-limiter-validation into master 2025-02-28 14:13:12 +00:00
Member

Validate that configured limits match the methods registered earlier. Tested on service start and reload for both frost.fs and neo.fs.v2 package names.

Validate that configured limits match the methods registered earlier. Tested on service start and reload for both `frost.fs` and `neo.fs.v2` package names.
a-savchuk added 2 commits 2025-02-28 13:24:33 +00:00
- Move all initialization logic to one place
- Initialize the limiter after all RPC services are registered to be able
  to validate that configured limits correspond to the methods registered
  earlier

Signed-off-by: Aleksey Savchuk <a.savchuk@yadro.com>
[#xx] node: Validate RPC limiter configuration
Some checks failed
DCO action / DCO (pull_request) Failing after 37s
Vulncheck / Vulncheck (pull_request) Successful in 1m12s
Build / Build Components (pull_request) Successful in 1m43s
Pre-commit hooks / Pre-commit (pull_request) Successful in 1m42s
Tests and linters / Run gofumpt (pull_request) Successful in 2m42s
Tests and linters / Tests with -race (pull_request) Successful in 3m1s
Tests and linters / Lint (pull_request) Successful in 3m11s
Tests and linters / Staticcheck (pull_request) Successful in 3m9s
Tests and linters / Tests (pull_request) Successful in 3m25s
Tests and linters / gopls check (pull_request) Successful in 3m36s
9e81952048
Validate that configured limits correspond to the methods registered earlier.

Signed-off-by: Aleksey Savchuk <a.savchuk@yadro.com>
a-savchuk force-pushed rpc-limiter-validation from 9e81952048 to 799ae3ce75 2025-02-28 13:26:26 +00:00 Compare
a-savchuk changed title from WIP: node: Validate RPC limiter configuration to node: Validate RPC limiter configuration 2025-02-28 13:28:03 +00:00
requested reviews from storage-core-committers, storage-core-developers 2025-02-28 13:28:04 +00:00
fyrchik reviewed 2025-02-28 13:30:07 +00:00
@ -234,0 +258,4 @@
for _, limit := range limits {
for _, method := range limit.Keys {
if _, ok := availableMethods[method]; !ok {
return fmt.Errorf("set limit on unknown method %q", method)
Owner

on unknown -> on an unknown

`on unknown` -> `on an unknown`
Author
Member

Fixed

Fixed
fyrchik marked this conversation as resolved
fyrchik approved these changes 2025-02-28 13:30:10 +00:00
Dismissed
fyrchik added this to the v0.45.0 milestone 2025-02-28 13:30:30 +00:00
fyrchik added the
enhancement
config
frostfs-node
labels 2025-02-28 13:30:55 +00:00
a-savchuk force-pushed rpc-limiter-validation from 799ae3ce75 to c660271039 2025-02-28 13:33:02 +00:00 Compare
a-savchuk dismissed fyrchik's review 2025-02-28 13:33:02 +00:00
Reason:

New commits pushed, approval review dismissed automatically according to repository settings

dstepanov-yadro reviewed 2025-02-28 13:58:36 +00:00
@ -234,0 +253,4 @@
return nil
}
func validateRPCLimits(c *cfg, limits []limiting.KeyLimit) error {

A controversial point.
What if I want to disable tree or control service? In this case, I definitely have to edit the configuration file for the limits.

A controversial point. What if I want to disable tree or control service? In this case, I definitely have to edit the configuration file for the limits.
Author
Member

Does "disable" mean setting limit to 0. What's the problem then? Can you please give more detail?

Does "disable" mean setting limit to 0. What's the problem then? Can you please give more detail?
Author
Member

As I remember, we also agreed that we don't apply limiting for control service

As I remember, we also agreed that we don't apply limiting for control service
Owner

Disabling tree service is a fine point.
But in this case editing configuration is probably warranted?
It is not an operation one would do frequently.

Disabling tree service is a fine point. But in this case editing configuration is probably warranted? It is not an operation one would do frequently.
dstepanov-yadro approved these changes 2025-02-28 13:58:41 +00:00
fyrchik approved these changes 2025-02-28 14:13:05 +00:00
fyrchik merged commit c660271039 into master 2025-02-28 14:13:12 +00:00
a-savchuk deleted branch rpc-limiter-validation 2025-02-28 14:29:04 +00:00
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: TrueCloudLab/frostfs-node#1658
No description provided.