fix(host): cache resolutions per job, batch big requests and validate the action path apart from owner and repository
- commit
- acae6da983023abd04290514f97780dc28186ff3
- author
- brice · · 2026-10-14 03:04 UTC
- parent
- 79b18d6
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
3 files · +220 -62
M internal/host/job.go +1 -0
| @@ -75,6 +75,7 @@ type jobRun struct { | |||
| 75 | 75 | // the budget of resolution requests of the job (see resolveActions). | |
| 76 | 76 | publicOrigin, serverOrigin, guestForge string | |
| 77 | 77 | resolveRounds, resolveRefs int | |
| 78 | + | resolved map[string]*execpb.ResolvedAction // by "uses", for the job | |
| 78 | 79 | // hard stops the sandbox at once; set before the session starts. | |
| 79 | 80 | hard func() | |
| 80 | 81 | result *execpb.JobResult | |
| @@ -75,6 +75,7 @@ type jobRun struct { | |||||
| 75 | // the budget of resolution requests of the job (see resolveActions). | 75 | // the budget of resolution requests of the job (see resolveActions). | ||
| 76 | publicOrigin, serverOrigin, guestForge string | 76 | publicOrigin, serverOrigin, guestForge string | ||
| 77 | resolveRounds, resolveRefs int | 77 | resolveRounds, resolveRefs int | ||
| 78 | + | resolved map[string]*execpb.ResolvedAction // by "uses", for the job | |||
| 78 | // hard stops the sandbox at once; set before the session starts. | 79 | // hard stops the sandbox at once; set before the session starts. | ||
| 79 | hard func() | 80 | hard func() | ||
| 80 | result *execpb.JobResult | 81 | result *execpb.JobResult | ||
M internal/host/resolve.go +120 -46
| @@ -7,6 +7,7 @@ import ( | |||
| 7 | 7 | "encoding/json" | |
| 8 | 8 | "fmt" | |
| 9 | 9 | "strings" | |
| 10 | + | "unicode/utf8" | |
| 10 | 11 | ||
| 11 | 12 | "connectrpc.com/connect" | |
| 12 | 13 | ||
| @@ -18,49 +19,67 @@ import ( | |||
| 18 | 19 | // Limits of what the sandbox may ask the server through the host. The sandbox | |
| 19 | 20 | // runs untrusted code, so the host does not trust its requests. | |
| 20 | 21 | // | |
| 21 | - | // A job asks once per level of the actions it uses (the runner's depth limit is | |
| 22 | - | // 9, so at most 10 rounds for the steps of the job and the repository | |
| 23 | - | // composites below them), and once more for every local composite action that | |
| 24 | - | // has repository actions in its steps, when its step runs: a workflow has at | |
| 25 | - | // most 1,000 steps. The budget below is generous for that and bounded: | |
| 26 | - | // maxResolveRounds requests and maxResolveRefsTotal references per job, whatever | |
| 27 | - | // they ask. (Local composites restart the depth at 1 as in the runner; if the | |
| 28 | - | // recording of ermite-conformance corpus/act-composite-depth shows that | |
| 29 | - | // github.com limits them, the executor will count them, and these limits stay | |
| 30 | - | // as the safety net.) | |
| 22 | + | // Resolutions are cached for the job, by "uses" value: an action that was | |
| 23 | + | // resolved once is never asked again (the same sha for the whole job is what the | |
| 24 | + | // server promises anyway), so a local composite action used 500 times costs one | |
| 25 | + | // resolution. The budget counts only what reaches the server: | |
| 26 | + | // | |
| 27 | + | // - a request from the sandbox has at most maxRequestRefs references (a | |
| 28 | + | // workflow has at most 1,000 steps), split into calls of maxResolveRefs, the | |
| 29 | + | // limit of ResolveActionsRequest; | |
| 30 | + | // - a job makes at most maxResolveRounds calls (up to 10 levels of repository | |
| 31 | + | // composites, then one call per local composite that brings new actions) and | |
| 32 | + | // asks for at most maxResolveRefsTotal distinct references. | |
| 33 | + | // | |
| 34 | + | // These are explicit limits of ermite, not of GitHub: a job that needs more | |
| 35 | + | // distinct actions than that fails "Set up job" with the message below. | |
| 31 | 36 | const ( | |
| 32 | - | maxResolveRefs = 100 // per request: ResolveActionsRequest.refs | |
| 33 | - | maxResolveRounds = 128 // per job | |
| 34 | - | maxResolveRefsTotal = 2048 // per job | |
| 35 | - | maxRefLen = 512 | |
| 36 | - | maxSegmentLen = 100 | |
| 37 | + | maxRequestRefs = 1000 | |
| 38 | + | maxResolveRefs = 100 // per call to the server: ResolveActionsRequest.refs | |
| 39 | + | maxResolveRounds = 128 // calls per job | |
| 40 | + | maxResolveRefsTotal = 2048 // distinct references per job | |
| 41 | + | maxRefLen = 1024 // the whole "uses" value | |
| 42 | + | maxOwnerRepoLen = 100 // owner and repository names | |
| 43 | + | maxPathElemLen = 255 // an element of the path of the action: a file name | |
| 37 | 44 | maxRefNameLen = 255 | |
| 38 | 45 | ) | |
| 39 | 46 | ||
| 40 | - | // checkUses validates a "uses" value of a request: owner/repo[/path]@ref with | |
| 41 | - | // the characters repository names, paths and refs have. The server decides | |
| 42 | - | // whether the action exists; this keeps garbage and oversized strings from | |
| 43 | - | // reaching it. | |
| 47 | + | // checkUses validates a "uses" value of a request: owner/repo[/path]@ref. The | |
| 48 | + | // owner and the repository have the characters of repository names; the path of | |
| 49 | + | // the action is a path in the repository (any characters but control ones, with | |
| 50 | + | // no empty, "." or ".." element and no backslash, which the runner reads as a | |
| 51 | + | // separator, action.ParseUses); the ref has the characters git allows. The | |
| 52 | + | // server decides whether the action exists; this keeps garbage and oversized | |
| 53 | + | // strings from reaching it. | |
| 44 | 54 | func checkUses(u string) error { | |
| 45 | - | if u == "" || len(u) > maxRefLen { | |
| 46 | - | return fmt.Errorf("%q is not a valid action reference", cut(u)) | |
| 55 | + | bad := func() error { return fmt.Errorf("%q is not a valid action reference", cut(u)) } | |
| 56 | + | if u == "" || len(u) > maxRefLen || !utf8.ValidString(u) { | |
| 57 | + | return bad() | |
| 47 | 58 | } | |
| 48 | 59 | name, ref, ok := strings.Cut(u, "@") | |
| 49 | 60 | if !ok || ref == "" || len(ref) > maxRefNameLen || strings.ContainsFunc(ref, badRefRune) { | |
| 50 | - | return fmt.Errorf("%q is not a valid action reference", cut(u)) | |
| 61 | + | return bad() | |
| 51 | 62 | } | |
| 52 | 63 | segs := strings.Split(name, "/") | |
| 53 | 64 | if len(segs) < 2 { | |
| 54 | - | return fmt.Errorf("%q is not a valid action reference", cut(u)) | |
| 65 | + | return bad() | |
| 66 | + | } | |
| 67 | + | for _, seg := range segs[:2] { | |
| 68 | + | if seg == "" || seg == "." || seg == ".." || len(seg) > maxOwnerRepoLen || strings.ContainsFunc(seg, badSegmentRune) { | |
| 69 | + | return bad() | |
| 70 | + | } | |
| 55 | 71 | } | |
| 56 | - | for _, seg := range segs { | |
| 57 | - | if seg == "" || seg == "." || seg == ".." || len(seg) > maxSegmentLen || strings.ContainsFunc(seg, badSegmentRune) { | |
| 58 | - | return fmt.Errorf("%q is not a valid action reference", cut(u)) | |
| 72 | + | for _, seg := range segs[2:] { | |
| 73 | + | if seg == "" || seg == "." || seg == ".." || len(seg) > maxPathElemLen || strings.ContainsFunc(seg, badPathRune) { | |
| 74 | + | return bad() | |
| 59 | 75 | } | |
| 60 | 76 | } | |
| 61 | 77 | return nil | |
| 62 | 78 | } | |
| 63 | 79 | ||
| 80 | + | // badPathRune: control characters (NUL included) and the backslash. | |
| 81 | + | func badPathRune(r rune) bool { return r < ' ' || r == 0x7f || r == '\\' || r == utf8.RuneError } | |
| 82 | + | ||
| 64 | 83 | func cut(s string) string { | |
| 65 | 84 | if len(s) > 80 { | |
| 66 | 85 | return s[:80] + "..." | |
| @@ -81,11 +100,13 @@ func badRefRune(r rune) bool { | |||
| 81 | 100 | // resolveActions answers a ResolveRequest of the executor with | |
| 82 | 101 | // RunnerService.ResolveActions. The job token stays here: the executor only | |
| 83 | 102 | // gets what the server returns (the per-archive tokens included, which it | |
| 84 | - | // masks). Requests come one at a time (see execstream.JobHandler.Resolve). A | |
| 85 | - | // call that fails without a verdict is retried like the other job calls; a | |
| 86 | - | // failure of the call itself is reported to the executor, which fails "Set up | |
| 87 | - | // job" (ActionManager: FailedToResolveActionDownloadInfoException). ctx ends | |
| 88 | - | // with the session. | |
| 103 | + | // masks). Requests come one at a time (see execstream.JobHandler.Resolve). What | |
| 104 | + | // was resolved before in the job comes from the cache; the rest goes to the | |
| 105 | + | // server in calls of at most maxResolveRefs, under the budget above. A call that | |
| 106 | + | // fails without a verdict is retried like the other job calls; a failure of the | |
| 107 | + | // call itself is reported to the executor, which fails "Set up job" | |
| 108 | + | // (ActionManager: FailedToResolveActionDownloadInfoException). ctx ends with the | |
| 109 | + | // session. | |
| 89 | 110 | func (j *jobRun) resolveActions(ctx context.Context, req *execpb.ResolveRequest) *execpb.ResolveResponse { | |
| 90 | 111 | fail := func(format string, a ...any) *execpb.ResolveResponse { | |
| 91 | 112 | return &execpb.ResolveResponse{Error: fmt.Sprintf(format, a...)} | |
| @@ -93,38 +114,91 @@ func (j *jobRun) resolveActions(ctx context.Context, req *execpb.ResolveRequest) | |||
| 93 | 114 | if len(req.Uses) == 0 { | |
| 94 | 115 | return fail("an empty request for action resolution") | |
| 95 | 116 | } | |
| 96 | - | if len(req.Uses) > maxResolveRefs { | |
| 97 | - | return fail("cannot resolve %d actions at once (the limit is %d)", len(req.Uses), maxResolveRefs) | |
| 117 | + | if len(req.Uses) > maxRequestRefs { | |
| 118 | + | return fail("cannot resolve %d actions at once (the limit is %d)", len(req.Uses), maxRequestRefs) | |
| 98 | 119 | } | |
| 99 | 120 | for _, u := range req.Uses { | |
| 100 | 121 | if err := checkUses(u); err != nil { | |
| 101 | 122 | return fail("%v", err) | |
| 102 | 123 | } | |
| 103 | 124 | } | |
| 125 | + | // The job ending, or the session, stops the calls. | |
| 126 | + | ctx, cancel := context.WithCancel(ctx) | |
| 127 | + | defer cancel() | |
| 128 | + | stop := context.AfterFunc(j.ctx, cancel) | |
| 129 | + | defer stop() | |
| 130 | + | ||
| 131 | + | got := map[string]*execpb.ResolvedAction{} | |
| 132 | + | var pending []string | |
| 104 | 133 | j.mu.Lock() | |
| 105 | - | j.resolveRounds++ | |
| 106 | - | j.resolveRefs += len(req.Uses) | |
| 107 | - | over := j.resolveRounds > maxResolveRounds || j.resolveRefs > maxResolveRefsTotal | |
| 134 | + | for _, u := range req.Uses { | |
| 135 | + | if _, dup := got[u]; dup { | |
| 136 | + | continue | |
| 137 | + | } | |
| 138 | + | if c, ok := j.resolved[u]; ok { | |
| 139 | + | got[u] = c | |
| 140 | + | continue | |
| 141 | + | } | |
| 142 | + | got[u] = nil | |
| 143 | + | pending = append(pending, u) | |
| 144 | + | } | |
| 108 | 145 | j.mu.Unlock() | |
| 109 | - | if over { | |
| 110 | - | return fail("this job asked to resolve too many actions (the limits are %d requests and %d references)", maxResolveRounds, maxResolveRefsTotal) | |
| 146 | + | for len(pending) > 0 { | |
| 147 | + | n := min(len(pending), maxResolveRefs) | |
| 148 | + | chunk := pending[:n] | |
| 149 | + | pending = pending[n:] | |
| 150 | + | j.mu.Lock() | |
| 151 | + | j.resolveRounds++ | |
| 152 | + | j.resolveRefs += len(chunk) | |
| 153 | + | over := j.resolveRounds > maxResolveRounds || j.resolveRefs > maxResolveRefsTotal | |
| 154 | + | j.mu.Unlock() | |
| 155 | + | if over { | |
| 156 | + | return fail("this job asked to resolve too many actions (the limits are %d requests and %d distinct references)", maxResolveRounds, maxResolveRefsTotal) | |
| 157 | + | } | |
| 158 | + | res, errResp := j.callResolve(ctx, chunk) | |
| 159 | + | if errResp != nil { | |
| 160 | + | return errResp | |
| 161 | + | } | |
| 162 | + | j.mu.Lock() | |
| 163 | + | for i, u := range chunk { | |
| 164 | + | got[u] = res[i] | |
| 165 | + | if res[i].Error == "" { // an error is not cached: it may be a passing one | |
| 166 | + | if j.resolved == nil { | |
| 167 | + | j.resolved = map[string]*execpb.ResolvedAction{} | |
| 168 | + | } | |
| 169 | + | j.resolved[u] = res[i] | |
| 170 | + | } | |
| 171 | + | } | |
| 172 | + | j.mu.Unlock() | |
| 173 | + | } | |
| 174 | + | out := &execpb.ResolveResponse{} | |
| 175 | + | for _, u := range req.Uses { | |
| 176 | + | out.Actions = append(out.Actions, got[u]) | |
| 111 | 177 | } | |
| 112 | - | refs := make([]*ermite.ActionRef, len(req.Uses)) | |
| 113 | - | for i, u := range req.Uses { | |
| 178 | + | return out | |
| 179 | + | } | |
| 180 | + | ||
| 181 | + | // callResolve makes one call to the server, with retries, and converts the | |
| 182 | + | // answer; the second result is the error to send to the executor. | |
| 183 | + | func (j *jobRun) callResolve(ctx context.Context, uses []string) ([]*execpb.ResolvedAction, *execpb.ResolveResponse) { | |
| 184 | + | fail := func(format string, a ...any) ([]*execpb.ResolvedAction, *execpb.ResolveResponse) { | |
| 185 | + | return nil, &execpb.ResolveResponse{Error: fmt.Sprintf(format, a...)} | |
| 186 | + | } | |
| 187 | + | refs := make([]*ermite.ActionRef, len(uses)) | |
| 188 | + | for i, u := range uses { | |
| 114 | 189 | refs[i] = &ermite.ActionRef{Uses: u} | |
| 115 | 190 | } | |
| 116 | - | // The job ending, or the session, stops the call. | |
| 117 | - | ctx, cancel := context.WithCancel(ctx) | |
| 118 | - | defer cancel() | |
| 119 | - | stop := context.AfterFunc(j.ctx, cancel) | |
| 120 | - | defer stop() | |
| 121 | 191 | b := backoff{min: j.t.BackoffMin, max: j.t.BackoffMax} | |
| 122 | 192 | for attempt := 1; ; attempt++ { | |
| 123 | 193 | cctx, ccancel := context.WithTimeout(ctx, j.t.CallTimeout) | |
| 124 | 194 | resp, err := j.client.ResolveActions(cctx, connect.NewRequest(&ermite.ResolveActionsRequest{JobId: j.job.JobId, Refs: refs})) | |
| 125 | 195 | ccancel() | |
| 126 | 196 | if err == nil { | |
| 127 | - | return j.resolveResponse(resp.Msg, len(req.Uses)) | |
| 197 | + | conv := j.resolveResponse(resp.Msg, len(uses)) | |
| 198 | + | if conv.Error != "" { | |
| 199 | + | return fail("%s", conv.Error) | |
| 200 | + | } | |
| 201 | + | return conv.Actions, nil | |
| 128 | 202 | } | |
| 129 | 203 | if ctx.Err() != nil { | |
| 130 | 204 | return fail("the job was cancelled while its actions were resolved") | |
| @@ -7,6 +7,7 @@ import ( | |||||
| 7 | "encoding/json" | 7 | "encoding/json" | ||
| 8 | "fmt" | 8 | "fmt" | ||
| 9 | "strings" | 9 | "strings" | ||
| 10 | + | "unicode/utf8" | |||
| 10 | 11 | ||||
| 11 | "connectrpc.com/connect" | 12 | "connectrpc.com/connect" | ||
| 12 | 13 | ||||
| @@ -18,49 +19,67 @@ import ( | |||||
| 18 | // Limits of what the sandbox may ask the server through the host. The sandbox | 19 | // Limits of what the sandbox may ask the server through the host. The sandbox | ||
| 19 | // runs untrusted code, so the host does not trust its requests. | 20 | // runs untrusted code, so the host does not trust its requests. | ||
| 20 | // | 21 | // | ||
| 21 | - | // A job asks once per level of the actions it uses (the runner's depth limit is | 22 | + | // Resolutions are cached for the job, by "uses" value: an action that was |
| 22 | - | // 9, so at most 10 rounds for the steps of the job and the repository | 23 | + | // resolved once is never asked again (the same sha for the whole job is what the |
| 23 | - | // composites below them), and once more for every local composite action that | 24 | + | // server promises anyway), so a local composite action used 500 times costs one |
| 24 | - | // has repository actions in its steps, when its step runs: a workflow has at | 25 | + | // resolution. The budget counts only what reaches the server: |
| 25 | - | // most 1,000 steps. The budget below is generous for that and bounded: | 26 | + | // |
| 26 | - | // maxResolveRounds requests and maxResolveRefsTotal references per job, whatever | 27 | + | // - a request from the sandbox has at most maxRequestRefs references (a |
| 27 | - | // they ask. (Local composites restart the depth at 1 as in the runner; if the | 28 | + | // workflow has at most 1,000 steps), split into calls of maxResolveRefs, the |
| 28 | - | // recording of ermite-conformance corpus/act-composite-depth shows that | 29 | + | // limit of ResolveActionsRequest; |
| 29 | - | // github.com limits them, the executor will count them, and these limits stay | 30 | + | // - a job makes at most maxResolveRounds calls (up to 10 levels of repository |
| 30 | - | // as the safety net.) | 31 | + | // composites, then one call per local composite that brings new actions) and |
| 32 | + | // asks for at most maxResolveRefsTotal distinct references. | |||
| 33 | + | // | |||
| 34 | + | // These are explicit limits of ermite, not of GitHub: a job that needs more | |||
| 35 | + | // distinct actions than that fails "Set up job" with the message below. | |||
| 31 | const ( | 36 | const ( | ||
| 32 | - | maxResolveRefs = 100 // per request: ResolveActionsRequest.refs | 37 | + | maxRequestRefs = 1000 |
| 33 | - | maxResolveRounds = 128 // per job | 38 | + | maxResolveRefs = 100 // per call to the server: ResolveActionsRequest.refs |
| 34 | - | maxResolveRefsTotal = 2048 // per job | 39 | + | maxResolveRounds = 128 // calls per job |
| 35 | - | maxRefLen = 512 | 40 | + | maxResolveRefsTotal = 2048 // distinct references per job |
| 36 | - | maxSegmentLen = 100 | 41 | + | maxRefLen = 1024 // the whole "uses" value |
| 42 | + | maxOwnerRepoLen = 100 // owner and repository names | |||
| 43 | + | maxPathElemLen = 255 // an element of the path of the action: a file name | |||
| 37 | maxRefNameLen = 255 | 44 | maxRefNameLen = 255 | ||
| 38 | ) | 45 | ) | ||
| 39 | 46 | ||||
| 40 | - | // checkUses validates a "uses" value of a request: owner/repo[/path]@ref with | 47 | + | // checkUses validates a "uses" value of a request: owner/repo[/path]@ref. The |
| 41 | - | // the characters repository names, paths and refs have. The server decides | 48 | + | // owner and the repository have the characters of repository names; the path of |
| 42 | - | // whether the action exists; this keeps garbage and oversized strings from | 49 | + | // the action is a path in the repository (any characters but control ones, with |
| 43 | - | // reaching it. | 50 | + | // no empty, "." or ".." element and no backslash, which the runner reads as a |
| 51 | + | // separator, action.ParseUses); the ref has the characters git allows. The | |||
| 52 | + | // server decides whether the action exists; this keeps garbage and oversized | |||
| 53 | + | // strings from reaching it. | |||
| 44 | func checkUses(u string) error { | 54 | func checkUses(u string) error { | ||
| 45 | - | if u == "" || len(u) > maxRefLen { | 55 | + | bad := func() error { return fmt.Errorf("%q is not a valid action reference", cut(u)) } |
| 46 | - | return fmt.Errorf("%q is not a valid action reference", cut(u)) | 56 | + | if u == "" || len(u) > maxRefLen || !utf8.ValidString(u) { |
| 57 | + | return bad() | |||
| 47 | } | 58 | } | ||
| 48 | name, ref, ok := strings.Cut(u, "@") | 59 | name, ref, ok := strings.Cut(u, "@") | ||
| 49 | if !ok || ref == "" || len(ref) > maxRefNameLen || strings.ContainsFunc(ref, badRefRune) { | 60 | if !ok || ref == "" || len(ref) > maxRefNameLen || strings.ContainsFunc(ref, badRefRune) { | ||
| 50 | - | return fmt.Errorf("%q is not a valid action reference", cut(u)) | 61 | + | return bad() |
| 51 | } | 62 | } | ||
| 52 | segs := strings.Split(name, "/") | 63 | segs := strings.Split(name, "/") | ||
| 53 | if len(segs) < 2 { | 64 | if len(segs) < 2 { | ||
| 54 | - | return fmt.Errorf("%q is not a valid action reference", cut(u)) | 65 | + | return bad() |
| 66 | + | } | |||
| 67 | + | for _, seg := range segs[:2] { | |||
| 68 | + | if seg == "" || seg == "." || seg == ".." || len(seg) > maxOwnerRepoLen || strings.ContainsFunc(seg, badSegmentRune) { | |||
| 69 | + | return bad() | |||
| 70 | + | } | |||
| 55 | } | 71 | } | ||
| 56 | - | for _, seg := range segs { | 72 | + | for _, seg := range segs[2:] { |
| 57 | - | if seg == "" || seg == "." || seg == ".." || len(seg) > maxSegmentLen || strings.ContainsFunc(seg, badSegmentRune) { | 73 | + | if seg == "" || seg == "." || seg == ".." || len(seg) > maxPathElemLen || strings.ContainsFunc(seg, badPathRune) { |
| 58 | - | return fmt.Errorf("%q is not a valid action reference", cut(u)) | 74 | + | return bad() |
| 59 | } | 75 | } | ||
| 60 | } | 76 | } | ||
| 61 | return nil | 77 | return nil | ||
| 62 | } | 78 | } | ||
| 63 | 79 | ||||
| 80 | + | // badPathRune: control characters (NUL included) and the backslash. | |||
| 81 | + | func badPathRune(r rune) bool { return r < ' ' || r == 0x7f || r == '\\' || r == utf8.RuneError } | |||
| 82 | + | ||||
| 64 | func cut(s string) string { | 83 | func cut(s string) string { | ||
| 65 | if len(s) > 80 { | 84 | if len(s) > 80 { | ||
| 66 | return s[:80] + "..." | 85 | return s[:80] + "..." | ||
| @@ -81,11 +100,13 @@ func badRefRune(r rune) bool { | |||||
| 81 | // resolveActions answers a ResolveRequest of the executor with | 100 | // resolveActions answers a ResolveRequest of the executor with | ||
| 82 | // RunnerService.ResolveActions. The job token stays here: the executor only | 101 | // RunnerService.ResolveActions. The job token stays here: the executor only | ||
| 83 | // gets what the server returns (the per-archive tokens included, which it | 102 | // gets what the server returns (the per-archive tokens included, which it | ||
| 84 | - | // masks). Requests come one at a time (see execstream.JobHandler.Resolve). A | 103 | + | // masks). Requests come one at a time (see execstream.JobHandler.Resolve). What |
| 85 | - | // call that fails without a verdict is retried like the other job calls; a | 104 | + | // was resolved before in the job comes from the cache; the rest goes to the |
| 86 | - | // failure of the call itself is reported to the executor, which fails "Set up | 105 | + | // server in calls of at most maxResolveRefs, under the budget above. A call that |
| 87 | - | // job" (ActionManager: FailedToResolveActionDownloadInfoException). ctx ends | 106 | + | // fails without a verdict is retried like the other job calls; a failure of the |
| 88 | - | // with the session. | 107 | + | // call itself is reported to the executor, which fails "Set up job" |
| 108 | + | // (ActionManager: FailedToResolveActionDownloadInfoException). ctx ends with the | |||
| 109 | + | // session. | |||
| 89 | func (j *jobRun) resolveActions(ctx context.Context, req *execpb.ResolveRequest) *execpb.ResolveResponse { | 110 | func (j *jobRun) resolveActions(ctx context.Context, req *execpb.ResolveRequest) *execpb.ResolveResponse { | ||
| 90 | fail := func(format string, a ...any) *execpb.ResolveResponse { | 111 | fail := func(format string, a ...any) *execpb.ResolveResponse { | ||
| 91 | return &execpb.ResolveResponse{Error: fmt.Sprintf(format, a...)} | 112 | return &execpb.ResolveResponse{Error: fmt.Sprintf(format, a...)} | ||
| @@ -93,38 +114,91 @@ func (j *jobRun) resolveActions(ctx context.Context, req *execpb.ResolveRequest) | |||||
| 93 | if len(req.Uses) == 0 { | 114 | if len(req.Uses) == 0 { | ||
| 94 | return fail("an empty request for action resolution") | 115 | return fail("an empty request for action resolution") | ||
| 95 | } | 116 | } | ||
| 96 | - | if len(req.Uses) > maxResolveRefs { | 117 | + | if len(req.Uses) > maxRequestRefs { |
| 97 | - | return fail("cannot resolve %d actions at once (the limit is %d)", len(req.Uses), maxResolveRefs) | 118 | + | return fail("cannot resolve %d actions at once (the limit is %d)", len(req.Uses), maxRequestRefs) |
| 98 | } | 119 | } | ||
| 99 | for _, u := range req.Uses { | 120 | for _, u := range req.Uses { | ||
| 100 | if err := checkUses(u); err != nil { | 121 | if err := checkUses(u); err != nil { | ||
| 101 | return fail("%v", err) | 122 | return fail("%v", err) | ||
| 102 | } | 123 | } | ||
| 103 | } | 124 | } | ||
| 125 | + | // The job ending, or the session, stops the calls. | |||
| 126 | + | ctx, cancel := context.WithCancel(ctx) | |||
| 127 | + | defer cancel() | |||
| 128 | + | stop := context.AfterFunc(j.ctx, cancel) | |||
| 129 | + | defer stop() | |||
| 130 | + | ||||
| 131 | + | got := map[string]*execpb.ResolvedAction{} | |||
| 132 | + | var pending []string | |||
| 104 | j.mu.Lock() | 133 | j.mu.Lock() | ||
| 105 | - | j.resolveRounds++ | 134 | + | for _, u := range req.Uses { |
| 106 | - | j.resolveRefs += len(req.Uses) | 135 | + | if _, dup := got[u]; dup { |
| 107 | - | over := j.resolveRounds > maxResolveRounds || j.resolveRefs > maxResolveRefsTotal | 136 | + | continue |
| 137 | + | } | |||
| 138 | + | if c, ok := j.resolved[u]; ok { | |||
| 139 | + | got[u] = c | |||
| 140 | + | continue | |||
| 141 | + | } | |||
| 142 | + | got[u] = nil | |||
| 143 | + | pending = append(pending, u) | |||
| 144 | + | } | |||
| 108 | j.mu.Unlock() | 145 | j.mu.Unlock() | ||
| 109 | - | if over { | 146 | + | for len(pending) > 0 { |
| 110 | - | return fail("this job asked to resolve too many actions (the limits are %d requests and %d references)", maxResolveRounds, maxResolveRefsTotal) | 147 | + | n := min(len(pending), maxResolveRefs) |
| 148 | + | chunk := pending[:n] | |||
| 149 | + | pending = pending[n:] | |||
| 150 | + | j.mu.Lock() | |||
| 151 | + | j.resolveRounds++ | |||
| 152 | + | j.resolveRefs += len(chunk) | |||
| 153 | + | over := j.resolveRounds > maxResolveRounds || j.resolveRefs > maxResolveRefsTotal | |||
| 154 | + | j.mu.Unlock() | |||
| 155 | + | if over { | |||
| 156 | + | return fail("this job asked to resolve too many actions (the limits are %d requests and %d distinct references)", maxResolveRounds, maxResolveRefsTotal) | |||
| 157 | + | } | |||
| 158 | + | res, errResp := j.callResolve(ctx, chunk) | |||
| 159 | + | if errResp != nil { | |||
| 160 | + | return errResp | |||
| 161 | + | } | |||
| 162 | + | j.mu.Lock() | |||
| 163 | + | for i, u := range chunk { | |||
| 164 | + | got[u] = res[i] | |||
| 165 | + | if res[i].Error == "" { // an error is not cached: it may be a passing one | |||
| 166 | + | if j.resolved == nil { | |||
| 167 | + | j.resolved = map[string]*execpb.ResolvedAction{} | |||
| 168 | + | } | |||
| 169 | + | j.resolved[u] = res[i] | |||
| 170 | + | } | |||
| 171 | + | } | |||
| 172 | + | j.mu.Unlock() | |||
| 173 | + | } | |||
| 174 | + | out := &execpb.ResolveResponse{} | |||
| 175 | + | for _, u := range req.Uses { | |||
| 176 | + | out.Actions = append(out.Actions, got[u]) | |||
| 111 | } | 177 | } | ||
| 112 | - | refs := make([]*ermite.ActionRef, len(req.Uses)) | 178 | + | return out |
| 113 | - | for i, u := range req.Uses { | 179 | + | } |
| 180 | + | ||||
| 181 | + | // callResolve makes one call to the server, with retries, and converts the | |||
| 182 | + | // answer; the second result is the error to send to the executor. | |||
| 183 | + | func (j *jobRun) callResolve(ctx context.Context, uses []string) ([]*execpb.ResolvedAction, *execpb.ResolveResponse) { | |||
| 184 | + | fail := func(format string, a ...any) ([]*execpb.ResolvedAction, *execpb.ResolveResponse) { | |||
| 185 | + | return nil, &execpb.ResolveResponse{Error: fmt.Sprintf(format, a...)} | |||
| 186 | + | } | |||
| 187 | + | refs := make([]*ermite.ActionRef, len(uses)) | |||
| 188 | + | for i, u := range uses { | |||
| 114 | refs[i] = &ermite.ActionRef{Uses: u} | 189 | refs[i] = &ermite.ActionRef{Uses: u} | ||
| 115 | } | 190 | } | ||
| 116 | - | // The job ending, or the session, stops the call. | |||
| 117 | - | ctx, cancel := context.WithCancel(ctx) | |||
| 118 | - | defer cancel() | |||
| 119 | - | stop := context.AfterFunc(j.ctx, cancel) | |||
| 120 | - | defer stop() | |||
| 121 | b := backoff{min: j.t.BackoffMin, max: j.t.BackoffMax} | 191 | b := backoff{min: j.t.BackoffMin, max: j.t.BackoffMax} | ||
| 122 | for attempt := 1; ; attempt++ { | 192 | for attempt := 1; ; attempt++ { | ||
| 123 | cctx, ccancel := context.WithTimeout(ctx, j.t.CallTimeout) | 193 | cctx, ccancel := context.WithTimeout(ctx, j.t.CallTimeout) | ||
| 124 | resp, err := j.client.ResolveActions(cctx, connect.NewRequest(&ermite.ResolveActionsRequest{JobId: j.job.JobId, Refs: refs})) | 194 | resp, err := j.client.ResolveActions(cctx, connect.NewRequest(&ermite.ResolveActionsRequest{JobId: j.job.JobId, Refs: refs})) | ||
| 125 | ccancel() | 195 | ccancel() | ||
| 126 | if err == nil { | 196 | if err == nil { | ||
| 127 | - | return j.resolveResponse(resp.Msg, len(req.Uses)) | 197 | + | conv := j.resolveResponse(resp.Msg, len(uses)) |
| 198 | + | if conv.Error != "" { | |||
| 199 | + | return fail("%s", conv.Error) | |||
| 200 | + | } | |||
| 201 | + | return conv.Actions, nil | |||
| 128 | } | 202 | } | ||
| 129 | if ctx.Err() != nil { | 203 | if ctx.Err() != nil { | ||
| 130 | return fail("the job was cancelled while its actions were resolved") | 204 | return fail("the job was cancelled while its actions were resolved") | ||
M internal/host/resolve_test.go +99 -16
| @@ -5,6 +5,7 @@ package host | |||
| 5 | 5 | import ( | |
| 6 | 6 | "context" | |
| 7 | 7 | "encoding/json" | |
| 8 | + | "fmt" | |
| 8 | 9 | "strings" | |
| 9 | 10 | "testing" | |
| 10 | 11 | "time" | |
| @@ -19,16 +20,26 @@ import ( | |||
| 19 | 20 | ||
| 20 | 21 | type resolveStub struct { | |
| 21 | 22 | stubClient | |
| 22 | - | got *ermite.ResolveActionsRequest | |
| 23 | - | resp *ermite.ResolveActionsResponse | |
| 24 | - | err error | |
| 23 | + | got *ermite.ResolveActionsRequest | |
| 24 | + | resp *ermite.ResolveActionsResponse | |
| 25 | + | err error | |
| 26 | + | fn func(*ermite.ResolveActionsRequest) *ermite.ResolveActionsResponse | |
| 27 | + | onCall func(*ermite.ResolveActionsRequest) | |
| 28 | + | calls int | |
| 25 | 29 | } | |
| 26 | 30 | ||
| 27 | 31 | func (c *resolveStub) ResolveActions(_ context.Context, r *connect.Request[ermite.ResolveActionsRequest]) (*connect.Response[ermite.ResolveActionsResponse], error) { | |
| 28 | 32 | c.got = r.Msg | |
| 33 | + | c.calls++ | |
| 34 | + | if c.onCall != nil { | |
| 35 | + | c.onCall(r.Msg) | |
| 36 | + | } | |
| 29 | 37 | if c.err != nil { | |
| 30 | 38 | return nil, c.err | |
| 31 | 39 | } | |
| 40 | + | if c.fn != nil { | |
| 41 | + | return connect.NewResponse(c.fn(r.Msg)), nil | |
| 42 | + | } | |
| 32 | 43 | return connect.NewResponse(c.resp), nil | |
| 33 | 44 | } | |
| 34 | 45 | ||
| @@ -99,15 +110,22 @@ func TestSetGitHubToken(t *testing.T) { | |||
| 99 | 110 | } | |
| 100 | 111 | ||
| 101 | 112 | func TestCheckUses(t *testing.T) { | |
| 113 | + | long := func(n int) string { return strings.Repeat("a", n) } | |
| 102 | 114 | for u, ok := range map[string]bool{ | |
| 103 | 115 | "actions/checkout@v4": true, "o/r/sub/dir@refs/heads/feat/x": true, "o/r@" + strings.Repeat("a", 40): true, "o/r.go_x-y@v1.2.3": true, | |
| 116 | + | // GO-011: the path of the action is a path, not a repository name. | |
| 117 | + | "o/r/action+tools@v1": true, "o/r/déploiement/日本語@v1": true, "o/r/with space/x@v1": true, "o/r/a(b)[c]{d}~e#f%g@v1": true, | |
| 118 | + | "o/r/" + long(255) + "@v1": true, "o/r/.github/actions/x@v1": true, "o/r/.hidden@v1": true, "o/r/a..b@v1": true, | |
| 104 | 119 | "": false, "o/r": false, "o@v1": false, "o/r@": false, "/r@v1": false, "o//r@v1": false, "o/../r@v1": false, "o/r/..@v1": false, | |
| 120 | + | "o/r/a/../b@v1": false, "o/r/./a@v1": false, "o/r/a//b@v1": false, "o/r/a/@v1": false, | |
| 121 | + | "o/r/a\\b@v1": false, "o/r/a\x00b@v1": false, "o/r/a\nb@v1": false, "o/r/a\x7fb@v1": false, "o/r/\xff@v1": false, | |
| 122 | + | "o/r/" + long(256) + "@v1": false, long(101) + "/r@v1": false, "o/" + long(101) + "@v1": false, | |
| 123 | + | "o/r@" + long(256): false, "o/r/" + strings.Repeat("a/", 600) + "x@v1": false, | |
| 105 | 124 | "o/r@v1 x": false, "o/r@v1\n": false, "o/r@v~1": false, "o/r@v:1": false, "o /r@v1": false, "o/r@v1@v2": false, | |
| 106 | - | "o/r\x00@v1": false, strings.Repeat("a", 101) + "/r@v1": false, "o/r@" + strings.Repeat("a", 256): false, | |
| 107 | - | "o/" + strings.Repeat("a/", 300) + "r@v1": false, "ö/r@v1": false, | |
| 125 | + | "o/r\x00@v1": false, "ö/r@v1": false, "o/r+x@v1": false, "o/r:x@v1": false, | |
| 108 | 126 | } { | |
| 109 | 127 | if err := checkUses(u); (err == nil) != ok { | |
| 110 | - | t.Errorf("%q: %v, want ok=%v", u, err, ok) | |
| 128 | + | t.Errorf("%q: %v, want ok=%v", cut(u), err, ok) | |
| 111 | 129 | } | |
| 112 | 130 | } | |
| 113 | 131 | } | |
| @@ -123,10 +141,14 @@ func TestResolveActionsRejectsAndBudgets(t *testing.T) { | |||
| 123 | 141 | t.Errorf("%q: error %q, called=%v", uses, r.Error, c.got != nil) | |
| 124 | 142 | } | |
| 125 | 143 | } | |
| 144 | + | // Distinct references, one per request: the rounds are the limit. | |
| 145 | + | c.resp = nil | |
| 146 | + | j = resolveJob(c) | |
| 147 | + | c.fn = func(r *ermite.ResolveActionsRequest) *ermite.ResolveActionsResponse { return echo(r) } | |
| 126 | 148 | var calls int | |
| 127 | 149 | for i := 0; i < maxResolveRounds+20; i++ { | |
| 128 | 150 | c.got = nil | |
| 129 | - | r := j.resolveActions(context.Background(), &execpb.ResolveRequest{Uses: []string{"a/b@v1"}}) | |
| 151 | + | r := j.resolveActions(context.Background(), &execpb.ResolveRequest{Uses: []string{fmt.Sprintf("a/b%d@v1", i)}}) | |
| 130 | 152 | if c.got != nil { | |
| 131 | 153 | calls++ | |
| 132 | 154 | } | |
| @@ -137,18 +159,14 @@ func TestResolveActionsRejectsAndBudgets(t *testing.T) { | |||
| 137 | 159 | if calls != maxResolveRounds { | |
| 138 | 160 | t.Errorf("%d calls, the budget is %d", calls, maxResolveRounds) | |
| 139 | 161 | } | |
| 140 | - | // The references count too. | |
| 162 | + | // Distinct references in big requests: the references are the limit. | |
| 141 | 163 | j = resolveJob(c) | |
| 142 | - | c.resp = &ermite.ResolveActionsResponse{} | |
| 143 | - | for i := 0; i < maxResolveRefs; i++ { | |
| 144 | - | c.resp.Actions = append(c.resp.Actions, &ermite.ResolvedAction{Ref: "a/b@v1"}) | |
| 145 | - | } | |
| 146 | - | big := make([]string, maxResolveRefs) | |
| 147 | - | for i := range big { | |
| 148 | - | big[i] = "a/b@v1" | |
| 149 | - | } | |
| 150 | 164 | served := 0 | |
| 151 | 165 | for i := 0; i < 40; i++ { | |
| 166 | + | big := make([]string, maxResolveRefs) | |
| 167 | + | for k := range big { | |
| 168 | + | big[k] = fmt.Sprintf("a/b%d_%d@v1", i, k) | |
| 169 | + | } | |
| 152 | 170 | if r := j.resolveActions(context.Background(), &execpb.ResolveRequest{Uses: big}); r.Error == "" { | |
| 153 | 171 | served++ | |
| 154 | 172 | } | |
| @@ -158,6 +176,71 @@ func TestResolveActionsRejectsAndBudgets(t *testing.T) { | |||
| 158 | 176 | } | |
| 159 | 177 | } | |
| 160 | 178 | ||
| 179 | + | func echo(r *ermite.ResolveActionsRequest) *ermite.ResolveActionsResponse { | |
| 180 | + | out := &ermite.ResolveActionsResponse{} | |
| 181 | + | for _, ref := range r.Refs { | |
| 182 | + | out.Actions = append(out.Actions, &ermite.ResolvedAction{Ref: ref.Uses, NameWithOwner: "a/b"}) | |
| 183 | + | } | |
| 184 | + | return out | |
| 185 | + | } | |
| 186 | + | ||
| 187 | + | // GO-010: a resolution is cached for the job, so repeated invocations of a local | |
| 188 | + | // composite action cost nothing, and a big request is split into calls. | |
| 189 | + | func TestResolveActionsCacheAndBatches(t *testing.T) { | |
| 190 | + | c := &resolveStub{} | |
| 191 | + | c.fn = echo | |
| 192 | + | j := resolveJob(c) | |
| 193 | + | for i := 0; i < 500; i++ { | |
| 194 | + | r := j.resolveActions(context.Background(), &execpb.ResolveRequest{Uses: []string{"a/b@v1", "a/c/sub@v2"}}) | |
| 195 | + | if r.Error != "" || len(r.Actions) != 2 || r.Actions[1].Ref != "a/c/sub@v2" { | |
| 196 | + | t.Fatalf("invocation %d: %+v", i, r) | |
| 197 | + | } | |
| 198 | + | } | |
| 199 | + | if c.calls != 1 { | |
| 200 | + | t.Errorf("%d calls for 500 invocations of the same actions", c.calls) | |
| 201 | + | } | |
| 202 | + | // A new action goes through; only it is asked. | |
| 203 | + | c.got = nil | |
| 204 | + | if r := j.resolveActions(context.Background(), &execpb.ResolveRequest{Uses: []string{"a/b@v1", "a/new@v1"}}); r.Error != "" || len(c.got.Refs) != 1 || c.got.Refs[0].Uses != "a/new@v1" { | |
| 205 | + | t.Errorf("%+v %v", r, c.got) | |
| 206 | + | } | |
| 207 | + | // Duplicates in one request are asked once, answered in order. | |
| 208 | + | j = resolveJob(c) | |
| 209 | + | c.calls = 0 | |
| 210 | + | r := j.resolveActions(context.Background(), &execpb.ResolveRequest{Uses: []string{"a/x@v1", "a/y@v1", "a/x@v1"}}) | |
| 211 | + | if r.Error != "" || len(r.Actions) != 3 || r.Actions[2].Ref != "a/x@v1" || r.Actions[1].Ref != "a/y@v1" || c.calls != 1 || len(c.got.Refs) != 2 { | |
| 212 | + | t.Errorf("%+v calls %d", r, c.calls) | |
| 213 | + | } | |
| 214 | + | // 250 distinct references: three calls of at most 100, in order. | |
| 215 | + | j = resolveJob(c) | |
| 216 | + | c.calls = 0 | |
| 217 | + | var sizes []int | |
| 218 | + | c.onCall = func(r *ermite.ResolveActionsRequest) { sizes = append(sizes, len(r.Refs)) } | |
| 219 | + | many := make([]string, 250) | |
| 220 | + | for i := range many { | |
| 221 | + | many[i] = fmt.Sprintf("a/b%d@v1", i) | |
| 222 | + | } | |
| 223 | + | r = j.resolveActions(context.Background(), &execpb.ResolveRequest{Uses: many}) | |
| 224 | + | if r.Error != "" || len(r.Actions) != 250 || r.Actions[249].Ref != many[249] || fmt.Sprint(sizes) != "[100 100 50]" { | |
| 225 | + | t.Errorf("%q calls %v", r.Error, sizes) | |
| 226 | + | } | |
| 227 | + | // An error of one action is not cached. | |
| 228 | + | c.fn = func(r *ermite.ResolveActionsRequest) *ermite.ResolveActionsResponse { | |
| 229 | + | return &ermite.ResolveActionsResponse{Actions: []*ermite.ResolvedAction{{Ref: r.Refs[0].Uses, Error: "not found"}}} | |
| 230 | + | } | |
| 231 | + | j = resolveJob(c) | |
| 232 | + | c.calls = 0 | |
| 233 | + | _ = j.resolveActions(context.Background(), &execpb.ResolveRequest{Uses: []string{"a/gone@v1"}}) | |
| 234 | + | _ = j.resolveActions(context.Background(), &execpb.ResolveRequest{Uses: []string{"a/gone@v1"}}) | |
| 235 | + | if c.calls != 2 { | |
| 236 | + | t.Errorf("an error was cached: %d calls", c.calls) | |
| 237 | + | } | |
| 238 | + | // More than the request limit. | |
| 239 | + | if r := j.resolveActions(context.Background(), &execpb.ResolveRequest{Uses: make([]string, maxRequestRefs+1)}); r.Error == "" { | |
| 240 | + | t.Error("a request over the limit was served") | |
| 241 | + | } | |
| 242 | + | } | |
| 243 | + | ||
| 161 | 244 | // GO-002 on the host side: the token only goes with an ermite archive whose URL | |
| 162 | 245 | // is on an origin of ermite; with guest forge access the URL is rewritten. | |
| 163 | 246 | func TestResolveResponseTokenOrigin(t *testing.T) { | |
| @@ -5,6 +5,7 @@ package host | |||||
| 5 | import ( | 5 | import ( | ||
| 6 | "context" | 6 | "context" | ||
| 7 | "encoding/json" | 7 | "encoding/json" | ||
| 8 | + | "fmt" | |||
| 8 | "strings" | 9 | "strings" | ||
| 9 | "testing" | 10 | "testing" | ||
| 10 | "time" | 11 | "time" | ||
| @@ -19,16 +20,26 @@ import ( | |||||
| 19 | 20 | ||||
| 20 | type resolveStub struct { | 21 | type resolveStub struct { | ||
| 21 | stubClient | 22 | stubClient | ||
| 22 | - | got *ermite.ResolveActionsRequest | 23 | + | got *ermite.ResolveActionsRequest |
| 23 | - | resp *ermite.ResolveActionsResponse | 24 | + | resp *ermite.ResolveActionsResponse |
| 24 | - | err error | 25 | + | err error |
| 26 | + | fn func(*ermite.ResolveActionsRequest) *ermite.ResolveActionsResponse | |||
| 27 | + | onCall func(*ermite.ResolveActionsRequest) | |||
| 28 | + | calls int | |||
| 25 | } | 29 | } | ||
| 26 | 30 | ||||
| 27 | func (c *resolveStub) ResolveActions(_ context.Context, r *connect.Request[ermite.ResolveActionsRequest]) (*connect.Response[ermite.ResolveActionsResponse], error) { | 31 | func (c *resolveStub) ResolveActions(_ context.Context, r *connect.Request[ermite.ResolveActionsRequest]) (*connect.Response[ermite.ResolveActionsResponse], error) { | ||
| 28 | c.got = r.Msg | 32 | c.got = r.Msg | ||
| 33 | + | c.calls++ | |||
| 34 | + | if c.onCall != nil { | |||
| 35 | + | c.onCall(r.Msg) | |||
| 36 | + | } | |||
| 29 | if c.err != nil { | 37 | if c.err != nil { | ||
| 30 | return nil, c.err | 38 | return nil, c.err | ||
| 31 | } | 39 | } | ||
| 40 | + | if c.fn != nil { | |||
| 41 | + | return connect.NewResponse(c.fn(r.Msg)), nil | |||
| 42 | + | } | |||
| 32 | return connect.NewResponse(c.resp), nil | 43 | return connect.NewResponse(c.resp), nil | ||
| 33 | } | 44 | } | ||
| 34 | 45 | ||||
| @@ -99,15 +110,22 @@ func TestSetGitHubToken(t *testing.T) { | |||||
| 99 | } | 110 | } | ||
| 100 | 111 | ||||
| 101 | func TestCheckUses(t *testing.T) { | 112 | func TestCheckUses(t *testing.T) { | ||
| 113 | + | long := func(n int) string { return strings.Repeat("a", n) } | |||
| 102 | for u, ok := range map[string]bool{ | 114 | for u, ok := range map[string]bool{ | ||
| 103 | "actions/checkout@v4": true, "o/r/sub/dir@refs/heads/feat/x": true, "o/r@" + strings.Repeat("a", 40): true, "o/r.go_x-y@v1.2.3": true, | 115 | "actions/checkout@v4": true, "o/r/sub/dir@refs/heads/feat/x": true, "o/r@" + strings.Repeat("a", 40): true, "o/r.go_x-y@v1.2.3": true, | ||
| 116 | + | // GO-011: the path of the action is a path, not a repository name. | |||
| 117 | + | "o/r/action+tools@v1": true, "o/r/déploiement/日本語@v1": true, "o/r/with space/x@v1": true, "o/r/a(b)[c]{d}~e#f%g@v1": true, | |||
| 118 | + | "o/r/" + long(255) + "@v1": true, "o/r/.github/actions/x@v1": true, "o/r/.hidden@v1": true, "o/r/a..b@v1": true, | |||
| 104 | "": false, "o/r": false, "o@v1": false, "o/r@": false, "/r@v1": false, "o//r@v1": false, "o/../r@v1": false, "o/r/..@v1": false, | 119 | "": false, "o/r": false, "o@v1": false, "o/r@": false, "/r@v1": false, "o//r@v1": false, "o/../r@v1": false, "o/r/..@v1": false, | ||
| 120 | + | "o/r/a/../b@v1": false, "o/r/./a@v1": false, "o/r/a//b@v1": false, "o/r/a/@v1": false, | |||
| 121 | + | "o/r/a\\b@v1": false, "o/r/a\x00b@v1": false, "o/r/a\nb@v1": false, "o/r/a\x7fb@v1": false, "o/r/\xff@v1": false, | |||
| 122 | + | "o/r/" + long(256) + "@v1": false, long(101) + "/r@v1": false, "o/" + long(101) + "@v1": false, | |||
| 123 | + | "o/r@" + long(256): false, "o/r/" + strings.Repeat("a/", 600) + "x@v1": false, | |||
| 105 | "o/r@v1 x": false, "o/r@v1\n": false, "o/r@v~1": false, "o/r@v:1": false, "o /r@v1": false, "o/r@v1@v2": false, | 124 | "o/r@v1 x": false, "o/r@v1\n": false, "o/r@v~1": false, "o/r@v:1": false, "o /r@v1": false, "o/r@v1@v2": false, | ||
| 106 | - | "o/r\x00@v1": false, strings.Repeat("a", 101) + "/r@v1": false, "o/r@" + strings.Repeat("a", 256): false, | 125 | + | "o/r\x00@v1": false, "ö/r@v1": false, "o/r+x@v1": false, "o/r:x@v1": false, |
| 107 | - | "o/" + strings.Repeat("a/", 300) + "r@v1": false, "ö/r@v1": false, | |||
| 108 | } { | 126 | } { | ||
| 109 | if err := checkUses(u); (err == nil) != ok { | 127 | if err := checkUses(u); (err == nil) != ok { | ||
| 110 | - | t.Errorf("%q: %v, want ok=%v", u, err, ok) | 128 | + | t.Errorf("%q: %v, want ok=%v", cut(u), err, ok) |
| 111 | } | 129 | } | ||
| 112 | } | 130 | } | ||
| 113 | } | 131 | } | ||
| @@ -123,10 +141,14 @@ func TestResolveActionsRejectsAndBudgets(t *testing.T) { | |||||
| 123 | t.Errorf("%q: error %q, called=%v", uses, r.Error, c.got != nil) | 141 | t.Errorf("%q: error %q, called=%v", uses, r.Error, c.got != nil) | ||
| 124 | } | 142 | } | ||
| 125 | } | 143 | } | ||
| 144 | + | // Distinct references, one per request: the rounds are the limit. | |||
| 145 | + | c.resp = nil | |||
| 146 | + | j = resolveJob(c) | |||
| 147 | + | c.fn = func(r *ermite.ResolveActionsRequest) *ermite.ResolveActionsResponse { return echo(r) } | |||
| 126 | var calls int | 148 | var calls int | ||
| 127 | for i := 0; i < maxResolveRounds+20; i++ { | 149 | for i := 0; i < maxResolveRounds+20; i++ { | ||
| 128 | c.got = nil | 150 | c.got = nil | ||
| 129 | - | r := j.resolveActions(context.Background(), &execpb.ResolveRequest{Uses: []string{"a/b@v1"}}) | 151 | + | r := j.resolveActions(context.Background(), &execpb.ResolveRequest{Uses: []string{fmt.Sprintf("a/b%d@v1", i)}}) |
| 130 | if c.got != nil { | 152 | if c.got != nil { | ||
| 131 | calls++ | 153 | calls++ | ||
| 132 | } | 154 | } | ||
| @@ -137,18 +159,14 @@ func TestResolveActionsRejectsAndBudgets(t *testing.T) { | |||||
| 137 | if calls != maxResolveRounds { | 159 | if calls != maxResolveRounds { | ||
| 138 | t.Errorf("%d calls, the budget is %d", calls, maxResolveRounds) | 160 | t.Errorf("%d calls, the budget is %d", calls, maxResolveRounds) | ||
| 139 | } | 161 | } | ||
| 140 | - | // The references count too. | 162 | + | // Distinct references in big requests: the references are the limit. |
| 141 | j = resolveJob(c) | 163 | j = resolveJob(c) | ||
| 142 | - | c.resp = &ermite.ResolveActionsResponse{} | |||
| 143 | - | for i := 0; i < maxResolveRefs; i++ { | |||
| 144 | - | c.resp.Actions = append(c.resp.Actions, &ermite.ResolvedAction{Ref: "a/b@v1"}) | |||
| 145 | - | } | |||
| 146 | - | big := make([]string, maxResolveRefs) | |||
| 147 | - | for i := range big { | |||
| 148 | - | big[i] = "a/b@v1" | |||
| 149 | - | } | |||
| 150 | served := 0 | 164 | served := 0 | ||
| 151 | for i := 0; i < 40; i++ { | 165 | for i := 0; i < 40; i++ { | ||
| 166 | + | big := make([]string, maxResolveRefs) | |||
| 167 | + | for k := range big { | |||
| 168 | + | big[k] = fmt.Sprintf("a/b%d_%d@v1", i, k) | |||
| 169 | + | } | |||
| 152 | if r := j.resolveActions(context.Background(), &execpb.ResolveRequest{Uses: big}); r.Error == "" { | 170 | if r := j.resolveActions(context.Background(), &execpb.ResolveRequest{Uses: big}); r.Error == "" { | ||
| 153 | served++ | 171 | served++ | ||
| 154 | } | 172 | } | ||
| @@ -158,6 +176,71 @@ func TestResolveActionsRejectsAndBudgets(t *testing.T) { | |||||
| 158 | } | 176 | } | ||
| 159 | } | 177 | } | ||
| 160 | 178 | ||||
| 179 | + | func echo(r *ermite.ResolveActionsRequest) *ermite.ResolveActionsResponse { | |||
| 180 | + | out := &ermite.ResolveActionsResponse{} | |||
| 181 | + | for _, ref := range r.Refs { | |||
| 182 | + | out.Actions = append(out.Actions, &ermite.ResolvedAction{Ref: ref.Uses, NameWithOwner: "a/b"}) | |||
| 183 | + | } | |||
| 184 | + | return out | |||
| 185 | + | } | |||
| 186 | + | ||||
| 187 | + | // GO-010: a resolution is cached for the job, so repeated invocations of a local | |||
| 188 | + | // composite action cost nothing, and a big request is split into calls. | |||
| 189 | + | func TestResolveActionsCacheAndBatches(t *testing.T) { | |||
| 190 | + | c := &resolveStub{} | |||
| 191 | + | c.fn = echo | |||
| 192 | + | j := resolveJob(c) | |||
| 193 | + | for i := 0; i < 500; i++ { | |||
| 194 | + | r := j.resolveActions(context.Background(), &execpb.ResolveRequest{Uses: []string{"a/b@v1", "a/c/sub@v2"}}) | |||
| 195 | + | if r.Error != "" || len(r.Actions) != 2 || r.Actions[1].Ref != "a/c/sub@v2" { | |||
| 196 | + | t.Fatalf("invocation %d: %+v", i, r) | |||
| 197 | + | } | |||
| 198 | + | } | |||
| 199 | + | if c.calls != 1 { | |||
| 200 | + | t.Errorf("%d calls for 500 invocations of the same actions", c.calls) | |||
| 201 | + | } | |||
| 202 | + | // A new action goes through; only it is asked. | |||
| 203 | + | c.got = nil | |||
| 204 | + | if r := j.resolveActions(context.Background(), &execpb.ResolveRequest{Uses: []string{"a/b@v1", "a/new@v1"}}); r.Error != "" || len(c.got.Refs) != 1 || c.got.Refs[0].Uses != "a/new@v1" { | |||
| 205 | + | t.Errorf("%+v %v", r, c.got) | |||
| 206 | + | } | |||
| 207 | + | // Duplicates in one request are asked once, answered in order. | |||
| 208 | + | j = resolveJob(c) | |||
| 209 | + | c.calls = 0 | |||
| 210 | + | r := j.resolveActions(context.Background(), &execpb.ResolveRequest{Uses: []string{"a/x@v1", "a/y@v1", "a/x@v1"}}) | |||
| 211 | + | if r.Error != "" || len(r.Actions) != 3 || r.Actions[2].Ref != "a/x@v1" || r.Actions[1].Ref != "a/y@v1" || c.calls != 1 || len(c.got.Refs) != 2 { | |||
| 212 | + | t.Errorf("%+v calls %d", r, c.calls) | |||
| 213 | + | } | |||
| 214 | + | // 250 distinct references: three calls of at most 100, in order. | |||
| 215 | + | j = resolveJob(c) | |||
| 216 | + | c.calls = 0 | |||
| 217 | + | var sizes []int | |||
| 218 | + | c.onCall = func(r *ermite.ResolveActionsRequest) { sizes = append(sizes, len(r.Refs)) } | |||
| 219 | + | many := make([]string, 250) | |||
| 220 | + | for i := range many { | |||
| 221 | + | many[i] = fmt.Sprintf("a/b%d@v1", i) | |||
| 222 | + | } | |||
| 223 | + | r = j.resolveActions(context.Background(), &execpb.ResolveRequest{Uses: many}) | |||
| 224 | + | if r.Error != "" || len(r.Actions) != 250 || r.Actions[249].Ref != many[249] || fmt.Sprint(sizes) != "[100 100 50]" { | |||
| 225 | + | t.Errorf("%q calls %v", r.Error, sizes) | |||
| 226 | + | } | |||
| 227 | + | // An error of one action is not cached. | |||
| 228 | + | c.fn = func(r *ermite.ResolveActionsRequest) *ermite.ResolveActionsResponse { | |||
| 229 | + | return &ermite.ResolveActionsResponse{Actions: []*ermite.ResolvedAction{{Ref: r.Refs[0].Uses, Error: "not found"}}} | |||
| 230 | + | } | |||
| 231 | + | j = resolveJob(c) | |||
| 232 | + | c.calls = 0 | |||
| 233 | + | _ = j.resolveActions(context.Background(), &execpb.ResolveRequest{Uses: []string{"a/gone@v1"}}) | |||
| 234 | + | _ = j.resolveActions(context.Background(), &execpb.ResolveRequest{Uses: []string{"a/gone@v1"}}) | |||
| 235 | + | if c.calls != 2 { | |||
| 236 | + | t.Errorf("an error was cached: %d calls", c.calls) | |||
| 237 | + | } | |||
| 238 | + | // More than the request limit. | |||
| 239 | + | if r := j.resolveActions(context.Background(), &execpb.ResolveRequest{Uses: make([]string, maxRequestRefs+1)}); r.Error == "" { | |||
| 240 | + | t.Error("a request over the limit was served") | |||
| 241 | + | } | |||
| 242 | + | } | |||
| 243 | + | ||||
| 161 | // GO-002 on the host side: the token only goes with an ermite archive whose URL | 244 | // GO-002 on the host side: the token only goes with an ermite archive whose URL | ||
| 162 | // is on an origin of ermite; with guest forge access the URL is rewritten. | 245 | // is on an origin of ermite; with guest forge access the URL is rewritten. | ||
| 163 | func TestResolveResponseTokenOrigin(t *testing.T) { | 246 | func TestResolveResponseTokenOrigin(t *testing.T) { | ||