diff --git a/pkg/model/workflow.go b/pkg/model/workflow.go index 8dd5f6634db..ac880167ab1 100644 --- a/pkg/model/workflow.go +++ b/pkg/model/workflow.go @@ -289,24 +289,32 @@ func (j *Job) Secrets() map[string]string { return val } -// Container details for the job -func (j *Job) Container() *ContainerSpec { +// DecodeContainerNode decodes a job-level container yaml node into a ContainerSpec +func DecodeContainerNode(node *yaml.Node) *ContainerSpec { + if node == nil { + return nil + } var val *ContainerSpec - switch j.RawContainer.Kind { + switch node.Kind { case yaml.ScalarNode: val = new(ContainerSpec) - if !decodeNode(j.RawContainer, &val.Image) { + if !decodeNode(*node, &val.Image) { return nil } case yaml.MappingNode: val = new(ContainerSpec) - if !decodeNode(j.RawContainer, val) { + if !decodeNode(*node, val) { return nil } } return val } +// Container details for the job +func (j *Job) Container() *ContainerSpec { + return DecodeContainerNode(&j.RawContainer) +} + // Needs list for Job func (j *Job) Needs() []string { switch j.RawNeeds.Kind { diff --git a/pkg/runner/action.go b/pkg/runner/action.go index b46a4d03a75..5cf8621dde6 100644 --- a/pkg/runner/action.go +++ b/pkg/runner/action.go @@ -406,7 +406,7 @@ func newStepContainer(ctx context.Context, step step, image string, cmd []string envList = append(envList, fmt.Sprintf("%s=%s", "RUNNER_ARCH", container.RunnerArch(ctx))) envList = append(envList, fmt.Sprintf("%s=%s", "RUNNER_TEMP", "/tmp")) - binds, mounts := rc.GetBindsAndMounts() + binds, mounts := rc.GetBindsAndMounts(ctx) networkMode := fmt.Sprintf("container:%s", rc.jobContainerName()) var workdir string if rc.IsHostEnv(ctx) { diff --git a/pkg/runner/run_context.go b/pkg/runner/run_context.go index 5d4277123ed..938f6d126b6 100644 --- a/pkg/runner/run_context.go +++ b/pkg/runner/run_context.go @@ -26,6 +26,7 @@ import ( "github.com/nektos/act/pkg/exprparser" "github.com/nektos/act/pkg/model" "github.com/opencontainers/selinux/go-selinux" + yaml "gopkg.in/yaml.v3" ) // RunContext contains info about current job @@ -53,6 +54,8 @@ type RunContext struct { caller *caller // job calling this RunContext (reusable workflows) Cancelled bool nodeToolFullPath string + + resolvedJobContainer *resolvedContainer // memoized per-cell resolved container } func (rc *RunContext) AddMask(mask string) { @@ -124,7 +127,7 @@ func getDockerDaemonSocketMountPath(daemonPath string) string { } // Returns the binds and mounts for the container, resolving paths as appropriate -func (rc *RunContext) GetBindsAndMounts() ([]string, map[string]string) { +func (rc *RunContext) GetBindsAndMounts(ctx context.Context) ([]string, map[string]string) { name := rc.jobContainerName() if rc.Config.ContainerDaemonSocket == "" { @@ -153,7 +156,7 @@ func (rc *RunContext) GetBindsAndMounts() ([]string, map[string]string) { } if job := rc.Run.Job(); job != nil { - if container := job.Container(); container != nil { + if container := rc.resolveJobContainer(ctx); container != nil { for _, v := range container.Volumes { if !strings.Contains(v, ":") || filepath.IsAbs(v) { // Bind anonymous volume or host file. @@ -283,7 +286,7 @@ func (rc *RunContext) startJobContainer() common.Executor { envList = append(envList, fmt.Sprintf("%s=%s", "LANG", "C.UTF-8")) // Use same locale as GitHub Actions ext := container.LinuxContainerEnvironmentExtensions{} - binds, mounts := rc.GetBindsAndMounts() + binds, mounts := rc.GetBindsAndMounts(ctx) // specify the network to which the container will connect when `docker create` stage. (like execute command line: docker create --network ) // if using service containers, will create a new network for the containers. @@ -731,12 +734,74 @@ func (rc *RunContext) Executor() (common.Executor, error) { }, nil } -func (rc *RunContext) containerImage(ctx context.Context) string { +// resolvedContainer distinguishes a memoized nil spec from a not-yet-resolved container. +type resolvedContainer struct { + spec *model.ContainerSpec +} + +func deepCopyYamlNode(n *yaml.Node) *yaml.Node { + if n == nil { + return nil + } + cp := *n + cp.Content = nil + if len(n.Content) > 0 { + cp.Content = make([]*yaml.Node, len(n.Content)) + for i, child := range n.Content { + cp.Content[i] = deepCopyYamlNode(child) + } + } + if n.Alias != nil { + cp.Alias = deepCopyYamlNode(n.Alias) + } + return &cp +} + +// resolveJobContainer evaluates the job container against this cell's context, +// deep-copying RawContainer so parallel matrix cells never evaluate the shared node +// in place. The result is memoized; nil means host execution. +func (rc *RunContext) resolveJobContainer(ctx context.Context) *model.ContainerSpec { + if rc.resolvedJobContainer != nil { + return rc.resolvedJobContainer.spec + } + + if rc.Run == nil { + rc.resolvedJobContainer = &resolvedContainer{spec: nil} + return nil + } job := rc.Run.Job() + if job == nil { + rc.resolvedJobContainer = &resolvedContainer{spec: nil} + return nil + } + + nodeCopy := deepCopyYamlNode(&job.RawContainer) + if nodeCopy == nil || nodeCopy.Kind == 0 { + rc.resolvedJobContainer = &resolvedContainer{spec: nil} + return nil + } + + // A nil ExprEval (consumer reached before startJob) falls back to a pure + // decode of the un-evaluated node, matching Job.Container(). + if rc.ExprEval != nil { + if err := rc.ExprEval.EvaluateYamlNode(ctx, nodeCopy); err != nil { + common.Logger(ctx).Errorf("Error while evaluating container: %v", err) + rc.resolvedJobContainer = &resolvedContainer{spec: nil} + return nil + } + } - c := job.Container() + spec := model.DecodeContainerNode(nodeCopy) + if rc.ExprEval != nil { + rc.resolvedJobContainer = &resolvedContainer{spec: spec} + } + return spec +} + +func (rc *RunContext) containerImage(ctx context.Context) string { + c := rc.resolveJobContainer(ctx) if c != nil { - return rc.ExprEval.Interpolate(ctx, c.Image) + return c.Image } return "" @@ -781,10 +846,9 @@ func (rc *RunContext) platformImage(ctx context.Context) string { } func (rc *RunContext) options(ctx context.Context) string { - job := rc.Run.Job() - c := job.Container() + c := rc.resolveJobContainer(ctx) if c != nil { - return rc.ExprEval.Interpolate(ctx, c.Options) + return c.Options } return rc.Config.ContainerOptions @@ -1098,7 +1162,7 @@ func (rc *RunContext) handleCredentials(ctx context.Context) (string, string, er username := rc.Config.Secrets["DOCKER_USERNAME"] password := rc.Config.Secrets["DOCKER_PASSWORD"] - container := rc.Run.Job().Container() + container := rc.resolveJobContainer(ctx) if container == nil || container.Credentials == nil { return username, password, nil } diff --git a/pkg/runner/run_context_test.go b/pkg/runner/run_context_test.go index de725b037e6..b67c2c458f8 100644 --- a/pkg/runner/run_context_test.go +++ b/pkg/runner/run_context_test.go @@ -1,6 +1,7 @@ package runner import ( + "bytes" "context" "fmt" "os" @@ -8,6 +9,7 @@ import ( "runtime" "sort" "strings" + "sync" "testing" "github.com/golang-jwt/jwt/v5" @@ -270,7 +272,7 @@ func TestRunContext_GetBindsAndMounts(t *testing.T) { config := testcase.rc.Config config.Workdir = testcase.name config.BindWorkdir = bindWorkDir - gotbind, gotmount := rctemplate.GetBindsAndMounts() + gotbind, gotmount := rctemplate.GetBindsAndMounts(context.Background()) // Name binds/mounts are either/or if config.BindWorkdir { @@ -322,7 +324,7 @@ func TestRunContext_GetBindsAndMounts(t *testing.T) { rc.Run.JobID = "job1" rc.Run.Workflow.Jobs = map[string]*model.Job{"job1": job} - gotbind, gotmount := rc.GetBindsAndMounts() + gotbind, gotmount := rc.GetBindsAndMounts(context.Background()) if len(testcase.wantbind) > 0 { assert.Contains(t, gotbind, testcase.wantbind) @@ -726,3 +728,140 @@ func TestSetRuntimeVariablesWithRunID(t *testing.T) { assert.True(t, ok, "scp claim exists") assert.Equal(t, "Actions.Results:45:45", scp, "contains expected scp claim") } + +func newContainerRC(ctx context.Context, t *testing.T, rawContainer interface{}, matrix map[string]interface{}) *RunContext { + t.Helper() + job := &model.Job{} + if rawContainer != nil { + if err := job.RawContainer.Encode(rawContainer); err != nil { + assert.NoError(t, err) + t.FailNow() + } + } + rc := &RunContext{ + Config: &Config{Workdir: "."}, + Run: &model.Run{ + JobID: "job1", + Workflow: &model.Workflow{ + Name: "test-workflow", + Jobs: map[string]*model.Job{"job1": job}, + }, + }, + Matrix: matrix, + } + rc.ExprEval = rc.NewExpressionEvaluator(ctx) + return rc +} + +func TestResolveJobContainer(t *testing.T) { + ctx := context.Background() + + t.Run("distinct images across cells (leak guard)", func(t *testing.T) { + job := &model.Job{} + assert.NoError(t, job.RawContainer.Encode("${{ matrix.container }}")) + wf := &model.Workflow{Name: "w", Jobs: map[string]*model.Job{"job1": job}} + mk := func(image string) *RunContext { + rc := &RunContext{ + Config: &Config{Workdir: "."}, + Run: &model.Run{JobID: "job1", Workflow: wf}, + Matrix: map[string]interface{}{"container": map[string]interface{}{"image": image}}, + } + rc.ExprEval = rc.NewExpressionEvaluator(ctx) + return rc + } + rcA, rcB := mk("img-a"), mk("img-b") + specA := rcA.resolveJobContainer(ctx) + specB := rcB.resolveJobContainer(ctx) + assert.NotNil(t, specA) + assert.NotNil(t, specB) + assert.Equal(t, "img-a", specA.Image) + assert.Equal(t, "img-b", specB.Image) + }) + + t.Run("sibling-field preservation", func(t *testing.T) { + rc := newContainerRC(ctx, t, "${{ matrix.container }}", map[string]interface{}{ + "container": map[string]interface{}{ + "image": "x", + "options": "--cpus 2", + "env": map[string]interface{}{"FOO": "bar"}, + "ports": []interface{}{"8080"}, + }, + }) + spec := rc.resolveJobContainer(ctx) + assert.NotNil(t, spec) + assert.Equal(t, "x", spec.Image) + assert.Equal(t, "--cpus 2", spec.Options) + assert.Equal(t, "bar", spec.Env["FOO"]) + assert.Equal(t, []string{"8080"}, spec.Ports) + }) + + t.Run("absent container -> host fallback (empty image, no Object leak)", func(t *testing.T) { + // An undefined `${{ matrix.container }}` resolves to an empty scalar, which + // decodes to &ContainerSpec{Image: ""} (NOT nil) without erroring. + var buf bytes.Buffer + prev := log.StandardLogger().Out + log.SetOutput(&buf) + defer log.SetOutput(prev) + + rc := newContainerRC(ctx, t, "${{ matrix.container }}", map[string]interface{}{"unused": "1"}) + spec := rc.resolveJobContainer(ctx) + assert.Equal(t, &model.ContainerSpec{Image: ""}, spec) + assert.NotContains(t, buf.String(), "Error while evaluating container") + }) + + t.Run("plain string unchanged (back-compat)", func(t *testing.T) { + rc := newContainerRC(ctx, t, "ghcr.io/foo/bar:tag", nil) + spec := rc.resolveJobContainer(ctx) + assert.NotNil(t, spec) + assert.Equal(t, "ghcr.io/foo/bar:tag", spec.Image) + }) + + t.Run("nested expr in map (recursive eval)", func(t *testing.T) { + rc := newContainerRC(ctx, t, map[string]interface{}{"image": "alpine:${{ matrix.tag }}"}, + map[string]interface{}{"tag": "3.19"}) + spec := rc.resolveJobContainer(ctx) + assert.NotNil(t, spec) + assert.Equal(t, "alpine:3.19", spec.Image) + }) + + t.Run("nil ExprEval -> pure decode, no panic (back-compat)", func(t *testing.T) { + job := &model.Job{} + assert.NoError(t, job.RawContainer.Encode(map[string][]string{"volumes": {"/volume"}})) + rc := &RunContext{ + Config: &Config{Workdir: "."}, + Run: &model.Run{ + JobID: "job1", + Workflow: &model.Workflow{Name: "w", Jobs: map[string]*model.Job{"job1": job}}, + }, + } + spec := rc.resolveJobContainer(ctx) + assert.Equal(t, &model.ContainerSpec{Volumes: []string{"/volume"}}, spec) + }) +} + +func TestResolveJobContainer_ParallelCellsNoLeak(t *testing.T) { + job := &model.Job{} + assert.NoError(t, job.RawContainer.Encode("${{ matrix.container }}")) + wf := &model.Workflow{Name: "w", Jobs: map[string]*model.Job{"job1": job}} + + mk := func(image string) *RunContext { + rc := &RunContext{ + Config: &Config{Workdir: "."}, + Run: &model.Run{JobID: "job1", Workflow: wf}, + Matrix: map[string]interface{}{"container": map[string]interface{}{"image": image}}, + } + rc.ExprEval = rc.NewExpressionEvaluator(context.Background()) + return rc + } + rcA, rcB := mk("img-a"), mk("img-b") + + var wg sync.WaitGroup + var gotA, gotB string + wg.Add(2) + go func() { defer wg.Done(); gotA = rcA.resolveJobContainer(context.Background()).Image }() + go func() { defer wg.Done(); gotB = rcB.resolveJobContainer(context.Background()).Image }() + wg.Wait() + + assert.Equal(t, "img-a", gotA) + assert.Equal(t, "img-b", gotB) +} diff --git a/pkg/runner/runner_test.go b/pkg/runner/runner_test.go index 5c3959af2cd..7850f840bea 100644 --- a/pkg/runner/runner_test.go +++ b/pkg/runner/runner_test.go @@ -273,6 +273,7 @@ func TestRunEvent(t *testing.T) { {workdir, "job-container", "push", "", platforms, secrets}, {workdir, "job-container-non-root", "push", "", platforms, secrets}, {workdir, "job-container-invalid-credentials", "push", "failed to handle credentials: failed to interpolate container.credentials.password", platforms, secrets}, + {workdir, "matrix-container", "push", "", platforms, secrets}, {workdir, "container-hostname", "push", "", platforms, secrets}, {workdir, "remote-action-docker", "push", "", platforms, secrets}, {workdir, "remote-action-docker-new-cache", "push", "", platforms, secrets}, diff --git a/pkg/runner/step.go b/pkg/runner/step.go index 5ee0dbcf050..01f4331dc6a 100644 --- a/pkg/runner/step.go +++ b/pkg/runner/step.go @@ -272,9 +272,8 @@ func setupEnv(ctx context.Context, step step) error { func mergeEnv(ctx context.Context, step step) { env := step.getEnv() rc := step.getRunContext() - job := rc.Run.Job() - c := job.Container() + c := rc.resolveJobContainer(ctx) if c != nil { mergeIntoMap(step, env, rc.GetEnv(), c.Env) } else { diff --git a/pkg/runner/step_docker.go b/pkg/runner/step_docker.go index cc28a3b53c5..1d8bd1dff34 100644 --- a/pkg/runner/step_docker.go +++ b/pkg/runner/step_docker.go @@ -112,7 +112,7 @@ func (sd *stepDocker) newStepContainer(ctx context.Context, image string, cmd [] envList = append(envList, fmt.Sprintf("%s=%s", "RUNNER_ARCH", container.RunnerArch(ctx))) envList = append(envList, fmt.Sprintf("%s=%s", "RUNNER_TEMP", "/tmp")) - binds, mounts := rc.GetBindsAndMounts() + binds, mounts := rc.GetBindsAndMounts(ctx) stepContainer := ContainerNewContainer(&container.NewContainerInput{ Cmd: cmd, Entrypoint: entrypoint, diff --git a/pkg/runner/testdata/matrix-container/push.yml b/pkg/runner/testdata/matrix-container/push.yml new file mode 100644 index 00000000000..809746ab073 --- /dev/null +++ b/pkg/runner/testdata/matrix-container/push.yml @@ -0,0 +1,15 @@ +name: matrix-container +on: push +jobs: + test: + strategy: + matrix: + include: + - container: + image: ghcr.io/catthehacker/ubuntu:act-latest + - container: + image: ghcr.io/catthehacker/ubuntu:act-latest + runs-on: ubuntu-latest + container: ${{ matrix.container }} + steps: + - run: echo "resolved container image OK"