From ad9eeb668a8d72d595d24cfd8b41b3cba642dffe Mon Sep 17 00:00:00 2001 From: silverwind Date: Sat, 10 Oct 2026 18:11:03 +0000 Subject: [PATCH] fix: job container, daemon startup, action cache and exec fixes (#1294) - Keep `GITHUB_ENV`, `GITHUB_OUTPUT`, `GITHUB_STATE` and `GITHUB_PATH` when a job times out or is cancelled, so post steps still get their state - Retry the startup Declare while Gitea is unreachable instead of exiting, still failing fast on rejected registrations, auth and misconfiguration - Reuse a local image when a platform variant such as `linux/arm/v7` is requested instead of pulling again - Read `PATH` from the created job container so cross-architecture jobs get tool paths for the right architecture - Extract action copies relative to the act volume so they never cross an image's `/var/run` symlink, which rootless Podman rejects - Keep the job container's entrypoint when container options set `--entrypoint`, warning instead of appending it to the keep-alive command - Lock action cache entries across runner processes that share a cache directory - `exec`: omit `GITHUB_SERVER_URL` and `GITHUB_API_URL` without `--gitea-instance` instead of exporting a bare `https://`, and read owner and repository from any origin remote Reviewed-on: https://gitea.com/gitea/runner/pulls/1294 Reviewed-by: bircni Co-authored-by: silverwind --- act/common/git/git.go | 83 +++++++++------ act/common/git/git_test.go | 98 ++++++++++++----- act/container/docker_images.go | 18 +++- act/container/docker_platform_test.go | 23 ++++ act/container/docker_run.go | 146 +++++++++----------------- act/container/docker_run_test.go | 126 ++++++++++++++-------- act/container/testdata/Dockerfile | 5 - act/ghcontext/github_context.go | 6 +- act/runner/action.go | 12 ++- act/runner/action_test.go | 10 +- act/runner/job_executor.go | 7 +- act/runner/reusable_workflow.go | 29 +++-- act/runner/reusable_workflow_test.go | 13 +-- act/runner/run_context.go | 64 +++++------ act/runner/run_context_test.go | 31 ++++++ act/runner/runner.go | 11 +- act/runner/step.go | 16 ++- act/runner/step_action_remote.go | 14 ++- act/runner/step_action_remote_test.go | 7 +- act/runner/step_test.go | 48 ++++++++- internal/app/cmd/daemon.go | 37 ++++++- internal/app/cmd/daemon_test.go | 37 ++++++- scripts/test-dind.sh | 4 +- 23 files changed, 537 insertions(+), 308 deletions(-) delete mode 100644 act/container/testdata/Dockerfile diff --git a/act/common/git/git.go b/act/common/git/git.go index 31793adb..b1cdc244 100644 --- a/act/common/git/git.go +++ b/act/common/git/git.go @@ -10,8 +10,10 @@ import ( "encoding/base64" "errors" "fmt" + "net/url" "os" "os/exec" + "path" "path/filepath" "regexp" "runtime" @@ -29,8 +31,6 @@ import ( var ( codeCommitHTTPRegex = regexp.MustCompile(`^https?://git-codecommit\.(.+)\.amazonaws.com/v1/repos/(.+)$`) codeCommitSSHRegex = regexp.MustCompile(`ssh://git-codecommit\.(.+)\.amazonaws.com/v1/repos/(.+)$`) - githubHTTPRegex = regexp.MustCompile(`^https?://.*github.com.*/(.+)/(.+?)(?:.git)?$`) - githubSSHRegex = regexp.MustCompile(`github.com[:/](.+)/(.+?)(?:.git)?$`) hexRefRegex = regexp.MustCompile(`^[0-9a-fA-F]+$`) LocalEnvVars = []string{ // `git rev-parse --local-env-vars`, inherited ones point git at another repository @@ -44,12 +44,29 @@ var ( ErrShortRef = errors.New("short SHA references are not supported") ) -// AcquireCloneLock returns an unlock function after locking the per-directory mutex for dir. -// Only concurrent operations targeting the same directory are serialized; clones into different directories run in parallel. -// Callers reading files inside dir (e.g. tarring a checked-out action into a job container) must hold this lock too, -// otherwise a concurrent NewGitCloneExecutor on the same dir can mutate the worktree mid-read. -func AcquireCloneLock(dir string) func() { - return cloneLocks.Lock(dir) +// AcquireCloneLock serializes access to dir across runner processes, or within this one where no lock file can be created. Readers of dir must hold it too. +func AcquireCloneLock(ctx context.Context, dir string) (func(), error) { + dir = filepath.Clean(dir) + if runtime.GOOS == "plan9" { + return cloneLocks.Lock(dir), nil + } + if err := os.MkdirAll(filepath.Dir(dir), 0o755); err != nil { + return nil, err + } + for { + release, err := lock.TryLock(dir) + if err == nil { + return func() { _ = release() }, nil + } + if !errors.Is(err, lock.ErrLocked) { + return cloneLocks.Lock(dir), nil + } + select { + case <-ctx.Done(): + return nil, ctx.Err() + case <-time.After(50 * time.Millisecond): + } + } } type Error struct { @@ -109,13 +126,12 @@ func FindGitRef(ctx context.Context, file string) (string, error) { } // FindGithubRepo get the repo -func FindGithubRepo(ctx context.Context, file, githubInstance string) (string, error) { +func FindGithubRepo(ctx context.Context, file string) (string, error) { url, err := findGitRemoteURL(ctx, file) if err != nil { return "", err } - _, slug := findGitSlug(url, githubInstance) - return slug, nil + return findGitSlug(url) } func findGitRemoteURL(ctx context.Context, file string) (string, error) { @@ -130,25 +146,25 @@ func findGitRemoteURL(ctx context.Context, file string) (string, error) { return url, nil } -func findGitSlug(url, githubInstance string) (string, string) { - if matches := codeCommitHTTPRegex.FindStringSubmatch(url); matches != nil { - return "CodeCommit", matches[2] - } else if matches := codeCommitSSHRegex.FindStringSubmatch(url); matches != nil { - return "CodeCommit", matches[2] - } else if matches := githubHTTPRegex.FindStringSubmatch(url); matches != nil { - return "GitHub", fmt.Sprintf("%s/%s", matches[1], matches[2]) - } else if matches := githubSSHRegex.FindStringSubmatch(url); matches != nil { - return "GitHub", fmt.Sprintf("%s/%s", matches[1], matches[2]) - } else if githubInstance != "github.com" { - gheHTTPRegex := regexp.MustCompile(fmt.Sprintf(`^https?://%s/(.+)/(.+?)(?:.git)?$`, githubInstance)) - gheSSHRegex := regexp.MustCompile(githubInstance + "[:/](.+)/(.+?)(?:.git)?$") - if matches := gheHTTPRegex.FindStringSubmatch(url); matches != nil { - return "GitHubEnterprise", fmt.Sprintf("%s/%s", matches[1], matches[2]) - } else if matches := gheSSHRegex.FindStringSubmatch(url); matches != nil { - return "GitHubEnterprise", fmt.Sprintf("%s/%s", matches[1], matches[2]) - } +func findGitSlug(remoteURL string) (string, error) { + if matches := codeCommitHTTPRegex.FindStringSubmatch(remoteURL); matches != nil { + return matches[2], nil + } else if matches := codeCommitSSHRegex.FindStringSubmatch(remoteURL); matches != nil { + return matches[2], nil } - return "", url + if host, repoPath, ok := strings.Cut(remoteURL, ":"); ok && filepath.VolumeName(remoteURL) == "" && !strings.Contains(host, "/") && !strings.HasPrefix(repoPath, "//") { + remoteURL = "ssh://" + host + "/" + repoPath // scp-like [user@]host:path + } + parsed, err := url.Parse(remoteURL) + if err != nil { + return "", errors.New("cannot parse the origin remote URL") + } + owner, repo := path.Split(strings.TrimSuffix(strings.Trim(parsed.Path, "/"), ".git")) + owner = path.Base(owner) + if parsed.Host == "" || repo == "" || owner == "." || owner == "/" { + return "", errors.New("cannot determine owner and repository from the origin remote URL") + } + return owner + "/" + repo, nil } // NewGitCloneExecutorInput the input for the NewGitCloneExecutor @@ -182,6 +198,9 @@ func CloneIfRequired(ctx context.Context, input NewGitCloneExecutorInput, logger if err == nil && origin == input.URL { return true, nil } + if ctxErr := ctx.Err(); ctxErr != nil { // a cancelled origin lookup must not discard a valid clone + return false, ctxErr + } if err == nil { logger.Debugf("Removing cached clone at %s because origin URL changed from %s to %s", input.Dir, origin, input.URL) } else if _, statErr := os.Stat(input.Dir); statErr == nil { @@ -209,7 +228,11 @@ func NewGitCloneExecutor(input NewGitCloneExecutorInput) common.Executor { logger.Infof("git clone '%s' # ref=%s", input.URL, input.Ref) } logger.Debugf(" cloning %s to %s", input.URL, input.Dir) - defer AcquireCloneLock(input.Dir)() + unlock, err := AcquireCloneLock(ctx, input.Dir) + if err != nil { + return err + } + defer unlock() reused, err := CloneIfRequired(ctx, input, logger) if err != nil { diff --git a/act/common/git/git_test.go b/act/common/git/git_test.go index 79ced67a..2c6d4bb4 100644 --- a/act/common/git/git_test.go +++ b/act/common/git/git_test.go @@ -18,9 +18,11 @@ import ( "sync" "syscall" "testing" + "testing/synctest" "time" "gitea.com/gitea/runner/act/common" + "gitea.com/gitea/runner/internal/pkg/lock" log "github.com/sirupsen/logrus" logrustest "github.com/sirupsen/logrus/hooks/test" @@ -29,29 +31,33 @@ import ( ) func TestFindGitSlug(t *testing.T) { - assert := assert.New(t) - - slugTests := []struct { - url string // input - provider string // expected result - slug string // expected result + for _, testcase := range []struct { + remoteURL string + slug string }{ - {"https://git-codecommit.us-east-1.amazonaws.com/v1/repos/my-repo-name", "CodeCommit", "my-repo-name"}, - {"ssh://git-codecommit.us-west-2.amazonaws.com/v1/repos/my-repo", "CodeCommit", "my-repo"}, - {"git@github.com:nektos/act.git", "GitHub", "nektos/act"}, - {"git@github.com:nektos/act", "GitHub", "nektos/act"}, - {"https://github.com/nektos/act.git", "GitHub", "nektos/act"}, - {"http://github.com/nektos/act.git", "GitHub", "nektos/act"}, - {"https://github.com/nektos/act", "GitHub", "nektos/act"}, - {"http://github.com/nektos/act", "GitHub", "nektos/act"}, - {"git+ssh://git@github.com/owner/repo.git", "GitHub", "owner/repo"}, - {"http://myotherrepo.com/act.git", "", "http://myotherrepo.com/act.git"}, - } - - for _, tt := range slugTests { - provider, slug := findGitSlug(tt.url, "github.com") - assert.Equal(tt.provider, provider) - assert.Equal(tt.slug, slug) + {"https://git-codecommit.us-east-1.amazonaws.com/v1/repos/my-repo-name", "my-repo-name"}, + {"ssh://git-codecommit.us-west-2.amazonaws.com/v1/repos/my-repo", "my-repo"}, + {"git@github.com:nektos/act.git", "nektos/act"}, + {"git@github.com:nektos/act", "nektos/act"}, + {"https://github.com/nektos/act.git", "nektos/act"}, + {"http://github.com/nektos/act.git", "nektos/act"}, + {"https://github.com/nektos/act", "nektos/act"}, + {"http://github.com/nektos/act", "nektos/act"}, + {"git+ssh://git@github.com/owner/repo.git", "owner/repo"}, + {"https://example.com:3000/gitea/owner/repo.git/", "owner/repo"}, + {"ssh://git@example.com:2222/gitea/owner/repo.git", "owner/repo"}, + {"example.com:owner/repo.git", "owner/repo"}, + {"http://myotherrepo.com/act.git", ""}, + {"https://user:secret@example.com/act.git", ""}, + {"/tmp/owner/repo.git", ""}, + {"./owner/repo:branch", ""}, + } { + t.Run(testcase.remoteURL, func(t *testing.T) { + slug, err := findGitSlug(testcase.remoteURL) + assert.Equal(t, testcase.slug == "", err != nil) + assert.NotContains(t, fmt.Sprint(err), "secret") + assert.Equal(t, testcase.slug, slug) + }) } } @@ -107,7 +113,7 @@ func TestFindGitMetadataOfSHA256RepositoryUsesOrigin(t *testing.T) { require.NoError(t, err) require.Equal(t, remoteURL, url) - slug, err := FindGithubRepo(context.Background(), basedir, "github.com") + slug, err := FindGithubRepo(context.Background(), basedir) require.NoError(t, err) require.Equal(t, "owner/repo", slug) } @@ -310,6 +316,12 @@ func TestGitCloneExecutorReclonesWhenOriginURLChanges(t *testing.T) { markerPath := filepath.Join(cacheDir, "stale-marker") require.NoError(t, os.WriteFile(markerPath, []byte("stale"), 0o644)) + cancelled, cancel := context.WithCancel(t.Context()) + cancel() + _, err := CloneIfRequired(cancelled, NewGitCloneExecutorInput{URL: oldRemoteDir, Ref: "main", Dir: cacheDir}, log.New()) + require.ErrorIs(t, err, context.Canceled) + require.FileExists(t, markerPath) + require.NoError(t, NewGitCloneExecutor(NewGitCloneExecutorInput{ URL: newRemoteDir, Ref: "main", @@ -631,14 +643,42 @@ func gitCmd(args ...string) error { } func TestAcquireCloneLock(t *testing.T) { + t.Run("blocks on a lock held by another process despite a trailing separator", func(t *testing.T) { + dir := filepath.Join(t.TempDir(), "cache") + release, err := lock.TryLock(dir) + require.NoError(t, err) + defer func() { require.NoError(t, release()) }() + synctest.Test(t, func(t *testing.T) { + ctx, cancel := context.WithTimeout(t.Context(), time.Second) + defer cancel() + _, err := AcquireCloneLock(ctx, dir+string(filepath.Separator)) + require.ErrorIs(t, err, context.DeadlineExceeded) + }) + }) + + t.Run("read-only parent falls back to an in-process lock", func(t *testing.T) { + parent := t.TempDir() + dir := filepath.Join(parent, "cache") + require.NoError(t, os.Mkdir(dir, 0o755)) + require.NoError(t, os.Chmod(parent, 0o555)) + defer func() { require.NoError(t, os.Chmod(parent, 0o755)) }() + unlock, err := AcquireCloneLock(t.Context(), dir) + require.NoError(t, err) + unlock() + }) + t.Run("same directory serializes", func(t *testing.T) { dir := t.TempDir() - unlock1 := AcquireCloneLock(dir) + unlock1, err := AcquireCloneLock(t.Context(), dir) + require.NoError(t, err) secondAcquired := make(chan struct{}) go func() { - unlock := AcquireCloneLock(dir) + unlock, err := AcquireCloneLock(t.Context(), dir) + if !assert.NoError(t, err) { + return + } close(secondAcquired) unlock() }() @@ -662,12 +702,16 @@ func TestAcquireCloneLock(t *testing.T) { dirA := t.TempDir() dirB := t.TempDir() - unlockA := AcquireCloneLock(dirA) + unlockA, err := AcquireCloneLock(t.Context(), dirA) + require.NoError(t, err) defer unlockA() done := make(chan struct{}) go func() { - unlock := AcquireCloneLock(dirB) + unlock, err := AcquireCloneLock(t.Context(), dirB) + if !assert.NoError(t, err) { + return + } unlock() close(done) }() diff --git a/act/container/docker_images.go b/act/container/docker_images.go index 69618e1d..1437bced 100644 --- a/act/container/docker_images.go +++ b/act/container/docker_images.go @@ -8,10 +8,10 @@ package container import ( "context" - "fmt" cerrdefs "github.com/containerd/errdefs" "github.com/moby/moby/client" + specs "github.com/opencontainers/image-spec/specs-go/v1" ) // ImageExistsLocally returns a boolean indicating if an image with the @@ -30,11 +30,23 @@ func ImageExistsLocally(ctx context.Context, imageName, platform string) (bool, return false, err } - if platform == "" || platform == "any" || fmt.Sprintf("%s/%s", inspectImage.Os, inspectImage.Architecture) == platform { + if platform == "" || platform == "any" { return true, nil } - return false, nil + requested, err := parsePlatform(platform) + if err != nil { + return false, err + } + return platformMatches(requested, &specs.Platform{OS: inspectImage.Os, Architecture: inspectImage.Architecture, Variant: inspectImage.Variant}), nil +} + +func platformMatches(requested, image *specs.Platform) bool { + variant := image.Variant + if variant == "" { + variant = map[string]string{"amd64": "v1", "arm": "v7", "arm64": "v8"}[image.Architecture] + } + return image.OS == requested.OS && image.Architecture == requested.Architecture && (requested.Variant == "" || requested.Variant == variant) } // RemoveImage removes image from local store, the function is used to run different diff --git a/act/container/docker_platform_test.go b/act/container/docker_platform_test.go index 46005a5c..e75eb310 100644 --- a/act/container/docker_platform_test.go +++ b/act/container/docker_platform_test.go @@ -61,3 +61,26 @@ func TestParsePlatform(t *testing.T) { }) } } + +func TestPlatformMatches(t *testing.T) { + for _, testcase := range []struct { + requested, image string + want bool + }{ + {"linux/amd64", "linux/amd64", true}, + {"linux/amd64/v1", "linux/amd64", true}, + {"linux/arm64", "linux/amd64", false}, + {"linux/arm64/v8", "linux/arm64", true}, + {"linux/arm/v7", "linux/arm/v7", true}, + {"linux/arm", "linux/arm/v7", true}, + {"linux/arm/v6", "linux/arm/v7", false}, + {"linux/arm", "linux/arm/v6", true}, + {"windows/arm/v7", "linux/arm/v7", false}, + } { + requested, err := parsePlatform(testcase.requested) + require.NoError(t, err) + image, err := parsePlatform(testcase.image) + require.NoError(t, err) + assert.Equal(t, testcase.want, platformMatches(requested, image), "%s on %s", testcase.requested, testcase.image) + } +} diff --git a/act/container/docker_run.go b/act/container/docker_run.go index 88fe07e4..cba68c9c 100644 --- a/act/container/docker_run.go +++ b/act/container/docker_run.go @@ -31,7 +31,7 @@ import ( cerrdefs "github.com/containerd/errdefs" "github.com/docker/cli/cli/compose/loader" "github.com/docker/cli/cli/connhelper" - "github.com/joho/godotenv" + "github.com/docker/cli/opts" "github.com/moby/moby/api/pkg/stdcopy" "github.com/moby/moby/api/types/container" "github.com/moby/moby/api/types/mount" @@ -95,8 +95,8 @@ func (cr *containerReference) Start(attach bool) common.Executor { cr.attach().IfBool(attach), cr.start(), cr.wait().IfBool(attach), - cr.tryReadUID(), - cr.tryReadGID(), + cr.tryReadID("-u", func(id int) { cr.UID = id }), + cr.tryReadID("-g", func(id int) { cr.GID = id }), func(ctx context.Context) error { // If this fails, then folders have wrong permissions on non root container if cr.UID != 0 || cr.GID != 0 { @@ -305,7 +305,25 @@ func (cr *containerReference) UpdateFromEnv(srcPath string, env *map[string]stri } func (cr *containerReference) UpdateFromImageEnv(env *map[string]string) common.Executor { - return cr.extractFromImageEnv(env).IfNot(common.Dryrun) + envMap := *env + return common.Executor(func(ctx context.Context) error { + inspect, err := cr.cli.ContainerInspect(ctx, cr.id, client.ContainerInspectOptions{}) + if err != nil { + common.Logger(ctx).Error(err) + return fmt.Errorf("inspect container: %w", err) + } + if inspect.Container.Config == nil { + return nil + } + for key, value := range opts.ConvertKVStringsToMap(inspect.Container.Config.Env) { + if envMap[key] == "" { + envMap[key] = value + } else if key == "PATH" { + envMap[key] += ":" + value + } + } + return nil + }).IfNot(common.Dryrun) } func (cr *containerReference) Exec(command []string, env map[string]string, user, workdir string) common.Executor { @@ -567,20 +585,6 @@ func (cr *containerReference) mergeContainerConfigs(ctx context.Context, config return nil, nil, fmt.Errorf("cannot process container options: '%s': '%w'", options, err) } - // FIXME: If everything is fine after gitea/act v0.260.0, remove the following comment. - // In the old fork version, the code is - // if len(copts.netMode.Value()) == 0 { - // if err = copts.netMode.Set("host"); err != nil { - // return nil, nil, fmt.Errorf("cannot parse networkmode=host. This is an internal error and should not happen: '%w'", err) - // } - // } - // And it has been commented with: - // If a service container's network is set to `host`, the container will not be able to - // connect to the specified network created for the job container and the service containers. - // So comment out the following code. - // Not the if it's necessary to comment it in the new version, - // since it's cr.input.NetworkMode now. - if len(copts.netMode.Value()) == 0 { if err = copts.netMode.Set(cr.input.NetworkMode); err != nil { return nil, nil, fmt.Errorf("cannot parse networkmode=%s. This is an internal error and should not happen: '%w'", cr.input.NetworkMode, err) @@ -644,6 +648,9 @@ func (cr *containerReference) mergeContainerConfigs(ctx context.Context, config if flags.Changed("network") || flags.Changed("net") { logger.Warn("--network and --net in the options will be ignored.") } + if len(cr.input.Entrypoint) != 0 && flags.Changed("entrypoint") { + logger.Warn("--entrypoint in the options will be ignored.") + } hostConfig.NetworkMode = networkMode logger.Debugf("Merged container.HostConfig ==> %+v", hostConfig) @@ -673,17 +680,11 @@ func (cr *containerReference) create(capAdd, capDrop []string) common.Executor { ExposedPorts: exposedPorts, Tty: input.AllocatePTY, } - // For Gitea, reduce log noise - // logger.Debugf("Common container.Config ==> %+v", config) if len(input.Cmd) != 0 { config.Cmd = input.Cmd } - if len(input.Entrypoint) != 0 { - config.Entrypoint = input.Entrypoint - } - mounts := make([]mount.Mount, 0) for mountSource, mountTarget := range input.Mounts { mounts = append(mounts, mount.Mount{ @@ -718,20 +719,20 @@ func (cr *containerReference) create(capAdd, capDrop []string) common.Executor { PortBindings: portBindings, AutoRemove: input.AutoRemove, } - // For Gitea, reduce log noise - // logger.Debugf("Common container.HostConfig ==> %+v", hostConfig) config, hostConfig, err = cr.mergeContainerConfigs(ctx, config, hostConfig) if err != nil { return err } + if len(input.Entrypoint) != 0 { + config.Entrypoint = input.Entrypoint + } + // For Gitea config, hostConfig = cr.sanitizeConfig(ctx, config, hostConfig) var networkingConfig *network.NetworkingConfig - // For Gitea, reduce log noise - // logger.Debugf("input.NetworkAliases ==> %v", input.NetworkAliases) n := hostConfig.NetworkMode // IsUserDefined and IsHost are broken on windows if n.IsUserDefined() && n != "host" && len(input.NetworkAliases) > 0 { @@ -764,44 +765,6 @@ func (cr *containerReference) create(capAdd, capDrop []string) common.Executor { } } -func (cr *containerReference) extractFromImageEnv(env *map[string]string) common.Executor { - envMap := *env - return func(ctx context.Context) error { - logger := common.Logger(ctx) - - inspect, err := cr.cli.ImageInspect(ctx, cr.input.Image) - if err != nil { - logger.Error(err) - return fmt.Errorf("inspect image: %w", err) - } - - if inspect.Config == nil { - return nil - } - - imageEnv, err := godotenv.Unmarshal(strings.Join(inspect.Config.Env, "\n")) - if err != nil { - logger.Error(err) - return fmt.Errorf("unmarshal image env: %w", err) - } - - for k, v := range imageEnv { - if k == "PATH" { - if envMap[k] == "" { - envMap[k] = v - } else { - envMap[k] += `:` + v - } - } else if envMap[k] == "" { - envMap[k] = v - } - } - - env = &envMap - return nil - } -} - func (cr *containerReference) exec(cmd []string, env map[string]string, user, workdir string) common.Executor { return func(ctx context.Context) error { if cr.id == "" { @@ -819,11 +782,6 @@ func (cr *containerReference) exec(cmd []string, env map[string]string, user, wo logger.Debugf("Exec command '%s'", cmd) isTerminal := cr.input.AllocatePTY - envList := make([]string, 0) - for k, v := range env { - envList = append(envList, fmt.Sprintf("%s=%s", k, v)) - } - var wd string if workdir != "" { if strings.HasPrefix(workdir, "/") { @@ -840,7 +798,7 @@ func (cr *containerReference) exec(cmd []string, env map[string]string, user, wo User: user, Cmd: cmd, WorkingDir: wd, - Env: envList, + Env: getEnvListFromMap(env), TTY: isTerminal, AttachStderr: true, AttachStdout: true, @@ -857,7 +815,7 @@ func (cr *containerReference) exec(cmd []string, env map[string]string, user, wo } defer resp.Close() - err = cr.waitForCommand(ctx, resp.HijackedResponse, idResp, user, workdir) + err = cr.waitForCommand(ctx, resp.HijackedResponse) if err != nil { return err } @@ -907,15 +865,7 @@ func (cr *containerReference) tryReadID(opt string, cbk func(id int)) common.Exe } } -func (cr *containerReference) tryReadUID() common.Executor { - return cr.tryReadID("-u", func(id int) { cr.UID = id }) -} - -func (cr *containerReference) tryReadGID() common.Executor { - return cr.tryReadID("-g", func(id int) { cr.GID = id }) -} - -func (cr *containerReference) waitForCommand(ctx context.Context, resp client.HijackedResponse, _ client.ExecCreateResult, _, _ string) error { +func (cr *containerReference) waitForCommand(ctx context.Context, resp client.HijackedResponse) error { logger := common.Logger(ctx) // Buffered so the copy goroutine never blocks on send if the grace-period @@ -957,6 +907,7 @@ func (cr *containerReference) waitForCommand(ctx context.Context, resp client.Hi } func (cr *containerReference) copyDir(dstPath, srcPath string, useGitIgnore, skipGitDir bool) common.Executor { + dstPath = path.Clean(dstPath) return func(ctx context.Context) error { if cr.id == "" { return cr.missingContainerError("copy directory to %s", dstPath) @@ -978,7 +929,11 @@ func (cr *containerReference) copyDir(dstPath, srcPath string, useGitIgnore, ski logger.Error(err) } }(tarFile) - if err := writeDirTar(ctx, tarFile, dstPath, srcPath, useGitIgnore, skipGitDir, cr.UID, cr.GID); err != nil { + extractionRoot := "/" + if actPath := cr.GetActPath(); strings.HasPrefix(dstPath+"/", actPath+"/") { + extractionRoot = actPath // keeps tar entries from crossing the image's /var/run symlink + } + if err := writeDirTar(ctx, tarFile, path.Join("/", strings.TrimPrefix(dstPath, extractionRoot)), srcPath, useGitIgnore, skipGitDir, cr.UID, cr.GID); err != nil { return err } @@ -987,14 +942,7 @@ func (cr *containerReference) copyDir(dstPath, srcPath string, useGitIgnore, ski if err != nil { return fmt.Errorf("failed to seek tar archive: %w", err) } - _, err = cr.cli.CopyToContainer(ctx, cr.id, client.CopyToContainerOptions{ - DestinationPath: "/", - Content: tarFile, - }) - if err != nil { - return fmt.Errorf("failed to copy content to container: %w", err) - } - return nil + return cr.upload(ctx, extractionRoot, tarFile) } } @@ -1009,17 +957,17 @@ func (cr *containerReference) copyContent(dstPath string, files ...*FileEntry) c } common.Logger(ctx).Debugf("Extracting content to '%s'", dstPath) - _, err := cr.cli.CopyToContainer(ctx, cr.id, client.CopyToContainerOptions{ - DestinationPath: dstPath, - Content: &buf, - }) - if err != nil { - return fmt.Errorf("failed to copy content to container: %w", err) - } - return nil + return cr.upload(ctx, dstPath, &buf) } } +func (cr *containerReference) upload(ctx context.Context, dstPath string, content io.Reader) error { + if _, err := cr.cli.CopyToContainer(ctx, cr.id, client.CopyToContainerOptions{DestinationPath: dstPath, Content: content}); err != nil { + return fmt.Errorf("failed to copy content to container: %w", err) + } + return nil +} + func (cr *containerReference) attach() common.Executor { return func(ctx context.Context) error { out, err := cr.cli.ContainerAttach(ctx, cr.id, client.ContainerAttachOptions{ diff --git a/act/container/docker_run_test.go b/act/container/docker_run_test.go index be684ad7..61a63eeb 100644 --- a/act/container/docker_run_test.go +++ b/act/container/docker_run_test.go @@ -5,6 +5,7 @@ package container import ( + "archive/tar" "bufio" "bytes" "context" @@ -31,44 +32,28 @@ import ( "github.com/stretchr/testify/require" ) -func TestDocker(t *testing.T) { - requireDocker(t) - ctx := context.Background() - client, err := GetDockerClient(ctx) - require.NoError(t, err) - defer client.Close() - - dockerBuild := NewDockerBuildExecutor(NewDockerBuildExecutorInput{ - ContextDir: "testdata", - ImageTag: "envmergetest", - }) - - err = dockerBuild(ctx) - assert.NoError(t, err) //nolint:testifylint // pre-existing issue from nektos/act - - cr := &containerReference{ - cli: client, - input: &NewContainerInput{ - Image: "envmergetest", - }, - } +func TestDockerEnvironmentUsesContainerPath(t *testing.T) { + client := &mockDockerClient{} + client.On("ContainerInspect", mock.Anything, "amd64-job", mobyclient.ContainerInspectOptions{}). + Return(mobyclient.ContainerInspectResult{Container: container.InspectResponse{Config: &container.Config{Env: []string{ + "PATH=/opt/node/x64/bin:/usr/bin:/bin", "CONFLICT_VAR=container-value", "SOME_RANDOM_VAR=", + "ANOTHER_ONE=old-value", "ANOTHER_ONE=BUT_I_HAVE_VALUE", "LITERAL='quoted'=$HOME\nsecond line", + }}}}, nil) env := map[string]string{ - "PATH": "/usr/local/bin:/usr/bin:/usr/sbin:/bin:/sbin", + "PATH": "/custom/bin", "RANDOM_VAR": "WITH_VALUE", "ANOTHER_VAR": "", - "CONFLICT_VAR": "I_EXIST_IN_MULTIPLE_PLACES", + "CONFLICT_VAR": "step-value", } - - envExecutor := cr.extractFromImageEnv(&env) - err = envExecutor(ctx) - assert.NoError(t, err) //nolint:testifylint // pre-existing issue from nektos/act + require.NoError(t, (&containerReference{cli: client, id: "amd64-job"}).UpdateFromImageEnv(&env)(t.Context())) assert.Equal(t, map[string]string{ - "PATH": "/usr/local/bin:/usr/bin:/usr/sbin:/bin:/sbin:/this/path/does/not/exists/anywhere:/this/either", + "PATH": "/custom/bin:/opt/node/x64/bin:/usr/bin:/bin", "RANDOM_VAR": "WITH_VALUE", "ANOTHER_VAR": "", "SOME_RANDOM_VAR": "", "ANOTHER_ONE": "BUT_I_HAVE_VALUE", - "CONFLICT_VAR": "I_EXIST_IN_MULTIPLE_PLACES", + "CONFLICT_VAR": "step-value", + "LITERAL": "'quoted'=$HOME\nsecond line", }, env) } @@ -498,25 +483,45 @@ func TestRejectsMissingContainer(t *testing.T) { check("Inspect after removal", err) } -// End-to-end: a stale cr.id is cleared, repopulated from name lookup, -// and the Copy completes against the fresh id. func TestPublicCopyPipelineHandlesStaleID(t *testing.T) { - ctx := context.Background() - client := &mockDockerClient{} - client.On("ContainerInspect", ctx, "stale", mobyclient.ContainerInspectOptions{}). - Return(mobyclient.ContainerInspectResult{}, cerrdefs.ErrNotFound.WithMessage("gone")) - client.On("ContainerList", ctx, mobyclient.ContainerListOptions{All: true}). - Return(mobyclient.ContainerListResult{Items: []container.Summary{ - {ID: "fresh", Names: []string{"/job-1"}}, - }}, nil) - client.On("CopyToContainer", ctx, "fresh", mock.MatchedBy(func(opts mobyclient.CopyToContainerOptions) bool { - return opts.DestinationPath == "/var/run/act" - })).Return(mobyclient.CopyToContainerResult{}, nil) + for _, testcase := range []struct { + name, destination, extractionRoot, entry string + directory bool + }{ + {name: "files", destination: "/var/run/act", extractionRoot: "/var/run/act", entry: "x"}, + {name: "action below act volume", directory: true, destination: "/var/run/act/actions/checkout/", extractionRoot: "/var/run/act", entry: "actions/checkout/x"}, + {name: "sibling prefix", directory: true, destination: "/var/run/actor/action", extractionRoot: "/", entry: "var/run/actor/action/x"}, + } { + t.Run(testcase.name, func(t *testing.T) { + ctx := t.Context() + client := &mockDockerClient{} + client.On("ContainerInspect", ctx, "stale", mobyclient.ContainerInspectOptions{}). + Return(mobyclient.ContainerInspectResult{}, cerrdefs.ErrNotFound.WithMessage("gone")) + client.On("ContainerList", ctx, mobyclient.ContainerListOptions{All: true}). + Return(mobyclient.ContainerListResult{Items: []container.Summary{ + {ID: "fresh", Names: []string{"/job-1"}}, + }}, nil) + client.On("CopyToContainer", ctx, "fresh", mock.Anything). + Run(func(args mock.Arguments) { + opts := args.Get(2).(mobyclient.CopyToContainerOptions) + assert.Equal(t, testcase.extractionRoot, opts.DestinationPath) + header, err := tar.NewReader(opts.Content).Next() + require.NoError(t, err) + assert.Equal(t, testcase.entry, header.Name) + }).Return(mobyclient.CopyToContainerResult{}, nil).Once() - cr := &containerReference{id: "stale", cli: client, input: &NewContainerInput{Name: "job-1"}} - require.NoError(t, cr.Copy("/var/run/act", &FileEntry{Name: "x", Mode: 0o644})(ctx)) - assert.Equal(t, "fresh", cr.id) - client.AssertExpectations(t) + cr := &containerReference{id: "stale", cli: client, input: &NewContainerInput{Name: "job-1"}} + if testcase.directory { + source := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(source, "x"), nil, 0o644)) + require.NoError(t, cr.CopyDir(testcase.destination, source+string(filepath.Separator), false, true)(ctx)) + } else { + require.NoError(t, cr.Copy(testcase.destination, &FileEntry{Name: "x", Mode: 0o644})(ctx)) + } + assert.Equal(t, "fresh", cr.id) + client.AssertExpectations(t) + }) + } } // Type assert containerReference implements ExecutionsEnvironment @@ -923,6 +928,35 @@ func TestMergeContainerConfigsWarnsOnlyAboutOptionsThatWereGiven(t *testing.T) { assert.Equal(t, 1, warnings("", "--privileged")) } +func TestDockerCreateKeepsInputEntrypoint(t *testing.T) { + for _, entrypoint := range [][]string{nil, {"sleep", "3600"}} { + for _, optionEntrypoint := range []string{"/sbin/dumb-init", ""} { + logger, hook := test.NewNullLogger() + cr := &containerReference{ + input: &NewContainerInput{ + NetworkMode: "bridge", + Entrypoint: entrypoint, + RunnerOptions: "--entrypoint=runner-entrypoint", + WorkflowOptions: "--entrypoint=" + optionEntrypoint + " --init", + }, + cli: &probeClient{create: func(opts mobyclient.ContainerCreateOptions) (mobyclient.ContainerCreateResult, error) { + if len(entrypoint) > 0 { + assert.Equal(t, entrypoint, opts.Config.Entrypoint) + } else { + assert.Equal(t, []string{optionEntrypoint}, opts.Config.Entrypoint) + } + require.NotNil(t, opts.HostConfig.Init) + assert.True(t, *opts.HostConfig.Init) + return mobyclient.ContainerCreateResult{ID: "created"}, nil + }}, + } + require.NoError(t, cr.create(nil, nil)(common.WithLogger(t.Context(), logger))) + assert.Equal(t, "created", cr.id) + assert.Equal(t, len(entrypoint) > 0, len(hook.AllEntries()) == 1) + } + } +} + func TestMergeContainerConfigsKeepsNetworkAliasesFromOptions(t *testing.T) { logger, _ := test.NewNullLogger() cr := &containerReference{input: &NewContainerInput{ diff --git a/act/container/testdata/Dockerfile b/act/container/testdata/Dockerfile deleted file mode 100644 index 3653a630..00000000 --- a/act/container/testdata/Dockerfile +++ /dev/null @@ -1,5 +0,0 @@ -FROM scratch -ENV PATH="/this/path/does/not/exists/anywhere:/this/either" -ENV SOME_RANDOM_VAR="" -ENV ANOTHER_ONE="BUT_I_HAVE_VALUE" -ENV CONFLICT_VAR="I_EXIST_ONLY_HERE" diff --git a/act/ghcontext/github_context.go b/act/ghcontext/github_context.go index cb52220c..d78cf0ed 100644 --- a/act/ghcontext/github_context.go +++ b/act/ghcontext/github_context.go @@ -130,11 +130,11 @@ func SetSha(ctx context.Context, ghc *model.GithubContext, repoPath string) { // SetRepositoryAndOwner resolves the repository of the context from the git // remote in repoPath when it is not set yet, and derives its owner. -func SetRepositoryAndOwner(ctx context.Context, ghc *model.GithubContext, githubInstance, repoPath string) { +func SetRepositoryAndOwner(ctx context.Context, ghc *model.GithubContext, repoPath string) { if ghc.Repository == "" { - repo, err := findGithubRepo(ctx, repoPath, githubInstance) + repo, err := findGithubRepo(ctx, repoPath) if err != nil { - common.Logger(ctx).Warningf("unable to get git repo (githubInstance: %v, repoPath: %v): %v", githubInstance, repoPath, err) + common.Logger(ctx).Warningf("unable to get git repo (repoPath: %v): %v", repoPath, err) return } ghc.Repository = repo diff --git a/act/runner/action.go b/act/runner/action.go index d99b0e2b..e4616444 100644 --- a/act/runner/action.go +++ b/act/runner/action.go @@ -145,7 +145,11 @@ func maybeCopyToActionDir(ctx context.Context, step actionStep, actionDir, actio containerActionDirCopy += `/` } - defer git.AcquireCloneLock(actionDir)() + unlock, err := git.AcquireCloneLock(ctx, actionDir) + if err != nil { + return err + } + defer unlock() if !rc.Config.NoActionPatch { // A concurrent job's prepare resets this directory, so patch under the copy's lock. @@ -366,7 +370,11 @@ func execAsDocker(ctx context.Context, step actionStep, actionName, actionDir, b // Held across the whole build: the daemon drains contextDir lazily. inner := prepImage prepImage = func(ctx context.Context) error { - defer git.AcquireCloneLock(actionDir)() + unlock, err := git.AcquireCloneLock(ctx, actionDir) + if err != nil { + return err + } + defer unlock() return inner(ctx) } } diff --git a/act/runner/action_test.go b/act/runner/action_test.go index 96951fbd..d390edd8 100644 --- a/act/runner/action_test.go +++ b/act/runner/action_test.go @@ -143,6 +143,7 @@ runs: } func TestActionRunner(t *testing.T) { + t.Chdir(t.TempDir()) table := []struct { name string step actionStep @@ -366,7 +367,10 @@ func TestMaybeCopyToActionDirHoldsCloneLock(t *testing.T) { peerAcquired := make(chan struct{}) go func() { - unlock := git.AcquireCloneLock(actionDir) + unlock, err := git.AcquireCloneLock(t.Context(), actionDir) + if !assert.NoError(t, err) { + return + } close(peerAcquired) unlock() }() @@ -400,7 +404,9 @@ func TestMaybeCopyToActionDirHoldsCloneLock(t *testing.T) { func TestExecAsDockerHoldsCloneLockForRemoteUncached(t *testing.T) { actionDir := t.TempDir() - unlockOnce := sync.OnceFunc(git.AcquireCloneLock(actionDir)) + unlock, err := git.AcquireCloneLock(t.Context(), actionDir) + require.NoError(t, err) + unlockOnce := sync.OnceFunc(unlock) defer unlockOnce() innerEntered := make(chan struct{}) diff --git a/act/runner/job_executor.go b/act/runner/job_executor.go index b878e272..cf83f113 100644 --- a/act/runner/job_executor.go +++ b/act/runner/job_executor.go @@ -29,7 +29,10 @@ import ( "gitea.dev/actionslib/pkg/model" ) -const maxJobSummaryBytes = 1024 * 1024 +const ( + jobCleanupTimeout = time.Minute + maxJobSummaryBytes = 1024 * 1024 +) // jobSummaryTruncationMarker is appended to a summary that exceeded the size limit // so the rendered output makes the truncation visible instead of silently cutting off. @@ -249,7 +252,7 @@ func newJobExecutor(info jobInfo, sf stepFactory, rc *RunContext) common.Executo jobError := common.JobError(ctx) var err error // always allow 1 min for stopping and removing the runner, even if we were cancelled - ctx, cancel := context.WithTimeout(common.WithLogger(context.Background(), common.Logger(ctx)), time.Minute) + ctx, cancel := context.WithTimeout(common.WithLogger(context.Background(), common.Logger(ctx)), jobCleanupTimeout) defer cancel() logger := common.Logger(ctx) diff --git a/act/runner/reusable_workflow.go b/act/runner/reusable_workflow.go index a5f9db4c..591a5b70 100644 --- a/act/runner/reusable_workflow.go +++ b/act/runner/reusable_workflow.go @@ -10,7 +10,6 @@ import ( "net/url" "path" "regexp" - "strings" "gitea.com/gitea/runner/act/common" "gitea.com/gitea/runner/act/common/git" @@ -27,7 +26,7 @@ func newLocalReusableWorkflowExecutor(rc *RunContext) common.Executor { if !rc.Config.NoSkipCheckout { // resolve the local workflow against the workspace root, not the process // working directory, so it is found regardless of where the runner is invoked - return newReusableWorkflowExecutor(rc, rc.Config.Workdir, localWorkflow.Path) + return newReusableWorkflowExecutor(rc, rc.Config.Workdir, localWorkflow.Path, false) } uses := fmt.Sprintf("%s/%s@%s", rc.Config.PresetGitHubContext.Repository, localWorkflow.Path, rc.Config.PresetGitHubContext.Sha) @@ -41,7 +40,7 @@ func newLocalReusableWorkflowExecutor(rc *RunContext) common.Executor { return common.NewPipelineExecutor( cloneRemoteReusableWorkflow(rc, remoteReusableWorkflow.CloneURL(), remoteReusableWorkflow.Ref, workflowDir), - newReusableWorkflowExecutor(rc, workflowDir, remoteReusableWorkflow.Path), + newReusableWorkflowExecutor(rc, workflowDir, remoteReusableWorkflow.Path, true), ) } @@ -59,7 +58,7 @@ func newRemoteReusableWorkflowExecutor(rc *RunContext) common.Executor { return common.NewPipelineExecutor( cloneRemoteReusableWorkflow(rc, remoteReusableWorkflow.CloneURL(), remoteReusableWorkflow.Ref, workflowDir), - newReusableWorkflowExecutor(rc, workflowDir, remoteReusableWorkflow.Path), + newReusableWorkflowExecutor(rc, workflowDir, remoteReusableWorkflow.Path, true), ) } @@ -87,11 +86,16 @@ func cloneRemoteReusableWorkflow(rc *RunContext, cloneURL, ref, targetDirectory } } -func newReusableWorkflowExecutor(rc *RunContext, directory, workflow string) common.Executor { +func newReusableWorkflowExecutor(rc *RunContext, directory, workflow string, cached bool) common.Executor { return func(ctx context.Context) error { - // Serialize workflow reads with cache updates. planner, err := func() (model.WorkflowPlanner, error) { - defer git.AcquireCloneLock(directory)() + if cached { + unlock, err := git.AcquireCloneLock(ctx, directory) + if err != nil { + return nil, err + } + defer unlock() + } return model.NewWorkflowPlanner(path.Join(directory, workflow), true) }() if err != nil { @@ -135,11 +139,7 @@ type remoteReusableWorkflow struct { } func (r *remoteReusableWorkflow) CloneURL() string { - // In Gitea, r.URL always has the protocol prefix, we don't need to add extra prefix in this case. - if strings.HasPrefix(r.URL, "http://") || strings.HasPrefix(r.URL, "https://") { - return fmt.Sprintf("%s/%s/%s", r.URL, r.Owner, r.Repo) - } - return fmt.Sprintf("https://%s/%s/%s", r.URL, r.Owner, r.Repo) + return fmt.Sprintf("%s/%s/%s", withHTTPS(r.URL), r.Owner, r.Repo) } var absoluteReusableWorkflowURLRegex = regexp.MustCompile(`^(https?://.*)/([^/]+/[^/]+/\.[^/]+/workflows/[^@]+@.*)$`) @@ -233,10 +233,7 @@ func onInstanceHost(host string, instanceURLs ...string) bool { if candidate == "" { continue } - if !strings.HasPrefix(candidate, "http://") && !strings.HasPrefix(candidate, "https://") { - candidate = "https://" + candidate - } - if u, err := url.Parse(candidate); err == nil && u.Host == host { + if u, err := url.Parse(withHTTPS(candidate)); err == nil && u.Host == host { return true } } diff --git a/act/runner/reusable_workflow_test.go b/act/runner/reusable_workflow_test.go index ec276308..58f6b09a 100644 --- a/act/runner/reusable_workflow_test.go +++ b/act/runner/reusable_workflow_test.go @@ -8,7 +8,6 @@ import ( "crypto/tls" "crypto/x509" "encoding/pem" - "io/fs" "net/http" "net/http/httptest" "os" @@ -84,14 +83,16 @@ func TestNewReusableWorkflowExecutorHoldsCloneLock(t *testing.T) { workflowDir := t.TempDir() require.NoError(t, os.WriteFile(filepath.Join(workflowDir, "reusable.yml"), []byte(":"), 0o644)) - unlockOnce := sync.OnceFunc(git.AcquireCloneLock(workflowDir)) + unlock, err := git.AcquireCloneLock(t.Context(), workflowDir) + require.NoError(t, err) + unlockOnce := sync.OnceFunc(unlock) defer unlockOnce() rc := &RunContext{ Config: &Config{}, Run: &model.Run{Workflow: &model.Workflow{Jobs: map[string]*model.Job{}}}, } - exec := newReusableWorkflowExecutor(rc, workflowDir, "reusable.yml") + exec := newReusableWorkflowExecutor(rc, workflowDir, "reusable.yml", true) done := make(chan error, 1) go func() { done <- exec(context.Background()) }() @@ -113,8 +114,9 @@ func TestNewReusableWorkflowExecutorHoldsCloneLock(t *testing.T) { } func TestNewLocalReusableWorkflowExecutorFindsSameRepositoryPaths(t *testing.T) { - workdir := t.TempDir() + workdir := filepath.Join(t.TempDir(), "workspace") require.NoError(t, os.MkdirAll(filepath.Join(workdir, ".gitea", "workflows"), 0o755)) + require.NoError(t, os.Mkdir(workdir+".lock", 0o755)) require.NoError(t, os.WriteFile(filepath.Join(workdir, ".gitea", "workflows", "reusable.yml"), []byte(":"), 0o644)) for _, uses := range []string{"./.gitea/workflows/reusable.yml", "$/.gitea/workflows/reusable.yml"} { @@ -123,8 +125,7 @@ func TestNewLocalReusableWorkflowExecutorFindsSameRepositoryPaths(t *testing.T) Run: &model.Run{JobID: "job", Workflow: &model.Workflow{Jobs: map[string]*model.Job{"job": {Uses: uses}}}}, } err := newLocalReusableWorkflowExecutor(rc)(t.Context()) - require.Error(t, err) - require.NotErrorIs(t, err, fs.ErrNotExist) + require.ErrorContains(t, err, "workflow is not valid") } } diff --git a/act/runner/run_context.go b/act/runner/run_context.go index f64b2191..40c0ae97 100644 --- a/act/runner/run_context.go +++ b/act/runner/run_context.go @@ -1541,14 +1541,8 @@ func (rc *RunContext) getGithubContext(ctx context.Context) *model.GithubContext merged.RunnerPerflog, merged.RunnerTrackingID = ghc.RunnerPerflog, ghc.RunnerTrackingID *ghc = merged - instance := rc.Config.GitHubInstance - if !strings.HasPrefix(instance, "http://") && - !strings.HasPrefix(instance, "https://") { - instance = "https://" + instance - } - ghc.ServerURL = instance - ghc.APIURL = instance + "/api/v1" // the version of Gitea is v1 - ghc.GraphQLURL = "" // Gitea doesn't support graphql + rc.setInstanceURLs(ghc) + ghc.GraphQLURL = "" // Gitea doesn't support graphql return ghc } } @@ -1562,7 +1556,7 @@ func (rc *RunContext) getGithubContext(ctx context.Context) *model.GithubContext ghc.SetBaseAndHeadRef() repoPath := rc.Config.Workdir - ghcontext.SetRepositoryAndOwner(ctx, ghc, rc.Config.GitHubInstance, repoPath) + ghcontext.SetRepositoryAndOwner(ctx, ghc, repoPath) if ghc.Ref == "" { ghcontext.SetRef(ctx, ghc, repoPath) } @@ -1572,27 +1566,7 @@ func (rc *RunContext) getGithubContext(ctx context.Context) *model.GithubContext ghc.SetRefTypeAndName() - // defaults - ghc.ServerURL = "https://github.com" - ghc.APIURL = "https://api.github.com" - ghc.GraphQLURL = "https://api.github.com/graphql" - // per GHES - if rc.Config.GitHubInstance != "github.com" { - ghc.ServerURL = "https://" + rc.Config.GitHubInstance - ghc.APIURL = fmt.Sprintf("https://%s/api/v3", rc.Config.GitHubInstance) - ghc.GraphQLURL = fmt.Sprintf("https://%s/api/graphql", rc.Config.GitHubInstance) - } - - { // Adapt to Gitea - instance := rc.Config.GitHubInstance - if !strings.HasPrefix(instance, "http://") && - !strings.HasPrefix(instance, "https://") { - instance = "https://" + instance - } - ghc.ServerURL = instance - ghc.APIURL = instance + "/api/v1" // the version of Gitea is v1 - ghc.GraphQLURL = "" // Gitea doesn't support graphql - } + rc.setInstanceURLs(ghc) // allow to be overridden by user if rc.Config.Env["GITHUB_SERVER_URL"] != "" { @@ -1608,6 +1582,20 @@ func (rc *RunContext) getGithubContext(ctx context.Context) *model.GithubContext return ghc } +func (rc *RunContext) setInstanceURLs(ghc *model.GithubContext) { + if instance := strings.TrimRight(rc.Config.GitHubInstance, "/"); instance != "" { + ghc.ServerURL = withHTTPS(instance) + ghc.APIURL = ghc.ServerURL + "/api/v1" + } +} + +func withHTTPS(u string) string { + if strings.HasPrefix(u, "http://") || strings.HasPrefix(u, "https://") { + return u + } + return "https://" + u +} + func isLocalCheckout(ghc *model.GithubContext, step *model.Step) bool { if step.Type() == model.StepTypeInvalid { // This will be errored out by the executor later, we need this here to avoid a null panic though @@ -1657,20 +1645,16 @@ func (rc *RunContext) withGithubEnv(ctx context.Context, github *model.GithubCon env["GITHUB_HEAD_REF"] = github.HeadRef env["GITHUB_SERVER_URL"] = github.ServerURL env["GITHUB_API_URL"] = github.APIURL - env["GITHUB_GRAPHQL_URL"] = github.GraphQLURL - { // Adapt to Gitea - instance := rc.Config.GitHubInstance - if !strings.HasPrefix(instance, "http://") && - !strings.HasPrefix(instance, "https://") { - instance = "https://" + instance + for _, name := range []string{"GITHUB_SERVER_URL", "GITHUB_API_URL"} { + if env[name] == "" { + delete(env, name) } - env["GITHUB_SERVER_URL"] = instance - env["GITHUB_API_URL"] = instance + "/api/v1" // the version of Gitea is v1 - env["GITHUB_GRAPHQL_URL"] = "" // Gitea doesn't support graphql - env["GITHUB_RUN_ATTEMPT"] = github.RunAttempt } + env["GITHUB_GRAPHQL_URL"] = "" + env["GITHUB_RUN_ATTEMPT"] = github.RunAttempt + env["RUNNER_NAME"] = rc.Config.RunnerName env["RUNNER_ENVIRONMENT"] = "self-hosted" if workspace := parentDir(github.Workspace); workspace != "" { diff --git a/act/runner/run_context_test.go b/act/runner/run_context_test.go index 46c3a39e..506fb687 100644 --- a/act/runner/run_context_test.go +++ b/act/runner/run_context_test.go @@ -915,6 +915,7 @@ func TestGetGitHubContext(t *testing.T) { Run: &model.Run{ Workflow: &model.Workflow{ Name: "GitHubContextTest", + Jobs: map[string]*model.Job{"job1": {}}, }, }, Name: "GitHubContextTest", @@ -960,6 +961,36 @@ func TestGetGitHubContext(t *testing.T) { assert.Equal(t, "preset-actor", ghc.Actor) assert.Equal(t, "triggerer", ghc.TriggeringActor) assert.Equal(t, "job1", ghc.Job) + + preset := &model.GithubContext{ServerURL: "https://preset.test", APIURL: "https://preset.test/api/v1"} + for _, testcase := range []struct { + name, instance, serverURL, apiURL string + preset *model.GithubContext + env map[string]string + }{ + {name: "unknown instance"}, + {name: "bare host with trailing slash", instance: "gitea.test/", serverURL: "https://gitea.test", apiURL: "https://gitea.test/api/v1"}, + {name: "scheme port and base path", instance: "http://gitea.test:3000/gitea/", serverURL: "http://gitea.test:3000/gitea", apiURL: "http://gitea.test:3000/gitea/api/v1"}, + {name: "daemon instance replaces preset endpoints", instance: "gitea.test", preset: preset, serverURL: "https://gitea.test", apiURL: "https://gitea.test/api/v1"}, + {name: "daemon keeps preset endpoints without instance", preset: preset, serverURL: "https://preset.test", apiURL: "https://preset.test/api/v1"}, + {name: "environment overrides", env: map[string]string{"GITHUB_SERVER_URL": "https://override.test", "GITHUB_API_URL": "https://override.test/api"}, serverURL: "https://override.test", apiURL: "https://override.test/api"}, + } { + t.Run(testcase.name, func(t *testing.T) { + rc.Config.GitHubInstance = testcase.instance + rc.Config.PresetGitHubContext = testcase.preset + rc.Config.Env = map[string]string{"GITHUB_REPOSITORY": "owner/repo", "GITHUB_REF": "refs/heads/main", "SHA_REF": "1234567890"} + maps.Copy(rc.Config.Env, testcase.env) + github := rc.getGithubContext(t.Context()) + assert.Equal(t, testcase.serverURL, github.ServerURL) + assert.Equal(t, testcase.apiURL, github.APIURL) + env := map[string]string{} + rc.withGithubEnv(t.Context(), github, env) + _, exported := env["GITHUB_SERVER_URL"] + assert.Equal(t, testcase.serverURL != "", exported) + assert.Equal(t, testcase.serverURL, env["GITHUB_SERVER_URL"]) + assert.Equal(t, testcase.apiURL, env["GITHUB_API_URL"]) + }) + } } func TestGetGithubContextRef(t *testing.T) { diff --git a/act/runner/runner.go b/act/runner/runner.go index 0d401352..1bada894 100644 --- a/act/runner/runner.go +++ b/act/runner/runner.go @@ -5,6 +5,7 @@ package runner import ( + "cmp" "context" "fmt" "net/http" @@ -49,7 +50,7 @@ type Config struct { ContainerDaemonSocket string // Path to Docker daemon socket ContainerOptions string // Options for the job container UseGitIgnore bool // controls if paths in .gitignore should not be copied into container, default true - GitHubInstance string // GitHub instance to use, default "github.com" + GitHubInstance string // instance hostname or URL, empty when unknown ContainerCapAdd []string // list of kernel capabilities to add to the containers ContainerCapDrop []string // list of kernel capabilities to remove from the containers ArtifactServerPath string // the path where the artifact server stores uploads @@ -111,13 +112,7 @@ func (c Config) GetToken() string { // DefaultActionURL returns the host used for implicit remote actions. func (c Config) DefaultActionURL() string { - if c.DefaultActionInstance != "" { - return c.DefaultActionInstance - } - if c.GitHubInstance != "" { - return c.GitHubInstance - } - return "github.com" + return cmp.Or(c.DefaultActionInstance, c.GitHubInstance, "github.com") } type caller struct { diff --git a/act/runner/step.go b/act/runner/step.go index 299c6f6d..5f435dee 100644 --- a/act/runner/step.go +++ b/act/runner/step.go @@ -186,19 +186,25 @@ func runStepExecutor(step step, stage stepStage, executor common.Executor) commo if err == nil { err = insecureErr } - if fileErr := processRunnerEnvFileCommand(ctx, envFileCommand, rc, rc.setEnvFile); fileErr != nil && err == nil { + fileCommandCtx := ctx + if ctx.Err() != nil { // an interrupted step still records its file commands, a live deadline stays in force + var cancelFileCommands context.CancelFunc + fileCommandCtx, cancelFileCommands = context.WithTimeout(context.WithoutCancel(ctx), jobCleanupTimeout) + defer cancelFileCommands() + } + if fileErr := processRunnerEnvFileCommand(fileCommandCtx, envFileCommand, rc, rc.setEnvFile); fileErr != nil && err == nil { err = fileErr } - if fileErr := processRunnerEnvFileCommand(ctx, stateFileCommand, rc, rc.saveState); fileErr != nil && err == nil { + if fileErr := processRunnerEnvFileCommand(fileCommandCtx, stateFileCommand, rc, rc.saveState); fileErr != nil && err == nil { err = fileErr } - if fileErr := processRunnerEnvFileCommand(ctx, outputFileCommand, rc, rc.setOutput); fileErr != nil && err == nil { + if fileErr := processRunnerEnvFileCommand(fileCommandCtx, outputFileCommand, rc, rc.setOutput); fileErr != nil && err == nil { err = fileErr } - if fileErr := rc.UpdateExtraPath(ctx, path.Join(actPath, pathFileCommand)); fileErr != nil && err == nil { + if fileErr := rc.UpdateExtraPath(fileCommandCtx, path.Join(actPath, pathFileCommand)); fileErr != nil && err == nil { err = fileErr } - _ = rc.JobContainer.Copy(actPath, files...)(ctx) + _ = rc.JobContainer.Copy(actPath, files...)(fileCommandCtx) if err == nil { logger.WithField("stepResult", stepResult.Conclusion).Infof("Success - %s %s", stage, stepString) diff --git a/act/runner/step_action_remote.go b/act/runner/step_action_remote.go index 1968d356..b3de2339 100644 --- a/act/runner/step_action_remote.go +++ b/act/runner/step_action_remote.go @@ -88,6 +88,12 @@ func (sar *stepActionRemote) prepareActionExecutor() common.Executor { } } + unlock, err := git.AcquireCloneLock(ctx, actionDir) + if err != nil { + return err + } + defer unlock() + // Best effort: the download report falls back to the ref alone when the commit is unknown. if _, sha, err := git.FindGitRevision(ctx, actionDir); err != nil { common.Logger(ctx).Debugf("unable to resolve the commit of %s: %v", sar.remoteAction.Reference(), err) @@ -100,7 +106,6 @@ func (sar *stepActionRemote) prepareActionExecutor() common.Executor { return f, f, err } - defer git.AcquireCloneLock(actionDir)() actionModel, err := sar.readAction(ctx, sar.Step, actionDir, sar.remoteAction.Path, remoteReader, os.WriteFile) sar.action = actionModel return err @@ -251,8 +256,8 @@ func (ra *remoteAction) CloneURL(u string) string { if ra.URL == "" { // keep an absolute local path as-is (used by tests to resolve actions from a local // repo); only bare host names get the https:// scheme prepended - if !strings.HasPrefix(u, "http://") && !strings.HasPrefix(u, "https://") && !filepath.IsAbs(u) { - u = "https://" + u + if !filepath.IsAbs(u) { + u = withHTTPS(u) } } else { u = ra.URL @@ -301,6 +306,9 @@ func newRemoteAction(action string, github *model.GithubContext, instanceURL str } ra.Org, ra.Repo, _ = strings.Cut(repo, "/") ra.URL, ra.Ref = github.ServerURL, ref + if ra.URL == "" { + return nil, fmt.Errorf("unable to resolve %q without a Gitea instance", action) + } if ra.Org == "" || ra.Repo == "" || ra.Ref == "" { return nil, fmt.Errorf("unable to resolve %q without a repository and commit", action) } diff --git a/act/runner/step_action_remote_test.go b/act/runner/step_action_remote_test.go index 30b4ccc1..bd510688 100644 --- a/act/runner/step_action_remote_test.go +++ b/act/runner/step_action_remote_test.go @@ -193,7 +193,7 @@ func TestStepActionRemote(t *testing.T) { RunContext: &RunContext{ Config: &Config{ GitHubInstance: "github.com", - ActionCacheDir: "/tmp/test-cache", + ActionCacheDir: t.TempDir(), }, Run: &model.Run{ JobID: "1", @@ -447,7 +447,7 @@ func TestStepActionRemotePost(t *testing.T) { RunContext: &RunContext{ Config: &Config{ GitHubInstance: "https://github.com", - ActionCacheDir: "/tmp/test-cache", + ActionCacheDir: t.TempDir(), }, JobContainer: cm, Run: &model.Run{ @@ -688,6 +688,7 @@ func Test_newRemoteActionSelfRepo(t *testing.T) { Ref: "v1", }, }, + {name: "unknown instance", action: "$/.gitea/actions/build", github: &model.GithubContext{Repository: "owner/workflow-repo", Sha: "abc123"}}, {name: "empty path", action: "$/", github: workflow}, {name: "ref suffix is not allowed", action: "$/.gitea/actions/build@v1", github: workflow}, {name: "path traversal", action: "$/../escape", github: workflow}, @@ -867,7 +868,7 @@ func TestStepActionRemoteCloneTokenSurvivesNilSecrets(t *testing.T) { Config: &Config{ GitHubInstance: tt.gitHubInstance, DefaultActionInstance: tt.defaultActionInstance, - ActionCacheDir: "/tmp/test-cache", + ActionCacheDir: t.TempDir(), // Mirrors the state of a composite RunContext: job secrets are // stripped, but the job token is still reachable via Config.Token. Secrets: nil, diff --git a/act/runner/step_test.go b/act/runner/step_test.go index 915c8166..ef7c64a5 100644 --- a/act/runner/step_test.go +++ b/act/runner/step_test.go @@ -7,9 +7,11 @@ package runner import ( "context" "errors" + "io" "os" "strings" "testing" + "testing/synctest" "gitea.com/gitea/runner/act/common" "gitea.com/gitea/runner/act/container" @@ -189,7 +191,6 @@ func TestSetupEnv(t *testing.T) { "GITHUB_ACTION_PATH": "", "GITHUB_ACTION_REF": "", "GITHUB_ACTION_REPOSITORY": "", - "GITHUB_API_URL": "https:///api/v1", // Gitea uses api/v1 (upstream GitHub: api/v3) "GITHUB_BASE_REF": "", "GITHUB_EVENT_NAME": "", "GITHUB_EVENT_PATH": "/var/run/act/workflow/event.json", @@ -200,7 +201,6 @@ func TestSetupEnv(t *testing.T) { "GITHUB_RUN_ATTEMPT": "", "GITHUB_RUN_ID": "runId", "GITHUB_RUN_NUMBER": "1", - "GITHUB_SERVER_URL": "https://", "GITHUB_WORKFLOW": "", "INPUT_ID": "1234567", "INPUT_STEP_WITH": "with-value", @@ -475,6 +475,26 @@ func TestRunStepExecutorDoesNotLeakRefusalToNextStep(t *testing.T) { require.NoError(t, errB) } +type fileCommandTestContainer struct { + *container.HostEnvironment +} + +func (c *fileCommandTestContainer) UpdateFromEnv(srcPath string, env *map[string]string) common.Executor { + return func(ctx context.Context) error { + if err := ctx.Err(); err != nil { + return err + } + return c.HostEnvironment.UpdateFromEnv(srcPath, env)(ctx) + } +} + +func (c *fileCommandTestContainer) GetContainerArchive(ctx context.Context, srcPath string) (io.ReadCloser, error) { + if err := ctx.Err(); err != nil { + return nil, err + } + return c.HostEnvironment.GetContainerArchive(ctx, srcPath) +} + func TestRunStepExecutorParity(t *testing.T) { newStep := func(t *testing.T, stepModel *model.Step) *stepRun { rc := createRunContext(t) @@ -527,6 +547,30 @@ func TestRunStepExecutorParity(t *testing.T) { } }) + t.Run("file commands survive a job timeout", func(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + step := newStep(t, &model.Step{ID: "interrupted"}) + ctx, cancel := applyJobTimeout(t.Context(), step.RunContext, &model.Job{TimeoutMinutes: "1"}) + defer cancel() + step.RunContext.JobContainer = &fileCommandTestContainer{step.RunContext.JobContainer.(*container.HostEnvironment)} + err := runStepExecutor(step, stepStageMain, func(execCtx context.Context) error { + for name, content := range map[string]string{ + "GITHUB_ENV": "KEPT_ENV=value\n", "GITHUB_STATE": "saved=value\n", + "GITHUB_OUTPUT": "kept=value\n", "GITHUB_PATH": "/kept/bin\n", + } { + require.NoError(t, os.WriteFile(step.env[name], []byte(content), 0o600)) + } + <-execCtx.Done() + return execCtx.Err() + })(ctx) + require.ErrorIs(t, err, context.DeadlineExceeded) + assert.Equal(t, "value", step.RunContext.Env["KEPT_ENV"]) + assert.Equal(t, "value", step.RunContext.IntraActionState[step.Step.ID]["saved"]) + assert.Equal(t, "value", step.RunContext.StepResults[step.Step.ID].Outputs["kept"]) + assert.Contains(t, step.RunContext.ExtraPath, "/kept/bin") + }) + }) + t.Run("composite child files do not leak", func(t *testing.T) { outer := newStep(t, &model.Step{ID: "outer"}) childRC := &RunContext{ diff --git a/internal/app/cmd/daemon.go b/internal/app/cmd/daemon.go index dd564301..a641b9a4 100644 --- a/internal/app/cmd/daemon.go +++ b/internal/app/cmd/daemon.go @@ -5,8 +5,10 @@ package cmd import ( "context" + "crypto/tls" "errors" "fmt" + "net/http" "os" "path" "path/filepath" @@ -29,6 +31,8 @@ import ( "gitea.com/gitea/runner/internal/pkg/ver" "connectrpc.com/connect" + runnerv1 "gitea.dev/actionslib/runner/v1" + "github.com/avast/retry-go/v5" "github.com/mattn/go-isatty" log "github.com/sirupsen/logrus" "github.com/spf13/cobra" @@ -185,8 +189,25 @@ func runDaemon(ctx context.Context, daemArgs *daemonArgs, configFile *string) fu }() // declare the labels of the runner before fetching tasks - resp, err := runner.Declare(ctx, ls.Names()) + resp, err := retry.NewWithData[*connect.Response[runnerv1.DeclareResponse]]( + retry.Context(ctx), + retry.UntilSucceeded(), + retry.Delay(declareRetryDelay), + retry.DelayType(retry.FixedDelay), + retry.RetryIf(shouldRetryDeclare), + retry.OnRetry(func(attempt uint, err error) { + if attempt == 0 { + log.WithError(err).Warn("fail to invoke Declare, retrying until Gitea is reachable") + } else { + log.WithError(err).Debug("fail to invoke Declare, retrying") + } + }), + ).Do(func() (*connect.Response[runnerv1.DeclareResponse], error) { + return runner.Declare(ctx, ls.Names()) + }) switch { + case err != nil && ctx.Err() != nil && !client.IsRegistrationRejected(err): + return nil case err != nil && connect.CodeOf(err) == connect.CodeUnimplemented: log.Errorf("Your Gitea version is too old to support runner declare, please upgrade to v1.21 or later") return err @@ -352,6 +373,20 @@ func initLogging(cfg *config.Config) { } } +var declareRetryDelay = 30 * time.Second + +func shouldRetryDeclare(err error) bool { + if errors.As(err, new(*tls.CertificateVerificationError)) || errors.As(err, new(tls.AlertError)) || errors.Is(err, http.ErrSchemeMismatch) { // misconfigurations never heal + return false + } + switch connect.CodeOf(err) { + case connect.CodeUnauthenticated, connect.CodeUnimplemented, connect.CodePermissionDenied, connect.CodeInvalidArgument: + return false + default: + return !client.IsRegistrationRejected(err) + } +} + var commonSocketPaths = []string{ "/var/run/docker.sock", "/run/podman/podman.sock", diff --git a/internal/app/cmd/daemon_test.go b/internal/app/cmd/daemon_test.go index a6a7a832..308b970a 100644 --- a/internal/app/cmd/daemon_test.go +++ b/internal/app/cmd/daemon_test.go @@ -5,11 +5,13 @@ package cmd import ( "context" + "crypto/tls" "errors" "net/http" "net/http/httptest" "os" "path/filepath" + "sync/atomic" "testing" "time" @@ -87,12 +89,19 @@ var errUnregisteredRunner = errors.New("rpc error: code = Unauthenticated desc = type daemonTestService struct { runnerv1connect.UnimplementedRunnerServiceHandler declareErr, fetchErr error + declareCalls atomic.Int32 beforeTask bool cancel context.CancelFunc } func (s *daemonTestService) Declare(context.Context, *connect.Request[runnerv1.DeclareRequest]) (*connect.Response[runnerv1.DeclareResponse], error) { + if s.declareCalls.Add(1) == 1 { + return nil, connect.NewError(connect.CodeUnavailable, errors.New("bad gateway")) + } if s.declareErr != nil { + if s.beforeTask { + s.cancel() + } return nil, s.declareErr } return connect.NewResponse(&runnerv1.DeclareResponse{Runner: &runnerv1.Runner{Name: "test"}}), nil @@ -117,7 +126,9 @@ func (*daemonTestService) UpdateLog(_ context.Context, req *connect.Request[runn return connect.NewResponse(&runnerv1.UpdateLogResponse{AckIndex: req.Msg.Index + int64(len(req.Msg.Rows))}), nil } -func TestDaemonRemovesOnlyConsumedEphemeralRegistration(t *testing.T) { +func TestDaemonRetriesDeclareOutagesAndRemovesOnlyConsumedEphemeralRegistration(t *testing.T) { + defer func(delay time.Duration) { declareRetryDelay = delay }(declareRetryDelay) + declareRetryDelay = 0 for _, tc := range []struct { name string ephemeral, beforeTask bool @@ -132,11 +143,13 @@ func TestDaemonRemovesOnlyConsumedEphemeralRegistration(t *testing.T) { {"ephemeral proxy 401 on fetch", true, false, nil, connect.NewError(connect.CodeUnauthenticated, errors.New("HTTP status 401 Unauthorized")), false}, {"ephemeral rejected on declare", true, false, errUnregisteredRunner, nil, true}, {"persistent rejected on declare", false, false, errUnregisteredRunner, nil, false}, + {"ephemeral stopped during declare outage", true, true, connect.NewError(connect.CodeUnavailable, errors.New("bad gateway")), nil, false}, } { t.Run(tc.name, func(t *testing.T) { ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second) defer cancel() - _, handler := runnerv1connect.NewRunnerServiceHandler(&daemonTestService{declareErr: tc.declareErr, fetchErr: tc.fetchErr, beforeTask: tc.beforeTask, cancel: cancel}) + service := &daemonTestService{declareErr: tc.declareErr, fetchErr: tc.fetchErr, beforeTask: tc.beforeTask, cancel: cancel} + _, handler := runnerv1connect.NewRunnerServiceHandler(service) server := httptest.NewServer(http.StripPrefix("/api/actions", handler)) defer server.Close() dir := t.TempDir() @@ -145,11 +158,12 @@ func TestDaemonRemovesOnlyConsumedEphemeralRegistration(t *testing.T) { require.NoError(t, os.WriteFile(configFile, []byte("runner:\n file: "+regFile+"\n idle_cleanup_interval: 0s\ncache:\n enabled: false\n"), 0o600)) require.NoError(t, config.SaveRegistration(regFile, &config.Registration{Address: server.URL, UUID: "test", Name: "test", Token: "test", Labels: []string{"host:host"}, Ephemeral: tc.ephemeral})) err := runDaemon(ctx, &daemonArgs{Once: true}, &configFile)(nil, nil) - if tc.declareErr == nil && tc.fetchErr == nil { + if tc.declareErr == nil && tc.fetchErr == nil || tc.beforeTask { require.NoError(t, err) } else { require.Error(t, err) } + require.Equal(t, int32(2), service.declareCalls.Load()) if tc.removed { require.NoFileExists(t, regFile) } else { @@ -158,3 +172,20 @@ func TestDaemonRemovesOnlyConsumedEphemeralRegistration(t *testing.T) { }) } } + +func TestShouldRetryDeclare(t *testing.T) { + for _, tc := range []struct { + err error + retry bool + }{ + {connect.NewError(connect.CodeUnavailable, errors.New("bad gateway")), true}, + {connect.NewWireError(connect.CodeInternal, errors.New("update runner")), true}, + {connect.NewError(connect.CodeUnavailable, &tls.CertificateVerificationError{Err: errors.New("unknown authority")}), false}, + {connect.NewError(connect.CodeUnavailable, http.ErrSchemeMismatch), false}, + {connect.NewError(connect.CodeUnavailable, tls.AlertError(42)), false}, + {connect.NewError(connect.CodePermissionDenied, errors.New("forbidden")), false}, + {connect.NewWireError(connect.CodeUnknown, errors.New("unregistered runner")), false}, + } { + require.Equal(t, tc.retry, shouldRetryDeclare(tc.err), tc.err.Error()) + } +} diff --git a/scripts/test-dind.sh b/scripts/test-dind.sh index 4643c2aa..7d93ee83 100755 --- a/scripts/test-dind.sh +++ b/scripts/test-dind.sh @@ -9,7 +9,7 @@ # # Usage: scripts/test-dind.sh [target] [-- go-test-args...] # target: dind (default), dind-rootless, or podman to run PODMAN_TEST_IMAGE's API service instead -# go-test-args: passed verbatim to `go test`. Defaults cover image env extraction, +# go-test-args: passed verbatim to `go test`. Defaults cover image platform detection, # symlink copying, a mounted Docker job using cached images and the # image's s6 supervision, or the Docker proxy probe for podman. # @@ -55,7 +55,7 @@ else daemon_args=(-e DOCKER_TLS_CERTDIR= --entrypoint dockerd-entrypoint.sh "$image" --host=tcp://0.0.0.0:2375) if [ $# -eq 0 ]; then default_tests=true - set -- -count=1 -race -run '^TestDocker$' ./act/container/ + set -- -count=1 -race -run '^TestImageExistsLocally$' ./act/container/ fi fi