`admin`: add `key` param to `get_cluster_config` endpoint #22674

WillemKauf · 2024-07-31T20:37:36Z

Fixes #22666, depends on redpanda-data/common-go#11

rpk cluster config get makes a request for a single property from the cluster. However, redpanda currently returns the cluster configuration in whole, and allows rpk to index the desired property.

This makes dealing with aliased property names tricky, as the configuration json contains only key-value information.
Since rpk cluster config get is only concerned with one parameter, we add the optional key query parameter, which will be used to handle the possibility of an aliased property name internally by redpanda and return only one key-value pair in the regular json object.

The returned JSON will reference the alias's name, i.e a GET request to /v1/cluster_config?key=data_transforms_per_core_memory_reservation will return

{'data_transforms_per_core_memory_reservation': 123456789},

and a request to /v1/cluster_config?key=wasm_per_core_memory_reservation will return

{'wasm_per_core_memory_reservation': 123456789}.

Backports Required

Release Notes

Improvements

Allows users to query the value of a cluster property with rpk cluster config get using either the original property name, or any of its aliases. Whereas before, rpk cluster config get using a property's aliased name would return a Property {} not found result.

Before Example:

$ rpk cluster config set wasm_per_core_memory_reservation=123456789
Successfully updated configuration. New configuration version is 3.

Cluster needs to be restarted. See more details with 'rpk cluster config status'.

$ rpk cluster config get  wasm_per_core_memory_reservation
Property 'wasm_per_core_memory_reservation' not found

$ rpk cluster config get  data_transforms_per_core_memory_reservation
123456789

After Example:

$ rpk cluster config set wasm_per_core_memory_reservation=123456789
Successfully updated configuration. New configuration version is 3.

Cluster needs to be restarted. See more details with 'rpk cluster config status'.

$ rpk cluster config get  wasm_per_core_memory_reservation
123456789

$ rpk cluster config get  data_transforms_per_core_memory_reservation
123456789

`rpk cluster config get` makes a request for a single property from the cluster. Previously, this would return the entire `configuration` json to `rpk`, which would pick the key out for the sole property it wanted. Add `key` as an optional query parameter to the `get_cluster_config` HTTP route, in order to support returning just a single property from a cluster config request. This will also support using property aliases with `rpk cluster config get` in the future.

src/go/rpk/pkg/cli/cluster/config/get.go

JFlath · 2024-07-31T22:15:56Z

This is super slick as a way of solving the issue! How does it work with back-compat? I.e. if I use an rpk with this change in it to talk to an earlier cluster, I guess the key param will be ignored and the full list returned? I think the sanity check you're talking about in a comment above would catch that, but at present my read of the code is that we'd take the first value in the full config and return it as though it was an alias, right?

WillemKauf · 2024-07-31T22:20:28Z

but at present my read of the code is that we'd take the first value in the full config and return it as though it was an alias, right?

@JFlath Great catch, in my mind I was thinking of backporting to try and avoid this issue, but the sanity check could also be used. It might not be a great user experience to die on a simple rpk cluster config get, but its an edge case we need to deal with.

WillemKauf · 2024-07-31T23:12:03Z

Force push to add sanity check in rpk cluster config get. If there is a version mismatch between redpanda (pre v24.2.x) and rpk, cluster config get key will ignore ?key= query parameter, and still return the entire config json.

~~If we are unable to find the original key (in the case of a property alias), we output a lengthy message and terminate the request~~

vbotbuildovich · 2024-08-01T01:49:43Z

new failures in https://buildkite.com/redpanda/redpanda/builds/52336#01910b54-c6b9-41a0-83d1-e1989562ab72:

"rptest.tests.cluster_config_test.ClusterConfigTest.test_get_config_nodefaults"
"rptest.tests.controller_snapshot_test.ControllerSnapshotTest.test_upgrade_compat"

new failures in https://buildkite.com/redpanda/redpanda/builds/52336#01910b6a-97e7-4755-b484-615f2d9e70ee:

"rptest.tests.cluster_config_test.ClusterConfigTest.test_get_config_nodefaults"
"rptest.tests.controller_snapshot_test.ControllerSnapshotTest.test_upgrade_compat"

new failures in https://buildkite.com/redpanda/redpanda/builds/52336#01910b54-c6bd-4c79-bb3f-5851cc50f076:

"rptest.tests.cluster_config_test.ClusterConfigTest.test_rpk_import_sparse.all=True"

new failures in https://buildkite.com/redpanda/redpanda/builds/52336#01910b6a-97ea-4f04-8bd3-99109fb3076d:

"rptest.tests.cluster_config_test.ClusterConfigTest.test_rpk_import_sparse.all=True"

vbotbuildovich · 2024-08-01T01:55:07Z

ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/52336#01910b54-c6b7-49ff-8dcd-d842a9c6e7f9

ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/52336#01910b6a-97ea-4f04-8bd3-99109fb3076d

ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/52342#01910c68-52be-41a2-a942-4105b5656879

src/go/rpk/pkg/cli/cluster/config/get.go

For writing a single key value property pair to json.

In order to use the query parameter `key` in the admin server, it is parsed out into a `filter` object in the request route. This also allows us to handle the possibility for aliased property requests with `rpk cluster config get`.

`newGetCommand` will issue a request for a single property from the `client` (to be changed in `common-go/rpadmin`). This allows for `rpk cluster config get` to work with property's aliased names, i.e `rpk cluster config get wasm_per_core_memory_reservation`, `rpk cluster config get data_transforms_per_core_memory_reservation` should both return the same value.

In order to allow for requests to be made by specifying a single key to the admin endpoint. Also adds some test coverage for use of aliased properties with `get` and `set` functionality with the admin endpoint and the new improvements.

WillemKauf · 2024-08-01T03:53:56Z

Force push to update behaviour of rpk cluster config get when an alias is used.

If key foo is an alias for property bar, the response JSON from rpk cluster config get foo is {"foo":"bar_value"}.

If property bar is queried, then the JSON is {"bar":"bar_value"}.

In the case of an older version of redpanda not respecting the query parameter ?key=, the same behaviour as seen before can be expected. That being, rpk cluster config get foo will return the response Property 'foo' not found (since the entire cluster configuration JSON will be returned, and the alias will not be found in the object.)

r-vasquez

rpk changes LGTM, thanks!

andrwng

LGTM!

andrwng · 2024-08-01T20:59:06Z

src/go/rpk/go.sum

+github.com/redpanda-data/common-go/rpadmin v0.1.3 h1:JRdr4rHcdr+A0hHr+viJYnPm+dP01bsgVUoQLM7Kz44=
+github.com/redpanda-data/common-go/rpadmin v0.1.3/go.mod h1:I7umqhnMhIOSEnIA3fvLtdQU7QO/SbWGCwFfFDs3De4=


nit: would be useful as a reviewer / git historian to see the commit you are intending to pull in with this bump mentioned in the commit message

Noted, thanks

WillemKauf requested review from andrwng and r-vasquez July 31, 2024 20:37

WillemKauf requested review from a team, twmb, gene-redpanda and Deflaimun as code owners July 31, 2024 20:37

github-actions bot added area/rpk area/redpanda labels Jul 31, 2024

WillemKauf changed the title ~~rpk: add single_property param to get_cluster_config endpoint~~ rpk: add key param to get_cluster_config endpoint Jul 31, 2024

WillemKauf changed the title ~~rpk: add key param to get_cluster_config endpoint~~ admin: add key param to get_cluster_config endpoint Jul 31, 2024

r-vasquez reviewed Jul 31, 2024

View reviewed changes

src/go/rpk/pkg/cli/cluster/config/get.go Outdated Show resolved Hide resolved

WillemKauf force-pushed the rpk_config_single_property branch 2 times, most recently from 8392ed1 to fa9bc0e Compare July 31, 2024 22:01

WillemKauf force-pushed the rpk_config_single_property branch from fa9bc0e to c634581 Compare July 31, 2024 23:08

twmb reviewed Aug 1, 2024

View reviewed changes

src/go/rpk/pkg/cli/cluster/config/get.go Outdated Show resolved Hide resolved

WillemKauf added 5 commits July 31, 2024 23:41

config: add config_store::to_json_single_key

1132b9c

For writing a single key value property pair to json.

admin: use key in get_cluster_config route

7bfbb32

In order to use the query parameter `key` in the admin server, it is parsed out into a `filter` object in the request route. This also allows us to handle the possibility for aliased property requests with `rpk cluster config get`.

rpk: bump go.mod version

b0cd673

WillemKauf force-pushed the rpk_config_single_property branch from c634581 to 8f3c771 Compare August 1, 2024 03:45

r-vasquez approved these changes Aug 1, 2024

View reviewed changes

andrwng approved these changes Aug 1, 2024

View reviewed changes

WillemKauf merged commit c269833 into redpanda-data:dev Aug 2, 2024
32 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`admin`: add `key` param to `get_cluster_config` endpoint #22674

`admin`: add `key` param to `get_cluster_config` endpoint #22674

WillemKauf commented Jul 31, 2024 •

edited

Loading

JFlath commented Jul 31, 2024

WillemKauf commented Jul 31, 2024

WillemKauf commented Jul 31, 2024 •

edited

Loading

vbotbuildovich commented Aug 1, 2024 •

edited

Loading

vbotbuildovich commented Aug 1, 2024 •

edited

Loading

WillemKauf commented Aug 1, 2024

r-vasquez left a comment

andrwng left a comment

andrwng Aug 1, 2024

WillemKauf Aug 2, 2024

		github.com/redpanda-data/common-go/rpadmin v0.1.3 h1:JRdr4rHcdr+A0hHr+viJYnPm+dP01bsgVUoQLM7Kz44=
		github.com/redpanda-data/common-go/rpadmin v0.1.3/go.mod h1:I7umqhnMhIOSEnIA3fvLtdQU7QO/SbWGCwFfFDs3De4=

admin: add key param to get_cluster_config endpoint #22674

admin: add key param to get_cluster_config endpoint #22674

Conversation

WillemKauf commented Jul 31, 2024 • edited Loading

Backports Required

Release Notes

Improvements

JFlath commented Jul 31, 2024

WillemKauf commented Jul 31, 2024

WillemKauf commented Jul 31, 2024 • edited Loading

vbotbuildovich commented Aug 1, 2024 • edited Loading

vbotbuildovich commented Aug 1, 2024 • edited Loading

WillemKauf commented Aug 1, 2024

r-vasquez left a comment

Choose a reason for hiding this comment

andrwng left a comment

Choose a reason for hiding this comment

andrwng Aug 1, 2024

Choose a reason for hiding this comment

WillemKauf Aug 2, 2024

Choose a reason for hiding this comment

`admin`: add `key` param to `get_cluster_config` endpoint #22674

`admin`: add `key` param to `get_cluster_config` endpoint #22674

WillemKauf commented Jul 31, 2024 •

edited

Loading

WillemKauf commented Jul 31, 2024 •

edited

Loading

vbotbuildovich commented Aug 1, 2024 •

edited

Loading

vbotbuildovich commented Aug 1, 2024 •

edited

Loading