Define a feature flag set by a secret once in the job environment

José M. Requena Plensrequested to merge
gitlab-community/gitlab-org/gitlab-runner:jmrp-feature-flag-env-followup into main

What does this MR do?

Follows up !7630 (merged) with the four non-blocking suggestions Lachlan Grant made when he approved it:

  • Filter feature flags out of the base variables that come after the job variables, change item 4 of the GetAllVariables() comment, and test the secret case (!7630 (comment 4176773378)).
  • Add the three precedence cases that resolve to false (!7630 (comment 4176773762)).
  • Reverse the values in the Kubernetes case, so the expected value differs from the default value of FF_USE_FASTZIP (!7630 (comment 4176774560)).
  • Say in the docs that an empty or non-boolean first value means the default value (!7630 (comment 4176774181)).

Why was this MR needed?

With FF_NETWORK_PER_BUILD = false in [runners.feature_flags] and a job secret FF_NETWORK_PER_BUILD set to true, GetAllVariables() returned false and then true, so the job saw true while IsFeatureFlagOn reported false. It is the problem !7630 (merged) fixed for the environment setting, from another source.

The test changes make the precedence cases catch an implementation that turns a flag on whenever any source enables it, and the Kubernetes case one that ignores the job variable.

What's the best way to test this MR?

go test ./common/ -run 'TestGetAllVariables_FeatureFlagFrom'
go test ./executors/kubernetes/ -run 'TestSetupBuildPod'
What I ran
  • go vet and go test -race -count=1 on ./common/ and ./executors/kubernetes/, with Go 1.26.10 and TZ=UTC: pass. golangci-lint 2.14.0 on both packages: 0 issues.
  • Each new or changed test fails without its fix: TestGetAllVariables_FeatureFlagFromSecrets with the common/build.go of main, where the flag is defined twice, and the three false cases and the Kubernetes case with the common/build.go change of !7630 (merged) reverted, where the Kubernetes case gets true.
  • On docs/configuration/feature-flags.md, markdownlint-cli2 0.23.2, rumdl and lychee (offline): no issues. Vale 3.21.0: no error, the level CI checks, and no new alert at any level.
  • Not run: a real job with an external secret, which needs a secrets provider such as Vault.

What are the relevant issue numbers?

Related to #36893 (closed).

Merge request reports