From a714e3bb4c3bc161cbcee83118d9ae3c41be0534 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Sun, 8 Jun 2025 13:37:52 +0200 Subject: [PATCH 1/2] fix: runs-on expression per matrix has different results --- pkg/runner/run_context.go | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/pkg/runner/run_context.go b/pkg/runner/run_context.go index 9deaef3c..4073ba10 100644 --- a/pkg/runner/run_context.go +++ b/pkg/runner/run_context.go @@ -824,16 +824,22 @@ func (rc *RunContext) runsOnImage(ctx context.Context) string { func (rc *RunContext) runsOnPlatformNames(ctx context.Context) []string { job := rc.Run.Job() - if job.RunsOn() == nil { + if job.RawRunsOn.IsZero() { return []string{} } - if err := rc.ExprEval.EvaluateYamlNode(ctx, &job.RawRunsOn); err != nil { + node := job.RawRunsOn + + if err := rc.ExprEval.EvaluateYamlNode(ctx, &node); err != nil { common.Logger(ctx).Errorf("error while evaluating runs-on: %v", err) return []string{} } - return job.RunsOn() + // Execute this method on a copy of the job to avoid modifying the original job + j := *job + j.RawRunsOn = node + + return j.RunsOn() } func (rc *RunContext) platformImage(ctx context.Context) string { From 383f3f2aedf17a566d359f6f101c046859e94fc1 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Wed, 23 Jul 2025 13:09:19 +0200 Subject: [PATCH 2/2] wip --- pkg/exprparser/functions.go | 2 ++ pkg/model/workflow.go | 6 +++--- pkg/runner/expression.go | 12 +++++------ pkg/runner/job_executor.go | 4 ++-- pkg/runner/reusable_workflow.go | 4 ++-- pkg/runner/run_context.go | 36 +++++++++++++++++++++------------ pkg/runner/runner.go | 1 + pkg/runner/step.go | 2 +- pkg/runner/step_run.go | 4 ++-- 9 files changed, 42 insertions(+), 29 deletions(-) diff --git a/pkg/exprparser/functions.go b/pkg/exprparser/functions.go index 3a261923..6ff7e673 100644 --- a/pkg/exprparser/functions.go +++ b/pkg/exprparser/functions.go @@ -256,6 +256,7 @@ func (impl *interperterImpl) always() (bool, error) { return true, nil } +// bug: this function uses a parsed workflow plan to check for job failures. func (impl *interperterImpl) jobSuccess() (bool, error) { jobs := impl.config.Run.Workflow.Jobs jobNeeds := impl.getNeedsTransitive(impl.config.Run.Job()) @@ -273,6 +274,7 @@ func (impl *interperterImpl) stepSuccess() (bool, error) { return impl.env.Job.Status == "success", nil } +// bug: this function uses a parsed workflow plan to check for job failures. func (impl *interperterImpl) jobFailure() (bool, error) { jobs := impl.config.Run.Workflow.Jobs jobNeeds := impl.getNeedsTransitive(impl.config.Run.Job()) diff --git a/pkg/model/workflow.go b/pkg/model/workflow.go index b2e79c12..d2f99fb5 100644 --- a/pkg/model/workflow.go +++ b/pkg/model/workflow.go @@ -201,13 +201,13 @@ type Job struct { Uses string `yaml:"uses"` With map[string]interface{} `yaml:"with"` RawSecrets yaml.Node `yaml:"secrets"` - Result string + Result string `yaml:"-"` } // Strategy for the job type Strategy struct { - FailFast bool - MaxParallel int + FailFast bool `yaml:"-"` + MaxParallel int `yaml:"-"` FailFastString string `yaml:"fail-fast"` MaxParallelString string `yaml:"max-parallel"` RawMatrix yaml.Node `yaml:"matrix"` diff --git a/pkg/runner/expression.go b/pkg/runner/expression.go index 5214371f..a6d59c69 100644 --- a/pkg/runner/expression.go +++ b/pkg/runner/expression.go @@ -38,14 +38,14 @@ func (rc *RunContext) NewExpressionEvaluatorWithEnv(ctx context.Context, env map using := make(map[string]exprparser.Needs) strategy := make(map[string]interface{}) if rc.Run != nil { - job := rc.Run.Job() + job := rc.Job() if job != nil && job.Strategy != nil { strategy["fail-fast"] = job.Strategy.FailFast strategy["max-parallel"] = job.Strategy.MaxParallel } jobs := rc.Run.Workflow.Jobs - jobNeeds := rc.Run.Job().Needs() + jobNeeds := rc.Job().Needs() for _, needs := range jobNeeds { using[needs] = exprparser.Needs{ @@ -124,7 +124,7 @@ func (rc *RunContext) NewStepExpressionEvaluatorExt(ctx context.Context, step st func (rc *RunContext) newStepExpressionEvaluator(ctx context.Context, step step, _ *model.GithubContext, inputs map[string]interface{}) ExpressionEvaluator { // todo: cleanup EvaluationEnvironment creation - job := rc.Run.Job() + job := rc.Job() strategy := make(map[string]interface{}) if job.Strategy != nil { strategy["fail-fast"] = job.Strategy.FailFast @@ -132,7 +132,7 @@ func (rc *RunContext) newStepExpressionEvaluator(ctx context.Context, step step, } jobs := rc.Run.Workflow.Jobs - jobNeeds := rc.Run.Job().Needs() + jobNeeds := rc.Job().Needs() using := make(map[string]exprparser.Needs) for _, needs := range jobNeeds { @@ -526,7 +526,7 @@ func setupWorkflowInputs(ctx context.Context, inputs *map[string]interface{}, rc config := rc.Run.Workflow.WorkflowCallConfig() for name, input := range config.Inputs { - value := rc.caller.runContext.Run.Job().With[name] + value := rc.caller.runContext.Job().With[name] if value != nil { node := yaml.Node{} @@ -554,7 +554,7 @@ func setupWorkflowInputs(ctx context.Context, inputs *map[string]interface{}, rc func getWorkflowSecrets(ctx context.Context, rc *RunContext) map[string]string { if rc.caller != nil { - job := rc.caller.runContext.Run.Job() + job := rc.caller.runContext.Job() secrets := job.Secrets() if secrets == nil && job.InheritSecrets() { diff --git a/pkg/runner/job_executor.go b/pkg/runner/job_executor.go index bf8db520..2339a776 100644 --- a/pkg/runner/job_executor.go +++ b/pkg/runner/job_executor.go @@ -160,8 +160,8 @@ func setJobResult(ctx context.Context, info jobInfo, rc *RunContext, success boo jobResult := "success" // we have only one result for a whole matrix build, so we need // to keep an existing result state if we run a matrix - if len(info.matrix()) > 0 && rc.Run.Job().Result != "" { - jobResult = rc.Run.Job().Result + if len(info.matrix()) > 0 && rc.Job().Result != "" { + jobResult = rc.Job().Result } if !success { diff --git a/pkg/runner/reusable_workflow.go b/pkg/runner/reusable_workflow.go index 94543de1..e305290a 100644 --- a/pkg/runner/reusable_workflow.go +++ b/pkg/runner/reusable_workflow.go @@ -12,11 +12,11 @@ import ( ) func newLocalReusableWorkflowExecutor(rc *RunContext) common.Executor { - return newReusableWorkflowExecutor(rc, rc.Config.Workdir, rc.Run.Job().Uses) + return newReusableWorkflowExecutor(rc, rc.Config.Workdir, rc.Job().Uses) } func newRemoteReusableWorkflowExecutor(rc *RunContext) common.Executor { - uses := rc.Run.Job().Uses + uses := rc.Job().Uses remoteReusableWorkflow := newRemoteReusableWorkflow(uses) if remoteReusableWorkflow == nil { diff --git a/pkg/runner/run_context.go b/pkg/runner/run_context.go index 4073ba10..5bf4c6b7 100644 --- a/pkg/runner/run_context.go +++ b/pkg/runner/run_context.go @@ -56,6 +56,16 @@ type RunContext struct { Cancelled bool ContextData map[string]interface{} nodeToolFullPath string + job *model.Job // the job that is currently being executed with evaluated fields +} + +func (rc *RunContext) Job() *model.Job { + if rc.job == nil { + // create a copy of the job to avoid modifying the original job + j := *rc.Run.Job() + rc.job = &j + } + return rc.job } func (rc *RunContext) AddMask(mask string) { @@ -82,7 +92,7 @@ func (rc *RunContext) GetEnv() map[string]string { if rc.Env == nil { rc.Env = map[string]string{} if rc.Run != nil && rc.Run.Workflow != nil && rc.Config != nil { - job := rc.Run.Job() + job := rc.Job() if job != nil { rc.Env = mergeMaps(rc.Run.Workflow.Env, job.Environment(), rc.Config.Env) } @@ -99,7 +109,7 @@ func (rc *RunContext) jobContainerName() string { // networkName return the name of the network which will be created by `act` automatically for job, // only create network if using a service container func (rc *RunContext) networkName() (string, bool) { - if len(rc.Run.Job().Services) > 0 { + if len(rc.Job().Services) > 0 { return fmt.Sprintf("%s-%s-network", rc.jobContainerName(), rc.Run.JobID), true } if rc.Config.ContainerNetworkMode == "" { @@ -155,7 +165,7 @@ func (rc *RunContext) GetBindsAndMounts() ([]string, map[string]string) { name + "-env": ext.GetActPath(), } - if job := rc.Run.Job(); job != nil { + if job := rc.Job(); job != nil { if container := job.Container(); container != nil { for _, v := range container.Volumes { if !strings.Contains(v, ":") || filepath.IsAbs(v) { @@ -406,7 +416,7 @@ func (rc *RunContext) prepareServiceContainers(ctx context.Context, logger logru networkName, createAndDeleteNetwork := rc.networkName() // add service containers - for serviceID, spec := range rc.Run.Job().Services { + for serviceID, spec := range rc.Job().Services { // interpolate env interpolatedEnvs := make(map[string]string, len(spec.Env)) for k, v := range spec.Env { @@ -746,17 +756,17 @@ func (rc *RunContext) matrix() map[string]interface{} { } func (rc *RunContext) result(result string) { - rc.Run.Job().Result = result + rc.Job().Result = result } func (rc *RunContext) steps() []*model.Step { - return rc.Run.Job().Steps + return rc.Job().Steps } // Executor returns a pipeline executor for all the steps in the job func (rc *RunContext) Executor() (common.Executor, error) { var executor common.Executor - var jobType, err = rc.Run.Job().Type() + var jobType, err = rc.Job().Type() if exec, ok := rc.Config.CustomExecutor[jobType]; ok { executor = exec(rc) @@ -796,7 +806,7 @@ func (rc *RunContext) Executor() (common.Executor, error) { } func (rc *RunContext) containerImage(ctx context.Context) string { - job := rc.Run.Job() + job := rc.Job() c := job.Container() if c != nil { @@ -807,7 +817,7 @@ func (rc *RunContext) containerImage(ctx context.Context) string { } func (rc *RunContext) runsOnImage(ctx context.Context) string { - if rc.Run.Job().RunsOn() == nil { + if rc.Job().RunsOn() == nil { common.Logger(ctx).Errorf("'runs-on' key not defined in %s", rc.String()) } @@ -822,7 +832,7 @@ func (rc *RunContext) runsOnImage(ctx context.Context) string { } func (rc *RunContext) runsOnPlatformNames(ctx context.Context) []string { - job := rc.Run.Job() + job := rc.Job() if job.RawRunsOn.IsZero() { return []string{} @@ -851,7 +861,7 @@ func (rc *RunContext) platformImage(ctx context.Context) string { } func (rc *RunContext) options(ctx context.Context) string { - job := rc.Run.Job() + job := rc.Job() c := job.Container() if c != nil { return rc.ExprEval.Interpolate(ctx, c.Options) @@ -861,7 +871,7 @@ func (rc *RunContext) options(ctx context.Context) string { } func (rc *RunContext) isEnabled(ctx context.Context) (bool, error) { - job := rc.Run.Job() + job := rc.Job() l := common.Logger(ctx) runJob, runJobErr := EvalBool(ctx, rc.ExprEval, job.If.Value, exprparser.DefaultStatusCheckSuccess) jobType, jobTypeErr := job.Type() @@ -1189,7 +1199,7 @@ func (rc *RunContext) handleCredentials(ctx context.Context) (string, string, er username := rc.Config.Secrets["DOCKER_USERNAME"] password := rc.Config.Secrets["DOCKER_PASSWORD"] - container := rc.Run.Job().Container() + container := rc.Job().Container() if container == nil || container.Credentials == nil { return username, password, nil } diff --git a/pkg/runner/runner.go b/pkg/runner/runner.go index 4a998f5a..0b95d9e7 100644 --- a/pkg/runner/runner.go +++ b/pkg/runner/runner.go @@ -252,6 +252,7 @@ func (runner *runnerImpl) NewPlanExecutor(plan *model.Plan) common.Executor { return common.NewPipelineExecutor(stagePipeline...).Then(handleFailure(plan)) } +// bug: this function uses a parsed workflow plan to check for job failures. func handleFailure(plan *model.Plan) common.Executor { return func(_ context.Context) error { for _, stage := range plan.Stages { diff --git a/pkg/runner/step.go b/pkg/runner/step.go index 46b39ec0..2bba531c 100644 --- a/pkg/runner/step.go +++ b/pkg/runner/step.go @@ -270,7 +270,7 @@ func setupEnv(ctx context.Context, step step) error { func mergeEnv(ctx context.Context, step step) { env := step.getEnv() rc := step.getRunContext() - job := rc.Run.Job() + job := rc.Job() c := job.Container() if c != nil { diff --git a/pkg/runner/step_run.go b/pkg/runner/step_run.go index bce9ad94..fd7414cd 100644 --- a/pkg/runner/step_run.go +++ b/pkg/runner/step_run.go @@ -166,7 +166,7 @@ func (sr *stepRun) setupShell(ctx context.Context) { step := sr.Step if step.Shell == "" { - step.WorkflowShell = rc.Run.Job().Defaults.Run.Shell + step.WorkflowShell = rc.Job().Defaults.Run.Shell } else { step.WorkflowShell = step.Shell } @@ -209,7 +209,7 @@ func (sr *stepRun) setupWorkingDirectory(ctx context.Context) { workingdirectory := "" if step.WorkingDirectory == "" { - workingdirectory = rc.Run.Job().Defaults.Run.WorkingDirectory + workingdirectory = rc.Job().Defaults.Run.WorkingDirectory } else { workingdirectory = step.WorkingDirectory }