Define each feature flag once in the job environment

What does this MR do?

It keeps the feature flags set in the runner's environment setting out of the base variables that GetAllVariables returns, the same way it already keeps them out of the job variables. Each feature flag is then defined once in the job environment, with the value the runner resolved for it.

It is five commits:

  1. A test that pins what GetAllVariables does with the runner environment today: an ordinary variable, and one named like a feature flag that no feature flag has, each reach it once. Nothing tested that before.
  2. getNonFeatureFlagJobVariables becomes withoutFeatureFlags, which takes the list to filter. No change in behavior.
  3. The fix: GetAllVariables filters the feature flags out of getBaseVariablesBeforeJob(). That function itself is unchanged, because the feature flag resolution reads the runner environment through it. Tests in common, and a TestSetupBuildPod case for the Kubernetes build container.
  4. The feature flags page states the order in which the places that set a feature flag take precedence, and that a feature flag only usable in [runners.feature_flags] takes its value from that section or its default value.
  5. The two suggestions from the documentation review, applied as proposed.

Why was this MR needed?

Since !5782 (merged), GetAllVariables starts with every feature flag at its resolved value and drops feature flags from the job variables, so a flag set in .gitlab-ci.yml, the case the issue describes, is defined once. The runner environment setting, the case reported later in the issue thread, was not filtered: getBaseVariablesBeforeJob appends Runner.GetVariables(), so environment = ["FF_X=..."] was defined a second time, after the resolved value, with its raw value.

Whatever reads the job environment in order keeps the last definition. The job script exports every variable in order (exportVariables in shells/abstract.go), and the Kubernetes executor has dropped duplicate names keeping the last one since !5325 (merged), so its container specs no longer define a name twice, which is what the warnings quoted in the issue came from. Both therefore give the job, and the helper commands its script runs, the runner environment's value even where the runner uses another one:

  • [runners.feature_flags] sets FF_USE_FASTZIP = true and environment sets FF_USE_FASTZIP=false: the runner resolves true, the job environment ends with false.
  • environment sets FF_USE_FASTZIP=false and a job sets the variable FF_USE_FASTZIP: "true": a job variable takes precedence over the runner environment, so the runner resolves true, and the job environment still ends with false.

The helper reads some flags from that environment. The artifacts upload stage exports the job variables and then runs the helper, which picks its archiver from FF_USE_FASTZIP (registerArchivers in commands/helpers/archiver.go), so in the second case the job's opt-in did not reach it. I have shown this with the tests below, not with a real job.

What's the best way to test this MR?

go test ./common/ -run 'TestGetAllVariables|TestIsFeatureFlagOn' -count=1 -v
go test ./executors/kubernetes/ -run 'TestSetupBuildPod/passes_the_resolved_value' -count=1 -v

The tests the third commit adds fail on the second commit and pass on the third. I also ran on each commit go build ./..., go vet and go test for ./common/ and ./executors/kubernetes/; on the last one go test ./common/... ./executors/... ./shells/... ./helpers/featureflags/..., go test -race ./common/ ./executors/kubernetes/, the feature flag tests with RUNNER_TEST_FEATURE_FLAGS set as the CI jobs set it, golangci-lint 2.12.2 from the CI runner-linters:2.12.2-go1.26.5 image on both packages (no issues), the check-test-directives script, and markdownlint-cli2 0.23.2 and Vale 3.21.0 on the docs page (no error or warning in the new section).

The third commit's tests on the second commit (excerpt)
Error:  "[FF_NETWORK_PER_BUILD=true FF_NETWORK_PER_BUILD=false]" should have 1 item(s), but has 2
Error:  "[FF_NETWORK_PER_BUILD=true FF_NETWORK_PER_BUILD=1]" should have 1 item(s), but has 2
--- FAIL: TestGetAllVariables_FeatureFlagFromRunnerEnvironment (0.00s)
    --- FAIL: TestGetAllVariables_FeatureFlagFromRunnerEnvironment/TOML_overrides_the_runner_environment (0.00s)
    --- FAIL: TestGetAllVariables_FeatureFlagFromRunnerEnvironment/job_variable_overrides_the_runner_environment (0.00s)
    --- FAIL: TestGetAllVariables_FeatureFlagFromRunnerEnvironment/runner_environment_enables_the_flag (0.00s)
    --- FAIL: TestGetAllVariables_FeatureFlagFromRunnerEnvironment/runner_environment_enables_the_flag_with_1 (0.00s)

Error:  Not equal:
        expected: []string{"true"}
        actual  : []string{"false"}
--- FAIL: TestSetupBuildPod (0.00s)
    --- FAIL: TestSetupBuildPod/passes_the_resolved_value_of_a_feature_flag_set_in_the_runner_environment (0.00s)

What are the relevant issue numbers?

Closes #36893

Edited by José M. Requena Plens

Merge request reports

Loading
Loading