fix(executor): evaluate the job name with the server's contexts and check the matrix before trusting job-index
- commit
- e0a17ad17d63b3e3feacd8551ab858155c79dad4
- author
- brice · · 2026-10-12 23:13 UTC
- parent
- 9845c39
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
4 files · +169 -42
M internal/executor/contexts.go +24 -2
| @@ -197,8 +197,13 @@ type stepCtx struct { | |||
| 197 | 197 | type State struct { | |
| 198 | 198 | layout Layout | |
| 199 | 199 | github *expr.Dictionary | |
| 200 | - | runner *expr.Dictionary | |
| 201 | - | job *expr.Dictionary | |
| 200 | + | // serverGitHub is the github context as the server sent it, before the | |
| 201 | + | // runner added its keys (token, workspace, the file commands, secret_source, | |
| 202 | + | // action...): the contexts that the server itself evaluates the job name | |
| 203 | + | // with (see ServerContexts). | |
| 204 | + | serverGitHub *expr.Dictionary | |
| 205 | + | runner *expr.Dictionary | |
| 206 | + | job *expr.Dictionary | |
| 202 | 207 | // other holds needs, strategy, matrix, inputs, vars: copied from the server. | |
| 203 | 208 | other map[string]expr.Value | |
| 204 | 209 | // env is Global.EnvironmentVariables: the job-level environment and what | |
| @@ -250,6 +255,10 @@ func NewState(contextsJSON, matrixJSON, jobKey string, r RunnerInfo, repoFallbac | |||
| 250 | 255 | gh.Add(p.Key, p.Value) | |
| 251 | 256 | } | |
| 252 | 257 | } | |
| 258 | + | s.serverGitHub = expr.NewDictionary() | |
| 259 | + | if v, ok := root.Get("github"); ok { | |
| 260 | + | s.serverGitHub = v.(*expr.Dictionary) | |
| 261 | + | } | |
| 253 | 262 | if str(gh, "job") == "" && jobKey != "" { | |
| 254 | 263 | gh = setKeySorted(gh, "job", expr.String(jobKey)) | |
| 255 | 264 | } | |
| @@ -456,6 +465,19 @@ func (s *State) Values(env *ordered) map[string]expr.Value { | |||
| 456 | 465 | return m | |
| 457 | 466 | } | |
| 458 | 467 | ||
| 468 | + | // ServerContexts returns the contexts that the server has when it evaluates | |
| 469 | + | // the name of a job: github as received (without the keys the runner adds), | |
| 470 | + | // needs, strategy, matrix, vars and inputs. | |
| 471 | + | func (s *State) ServerContexts() map[string]expr.Value { | |
| 472 | + | m := map[string]expr.Value{"github": s.serverGitHub} | |
| 473 | + | for _, k := range []string{"needs", "strategy", "matrix", "vars", "inputs"} { | |
| 474 | + | if v, ok := s.other[k]; ok { | |
| 475 | + | m[k] = v | |
| 476 | + | } | |
| 477 | + | } | |
| 478 | + | return m | |
| 479 | + | } | |
| 480 | + | ||
| 459 | 481 | // githubEnvAllowlist is GitHubContext._contextEnvAllowlist (GitHubContext.cs:9). | |
| 460 | 482 | var githubEnvAllowlist = map[string]bool{ | |
| 461 | 483 | "action_path": true, "action_ref": true, "action_repository": true, "action": true, "actor": true, | |
| @@ -197,8 +197,13 @@ type stepCtx struct { | |||||
| 197 | type State struct { | 197 | type State struct { | ||
| 198 | layout Layout | 198 | layout Layout | ||
| 199 | github *expr.Dictionary | 199 | github *expr.Dictionary | ||
| 200 | - | runner *expr.Dictionary | 200 | + | // serverGitHub is the github context as the server sent it, before the |
| 201 | - | job *expr.Dictionary | 201 | + | // runner added its keys (token, workspace, the file commands, secret_source, |
| 202 | + | // action...): the contexts that the server itself evaluates the job name | |||
| 203 | + | // with (see ServerContexts). | |||
| 204 | + | serverGitHub *expr.Dictionary | |||
| 205 | + | runner *expr.Dictionary | |||
| 206 | + | job *expr.Dictionary | |||
| 202 | // other holds needs, strategy, matrix, inputs, vars: copied from the server. | 207 | // other holds needs, strategy, matrix, inputs, vars: copied from the server. | ||
| 203 | other map[string]expr.Value | 208 | other map[string]expr.Value | ||
| 204 | // env is Global.EnvironmentVariables: the job-level environment and what | 209 | // env is Global.EnvironmentVariables: the job-level environment and what | ||
| @@ -250,6 +255,10 @@ func NewState(contextsJSON, matrixJSON, jobKey string, r RunnerInfo, repoFallbac | |||||
| 250 | gh.Add(p.Key, p.Value) | 255 | gh.Add(p.Key, p.Value) | ||
| 251 | } | 256 | } | ||
| 252 | } | 257 | } | ||
| 258 | + | s.serverGitHub = expr.NewDictionary() | |||
| 259 | + | if v, ok := root.Get("github"); ok { | |||
| 260 | + | s.serverGitHub = v.(*expr.Dictionary) | |||
| 261 | + | } | |||
| 253 | if str(gh, "job") == "" && jobKey != "" { | 262 | if str(gh, "job") == "" && jobKey != "" { | ||
| 254 | gh = setKeySorted(gh, "job", expr.String(jobKey)) | 263 | gh = setKeySorted(gh, "job", expr.String(jobKey)) | ||
| 255 | } | 264 | } | ||
| @@ -456,6 +465,19 @@ func (s *State) Values(env *ordered) map[string]expr.Value { | |||||
| 456 | return m | 465 | return m | ||
| 457 | } | 466 | } | ||
| 458 | 467 | ||||
| 468 | + | // ServerContexts returns the contexts that the server has when it evaluates | |||
| 469 | + | // the name of a job: github as received (without the keys the runner adds), | |||
| 470 | + | // needs, strategy, matrix, vars and inputs. | |||
| 471 | + | func (s *State) ServerContexts() map[string]expr.Value { | |||
| 472 | + | m := map[string]expr.Value{"github": s.serverGitHub} | |||
| 473 | + | for _, k := range []string{"needs", "strategy", "matrix", "vars", "inputs"} { | |||
| 474 | + | if v, ok := s.other[k]; ok { | |||
| 475 | + | m[k] = v | |||
| 476 | + | } | |||
| 477 | + | } | |||
| 478 | + | return m | |||
| 479 | + | } | |||
| 480 | + | ||||
| 459 | // githubEnvAllowlist is GitHubContext._contextEnvAllowlist (GitHubContext.cs:9). | 481 | // githubEnvAllowlist is GitHubContext._contextEnvAllowlist (GitHubContext.cs:9). | ||
| 460 | var githubEnvAllowlist = map[string]bool{ | 482 | var githubEnvAllowlist = map[string]bool{ | ||
| 461 | "action_path": true, "action_ref": true, "action_repository": true, "action": true, "actor": true, | 483 | "action_path": true, "action_ref": true, "action_repository": true, "action": true, "actor": true, | ||
M internal/executor/job.go +65 -39
| @@ -16,6 +16,7 @@ import ( | |||
| 16 | 16 | "bnema.dev/ermite-actions/expr" | |
| 17 | 17 | "bnema.dev/ermite-actions/template" | |
| 18 | 18 | "bnema.dev/ermite-actions/workflow" | |
| 19 | + | "github.com/bnema/zerowrap" | |
| 19 | 20 | "github.com/google/uuid" | |
| 20 | 21 | ) | |
| 21 | 22 | ||
| @@ -75,6 +76,9 @@ type Config struct { | |||
| 75 | 76 | Debug bool | |
| 76 | 77 | ||
| 77 | 78 | Reporter Reporter | |
| 79 | + | // Log receives the runner's own diagnostics (not the job log); the zero value | |
| 80 | + | // discards them. | |
| 81 | + | Log zerowrap.Logger | |
| 78 | 82 | ||
| 79 | 83 | // Credential, when set, is the user the steps run as. | |
| 80 | 84 | Credential *syscall.Credential | |
| @@ -610,59 +614,81 @@ func (j *job) printTokenInfo() { | |||
| 610 | 614 | // jobDisplayName is the name of the job as GitHub shows it and the runner | |
| 611 | 615 | // prints it after "Complete job name:": the "name" of the job (the job key when | |
| 612 | 616 | // it has none) evaluated with github, needs, strategy, matrix, vars and inputs | |
| 613 | - | // (the contexts the schema allows there), followed by the matrix values in | |
| 614 | - | // parentheses unless the name reads the matrix (recorded: matrix-job-name, see | |
| 615 | - | // workflow.Job.DisplayName). The combination of this job is the one at | |
| 616 | - | // strategy.job-index, else the one whose values equal the matrix. On any | |
| 617 | - | // failure the job key is used: the name is not worth failing the job for. | |
| 617 | + | // as the server has them (State.ServerContexts: github without the keys the | |
| 618 | + | // runner adds), followed by the matrix values in parentheses unless the name | |
| 619 | + | // reads the matrix (recorded: matrix-job-name, see workflow.Job.DisplayName). | |
| 620 | + | // When the name cannot be worked out the job key is used and a warning is | |
| 621 | + | // logged: the name is not worth failing the job for. | |
| 618 | 622 | func (j *job) jobDisplayName() string { | |
| 619 | 623 | ctx := context.Background() | |
| 620 | - | all := j.evalEnv(newOrdered()) | |
| 621 | - | env := all | |
| 622 | - | env.Contexts = map[string]expr.Value{} | |
| 623 | - | for _, k := range []string{"github", "needs", "strategy", "matrix", "vars", "inputs"} { | |
| 624 | - | if v, ok := all.Contexts[k]; ok { | |
| 625 | - | env.Contexts[k] = v | |
| 626 | - | } | |
| 624 | + | fallback := func(why string, err error) string { | |
| 625 | + | j.cfg.Log.Warn().Err(err).Str("job_key", j.cfg.JobKey).Msg("job display name: " + why + ", using the job key") | |
| 626 | + | return j.cfg.JobKey | |
| 627 | 627 | } | |
| 628 | + | j.mu.Lock() | |
| 629 | + | all := j.state.ServerContexts() | |
| 630 | + | j.mu.Unlock() | |
| 631 | + | env := j.evalEnv(newOrdered()) | |
| 632 | + | env.Contexts = all | |
| 628 | 633 | // The strategy itself sees neither the strategy nor the matrix context. | |
| 629 | 634 | senv := env | |
| 630 | 635 | senv.Contexts = map[string]expr.Value{} | |
| 631 | 636 | for _, k := range []string{"github", "needs", "vars", "inputs"} { | |
| 632 | - | if v, ok := env.Contexts[k]; ok { | |
| 633 | - | senv.Contexts[k] = v | |
| 634 | - | } | |
| 637 | + | senv.Contexts[k] = all[k] | |
| 635 | 638 | } | |
| 636 | 639 | st, err := j.jb.EvaluateStrategy(ctx, senv) | |
| 637 | 640 | if err != nil { | |
| 638 | - | return j.cfg.JobKey | |
| 641 | + | return fallback("the strategy does not evaluate", err) | |
| 639 | 642 | } | |
| 640 | - | idx := 0 | |
| 641 | - | if st.HasMatrix { | |
| 642 | - | idx = -1 | |
| 643 | - | if d, ok := env.Contexts["strategy"].(*expr.Dictionary); ok { | |
| 644 | - | if n, ok := d.Get("job-index"); ok { | |
| 645 | - | if f, ok := n.(expr.Number); ok && int(f) >= 0 && int(f) < len(st.Combos) { | |
| 646 | - | idx = int(f) | |
| 647 | - | } | |
| 648 | - | } | |
| 649 | - | } | |
| 650 | - | if idx < 0 { | |
| 651 | - | want := expr.ToJSON(env.Contexts["matrix"]) | |
| 652 | - | for i, c := range st.Combos { | |
| 653 | - | if expr.ToJSON(c.Values) == want { | |
| 654 | - | idx = i | |
| 655 | - | break | |
| 656 | - | } | |
| 657 | - | } | |
| 658 | - | } | |
| 659 | - | if idx < 0 { | |
| 660 | - | return j.cfg.JobKey | |
| 661 | - | } | |
| 643 | + | idx, ok := comboIndex(st, all["strategy"], all["matrix"]) | |
| 644 | + | if !ok { | |
| 645 | + | return fallback("the matrix of the job is not one of the combinations of its strategy", nil) | |
| 662 | 646 | } | |
| 663 | 647 | name, err := j.jb.DisplayName(ctx, env, st, idx) | |
| 664 | 648 | if err != nil { | |
| 665 | - | return j.cfg.JobKey | |
| 649 | + | return fallback("the name does not evaluate", err) | |
| 666 | 650 | } | |
| 667 | 651 | return name | |
| 668 | 652 | } | |
| 653 | + | ||
| 654 | + | // comboIndex finds the combination of the strategy that is this job. Without a | |
| 655 | + | // matrix it is the only one. The index of the strategy context is trusted only | |
| 656 | + | // when that combination has the matrix of the job; otherwise the first | |
| 657 | + | // combination with equal values is taken. ok is false when there is none. | |
| 658 | + | func comboIndex(st *workflow.Strategy, strategyCtx, matrix expr.Value) (idx int, ok bool) { | |
| 659 | + | if !st.HasMatrix { | |
| 660 | + | return 0, true | |
| 661 | + | } | |
| 662 | + | m, isDict := matrix.(*expr.Dictionary) | |
| 663 | + | if !isDict { | |
| 664 | + | return 0, false | |
| 665 | + | } | |
| 666 | + | if d, isDict := strategyCtx.(*expr.Dictionary); isDict { | |
| 667 | + | if n, found := d.Get("job-index"); found { | |
| 668 | + | if f, isNum := n.(expr.Number); isNum && f >= 0 && int(f) < len(st.Combos) && sameMatrix(st.Combos[int(f)].Values, m) { | |
| 669 | + | return int(f), true | |
| 670 | + | } | |
| 671 | + | } | |
| 672 | + | } | |
| 673 | + | for i, c := range st.Combos { | |
| 674 | + | if sameMatrix(c.Values, m) { | |
| 675 | + | return i, true | |
| 676 | + | } | |
| 677 | + | } | |
| 678 | + | return 0, false | |
| 679 | + | } | |
| 680 | + | ||
| 681 | + | // sameMatrix compares two matrix values: the same keys (ignoring case and | |
| 682 | + | // order) with equal values. | |
| 683 | + | func sameMatrix(a, b *expr.Dictionary) bool { | |
| 684 | + | if a.Len() != b.Len() { | |
| 685 | + | return false | |
| 686 | + | } | |
| 687 | + | for _, p := range a.Pairs() { | |
| 688 | + | v, found := b.Get(p.Key) | |
| 689 | + | if !found || expr.ToJSON(v) != expr.ToJSON(p.Value) { | |
| 690 | + | return false | |
| 691 | + | } | |
| 692 | + | } | |
| 693 | + | return true | |
| 694 | + | } | |
| @@ -16,6 +16,7 @@ import ( | |||||
| 16 | "bnema.dev/ermite-actions/expr" | 16 | "bnema.dev/ermite-actions/expr" | ||
| 17 | "bnema.dev/ermite-actions/template" | 17 | "bnema.dev/ermite-actions/template" | ||
| 18 | "bnema.dev/ermite-actions/workflow" | 18 | "bnema.dev/ermite-actions/workflow" | ||
| 19 | + | "github.com/bnema/zerowrap" | |||
| 19 | "github.com/google/uuid" | 20 | "github.com/google/uuid" | ||
| 20 | ) | 21 | ) | ||
| 21 | 22 | ||||
| @@ -75,6 +76,9 @@ type Config struct { | |||||
| 75 | Debug bool | 76 | Debug bool | ||
| 76 | 77 | ||||
| 77 | Reporter Reporter | 78 | Reporter Reporter | ||
| 79 | + | // Log receives the runner's own diagnostics (not the job log); the zero value | |||
| 80 | + | // discards them. | |||
| 81 | + | Log zerowrap.Logger | |||
| 78 | 82 | ||||
| 79 | // Credential, when set, is the user the steps run as. | 83 | // Credential, when set, is the user the steps run as. | ||
| 80 | Credential *syscall.Credential | 84 | Credential *syscall.Credential | ||
| @@ -610,59 +614,81 @@ func (j *job) printTokenInfo() { | |||||
| 610 | // jobDisplayName is the name of the job as GitHub shows it and the runner | 614 | // jobDisplayName is the name of the job as GitHub shows it and the runner | ||
| 611 | // prints it after "Complete job name:": the "name" of the job (the job key when | 615 | // prints it after "Complete job name:": the "name" of the job (the job key when | ||
| 612 | // it has none) evaluated with github, needs, strategy, matrix, vars and inputs | 616 | // it has none) evaluated with github, needs, strategy, matrix, vars and inputs | ||
| 613 | - | // (the contexts the schema allows there), followed by the matrix values in | 617 | + | // as the server has them (State.ServerContexts: github without the keys the |
| 614 | - | // parentheses unless the name reads the matrix (recorded: matrix-job-name, see | 618 | + | // runner adds), followed by the matrix values in parentheses unless the name |
| 615 | - | // workflow.Job.DisplayName). The combination of this job is the one at | 619 | + | // reads the matrix (recorded: matrix-job-name, see workflow.Job.DisplayName). |
| 616 | - | // strategy.job-index, else the one whose values equal the matrix. On any | 620 | + | // When the name cannot be worked out the job key is used and a warning is |
| 617 | - | // failure the job key is used: the name is not worth failing the job for. | 621 | + | // logged: the name is not worth failing the job for. |
| 618 | func (j *job) jobDisplayName() string { | 622 | func (j *job) jobDisplayName() string { | ||
| 619 | ctx := context.Background() | 623 | ctx := context.Background() | ||
| 620 | - | all := j.evalEnv(newOrdered()) | 624 | + | fallback := func(why string, err error) string { |
| 621 | - | env := all | 625 | + | j.cfg.Log.Warn().Err(err).Str("job_key", j.cfg.JobKey).Msg("job display name: " + why + ", using the job key") |
| 622 | - | env.Contexts = map[string]expr.Value{} | 626 | + | return j.cfg.JobKey |
| 623 | - | for _, k := range []string{"github", "needs", "strategy", "matrix", "vars", "inputs"} { | |||
| 624 | - | if v, ok := all.Contexts[k]; ok { | |||
| 625 | - | env.Contexts[k] = v | |||
| 626 | - | } | |||
| 627 | } | 627 | } | ||
| 628 | + | j.mu.Lock() | |||
| 629 | + | all := j.state.ServerContexts() | |||
| 630 | + | j.mu.Unlock() | |||
| 631 | + | env := j.evalEnv(newOrdered()) | |||
| 632 | + | env.Contexts = all | |||
| 628 | // The strategy itself sees neither the strategy nor the matrix context. | 633 | // The strategy itself sees neither the strategy nor the matrix context. | ||
| 629 | senv := env | 634 | senv := env | ||
| 630 | senv.Contexts = map[string]expr.Value{} | 635 | senv.Contexts = map[string]expr.Value{} | ||
| 631 | for _, k := range []string{"github", "needs", "vars", "inputs"} { | 636 | for _, k := range []string{"github", "needs", "vars", "inputs"} { | ||
| 632 | - | if v, ok := env.Contexts[k]; ok { | 637 | + | senv.Contexts[k] = all[k] |
| 633 | - | senv.Contexts[k] = v | |||
| 634 | - | } | |||
| 635 | } | 638 | } | ||
| 636 | st, err := j.jb.EvaluateStrategy(ctx, senv) | 639 | st, err := j.jb.EvaluateStrategy(ctx, senv) | ||
| 637 | if err != nil { | 640 | if err != nil { | ||
| 638 | - | return j.cfg.JobKey | 641 | + | return fallback("the strategy does not evaluate", err) |
| 639 | } | 642 | } | ||
| 640 | - | idx := 0 | 643 | + | idx, ok := comboIndex(st, all["strategy"], all["matrix"]) |
| 641 | - | if st.HasMatrix { | 644 | + | if !ok { |
| 642 | - | idx = -1 | 645 | + | return fallback("the matrix of the job is not one of the combinations of its strategy", nil) |
| 643 | - | if d, ok := env.Contexts["strategy"].(*expr.Dictionary); ok { | |||
| 644 | - | if n, ok := d.Get("job-index"); ok { | |||
| 645 | - | if f, ok := n.(expr.Number); ok && int(f) >= 0 && int(f) < len(st.Combos) { | |||
| 646 | - | idx = int(f) | |||
| 647 | - | } | |||
| 648 | - | } | |||
| 649 | - | } | |||
| 650 | - | if idx < 0 { | |||
| 651 | - | want := expr.ToJSON(env.Contexts["matrix"]) | |||
| 652 | - | for i, c := range st.Combos { | |||
| 653 | - | if expr.ToJSON(c.Values) == want { | |||
| 654 | - | idx = i | |||
| 655 | - | break | |||
| 656 | - | } | |||
| 657 | - | } | |||
| 658 | - | } | |||
| 659 | - | if idx < 0 { | |||
| 660 | - | return j.cfg.JobKey | |||
| 661 | - | } | |||
| 662 | } | 646 | } | ||
| 663 | name, err := j.jb.DisplayName(ctx, env, st, idx) | 647 | name, err := j.jb.DisplayName(ctx, env, st, idx) | ||
| 664 | if err != nil { | 648 | if err != nil { | ||
| 665 | - | return j.cfg.JobKey | 649 | + | return fallback("the name does not evaluate", err) |
| 666 | } | 650 | } | ||
| 667 | return name | 651 | return name | ||
| 668 | } | 652 | } | ||
| 653 | + | ||||
| 654 | + | // comboIndex finds the combination of the strategy that is this job. Without a | |||
| 655 | + | // matrix it is the only one. The index of the strategy context is trusted only | |||
| 656 | + | // when that combination has the matrix of the job; otherwise the first | |||
| 657 | + | // combination with equal values is taken. ok is false when there is none. | |||
| 658 | + | func comboIndex(st *workflow.Strategy, strategyCtx, matrix expr.Value) (idx int, ok bool) { | |||
| 659 | + | if !st.HasMatrix { | |||
| 660 | + | return 0, true | |||
| 661 | + | } | |||
| 662 | + | m, isDict := matrix.(*expr.Dictionary) | |||
| 663 | + | if !isDict { | |||
| 664 | + | return 0, false | |||
| 665 | + | } | |||
| 666 | + | if d, isDict := strategyCtx.(*expr.Dictionary); isDict { | |||
| 667 | + | if n, found := d.Get("job-index"); found { | |||
| 668 | + | if f, isNum := n.(expr.Number); isNum && f >= 0 && int(f) < len(st.Combos) && sameMatrix(st.Combos[int(f)].Values, m) { | |||
| 669 | + | return int(f), true | |||
| 670 | + | } | |||
| 671 | + | } | |||
| 672 | + | } | |||
| 673 | + | for i, c := range st.Combos { | |||
| 674 | + | if sameMatrix(c.Values, m) { | |||
| 675 | + | return i, true | |||
| 676 | + | } | |||
| 677 | + | } | |||
| 678 | + | return 0, false | |||
| 679 | + | } | |||
| 680 | + | ||||
| 681 | + | // sameMatrix compares two matrix values: the same keys (ignoring case and | |||
| 682 | + | // order) with equal values. | |||
| 683 | + | func sameMatrix(a, b *expr.Dictionary) bool { | |||
| 684 | + | if a.Len() != b.Len() { | |||
| 685 | + | return false | |||
| 686 | + | } | |||
| 687 | + | for _, p := range a.Pairs() { | |||
| 688 | + | v, found := b.Get(p.Key) | |||
| 689 | + | if !found || expr.ToJSON(v) != expr.ToJSON(p.Value) { | |||
| 690 | + | return false | |||
| 691 | + | } | |||
| 692 | + | } | |||
| 693 | + | return true | |||
| 694 | + | } | |||
M internal/executor/job_test.go +77 -0
| @@ -3,12 +3,15 @@ | |||
| 3 | 3 | package executor | |
| 4 | 4 | ||
| 5 | 5 | import ( | |
| 6 | + | "bytes" | |
| 6 | 7 | "os" | |
| 7 | 8 | "path/filepath" | |
| 8 | 9 | "strings" | |
| 9 | 10 | "syscall" | |
| 10 | 11 | "testing" | |
| 11 | 12 | "time" | |
| 13 | + | ||
| 14 | + | "github.com/bnema/zerowrap" | |
| 12 | 15 | ) | |
| 13 | 16 | ||
| 14 | 17 | const wfHead = "on: push\njobs:\n j:\n runs-on: x\n" | |
| @@ -752,6 +755,9 @@ jobs: | |||
| 752 | 755 | {"static-name", `{"n":2}`, `{"job-index":1}`, "same name for every leg (2)"}, | |
| 753 | 756 | {"plain", `{}`, ``, "plain push"}, | |
| 754 | 757 | {"static-name", `{"n":9}`, ``, "static-name"}, // not one of the combinations: the job key | |
| 758 | + | // job-index points to another combination: it is not trusted. | |
| 759 | + | {"default-name", `{"os":"a","n":2}`, `{"job-index":0}`, "default-name (a, 2)"}, | |
| 760 | + | {"default-name", `{"os":"b","n":2}`, `{"job-index":7}`, "default-name (b, 2)"}, | |
| 755 | 761 | } { | |
| 756 | 762 | t.Run(tc.want, func(t *testing.T) { | |
| 757 | 763 | got := setupLog(t, yaml, tc.job, func(c *Config) { | |
| @@ -770,3 +776,74 @@ jobs: | |||
| 770 | 776 | }) | |
| 771 | 777 | } | |
| 772 | 778 | } | |
| 779 | + | ||
| 780 | + | func TestJobDisplayNameReadsNeedsVarsAndIgnoresRunnerKeys(t *testing.T) { | |
| 781 | + | needTools(t, "bash") | |
| 782 | + | yaml := `on: push | |
| 783 | + | jobs: | |
| 784 | + | j: | |
| 785 | + | name: "${{ needs.a.outputs.o }}-${{ vars.V }}-${{ github.workspace }}-${{ github.secret_source }}-${{ github.token }}-${{ github.event_name }}" | |
| 786 | + | runs-on: x | |
| 787 | + | steps: | |
| 788 | + | - run: echo hi | |
| 789 | + | ` | |
| 790 | + | got := setupLog(t, yaml, "j", func(c *Config) { | |
| 791 | + | c.ContextsJSON = `{"github":{"repository":"octo/hello","event_name":"push","job":"j"},"needs":{"a":{"outputs":{"o":"out"}}},"vars":{"V":"v1"}}` | |
| 792 | + | }) | |
| 793 | + | // workspace, secret_source and token are added by the runner: empty for the name. | |
| 794 | + | if !strings.HasSuffix(got, "Complete job name: out-v1----push") { | |
| 795 | + | t.Errorf("got:\n%s", got) | |
| 796 | + | } | |
| 797 | + | } | |
| 798 | + | ||
| 799 | + | func TestJobDisplayNameIncludeOnlyMatrix(t *testing.T) { | |
| 800 | + | needTools(t, "bash") | |
| 801 | + | yaml := `on: push | |
| 802 | + | jobs: | |
| 803 | + | j: | |
| 804 | + | runs-on: x | |
| 805 | + | strategy: | |
| 806 | + | matrix: | |
| 807 | + | include: | |
| 808 | + | - os: a | |
| 809 | + | n: 1 | |
| 810 | + | - os: b | |
| 811 | + | n: 2 | |
| 812 | + | steps: | |
| 813 | + | - run: echo hi | |
| 814 | + | ` | |
| 815 | + | for matrix, want := range map[string]string{`{"os":"b","n":2}`: "j (b, 2)", `{"os":"a","n":1}`: "j (a, 1)"} { | |
| 816 | + | got := setupLog(t, yaml, "j", func(c *Config) { | |
| 817 | + | c.MatrixJSON = matrix | |
| 818 | + | c.ContextsJSON = strings.TrimSuffix(testGitHubContext("j"), "}") + `,"matrix":` + matrix + `}` | |
| 819 | + | }) | |
| 820 | + | if !strings.HasSuffix(got, "Complete job name: "+want) { | |
| 821 | + | t.Errorf("%s:\n%s", matrix, got) | |
| 822 | + | } | |
| 823 | + | } | |
| 824 | + | } | |
| 825 | + | ||
| 826 | + | func TestJobDisplayNameFallsBackToTheKeyWithAWarning(t *testing.T) { | |
| 827 | + | needTools(t, "bash") | |
| 828 | + | yaml := `on: push | |
| 829 | + | jobs: | |
| 830 | + | j: | |
| 831 | + | runs-on: x | |
| 832 | + | strategy: | |
| 833 | + | matrix: | |
| 834 | + | n: [1, 2] | |
| 835 | + | steps: | |
| 836 | + | - run: echo hi | |
| 837 | + | ` | |
| 838 | + | var logs bytes.Buffer | |
| 839 | + | got := setupLog(t, yaml, "j", func(c *Config) { | |
| 840 | + | c.MatrixJSON = `{"n":5}` | |
| 841 | + | c.Log = zerowrap.New(zerowrap.Config{Level: "debug", Format: "json", Output: &logs}) | |
| 842 | + | }) | |
| 843 | + | if !strings.HasSuffix(got, "Complete job name: j") { | |
| 844 | + | t.Errorf("got:\n%s", got) | |
| 845 | + | } | |
| 846 | + | if !strings.Contains(logs.String(), `"level":"warn"`) || !strings.Contains(logs.String(), "using the job key") { | |
| 847 | + | t.Errorf("no warning: %q", logs.String()) | |
| 848 | + | } | |
| 849 | + | } | |
| @@ -3,12 +3,15 @@ | |||||
| 3 | package executor | 3 | package executor | ||
| 4 | 4 | ||||
| 5 | import ( | 5 | import ( | ||
| 6 | + | "bytes" | |||
| 6 | "os" | 7 | "os" | ||
| 7 | "path/filepath" | 8 | "path/filepath" | ||
| 8 | "strings" | 9 | "strings" | ||
| 9 | "syscall" | 10 | "syscall" | ||
| 10 | "testing" | 11 | "testing" | ||
| 11 | "time" | 12 | "time" | ||
| 13 | + | ||||
| 14 | + | "github.com/bnema/zerowrap" | |||
| 12 | ) | 15 | ) | ||
| 13 | 16 | ||||
| 14 | const wfHead = "on: push\njobs:\n j:\n runs-on: x\n" | 17 | const wfHead = "on: push\njobs:\n j:\n runs-on: x\n" | ||
| @@ -752,6 +755,9 @@ jobs: | |||||
| 752 | {"static-name", `{"n":2}`, `{"job-index":1}`, "same name for every leg (2)"}, | 755 | {"static-name", `{"n":2}`, `{"job-index":1}`, "same name for every leg (2)"}, | ||
| 753 | {"plain", `{}`, ``, "plain push"}, | 756 | {"plain", `{}`, ``, "plain push"}, | ||
| 754 | {"static-name", `{"n":9}`, ``, "static-name"}, // not one of the combinations: the job key | 757 | {"static-name", `{"n":9}`, ``, "static-name"}, // not one of the combinations: the job key | ||
| 758 | + | // job-index points to another combination: it is not trusted. | |||
| 759 | + | {"default-name", `{"os":"a","n":2}`, `{"job-index":0}`, "default-name (a, 2)"}, | |||
| 760 | + | {"default-name", `{"os":"b","n":2}`, `{"job-index":7}`, "default-name (b, 2)"}, | |||
| 755 | } { | 761 | } { | ||
| 756 | t.Run(tc.want, func(t *testing.T) { | 762 | t.Run(tc.want, func(t *testing.T) { | ||
| 757 | got := setupLog(t, yaml, tc.job, func(c *Config) { | 763 | got := setupLog(t, yaml, tc.job, func(c *Config) { | ||
| @@ -770,3 +776,74 @@ jobs: | |||||
| 770 | }) | 776 | }) | ||
| 771 | } | 777 | } | ||
| 772 | } | 778 | } | ||
| 779 | + | ||||
| 780 | + | func TestJobDisplayNameReadsNeedsVarsAndIgnoresRunnerKeys(t *testing.T) { | |||
| 781 | + | needTools(t, "bash") | |||
| 782 | + | yaml := `on: push | |||
| 783 | + | jobs: | |||
| 784 | + | j: | |||
| 785 | + | name: "${{ needs.a.outputs.o }}-${{ vars.V }}-${{ github.workspace }}-${{ github.secret_source }}-${{ github.token }}-${{ github.event_name }}" | |||
| 786 | + | runs-on: x | |||
| 787 | + | steps: | |||
| 788 | + | - run: echo hi | |||
| 789 | + | ` | |||
| 790 | + | got := setupLog(t, yaml, "j", func(c *Config) { | |||
| 791 | + | c.ContextsJSON = `{"github":{"repository":"octo/hello","event_name":"push","job":"j"},"needs":{"a":{"outputs":{"o":"out"}}},"vars":{"V":"v1"}}` | |||
| 792 | + | }) | |||
| 793 | + | // workspace, secret_source and token are added by the runner: empty for the name. | |||
| 794 | + | if !strings.HasSuffix(got, "Complete job name: out-v1----push") { | |||
| 795 | + | t.Errorf("got:\n%s", got) | |||
| 796 | + | } | |||
| 797 | + | } | |||
| 798 | + | ||||
| 799 | + | func TestJobDisplayNameIncludeOnlyMatrix(t *testing.T) { | |||
| 800 | + | needTools(t, "bash") | |||
| 801 | + | yaml := `on: push | |||
| 802 | + | jobs: | |||
| 803 | + | j: | |||
| 804 | + | runs-on: x | |||
| 805 | + | strategy: | |||
| 806 | + | matrix: | |||
| 807 | + | include: | |||
| 808 | + | - os: a | |||
| 809 | + | n: 1 | |||
| 810 | + | - os: b | |||
| 811 | + | n: 2 | |||
| 812 | + | steps: | |||
| 813 | + | - run: echo hi | |||
| 814 | + | ` | |||
| 815 | + | for matrix, want := range map[string]string{`{"os":"b","n":2}`: "j (b, 2)", `{"os":"a","n":1}`: "j (a, 1)"} { | |||
| 816 | + | got := setupLog(t, yaml, "j", func(c *Config) { | |||
| 817 | + | c.MatrixJSON = matrix | |||
| 818 | + | c.ContextsJSON = strings.TrimSuffix(testGitHubContext("j"), "}") + `,"matrix":` + matrix + `}` | |||
| 819 | + | }) | |||
| 820 | + | if !strings.HasSuffix(got, "Complete job name: "+want) { | |||
| 821 | + | t.Errorf("%s:\n%s", matrix, got) | |||
| 822 | + | } | |||
| 823 | + | } | |||
| 824 | + | } | |||
| 825 | + | ||||
| 826 | + | func TestJobDisplayNameFallsBackToTheKeyWithAWarning(t *testing.T) { | |||
| 827 | + | needTools(t, "bash") | |||
| 828 | + | yaml := `on: push | |||
| 829 | + | jobs: | |||
| 830 | + | j: | |||
| 831 | + | runs-on: x | |||
| 832 | + | strategy: | |||
| 833 | + | matrix: | |||
| 834 | + | n: [1, 2] | |||
| 835 | + | steps: | |||
| 836 | + | - run: echo hi | |||
| 837 | + | ` | |||
| 838 | + | var logs bytes.Buffer | |||
| 839 | + | got := setupLog(t, yaml, "j", func(c *Config) { | |||
| 840 | + | c.MatrixJSON = `{"n":5}` | |||
| 841 | + | c.Log = zerowrap.New(zerowrap.Config{Level: "debug", Format: "json", Output: &logs}) | |||
| 842 | + | }) | |||
| 843 | + | if !strings.HasSuffix(got, "Complete job name: j") { | |||
| 844 | + | t.Errorf("got:\n%s", got) | |||
| 845 | + | } | |||
| 846 | + | if !strings.Contains(logs.String(), `"level":"warn"`) || !strings.Contains(logs.String(), "using the job key") { | |||
| 847 | + | t.Errorf("no warning: %q", logs.String()) | |||
| 848 | + | } | |||
| 849 | + | } | |||
M internal/guest/jobexec.go +3 -1
| @@ -12,6 +12,8 @@ import ( | |||
| 12 | 12 | "syscall" | |
| 13 | 13 | "time" | |
| 14 | 14 | ||
| 15 | + | "github.com/bnema/zerowrap" | |
| 16 | + | ||
| 15 | 17 | "bnema.dev/ermite-runner/internal/execstream" | |
| 16 | 18 | "bnema.dev/ermite-runner/internal/execstream/execpb" | |
| 17 | 19 | "bnema.dev/ermite-runner/internal/executor" | |
| @@ -204,7 +206,7 @@ func runJob(ctx context.Context, conn *execstream.Conn, cfg ExecConfig, spec *ex | |||
| 204 | 206 | Runner: executor.RunnerInfo{Name: spec.GetRunnerName(), Environment: env}, | |
| 205 | 207 | RunnerVersion: spec.GetRunnerVersion(), | |
| 206 | 208 | Home: su.Home, ToolCache: os.Getenv("RUNNER_TOOL_CACHE"), BaseEnv: stepEnv(os.Environ(), su), Credential: su.Credential, | |
| 207 | - | Debug: spec.GetDebug(), Reporter: rep, | |
| 209 | + | Debug: spec.GetDebug(), Reporter: rep, Log: zerowrap.FromCtx(ctx), | |
| 208 | 210 | TermAfter: cfg.TermAfter, KillAfter: cfg.KillAfter, WaitDelay: cfg.WaitDelay, | |
| 209 | 211 | }) | |
| 210 | 212 | if res.Conclusion == executor.ConclusionSuccess { | |
| @@ -12,6 +12,8 @@ import ( | |||||
| 12 | "syscall" | 12 | "syscall" | ||
| 13 | "time" | 13 | "time" | ||
| 14 | 14 | ||||
| 15 | + | "github.com/bnema/zerowrap" | |||
| 16 | + | ||||
| 15 | "bnema.dev/ermite-runner/internal/execstream" | 17 | "bnema.dev/ermite-runner/internal/execstream" | ||
| 16 | "bnema.dev/ermite-runner/internal/execstream/execpb" | 18 | "bnema.dev/ermite-runner/internal/execstream/execpb" | ||
| 17 | "bnema.dev/ermite-runner/internal/executor" | 19 | "bnema.dev/ermite-runner/internal/executor" | ||
| @@ -204,7 +206,7 @@ func runJob(ctx context.Context, conn *execstream.Conn, cfg ExecConfig, spec *ex | |||||
| 204 | Runner: executor.RunnerInfo{Name: spec.GetRunnerName(), Environment: env}, | 206 | Runner: executor.RunnerInfo{Name: spec.GetRunnerName(), Environment: env}, | ||
| 205 | RunnerVersion: spec.GetRunnerVersion(), | 207 | RunnerVersion: spec.GetRunnerVersion(), | ||
| 206 | Home: su.Home, ToolCache: os.Getenv("RUNNER_TOOL_CACHE"), BaseEnv: stepEnv(os.Environ(), su), Credential: su.Credential, | 208 | Home: su.Home, ToolCache: os.Getenv("RUNNER_TOOL_CACHE"), BaseEnv: stepEnv(os.Environ(), su), Credential: su.Credential, | ||
| 207 | - | Debug: spec.GetDebug(), Reporter: rep, | 209 | + | Debug: spec.GetDebug(), Reporter: rep, Log: zerowrap.FromCtx(ctx), |
| 208 | TermAfter: cfg.TermAfter, KillAfter: cfg.KillAfter, WaitDelay: cfg.WaitDelay, | 210 | TermAfter: cfg.TermAfter, KillAfter: cfg.KillAfter, WaitDelay: cfg.WaitDelay, | ||
| 209 | }) | 211 | }) | ||
| 210 | if res.Conclusion == executor.ConclusionSuccess { | 212 | if res.Conclusion == executor.ConclusionSuccess { | ||