diff --git a/CHANGELOG.md b/CHANGELOG.md index 85a5e3b..5c4bc22 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,8 @@ This project uses [Semantic Versioning](https://semver.org). ### Added +- **`workflow:rules:variables:` now propagate to job rule evaluation** — when a `workflow:rules:` entry matches, any `variables:` it defines are injected into the evaluation context so job `rules:if:` expressions can reference them. Pipeline-level `variables:` are also available as defaults (lower priority). Variable priority order, highest first: `--var` CLI overrides → `--branch`/`--tag`/`--source` shortcuts → workflow-rule variables → pipeline-level variable defaults. This means `$DEPLOY_TARGET == "production"` in a job rule correctly evaluates when a workflow rule sets `DEPLOY_TARGET: production` for the matching branch. The `glint graph tree` command benefits from the same enrichment. + - **`rules:if:` expression evaluator improvements** — six correctness fixes to the GitLab CI expression parser: - **Multi-line expressions** — newlines (`\n`, `\r`) are now treated as whitespace between tokens, so block-scalar `if:` values (e.g. `if: | ...`) and folded YAML scalars with `||`/`&&` on a continuation line are parsed correctly instead of falling back to permissive `true`. - **`${VAR}` curly-brace variable syntax** — `${CI_COMMIT_BRANCH}` is now equivalent to `$CI_COMMIT_BRANCH` everywhere a value is expected. diff --git a/Taskfile.yml b/Taskfile.yml index d07f431..acf2697 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -65,6 +65,14 @@ tasks: ignore_error: false - cmd: ./{{.BINARY}} check --branch feat/x testdata/rules_if_expr.yml ignore_error: false + - cmd: ./{{.BINARY}} check testdata/workflow_vars.yml + ignore_error: false + - cmd: ./{{.BINARY}} check --branch main testdata/workflow_vars.yml + ignore_error: false + - cmd: ./{{.BINARY}} check --branch develop testdata/workflow_vars.yml + ignore_error: false + - cmd: ./{{.BINARY}} check --branch feat/x testdata/workflow_vars.yml + ignore_error: false - cmd: ./{{.BINARY}} check samba-testdata/.gitlab-ci.yml ignore_error: false - cmd: ./{{.BINARY}} check samba-testdata/.gitlab-ci-coverage.yml diff --git a/cmd/glint/main.go b/cmd/glint/main.go index 94a457f..7e1bd30 100644 --- a/cmd/glint/main.go +++ b/cmd/glint/main.go @@ -147,6 +147,7 @@ Examples: ctx := cicontext.New(*branch, *tag, *source, vars) if !ctx.IsEmpty() { + enrichContext(ctx, p) printContext(p, ctx) } @@ -272,6 +273,9 @@ Examples: resolver.Resolve(p) //nolint:errcheck ctx := cicontext.New(*branch, *tag, *source, vars) + if !ctx.IsEmpty() { + enrichContext(ctx, p) + } switch mode { case "default": @@ -300,6 +304,24 @@ Examples: } } +// enrichContext injects pipeline-level variable defaults and then +// workflow-rule-generated variables into ctx before job evaluation. +// Injection respects pinned variables (--branch/--tag/--source/--var always win). +func enrichContext(ctx *cicontext.Context, p *model.Pipeline) { + // Pipeline variables: injected as defaults (lowest priority). + for k, v := range cicontext.ExtractStringVars(p.Variables) { + ctx.Inject(k, v) + } + // Workflow rules: evaluate to find which rule matches, then inject its variables. + runs, ruleVars := cicontext.EvalWorkflow(p, ctx) + if !runs { + fmt.Fprintln(os.Stderr, "[WARNING] workflow:rules: pipeline would not start for this context") + } + for k, v := range ruleVars { + ctx.Inject(k, v) + } +} + func printContext(p *model.Pipeline, ctx *cicontext.Context) { fmt.Printf("Context: %s\n\n", ctx.Summary()) diff --git a/internal/cicontext/context.go b/internal/cicontext/context.go index aca1d81..8522b86 100644 --- a/internal/cicontext/context.go +++ b/internal/cicontext/context.go @@ -6,7 +6,8 @@ import "strings" // pipeline evaluation (rules:if:, only:, except:, workflow:rules:). // Variables are keyed by their name without the leading $. type Context struct { - Vars map[string]string + Vars map[string]string + pinned map[string]bool // vars set via --var or shortcuts; never overwritten by Inject } // New builds a Context from high-level shortcut values and optional KEY=VALUE @@ -17,46 +18,55 @@ type Context struct { // preserving the existing linting behaviour when no context flags are given. // // Override priority (highest wins): extraVars > branch/tag/source shortcuts. +// Both shortcut-derived and extraVar variables are pinned — they will not be +// overwritten by Inject (used for pipeline-level and workflow-rule variables). func New(branch, tag, source string, extraVars []string) *Context { if branch == "" && tag == "" && source == "" && len(extraVars) == 0 { return &Context{} } vars := make(map[string]string) + pinned := make(map[string]bool) + + pin := func(k, v string) { + vars[k] = v + pinned[k] = true + } if branch != "" { - vars["CI_COMMIT_BRANCH"] = branch - vars["CI_COMMIT_REF_NAME"] = branch - vars["CI_COMMIT_REF_SLUG"] = slugify(branch) + pin("CI_COMMIT_BRANCH", branch) + pin("CI_COMMIT_REF_NAME", branch) + pin("CI_COMMIT_REF_SLUG", slugify(branch)) if source == "" { source = "push" } } if tag != "" { - vars["CI_COMMIT_TAG"] = tag - vars["CI_COMMIT_REF_NAME"] = tag - vars["CI_COMMIT_REF_SLUG"] = slugify(tag) - delete(vars, "CI_COMMIT_BRANCH") // tag pushes have no branch variable + pin("CI_COMMIT_TAG", tag) + pin("CI_COMMIT_REF_NAME", tag) + pin("CI_COMMIT_REF_SLUG", slugify(tag)) + delete(vars, "CI_COMMIT_BRANCH") + delete(pinned, "CI_COMMIT_BRANCH") if source == "" { source = "push" } } if source != "" { - vars["CI_PIPELINE_SOURCE"] = source + pin("CI_PIPELINE_SOURCE", source) } if _, ok := vars["CI_DEFAULT_BRANCH"]; !ok { vars["CI_DEFAULT_BRANCH"] = "main" } - // KEY=VALUE overrides win over shortcuts. + // KEY=VALUE overrides win over shortcuts and everything else. for _, kv := range extraVars { k, v, ok := strings.Cut(kv, "=") if ok { - vars[k] = v + pin(k, v) } } - return &Context{Vars: vars} + return &Context{Vars: vars, pinned: pinned} } // IsEmpty reports whether no variables have been set (no context flags given). @@ -73,6 +83,21 @@ func (c *Context) Get(key string) string { return c.Vars[key] } +// Inject sets key=value only if key is not already pinned (i.e. not set via +// --branch / --tag / --source / --var). Used to inject pipeline-level variable +// defaults and workflow-rule variables without overriding explicit user input. +// Calling Inject in order from lowest-priority to highest-priority source +// ensures later calls win over earlier ones. +func (c *Context) Inject(key, value string) { + if c.pinned[key] { + return + } + if c.Vars == nil { + c.Vars = make(map[string]string) + } + c.Vars[key] = value +} + // Summary returns a short human-readable description of the context for CLI output. func (c *Context) Summary() string { if c.IsEmpty() { @@ -90,6 +115,28 @@ func (c *Context) Summary() string { return strings.Join(parts, ", ") } +// ExtractStringVars converts a map[string]any variable block (as used by +// Pipeline.Variables and Rule.Variables) to a flat map[string]string. +// Plain string values are used directly. Extended {value: "..."} map form +// uses the "value" key. Other forms are skipped. +func ExtractStringVars(m map[string]any) map[string]string { + if len(m) == 0 { + return nil + } + out := make(map[string]string, len(m)) + for k, v := range m { + switch val := v.(type) { + case string: + out[k] = val + case map[string]any: + if s, ok := val["value"].(string); ok { + out[k] = s + } + } + } + return out +} + // slugify converts a ref name to its GitLab slug form: // lowercased, non-alphanumeric characters replaced with '-', leading/trailing '-' removed. func slugify(s string) string { diff --git a/internal/cicontext/reachability.go b/internal/cicontext/reachability.go index ce43089..f4346fc 100644 --- a/internal/cicontext/reachability.go +++ b/internal/cicontext/reachability.go @@ -28,13 +28,17 @@ func (s JobState) String() string { } } -// EvalWorkflow returns false when the pipeline's workflow:rules block would -// prevent any pipeline from starting in the given context. -// Returns true when ctx is empty, when there is no workflow block, or when no -// rule is configured. -func EvalWorkflow(p *model.Pipeline, ctx *Context) bool { +// EvalWorkflow evaluates the pipeline's workflow:rules block against ctx. +// Returns (runs, ruleVars): +// - runs=false means the pipeline would not start for this context. +// - ruleVars holds any variables: defined on the matching rule; inject these +// into the context so job rules can reference them. +// +// Returns (true, nil) when ctx is empty, when there is no workflow block, or +// when no rules are configured. +func EvalWorkflow(p *model.Pipeline, ctx *Context) (bool, map[string]string) { if ctx.IsEmpty() || p.Workflow == nil || len(p.Workflow.Rules) == 0 { - return true + return true, nil } vars := ctx.Get for _, rule := range p.Workflow.Rules { @@ -45,9 +49,9 @@ func EvalWorkflow(p *model.Pipeline, ctx *Context) bool { if when == "" { when = "always" } - return when != "never" + return when != "never", ExtractStringVars(rule.Variables) } - return false // no rule matched → pipeline does not run + return false, nil // no rule matched → pipeline does not run } // EvalJob returns the effective JobState for job in the given context. diff --git a/internal/cicontext/reachability_test.go b/internal/cicontext/reachability_test.go new file mode 100644 index 0000000..6963f17 --- /dev/null +++ b/internal/cicontext/reachability_test.go @@ -0,0 +1,243 @@ +package cicontext + +import ( + "testing" + + "git.k3nny.fr/glint/internal/model" +) + +func TestEvalWorkflow(t *testing.T) { + makePipeline := func(rules []model.Rule) *model.Pipeline { + return &model.Pipeline{Workflow: &model.Workflow{Rules: rules}} + } + + tests := []struct { + name string + rules []model.Rule + branch string + wantRuns bool + wantVars map[string]string + }{ + { + name: "no workflow block", + rules: nil, + branch: "main", + wantRuns: true, + wantVars: nil, + }, + { + name: "matching rule runs always", + rules: []model.Rule{ + {If: `$CI_COMMIT_BRANCH == "main"`, When: "always"}, + }, + branch: "main", + wantRuns: true, + }, + { + name: "matching rule when never", + rules: []model.Rule{ + {If: `$CI_COMMIT_BRANCH == "main"`, When: "never"}, + }, + branch: "main", + wantRuns: false, + }, + { + name: "no rule matched", + rules: []model.Rule{ + {If: `$CI_COMMIT_BRANCH == "main"`}, + }, + branch: "develop", + wantRuns: false, + }, + { + name: "matching rule with variables", + rules: []model.Rule{ + { + If: `$CI_COMMIT_BRANCH == "main"`, + When: "always", + Variables: map[string]any{ + "DEPLOY_TARGET": "production", + "ENVIRONMENT": "prod", + }, + }, + {When: "always"}, + }, + branch: "main", + wantRuns: true, + wantVars: map[string]string{ + "DEPLOY_TARGET": "production", + "ENVIRONMENT": "prod", + }, + }, + { + name: "fallback rule with different variables", + rules: []model.Rule{ + {If: `$CI_COMMIT_BRANCH == "main"`, Variables: map[string]any{"DEPLOY_TARGET": "production"}}, + {When: "always", Variables: map[string]any{"DEPLOY_TARGET": "staging"}}, + }, + branch: "develop", + wantRuns: true, + wantVars: map[string]string{"DEPLOY_TARGET": "staging"}, + }, + { + name: "empty context always runs", + rules: []model.Rule{ + {If: `$CI_COMMIT_BRANCH == "main"`, When: "never"}, + }, + branch: "", // no context + wantRuns: true, + wantVars: nil, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + var p *model.Pipeline + if tc.rules == nil { + p = &model.Pipeline{} + } else { + p = makePipeline(tc.rules) + } + ctx := New(tc.branch, "", "", nil) + runs, vars := EvalWorkflow(p, ctx) + if runs != tc.wantRuns { + t.Errorf("EvalWorkflow runs = %v, want %v", runs, tc.wantRuns) + } + for k, want := range tc.wantVars { + if got := vars[k]; got != want { + t.Errorf("EvalWorkflow vars[%q] = %q, want %q", k, got, want) + } + } + if len(vars) != len(tc.wantVars) { + t.Errorf("EvalWorkflow returned %d vars, want %d; got %v", len(vars), len(tc.wantVars), vars) + } + }) + } +} + +func TestContextInject(t *testing.T) { + t.Run("inject does not overwrite pinned var", func(t *testing.T) { + ctx := New("main", "", "", []string{"DEPLOY_TARGET=override"}) + ctx.Inject("DEPLOY_TARGET", "workflow-value") + if got := ctx.Get("DEPLOY_TARGET"); got != "override" { + t.Errorf("Inject overwrote pinned var: got %q, want %q", got, "override") + } + }) + t.Run("inject does not overwrite shortcut var", func(t *testing.T) { + ctx := New("main", "", "", nil) + ctx.Inject("CI_COMMIT_BRANCH", "other") + if got := ctx.Get("CI_COMMIT_BRANCH"); got != "main" { + t.Errorf("Inject overwrote shortcut var: got %q, want %q", got, "main") + } + }) + t.Run("inject sets new variable", func(t *testing.T) { + ctx := New("main", "", "", nil) + ctx.Inject("DEPLOY_TARGET", "production") + if got := ctx.Get("DEPLOY_TARGET"); got != "production" { + t.Errorf("Inject did not set variable: got %q", got) + } + }) + t.Run("inject later call overrides earlier call", func(t *testing.T) { + ctx := New("main", "", "", nil) + ctx.Inject("DEPLOY_TARGET", "pipeline-default") + ctx.Inject("DEPLOY_TARGET", "workflow-override") + if got := ctx.Get("DEPLOY_TARGET"); got != "workflow-override" { + t.Errorf("second Inject did not win: got %q, want %q", got, "workflow-override") + } + }) +} + +func TestExtractStringVars(t *testing.T) { + tests := []struct { + name string + in map[string]any + want map[string]string + }{ + { + name: "plain strings", + in: map[string]any{"A": "hello", "B": "world"}, + want: map[string]string{"A": "hello", "B": "world"}, + }, + { + name: "extended value form", + in: map[string]any{ + "KEY": map[string]any{"value": "extended", "description": "some desc"}, + }, + want: map[string]string{"KEY": "extended"}, + }, + { + name: "mixed forms", + in: map[string]any{ + "PLAIN": "str", + "COMPLEX": map[string]any{"value": "val"}, + }, + want: map[string]string{"PLAIN": "str", "COMPLEX": "val"}, + }, + { + name: "nil map", + in: nil, + want: nil, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := ExtractStringVars(tc.in) + for k, want := range tc.want { + if got[k] != want { + t.Errorf("ExtractStringVars[%q] = %q, want %q", k, got[k], want) + } + } + if len(got) != len(tc.want) { + t.Errorf("ExtractStringVars returned %d entries, want %d", len(got), len(tc.want)) + } + }) + } +} + +// TestWorkflowVarsJobEval verifies the end-to-end flow: workflow rule injects +// DEPLOY_TARGET, which is then used in a job's rules:if: expression. +func TestWorkflowVarsJobEval(t *testing.T) { + rules := []model.Rule{ + {If: `$CI_COMMIT_BRANCH == "main"`, Variables: map[string]any{"DEPLOY_TARGET": "production"}}, + {When: "always", Variables: map[string]any{"DEPLOY_TARGET": "staging"}}, + } + p := &model.Pipeline{ + Workflow: &model.Workflow{Rules: rules}, + Jobs: map[string]model.Job{ + "deploy-prod": { + Rules: []model.Rule{ + {If: `$DEPLOY_TARGET == "production"`, When: "on_success"}, + {When: "never"}, + }, + }, + "deploy-staging": { + Rules: []model.Rule{ + {If: `$DEPLOY_TARGET == "staging"`, When: "on_success"}, + {When: "never"}, + }, + }, + }, + } + + for _, tc := range []struct { + branch string + wantProd JobState + wantStaging JobState + }{ + {"main", JobActive, JobSkipped}, + {"develop", JobSkipped, JobActive}, + } { + ctx := New(tc.branch, "", "", nil) + _, ruleVars := EvalWorkflow(p, ctx) + for k, v := range ruleVars { + ctx.Inject(k, v) + } + if got := EvalJob(p.Jobs["deploy-prod"], ctx); got != tc.wantProd { + t.Errorf("branch=%q deploy-prod = %v, want %v", tc.branch, got, tc.wantProd) + } + if got := EvalJob(p.Jobs["deploy-staging"], ctx); got != tc.wantStaging { + t.Errorf("branch=%q deploy-staging = %v, want %v", tc.branch, got, tc.wantStaging) + } + } +} diff --git a/internal/model/pipeline.go b/internal/model/pipeline.go index b5869ed..7d4c37d 100644 --- a/internal/model/pipeline.go +++ b/internal/model/pipeline.go @@ -80,10 +80,11 @@ type Job struct { } type Rule struct { - If string `yaml:"if"` - When string `yaml:"when"` - Changes any `yaml:"changes"` // []string or {paths,compare_to} map - Exists any `yaml:"exists"` // []string or map form + If string `yaml:"if"` + When string `yaml:"when"` + Changes any `yaml:"changes"` // []string or {paths,compare_to} map + Exists any `yaml:"exists"` // []string or map form + Variables map[string]any `yaml:"variables"` // set/override variables when rule matches (GitLab CI 15.0+) } // ReservedKeys are top-level GitLab CI keys that are NOT job definitions. diff --git a/testdata/workflow_vars.yml b/testdata/workflow_vars.yml new file mode 100644 index 0000000..064126c --- /dev/null +++ b/testdata/workflow_vars.yml @@ -0,0 +1,51 @@ +--- +# workflow_vars.yml +# Exercises workflow:rules:variables: injection. +# The matching workflow rule sets DEPLOY_TARGET; job rules use it. +# +# Expected behaviour per context: +# (no context) → all jobs active (no context evaluation) +# --branch main → deploy-prod active, deploy-staging skipped +# --branch develop → deploy-prod skipped, deploy-staging active +# --branch feat/x → deploy-prod skipped, deploy-staging skipped (manual) + +stages: + - build + - deploy + +variables: + DEPLOY_TARGET: + value: "" + description: "Deployment target — set by workflow rules" + +workflow: + rules: + - if: '$CI_COMMIT_BRANCH == "main"' + variables: + DEPLOY_TARGET: production + - if: '$CI_COMMIT_BRANCH == "develop"' + variables: + DEPLOY_TARGET: staging + - when: always + +build: + stage: build + script: make build + rules: + - when: always + +deploy-prod: + stage: deploy + script: make deploy ENV=production + rules: + - if: '$DEPLOY_TARGET == "production"' + when: on_success + - when: never + +deploy-staging: + stage: deploy + script: make deploy ENV=staging + rules: + - if: '$DEPLOY_TARGET == "staging"' + when: on_success + - when: never