diff --git a/act/container/docker_pull.go b/act/container/docker_pull.go index dc0f28f4..6bb3f6c4 100644 --- a/act/container/docker_pull.go +++ b/act/container/docker_pull.go @@ -72,7 +72,9 @@ func NewDockerPullExecutor(input NewDockerPullExecutorInput) common.Executor { _ = logDockerResponse(logger, reader, err != nil) } - return err + if err != nil { + return fmt.Errorf("failed to pull image '%s' (%s): %w", imageRef, input.Platform, err) + } } return nil } diff --git a/act/runner/expression.go b/act/runner/expression.go index 060ac5fd..3dc9a108 100644 --- a/act/runner/expression.go +++ b/act/runner/expression.go @@ -229,7 +229,8 @@ func (ee expressionEvaluator) evaluate(ctx context.Context, in string, defaultSt logger.Debugf("evaluating expression '%s'", in) evaluated, err := ee.interpreter.Evaluate(in, defaultStatusCheck) - printable := regexp.MustCompile(`::add-mask::.*`).ReplaceAllString(fmt.Sprintf("%t", evaluated), "::add-mask::***)") + // evaluated is an any: %t renders everything but a bool as "%!t(string=...)" + printable := regexp.MustCompile(`::add-mask::.*`).ReplaceAllString(fmt.Sprintf("%v", evaluated), "::add-mask::***)") logger.Debugf("expression '%s' evaluated to '%s'", in, printable) return evaluated, err diff --git a/act/runner/run_context.go b/act/runner/run_context.go index 1f36da0c..1f85029f 100644 --- a/act/runner/run_context.go +++ b/act/runner/run_context.go @@ -339,6 +339,9 @@ func printStartJobContainerGroup(ctx context.Context, image, name, network strin } } +// newContainer is a variable so tests can substitute a container that needs no Docker daemon. +var newContainer = container.NewContainer + func (rc *RunContext) startJobContainer() common.Executor { return func(ctx context.Context) error { logger := common.Logger(ctx) @@ -402,7 +405,9 @@ func (rc *RunContext) startJobContainer() common.Executor { for _, v := range spec.Cmd { interpolatedCmd = append(interpolatedCmd, rc.ExprEval.Interpolate(ctx, v)) } - username, password, err = rc.handleServiceCredentials(ctx, spec.Credentials) + // keep these local: reusing username/password would overwrite the + // credentials the job container is pulled with further down + serviceUsername, servicePassword, err := rc.handleServiceCredentials(ctx, spec.Credentials) if err != nil { return fmt.Errorf("failed to handle service %s credentials: %w", serviceID, err) } @@ -423,12 +428,12 @@ func (rc *RunContext) startJobContainer() common.Executor { } serviceContainerName := createContainerName(rc.jobContainerName(), serviceID) - c := container.NewContainer(&container.NewContainerInput{ + c := newContainer(&container.NewContainerInput{ Name: serviceContainerName, WorkingDir: ext.ToContainerPath(rc.Config.Workdir), Image: serviceImage, - Username: username, - Password: password, + Username: serviceUsername, + Password: servicePassword, Cmd: interpolatedCmd, Env: envs, Mounts: serviceMounts, @@ -484,7 +489,7 @@ func (rc *RunContext) startJobContainer() common.Executor { // For Gitea, `jobContainerNetwork` should be the same as `networkName` jobContainerNetwork := networkName - rc.JobContainer = container.NewContainer(&container.NewContainerInput{ + rc.JobContainer = newContainer(&container.NewContainerInput{ Cmd: nil, Entrypoint: []string{"/bin/sleep", fmt.Sprint(rc.Config.ContainerMaxLifetime.Round(time.Second).Seconds())}, WorkingDir: ext.ToContainerPath(rc.Config.Workdir), diff --git a/act/runner/run_context_test.go b/act/runner/run_context_test.go index 8b2563af..46b67a8f 100644 --- a/act/runner/run_context_test.go +++ b/act/runner/run_context_test.go @@ -14,6 +14,7 @@ import ( "testing" "gitea.com/gitea/runner/act/common" + "gitea.com/gitea/runner/act/container" "gitea.com/gitea/runner/act/exprparser" "gitea.com/gitea/runner/act/model" @@ -202,6 +203,93 @@ jobs: assert.Empty(t, password) } +// fakeContainer turns every container operation into a no-op, so startJobContainer +// runs without a Docker daemon. The embedded interface is nil, so any method the +// test does not exercise panics rather than silently doing the wrong thing. +type fakeContainer struct { + container.ExecutionsEnvironment +} + +func (fakeContainer) Pull(bool) common.Executor { return func(context.Context) error { return nil } } +func (fakeContainer) Start(bool) common.Executor { return func(context.Context) error { return nil } } +func (fakeContainer) Remove() common.Executor { return func(context.Context) error { return nil } } +func (fakeContainer) Close() common.Executor { return func(context.Context) error { return nil } } +func (fakeContainer) GetActPath() string { return "/var/run/act" } +func (fakeContainer) Create([]string, []string) common.Executor { + return func(context.Context) error { return nil } +} + +func (fakeContainer) Copy(string, ...*container.FileEntry) common.Executor { + return func(context.Context) error { return nil } +} + +// Regression test: a service without a `credentials:` block resolves to empty +// credentials, which used to overwrite the job container's own credentials. +func TestStartJobContainerKeepsJobCredentialsWithServices(t *testing.T) { + workflow, err := model.ReadWorkflow(strings.NewReader(` +name: test +on: push +jobs: + job: + runs-on: ubuntu-latest + container: + image: registry.example/private:latest + credentials: + username: job-user + password: job-password + services: + redis: + image: redis:latest + db: + image: postgres:latest + credentials: + username: db-user + password: db-password + steps: [] +`)) + require.NoError(t, err) + + var inputs []*container.NewContainerInput + origNewContainer := newContainer + newContainer = func(input *container.NewContainerInput) container.ExecutionsEnvironment { + inputs = append(inputs, input) + return fakeContainer{} + } + t.Cleanup(func() { newContainer = origNewContainer }) + + rc := &RunContext{ + Name: "test", + Config: &Config{ + Workdir: "/tmp", + // no daemon: an explicit network mode creates no network, and + // reusing containers short-circuits the volume cleanup executors + ContainerNetworkMode: "host", + ReuseContainers: true, + Env: map[string]string{}, + Secrets: map[string]string{}, + }, + Env: map[string]string{}, + Run: &model.Run{ + JobID: "job", + Workflow: workflow, + }, + } + rc.ExprEval = rc.NewExpressionEvaluator(t.Context()) + + require.NoError(t, rc.startJobContainer()(t.Context())) + + credentials := map[string][2]string{} + for _, in := range inputs { + credentials[in.Image] = [2]string{in.Username, in.Password} + } + + // the job container keeps its own credentials, whichever services exist + require.Equal(t, [2]string{"job-user", "job-password"}, credentials["registry.example/private:latest"]) + // each service keeps its own, and a service without credentials gets none + require.Equal(t, [2]string{"db-user", "db-password"}, credentials["postgres:latest"]) + require.Equal(t, [2]string{"", ""}, credentials["redis:latest"]) +} + func TestRunContext_GetBindsAndMounts(t *testing.T) { rctemplate := &RunContext{ Name: "TestRCName",