From 41f8b03b791ecb22e7c4d9679e2618e403e75da6 Mon Sep 17 00:00:00 2001 From: Earl Warren Date: Tue, 12 Aug 2025 10:09:42 +0000 Subject: [PATCH] fix: the configuration must not be used as temporary storage (#849) rc.Config should be treated as read-only and not as a temporary storage for the variable list of valid volumes for containers sharing this configuration. Refs forgejo/runner#848 - bug fixes - [PR](https://code.forgejo.org/forgejo/runner/pulls/849): fix: the configuration must not be used as temporary storage Reviewed-on: https://code.forgejo.org/forgejo/runner/pulls/849 Reviewed-by: Mathieu Fenniak Reviewed-by: Michael Kriese Co-authored-by: Earl Warren Co-committed-by: Earl Warren --- act/runner/action.go | 4 ++-- act/runner/run_context.go | 20 ++++++++++---------- act/runner/run_context_test.go | 14 ++------------ act/runner/step_docker.go | 4 ++-- 4 files changed, 16 insertions(+), 26 deletions(-) diff --git a/act/runner/action.go b/act/runner/action.go index 6b27288e..3f50bccc 100644 --- a/act/runner/action.go +++ b/act/runner/action.go @@ -428,7 +428,7 @@ func newStepContainer(ctx context.Context, step step, image string, cmd, entrypo 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, validVolumes := rc.GetBindsAndMounts() networkMode := fmt.Sprintf("container:%s", rc.jobContainerName()) if rc.IsHostEnv(ctx) { networkMode = "default" @@ -452,7 +452,7 @@ func newStepContainer(ctx context.Context, step step, image string, cmd, entrypo UsernsMode: rc.Config.UsernsMode, Platform: rc.Config.ContainerArchitecture, AutoRemove: rc.Config.AutoRemove, - ValidVolumes: rc.Config.ValidVolumes, + ValidVolumes: validVolumes, ConfigOptions: rc.Config.ContainerOptions, }) diff --git a/act/runner/run_context.go b/act/runner/run_context.go index 97615023..219adea4 100644 --- a/act/runner/run_context.go +++ b/act/runner/run_context.go @@ -124,7 +124,7 @@ func getDockerDaemonSocketMountPath(daemonPath string) string { } // Returns the binds and mounts for the container, resolving paths as appopriate -func (rc *RunContext) GetBindsAndMounts() ([]string, map[string]string) { +func (rc *RunContext) GetBindsAndMounts() ([]string, map[string]string, []string) { name := rc.jobContainerName() if rc.Config.ContainerDaemonSocket == "" { @@ -171,13 +171,13 @@ func (rc *RunContext) GetBindsAndMounts() ([]string, map[string]string) { mounts[name] = ext.ToContainerPath(rc.Config.Workdir) } - // add some default binds and mounts to ValidVolumes - rc.Config.ValidVolumes = append(rc.Config.ValidVolumes, name) - rc.Config.ValidVolumes = append(rc.Config.ValidVolumes, name+"-env") - // TODO: add a new configuration to control whether the docker daemon can be mounted - rc.Config.ValidVolumes = append(rc.Config.ValidVolumes, getDockerDaemonSocketMountPath(rc.Config.ContainerDaemonSocket)) - - return binds, mounts + validVolumes := []string{ + name, + name + "-env", + getDockerDaemonSocketMountPath(rc.Config.ContainerDaemonSocket), + } + validVolumes = append(validVolumes, rc.Config.ValidVolumes...) + return binds, mounts, validVolumes } //go:embed lxc-helpers-lib.sh @@ -446,7 +446,7 @@ func (rc *RunContext) prepareJobContainer(ctx context.Context) error { 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, validVolumes := rc.GetBindsAndMounts() networkName, createAndDeleteNetwork := rc.getNetworkName(ctx) // add service containers @@ -570,7 +570,7 @@ func (rc *RunContext) prepareJobContainer(ctx context.Context) error { UsernsMode: rc.Config.UsernsMode, Platform: rc.Config.ContainerArchitecture, AutoRemove: rc.Config.AutoRemove, - ValidVolumes: rc.Config.ValidVolumes, + ValidVolumes: validVolumes, JobOptions: rc.options(ctx), ConfigOptions: rc.Config.ContainerOptions, diff --git a/act/runner/run_context_test.go b/act/runner/run_context_test.go index 52dc86f8..1a6aa66c 100644 --- a/act/runner/run_context_test.go +++ b/act/runner/run_context_test.go @@ -219,7 +219,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() // Name binds/mounts are either/or if config.BindWorkdir { @@ -271,7 +271,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() if len(testcase.wantbind) > 0 { assert.Contains(t, gotbind, testcase.wantbind) @@ -764,11 +764,6 @@ jobs: ConfigOptions: "", JobOptions: "", AutoRemove: false, - ValidVolumes: []string{ - "WORKFLOW-e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855_JOB", - "WORKFLOW-e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855_JOB-env", - "/var/run/docker.sock", - }, }, { Name: "WORKFLOW-e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca49599-c233cf913e1d0c90cc1404ee09917e625f9cb82156ca3d7cb10b729d563728ea", @@ -792,11 +787,6 @@ jobs: ConfigOptions: "", JobOptions: "", AutoRemove: false, - ValidVolumes: []string{ - "WORKFLOW-e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855_JOB", - "WORKFLOW-e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855_JOB-env", - "/var/run/docker.sock", - }, }, }, }, diff --git a/act/runner/step_docker.go b/act/runner/step_docker.go index 5ecddd21..db86ad35 100644 --- a/act/runner/step_docker.go +++ b/act/runner/step_docker.go @@ -110,7 +110,7 @@ func (sd *stepDocker) newStepContainer(ctx context.Context, image string, cmd, e 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, validVolumes := rc.GetBindsAndMounts() stepContainer := ContainerNewContainer(&container.NewContainerInput{ Cmd: cmd, Entrypoint: entrypoint, @@ -130,7 +130,7 @@ func (sd *stepDocker) newStepContainer(ctx context.Context, image string, cmd, e UsernsMode: rc.Config.UsernsMode, Platform: rc.Config.ContainerArchitecture, AutoRemove: rc.Config.AutoRemove, - ValidVolumes: rc.Config.ValidVolumes, + ValidVolumes: validVolumes, }) return stepContainer }