From 95e411da06db962cb57e1d4744812b6a1d733a5d Mon Sep 17 00:00:00 2001 From: ChristopherHX Date: Tue, 25 Feb 2025 03:28:17 +0100 Subject: [PATCH] fix: remote docker actions new action cache and dry run mode (#2513) * fixes * Add TestDockerCopyTarStreamDryRun * increase coverage a bit * fixup * fixup --------- Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> --- pkg/container/docker_run.go | 3 +++ pkg/container/docker_run_test.go | 23 ++++++++++++++++++ pkg/runner/action.go | 24 +++++++++---------- pkg/runner/runner_test.go | 1 + .../config/config.yml | 1 + .../remote-action-docker-new-cache/push.yml | 10 ++++++++ 6 files changed, 50 insertions(+), 12 deletions(-) create mode 100644 pkg/runner/testdata/remote-action-docker-new-cache/config/config.yml create mode 100644 pkg/runner/testdata/remote-action-docker-new-cache/push.yml diff --git a/pkg/container/docker_run.go b/pkg/container/docker_run.go index d2dc2fd..0ce1cb9 100644 --- a/pkg/container/docker_run.go +++ b/pkg/container/docker_run.go @@ -699,6 +699,9 @@ func (cr *containerReference) waitForCommand(ctx context.Context, isTerminal boo } func (cr *containerReference) CopyTarStream(ctx context.Context, destPath string, tarStream io.Reader) error { + if common.Dryrun(ctx) { + return nil + } // Mkdir buf := &bytes.Buffer{} tw := tar.NewWriter(buf) diff --git a/pkg/container/docker_run_test.go b/pkg/container/docker_run_test.go index ec992b6..5256cfe 100644 --- a/pkg/container/docker_run_test.go +++ b/pkg/container/docker_run_test.go @@ -14,6 +14,7 @@ import ( "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/container" "github.com/docker/docker/client" + "github.com/nektos/act/pkg/common" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" ) @@ -195,6 +196,28 @@ func TestDockerCopyTarStream(t *testing.T) { client.AssertExpectations(t) } +func TestDockerCopyTarStreamDryRun(t *testing.T) { + ctx := common.WithDryrun(context.Background(), true) + + conn := &mockConn{} + + client := &mockDockerClient{} + client.AssertNotCalled(t, "CopyToContainer", ctx, "123", "/", mock.Anything, mock.AnythingOfType("container.CopyToContainerOptions")) + client.AssertNotCalled(t, "CopyToContainer", ctx, "123", "/var/run/act", mock.Anything, mock.AnythingOfType("container.CopyToContainerOptions")) + cr := &containerReference{ + id: "123", + cli: client, + input: &NewContainerInput{ + Image: "image", + }, + } + + _ = cr.CopyTarStream(ctx, "/var/run/act", &bytes.Buffer{}) + + conn.AssertExpectations(t) + client.AssertExpectations(t) +} + func TestDockerCopyTarStreamErrorInCopyFiles(t *testing.T) { ctx := context.Background() diff --git a/pkg/runner/action.go b/pkg/runner/action.go index 1b916af..d86b7cc 100644 --- a/pkg/runner/action.go +++ b/pkg/runner/action.go @@ -186,11 +186,11 @@ func runActionImpl(step actionStep, actionDir string, remoteAction *remoteAction return rc.execJobContainer(containerArgs, *step.getEnv(), "", "")(ctx) case model.ActionRunsUsingDocker: - location := actionLocation if remoteAction == nil { - location = containerActionDir + actionDir = "" + actionPath = containerActionDir } - return execAsDocker(ctx, step, actionName, location, remoteAction == nil, "entrypoint") + return execAsDocker(ctx, step, actionName, actionDir, actionPath, remoteAction == nil, "entrypoint") case model.ActionRunsUsingComposite: if err := maybeCopyToActionDir(ctx, step, actionDir, actionPath, containerActionDir); err != nil { return err @@ -243,7 +243,7 @@ func removeGitIgnore(ctx context.Context, directory string) error { // TODO: break out parts of function to reduce complexicity // //nolint:gocyclo -func execAsDocker(ctx context.Context, step actionStep, actionName string, basedir string, localAction bool, entrypointType string) error { +func execAsDocker(ctx context.Context, step actionStep, actionName, basedir, subpath string, localAction bool, entrypointType string) error { logger := common.Logger(ctx) rc := step.getRunContext() action := step.getActionModel() @@ -260,7 +260,7 @@ func execAsDocker(ctx context.Context, step actionStep, actionName string, based image = fmt.Sprintf("%s-dockeraction:%s", regexp.MustCompile("[^a-zA-Z0-9]").ReplaceAllString(actionName, "-"), "latest") image = fmt.Sprintf("act-%s", strings.TrimLeft(image, "-")) image = strings.ToLower(image) - contextDir, fileName := filepath.Split(filepath.Join(basedir, action.Runs.Image)) + contextDir, fileName := path.Split(path.Join(subpath, action.Runs.Image)) anyArchExists, err := container.ImageExistsLocally(ctx, image, "any") if err != nil { @@ -300,7 +300,7 @@ func execAsDocker(ctx context.Context, step actionStep, actionName string, based defer buildContext.Close() } prepImage = container.NewDockerBuildExecutor(container.NewDockerBuildExecutorInput{ - ContextDir: contextDir, + ContextDir: filepath.Join(basedir, contextDir), Dockerfile: fileName, ImageTag: image, BuildContext: buildContext, @@ -557,11 +557,11 @@ func runPreStep(step actionStep) common.Executor { return rc.execJobContainer(containerArgs, *step.getEnv(), "", "")(ctx) case model.ActionRunsUsingDocker: - location := actionLocation if remoteAction == nil { - location = containerActionDir + actionDir = "" + actionPath = containerActionDir } - return execAsDocker(ctx, step, actionName, location, remoteAction == nil, "pre-entrypoint") + return execAsDocker(ctx, step, actionName, actionDir, actionPath, remoteAction == nil, "pre-entrypoint") case model.ActionRunsUsingComposite: if step.getCompositeSteps() == nil { @@ -662,11 +662,11 @@ func runPostStep(step actionStep) common.Executor { return rc.execJobContainer(containerArgs, *step.getEnv(), "", "")(ctx) case model.ActionRunsUsingDocker: - location := actionLocation if remoteAction == nil { - location = containerActionDir + actionDir = "" + actionPath = containerActionDir } - return execAsDocker(ctx, step, actionName, location, remoteAction == nil, "post-entrypoint") + return execAsDocker(ctx, step, actionName, actionDir, actionPath, remoteAction == nil, "post-entrypoint") case model.ActionRunsUsingComposite: if err := maybeCopyToActionDir(ctx, step, actionDir, actionPath, containerActionDir); err != nil { diff --git a/pkg/runner/runner_test.go b/pkg/runner/runner_test.go index 029fca7..cac1b83 100644 --- a/pkg/runner/runner_test.go +++ b/pkg/runner/runner_test.go @@ -271,6 +271,7 @@ func TestRunEvent(t *testing.T) { {workdir, "job-container-invalid-credentials", "push", "failed to handle credentials: failed to interpolate container.credentials.password", platforms, secrets}, {workdir, "container-hostname", "push", "", platforms, secrets}, {workdir, "remote-action-docker", "push", "", platforms, secrets}, + {workdir, "remote-action-docker-new-cache", "push", "", platforms, secrets}, {workdir, "remote-action-js", "push", "", platforms, secrets}, {workdir, "remote-action-js-node-user", "push", "", platforms, secrets}, // Test if this works with non root container {workdir, "matrix", "push", "", platforms, secrets}, diff --git a/pkg/runner/testdata/remote-action-docker-new-cache/config/config.yml b/pkg/runner/testdata/remote-action-docker-new-cache/config/config.yml new file mode 100644 index 0000000..43befe2 --- /dev/null +++ b/pkg/runner/testdata/remote-action-docker-new-cache/config/config.yml @@ -0,0 +1 @@ +local-repositories: {} diff --git a/pkg/runner/testdata/remote-action-docker-new-cache/push.yml b/pkg/runner/testdata/remote-action-docker-new-cache/push.yml new file mode 100644 index 0000000..a1ba05b --- /dev/null +++ b/pkg/runner/testdata/remote-action-docker-new-cache/push.yml @@ -0,0 +1,10 @@ +name: remote-action-docker +on: push + +jobs: + test: + runs-on: ubuntu-latest + steps: + - uses: actions/hello-world-docker-action@v1 + with: + who-to-greet: 'Mona the Octocat'