From 4d7dcf10ed9ccf18d87c87b0fb1b1018c8bf8a17 Mon Sep 17 00:00:00 2001 From: Vincent Ambo Date: Sat, 22 Jan 2022 16:04:39 +0300 Subject: [PATCH] refactor(nix/buildkite): Restrict step conditionals to refs only The previous `condition` abstraction which allowed the full set of Buildkite conditionals is way too leaky (it lets users to very Buildkite-specific things which we may not want to allow, and which are mostly not relevant to a pure evaluation). Supporting only the `branches` condition (native to Buildkite) should make it possible to port this to other future CI systems later. Change-Id: Ib8adcc41db4f1a3566cbeecf13a4228403105c1f Reviewed-on: https://cl.tvl.fyi/c/depot/+/5051 Tested-by: BuildkiteCI Reviewed-by: sterni Reviewed-by: ezemtsov Autosubmit: tazjin --- nix/buildkite/default.nix | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/nix/buildkite/default.nix b/nix/buildkite/default.nix index 29c6a2dc0..6bf7feb8b 100644 --- a/nix/buildkite/default.nix +++ b/nix/buildkite/default.nix @@ -243,9 +243,8 @@ in rec { # command. Output will be available as 'result'. # TODO: Figure out multiple-output derivations. # - # condition (optional): Any other Buildkite condition, such as - # specific branch requirements, for this step. - # See https://buildkite.com/docs/pipelines/conditionals + # branches (optional): Git references (branches, tags ... ) on + # which this step should be allowed to run. List of strings. # # alwaysRun (optional): If set to true, this step will always run, # even if its parent has not been rebuilt. @@ -254,17 +253,16 @@ in rec { # Create a gated step in a step group, independent from any other # steps. - mkGatedStep = { step, label, parent, prompt, condition }: { + mkGatedStep = { step, label, parent, prompt }: { + inherit (step) branches depends_on; group = label; - depends_on = step.depends_on; skip = parent.skip or false; - "if" = condition; steps = [ { + inherit (step) branches; inherit prompt; block = ":radio_button: Run ${label}? (from ${parent.env.READTREE_TARGET})"; - "if" = condition; } # The explicit depends_on of the wrapped step must be removed, @@ -281,16 +279,16 @@ in rec { label ? key, prompt ? false, needsOutput ? false, - condition ? null, + branches ? null, alwaysRun ? false }@cfg: let parentLabel = parent.env.READTREE_TARGET; + step = { label = ":gear: ${label} (from ${parentLabel})"; skip = if alwaysRun then false else parent.skip or false; - "if" = condition; - depends_on = lib.optional (!alwaysRun && !needsOutput) parent.key; + branches = if branches != null then lib.concatStringsSep " " branches else null; command = pkgs.writeShellScript "${key}-script" '' set -ueo pipefail @@ -302,7 +300,7 @@ in rec { }; in if (isString prompt) then mkGatedStep { - inherit step label parent prompt condition; + inherit step label parent prompt; } else step; }