-
Notifications
You must be signed in to change notification settings - Fork 505
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
feat: Recoverable keys feature for the dashboard #2110
Changes from 16 commits
75db25a
365117e
ab6ad58
197fe6e
b795941
e1a92c2
3379c0e
7cd8dc6
6f903de
43aa747
e89dfa3
c8d08d0
babbde4
4b74619
596b315
7ffdbc4
7028fc4
2d6d304
68c6599
ee40896
ca22641
2bcfb89
9302c62
9699b48
c46e0a0
77b5e03
ee5e94a
fce16ae
f9877c7
314ac7a
bf876bc
441a038
8125c72
050145c
0535b2b
4944c5f
3a5bf01
1545e7b
b15c192
bf1dcf7
e7f6bf1
a78085a
e8f0f5d
7da6d0d
72f9983
69942ca
ebea66b
6a35257
c739b68
c25761f
5db2d9a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,150 @@ | ||
package identities | ||
|
||
import ( | ||
"context" | ||
"fmt" | ||
"os" | ||
"testing" | ||
"time" | ||
|
||
"github.com/stretchr/testify/require" | ||
unkey "github.com/unkeyed/unkey-go" | ||
"github.com/unkeyed/unkey-go/models/components" | ||
"github.com/unkeyed/unkey-go/models/operations" | ||
attack "github.com/unkeyed/unkey/apps/agent/pkg/testutil" | ||
"github.com/unkeyed/unkey/apps/agent/pkg/uid" | ||
"github.com/unkeyed/unkey/apps/agent/pkg/util" | ||
) | ||
|
||
func TestIdentitiesRatelimitAccuracy(t *testing.T) { | ||
// Step 1 -------------------------------------------------------------------- | ||
// Setup the sdk, create an API and an identity | ||
// --------------------------------------------------------------------------- | ||
|
||
ctx := context.Background() | ||
rootKey := os.Getenv("INTEGRATION_TEST_ROOT_KEY") | ||
require.NotEmpty(t, rootKey, "INTEGRATION_TEST_ROOT_KEY must be set") | ||
baseURL := os.Getenv("UNKEY_BASE_URL") | ||
require.NotEmpty(t, baseURL, "UNKEY_BASE_URL must be set") | ||
|
||
sdk := unkey.New( | ||
unkey.WithServerURL(baseURL), | ||
unkey.WithSecurity(rootKey), | ||
) | ||
|
||
for _, nKeys := range []int{1} { //, 3, 10, 1000} { | ||
t.Run(fmt.Sprintf("with %d keys", nKeys), func(t *testing.T) { | ||
|
||
for _, tc := range []struct { | ||
rate attack.Rate | ||
testDuration time.Duration | ||
}{ | ||
{ | ||
rate: attack.Rate{Freq: 20, Per: time.Second}, | ||
testDuration: 1 * time.Minute, | ||
}, | ||
{ | ||
rate: attack.Rate{Freq: 100, Per: time.Second}, | ||
testDuration: 5 * time.Minute, | ||
}, | ||
perkinsjr marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} { | ||
t.Run(fmt.Sprintf("[%s] over %s", tc.rate.String(), tc.testDuration), func(t *testing.T) { | ||
api, err := sdk.Apis.CreateAPI(ctx, operations.CreateAPIRequestBody{ | ||
Name: uid.New("testapi"), | ||
}) | ||
require.NoError(t, err) | ||
|
||
externalId := uid.New("testuser") | ||
|
||
_, err = sdk.Identities.CreateIdentity(ctx, operations.CreateIdentityRequestBody{ | ||
ExternalID: externalId, | ||
Meta: map[string]any{ | ||
"email": "[email protected]", | ||
}, | ||
}) | ||
require.NoError(t, err) | ||
|
||
// Step 2 -------------------------------------------------------------------- | ||
// Update the identity with ratelimits | ||
// --------------------------------------------------------------------------- | ||
|
||
inferenceLimit := operations.UpdateIdentityRatelimits{ | ||
Name: "inferenceLimit", | ||
Limit: 100, | ||
Duration: time.Minute.Milliseconds(), | ||
} | ||
|
||
_, err = sdk.Identities.UpdateIdentity(ctx, operations.UpdateIdentityRequestBody{ | ||
ExternalID: unkey.String(externalId), | ||
Ratelimits: []operations.UpdateIdentityRatelimits{inferenceLimit}, | ||
}) | ||
require.NoError(t, err) | ||
|
||
// Step 4 -------------------------------------------------------------------- | ||
// Create keys that share the same identity and therefore the same ratelimits | ||
// --------------------------------------------------------------------------- | ||
|
||
keys := make([]operations.CreateKeyResponseBody, nKeys) | ||
for i := 0; i < len(keys); i++ { | ||
key, err := sdk.Keys.CreateKey(ctx, operations.CreateKeyRequestBody{ | ||
APIID: api.Object.APIID, | ||
ExternalID: unkey.String(externalId), | ||
Environment: unkey.String("integration_test"), | ||
}) | ||
require.NoError(t, err) | ||
keys[i] = *key.Object | ||
} | ||
perkinsjr marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
// Step 5 -------------------------------------------------------------------- | ||
// Test ratelimits | ||
// --------------------------------------------------------------------------- | ||
|
||
total := 0 | ||
passed := 0 | ||
|
||
results := attack.Attack(t, tc.rate, tc.testDuration, func() bool { | ||
|
||
// Each request uses one of the keys randomly | ||
key := util.RandomElement(keys).Key | ||
|
||
res, err := sdk.Keys.VerifyKey(context.Background(), components.V1KeysVerifyKeyRequest{ | ||
APIID: unkey.String(api.Object.APIID), | ||
Key: key, | ||
Ratelimits: []components.Ratelimits{ | ||
{Name: inferenceLimit.Name}, | ||
}, | ||
}) | ||
require.NoError(t, err) | ||
perkinsjr marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
return res.V1KeysVerifyKeyResponse.Valid | ||
|
||
}) | ||
|
||
for valid := range results { | ||
total++ | ||
if valid { | ||
passed++ | ||
} | ||
|
||
} | ||
|
||
// Step 6 -------------------------------------------------------------------- | ||
// Assert ratelimits worked | ||
// --------------------------------------------------------------------------- | ||
|
||
exactLimit := int(inferenceLimit.Limit) * int(tc.testDuration/(time.Duration(inferenceLimit.Duration)*time.Millisecond)) | ||
upperLimit := int(1.2 * float64(exactLimit)) | ||
lowerLimit := exactLimit | ||
if total < lowerLimit { | ||
lowerLimit = total | ||
} | ||
t.Logf("Total: %d, Passed: %d, lowerLimit: %d, exactLimit: %d, upperLimit: %d", total, passed, lowerLimit, exactLimit, upperLimit) | ||
|
||
// check requests::api is not exceeded | ||
require.GreaterOrEqual(t, passed, lowerLimit) | ||
require.LessOrEqual(t, passed, upperLimit) | ||
perkinsjr marked this conversation as resolved.
Show resolved
Hide resolved
|
||
}) | ||
} | ||
}) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -28,14 +28,10 @@ func TestClusterRatelimitAccuracy(t *testing.T) { | |
baseURL := os.Getenv("UNKEY_BASE_URL") | ||
require.NotEmpty(t, baseURL, "UNKEY_BASE_URL must be set") | ||
|
||
options := []unkey.SDKOption{ | ||
sdk := unkey.New( | ||
unkey.WithServerURL(baseURL), | ||
unkey.WithSecurity(rootKey), | ||
} | ||
|
||
if baseURL != "" { | ||
options = append(options, unkey.WithServerURL(baseURL)) | ||
} | ||
sdk := unkey.New(options...) | ||
) | ||
|
||
api, err := sdk.Apis.CreateAPI(ctx, operations.CreateAPIRequestBody{ | ||
Name: uid.New("testapi"), | ||
|
@@ -47,7 +43,7 @@ func TestClusterRatelimitAccuracy(t *testing.T) { | |
_, err = sdk.Identities.CreateIdentity(ctx, operations.CreateIdentityRequestBody{ | ||
ExternalID: externalId, | ||
Meta: map[string]any{ | ||
"email": "[email protected]", | ||
"email": "[email protected]", | ||
}, | ||
}) | ||
require.NoError(t, err) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,125 @@ | ||
package keys_test | ||
|
||
import ( | ||
"context" | ||
"fmt" | ||
"os" | ||
"testing" | ||
"time" | ||
|
||
"github.com/stretchr/testify/require" | ||
unkey "github.com/unkeyed/unkey-go" | ||
"github.com/unkeyed/unkey-go/models/components" | ||
"github.com/unkeyed/unkey-go/models/operations" | ||
attack "github.com/unkeyed/unkey/apps/agent/pkg/testutil" | ||
"github.com/unkeyed/unkey/apps/agent/pkg/uid" | ||
"github.com/unkeyed/unkey/apps/agent/pkg/util" | ||
) | ||
|
||
func TestDefaultRatelimitAccuracy(t *testing.T) { | ||
// Step 1 -------------------------------------------------------------------- | ||
// Setup the sdk, create an API and a key | ||
// --------------------------------------------------------------------------- | ||
|
||
ctx := context.Background() | ||
rootKey := os.Getenv("INTEGRATION_TEST_ROOT_KEY") | ||
require.NotEmpty(t, rootKey, "INTEGRATION_TEST_ROOT_KEY must be set") | ||
baseURL := os.Getenv("UNKEY_BASE_URL") | ||
require.NotEmpty(t, baseURL, "UNKEY_BASE_URL must be set") | ||
perkinsjr marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
options := []unkey.SDKOption{ | ||
unkey.WithSecurity(rootKey), | ||
} | ||
|
||
if baseURL != "" { | ||
options = append(options, unkey.WithServerURL(baseURL)) | ||
} | ||
sdk := unkey.New(options...) | ||
|
||
for _, tc := range []struct { | ||
rate attack.Rate | ||
testDuration time.Duration | ||
}{ | ||
{ | ||
rate: attack.Rate{Freq: 20, Per: time.Second}, | ||
testDuration: 1 * time.Minute, | ||
}, | ||
{ | ||
rate: attack.Rate{Freq: 100, Per: time.Second}, | ||
testDuration: 5 * time.Minute, | ||
}, | ||
} { | ||
t.Run(fmt.Sprintf("[%s] over %s", tc.rate.String(), tc.testDuration), func(t *testing.T) { | ||
api, err := sdk.Apis.CreateAPI(ctx, operations.CreateAPIRequestBody{ | ||
Name: uid.New("testapi"), | ||
}) | ||
require.NoError(t, err) | ||
|
||
// Step 2 -------------------------------------------------------------------- | ||
// Update the identity with ratelimits | ||
// --------------------------------------------------------------------------- | ||
|
||
// Step 3 -------------------------------------------------------------------- | ||
// Create keys that share the same identity and therefore the same ratelimits | ||
// --------------------------------------------------------------------------- | ||
|
||
ratelimit := operations.Ratelimit{ | ||
Limit: 100, | ||
Duration: util.Pointer(time.Minute.Milliseconds()), | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hardcoded rate limit values reduce test flexibility The rate limit applied to the key is hardcoded and does not vary with test cases. Currently, the ratelimit := operations.Ratelimit{
Limit: 100,
Duration: util.Pointer(time.Minute.Milliseconds()),
} Consider parameterizing the - ratelimit := operations.Ratelimit{
- Limit: 100,
- Duration: util.Pointer(time.Minute.Milliseconds()),
- }
+ ratelimit := operations.Ratelimit{
+ Limit: int64(tc.rate.Freq * int(tc.rate.Per.Seconds())),
+ Duration: util.Pointer(tc.rate.Per.Milliseconds()),
+ } This change allows each test case to define its own rate limit, aligning with the |
||
|
||
key, err := sdk.Keys.CreateKey(ctx, operations.CreateKeyRequestBody{ | ||
APIID: api.Object.APIID, | ||
Ratelimit: &ratelimit, | ||
}) | ||
require.NoError(t, err) | ||
|
||
// Step 5 -------------------------------------------------------------------- | ||
// Test ratelimits | ||
// --------------------------------------------------------------------------- | ||
|
||
total := 0 | ||
passed := 0 | ||
|
||
results := attack.Attack(t, tc.rate, tc.testDuration, func() bool { | ||
|
||
res, err := sdk.Keys.VerifyKey(context.Background(), components.V1KeysVerifyKeyRequest{ | ||
APIID: unkey.String(api.Object.APIID), | ||
Key: key.Object.Key, | ||
Ratelimits: []components.Ratelimits{ | ||
{Name: "default"}, | ||
}, | ||
}) | ||
require.NoError(t, err) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Potential misuse of Using The Consider capturing the error and returning it to the main test function for proper assertion. -res, err := sdk.Keys.VerifyKey(context.Background(), components.V1KeysVerifyKeyRequest{
+res, err := sdk.Keys.VerifyKey(ctx, components.V1KeysVerifyKeyRequest{
APIID: unkey.String(api.Object.APIID),
Key: key.Object.Key,
Ratelimits: []components.Ratelimits{
{Name: "default"},
},
})
-require.NoError(t, err)
+if err != nil {
+ t.Errorf("VerifyKey failed: %v", err)
+ return false
+} Alternatively, ensure that all assertions within the function are safe to use in concurrent contexts or adjust
|
||
|
||
return res.V1KeysVerifyKeyResponse.Valid | ||
|
||
}) | ||
perkinsjr marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
for valid := range results { | ||
total++ | ||
if valid { | ||
passed++ | ||
} | ||
|
||
} | ||
|
||
// Step 6 -------------------------------------------------------------------- | ||
// Assert ratelimits worked | ||
// --------------------------------------------------------------------------- | ||
|
||
exactLimit := int(ratelimit.Limit) * int(tc.testDuration/(time.Duration(*ratelimit.Duration)*time.Millisecond)) | ||
upperLimit := int(1.2 * float64(exactLimit)) | ||
lowerLimit := exactLimit | ||
if total < lowerLimit { | ||
lowerLimit = total | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Logical error in Adjusting At lines 114-116: if total < lowerLimit {
lowerLimit = total
} This adjustment allows For example, you might want to remove the adjustment or revise it based on the test's intent. |
||
t.Logf("Total: %d, Passed: %d, lowerLimit: %d, exactLimit: %d, upperLimit: %d", total, passed, lowerLimit, exactLimit, upperLimit) | ||
|
||
// check requests::api is not exceeded | ||
require.GreaterOrEqual(t, passed, lowerLimit) | ||
require.LessOrEqual(t, passed, upperLimit) | ||
}) | ||
|
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,69 @@ | ||
package attack | ||
|
||
import ( | ||
"fmt" | ||
"sync" | ||
"testing" | ||
"time" | ||
) | ||
|
||
type Rate struct { | ||
Freq int | ||
Per time.Duration | ||
} | ||
|
||
func (r Rate) String() string { | ||
return fmt.Sprintf("%d per %s", r.Freq, r.Per) | ||
} | ||
|
||
// Attack executes the given function at the given rate for the given duration | ||
// and returns a channel on which the results are sent. | ||
// | ||
// The caller must process the results as they arrive on the channel to avoid | ||
// blocking the worker goroutines. | ||
func Attack[Response any](t *testing.T, rate Rate, duration time.Duration, fn func() Response) <-chan Response { | ||
t.Log("attacking") | ||
wg := sync.WaitGroup{} | ||
workers := 256 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider making Currently, the number of worker goroutines is hardcoded to workers := 256 To make the function more flexible and adaptable to different testing environments, consider making workers := runtime.NumCPU() Don't forget to import the import (
"fmt"
"runtime"
"sync"
"testing"
"time"
) |
||
|
||
ticks := make(chan struct{}) | ||
responses := make(chan Response) | ||
|
||
totalRequests := rate.Freq * int(duration/rate.Per) | ||
dt := rate.Per / time.Duration(rate.Freq) | ||
|
||
wg.Add(totalRequests) | ||
|
||
go func() { | ||
for i := 0; i < totalRequests; i++ { | ||
ticks <- struct{}{} | ||
time.Sleep(dt) | ||
} | ||
}() | ||
|
||
for i := 0; i < workers; i++ { | ||
go func() { | ||
for range ticks { | ||
responses <- fn() | ||
wg.Done() | ||
|
||
} | ||
}() | ||
} | ||
|
||
go func() { | ||
wg.Wait() | ||
t.Log("attack done, waiting for responses to be processed") | ||
|
||
close(ticks) | ||
pending := len(responses) | ||
for pending > 0 { | ||
t.Logf("waiting for responses to be processed: %d", pending) | ||
time.Sleep(100 * time.Millisecond) | ||
} | ||
close(responses) | ||
perkinsjr marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
}() | ||
|
||
return responses | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update import paths to use module references instead of relative paths
The import paths for internal packages use relative paths, which may cause issues with module resolution. Consider updating them to use module references.
Apply this diff to update the import paths:
Ensure that the module path
unkey
is correctly specified in yourgo.mod
file.Committable suggestion