feat(executor): translate container paths in command output back to the host
- commit
- 994b8696fb03df3209910979bf3e52c571849264
- author
- brice · · 2026-10-14 05:54 UTC
- parent
- 40f32c5
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
5 files · +58 -1
M internal/executor/commands.go +5 -0
| @@ -144,6 +144,10 @@ type commandHost interface { | |||
| 144 | 144 | AddIssue(issue) | |
| 145 | 145 | // Workspace is github.workspace. | |
| 146 | 146 | Workspace() string | |
| 147 | + | // HostPath translates a path printed by a process in the job container | |
| 148 | + | // back to the host's (ContainerInfo.TranslateToHostPath); without a | |
| 149 | + | // container it is the identity. | |
| 150 | + | HostPath(p string) string | |
| 147 | 151 | // AddMask is SecretMasker.AddValue. | |
| 148 | 152 | AddMask(value string) | |
| 149 | 153 | // Matchers is the root of the problem matchers of the job. | |
| @@ -339,6 +343,7 @@ func (m *commandManager) addMask(c *actionCommand) { | |||
| 339 | 343 | ||
| 340 | 344 | // matcherPath roots a path of add-matcher and remove-matcher in the workspace. | |
| 341 | 345 | func (m *commandManager) matcherPath(file string) string { | |
| 346 | + | file = m.host.HostPath(file) // ActionCommandManager.cs:513-517, :575-580 | |
| 342 | 347 | if !filepath.IsAbs(file) { | |
| 343 | 348 | return filepath.Join(m.host.Workspace(), file) | |
| 344 | 349 | } | |
| @@ -144,6 +144,10 @@ type commandHost interface { | |||||
| 144 | AddIssue(issue) | 144 | AddIssue(issue) | ||
| 145 | // Workspace is github.workspace. | 145 | // Workspace is github.workspace. | ||
| 146 | Workspace() string | 146 | Workspace() string | ||
| 147 | + | // HostPath translates a path printed by a process in the job container | |||
| 148 | + | // back to the host's (ContainerInfo.TranslateToHostPath); without a | |||
| 149 | + | // container it is the identity. | |||
| 150 | + | HostPath(p string) string | |||
| 147 | // AddMask is SecretMasker.AddValue. | 151 | // AddMask is SecretMasker.AddValue. | ||
| 148 | AddMask(value string) | 152 | AddMask(value string) | ||
| 149 | // Matchers is the root of the problem matchers of the job. | 153 | // Matchers is the root of the problem matchers of the job. | ||
| @@ -339,6 +343,7 @@ func (m *commandManager) addMask(c *actionCommand) { | |||||
| 339 | 343 | ||||
| 340 | // matcherPath roots a path of add-matcher and remove-matcher in the workspace. | 344 | // matcherPath roots a path of add-matcher and remove-matcher in the workspace. | ||
| 341 | func (m *commandManager) matcherPath(file string) string { | 345 | func (m *commandManager) matcherPath(file string) string { | ||
| 346 | + | file = m.host.HostPath(file) // ActionCommandManager.cs:513-517, :575-580 | |||
| 342 | if !filepath.IsAbs(file) { | 347 | if !filepath.IsAbs(file) { | ||
| 343 | return filepath.Join(m.host.Workspace(), file) | 348 | return filepath.Join(m.host.Workspace(), file) | ||
| 344 | } | 349 | } | ||
M internal/executor/containers_exec_test.go +35 -0
| @@ -116,3 +116,38 @@ func TestNodeActionInContainerUsesExternalsAndAlpine(t *testing.T) { | |||
| 116 | 116 | } | |
| 117 | 117 | } | |
| 118 | 118 | } | |
| 119 | + | ||
| 120 | + | func TestCommandPathsFromTheContainerAreTranslated(t *testing.T) { | |
| 121 | + | f := newFakeCtr(t) | |
| 122 | + | // The step prints an annotation for a file at its container path. | |
| 123 | + | f.set(t, "exec_out", "::error file=/__w/hello/hello/src/a.go,line=3::boom\n") | |
| 124 | + | rec, res := ctrRun(t, f, ctrJob, " - name: s\n run: echo\n", nil) | |
| 125 | + | if res.Conclusion != ConclusionSuccess { | |
| 126 | + | t.Fatalf("%+v", res) | |
| 127 | + | } | |
| 128 | + | log := logOf(rec.step(t, "s")) | |
| 129 | + | if !strings.Contains(log, "##[error]boom") { | |
| 130 | + | t.Errorf("log:\n%s", log) | |
| 131 | + | } | |
| 132 | + | } | |
| 133 | + | ||
| 134 | + | func TestDockerActionJoinsTheJobNetworkAndContextHasContainerKey(t *testing.T) { | |
| 135 | + | f := newFakeCtr(t) | |
| 136 | + | rec, res := ctrRun(t, f, " services:\n redis:\n image: redis:7\n", | |
| 137 | + | " - uses: docker://alpine:3.20\n - name: ctx\n run: echo \"${{ toJSON(job.container) }}\"\n", nil) | |
| 138 | + | if res.Conclusion != ConclusionSuccess { | |
| 139 | + | t.Fatalf("%+v %v", res, rec.names()) | |
| 140 | + | } | |
| 141 | + | var run []string | |
| 142 | + | for _, c := range f.calls(t) { | |
| 143 | + | if c[0] == "run" { | |
| 144 | + | run = c | |
| 145 | + | } | |
| 146 | + | } | |
| 147 | + | if run == nil || !strings.Contains(strings.Join(run, "\n"), "\n--network\ngithub_network_") { | |
| 148 | + | t.Errorf("docker run without the job network: %v", run) | |
| 149 | + | } | |
| 150 | + | if got := logOf(rec.step(t, "ctx")); !strings.Contains(got, "\"network\": \"github_network_") { | |
| 151 | + | t.Errorf("ctx:\n%s", got) | |
| 152 | + | } | |
| 153 | + | } | |
| @@ -116,3 +116,38 @@ func TestNodeActionInContainerUsesExternalsAndAlpine(t *testing.T) { | |||||
| 116 | } | 116 | } | ||
| 117 | } | 117 | } | ||
| 118 | } | 118 | } | ||
| 119 | + | ||||
| 120 | + | func TestCommandPathsFromTheContainerAreTranslated(t *testing.T) { | |||
| 121 | + | f := newFakeCtr(t) | |||
| 122 | + | // The step prints an annotation for a file at its container path. | |||
| 123 | + | f.set(t, "exec_out", "::error file=/__w/hello/hello/src/a.go,line=3::boom\n") | |||
| 124 | + | rec, res := ctrRun(t, f, ctrJob, " - name: s\n run: echo\n", nil) | |||
| 125 | + | if res.Conclusion != ConclusionSuccess { | |||
| 126 | + | t.Fatalf("%+v", res) | |||
| 127 | + | } | |||
| 128 | + | log := logOf(rec.step(t, "s")) | |||
| 129 | + | if !strings.Contains(log, "##[error]boom") { | |||
| 130 | + | t.Errorf("log:\n%s", log) | |||
| 131 | + | } | |||
| 132 | + | } | |||
| 133 | + | ||||
| 134 | + | func TestDockerActionJoinsTheJobNetworkAndContextHasContainerKey(t *testing.T) { | |||
| 135 | + | f := newFakeCtr(t) | |||
| 136 | + | rec, res := ctrRun(t, f, " services:\n redis:\n image: redis:7\n", | |||
| 137 | + | " - uses: docker://alpine:3.20\n - name: ctx\n run: echo \"${{ toJSON(job.container) }}\"\n", nil) | |||
| 138 | + | if res.Conclusion != ConclusionSuccess { | |||
| 139 | + | t.Fatalf("%+v %v", res, rec.names()) | |||
| 140 | + | } | |||
| 141 | + | var run []string | |||
| 142 | + | for _, c := range f.calls(t) { | |||
| 143 | + | if c[0] == "run" { | |||
| 144 | + | run = c | |||
| 145 | + | } | |||
| 146 | + | } | |||
| 147 | + | if run == nil || !strings.Contains(strings.Join(run, "\n"), "\n--network\ngithub_network_") { | |||
| 148 | + | t.Errorf("docker run without the job network: %v", run) | |||
| 149 | + | } | |||
| 150 | + | if got := logOf(rec.step(t, "ctx")); !strings.Contains(got, "\"network\": \"github_network_") { | |||
| 151 | + | t.Errorf("ctx:\n%s", got) | |||
| 152 | + | } | |||
| 153 | + | } | |||
M internal/executor/issues.go +5 -0
| @@ -290,6 +290,11 @@ func (m *commandManager) commandIssue(level AnnotationLevel, name string, c *act | |||
| 290 | 290 | } | |
| 291 | 291 | validateLinesAndColumns(props, name, h.Debug) | |
| 292 | 292 | if file := props["file"]; file != "" { | |
| 293 | + | // ActionCommandManager.cs:675-681: the path of a container is the host's. | |
| 294 | + | if hp := h.HostPath(file); hp != file { | |
| 295 | + | file = hp | |
| 296 | + | props["file"] = hp | |
| 297 | + | } | |
| 293 | 298 | if rel := makeRelative(file, h.Workspace()); rel != file && rel != "" { | |
| 294 | 299 | props["file"] = rel | |
| 295 | 300 | } | |
| @@ -290,6 +290,11 @@ func (m *commandManager) commandIssue(level AnnotationLevel, name string, c *act | |||||
| 290 | } | 290 | } | ||
| 291 | validateLinesAndColumns(props, name, h.Debug) | 291 | validateLinesAndColumns(props, name, h.Debug) | ||
| 292 | if file := props["file"]; file != "" { | 292 | if file := props["file"]; file != "" { | ||
| 293 | + | // ActionCommandManager.cs:675-681: the path of a container is the host's. | |||
| 294 | + | if hp := h.HostPath(file); hp != file { | |||
| 295 | + | file = hp | |||
| 296 | + | props["file"] = hp | |||
| 297 | + | } | |||
| 293 | if rel := makeRelative(file, h.Workspace()); rel != file && rel != "" { | 298 | if rel := makeRelative(file, h.Workspace()); rel != file && rel != "" { | ||
| 294 | props["file"] = rel | 299 | props["file"] = rel | ||
| 295 | } | 300 | } | ||
M internal/executor/outputmgr.go +5 -1
| @@ -343,9 +343,13 @@ func (o *outputManager) convertToIssue(m *issueMatch) (issue, bool) { | |||
| 343 | 343 | file = filepath.Join(dir, file) | |
| 344 | 344 | } | |
| 345 | 345 | } | |
| 346 | - | // Root using workspace. | |
| 346 | + | // Root using workspace; an absolute path is the container's and is | |
| 347 | + | // translated to the host's (OutputManager.cs:281-311). | |
| 347 | 348 | if !filepath.IsAbs(file) { | |
| 348 | 349 | file = filepath.Join(s.Workspace(), file) | |
| 350 | + | } else { | |
| 351 | + | file = filepath.Clean(file) | |
| 352 | + | file = filepath.Clean(s.HostPath(file)) | |
| 349 | 353 | } | |
| 350 | 354 | file = filepath.Clean(file) | |
| 351 | 355 | if st, err := os.Stat(file); err == nil && !st.IsDir() { | |
| @@ -343,9 +343,13 @@ func (o *outputManager) convertToIssue(m *issueMatch) (issue, bool) { | |||||
| 343 | file = filepath.Join(dir, file) | 343 | file = filepath.Join(dir, file) | ||
| 344 | } | 344 | } | ||
| 345 | } | 345 | } | ||
| 346 | - | // Root using workspace. | 346 | + | // Root using workspace; an absolute path is the container's and is |
| 347 | + | // translated to the host's (OutputManager.cs:281-311). | |||
| 347 | if !filepath.IsAbs(file) { | 348 | if !filepath.IsAbs(file) { | ||
| 348 | file = filepath.Join(s.Workspace(), file) | 349 | file = filepath.Join(s.Workspace(), file) | ||
| 350 | + | } else { | |||
| 351 | + | file = filepath.Clean(file) | |||
| 352 | + | file = filepath.Clean(s.HostPath(file)) | |||
| 349 | } | 353 | } | ||
| 350 | file = filepath.Clean(file) | 354 | file = filepath.Clean(file) | ||
| 351 | if st, err := os.Stat(file); err == nil && !st.IsDir() { | 355 | if st, err := os.Stat(file); err == nil && !st.IsDir() { | ||
M internal/executor/results.go +8 -0
| @@ -42,6 +42,14 @@ func (j *job) addIssueFor(s *stepRun, is issue) { | |||
| 42 | 42 | // Workspace is github.workspace. | |
| 43 | 43 | func (s *stepRun) Workspace() string { return s.j.githubValue("workspace") } | |
| 44 | 44 | ||
| 45 | + | // HostPath is TranslateToHostPath of the job container. | |
| 46 | + | func (s *stepRun) HostPath(p string) string { | |
| 47 | + | if c := s.j.stepContainer(); c != nil { | |
| 48 | + | return c.translateToHost(p) | |
| 49 | + | } | |
| 50 | + | return p | |
| 51 | + | } | |
| 52 | + | ||
| 45 | 53 | func (j *job) githubValue(k string) string { | |
| 46 | 54 | j.mu.Lock() | |
| 47 | 55 | defer j.mu.Unlock() | |
| @@ -42,6 +42,14 @@ func (j *job) addIssueFor(s *stepRun, is issue) { | |||||
| 42 | // Workspace is github.workspace. | 42 | // Workspace is github.workspace. | ||
| 43 | func (s *stepRun) Workspace() string { return s.j.githubValue("workspace") } | 43 | func (s *stepRun) Workspace() string { return s.j.githubValue("workspace") } | ||
| 44 | 44 | ||||
| 45 | + | // HostPath is TranslateToHostPath of the job container. | |||
| 46 | + | func (s *stepRun) HostPath(p string) string { | |||
| 47 | + | if c := s.j.stepContainer(); c != nil { | |||
| 48 | + | return c.translateToHost(p) | |||
| 49 | + | } | |||
| 50 | + | return p | |||
| 51 | + | } | |||
| 52 | + | ||||
| 45 | func (j *job) githubValue(k string) string { | 53 | func (j *job) githubValue(k string) string { | ||
| 46 | j.mu.Lock() | 54 | j.mu.Lock() | ||
| 47 | defer j.mu.Unlock() | 55 | defer j.mu.Unlock() | ||