fix: apply a job container without image to the runs-on image (#1252)

Fixes https://gitea.com/gitea/runner/issues/1251, a regression from https://gitea.com/gitea/runner/pulls/1233.

A job `container:` without an `image` key configures the runs-on image again, as in 3.x, so its `volumes`, `options`, `credentials` and `env` apply. An image that evaluates to empty still skips the container, as on GitHub.

---------

Co-authored-by: bircni <[email protected]>
Reviewed-on: https://gitea.com/gitea/runner/pulls/1252
Reviewed-by: Lunny Xiao <[email protected]>
Reviewed-by: bircni <[email protected]>
Co-authored-by: silverwind <[email protected]>

act/runner/job_executor.go

@@ -435,11 +435,7 @@ func evaluateJobEnvAndDefaults(ctx context.Context, rc *RunContext) error {
 		return err
 	}
 	rc.jobRunDefaults = defaults.Run
-	if rc.containerSpec.Image == "" {
-		return nil
-	}
-	_, containerEnv := splitContainerEnv(rc.Run.Job().RawContainer)
-	return model.DecodeEvaluated("container env", containerEnv, rc.ExprEval.shared(ctx).EvaluateYamlNode, &rc.containerSpec.Env)
+	return model.DecodeEvaluated("container env", rc.containerEnv, rc.ExprEval.shared(ctx).EvaluateYamlNode, &rc.containerSpec.Env)
 }
 
 func setJobOutputs(ctx context.Context, rc *RunContext) error {

act/runner/run_context.go

@@ -64,7 +64,8 @@ type RunContext struct {
 	ExprEval            *expressionEvaluator
 	JobContainer        container.ExecutionsEnvironment
 	serviceContainers   []*serviceContainer
-	containerSpec       model.ContainerSpec // container:, resolved once with platformImage, zero without a container image
+	containerSpec       model.ContainerSpec // container:, resolved once with platformImage
+	containerEnv        yaml.Node           // container.env of the kept containerSpec, decoded once the job env resolves
 	JobName             string
 	ActionPath          string
 	Parent              *RunContext
@@ -1189,16 +1190,19 @@ func (rc *RunContext) runsOnPlatformNames(ctx context.Context) []string {
 	return model.RunsOnFromNode(rawRunsOn)
 }
 
-// resolvePlatformImage evaluates the job's container once for every consumer, ignoring one without an image as GitHub does.
+// resolvePlatformImage evaluates the job's container once for every consumer, skipping one whose image is empty as GitHub does.
 func (rc *RunContext) resolvePlatformImage(ctx context.Context) error {
-	withoutEnv, _ := splitContainerEnv(rc.Run.Job().RawContainer)
+	withoutEnv, env := splitContainerEnv(rc.Run.Job().RawContainer)
 	var spec model.ContainerSpec
 	if err := model.DecodeEvaluated("container", withoutEnv, rc.ExprEval.shared(ctx).EvaluateYamlNode, &spec); err != nil {
 		return err
 	}
 	spec.Image = strings.TrimPrefix(spec.Image, "docker://")
-	if spec.Image != "" {
-		rc.containerSpec, rc.platformImage = spec, spec.Image
+	if spec.Image != "" || imagelessContainer(withoutEnv) {
+		rc.containerSpec, rc.containerEnv = spec, env
+	}
+	if rc.containerSpec.Image != "" {
+		rc.platformImage = rc.containerSpec.Image
 		return nil
 	}
 	image, err := rc.runsOnImage(ctx)
@@ -1206,6 +1210,19 @@ func (rc *RunContext) resolvePlatformImage(ctx context.Context) error {
 	return err
 }
 
+// imagelessContainer reports a container: mapping without an image key, which configures the runs-on image.
+func imagelessContainer(container yaml.Node) bool {
+	if container.Kind != yaml.MappingNode {
+		return false
+	}
+	for i := 0; i < len(container.Content); i += 2 {
+		if container.Content[i].Value == "image" {
+			return false
+		}
+	}
+	return true
+}
+
 // splitContainerEnv takes `env` out of a container mapping, since env reads the job env that resolves after the job environment starts.
 func splitContainerEnv(container yaml.Node) (withoutEnv, env yaml.Node) {
 	if container.Kind != yaml.MappingNode {

act/runner/run_context_test.go

@@ -1601,6 +1601,27 @@ func TestImageOSFromImage(t *testing.T) {
 	}
 }
 
+func TestResolvePlatformImageAppliesImagelessContainerToRunsOnImage(t *testing.T) {
+	for _, tc := range []struct {
+		name, image string
+		want        model.ContainerSpec
+	}{
+		{"without image key", "", model.ContainerSpec{Volumes: []string{"/host/data:/data"}, Env: map[string]string{"KEY": "value"}}},
+		{"with empty image", "image: '', ", model.ContainerSpec{}},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			rc := createIfTestRunContext(map[string]*model.Job{
+				"job1": createJob(t, "runs-on: toolkit\ncontainer: {"+tc.image+"volumes: [/host/data:/data], env: {KEY: value}}", ""),
+			})
+			require.NoError(t, rc.resolvePlatformImage(t.Context()))
+			require.NoError(t, evaluateJobEnvAndDefaults(t.Context(), rc))
+
+			assert.Equal(t, "ubuntu-latest", rc.platformImage)
+			assert.Equal(t, tc.want, rc.containerSpec)
+		})
+	}
+}
+
 func createRunsOnRunContext(t *testing.T, runsOn string) *RunContext {
 	return createIfTestRunContext(map[string]*model.Job{
 		"job1": createJob(t, "runs-on: "+runsOn, ""),

e2e/compatibility_test.go

@@ -69,6 +69,7 @@ func testPayloads(t *testing.T) {
 		"cell-a-2",
 		"cell-b-1",
 		"cell-b-2",
+		"imageless env=imageless-env-value host=imageless-host",
 	} {
 		if !strings.Contains(logs, want) {
 			t.Errorf("stored logs are missing %q", want)

e2e/testdata/workflows/payloads.yml

@@ -3,6 +3,10 @@ on: push
 jobs:
   verify:
     runs-on: ubuntu-latest
+    container:
+      env:
+        IMAGELESS_ENV: imageless-env-value
+      options: --hostname imageless-host
     services:
       web:
         image: ${{ vars.E2E_SERVICE_IMAGE }}
@@ -19,6 +23,7 @@ jobs:
           echo '::notice::encoded-first%0Aencoded-second'
           echo "the variable is ${{ vars.GREETING }}"
           echo "the secret is ${{ secrets.FOO }}"
+          echo "imageless env=$IMAGELESS_ENV host=$(hostname)"
           curl --connect-timeout 1 --max-time 2 --retry 30 --retry-delay 1 --retry-max-time 30 --retry-all-errors -fsS -o /dev/null http://web/
   produce:
     runs-on: ubuntu-latest