Skip to content

Commit

Permalink
fix: patched ServiceKafkaQuotaDescribe parameters. Limited the "limit…
Browse files Browse the repository at this point in the history
…" query param that added to the queries.
  • Loading branch information
vmyroslav committed Dec 18, 2024
1 parent cdf5bca commit 07eab33
Show file tree
Hide file tree
Showing 4 changed files with 164 additions and 12 deletions.
37 changes: 29 additions & 8 deletions client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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, method, 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
}
Expand All @@ -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)) {
Expand All @@ -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, method, query...)

return d.doer.Do(req)
}

Expand All @@ -185,18 +188,36 @@ func isEmpty(a any) bool {
return v.IsZero() || v.Kind() == reflect.Ptr && v.IsNil()
}

func fmtQuery(query ...[2]string) string {
func fmtQuery(operationID, method 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 v[0] != "" { // Skip empty keys
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, method, 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, method string, q url.Values) bool {
var operationsWithoutLimit = []string{
"ServiceKafkaQuotaDescribe",
"ServiceKafkaQuotaDelete",
}

return !q.Has("limit") &&
method == http.MethodGet &&
!slices.Contains(operationsWithoutLimit, operationID)
}
89 changes: 89 additions & 0 deletions client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -197,3 +197,92 @@ func TestServiceCreateErrorsRetries(t *testing.T) {
})
}
}

// Tests
func TestFmtQuery(t *testing.T) {
t.Parallel()

tests := []struct {
name string
operationID string
method string
query [][2]string
want string
}{
{
name: "Basic GET with no params",
operationID: "TestOperation",
method: http.MethodGet,
query: nil,
want: "limit=999",
},
{
name: "GET with existing params",
operationID: "TestOperation",
method: http.MethodGet,
query: [][2]string{
{"foo", "bar"},
{"baz", "qux"},
},
want: "baz=qux&foo=bar&limit=999",
},
{
name: "GET with custom limit",
operationID: "TestOperation",
method: http.MethodGet,
query: [][2]string{
{"limit", "50"},
},
want: "limit=50",
},
{
name: "Ignored operation",
operationID: "ServiceKafkaQuotaDescribe",
method: http.MethodGet,
query: [][2]string{
{"foo", "bar"},
},
want: "foo=bar",
},
{
name: "POST request",
operationID: "TestOperation",
method: http.MethodPost,
query: [][2]string{
{"foo", "bar"},
},
want: "foo=bar",
},
{
name: "Empty param key is skipped",
operationID: "TestOperation",
method: http.MethodGet,
query: [][2]string{
{"", "value"},
{"valid", "param"},
},
want: "limit=999&valid=param",
},
{
name: "Multiple parameters with same key",
operationID: "TestOperation",
method: http.MethodGet,
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.method, tt.query...)
if got != tt.want {
t.Errorf("fmtQuery() = %v, want %v", got, tt.want)
}
})
}
}
22 changes: 18 additions & 4 deletions handler/kafka/kafka.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

28 changes: 28 additions & 0 deletions openapi_patch.yaml
Original file line number Diff line number Diff line change
@@ -1,3 +1,18 @@
paths:

Check failure on line 1 in openapi_patch.yaml

View workflow job for this annotation

GitHub Actions / Trunk Check

prettier

Incorrect formatting, autoformat by running 'trunk fmt'
/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:
Expand Down Expand Up @@ -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

0 comments on commit 07eab33

Please sign in to comment.