From 5fd0da68ea3e798ee26334b7fde64062f22b8247 Mon Sep 17 00:00:00 2001 From: Myroslav Vivcharyk Date: Wed, 18 Dec 2024 10:40:16 +0100 Subject: [PATCH] fix: patched ServiceKafkaQuotaDescribe params. Restricted "limit" query param in the queries --- client.go | 32 ++++++++++++++++----- client_test.go | 64 ++++++++++++++++++++++++++++++++++++++++++ handler/kafka/kafka.go | 22 ++++++++++++--- openapi_patch.yaml | 28 ++++++++++++++++++ 4 files changed, 135 insertions(+), 11 deletions(-) diff --git a/client.go b/client.go index 05587cf..75bc3b0 100644 --- a/client.go +++ b/client.go @@ -15,6 +15,8 @@ import ( "strings" "time" + "golang.org/x/exp/slices" + "github.com/hashicorp/go-multierror" "github.com/hashicorp/go-retryablehttp" "github.com/kelseyhightower/envconfig" @@ -139,12 +141,12 @@ func (d *aivenClient) Do(ctx context.Context, operationID, method, path string, Str("operationID", operationID). Str("method", method). Str("path", path). - Str("query", fmtQuery(query...)). + Str("query", fmtQuery(operationID, query...)). Send() }() } - rsp, err = d.do(ctx, method, path, in, query...) + rsp, err = d.do(ctx, operationID, method, path, in, query...) if err != nil { return nil, err } @@ -156,7 +158,7 @@ func (d *aivenClient) Do(ctx context.Context, operationID, method, path string, return fromResponse(operationID, rsp) } -func (d *aivenClient) do(ctx context.Context, method, path string, in any, query ...[2]string) (*http.Response, error) { +func (d *aivenClient) do(ctx context.Context, operationID, method, path string, in any, query ...[2]string) (*http.Response, error) { var body io.Reader if !(in == nil || isEmpty(in)) { @@ -176,7 +178,8 @@ func (d *aivenClient) do(ctx context.Context, method, path string, in any, query req.Header.Set("Content-Type", "application/json") req.Header.Set("User-Agent", d.UserAgent) req.Header.Set("Authorization", "aivenv1 "+d.Token) - req.URL.RawQuery = fmtQuery(query...) + req.URL.RawQuery = fmtQuery(operationID, query...) + return d.doer.Do(req) } @@ -185,18 +188,33 @@ func isEmpty(a any) bool { return v.IsZero() || v.Kind() == reflect.Ptr && v.IsNil() } -func fmtQuery(query ...[2]string) string { +func fmtQuery(operationID string, query ...[2]string) string { q := make(url.Values) + + // Add all provided query parameters for _, v := range query { q.Add(v[0], v[1]) } - if !q.Has("limit") { + // Add default limit for GET requests if conditions are met + const defaultLimit = "999" + if shouldAddDefaultLimit(operationID, q) { // TODO: BAD hack to get around pagination in most cases // we should implement this properly at some point but for now // that should be its own issue - q.Add("limit", "999") + q.Add("limit", defaultLimit) } return q.Encode() } + +// shouldAddDefaultLimit determines if default limit should be added +func shouldAddDefaultLimit(operationID string, q url.Values) bool { + var operationsWithoutLimit = []string{ + "ServiceKafkaQuotaDescribe", + "ServiceKafkaQuotaDelete", + } + + return !q.Has("limit") && + !slices.Contains(operationsWithoutLimit, operationID) +} diff --git a/client_test.go b/client_test.go index f1aed0a..9288126 100644 --- a/client_test.go +++ b/client_test.go @@ -197,3 +197,67 @@ func TestServiceCreateErrorsRetries(t *testing.T) { }) } } + +// Tests +func TestFmtQuery(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + operationID string + query [][2]string + want string + }{ + { + name: "With no params", + operationID: "TestOperation", + query: nil, + want: "limit=999", + }, + { + name: "With existing params", + operationID: "TestOperation", + query: [][2]string{ + {"foo", "bar"}, + {"baz", "qux"}, + }, + want: "baz=qux&foo=bar&limit=999", + }, + { + name: "With custom limit", + operationID: "TestOperation", + query: [][2]string{ + {"limit", "50"}, + }, + want: "limit=50", + }, + { + name: "Ignored operation", + operationID: "ServiceKafkaQuotaDescribe", + query: [][2]string{ + {"foo", "bar"}, + }, + want: "foo=bar", + }, + { + name: "Multiple parameters with same key", + operationID: "TestOperation", + query: [][2]string{ + {"tag", "v1"}, + {"tag", "v2"}, + }, + want: "limit=999&tag=v1&tag=v2", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + got := fmtQuery(tt.operationID, tt.query...) + if got != tt.want { + t.Errorf("fmtQuery() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/handler/kafka/kafka.go b/handler/kafka/kafka.go index c589d45..a917be1 100644 --- a/handler/kafka/kafka.go +++ b/handler/kafka/kafka.go @@ -55,10 +55,10 @@ type Handler interface { // https://api.aiven.io/doc/#tag/Service:_Kafka/operation/ServiceKafkaQuotaDelete ServiceKafkaQuotaDelete(ctx context.Context, project string, serviceName string, query ...[2]string) error - // ServiceKafkaQuotaDescribe describe Specific Kafka quotas + // ServiceKafkaQuotaDescribe get service quota configuration // GET /v1/project/{project}/service/{service_name}/quota/describe // https://api.aiven.io/doc/#tag/Service:_Kafka/operation/ServiceKafkaQuotaDescribe - ServiceKafkaQuotaDescribe(ctx context.Context, project string, serviceName string) (*ServiceKafkaQuotaDescribeOut, error) + ServiceKafkaQuotaDescribe(ctx context.Context, project string, serviceName string, query ...[2]string) (*ServiceKafkaQuotaDescribeOut, error) // ServiceKafkaQuotaList list Kafka quotas // GET /v1/project/{project}/service/{service_name}/quota @@ -201,9 +201,23 @@ func (h *KafkaHandler) ServiceKafkaQuotaDelete(ctx context.Context, project stri _, err := h.doer.Do(ctx, "ServiceKafkaQuotaDelete", "DELETE", path, nil, p...) return err } -func (h *KafkaHandler) ServiceKafkaQuotaDescribe(ctx context.Context, project string, serviceName string) (*ServiceKafkaQuotaDescribeOut, error) { + +// ServiceKafkaQuotaDescribeUser +func ServiceKafkaQuotaDescribeUser(user string) [2]string { + return [2]string{"user", user} +} + +// ServiceKafkaQuotaDescribeClientId +func ServiceKafkaQuotaDescribeClientId(clientId string) [2]string { + return [2]string{"client-id", clientId} +} +func (h *KafkaHandler) ServiceKafkaQuotaDescribe(ctx context.Context, project string, serviceName string, query ...[2]string) (*ServiceKafkaQuotaDescribeOut, error) { + p := make([][2]string, 0, len(query)) + for _, v := range query { + p = append(p, v) + } path := fmt.Sprintf("/v1/project/%s/service/%s/quota/describe", url.PathEscape(project), url.PathEscape(serviceName)) - b, err := h.doer.Do(ctx, "ServiceKafkaQuotaDescribe", "GET", path, nil) + b, err := h.doer.Do(ctx, "ServiceKafkaQuotaDescribe", "GET", path, nil, p...) if err != nil { return nil, err } diff --git a/openapi_patch.yaml b/openapi_patch.yaml index 357c964..f51b28f 100644 --- a/openapi_patch.yaml +++ b/openapi_patch.yaml @@ -1,3 +1,18 @@ +paths: + /project/{project}/service/{service_name}/quota/describe: + get: + summary: Get service quota configuration + description: | + Retrieves quota configuration for a specific service based on the following rules: + - When both user and client ID are specified: Returns quota for that specific combination + - When only user is specified: Returns ONLY the quota configured for this user without any client ID + - When only client ID is specified: Returns ONLY the quota configured for this client ID without any user + parameters: + - $ref: "#/components/parameters/project" + - $ref: "#/components/parameters/service_name" + - $ref: "#/components/parameters/service_kafka_quota_user" + - $ref: "#/components/parameters/service_kafka_quota_client_id" + components: schemas: ServiceDatabaseListResponse: @@ -288,3 +303,16 @@ components: items: type: string type: array + parameters: + service_kafka_quota_user: + in: query + name: user + schema: + type: string + required: false + service_kafka_quota_client_id: + in: query + name: client-id + schema: + type: string + required: false