From 772b7efac992c8f33dc971515dea21af4c7a0ec6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sandro=20J=C3=A4ckel?= Date: Mon, 25 Sep 2023 15:30:34 +0200 Subject: [PATCH 1/3] Extract and reuse deterministicDummyDigest --- internal/api/keppel/manifests_test.go | 46 ++++++++++++------------- internal/api/keppel/quotas_test.go | 4 +-- internal/api/keppel/repos_test.go | 10 ++---- internal/api/registry/blobs_test.go | 10 +++--- internal/api/registry/manifests_test.go | 4 +-- internal/api/registry/ratelimit_test.go | 5 ++- internal/api/registry/shared_test.go | 7 ---- internal/api/registry/tags_test.go | 4 +-- internal/test/content.go | 4 +++ 9 files changed, 41 insertions(+), 53 deletions(-) diff --git a/internal/api/keppel/manifests_test.go b/internal/api/keppel/manifests_test.go index cfa4fdd5e..b06c5a916 100644 --- a/internal/api/keppel/manifests_test.go +++ b/internal/api/keppel/manifests_test.go @@ -106,7 +106,7 @@ func TestManifestsAPI(t *testing.T) { repo := repos[repoID-1] for idx := 1; idx <= 10; idx++ { - dummyDigest := deterministicDummyDigest(repoID*10 + idx) + dummyDigest := test.DeterministicDummyDigest(repoID*10 + idx) sizeBytes := uint64(1000 * idx) pushedAt := time.Unix(int64(1000*(repoID*10+idx)), 0) @@ -145,21 +145,21 @@ func TestManifestsAPI(t *testing.T) { mustInsert(t, s.DB, &keppel.Tag{ RepositoryID: int64(repoID), Name: "first", - Digest: deterministicDummyDigest(repoID*10 + 1), + Digest: test.DeterministicDummyDigest(repoID*10 + 1), PushedAt: time.Unix(20001, 0), LastPulledAt: p2time(time.Unix(20101, 0)), }) mustInsert(t, s.DB, &keppel.Tag{ RepositoryID: int64(repoID), Name: "stillfirst", - Digest: deterministicDummyDigest(repoID*10 + 1), + Digest: test.DeterministicDummyDigest(repoID*10 + 1), PushedAt: time.Unix(20002, 0), LastPulledAt: nil, }) mustInsert(t, s.DB, &keppel.Tag{ RepositoryID: int64(repoID), Name: "second", - Digest: deterministicDummyDigest(repoID*10 + 2), + Digest: test.DeterministicDummyDigest(repoID*10 + 2), PushedAt: time.Unix(20003, 0), LastPulledAt: nil, }) @@ -170,7 +170,7 @@ func TestManifestsAPI(t *testing.T) { renderedManifests := make([]assert.JSONObject, 10) for idx := 1; idx <= 10; idx++ { renderedManifests[idx-1] = assert.JSONObject{ - "digest": deterministicDummyDigest(10 + idx), + "digest": test.DeterministicDummyDigest(10 + idx), "media_type": schema2.MediaTypeManifest, "size_bytes": uint64(1000 * idx), "pushed_at": int64(1000 * (10 + idx)), @@ -270,14 +270,14 @@ func TestManifestsAPI(t *testing.T) { easypg.AssertDBContent(t, s.DB.DbMap.Db, "fixtures/before-delete-manifest.sql") assert.HTTPRequest{ Method: "DELETE", - Path: "/keppel/v1/accounts/test1/repositories/repo1-1/_manifests/" + deterministicDummyDigest(11).String(), + Path: "/keppel/v1/accounts/test1/repositories/repo1-1/_manifests/" + test.DeterministicDummyDigest(11).String(), Header: map[string]string{"X-Test-Perms": "view:tenant1,delete:tenant1"}, ExpectStatus: http.StatusNoContent, }.Check(t, h) easypg.AssertDBContent(t, s.DB.DbMap.Db, "fixtures/after-delete-manifest.sql") s.Auditor.ExpectEvents(t, cadf.Event{ - RequestPath: "/keppel/v1/accounts/test1/repositories/repo1-1/_manifests/" + deterministicDummyDigest(11).String(), + RequestPath: "/keppel/v1/accounts/test1/repositories/repo1-1/_manifests/" + test.DeterministicDummyDigest(11).String(), Action: cadf.DeleteAction, Outcome: "success", Reason: test.CADFReasonOK, @@ -288,8 +288,8 @@ func TestManifestsAPI(t *testing.T) { Content: "[\"first\",\"stillfirst\"]", }}, TypeURI: "docker-registry/account/repository/manifest", - Name: "test1/repo1-1@" + deterministicDummyDigest(11).String(), - ID: deterministicDummyDigest(11).String(), + Name: "test1/repo1-1@" + test.DeterministicDummyDigest(11).String(), + ID: test.DeterministicDummyDigest(11).String(), ProjectID: "tenant1", }, }) @@ -311,7 +311,7 @@ func TestManifestsAPI(t *testing.T) { Target: cadf.Resource{ TypeURI: "docker-registry/account/repository/tag", Name: "test1/repo1-2:stillfirst", - ID: deterministicDummyDigest(21).String(), + ID: test.DeterministicDummyDigest(21).String(), ProjectID: "tenant1", }, }) @@ -319,33 +319,33 @@ func TestManifestsAPI(t *testing.T) { //test DELETE manifest failure cases assert.HTTPRequest{ Method: "DELETE", - Path: "/keppel/v1/accounts/test2/repositories/repo2-1/_manifests/" + deterministicDummyDigest(31).String(), + Path: "/keppel/v1/accounts/test2/repositories/repo2-1/_manifests/" + test.DeterministicDummyDigest(31).String(), Header: map[string]string{"X-Test-Perms": "delete:tenant1,view:tenant1,pull:tenant1"}, ExpectStatus: http.StatusForbidden, ExpectBody: assert.StringData("no permission for repository:test2/repo2-1:delete\n"), }.Check(t, h) assert.HTTPRequest{ Method: "DELETE", - Path: "/keppel/v1/accounts/test1/repositories/repo1-2/_manifests/" + deterministicDummyDigest(21).String(), + Path: "/keppel/v1/accounts/test1/repositories/repo1-2/_manifests/" + test.DeterministicDummyDigest(21).String(), Header: map[string]string{"X-Test-Perms": "view:tenant1,pull:tenant1"}, ExpectStatus: http.StatusForbidden, }.Check(t, h) assert.HTTPRequest{ Method: "DELETE", - Path: "/keppel/v1/accounts/doesnotexist/repositories/repo1-2/_manifests/" + deterministicDummyDigest(11).String(), + Path: "/keppel/v1/accounts/doesnotexist/repositories/repo1-2/_manifests/" + test.DeterministicDummyDigest(11).String(), Header: map[string]string{"X-Test-Perms": "delete:tenant1,view:tenant1,pull:tenant1"}, ExpectStatus: http.StatusForbidden, ExpectBody: assert.StringData("no permission for repository:doesnotexist/repo1-2:delete\n"), }.Check(t, h) assert.HTTPRequest{ Method: "DELETE", - Path: "/keppel/v1/accounts/test1/repositories/doesnotexist/_manifests/" + deterministicDummyDigest(11).String(), + Path: "/keppel/v1/accounts/test1/repositories/doesnotexist/_manifests/" + test.DeterministicDummyDigest(11).String(), Header: map[string]string{"X-Test-Perms": "delete:tenant1,view:tenant1,pull:tenant1"}, ExpectStatus: http.StatusNotFound, }.Check(t, h) assert.HTTPRequest{ Method: "DELETE", - Path: "/keppel/v1/accounts/test1/repositories/repo1-1/_manifests/" + deterministicDummyDigest(11).String(), + Path: "/keppel/v1/accounts/test1/repositories/repo1-1/_manifests/" + test.DeterministicDummyDigest(11).String(), Header: map[string]string{"X-Test-Perms": "view:tenant1,delete:tenant1"}, ExpectStatus: http.StatusNotFound, }.Check(t, h) @@ -371,7 +371,7 @@ func TestManifestsAPI(t *testing.T) { }.Check(t, h) assert.HTTPRequest{ Method: "DELETE", - Path: "/keppel/v1/accounts/test2/repositories/repo2-1/_tags/" + deterministicDummyDigest(31).String(), //this endpoint only works with tags + Path: "/keppel/v1/accounts/test2/repositories/repo2-1/_tags/" + test.DeterministicDummyDigest(31).String(), //this endpoint only works with tags Header: map[string]string{"X-Test-Perms": "delete:tenant2,view:tenant2"}, ExpectStatus: http.StatusNotFound, }.Check(t, h) @@ -391,13 +391,13 @@ func TestManifestsAPI(t *testing.T) { //test GET vulnerability report failure cases assert.HTTPRequest{ Method: "GET", - Path: "/keppel/v1/accounts/test1/repositories/repo1-1/_manifests/" + deterministicDummyDigest(11).String() + "/trivy_report", + Path: "/keppel/v1/accounts/test1/repositories/repo1-1/_manifests/" + test.DeterministicDummyDigest(11).String() + "/trivy_report", Header: map[string]string{"X-Test-Perms": "view:tenant1,pull:tenant1"}, ExpectStatus: http.StatusNotFound, //this manifest was deleted above }.Check(t, h) assert.HTTPRequest{ Method: "GET", - Path: "/keppel/v1/accounts/test1/repositories/repo1-1/_manifests/" + deterministicDummyDigest(12).String() + "/trivy_report", + Path: "/keppel/v1/accounts/test1/repositories/repo1-1/_manifests/" + test.DeterministicDummyDigest(12).String() + "/trivy_report", Header: map[string]string{"X-Test-Perms": "view:tenant1,pull:tenant1"}, ExpectStatus: http.StatusMethodNotAllowed, //manifest cannot have vulnerability report because it does not have manifest-blob refs }.Check(t, h) @@ -406,7 +406,7 @@ func TestManifestsAPI(t *testing.T) { //so that the vulnerability report can actually be shown dummyBlob := keppel.Blob{ AccountName: "test1", - Digest: deterministicDummyDigest(101), + Digest: test.DeterministicDummyDigest(101), } mustInsert(t, s.DB, &dummyBlob) err := keppel.MountBlobIntoRepo(s.DB, dummyBlob, *repos[0]) @@ -415,21 +415,21 @@ func TestManifestsAPI(t *testing.T) { } _, err = s.DB.Exec( `INSERT INTO manifest_blob_refs (repo_id, digest, blob_id) VALUES ($1, $2, $3)`, - repos[0].ID, deterministicDummyDigest(12), dummyBlob.ID, + repos[0].ID, test.DeterministicDummyDigest(12), dummyBlob.ID, ) if err != nil { t.Fatal(err.Error()) } //test GET vulnerability report success case - imageRef, _, err := models.ParseImageReference("registry.example.org/test1/repo1-1@" + deterministicDummyDigest(12).String()) + imageRef, _, err := models.ParseImageReference("registry.example.org/test1/repo1-1@" + test.DeterministicDummyDigest(12).String()) if err != nil { t.Fatal(err.Error()) } s.TrivyDouble.ReportFixtures[imageRef] = "../../tasks/fixtures/trivy/report-vulnerable-with-fixes.json" assert.HTTPRequest{ Method: "GET", - Path: "/keppel/v1/accounts/test1/repositories/repo1-1/_manifests/" + deterministicDummyDigest(12).String() + "/trivy_report", + Path: "/keppel/v1/accounts/test1/repositories/repo1-1/_manifests/" + test.DeterministicDummyDigest(12).String() + "/trivy_report", Header: map[string]string{"X-Test-Perms": "view:tenant1,pull:tenant1"}, ExpectStatus: http.StatusOK, ExpectBody: assert.JSONFixtureFile("../../tasks/fixtures/trivy/report-vulnerable-with-fixes.json"), @@ -460,7 +460,7 @@ func TestManifestsAPI(t *testing.T) { assert.HTTPRequest{ Method: "GET", - Path: "/keppel/v1/accounts/test1/repositories/repo1-1/_manifests/" + deterministicDummyDigest(12).String() + "/trivy_report", + Path: "/keppel/v1/accounts/test1/repositories/repo1-1/_manifests/" + test.DeterministicDummyDigest(12).String() + "/trivy_report", Header: map[string]string{"X-Test-Perms": "view:tenant1,pull:tenant1"}, ExpectStatus: http.StatusOK, ExpectBody: assert.JSONFixtureFile("../../tasks/fixtures/trivy/report-vulnerable-with-fixes-enriched.json"), diff --git a/internal/api/keppel/quotas_test.go b/internal/api/keppel/quotas_test.go index 5ae053e04..a553dfe1e 100644 --- a/internal/api/keppel/quotas_test.go +++ b/internal/api/keppel/quotas_test.go @@ -131,7 +131,7 @@ func TestQuotasAPI(t *testing.T) { pushedAt := time.Unix(int64(10000+10*idx), 0) mustInsert(t, s.DB, &keppel.Manifest{ RepositoryID: 1, - Digest: deterministicDummyDigest(idx), + Digest: test.DeterministicDummyDigest(idx), MediaType: "", SizeBytes: uint64(1000 * idx), PushedAt: pushedAt, @@ -139,7 +139,7 @@ func TestQuotasAPI(t *testing.T) { }) mustInsert(t, s.DB, &keppel.TrivySecurityInfo{ RepositoryID: 1, - Digest: deterministicDummyDigest(idx), + Digest: test.DeterministicDummyDigest(idx), VulnerabilityStatus: trivy.PendingVulnerabilityStatus, NextCheckAt: time.Unix(0, 0), }) diff --git a/internal/api/keppel/repos_test.go b/internal/api/keppel/repos_test.go index 14b92722b..4d886e5dc 100644 --- a/internal/api/keppel/repos_test.go +++ b/internal/api/keppel/repos_test.go @@ -19,13 +19,11 @@ package keppelv1_test import ( - "bytes" "fmt" "net/http" "testing" "time" - "github.com/opencontainers/go-digest" "github.com/sapcc/go-bits/assert" "github.com/sapcc/go-bits/easypg" @@ -57,10 +55,6 @@ func mustExec(t *testing.T, db *keppel.DB, query string, args ...interface{}) { } } -func deterministicDummyDigest(counter int) digest.Digest { - return digest.SHA256.FromBytes(bytes.Repeat([]byte{1}, counter)) -} - func TestReposAPI(t *testing.T) { s := test.NewSetup(t, test.WithKeppelAPI) h := s.Handler @@ -107,7 +101,7 @@ func TestReposAPI(t *testing.T) { //blob size statistics filledRepo := keppel.Repository{ID: 5} //repo1-3 for idx := 1; idx <= 10; idx++ { - dummyDigest := deterministicDummyDigest(1000 + idx) + dummyDigest := test.DeterministicDummyDigest(1000 + idx) blobPushedAt := time.Unix(int64(1000+10*idx), 0) blob := keppel.Blob{ AccountName: "test1", @@ -126,7 +120,7 @@ func TestReposAPI(t *testing.T) { //insert some dummy manifests and tags into one of the repos to check the //manifest/tag counting for idx := 1; idx <= 10; idx++ { - dummyDigest := deterministicDummyDigest(idx) + dummyDigest := test.DeterministicDummyDigest(idx) manifestPushedAt := time.Unix(int64(10000+10*idx), 0) mustInsert(t, s.DB, &keppel.Manifest{ RepositoryID: filledRepo.ID, diff --git a/internal/api/registry/blobs_test.go b/internal/api/registry/blobs_test.go index 02dee7d4f..8562468c1 100644 --- a/internal/api/registry/blobs_test.go +++ b/internal/api/registry/blobs_test.go @@ -76,7 +76,7 @@ func TestBlobMonolithicUpload(t *testing.T) { }) //test failure cases: digest is wrong - for _, wrongDigest := range []string{"wrong", "sha256:" + sha256Of([]byte("something else"))} { + for _, wrongDigest := range []string{"wrong", test.DeterministicDummyDigest(1).String()} { assert.HTTPRequest{ Method: "POST", Path: "/v2/test1/foo/blobs/uploads/?digest=" + wrongDigest, @@ -408,7 +408,7 @@ func TestBlobStreamedAndChunkedUpload(t *testing.T) { } //test failure cases during PUT: digest is missing or wrong - for _, wrongDigest := range []string{"", "wrong", "sha256:" + sha256Of([]byte("something else"))} { + for _, wrongDigest := range []string{"", "wrong", test.DeterministicDummyDigest(2).String()} { //upload all the blob contents at once (we're only interested in the final PUT) resp, _ := assert.HTTPRequest{ Method: "PATCH", @@ -793,10 +793,9 @@ func TestDeleteBlob(t *testing.T) { }.Check(t, h) //test failure case: no such blob - bogusDigest := "sha256:" + sha256Of([]byte("something else")) assert.HTTPRequest{ Method: "DELETE", - Path: "/v2/test1/foo/blobs/" + bogusDigest, + Path: "/v2/test1/foo/blobs/" + test.DeterministicDummyDigest(1).String(), Header: map[string]string{"Authorization": "Bearer " + deleteToken}, ExpectStatus: http.StatusNotFound, ExpectHeader: test.VersionHeader, @@ -892,7 +891,6 @@ func TestCrossRepositoryBlobMount(t *testing.T) { }.Check(t, h) //test failure cases: digest is malformed or wrong - bogusDigest := "sha256:" + sha256Of([]byte("something else")) assert.HTTPRequest{ Method: "POST", Path: "/v2/test1/foo/blobs/uploads/?from=test1/bar&mount=wrong", @@ -903,7 +901,7 @@ func TestCrossRepositoryBlobMount(t *testing.T) { }.Check(t, h) assert.HTTPRequest{ Method: "POST", - Path: "/v2/test1/foo/blobs/uploads/?from=test1/bar&mount=" + bogusDigest, + Path: "/v2/test1/foo/blobs/uploads/?from=test1/bar&mount=" + test.DeterministicDummyDigest(1).String(), Header: map[string]string{"Authorization": "Bearer " + token}, ExpectStatus: http.StatusNotFound, ExpectHeader: test.VersionHeader, diff --git a/internal/api/registry/manifests_test.go b/internal/api/registry/manifests_test.go index 7bb9db2cf..df8b8dc61 100644 --- a/internal/api/registry/manifests_test.go +++ b/internal/api/registry/manifests_test.go @@ -130,7 +130,7 @@ func TestImageManifestLifecycle(t *testing.T) { //PUT failure case: wrong digest assert.HTTPRequest{ Method: "PUT", - Path: "/v2/test1/foo/manifests/sha256:" + sha256Of([]byte("something else")), + Path: "/v2/test1/foo/manifests/" + test.DeterministicDummyDigest(1).String(), Header: map[string]string{ "Authorization": "Bearer " + token, "Content-Type": image.Manifest.MediaType, @@ -363,7 +363,7 @@ func TestImageManifestLifecycle(t *testing.T) { //DELETE failure case: unknown manifest assert.HTTPRequest{ Method: "DELETE", - Path: "/v2/test1/foo/manifests/sha256:" + sha256Of([]byte("something else")), + Path: "/v2/test1/foo/manifests/" + test.DeterministicDummyDigest(1).String(), Header: map[string]string{"Authorization": "Bearer " + deleteToken}, ExpectStatus: http.StatusNotFound, ExpectHeader: test.VersionHeader, diff --git a/internal/api/registry/ratelimit_test.go b/internal/api/registry/ratelimit_test.go index d670ea007..503b5daaa 100644 --- a/internal/api/registry/ratelimit_test.go +++ b/internal/api/registry/ratelimit_test.go @@ -60,7 +60,7 @@ func TestRateLimits(t *testing.T) { h := s.Handler token := s.GetToken(t, "repository:test1/foo:pull,push") - bogusDigest := "sha256:" + sha256Of([]byte("something else")) + bogusDigest := test.DeterministicDummyDigest(1).String() //prepare some test requests that should be affected by rate limiting //(some of these fail with 404 or 400, but that's okay; the important part is @@ -102,8 +102,7 @@ func TestRateLimits(t *testing.T) { for _, req := range testRequests { s.Clock.StepBy(time.Hour) - //we can always execute 1 request initially, and then we can burst on top - //of that + //we can always execute 1 request initially, and then we can burst on top of that for i := 0; i < limit.Burst; i++ { req.Check(t, h) s.Clock.StepBy(time.Second) diff --git a/internal/api/registry/shared_test.go b/internal/api/registry/shared_test.go index b5d9880f5..eb1902ba2 100644 --- a/internal/api/registry/shared_test.go +++ b/internal/api/registry/shared_test.go @@ -282,10 +282,3 @@ func testWithAccountInMaintenance(t *testing.T, db *keppel.DB, accountName strin t.Fatal(err.Error()) } } - -//////////////////////////////////////////////////////////////////////////////// - -func sha256Of(data []byte) string { - sha256Hash := sha256.Sum256(data) - return hex.EncodeToString(sha256Hash[:]) -} diff --git a/internal/api/registry/tags_test.go b/internal/api/registry/tags_test.go index 1046583d5..5a0b24e95 100644 --- a/internal/api/registry/tags_test.go +++ b/internal/api/registry/tags_test.go @@ -23,7 +23,6 @@ import ( "math/rand" "net/http" "sort" - "strconv" "strings" "testing" @@ -71,8 +70,9 @@ func TestListTags(t *testing.T) { //generate pseudo-random, but deterministic tag names allTagNames := make([]string, 10) + sidGen := test.StorageIDGenerator{} for idx := range allTagNames { - allTagNames[idx] = sha256Of([]byte(strconv.Itoa(idx))) + allTagNames[idx] = sidGen.Next() } //upload test image under all of them (in randomized order!) diff --git a/internal/test/content.go b/internal/test/content.go index b6940529a..054a1ac08 100644 --- a/internal/test/content.go +++ b/internal/test/content.go @@ -324,3 +324,7 @@ func (l ImageList) ImageRef(s Setup, repo keppel.Repository) models.ImageReferen func makeTimestamp(seconds int) string { return time.Unix(int64(seconds), 0).UTC().Format(time.RFC3339Nano) } + +func DeterministicDummyDigest(counter int) digest.Digest { + return digest.SHA256.FromBytes(bytes.Repeat([]byte{1}, counter)) +} From ee21b555a3726e65c2777ea7dfc07cd37166bee4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sandro=20J=C3=A4ckel?= Date: Mon, 25 Sep 2023 15:31:24 +0200 Subject: [PATCH 2/3] Make testWithPrimary extensible --- internal/api/registry/ratelimit_test.go | 10 ++++++++-- internal/api/registry/replication_test.go | 6 +++++- internal/api/registry/shared_test.go | 9 +++------ 3 files changed, 16 insertions(+), 9 deletions(-) diff --git a/internal/api/registry/ratelimit_test.go b/internal/api/registry/ratelimit_test.go index 503b5daaa..e7be4dfd3 100644 --- a/internal/api/registry/ratelimit_test.go +++ b/internal/api/registry/ratelimit_test.go @@ -45,8 +45,11 @@ func TestRateLimits(t *testing.T) { }, } rle := &keppel.RateLimitEngine{Driver: rld, Client: nil} + setupOptions := []test.SetupOption{ + test.WithRateLimitEngine(rle), + } - testWithPrimary(t, rle, func(s test.Setup) { + testWithPrimary(t, setupOptions, func(s test.Setup) { sr := miniredis.RunT(t) s.Clock.AddListener(sr.SetTime) rle.Client = redis.NewClient(&redis.Options{Addr: sr.Addr()}) @@ -148,8 +151,11 @@ func TestAnycastRateLimits(t *testing.T) { }, } rle := &keppel.RateLimitEngine{Driver: rld, Client: nil} + setupOptions := []test.SetupOption{ + test.WithRateLimitEngine(rle), + } - testWithPrimary(t, rle, func(s test.Setup) { + testWithPrimary(t, setupOptions, func(s test.Setup) { if !currentlyWithAnycast { return } diff --git a/internal/api/registry/replication_test.go b/internal/api/registry/replication_test.go index 76c742cc9..f2aba75bf 100644 --- a/internal/api/registry/replication_test.go +++ b/internal/api/registry/replication_test.go @@ -482,7 +482,11 @@ func TestReplicationFailingOverIntoPullDelegation(t *testing.T) { //- "test1" is reconfigured into an external replica of tertiary on both primary and secondary. //- Tertiary rejects the first pull to trigger the pull delegation code path. - testWithPrimary(t, nil, func(s1 test.Setup) { + setupOptions := []test.SetupOption{ + test.WithPeerAPI, + } + + testWithPrimary(t, setupOptions, func(s1 test.Setup) { testWithReplica(t, s1, "on_first_use", func(firstPass bool, s2 test.Setup) { if !firstPass { return //no second pass needed diff --git a/internal/api/registry/shared_test.go b/internal/api/registry/shared_test.go index eb1902ba2..77a468999 100644 --- a/internal/api/registry/shared_test.go +++ b/internal/api/registry/shared_test.go @@ -19,8 +19,6 @@ package registryv2_test import ( - "crypto/sha256" - "encoding/hex" "fmt" "net/http" "strconv" @@ -44,16 +42,15 @@ var ( // the auth tenant ID that all test accounts use const authTenantID = "test1authtenant" -func testWithPrimary(t *testing.T, rle *keppel.RateLimitEngine, action func(test.Setup)) { +func testWithPrimary(t *testing.T, setupOptions []test.SetupOption, action func(test.Setup)) { test.WithRoundTripper(func(tt *test.RoundTripper) { for _, withAnycast := range []bool{false, true} { - s := test.NewSetup(t, + options := append(setupOptions, test.WithAnycast(withAnycast), test.WithAccount(keppel.Account{Name: "test1", AuthTenantID: authTenantID}), test.WithQuotas, - test.WithPeerAPI, - test.WithRateLimitEngine(rle), ) + s := test.NewSetup(t, options...) currentlyWithAnycast = withAnycast //run the tests for this scenario From ec406fcb42c367f4d05024cc58d8a63ecaba95a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sandro=20J=C3=A4ckel?= Date: Mon, 25 Sep 2023 16:20:12 +0200 Subject: [PATCH 3/3] Add test for trivy ratelimit --- internal/api/keppel/manifests_test.go | 77 +++++++++++++++++++++++++++ 1 file changed, 77 insertions(+) diff --git a/internal/api/keppel/manifests_test.go b/internal/api/keppel/manifests_test.go index b06c5a916..c5debe1be 100644 --- a/internal/api/keppel/manifests_test.go +++ b/internal/api/keppel/manifests_test.go @@ -20,19 +20,25 @@ package keppelv1_test import ( "encoding/json" + "fmt" "net/http" "sort" + "strconv" "strings" "testing" "time" + "github.com/alicebob/miniredis/v2" "github.com/docker/distribution/manifest/schema2" + "github.com/go-redis/redis_rate/v10" "github.com/opencontainers/go-digest" + "github.com/redis/go-redis/v9" "github.com/sapcc/go-api-declarations/cadf" "github.com/sapcc/go-bits/assert" "github.com/sapcc/go-bits/easypg" "github.com/sapcc/go-bits/must" + "github.com/sapcc/keppel/internal/drivers/basic" "github.com/sapcc/keppel/internal/keppel" "github.com/sapcc/keppel/internal/models" "github.com/sapcc/keppel/internal/test" @@ -471,3 +477,74 @@ func TestManifestsAPI(t *testing.T) { func p2time(x time.Time) *time.Time { return &x } + +func TestRateLimitsTrivyReport(t *testing.T) { + limit := redis_rate.Limit{Rate: 2, Period: time.Minute, Burst: 3} + rld := basic.RateLimitDriver{ + Limits: map[keppel.RateLimitedAction]redis_rate.Limit{ + keppel.TrivyReportRetrieveAction: limit, + }, + } + rle := &keppel.RateLimitEngine{Driver: rld, Client: nil} + + test.WithRoundTripper(func(tt *test.RoundTripper) { + s := test.NewSetup(t, + test.WithKeppelAPI, + test.WithTrivyDouble, + test.WithRateLimitEngine(rle), + test.WithAccount(keppel.Account{Name: "test1"}), + ) + h := s.Handler + + sr := miniredis.RunT(t) + s.Clock.AddListener(sr.SetTime) + rle.Client = redis.NewClient(&redis.Options{Addr: sr.Addr()}) + + _, err := keppel.FindOrCreateRepository(s.DB, "foo", keppel.Account{Name: "test1"}) + if err != nil { + t.Fatal(err.Error()) + } + + token := s.GetToken(t, "repository:test1/foo:pull,push") + + req := assert.HTTPRequest{ + Method: "GET", + Path: fmt.Sprintf("/keppel/v1/accounts/test1/repositories/foo/_manifests/%s/trivy_report", test.DeterministicDummyDigest(1)), + Header: map[string]string{"Authorization": "Bearer " + token}, + ExpectStatus: http.StatusNotFound, + ExpectHeader: map[string]string{}, + ExpectBody: assert.StringData("not found\n"), + } + + s.Clock.StepBy(time.Hour) + + //we can always execute 1 request initially, and then we can burst on top of that + for i := 0; i < limit.Burst; i++ { + req.Check(t, h) + s.Clock.StepBy(time.Second) + } + + //then the next request should be rate-limited + failingReq := req + failingReq.ExpectBody = test.ErrorCode(keppel.ErrTooManyRequests) + failingReq.ExpectStatus = http.StatusTooManyRequests + failingReq.ExpectHeader = map[string]string{ + "Retry-After": strconv.Itoa(30 - limit.Burst), + } + failingReq.Check(t, h) + + //be impatient + s.Clock.StepBy(time.Duration(29-limit.Burst) * time.Second) + failingReq.ExpectHeader["Retry-After"] = "1" + failingReq.Check(t, h) + + //finally! + s.Clock.StepBy(time.Second) + req.Check(t, h) + + //aaaand... we're rate-limited again immediately because we haven't + //recovered our burst budget yet + failingReq.ExpectHeader["Retry-After"] = "30" + failingReq.Check(t, h) + }) +}