feat(ci): raise the archive limits and test the REST handler
Eight archives at once per client address (a runner host runs several jobs) and sixteen on the server; 429 and 503 carry Retry-After: 5. Unit tests for the handler: HEAD, both limits, the idle write deadline on a real stalled connection, credentials and GitHub's 404. Tests for the merge and the fall-through cases of the previous commits.
- commit
- 71757c4e43311b041b2ba20ed226d422394f095f
- author
- brice · · 2026-10-13 11:34 UTC
- parent
- 89ff433
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
4 files · +621 -15
M README.md +1 -1
| @@ -120,7 +120,7 @@ Runs execute on [ermite-runner](https://git.bnema.dev/bnema/ermite-runner) hosts | |||
| 120 | 120 | ||
| 121 | 121 | Every job gets a **repository token** (`github.token`, `ermitegh_…`) when a runner takes it: it reads the repository of the run, even a private one, over Git and the REST subset below, writes nothing, and ends with the job (completion, cancel, lost runner) or five minutes after the job's deadline. It is not a stored token: `ermite token list` never shows it and no scope manages it. | |
| 122 | 122 | ||
| 123 | - | An action of `ERMITE_OWNER/<repo>` that exists on this server wins over the same name on github.com. It is resolved **on behalf of whoever pushed the run**: ermite records the pusher with the run (the admin token, the owner's session, or the stored token by its id) and, at every answer, asks whether that pusher can read the action's repository, with the same rule as everywhere else: a public repository, or `repo:read` on a repository of the token's list. The token is looked up again each time, so a token revoked or expired since reads nothing for the run. A repository the pusher cannot read answers like one that does not exist (`repository not found`), whatever the ref, so it does not reveal itself. A job can therefore use an action from another private repository only when its pusher could read it. The runner gets an **archive token** for each such action: it is bound to that repository and that commit and downloads that one archive (`/-/github/api/repos/<owner>/<repo>/tarball/<sha>`) and nothing else. Git over HTTP, the repository endpoint and the rest of the API see it as a caller who can read nothing, not even a public repository. It goes with its job, and with the repository it reads if that is deleted. At most three archives are written at once for one client address (`429` beyond), eight on the server, and a client that stops reading is cut after two minutes without progress. | |
| 123 | + | An action of `ERMITE_OWNER/<repo>` that exists on this server wins over the same name on github.com. It is resolved **on behalf of whoever pushed the run**: ermite records the pusher with the run (the admin token, the owner's session, or the stored token by its id) and, at every answer, asks whether that pusher can read the action's repository, with the same rule as everywhere else: a public repository, or `repo:read` on a repository of the token's list. The token is looked up again each time, so a token revoked or expired since reads nothing for the run. A repository the pusher cannot read answers like one that does not exist (`repository not found`), whatever the ref, so it does not reveal itself. A job can therefore use an action from another private repository only when its pusher could read it. The runner gets an **archive token** for each such action: it is bound to that repository and that commit and downloads that one archive (`/-/github/api/repos/<owner>/<repo>/tarball/<sha>`) and nothing else. Git over HTTP, the repository endpoint and the rest of the API see it as a caller who can read nothing, not even a public repository. It goes with its job, and with the repository it reads if that is deleted. At most eight archives are written at once for one client address (`429` beyond: a runner host runs several jobs, and all come from one address) and sixteen on the server (`503`); both answers carry `Retry-After: 5`. A client that stops reading is cut after two minutes without progress. | |
| 124 | 124 | ||
| 125 | 125 | The Actions that run here call a small part of GitHub's REST API, served at `github.api_url` (`/-/github/api`): `GET /repos/{owner}/{repo}`, `/repos/{owner}/{repo}/hash-algorithm` and `/repos/{owner}/{repo}/tarball[/{ref}]` (streamed with 200, not redirected; a `HEAD` answers the headers without building the archive). The owner in these paths and in the Git URLs is matched without regard to case, as on GitHub. Any other path is GitHub's `404 {"message":"Not Found",…}`, so an action that needs more fails as on a GitHub instance without that feature. | |
| 126 | 126 | ||
| @@ -120,7 +120,7 @@ Runs execute on [ermite-runner](https://git.bnema.dev/bnema/ermite-runner) hosts | |||||
| 120 | 120 | ||||
| 121 | Every job gets a **repository token** (`github.token`, `ermitegh_…`) when a runner takes it: it reads the repository of the run, even a private one, over Git and the REST subset below, writes nothing, and ends with the job (completion, cancel, lost runner) or five minutes after the job's deadline. It is not a stored token: `ermite token list` never shows it and no scope manages it. | 121 | Every job gets a **repository token** (`github.token`, `ermitegh_…`) when a runner takes it: it reads the repository of the run, even a private one, over Git and the REST subset below, writes nothing, and ends with the job (completion, cancel, lost runner) or five minutes after the job's deadline. It is not a stored token: `ermite token list` never shows it and no scope manages it. | ||
| 122 | 122 | ||||
| 123 | - | An action of `ERMITE_OWNER/<repo>` that exists on this server wins over the same name on github.com. It is resolved **on behalf of whoever pushed the run**: ermite records the pusher with the run (the admin token, the owner's session, or the stored token by its id) and, at every answer, asks whether that pusher can read the action's repository, with the same rule as everywhere else: a public repository, or `repo:read` on a repository of the token's list. The token is looked up again each time, so a token revoked or expired since reads nothing for the run. A repository the pusher cannot read answers like one that does not exist (`repository not found`), whatever the ref, so it does not reveal itself. A job can therefore use an action from another private repository only when its pusher could read it. The runner gets an **archive token** for each such action: it is bound to that repository and that commit and downloads that one archive (`/-/github/api/repos/<owner>/<repo>/tarball/<sha>`) and nothing else. Git over HTTP, the repository endpoint and the rest of the API see it as a caller who can read nothing, not even a public repository. It goes with its job, and with the repository it reads if that is deleted. At most three archives are written at once for one client address (`429` beyond), eight on the server, and a client that stops reading is cut after two minutes without progress. | 123 | + | An action of `ERMITE_OWNER/<repo>` that exists on this server wins over the same name on github.com. It is resolved **on behalf of whoever pushed the run**: ermite records the pusher with the run (the admin token, the owner's session, or the stored token by its id) and, at every answer, asks whether that pusher can read the action's repository, with the same rule as everywhere else: a public repository, or `repo:read` on a repository of the token's list. The token is looked up again each time, so a token revoked or expired since reads nothing for the run. A repository the pusher cannot read answers like one that does not exist (`repository not found`), whatever the ref, so it does not reveal itself. A job can therefore use an action from another private repository only when its pusher could read it. The runner gets an **archive token** for each such action: it is bound to that repository and that commit and downloads that one archive (`/-/github/api/repos/<owner>/<repo>/tarball/<sha>`) and nothing else. Git over HTTP, the repository endpoint and the rest of the API see it as a caller who can read nothing, not even a public repository. It goes with its job, and with the repository it reads if that is deleted. At most eight archives are written at once for one client address (`429` beyond: a runner host runs several jobs, and all come from one address) and sixteen on the server (`503`); both answers carry `Retry-After: 5`. A client that stops reading is cut after two minutes without progress. |
| 124 | 124 | ||||
| 125 | The Actions that run here call a small part of GitHub's REST API, served at `github.api_url` (`/-/github/api`): `GET /repos/{owner}/{repo}`, `/repos/{owner}/{repo}/hash-algorithm` and `/repos/{owner}/{repo}/tarball[/{ref}]` (streamed with 200, not redirected; a `HEAD` answers the headers without building the archive). The owner in these paths and in the Git URLs is matched without regard to case, as on GitHub. Any other path is GitHub's `404 {"message":"Not Found",…}`, so an action that needs more fails as on a GitHub instance without that feature. | 125 | The Actions that run here call a small part of GitHub's REST API, served at `github.api_url` (`/-/github/api`): `GET /repos/{owner}/{repo}`, `/repos/{owner}/{repo}/hash-algorithm` and `/repos/{owner}/{repo}/tarball[/{ref}]` (streamed with 200, not redirected; a `HEAD` answers the headers without building the archive). The owner in these paths and in the Git URLs is matched without regard to case, as on GitHub. Any other path is GitHub's `404 {"message":"Not Found",…}`, so an action that needs more fails as on a GitHub instance without that feature. | ||
| 126 | 126 | ||||
M internal/adapters/githubapi/handler.go +13 -7
| @@ -43,14 +43,20 @@ var Patterns = []string{Prefix + "/"} | |||
| 43 | 43 | // notFoundBody is GitHub's body for an unknown endpoint or repository. | |
| 44 | 44 | const notFoundBody = `{"message":"Not Found","documentation_url":"https://docs.github.com/rest"}` | |
| 45 | 45 | ||
| 46 | + | // retryAfter is the Retry-After, in seconds, of an answer that asks the client to | |
| 47 | + | // come back (429 and 503). | |
| 48 | + | const retryAfter = "5" | |
| 49 | + | ||
| 46 | 50 | // Bounds on the archives written at once. | |
| 47 | 51 | const ( | |
| 48 | 52 | // DefaultMaxArchives is the archives written at once on the server. | |
| 49 | - | DefaultMaxArchives = 8 | |
| 53 | + | DefaultMaxArchives = 16 | |
| 50 | 54 | // DefaultMaxArchivesPerClient is the archives written at once for one client | |
| 51 | - | // (see clientlimit.Key): a runner downloads the actions of a job one by one, | |
| 52 | - | // so a few are plenty, and one client cannot take every slot. | |
| 53 | - | DefaultMaxArchivesPerClient = 3 | |
| 55 | + | // (see clientlimit.Key). A runner host runs several jobs at once, each | |
| 56 | + | // downloading its actions, and all come from one address; eight is half of | |
| 57 | + | // the server's slots, so one host cannot take them all. Beyond it the answer | |
| 58 | + | // is 429 with Retry-After: the runner waits and tries again. | |
| 59 | + | DefaultMaxArchivesPerClient = 8 | |
| 54 | 60 | // DefaultWriteIdleTimeout is how long a write of the archive may wait for the | |
| 55 | 61 | // client. The deadline moves forward with every chunk: an archive is bounded | |
| 56 | 62 | // by its size, not by its duration, but a client that stops reading is cut. | |
| @@ -222,7 +228,7 @@ func (h *Handler) acquire(w http.ResponseWriter, client string) (release func(), | |||
| 222 | 228 | h.mu.Lock() | |
| 223 | 229 | if h.clients[client] >= h.perCli { | |
| 224 | 230 | h.mu.Unlock() | |
| 225 | - | w.Header().Set("Retry-After", "5") | |
| 231 | + | w.Header().Set("Retry-After", retryAfter) | |
| 226 | 232 | jsonError(w, http.StatusTooManyRequests, "Too many archives in progress") | |
| 227 | 233 | return nil, false | |
| 228 | 234 | } | |
| @@ -240,7 +246,7 @@ func (h *Handler) acquire(w http.ResponseWriter, client string) (release func(), | |||
| 240 | 246 | return func() { <-h.archives; releaseClient() }, true | |
| 241 | 247 | default: | |
| 242 | 248 | releaseClient() | |
| 243 | - | w.Header().Set("Retry-After", "5") | |
| 249 | + | w.Header().Set("Retry-After", retryAfter) | |
| 244 | 250 | jsonError(w, http.StatusServiceUnavailable, "Server busy") | |
| 245 | 251 | return nil, false | |
| 246 | 252 | } | |
| @@ -307,7 +313,7 @@ func (h *Handler) fail(ctx context.Context, w http.ResponseWriter, err error) { | |||
| 307 | 313 | case errors.Is(err, domain.ErrNotFound), domain.IsInvalid(err): | |
| 308 | 314 | notFound(w) | |
| 309 | 315 | case errors.Is(err, domain.ErrResourceExhausted): | |
| 310 | - | w.Header().Set("Retry-After", "5") | |
| 316 | + | w.Header().Set("Retry-After", retryAfter) | |
| 311 | 317 | jsonError(w, http.StatusServiceUnavailable, "Server busy") | |
| 312 | 318 | case errors.Is(err, context.Canceled): | |
| 313 | 319 | default: | |
| @@ -43,14 +43,20 @@ var Patterns = []string{Prefix + "/"} | |||||
| 43 | // notFoundBody is GitHub's body for an unknown endpoint or repository. | 43 | // notFoundBody is GitHub's body for an unknown endpoint or repository. | ||
| 44 | const notFoundBody = `{"message":"Not Found","documentation_url":"https://docs.github.com/rest"}` | 44 | const notFoundBody = `{"message":"Not Found","documentation_url":"https://docs.github.com/rest"}` | ||
| 45 | 45 | ||||
| 46 | + | // retryAfter is the Retry-After, in seconds, of an answer that asks the client to | |||
| 47 | + | // come back (429 and 503). | |||
| 48 | + | const retryAfter = "5" | |||
| 49 | + | ||||
| 46 | // Bounds on the archives written at once. | 50 | // Bounds on the archives written at once. | ||
| 47 | const ( | 51 | const ( | ||
| 48 | // DefaultMaxArchives is the archives written at once on the server. | 52 | // DefaultMaxArchives is the archives written at once on the server. | ||
| 49 | - | DefaultMaxArchives = 8 | 53 | + | DefaultMaxArchives = 16 |
| 50 | // DefaultMaxArchivesPerClient is the archives written at once for one client | 54 | // DefaultMaxArchivesPerClient is the archives written at once for one client | ||
| 51 | - | // (see clientlimit.Key): a runner downloads the actions of a job one by one, | 55 | + | // (see clientlimit.Key). A runner host runs several jobs at once, each |
| 52 | - | // so a few are plenty, and one client cannot take every slot. | 56 | + | // downloading its actions, and all come from one address; eight is half of |
| 53 | - | DefaultMaxArchivesPerClient = 3 | 57 | + | // the server's slots, so one host cannot take them all. Beyond it the answer |
| 58 | + | // is 429 with Retry-After: the runner waits and tries again. | |||
| 59 | + | DefaultMaxArchivesPerClient = 8 | |||
| 54 | // DefaultWriteIdleTimeout is how long a write of the archive may wait for the | 60 | // DefaultWriteIdleTimeout is how long a write of the archive may wait for the | ||
| 55 | // client. The deadline moves forward with every chunk: an archive is bounded | 61 | // client. The deadline moves forward with every chunk: an archive is bounded | ||
| 56 | // by its size, not by its duration, but a client that stops reading is cut. | 62 | // by its size, not by its duration, but a client that stops reading is cut. | ||
| @@ -222,7 +228,7 @@ func (h *Handler) acquire(w http.ResponseWriter, client string) (release func(), | |||||
| 222 | h.mu.Lock() | 228 | h.mu.Lock() | ||
| 223 | if h.clients[client] >= h.perCli { | 229 | if h.clients[client] >= h.perCli { | ||
| 224 | h.mu.Unlock() | 230 | h.mu.Unlock() | ||
| 225 | - | w.Header().Set("Retry-After", "5") | 231 | + | w.Header().Set("Retry-After", retryAfter) |
| 226 | jsonError(w, http.StatusTooManyRequests, "Too many archives in progress") | 232 | jsonError(w, http.StatusTooManyRequests, "Too many archives in progress") | ||
| 227 | return nil, false | 233 | return nil, false | ||
| 228 | } | 234 | } | ||
| @@ -240,7 +246,7 @@ func (h *Handler) acquire(w http.ResponseWriter, client string) (release func(), | |||||
| 240 | return func() { <-h.archives; releaseClient() }, true | 246 | return func() { <-h.archives; releaseClient() }, true | ||
| 241 | default: | 247 | default: | ||
| 242 | releaseClient() | 248 | releaseClient() | ||
| 243 | - | w.Header().Set("Retry-After", "5") | 249 | + | w.Header().Set("Retry-After", retryAfter) |
| 244 | jsonError(w, http.StatusServiceUnavailable, "Server busy") | 250 | jsonError(w, http.StatusServiceUnavailable, "Server busy") | ||
| 245 | return nil, false | 251 | return nil, false | ||
| 246 | } | 252 | } | ||
| @@ -307,7 +313,7 @@ func (h *Handler) fail(ctx context.Context, w http.ResponseWriter, err error) { | |||||
| 307 | case errors.Is(err, domain.ErrNotFound), domain.IsInvalid(err): | 313 | case errors.Is(err, domain.ErrNotFound), domain.IsInvalid(err): | ||
| 308 | notFound(w) | 314 | notFound(w) | ||
| 309 | case errors.Is(err, domain.ErrResourceExhausted): | 315 | case errors.Is(err, domain.ErrResourceExhausted): | ||
| 310 | - | w.Header().Set("Retry-After", "5") | 316 | + | w.Header().Set("Retry-After", retryAfter) |
| 311 | jsonError(w, http.StatusServiceUnavailable, "Server busy") | 317 | jsonError(w, http.StatusServiceUnavailable, "Server busy") | ||
| 312 | case errors.Is(err, context.Canceled): | 318 | case errors.Is(err, context.Canceled): | ||
| 313 | default: | 319 | default: | ||
A internal/adapters/githubapi/handler_test.go +370 -0
| @@ -0,0 +1,370 @@ | |||
| 1 | + | // SPDX-License-Identifier: AGPL-3.0-or-later | |
| 2 | + | ||
| 3 | + | package githubapi | |
| 4 | + | ||
| 5 | + | import ( | |
| 6 | + | "context" | |
| 7 | + | "errors" | |
| 8 | + | "io" | |
| 9 | + | "net" | |
| 10 | + | "net/http" | |
| 11 | + | "net/http/httptest" | |
| 12 | + | "strings" | |
| 13 | + | "sync" | |
| 14 | + | "testing" | |
| 15 | + | "time" | |
| 16 | + | ||
| 17 | + | "github.com/stretchr/testify/assert" | |
| 18 | + | "github.com/stretchr/testify/mock" | |
| 19 | + | "github.com/stretchr/testify/require" | |
| 20 | + | ||
| 21 | + | "bnema.dev/ermite/internal/domain" | |
| 22 | + | "bnema.dev/ermite/internal/ports/mocks" | |
| 23 | + | ) | |
| 24 | + | ||
| 25 | + | type rig struct { | |
| 26 | + | h *Handler | |
| 27 | + | srv *httptest.Server | |
| 28 | + | api *mocks.GitHubAPIService | |
| 29 | + | auth *mocks.AuthService | |
| 30 | + | } | |
| 31 | + | ||
| 32 | + | func newRig(t *testing.T, mut func(*Config)) *rig { | |
| 33 | + | t.Helper() | |
| 34 | + | r := &rig{api: mocks.NewGitHubAPIService(t), auth: mocks.NewAuthService(t)} | |
| 35 | + | cfg := Config{Auth: r.auth, API: r.api} | |
| 36 | + | if mut != nil { | |
| 37 | + | mut(&cfg) | |
| 38 | + | } | |
| 39 | + | r.h = New(cfg) | |
| 40 | + | mux := http.NewServeMux() | |
| 41 | + | mux.Handle(Prefix+"/", r.h) | |
| 42 | + | r.srv = httptest.NewServer(mux) | |
| 43 | + | t.Cleanup(r.srv.Close) | |
| 44 | + | return r | |
| 45 | + | } | |
| 46 | + | ||
| 47 | + | func (r *rig) do(t *testing.T, method, path, authorization string) (*http.Response, string) { | |
| 48 | + | t.Helper() | |
| 49 | + | req, err := http.NewRequest(method, r.srv.URL+Prefix+path, nil) | |
| 50 | + | require.NoError(t, err) | |
| 51 | + | if authorization != "" { | |
| 52 | + | req.Header.Set("Authorization", authorization) | |
| 53 | + | } | |
| 54 | + | resp, err := http.DefaultClient.Do(req) | |
| 55 | + | require.NoError(t, err) | |
| 56 | + | defer func() { _ = resp.Body.Close() }() | |
| 57 | + | b, err := io.ReadAll(resp.Body) | |
| 58 | + | require.NoError(t, err) | |
| 59 | + | return resp, string(b) | |
| 60 | + | } | |
| 61 | + | ||
| 62 | + | const tarballPath = "/repos/acme/proj/tarball/main" | |
| 63 | + | ||
| 64 | + | // blockingArchive makes Tarball write one chunk and then wait for release, so | |
| 65 | + | // that the archive stays in progress while the test looks at the others. | |
| 66 | + | type blockingArchive struct { | |
| 67 | + | started chan struct{} | |
| 68 | + | release chan struct{} | |
| 69 | + | } | |
| 70 | + | ||
| 71 | + | func newBlockingArchive() *blockingArchive { | |
| 72 | + | return &blockingArchive{started: make(chan struct{}, 32), release: make(chan struct{})} | |
| 73 | + | } | |
| 74 | + | ||
| 75 | + | func (b *blockingArchive) run(ctx context.Context, _ domain.Principal, _, _, _ string, begin func(string) io.Writer) error { | |
| 76 | + | w := begin("acme-proj-abcdef0.tar.gz") | |
| 77 | + | _, _ = w.Write([]byte("chunk")) | |
| 78 | + | b.started <- struct{}{} | |
| 79 | + | select { | |
| 80 | + | case <-b.release: | |
| 81 | + | case <-ctx.Done(): | |
| 82 | + | } | |
| 83 | + | return nil | |
| 84 | + | } | |
| 85 | + | ||
| 86 | + | // A HEAD resolves the ref and answers the headers of the archive: git archive is | |
| 87 | + | // not run (the mock fails on a call to Tarball), no slot is taken, and an error | |
| 88 | + | // is answered as for a GET. | |
| 89 | + | func TestHeadOfTheTarballRunsNoArchive(t *testing.T) { | |
| 90 | + | t.Parallel() | |
| 91 | + | r := newRig(t, func(c *Config) { c.MaxArchives, c.MaxArchivesPerClient = 1, 1 }) | |
| 92 | + | r.api.EXPECT().TarballName(mock.Anything, domain.Anonymous(), "acme", "proj", "main").Return("acme-proj-abcdef0.tar.gz", nil) | |
| 93 | + | r.api.EXPECT().TarballName(mock.Anything, domain.Anonymous(), "acme", "proj", "nope").Return("", domain.ErrNotFound) | |
| 94 | + | r.api.EXPECT().TarballName(mock.Anything, domain.Anonymous(), "acme", "proj", "").Return("", errors.New("disk")) | |
| 95 | + | ||
| 96 | + | resp, body := r.do(t, http.MethodHead, tarballPath, "") | |
| 97 | + | assert.Equal(t, http.StatusOK, resp.StatusCode) | |
| 98 | + | assert.Empty(t, body) | |
| 99 | + | assert.Equal(t, "attachment; filename=acme-proj-abcdef0.tar.gz", resp.Header.Get("Content-Disposition")) | |
| 100 | + | assert.Equal(t, "application/x-gzip", resp.Header.Get("Content-Type")) | |
| 101 | + | ||
| 102 | + | resp, body = r.do(t, http.MethodHead, "/repos/acme/proj/tarball/nope", "") | |
| 103 | + | assert.Equal(t, http.StatusNotFound, resp.StatusCode) | |
| 104 | + | assert.Empty(t, body, "a HEAD has no body, the status says it") | |
| 105 | + | resp, _ = r.do(t, http.MethodHead, "/repos/acme/proj/tarball", "") | |
| 106 | + | assert.Equal(t, http.StatusInternalServerError, resp.StatusCode) | |
| 107 | + | ||
| 108 | + | // The one slot is free for a GET after any number of HEADs. | |
| 109 | + | for range 5 { | |
| 110 | + | resp, _ := r.do(t, http.MethodHead, tarballPath, "") | |
| 111 | + | assert.Equal(t, http.StatusOK, resp.StatusCode) | |
| 112 | + | } | |
| 113 | + | } | |
| 114 | + | ||
| 115 | + | // The per-client cap answers 429 with Retry-After, the global cap 503 with | |
| 116 | + | // Retry-After, and a finished archive gives its slot back. | |
| 117 | + | func TestArchiveSlots(t *testing.T) { | |
| 118 | + | t.Parallel() | |
| 119 | + | t.Run("per client", func(t *testing.T) { | |
| 120 | + | t.Parallel() | |
| 121 | + | arch := newBlockingArchive() | |
| 122 | + | r := newRig(t, func(c *Config) { c.MaxArchives, c.MaxArchivesPerClient = 8, 2 }) | |
| 123 | + | r.api.EXPECT().Tarball(mock.Anything, mock.Anything, "acme", "proj", "main", mock.Anything).RunAndReturn(arch.run) | |
| 124 | + | ||
| 125 | + | var wg sync.WaitGroup | |
| 126 | + | for range 2 { | |
| 127 | + | wg.Add(1) | |
| 128 | + | go func() { defer wg.Done(); r.do(t, http.MethodGet, tarballPath, "") }() | |
| 129 | + | } | |
| 130 | + | for range 2 { | |
| 131 | + | <-arch.started | |
| 132 | + | } | |
| 133 | + | resp, body := r.do(t, http.MethodGet, tarballPath, "") | |
| 134 | + | assert.Equal(t, http.StatusTooManyRequests, resp.StatusCode) | |
| 135 | + | assert.Equal(t, "5", resp.Header.Get("Retry-After")) | |
| 136 | + | assert.Contains(t, body, "Too many archives") | |
| 137 | + | assert.Contains(t, resp.Header.Get("Content-Type"), "application/json") | |
| 138 | + | ||
| 139 | + | // Other endpoints and HEAD are not archives. | |
| 140 | + | r.api.EXPECT().HashAlgorithm(mock.Anything, mock.Anything, "acme", "proj").Return("sha1", nil) | |
| 141 | + | resp, _ = r.do(t, http.MethodGet, "/repos/acme/proj/hash-algorithm", "") | |
| 142 | + | assert.Equal(t, http.StatusOK, resp.StatusCode) | |
| 143 | + | ||
| 144 | + | close(arch.release) | |
| 145 | + | wg.Wait() | |
| 146 | + | // The slots are given back: two more go through. | |
| 147 | + | arch2 := newBlockingArchive() | |
| 148 | + | close(arch2.release) | |
| 149 | + | r.api.ExpectedCalls = nil | |
| 150 | + | r.api.EXPECT().Tarball(mock.Anything, mock.Anything, "acme", "proj", "main", mock.Anything).RunAndReturn(arch2.run) | |
| 151 | + | for range 4 { | |
| 152 | + | resp, _ := r.do(t, http.MethodGet, tarballPath, "") | |
| 153 | + | assert.Equal(t, http.StatusOK, resp.StatusCode) | |
| 154 | + | } | |
| 155 | + | }) | |
| 156 | + | ||
| 157 | + | t.Run("global", func(t *testing.T) { | |
| 158 | + | t.Parallel() | |
| 159 | + | arch := newBlockingArchive() | |
| 160 | + | r := newRig(t, func(c *Config) { c.MaxArchives, c.MaxArchivesPerClient = 1, 8 }) | |
| 161 | + | r.api.EXPECT().Tarball(mock.Anything, mock.Anything, "acme", "proj", "main", mock.Anything).RunAndReturn(arch.run) | |
| 162 | + | ||
| 163 | + | done := make(chan struct{}) | |
| 164 | + | go func() { defer close(done); r.do(t, http.MethodGet, tarballPath, "") }() | |
| 165 | + | <-arch.started | |
| 166 | + | resp, body := r.do(t, http.MethodGet, tarballPath, "") | |
| 167 | + | assert.Equal(t, http.StatusServiceUnavailable, resp.StatusCode) | |
| 168 | + | assert.Equal(t, "5", resp.Header.Get("Retry-After")) | |
| 169 | + | assert.Contains(t, body, "Server busy") | |
| 170 | + | close(arch.release) | |
| 171 | + | <-done | |
| 172 | + | ||
| 173 | + | // The client's own count was given back when the global slot refused it: | |
| 174 | + | // with a cap of one per client, the next one still gets a slot. | |
| 175 | + | r2 := newRig(t, func(c *Config) { c.MaxArchives, c.MaxArchivesPerClient = 1, 1 }) | |
| 176 | + | arch2 := newBlockingArchive() | |
| 177 | + | r2.api.EXPECT().Tarball(mock.Anything, mock.Anything, "acme", "proj", "main", mock.Anything).RunAndReturn(arch2.run) | |
| 178 | + | done2 := make(chan struct{}) | |
| 179 | + | go func() { defer close(done2); r2.do(t, http.MethodGet, tarballPath, "") }() | |
| 180 | + | <-arch2.started | |
| 181 | + | resp, _ = r2.do(t, http.MethodGet, tarballPath, "") | |
| 182 | + | assert.Equal(t, http.StatusTooManyRequests, resp.StatusCode, "the client cap answers first") | |
| 183 | + | close(arch2.release) | |
| 184 | + | <-done2 | |
| 185 | + | close(arch.started) | |
| 186 | + | }) | |
| 187 | + | ||
| 188 | + | t.Run("a server busy answer does not leak the client slot", func(t *testing.T) { | |
| 189 | + | t.Parallel() | |
| 190 | + | h := New(Config{MaxArchives: 1, MaxArchivesPerClient: 3, Auth: mocks.NewAuthService(t), API: mocks.NewGitHubAPIService(t)}) | |
| 191 | + | h.archives <- struct{}{} // the server is full | |
| 192 | + | for range 10 { | |
| 193 | + | rec := httptest.NewRecorder() | |
| 194 | + | _, ok := h.acquire(rec, "client") | |
| 195 | + | require.False(t, ok) | |
| 196 | + | assert.Equal(t, http.StatusServiceUnavailable, rec.Code) | |
| 197 | + | assert.Equal(t, "5", rec.Header().Get("Retry-After")) | |
| 198 | + | } | |
| 199 | + | assert.Empty(t, h.clients, "no client count was left behind") | |
| 200 | + | <-h.archives | |
| 201 | + | release, ok := h.acquire(httptest.NewRecorder(), "client") | |
| 202 | + | require.True(t, ok) | |
| 203 | + | release() | |
| 204 | + | assert.Empty(t, h.clients) | |
| 205 | + | assert.Empty(t, h.archives) | |
| 206 | + | }) | |
| 207 | + | } | |
| 208 | + | ||
| 209 | + | // fakeRW is a response writer that can set a write deadline, like the one of | |
| 210 | + | // net/http, and records the deadlines and the writes. | |
| 211 | + | type fakeRW struct { | |
| 212 | + | header http.Header | |
| 213 | + | body strings.Builder | |
| 214 | + | deadlines []time.Time | |
| 215 | + | failWith error | |
| 216 | + | } | |
| 217 | + | ||
| 218 | + | func (f *fakeRW) Header() http.Header { return f.header } | |
| 219 | + | func (f *fakeRW) WriteHeader(int) {} | |
| 220 | + | func (f *fakeRW) Write(b []byte) (int, error) { | |
| 221 | + | if f.failWith != nil { | |
| 222 | + | return 0, f.failWith | |
| 223 | + | } | |
| 224 | + | return f.body.Write(b) | |
| 225 | + | } | |
| 226 | + | func (f *fakeRW) SetWriteDeadline(t time.Time) error { | |
| 227 | + | f.deadlines = append(f.deadlines, t) | |
| 228 | + | return nil | |
| 229 | + | } | |
| 230 | + | ||
| 231 | + | // The deadline of the connection moves forward before every chunk, by the idle | |
| 232 | + | // timeout, so that only a client that stops reading is cut. | |
| 233 | + | func TestIdleWriterMovesTheDeadlineForEveryChunk(t *testing.T) { | |
| 234 | + | t.Parallel() | |
| 235 | + | rw := &fakeRW{header: http.Header{}} | |
| 236 | + | w := &idleWriter{w: rw, rc: http.NewResponseController(rw), idle: 90 * time.Second} | |
| 237 | + | ||
| 238 | + | before := time.Now() | |
| 239 | + | for i := range 3 { | |
| 240 | + | n, err := w.Write([]byte("chunk")) | |
| 241 | + | require.NoError(t, err, i) | |
| 242 | + | assert.Equal(t, 5, n) | |
| 243 | + | time.Sleep(5 * time.Millisecond) | |
| 244 | + | } | |
| 245 | + | assert.Equal(t, "chunkchunkchunk", rw.body.String()) | |
| 246 | + | require.Len(t, rw.deadlines, 3, "one deadline per chunk") | |
| 247 | + | for i, d := range rw.deadlines { | |
| 248 | + | assert.WithinDuration(t, before.Add(90*time.Second), d, 2*time.Second, i) | |
| 249 | + | if i > 0 { | |
| 250 | + | assert.True(t, d.After(rw.deadlines[i-1]), "it moves forward: %d", i) | |
| 251 | + | } | |
| 252 | + | } | |
| 253 | + | ||
| 254 | + | // A write error is returned as it is. | |
| 255 | + | rw.failWith = errors.New("broken pipe") | |
| 256 | + | _, err := w.Write([]byte("x")) | |
| 257 | + | assert.EqualError(t, err, "broken pipe") | |
| 258 | + | ||
| 259 | + | // A writer that cannot set a deadline still writes: the bound is then not | |
| 260 | + | // applied here, and nothing fails. | |
| 261 | + | plain := httptest.NewRecorder() | |
| 262 | + | pw := &idleWriter{w: plain, rc: http.NewResponseController(plain), idle: time.Second} | |
| 263 | + | _, err = pw.Write([]byte("ok")) | |
| 264 | + | require.NoError(t, err) | |
| 265 | + | assert.Equal(t, "ok", plain.Body.String()) | |
| 266 | + | } | |
| 267 | + | ||
| 268 | + | // A client that stops reading is cut after the idle timeout, with a real | |
| 269 | + | // connection: the archive is cut short, the slot is given back, and the next | |
| 270 | + | // download goes through. | |
| 271 | + | func TestAClientThatStopsReadingIsCut(t *testing.T) { | |
| 272 | + | t.Parallel() | |
| 273 | + | var writeErr = make(chan error, 1) | |
| 274 | + | r := newRig(t, func(c *Config) { | |
| 275 | + | c.MaxArchives, c.MaxArchivesPerClient, c.WriteIdleTimeout = 2, 1, 300*time.Millisecond | |
| 276 | + | }) | |
| 277 | + | chunk := make([]byte, 1<<20) | |
| 278 | + | r.api.EXPECT().Tarball(mock.Anything, mock.Anything, "acme", "proj", "main", mock.Anything).RunAndReturn( | |
| 279 | + | func(ctx context.Context, _ domain.Principal, _, _, _ string, begin func(string) io.Writer) error { | |
| 280 | + | w := begin("a.tar.gz") | |
| 281 | + | // More than the socket buffers hold: the write blocks once the client | |
| 282 | + | // reads nothing, and fails when the idle deadline passes. | |
| 283 | + | for range 1024 { | |
| 284 | + | if _, err := w.Write(chunk); err != nil { | |
| 285 | + | writeErr <- err | |
| 286 | + | return err | |
| 287 | + | } | |
| 288 | + | } | |
| 289 | + | writeErr <- nil | |
| 290 | + | return nil | |
| 291 | + | }).Once() | |
| 292 | + | ||
| 293 | + | // The client takes the headers and reads no body. | |
| 294 | + | conn, err := net.Dial("tcp", strings.TrimPrefix(r.srv.URL, "http://")) | |
| 295 | + | require.NoError(t, err) | |
| 296 | + | t.Cleanup(func() { _ = conn.Close() }) | |
| 297 | + | _, err = io.WriteString(conn, "GET "+Prefix+tarballPath+" HTTP/1.1\r\nHost: x\r\n\r\n") | |
| 298 | + | require.NoError(t, err) | |
| 299 | + | ||
| 300 | + | select { | |
| 301 | + | case err := <-writeErr: | |
| 302 | + | require.Error(t, err, "the server gave up on a client that reads nothing") | |
| 303 | + | assert.Contains(t, err.Error(), "i/o timeout") | |
| 304 | + | case <-time.After(15 * time.Second): | |
| 305 | + | t.Fatal("the stalled client was never cut") | |
| 306 | + | } | |
| 307 | + | ||
| 308 | + | // The slot came back: with a cap of one per client, another download starts. | |
| 309 | + | r.api.EXPECT().Tarball(mock.Anything, mock.Anything, "acme", "proj", "main", mock.Anything).RunAndReturn( | |
| 310 | + | func(_ context.Context, _ domain.Principal, _, _, _ string, begin func(string) io.Writer) error { | |
| 311 | + | _, err := begin("a.tar.gz").Write([]byte("fine")) | |
| 312 | + | return err | |
| 313 | + | }).Once() | |
| 314 | + | require.Eventually(t, func() bool { | |
| 315 | + | resp, body := r.do(t, http.MethodGet, tarballPath, "") | |
| 316 | + | return resp.StatusCode == http.StatusOK && body == "fine" | |
| 317 | + | }, 5*time.Second, 50*time.Millisecond) | |
| 318 | + | } | |
| 319 | + | ||
| 320 | + | // The credentials of a request are the forge's: Basic with any user, Bearer or | |
| 321 | + | // token; none is anonymous and does not touch the failure limiter. | |
| 322 | + | func TestCredentialsAndNotFound(t *testing.T) { | |
| 323 | + | t.Parallel() | |
| 324 | + | r := newRig(t, nil) | |
| 325 | + | p := domain.Root() | |
| 326 | + | r.auth.EXPECT().Authenticate(mock.Anything, "good").Return(p, nil) | |
| 327 | + | r.auth.EXPECT().Authenticate(mock.Anything, "bad").Return(domain.Principal{}, domain.ErrUnauthenticated) | |
| 328 | + | // A header that carries no usable token authenticates the empty secret. | |
| 329 | + | r.auth.EXPECT().Authenticate(mock.Anything, "").Return(domain.Principal{}, domain.ErrUnauthenticated) | |
| 330 | + | r.api.EXPECT().HashAlgorithm(mock.Anything, p, "acme", "proj").Return("sha1", nil) | |
| 331 | + | r.api.EXPECT().HashAlgorithm(mock.Anything, domain.Anonymous(), "acme", "proj").Return("sha1", nil) | |
| 332 | + | ||
| 333 | + | path := "/repos/acme/proj/hash-algorithm" | |
| 334 | + | for _, h := range []string{"Basic eC1hY2Nlc3MtdG9rZW46Z29vZA==", "Bearer good", "token good", "bearer good"} { | |
| 335 | + | resp, body := r.do(t, http.MethodGet, path, h) | |
| 336 | + | assert.Equal(t, http.StatusOK, resp.StatusCode, h) | |
| 337 | + | assert.JSONEq(t, `{"hash_algorithm":"sha1"}`, body) | |
| 338 | + | } | |
| 339 | + | resp, _ := r.do(t, http.MethodGet, path, "") | |
| 340 | + | assert.Equal(t, http.StatusOK, resp.StatusCode, "anonymous") | |
| 341 | + | for _, h := range []string{"Bearer bad", "Digest x", "Basic !!!", "Bearer"} { | |
| 342 | + | resp, body := r.do(t, http.MethodGet, path, h) | |
| 343 | + | assert.Equal(t, http.StatusUnauthorized, resp.StatusCode, h) | |
| 344 | + | assert.Contains(t, body, "Bad credentials") | |
| 345 | + | } | |
| 346 | + | ||
| 347 | + | // Not found is GitHub's body, for an unknown endpoint and for a use case that | |
| 348 | + | // says so, and a method other than GET and HEAD is 405. | |
| 349 | + | const gh = `{"message":"Not Found","documentation_url":"https://docs.github.com/rest"}` | |
| 350 | + | for _, p := range []string{"", "/", "/user", "/repos", "/repos/acme", "/repos//proj", "/repos/acme/proj/issues", "/repos/acme/proj/hash-algorithm/x"} { | |
| 351 | + | resp, body := r.do(t, http.MethodGet, p, "") | |
| 352 | + | assert.Equal(t, http.StatusNotFound, resp.StatusCode, p) | |
| 353 | + | assert.JSONEq(t, gh, body, p) | |
| 354 | + | } | |
| 355 | + | r.api.EXPECT().GetRepo(mock.Anything, domain.Anonymous(), "acme", "gone").Return(nil, domain.ErrNotFound) | |
| 356 | + | resp, body := r.do(t, http.MethodGet, "/repos/acme/gone", "") | |
| 357 | + | assert.Equal(t, http.StatusNotFound, resp.StatusCode) | |
| 358 | + | assert.JSONEq(t, gh, body) | |
| 359 | + | req, _ := http.NewRequest(http.MethodPost, r.srv.URL+Prefix+path, nil) | |
| 360 | + | post, err := http.DefaultClient.Do(req) | |
| 361 | + | require.NoError(t, err) | |
| 362 | + | _ = post.Body.Close() | |
| 363 | + | assert.Equal(t, http.StatusMethodNotAllowed, post.StatusCode) | |
| 364 | + | assert.Equal(t, "GET, HEAD", post.Header.Get("Allow")) | |
| 365 | + | ||
| 366 | + | // A ref with slashes is passed whole. | |
| 367 | + | r.api.EXPECT().TarballName(mock.Anything, domain.Anonymous(), "acme", "proj", "feature/x/y").Return("n.tar.gz", nil) | |
| 368 | + | resp, _ = r.do(t, http.MethodHead, "/repos/acme/proj/tarball/feature/x/y", "") | |
| 369 | + | assert.Equal(t, http.StatusOK, resp.StatusCode) | |
| 370 | + | } | |
| @@ -0,0 +1,370 @@ | |||||
| 1 | + | // SPDX-License-Identifier: AGPL-3.0-or-later | |||
| 2 | + | ||||
| 3 | + | package githubapi | |||
| 4 | + | ||||
| 5 | + | import ( | |||
| 6 | + | "context" | |||
| 7 | + | "errors" | |||
| 8 | + | "io" | |||
| 9 | + | "net" | |||
| 10 | + | "net/http" | |||
| 11 | + | "net/http/httptest" | |||
| 12 | + | "strings" | |||
| 13 | + | "sync" | |||
| 14 | + | "testing" | |||
| 15 | + | "time" | |||
| 16 | + | ||||
| 17 | + | "github.com/stretchr/testify/assert" | |||
| 18 | + | "github.com/stretchr/testify/mock" | |||
| 19 | + | "github.com/stretchr/testify/require" | |||
| 20 | + | ||||
| 21 | + | "bnema.dev/ermite/internal/domain" | |||
| 22 | + | "bnema.dev/ermite/internal/ports/mocks" | |||
| 23 | + | ) | |||
| 24 | + | ||||
| 25 | + | type rig struct { | |||
| 26 | + | h *Handler | |||
| 27 | + | srv *httptest.Server | |||
| 28 | + | api *mocks.GitHubAPIService | |||
| 29 | + | auth *mocks.AuthService | |||
| 30 | + | } | |||
| 31 | + | ||||
| 32 | + | func newRig(t *testing.T, mut func(*Config)) *rig { | |||
| 33 | + | t.Helper() | |||
| 34 | + | r := &rig{api: mocks.NewGitHubAPIService(t), auth: mocks.NewAuthService(t)} | |||
| 35 | + | cfg := Config{Auth: r.auth, API: r.api} | |||
| 36 | + | if mut != nil { | |||
| 37 | + | mut(&cfg) | |||
| 38 | + | } | |||
| 39 | + | r.h = New(cfg) | |||
| 40 | + | mux := http.NewServeMux() | |||
| 41 | + | mux.Handle(Prefix+"/", r.h) | |||
| 42 | + | r.srv = httptest.NewServer(mux) | |||
| 43 | + | t.Cleanup(r.srv.Close) | |||
| 44 | + | return r | |||
| 45 | + | } | |||
| 46 | + | ||||
| 47 | + | func (r *rig) do(t *testing.T, method, path, authorization string) (*http.Response, string) { | |||
| 48 | + | t.Helper() | |||
| 49 | + | req, err := http.NewRequest(method, r.srv.URL+Prefix+path, nil) | |||
| 50 | + | require.NoError(t, err) | |||
| 51 | + | if authorization != "" { | |||
| 52 | + | req.Header.Set("Authorization", authorization) | |||
| 53 | + | } | |||
| 54 | + | resp, err := http.DefaultClient.Do(req) | |||
| 55 | + | require.NoError(t, err) | |||
| 56 | + | defer func() { _ = resp.Body.Close() }() | |||
| 57 | + | b, err := io.ReadAll(resp.Body) | |||
| 58 | + | require.NoError(t, err) | |||
| 59 | + | return resp, string(b) | |||
| 60 | + | } | |||
| 61 | + | ||||
| 62 | + | const tarballPath = "/repos/acme/proj/tarball/main" | |||
| 63 | + | ||||
| 64 | + | // blockingArchive makes Tarball write one chunk and then wait for release, so | |||
| 65 | + | // that the archive stays in progress while the test looks at the others. | |||
| 66 | + | type blockingArchive struct { | |||
| 67 | + | started chan struct{} | |||
| 68 | + | release chan struct{} | |||
| 69 | + | } | |||
| 70 | + | ||||
| 71 | + | func newBlockingArchive() *blockingArchive { | |||
| 72 | + | return &blockingArchive{started: make(chan struct{}, 32), release: make(chan struct{})} | |||
| 73 | + | } | |||
| 74 | + | ||||
| 75 | + | func (b *blockingArchive) run(ctx context.Context, _ domain.Principal, _, _, _ string, begin func(string) io.Writer) error { | |||
| 76 | + | w := begin("acme-proj-abcdef0.tar.gz") | |||
| 77 | + | _, _ = w.Write([]byte("chunk")) | |||
| 78 | + | b.started <- struct{}{} | |||
| 79 | + | select { | |||
| 80 | + | case <-b.release: | |||
| 81 | + | case <-ctx.Done(): | |||
| 82 | + | } | |||
| 83 | + | return nil | |||
| 84 | + | } | |||
| 85 | + | ||||
| 86 | + | // A HEAD resolves the ref and answers the headers of the archive: git archive is | |||
| 87 | + | // not run (the mock fails on a call to Tarball), no slot is taken, and an error | |||
| 88 | + | // is answered as for a GET. | |||
| 89 | + | func TestHeadOfTheTarballRunsNoArchive(t *testing.T) { | |||
| 90 | + | t.Parallel() | |||
| 91 | + | r := newRig(t, func(c *Config) { c.MaxArchives, c.MaxArchivesPerClient = 1, 1 }) | |||
| 92 | + | r.api.EXPECT().TarballName(mock.Anything, domain.Anonymous(), "acme", "proj", "main").Return("acme-proj-abcdef0.tar.gz", nil) | |||
| 93 | + | r.api.EXPECT().TarballName(mock.Anything, domain.Anonymous(), "acme", "proj", "nope").Return("", domain.ErrNotFound) | |||
| 94 | + | r.api.EXPECT().TarballName(mock.Anything, domain.Anonymous(), "acme", "proj", "").Return("", errors.New("disk")) | |||
| 95 | + | ||||
| 96 | + | resp, body := r.do(t, http.MethodHead, tarballPath, "") | |||
| 97 | + | assert.Equal(t, http.StatusOK, resp.StatusCode) | |||
| 98 | + | assert.Empty(t, body) | |||
| 99 | + | assert.Equal(t, "attachment; filename=acme-proj-abcdef0.tar.gz", resp.Header.Get("Content-Disposition")) | |||
| 100 | + | assert.Equal(t, "application/x-gzip", resp.Header.Get("Content-Type")) | |||
| 101 | + | ||||
| 102 | + | resp, body = r.do(t, http.MethodHead, "/repos/acme/proj/tarball/nope", "") | |||
| 103 | + | assert.Equal(t, http.StatusNotFound, resp.StatusCode) | |||
| 104 | + | assert.Empty(t, body, "a HEAD has no body, the status says it") | |||
| 105 | + | resp, _ = r.do(t, http.MethodHead, "/repos/acme/proj/tarball", "") | |||
| 106 | + | assert.Equal(t, http.StatusInternalServerError, resp.StatusCode) | |||
| 107 | + | ||||
| 108 | + | // The one slot is free for a GET after any number of HEADs. | |||
| 109 | + | for range 5 { | |||
| 110 | + | resp, _ := r.do(t, http.MethodHead, tarballPath, "") | |||
| 111 | + | assert.Equal(t, http.StatusOK, resp.StatusCode) | |||
| 112 | + | } | |||
| 113 | + | } | |||
| 114 | + | ||||
| 115 | + | // The per-client cap answers 429 with Retry-After, the global cap 503 with | |||
| 116 | + | // Retry-After, and a finished archive gives its slot back. | |||
| 117 | + | func TestArchiveSlots(t *testing.T) { | |||
| 118 | + | t.Parallel() | |||
| 119 | + | t.Run("per client", func(t *testing.T) { | |||
| 120 | + | t.Parallel() | |||
| 121 | + | arch := newBlockingArchive() | |||
| 122 | + | r := newRig(t, func(c *Config) { c.MaxArchives, c.MaxArchivesPerClient = 8, 2 }) | |||
| 123 | + | r.api.EXPECT().Tarball(mock.Anything, mock.Anything, "acme", "proj", "main", mock.Anything).RunAndReturn(arch.run) | |||
| 124 | + | ||||
| 125 | + | var wg sync.WaitGroup | |||
| 126 | + | for range 2 { | |||
| 127 | + | wg.Add(1) | |||
| 128 | + | go func() { defer wg.Done(); r.do(t, http.MethodGet, tarballPath, "") }() | |||
| 129 | + | } | |||
| 130 | + | for range 2 { | |||
| 131 | + | <-arch.started | |||
| 132 | + | } | |||
| 133 | + | resp, body := r.do(t, http.MethodGet, tarballPath, "") | |||
| 134 | + | assert.Equal(t, http.StatusTooManyRequests, resp.StatusCode) | |||
| 135 | + | assert.Equal(t, "5", resp.Header.Get("Retry-After")) | |||
| 136 | + | assert.Contains(t, body, "Too many archives") | |||
| 137 | + | assert.Contains(t, resp.Header.Get("Content-Type"), "application/json") | |||
| 138 | + | ||||
| 139 | + | // Other endpoints and HEAD are not archives. | |||
| 140 | + | r.api.EXPECT().HashAlgorithm(mock.Anything, mock.Anything, "acme", "proj").Return("sha1", nil) | |||
| 141 | + | resp, _ = r.do(t, http.MethodGet, "/repos/acme/proj/hash-algorithm", "") | |||
| 142 | + | assert.Equal(t, http.StatusOK, resp.StatusCode) | |||
| 143 | + | ||||
| 144 | + | close(arch.release) | |||
| 145 | + | wg.Wait() | |||
| 146 | + | // The slots are given back: two more go through. | |||
| 147 | + | arch2 := newBlockingArchive() | |||
| 148 | + | close(arch2.release) | |||
| 149 | + | r.api.ExpectedCalls = nil | |||
| 150 | + | r.api.EXPECT().Tarball(mock.Anything, mock.Anything, "acme", "proj", "main", mock.Anything).RunAndReturn(arch2.run) | |||
| 151 | + | for range 4 { | |||
| 152 | + | resp, _ := r.do(t, http.MethodGet, tarballPath, "") | |||
| 153 | + | assert.Equal(t, http.StatusOK, resp.StatusCode) | |||
| 154 | + | } | |||
| 155 | + | }) | |||
| 156 | + | ||||
| 157 | + | t.Run("global", func(t *testing.T) { | |||
| 158 | + | t.Parallel() | |||
| 159 | + | arch := newBlockingArchive() | |||
| 160 | + | r := newRig(t, func(c *Config) { c.MaxArchives, c.MaxArchivesPerClient = 1, 8 }) | |||
| 161 | + | r.api.EXPECT().Tarball(mock.Anything, mock.Anything, "acme", "proj", "main", mock.Anything).RunAndReturn(arch.run) | |||
| 162 | + | ||||
| 163 | + | done := make(chan struct{}) | |||
| 164 | + | go func() { defer close(done); r.do(t, http.MethodGet, tarballPath, "") }() | |||
| 165 | + | <-arch.started | |||
| 166 | + | resp, body := r.do(t, http.MethodGet, tarballPath, "") | |||
| 167 | + | assert.Equal(t, http.StatusServiceUnavailable, resp.StatusCode) | |||
| 168 | + | assert.Equal(t, "5", resp.Header.Get("Retry-After")) | |||
| 169 | + | assert.Contains(t, body, "Server busy") | |||
| 170 | + | close(arch.release) | |||
| 171 | + | <-done | |||
| 172 | + | ||||
| 173 | + | // The client's own count was given back when the global slot refused it: | |||
| 174 | + | // with a cap of one per client, the next one still gets a slot. | |||
| 175 | + | r2 := newRig(t, func(c *Config) { c.MaxArchives, c.MaxArchivesPerClient = 1, 1 }) | |||
| 176 | + | arch2 := newBlockingArchive() | |||
| 177 | + | r2.api.EXPECT().Tarball(mock.Anything, mock.Anything, "acme", "proj", "main", mock.Anything).RunAndReturn(arch2.run) | |||
| 178 | + | done2 := make(chan struct{}) | |||
| 179 | + | go func() { defer close(done2); r2.do(t, http.MethodGet, tarballPath, "") }() | |||
| 180 | + | <-arch2.started | |||
| 181 | + | resp, _ = r2.do(t, http.MethodGet, tarballPath, "") | |||
| 182 | + | assert.Equal(t, http.StatusTooManyRequests, resp.StatusCode, "the client cap answers first") | |||
| 183 | + | close(arch2.release) | |||
| 184 | + | <-done2 | |||
| 185 | + | close(arch.started) | |||
| 186 | + | }) | |||
| 187 | + | ||||
| 188 | + | t.Run("a server busy answer does not leak the client slot", func(t *testing.T) { | |||
| 189 | + | t.Parallel() | |||
| 190 | + | h := New(Config{MaxArchives: 1, MaxArchivesPerClient: 3, Auth: mocks.NewAuthService(t), API: mocks.NewGitHubAPIService(t)}) | |||
| 191 | + | h.archives <- struct{}{} // the server is full | |||
| 192 | + | for range 10 { | |||
| 193 | + | rec := httptest.NewRecorder() | |||
| 194 | + | _, ok := h.acquire(rec, "client") | |||
| 195 | + | require.False(t, ok) | |||
| 196 | + | assert.Equal(t, http.StatusServiceUnavailable, rec.Code) | |||
| 197 | + | assert.Equal(t, "5", rec.Header().Get("Retry-After")) | |||
| 198 | + | } | |||
| 199 | + | assert.Empty(t, h.clients, "no client count was left behind") | |||
| 200 | + | <-h.archives | |||
| 201 | + | release, ok := h.acquire(httptest.NewRecorder(), "client") | |||
| 202 | + | require.True(t, ok) | |||
| 203 | + | release() | |||
| 204 | + | assert.Empty(t, h.clients) | |||
| 205 | + | assert.Empty(t, h.archives) | |||
| 206 | + | }) | |||
| 207 | + | } | |||
| 208 | + | ||||
| 209 | + | // fakeRW is a response writer that can set a write deadline, like the one of | |||
| 210 | + | // net/http, and records the deadlines and the writes. | |||
| 211 | + | type fakeRW struct { | |||
| 212 | + | header http.Header | |||
| 213 | + | body strings.Builder | |||
| 214 | + | deadlines []time.Time | |||
| 215 | + | failWith error | |||
| 216 | + | } | |||
| 217 | + | ||||
| 218 | + | func (f *fakeRW) Header() http.Header { return f.header } | |||
| 219 | + | func (f *fakeRW) WriteHeader(int) {} | |||
| 220 | + | func (f *fakeRW) Write(b []byte) (int, error) { | |||
| 221 | + | if f.failWith != nil { | |||
| 222 | + | return 0, f.failWith | |||
| 223 | + | } | |||
| 224 | + | return f.body.Write(b) | |||
| 225 | + | } | |||
| 226 | + | func (f *fakeRW) SetWriteDeadline(t time.Time) error { | |||
| 227 | + | f.deadlines = append(f.deadlines, t) | |||
| 228 | + | return nil | |||
| 229 | + | } | |||
| 230 | + | ||||
| 231 | + | // The deadline of the connection moves forward before every chunk, by the idle | |||
| 232 | + | // timeout, so that only a client that stops reading is cut. | |||
| 233 | + | func TestIdleWriterMovesTheDeadlineForEveryChunk(t *testing.T) { | |||
| 234 | + | t.Parallel() | |||
| 235 | + | rw := &fakeRW{header: http.Header{}} | |||
| 236 | + | w := &idleWriter{w: rw, rc: http.NewResponseController(rw), idle: 90 * time.Second} | |||
| 237 | + | ||||
| 238 | + | before := time.Now() | |||
| 239 | + | for i := range 3 { | |||
| 240 | + | n, err := w.Write([]byte("chunk")) | |||
| 241 | + | require.NoError(t, err, i) | |||
| 242 | + | assert.Equal(t, 5, n) | |||
| 243 | + | time.Sleep(5 * time.Millisecond) | |||
| 244 | + | } | |||
| 245 | + | assert.Equal(t, "chunkchunkchunk", rw.body.String()) | |||
| 246 | + | require.Len(t, rw.deadlines, 3, "one deadline per chunk") | |||
| 247 | + | for i, d := range rw.deadlines { | |||
| 248 | + | assert.WithinDuration(t, before.Add(90*time.Second), d, 2*time.Second, i) | |||
| 249 | + | if i > 0 { | |||
| 250 | + | assert.True(t, d.After(rw.deadlines[i-1]), "it moves forward: %d", i) | |||
| 251 | + | } | |||
| 252 | + | } | |||
| 253 | + | ||||
| 254 | + | // A write error is returned as it is. | |||
| 255 | + | rw.failWith = errors.New("broken pipe") | |||
| 256 | + | _, err := w.Write([]byte("x")) | |||
| 257 | + | assert.EqualError(t, err, "broken pipe") | |||
| 258 | + | ||||
| 259 | + | // A writer that cannot set a deadline still writes: the bound is then not | |||
| 260 | + | // applied here, and nothing fails. | |||
| 261 | + | plain := httptest.NewRecorder() | |||
| 262 | + | pw := &idleWriter{w: plain, rc: http.NewResponseController(plain), idle: time.Second} | |||
| 263 | + | _, err = pw.Write([]byte("ok")) | |||
| 264 | + | require.NoError(t, err) | |||
| 265 | + | assert.Equal(t, "ok", plain.Body.String()) | |||
| 266 | + | } | |||
| 267 | + | ||||
| 268 | + | // A client that stops reading is cut after the idle timeout, with a real | |||
| 269 | + | // connection: the archive is cut short, the slot is given back, and the next | |||
| 270 | + | // download goes through. | |||
| 271 | + | func TestAClientThatStopsReadingIsCut(t *testing.T) { | |||
| 272 | + | t.Parallel() | |||
| 273 | + | var writeErr = make(chan error, 1) | |||
| 274 | + | r := newRig(t, func(c *Config) { | |||
| 275 | + | c.MaxArchives, c.MaxArchivesPerClient, c.WriteIdleTimeout = 2, 1, 300*time.Millisecond | |||
| 276 | + | }) | |||
| 277 | + | chunk := make([]byte, 1<<20) | |||
| 278 | + | r.api.EXPECT().Tarball(mock.Anything, mock.Anything, "acme", "proj", "main", mock.Anything).RunAndReturn( | |||
| 279 | + | func(ctx context.Context, _ domain.Principal, _, _, _ string, begin func(string) io.Writer) error { | |||
| 280 | + | w := begin("a.tar.gz") | |||
| 281 | + | // More than the socket buffers hold: the write blocks once the client | |||
| 282 | + | // reads nothing, and fails when the idle deadline passes. | |||
| 283 | + | for range 1024 { | |||
| 284 | + | if _, err := w.Write(chunk); err != nil { | |||
| 285 | + | writeErr <- err | |||
| 286 | + | return err | |||
| 287 | + | } | |||
| 288 | + | } | |||
| 289 | + | writeErr <- nil | |||
| 290 | + | return nil | |||
| 291 | + | }).Once() | |||
| 292 | + | ||||
| 293 | + | // The client takes the headers and reads no body. | |||
| 294 | + | conn, err := net.Dial("tcp", strings.TrimPrefix(r.srv.URL, "http://")) | |||
| 295 | + | require.NoError(t, err) | |||
| 296 | + | t.Cleanup(func() { _ = conn.Close() }) | |||
| 297 | + | _, err = io.WriteString(conn, "GET "+Prefix+tarballPath+" HTTP/1.1\r\nHost: x\r\n\r\n") | |||
| 298 | + | require.NoError(t, err) | |||
| 299 | + | ||||
| 300 | + | select { | |||
| 301 | + | case err := <-writeErr: | |||
| 302 | + | require.Error(t, err, "the server gave up on a client that reads nothing") | |||
| 303 | + | assert.Contains(t, err.Error(), "i/o timeout") | |||
| 304 | + | case <-time.After(15 * time.Second): | |||
| 305 | + | t.Fatal("the stalled client was never cut") | |||
| 306 | + | } | |||
| 307 | + | ||||
| 308 | + | // The slot came back: with a cap of one per client, another download starts. | |||
| 309 | + | r.api.EXPECT().Tarball(mock.Anything, mock.Anything, "acme", "proj", "main", mock.Anything).RunAndReturn( | |||
| 310 | + | func(_ context.Context, _ domain.Principal, _, _, _ string, begin func(string) io.Writer) error { | |||
| 311 | + | _, err := begin("a.tar.gz").Write([]byte("fine")) | |||
| 312 | + | return err | |||
| 313 | + | }).Once() | |||
| 314 | + | require.Eventually(t, func() bool { | |||
| 315 | + | resp, body := r.do(t, http.MethodGet, tarballPath, "") | |||
| 316 | + | return resp.StatusCode == http.StatusOK && body == "fine" | |||
| 317 | + | }, 5*time.Second, 50*time.Millisecond) | |||
| 318 | + | } | |||
| 319 | + | ||||
| 320 | + | // The credentials of a request are the forge's: Basic with any user, Bearer or | |||
| 321 | + | // token; none is anonymous and does not touch the failure limiter. | |||
| 322 | + | func TestCredentialsAndNotFound(t *testing.T) { | |||
| 323 | + | t.Parallel() | |||
| 324 | + | r := newRig(t, nil) | |||
| 325 | + | p := domain.Root() | |||
| 326 | + | r.auth.EXPECT().Authenticate(mock.Anything, "good").Return(p, nil) | |||
| 327 | + | r.auth.EXPECT().Authenticate(mock.Anything, "bad").Return(domain.Principal{}, domain.ErrUnauthenticated) | |||
| 328 | + | // A header that carries no usable token authenticates the empty secret. | |||
| 329 | + | r.auth.EXPECT().Authenticate(mock.Anything, "").Return(domain.Principal{}, domain.ErrUnauthenticated) | |||
| 330 | + | r.api.EXPECT().HashAlgorithm(mock.Anything, p, "acme", "proj").Return("sha1", nil) | |||
| 331 | + | r.api.EXPECT().HashAlgorithm(mock.Anything, domain.Anonymous(), "acme", "proj").Return("sha1", nil) | |||
| 332 | + | ||||
| 333 | + | path := "/repos/acme/proj/hash-algorithm" | |||
| 334 | + | for _, h := range []string{"Basic eC1hY2Nlc3MtdG9rZW46Z29vZA==", "Bearer good", "token good", "bearer good"} { | |||
| 335 | + | resp, body := r.do(t, http.MethodGet, path, h) | |||
| 336 | + | assert.Equal(t, http.StatusOK, resp.StatusCode, h) | |||
| 337 | + | assert.JSONEq(t, `{"hash_algorithm":"sha1"}`, body) | |||
| 338 | + | } | |||
| 339 | + | resp, _ := r.do(t, http.MethodGet, path, "") | |||
| 340 | + | assert.Equal(t, http.StatusOK, resp.StatusCode, "anonymous") | |||
| 341 | + | for _, h := range []string{"Bearer bad", "Digest x", "Basic !!!", "Bearer"} { | |||
| 342 | + | resp, body := r.do(t, http.MethodGet, path, h) | |||
| 343 | + | assert.Equal(t, http.StatusUnauthorized, resp.StatusCode, h) | |||
| 344 | + | assert.Contains(t, body, "Bad credentials") | |||
| 345 | + | } | |||
| 346 | + | ||||
| 347 | + | // Not found is GitHub's body, for an unknown endpoint and for a use case that | |||
| 348 | + | // says so, and a method other than GET and HEAD is 405. | |||
| 349 | + | const gh = `{"message":"Not Found","documentation_url":"https://docs.github.com/rest"}` | |||
| 350 | + | for _, p := range []string{"", "/", "/user", "/repos", "/repos/acme", "/repos//proj", "/repos/acme/proj/issues", "/repos/acme/proj/hash-algorithm/x"} { | |||
| 351 | + | resp, body := r.do(t, http.MethodGet, p, "") | |||
| 352 | + | assert.Equal(t, http.StatusNotFound, resp.StatusCode, p) | |||
| 353 | + | assert.JSONEq(t, gh, body, p) | |||
| 354 | + | } | |||
| 355 | + | r.api.EXPECT().GetRepo(mock.Anything, domain.Anonymous(), "acme", "gone").Return(nil, domain.ErrNotFound) | |||
| 356 | + | resp, body := r.do(t, http.MethodGet, "/repos/acme/gone", "") | |||
| 357 | + | assert.Equal(t, http.StatusNotFound, resp.StatusCode) | |||
| 358 | + | assert.JSONEq(t, gh, body) | |||
| 359 | + | req, _ := http.NewRequest(http.MethodPost, r.srv.URL+Prefix+path, nil) | |||
| 360 | + | post, err := http.DefaultClient.Do(req) | |||
| 361 | + | require.NoError(t, err) | |||
| 362 | + | _ = post.Body.Close() | |||
| 363 | + | assert.Equal(t, http.StatusMethodNotAllowed, post.StatusCode) | |||
| 364 | + | assert.Equal(t, "GET, HEAD", post.Header.Get("Allow")) | |||
| 365 | + | ||||
| 366 | + | // A ref with slashes is passed whole. | |||
| 367 | + | r.api.EXPECT().TarballName(mock.Anything, domain.Anonymous(), "acme", "proj", "feature/x/y").Return("n.tar.gz", nil) | |||
| 368 | + | resp, _ = r.do(t, http.MethodHead, "/repos/acme/proj/tarball/feature/x/y", "") | |||
| 369 | + | assert.Equal(t, http.StatusOK, resp.StatusCode) | |||
| 370 | + | } | |||
M internal/app/ci_execution_test.go +237 -7
| @@ -944,13 +944,6 @@ func TestResolveActionsOfAnotherRepositoryNeedsAPusherWhoReadsIt(t *testing.T) { | |||
| 944 | 944 | require.Empty(t, got[0].GetError()) | |
| 945 | 945 | assert.Equal(t, pb.ActionSource_ACTION_SOURCE_ERMITE, got[0].GetSource()) | |
| 946 | 946 | }) | |
| 947 | - | ||
| 948 | - | t.Run("a merge made by the owner's session", func(t *testing.T) { | |
| 949 | - | // The runs of merged requests record the owner: a principal that reads | |
| 950 | - | // everything. | |
| 951 | - | p := domain.PusherOf(domain.Owner()) | |
| 952 | - | assert.Equal(t, domain.CIPusherOwner, p.Kind) | |
| 953 | - | }) | |
| 954 | 947 | } | |
| 955 | 948 | ||
| 956 | 949 | // GO-001b: the archive token is bound to (repository, commit). Only the archive | |
| @@ -1309,3 +1302,240 @@ func TestResolveActionsBoundsRemoteWork(t *testing.T) { | |||
| 1309 | 1302 | assert.Empty(t, ok[0].GetError()) | |
| 1310 | 1303 | }) | |
| 1311 | 1304 | } | |
| 1305 | + | ||
| 1306 | + | // mergeWorkflow runs on main only, so that the branch that carries it runs | |
| 1307 | + | // nothing until it is merged. | |
| 1308 | + | const mergeWorkflow = `on: | |
| 1309 | + | push: | |
| 1310 | + | branches: [main] | |
| 1311 | + | jobs: | |
| 1312 | + | j: | |
| 1313 | + | runs-on: ubuntu-latest | |
| 1314 | + | steps: | |
| 1315 | + | - uses: owner/private-action@v1 | |
| 1316 | + | ` | |
| 1317 | + | ||
| 1318 | + | // GO-R1: a merge runs the workflow with the rights of the principal that | |
| 1319 | + | // merged, not the owner's: a token limited to a repository that merges a | |
| 1320 | + | // workflow using a private action of another repository is told "repository not | |
| 1321 | + | // found", while the owner merging the same workflow gets the action. The actor | |
| 1322 | + | // shown on the run stays the owner, as GitHub shows it. | |
| 1323 | + | func TestMergeResolvesActionsWithTheRightsOfTheMerger(t *testing.T) { | |
| 1324 | + | merge := func(c *ciHarness, as func(mr int64, head string) error) (*pb.Job, *pb.Run) { | |
| 1325 | + | c.t.Helper() | |
| 1326 | + | runner := c.registerRunner("alpha", "ubuntu-latest") | |
| 1327 | + | c.push("main", map[string]string{"README": "x\n"}, "") | |
| 1328 | + | feat := c.push("feat", map[string]string{".github/workflows/m.yml": mergeWorkflow}, "") | |
| 1329 | + | mr, err := c.root.mr.CreateMergeRequest(c.ctx, connect.NewRequest(&pb.CreateMergeRequestRequest{Repo: "proj", Title: "CI", SourceBranch: "feat"})) | |
| 1330 | + | require.NoError(c.t, err) | |
| 1331 | + | require.Empty(c.t, c.runs(), "nothing runs before the merge") | |
| 1332 | + | require.NoError(c.t, as(mr.Msg.GetMergeRequest().GetNumber(), feat)) | |
| 1333 | + | job := runner.await() | |
| 1334 | + | c.heartbeat(runner, job) | |
| 1335 | + | return job, c.runs()[0] | |
| 1336 | + | } | |
| 1337 | + | setup := func(t *testing.T) *ciHarness { | |
| 1338 | + | c := newExecHarness(t, newFakeRemote(t), nil) | |
| 1339 | + | c.seed("private-action", domain.VisibilityPrivate, "action.yml") | |
| 1340 | + | c.mustGit(c.repoDir("private-action"), "tag", "v1") | |
| 1341 | + | return c | |
| 1342 | + | } | |
| 1343 | + | asToken := func(c *ciHarness, secret string) func(int64, string) error { | |
| 1344 | + | return func(n int64, head string) error { | |
| 1345 | + | _, err := c.api(secret).mr.MergeMergeRequest(c.ctx, connect.NewRequest(&pb.MergeMergeRequestRequest{Repo: "proj", Number: n, HeadId: head})) | |
| 1346 | + | return err | |
| 1347 | + | } | |
| 1348 | + | } | |
| 1349 | + | ||
| 1350 | + | t.Run("a token limited to the repository", func(t *testing.T) { | |
| 1351 | + | c := setup(t) | |
| 1352 | + | secret, _ := c.branchToken([]domain.Scope{domain.ScopeRepoRead, domain.ScopeRepoWrite, domain.ScopeMRMerge}, nil, "proj") | |
| 1353 | + | job, run := merge(c, asToken(c, secret)) | |
| 1354 | + | assert.Equal(t, "owner", run.GetActor(), "GitHub shows the owner as the actor of a merge") | |
| 1355 | + | got := c.resolve(&fakeRunner{c: c, api: c.api(job.GetJobToken())}, job, "owner/private-action@v1") | |
| 1356 | + | assert.Equal(t, "Unable to resolve action `owner/private-action@v1`, repository not found", got[0].GetError()) | |
| 1357 | + | assert.Empty(t, got[0].GetToken()) | |
| 1358 | + | }) | |
| 1359 | + | ||
| 1360 | + | t.Run("a token that lists both repositories", func(t *testing.T) { | |
| 1361 | + | c := setup(t) | |
| 1362 | + | secret, _ := c.branchToken([]domain.Scope{domain.ScopeRepoRead, domain.ScopeRepoWrite, domain.ScopeMRMerge}, nil, "proj", "private-action") | |
| 1363 | + | job, _ := merge(c, asToken(c, secret)) | |
| 1364 | + | got := c.resolve(&fakeRunner{c: c, api: c.api(job.GetJobToken())}, job, "owner/private-action@v1") | |
| 1365 | + | require.Empty(t, got[0].GetError()) | |
| 1366 | + | assert.Equal(t, pb.ActionSource_ACTION_SOURCE_ERMITE, got[0].GetSource()) | |
| 1367 | + | }) | |
| 1368 | + | ||
| 1369 | + | t.Run("the owner", func(t *testing.T) { | |
| 1370 | + | c := setup(t) | |
| 1371 | + | job, run := merge(c, func(n int64, head string) error { | |
| 1372 | + | id, err := domain.ParseObjectID(head) | |
| 1373 | + | require.NoError(t, err) | |
| 1374 | + | _, err = c.forge.MergeRequests.Merge(c.ctx, domain.Owner(), c.h("proj"), n, ports.MergeParams{HeadID: id}) | |
| 1375 | + | return err | |
| 1376 | + | }) | |
| 1377 | + | assert.Equal(t, "owner", run.GetActor()) | |
| 1378 | + | got := c.resolve(&fakeRunner{c: c, api: c.api(job.GetJobToken())}, job, "owner/private-action@v1") | |
| 1379 | + | require.Empty(t, got[0].GetError()) | |
| 1380 | + | assert.Equal(t, pb.ActionSource_ACTION_SOURCE_ERMITE, got[0].GetSource()) | |
| 1381 | + | assert.True(t, domain.IsRepoToken(got[0].GetToken())) | |
| 1382 | + | }) | |
| 1383 | + | ||
| 1384 | + | t.Run("the admin token", func(t *testing.T) { | |
| 1385 | + | c := setup(t) | |
| 1386 | + | job, _ := merge(c, asToken(c, adminToken)) | |
| 1387 | + | got := c.resolve(&fakeRunner{c: c, api: c.api(job.GetJobToken())}, job, "owner/private-action@v1") | |
| 1388 | + | require.Empty(t, got[0].GetError()) | |
| 1389 | + | }) | |
| 1390 | + | } | |
| 1391 | + | ||
| 1392 | + | // GO-R4: a repository of this server that the pusher cannot read never falls | |
| 1393 | + | // through to github.com under the same name, at the first resolution and when | |
| 1394 | + | // the answer comes from a stored one; only a repository that does not exist | |
| 1395 | + | // here is github.com's. | |
| 1396 | + | func TestAnActionRepositoryOfTheServerNeverFallsThroughToGitHub(t *testing.T) { | |
| 1397 | + | const ref = "owner/b@v1" | |
| 1398 | + | notFound := "Unable to resolve action `owner/b@v1`, repository not found" | |
| 1399 | + | limited := func(c *ciHarness) string { | |
| 1400 | + | s, _ := c.branchToken([]domain.Scope{domain.ScopeRepoRead, domain.ScopeRepoWrite}, nil, "proj") | |
| 1401 | + | return s | |
| 1402 | + | } | |
| 1403 | + | setup := func(t *testing.T) (*ciHarness, *fakeRemote, *fakeRunner) { | |
| 1404 | + | remote := newFakeRemote(t) | |
| 1405 | + | remote.set("owner/b", ptrRef("refs/tags/v1", shaOther)) // the repository of the same name on github.com | |
| 1406 | + | c := newExecHarness(t, remote, nil) | |
| 1407 | + | return c, remote, c.registerRunner("alpha", "ubuntu-latest") | |
| 1408 | + | } | |
| 1409 | + | // run pushes the workflow (first) or a new commit as the secret and returns the | |
| 1410 | + | // job the push made. | |
| 1411 | + | run := func(c *ciHarness, runner *fakeRunner, secret string, files map[string]string) *pb.Job { | |
| 1412 | + | c.t.Helper() | |
| 1413 | + | c.pushAs(secret, "main", files) | |
| 1414 | + | job := runner.await() | |
| 1415 | + | c.heartbeat(runner, job) | |
| 1416 | + | return job | |
| 1417 | + | } | |
| 1418 | + | resolve := func(c *ciHarness, job *pb.Job) *pb.ResolvedAction { | |
| 1419 | + | return c.resolve(&fakeRunner{c: c, api: c.api(job.GetJobToken())}, job, ref)[0] | |
| 1420 | + | } | |
| 1421 | + | ||
| 1422 | + | t.Run("first resolution", func(t *testing.T) { | |
| 1423 | + | c, remote, runner := setup(t) | |
| 1424 | + | c.seed("b", domain.VisibilityPrivate, "action.yml") | |
| 1425 | + | c.mustGit(c.repoDir("b"), "tag", "v1") | |
| 1426 | + | job := run(c, runner, limited(c), map[string]string{oneJobWorkflow: oneJobYAML()}) | |
| 1427 | + | got := resolve(c, job) | |
| 1428 | + | assert.Equal(t, notFound, got.GetError(), "the same text as for a repository that is not there") | |
| 1429 | + | assert.Empty(t, got.GetResolvedSha()) | |
| 1430 | + | assert.Empty(t, got.GetTarballUrl()) | |
| 1431 | + | assert.Empty(t, got.GetToken()) | |
| 1432 | + | assert.False(t, remote.asked("owner/b"), "github.com is never asked about a name the server holds") | |
| 1433 | + | // And not the second time, from the same attempt. | |
| 1434 | + | assert.Equal(t, notFound, resolve(c, job).GetError()) | |
| 1435 | + | assert.False(t, remote.asked("owner/b")) | |
| 1436 | + | }) | |
| 1437 | + | ||
| 1438 | + | t.Run("a pusher who reads it, then one who does not, on the same ref", func(t *testing.T) { | |
| 1439 | + | c, remote, runner := setup(t) | |
| 1440 | + | c.seed("b", domain.VisibilityPrivate, "action.yml") | |
| 1441 | + | c.mustGit(c.repoDir("b"), "tag", "v1") | |
| 1442 | + | bSha := c.bareGit("b", "rev-parse", "HEAD") | |
| 1443 | + | ||
| 1444 | + | first := run(c, runner, adminToken, map[string]string{oneJobWorkflow: oneJobYAML()}) | |
| 1445 | + | got := resolve(c, first) | |
| 1446 | + | require.Empty(t, got.GetError()) | |
| 1447 | + | assert.Equal(t, pb.ActionSource_ACTION_SOURCE_ERMITE, got.GetSource()) | |
| 1448 | + | assert.Equal(t, bSha, got.GetResolvedSha()) | |
| 1449 | + | ||
| 1450 | + | second := run(c, runner, limited(c), map[string]string{"two.txt": "2\n"}) | |
| 1451 | + | got = resolve(c, second) | |
| 1452 | + | assert.Equal(t, notFound, got.GetError(), "another run, another pusher: its own answer") | |
| 1453 | + | assert.Empty(t, got.GetToken()) | |
| 1454 | + | assert.False(t, remote.asked("owner/b")) | |
| 1455 | + | // The first run still resolves. | |
| 1456 | + | assert.Equal(t, bSha, resolve(c, first).GetResolvedSha()) | |
| 1457 | + | }) | |
| 1458 | + | ||
| 1459 | + | t.Run("a pusher who does not read it, then one who does, on the same ref", func(t *testing.T) { | |
| 1460 | + | c, remote, runner := setup(t) | |
| 1461 | + | c.seed("b", domain.VisibilityPrivate, "action.yml") | |
| 1462 | + | c.mustGit(c.repoDir("b"), "tag", "v1") | |
| 1463 | + | bSha := c.bareGit("b", "rev-parse", "HEAD") | |
| 1464 | + | ||
| 1465 | + | first := run(c, runner, limited(c), map[string]string{oneJobWorkflow: oneJobYAML()}) | |
| 1466 | + | assert.Equal(t, notFound, resolve(c, first).GetError()) | |
| 1467 | + | ||
| 1468 | + | second := run(c, runner, adminToken, map[string]string{"two.txt": "2\n"}) | |
| 1469 | + | got := resolve(c, second) | |
| 1470 | + | require.Empty(t, got.GetError(), "the refusal of the first run is not kept for the second") | |
| 1471 | + | assert.Equal(t, pb.ActionSource_ACTION_SOURCE_ERMITE, got.GetSource()) | |
| 1472 | + | assert.Equal(t, bSha, got.GetResolvedSha()) | |
| 1473 | + | assert.NotEqual(t, strings.ToLower(shaOther), got.GetResolvedSha(), "not github.com's commit") | |
| 1474 | + | assert.False(t, remote.asked("owner/b")) | |
| 1475 | + | assert.Equal(t, notFound, resolve(c, first).GetError(), "and the first run is still refused") | |
| 1476 | + | }) | |
| 1477 | + | ||
| 1478 | + | t.Run("a stored resolution from github.com, then the repository appears here", func(t *testing.T) { | |
| 1479 | + | c, remote, runner := setup(t) | |
| 1480 | + | job := run(c, runner, limited(c), map[string]string{oneJobWorkflow: oneJobYAML()}) | |
| 1481 | + | before := resolve(c, job) | |
| 1482 | + | require.Empty(t, before.GetError(), "no such repository here: github.com answers") | |
| 1483 | + | assert.Equal(t, pb.ActionSource_ACTION_SOURCE_GITHUB, before.GetSource()) | |
| 1484 | + | assert.Equal(t, strings.ToLower(shaOther), before.GetResolvedSha()) | |
| 1485 | + | require.Equal(t, 1, remote.count()) | |
| 1486 | + | ||
| 1487 | + | // The name is taken on the server, privately. | |
| 1488 | + | c.seed("b", domain.VisibilityPrivate, "action.yml") | |
| 1489 | + | after := resolve(c, job) | |
| 1490 | + | assert.Equal(t, notFound, after.GetError(), "the stored github.com answer is not served for a repository the pusher may not read") | |
| 1491 | + | assert.Empty(t, after.GetResolvedSha()) | |
| 1492 | + | assert.Empty(t, after.GetTarballUrl()) | |
| 1493 | + | assert.Equal(t, 1, remote.count(), "and github.com was not asked again") | |
| 1494 | + | }) | |
| 1495 | + | ||
| 1496 | + | t.Run("a stored resolution from github.com, a pusher who reads the new repository", func(t *testing.T) { | |
| 1497 | + | c, _, runner := setup(t) | |
| 1498 | + | job := run(c, runner, adminToken, map[string]string{oneJobWorkflow: oneJobYAML()}) | |
| 1499 | + | before := resolve(c, job) | |
| 1500 | + | require.Equal(t, pb.ActionSource_ACTION_SOURCE_GITHUB, before.GetSource()) | |
| 1501 | + | c.seed("b", domain.VisibilityPrivate, "action.yml") | |
| 1502 | + | after := resolve(c, job) | |
| 1503 | + | require.Empty(t, after.GetError()) | |
| 1504 | + | assert.Equal(t, before.GetResolvedSha(), after.GetResolvedSha(), "stable for the attempt") | |
| 1505 | + | assert.Empty(t, after.GetToken(), "no archive token for a commit of github.com") | |
| 1506 | + | }) | |
| 1507 | + | ||
| 1508 | + | t.Run("a stored resolution from the server, then the repository goes", func(t *testing.T) { | |
| 1509 | + | c, remote, runner := setup(t) | |
| 1510 | + | c.seed("b", domain.VisibilityPrivate, "action.yml") | |
| 1511 | + | c.mustGit(c.repoDir("b"), "tag", "v1") | |
| 1512 | + | job := run(c, runner, adminToken, map[string]string{oneJobWorkflow: oneJobYAML()}) | |
| 1513 | + | require.Empty(t, resolve(c, job).GetError()) | |
| 1514 | + | require.NoError(t, c.forge.Repos.Delete(c.ctx, domain.Root(), c.h("b"))) | |
| 1515 | + | got := resolve(c, job) | |
| 1516 | + | assert.Equal(t, notFound, got.GetError(), "there is nothing left to serve the stored commit from") | |
| 1517 | + | assert.Empty(t, got.GetToken()) | |
| 1518 | + | assert.False(t, remote.asked("owner/b")) | |
| 1519 | + | }) | |
| 1520 | + | ||
| 1521 | + | t.Run("a repository that does not exist here is github.com's", func(t *testing.T) { | |
| 1522 | + | c, remote, runner := setup(t) | |
| 1523 | + | job := run(c, runner, limited(c), map[string]string{oneJobWorkflow: oneJobYAML()}) | |
| 1524 | + | got := resolve(c, job) | |
| 1525 | + | require.Empty(t, got.GetError()) | |
| 1526 | + | assert.Equal(t, pb.ActionSource_ACTION_SOURCE_GITHUB, got.GetSource()) | |
| 1527 | + | assert.True(t, remote.asked("owner/b")) | |
| 1528 | + | }) | |
| 1529 | + | ||
| 1530 | + | t.Run("the owner's case does not open a way around", func(t *testing.T) { | |
| 1531 | + | c, remote, runner := setup(t) | |
| 1532 | + | c.seed("b", domain.VisibilityPrivate, "action.yml") | |
| 1533 | + | c.mustGit(c.repoDir("b"), "tag", "v1") | |
| 1534 | + | job := run(c, runner, limited(c), map[string]string{oneJobWorkflow: oneJobYAML()}) | |
| 1535 | + | for _, u := range []string{"OWNER/b@v1", "Owner/B@v1", "owner/B@v1"} { | |
| 1536 | + | got := c.resolve(&fakeRunner{c: c, api: c.api(job.GetJobToken())}, job, u)[0] | |
| 1537 | + | assert.Equal(t, "Unable to resolve action `"+strings.TrimSuffix(u, "@v1")+"@v1`, repository not found", got.GetError(), u) | |
| 1538 | + | } | |
| 1539 | + | assert.Zero(t, remote.count()) | |
| 1540 | + | }) | |
| 1541 | + | } | |
| @@ -944,13 +944,6 @@ func TestResolveActionsOfAnotherRepositoryNeedsAPusherWhoReadsIt(t *testing.T) { | |||||
| 944 | require.Empty(t, got[0].GetError()) | 944 | require.Empty(t, got[0].GetError()) | ||
| 945 | assert.Equal(t, pb.ActionSource_ACTION_SOURCE_ERMITE, got[0].GetSource()) | 945 | assert.Equal(t, pb.ActionSource_ACTION_SOURCE_ERMITE, got[0].GetSource()) | ||
| 946 | }) | 946 | }) | ||
| 947 | - | ||||
| 948 | - | t.Run("a merge made by the owner's session", func(t *testing.T) { | |||
| 949 | - | // The runs of merged requests record the owner: a principal that reads | |||
| 950 | - | // everything. | |||
| 951 | - | p := domain.PusherOf(domain.Owner()) | |||
| 952 | - | assert.Equal(t, domain.CIPusherOwner, p.Kind) | |||
| 953 | - | }) | |||
| 954 | } | 947 | } | ||
| 955 | 948 | ||||
| 956 | // GO-001b: the archive token is bound to (repository, commit). Only the archive | 949 | // GO-001b: the archive token is bound to (repository, commit). Only the archive | ||
| @@ -1309,3 +1302,240 @@ func TestResolveActionsBoundsRemoteWork(t *testing.T) { | |||||
| 1309 | assert.Empty(t, ok[0].GetError()) | 1302 | assert.Empty(t, ok[0].GetError()) | ||
| 1310 | }) | 1303 | }) | ||
| 1311 | } | 1304 | } | ||
| 1305 | + | ||||
| 1306 | + | // mergeWorkflow runs on main only, so that the branch that carries it runs | |||
| 1307 | + | // nothing until it is merged. | |||
| 1308 | + | const mergeWorkflow = `on: | |||
| 1309 | + | push: | |||
| 1310 | + | branches: [main] | |||
| 1311 | + | jobs: | |||
| 1312 | + | j: | |||
| 1313 | + | runs-on: ubuntu-latest | |||
| 1314 | + | steps: | |||
| 1315 | + | - uses: owner/private-action@v1 | |||
| 1316 | + | ` | |||
| 1317 | + | ||||
| 1318 | + | // GO-R1: a merge runs the workflow with the rights of the principal that | |||
| 1319 | + | // merged, not the owner's: a token limited to a repository that merges a | |||
| 1320 | + | // workflow using a private action of another repository is told "repository not | |||
| 1321 | + | // found", while the owner merging the same workflow gets the action. The actor | |||
| 1322 | + | // shown on the run stays the owner, as GitHub shows it. | |||
| 1323 | + | func TestMergeResolvesActionsWithTheRightsOfTheMerger(t *testing.T) { | |||
| 1324 | + | merge := func(c *ciHarness, as func(mr int64, head string) error) (*pb.Job, *pb.Run) { | |||
| 1325 | + | c.t.Helper() | |||
| 1326 | + | runner := c.registerRunner("alpha", "ubuntu-latest") | |||
| 1327 | + | c.push("main", map[string]string{"README": "x\n"}, "") | |||
| 1328 | + | feat := c.push("feat", map[string]string{".github/workflows/m.yml": mergeWorkflow}, "") | |||
| 1329 | + | mr, err := c.root.mr.CreateMergeRequest(c.ctx, connect.NewRequest(&pb.CreateMergeRequestRequest{Repo: "proj", Title: "CI", SourceBranch: "feat"})) | |||
| 1330 | + | require.NoError(c.t, err) | |||
| 1331 | + | require.Empty(c.t, c.runs(), "nothing runs before the merge") | |||
| 1332 | + | require.NoError(c.t, as(mr.Msg.GetMergeRequest().GetNumber(), feat)) | |||
| 1333 | + | job := runner.await() | |||
| 1334 | + | c.heartbeat(runner, job) | |||
| 1335 | + | return job, c.runs()[0] | |||
| 1336 | + | } | |||
| 1337 | + | setup := func(t *testing.T) *ciHarness { | |||
| 1338 | + | c := newExecHarness(t, newFakeRemote(t), nil) | |||
| 1339 | + | c.seed("private-action", domain.VisibilityPrivate, "action.yml") | |||
| 1340 | + | c.mustGit(c.repoDir("private-action"), "tag", "v1") | |||
| 1341 | + | return c | |||
| 1342 | + | } | |||
| 1343 | + | asToken := func(c *ciHarness, secret string) func(int64, string) error { | |||
| 1344 | + | return func(n int64, head string) error { | |||
| 1345 | + | _, err := c.api(secret).mr.MergeMergeRequest(c.ctx, connect.NewRequest(&pb.MergeMergeRequestRequest{Repo: "proj", Number: n, HeadId: head})) | |||
| 1346 | + | return err | |||
| 1347 | + | } | |||
| 1348 | + | } | |||
| 1349 | + | ||||
| 1350 | + | t.Run("a token limited to the repository", func(t *testing.T) { | |||
| 1351 | + | c := setup(t) | |||
| 1352 | + | secret, _ := c.branchToken([]domain.Scope{domain.ScopeRepoRead, domain.ScopeRepoWrite, domain.ScopeMRMerge}, nil, "proj") | |||
| 1353 | + | job, run := merge(c, asToken(c, secret)) | |||
| 1354 | + | assert.Equal(t, "owner", run.GetActor(), "GitHub shows the owner as the actor of a merge") | |||
| 1355 | + | got := c.resolve(&fakeRunner{c: c, api: c.api(job.GetJobToken())}, job, "owner/private-action@v1") | |||
| 1356 | + | assert.Equal(t, "Unable to resolve action `owner/private-action@v1`, repository not found", got[0].GetError()) | |||
| 1357 | + | assert.Empty(t, got[0].GetToken()) | |||
| 1358 | + | }) | |||
| 1359 | + | ||||
| 1360 | + | t.Run("a token that lists both repositories", func(t *testing.T) { | |||
| 1361 | + | c := setup(t) | |||
| 1362 | + | secret, _ := c.branchToken([]domain.Scope{domain.ScopeRepoRead, domain.ScopeRepoWrite, domain.ScopeMRMerge}, nil, "proj", "private-action") | |||
| 1363 | + | job, _ := merge(c, asToken(c, secret)) | |||
| 1364 | + | got := c.resolve(&fakeRunner{c: c, api: c.api(job.GetJobToken())}, job, "owner/private-action@v1") | |||
| 1365 | + | require.Empty(t, got[0].GetError()) | |||
| 1366 | + | assert.Equal(t, pb.ActionSource_ACTION_SOURCE_ERMITE, got[0].GetSource()) | |||
| 1367 | + | }) | |||
| 1368 | + | ||||
| 1369 | + | t.Run("the owner", func(t *testing.T) { | |||
| 1370 | + | c := setup(t) | |||
| 1371 | + | job, run := merge(c, func(n int64, head string) error { | |||
| 1372 | + | id, err := domain.ParseObjectID(head) | |||
| 1373 | + | require.NoError(t, err) | |||
| 1374 | + | _, err = c.forge.MergeRequests.Merge(c.ctx, domain.Owner(), c.h("proj"), n, ports.MergeParams{HeadID: id}) | |||
| 1375 | + | return err | |||
| 1376 | + | }) | |||
| 1377 | + | assert.Equal(t, "owner", run.GetActor()) | |||
| 1378 | + | got := c.resolve(&fakeRunner{c: c, api: c.api(job.GetJobToken())}, job, "owner/private-action@v1") | |||
| 1379 | + | require.Empty(t, got[0].GetError()) | |||
| 1380 | + | assert.Equal(t, pb.ActionSource_ACTION_SOURCE_ERMITE, got[0].GetSource()) | |||
| 1381 | + | assert.True(t, domain.IsRepoToken(got[0].GetToken())) | |||
| 1382 | + | }) | |||
| 1383 | + | ||||
| 1384 | + | t.Run("the admin token", func(t *testing.T) { | |||
| 1385 | + | c := setup(t) | |||
| 1386 | + | job, _ := merge(c, asToken(c, adminToken)) | |||
| 1387 | + | got := c.resolve(&fakeRunner{c: c, api: c.api(job.GetJobToken())}, job, "owner/private-action@v1") | |||
| 1388 | + | require.Empty(t, got[0].GetError()) | |||
| 1389 | + | }) | |||
| 1390 | + | } | |||
| 1391 | + | ||||
| 1392 | + | // GO-R4: a repository of this server that the pusher cannot read never falls | |||
| 1393 | + | // through to github.com under the same name, at the first resolution and when | |||
| 1394 | + | // the answer comes from a stored one; only a repository that does not exist | |||
| 1395 | + | // here is github.com's. | |||
| 1396 | + | func TestAnActionRepositoryOfTheServerNeverFallsThroughToGitHub(t *testing.T) { | |||
| 1397 | + | const ref = "owner/b@v1" | |||
| 1398 | + | notFound := "Unable to resolve action `owner/b@v1`, repository not found" | |||
| 1399 | + | limited := func(c *ciHarness) string { | |||
| 1400 | + | s, _ := c.branchToken([]domain.Scope{domain.ScopeRepoRead, domain.ScopeRepoWrite}, nil, "proj") | |||
| 1401 | + | return s | |||
| 1402 | + | } | |||
| 1403 | + | setup := func(t *testing.T) (*ciHarness, *fakeRemote, *fakeRunner) { | |||
| 1404 | + | remote := newFakeRemote(t) | |||
| 1405 | + | remote.set("owner/b", ptrRef("refs/tags/v1", shaOther)) // the repository of the same name on github.com | |||
| 1406 | + | c := newExecHarness(t, remote, nil) | |||
| 1407 | + | return c, remote, c.registerRunner("alpha", "ubuntu-latest") | |||
| 1408 | + | } | |||
| 1409 | + | // run pushes the workflow (first) or a new commit as the secret and returns the | |||
| 1410 | + | // job the push made. | |||
| 1411 | + | run := func(c *ciHarness, runner *fakeRunner, secret string, files map[string]string) *pb.Job { | |||
| 1412 | + | c.t.Helper() | |||
| 1413 | + | c.pushAs(secret, "main", files) | |||
| 1414 | + | job := runner.await() | |||
| 1415 | + | c.heartbeat(runner, job) | |||
| 1416 | + | return job | |||
| 1417 | + | } | |||
| 1418 | + | resolve := func(c *ciHarness, job *pb.Job) *pb.ResolvedAction { | |||
| 1419 | + | return c.resolve(&fakeRunner{c: c, api: c.api(job.GetJobToken())}, job, ref)[0] | |||
| 1420 | + | } | |||
| 1421 | + | ||||
| 1422 | + | t.Run("first resolution", func(t *testing.T) { | |||
| 1423 | + | c, remote, runner := setup(t) | |||
| 1424 | + | c.seed("b", domain.VisibilityPrivate, "action.yml") | |||
| 1425 | + | c.mustGit(c.repoDir("b"), "tag", "v1") | |||
| 1426 | + | job := run(c, runner, limited(c), map[string]string{oneJobWorkflow: oneJobYAML()}) | |||
| 1427 | + | got := resolve(c, job) | |||
| 1428 | + | assert.Equal(t, notFound, got.GetError(), "the same text as for a repository that is not there") | |||
| 1429 | + | assert.Empty(t, got.GetResolvedSha()) | |||
| 1430 | + | assert.Empty(t, got.GetTarballUrl()) | |||
| 1431 | + | assert.Empty(t, got.GetToken()) | |||
| 1432 | + | assert.False(t, remote.asked("owner/b"), "github.com is never asked about a name the server holds") | |||
| 1433 | + | // And not the second time, from the same attempt. | |||
| 1434 | + | assert.Equal(t, notFound, resolve(c, job).GetError()) | |||
| 1435 | + | assert.False(t, remote.asked("owner/b")) | |||
| 1436 | + | }) | |||
| 1437 | + | ||||
| 1438 | + | t.Run("a pusher who reads it, then one who does not, on the same ref", func(t *testing.T) { | |||
| 1439 | + | c, remote, runner := setup(t) | |||
| 1440 | + | c.seed("b", domain.VisibilityPrivate, "action.yml") | |||
| 1441 | + | c.mustGit(c.repoDir("b"), "tag", "v1") | |||
| 1442 | + | bSha := c.bareGit("b", "rev-parse", "HEAD") | |||
| 1443 | + | ||||
| 1444 | + | first := run(c, runner, adminToken, map[string]string{oneJobWorkflow: oneJobYAML()}) | |||
| 1445 | + | got := resolve(c, first) | |||
| 1446 | + | require.Empty(t, got.GetError()) | |||
| 1447 | + | assert.Equal(t, pb.ActionSource_ACTION_SOURCE_ERMITE, got.GetSource()) | |||
| 1448 | + | assert.Equal(t, bSha, got.GetResolvedSha()) | |||
| 1449 | + | ||||
| 1450 | + | second := run(c, runner, limited(c), map[string]string{"two.txt": "2\n"}) | |||
| 1451 | + | got = resolve(c, second) | |||
| 1452 | + | assert.Equal(t, notFound, got.GetError(), "another run, another pusher: its own answer") | |||
| 1453 | + | assert.Empty(t, got.GetToken()) | |||
| 1454 | + | assert.False(t, remote.asked("owner/b")) | |||
| 1455 | + | // The first run still resolves. | |||
| 1456 | + | assert.Equal(t, bSha, resolve(c, first).GetResolvedSha()) | |||
| 1457 | + | }) | |||
| 1458 | + | ||||
| 1459 | + | t.Run("a pusher who does not read it, then one who does, on the same ref", func(t *testing.T) { | |||
| 1460 | + | c, remote, runner := setup(t) | |||
| 1461 | + | c.seed("b", domain.VisibilityPrivate, "action.yml") | |||
| 1462 | + | c.mustGit(c.repoDir("b"), "tag", "v1") | |||
| 1463 | + | bSha := c.bareGit("b", "rev-parse", "HEAD") | |||
| 1464 | + | ||||
| 1465 | + | first := run(c, runner, limited(c), map[string]string{oneJobWorkflow: oneJobYAML()}) | |||
| 1466 | + | assert.Equal(t, notFound, resolve(c, first).GetError()) | |||
| 1467 | + | ||||
| 1468 | + | second := run(c, runner, adminToken, map[string]string{"two.txt": "2\n"}) | |||
| 1469 | + | got := resolve(c, second) | |||
| 1470 | + | require.Empty(t, got.GetError(), "the refusal of the first run is not kept for the second") | |||
| 1471 | + | assert.Equal(t, pb.ActionSource_ACTION_SOURCE_ERMITE, got.GetSource()) | |||
| 1472 | + | assert.Equal(t, bSha, got.GetResolvedSha()) | |||
| 1473 | + | assert.NotEqual(t, strings.ToLower(shaOther), got.GetResolvedSha(), "not github.com's commit") | |||
| 1474 | + | assert.False(t, remote.asked("owner/b")) | |||
| 1475 | + | assert.Equal(t, notFound, resolve(c, first).GetError(), "and the first run is still refused") | |||
| 1476 | + | }) | |||
| 1477 | + | ||||
| 1478 | + | t.Run("a stored resolution from github.com, then the repository appears here", func(t *testing.T) { | |||
| 1479 | + | c, remote, runner := setup(t) | |||
| 1480 | + | job := run(c, runner, limited(c), map[string]string{oneJobWorkflow: oneJobYAML()}) | |||
| 1481 | + | before := resolve(c, job) | |||
| 1482 | + | require.Empty(t, before.GetError(), "no such repository here: github.com answers") | |||
| 1483 | + | assert.Equal(t, pb.ActionSource_ACTION_SOURCE_GITHUB, before.GetSource()) | |||
| 1484 | + | assert.Equal(t, strings.ToLower(shaOther), before.GetResolvedSha()) | |||
| 1485 | + | require.Equal(t, 1, remote.count()) | |||
| 1486 | + | ||||
| 1487 | + | // The name is taken on the server, privately. | |||
| 1488 | + | c.seed("b", domain.VisibilityPrivate, "action.yml") | |||
| 1489 | + | after := resolve(c, job) | |||
| 1490 | + | assert.Equal(t, notFound, after.GetError(), "the stored github.com answer is not served for a repository the pusher may not read") | |||
| 1491 | + | assert.Empty(t, after.GetResolvedSha()) | |||
| 1492 | + | assert.Empty(t, after.GetTarballUrl()) | |||
| 1493 | + | assert.Equal(t, 1, remote.count(), "and github.com was not asked again") | |||
| 1494 | + | }) | |||
| 1495 | + | ||||
| 1496 | + | t.Run("a stored resolution from github.com, a pusher who reads the new repository", func(t *testing.T) { | |||
| 1497 | + | c, _, runner := setup(t) | |||
| 1498 | + | job := run(c, runner, adminToken, map[string]string{oneJobWorkflow: oneJobYAML()}) | |||
| 1499 | + | before := resolve(c, job) | |||
| 1500 | + | require.Equal(t, pb.ActionSource_ACTION_SOURCE_GITHUB, before.GetSource()) | |||
| 1501 | + | c.seed("b", domain.VisibilityPrivate, "action.yml") | |||
| 1502 | + | after := resolve(c, job) | |||
| 1503 | + | require.Empty(t, after.GetError()) | |||
| 1504 | + | assert.Equal(t, before.GetResolvedSha(), after.GetResolvedSha(), "stable for the attempt") | |||
| 1505 | + | assert.Empty(t, after.GetToken(), "no archive token for a commit of github.com") | |||
| 1506 | + | }) | |||
| 1507 | + | ||||
| 1508 | + | t.Run("a stored resolution from the server, then the repository goes", func(t *testing.T) { | |||
| 1509 | + | c, remote, runner := setup(t) | |||
| 1510 | + | c.seed("b", domain.VisibilityPrivate, "action.yml") | |||
| 1511 | + | c.mustGit(c.repoDir("b"), "tag", "v1") | |||
| 1512 | + | job := run(c, runner, adminToken, map[string]string{oneJobWorkflow: oneJobYAML()}) | |||
| 1513 | + | require.Empty(t, resolve(c, job).GetError()) | |||
| 1514 | + | require.NoError(t, c.forge.Repos.Delete(c.ctx, domain.Root(), c.h("b"))) | |||
| 1515 | + | got := resolve(c, job) | |||
| 1516 | + | assert.Equal(t, notFound, got.GetError(), "there is nothing left to serve the stored commit from") | |||
| 1517 | + | assert.Empty(t, got.GetToken()) | |||
| 1518 | + | assert.False(t, remote.asked("owner/b")) | |||
| 1519 | + | }) | |||
| 1520 | + | ||||
| 1521 | + | t.Run("a repository that does not exist here is github.com's", func(t *testing.T) { | |||
| 1522 | + | c, remote, runner := setup(t) | |||
| 1523 | + | job := run(c, runner, limited(c), map[string]string{oneJobWorkflow: oneJobYAML()}) | |||
| 1524 | + | got := resolve(c, job) | |||
| 1525 | + | require.Empty(t, got.GetError()) | |||
| 1526 | + | assert.Equal(t, pb.ActionSource_ACTION_SOURCE_GITHUB, got.GetSource()) | |||
| 1527 | + | assert.True(t, remote.asked("owner/b")) | |||
| 1528 | + | }) | |||
| 1529 | + | ||||
| 1530 | + | t.Run("the owner's case does not open a way around", func(t *testing.T) { | |||
| 1531 | + | c, remote, runner := setup(t) | |||
| 1532 | + | c.seed("b", domain.VisibilityPrivate, "action.yml") | |||
| 1533 | + | c.mustGit(c.repoDir("b"), "tag", "v1") | |||
| 1534 | + | job := run(c, runner, limited(c), map[string]string{oneJobWorkflow: oneJobYAML()}) | |||
| 1535 | + | for _, u := range []string{"OWNER/b@v1", "Owner/B@v1", "owner/B@v1"} { | |||
| 1536 | + | got := c.resolve(&fakeRunner{c: c, api: c.api(job.GetJobToken())}, job, u)[0] | |||
| 1537 | + | assert.Equal(t, "Unable to resolve action `"+strings.TrimSuffix(u, "@v1")+"@v1`, repository not found", got.GetError(), u) | |||
| 1538 | + | } | |||
| 1539 | + | assert.Zero(t, remote.count()) | |||
| 1540 | + | }) | |||
| 1541 | + | } | |||