diff options
author | Patrick Steinhardt <psteinhardt@gitlab.com> | 2022-06-29 16:49:08 +0300 |
---|---|---|
committer | Patrick Steinhardt <psteinhardt@gitlab.com> | 2022-06-29 16:49:08 +0300 |
commit | 3e466e74e72b5b02c5da0b57bb771995f199efae (patch) | |
tree | 6fd68ef61bd2d14bd81c88819705f88a83f3e95d | |
parent | 06b6a8078f62fea99194b830894116dfcdaab745 (diff) |
command: Export environment variable to force-enable all feature flags
In Rails we're setting an environment variable that force-enables all
feature flags in Gitaly so that we can ensure that newly introduced code
hidden behind any flag is still compatible with the old way of doing
things in Rails. With a recent change though we have started to export
feature flags to the `gitaly-git2go` child process so that it knows
which flags have explicitly been enabled and/or disabled. In combination
both these features result to inconsistent state: we don't export the
environment variable to the child process, and neither do we inject the
force-enabled feature flags. This means that we'll now see all feature
flags as enabled in Gitaly, while we see the default values in the child
process.
Fix this by exporting the environment variable to child processes. It
should only ever be set for tests, so that should be a fine thing to do.
And by exporting it we now have the same and thus consistent view of
overridden feature flags.
Changelog: fixed
-rw-r--r-- | internal/command/command.go | 5 | ||||
-rw-r--r-- | internal/metadata/featureflag/featureflag.go | 6 |
2 files changed, 10 insertions, 1 deletions
diff --git a/internal/command/command.go b/internal/command/command.go index abf008f3d..5f3f3a8f1 100644 --- a/internal/command/command.go +++ b/internal/command/command.go @@ -105,6 +105,11 @@ var ( // libcurl settings: https://curl.haxx.se/libcurl/c/CURLOPT_NOPROXY.html "no_proxy", "NO_PROXY", + + // We must export this variable to child processes or otherwise we end up in + // an inconsistent state, where the parent process has all feature flags + // force-enabled while the child is using the usual defaults. + featureflag.EnableAllFeatureFlagsEnvVar, } // envInjector is responsible for injecting environment variables required for tracing into diff --git a/internal/metadata/featureflag/featureflag.go b/internal/metadata/featureflag/featureflag.go index 9607c9b55..edb0f8b16 100644 --- a/internal/metadata/featureflag/featureflag.go +++ b/internal/metadata/featureflag/featureflag.go @@ -13,12 +13,16 @@ import ( ) var ( + // EnableAllFeatureFlagsEnvVar will cause Gitaly to treat all feature flags as + // enabled in case its value is set to `true`. Only used for testing purposes. + EnableAllFeatureFlagsEnvVar = "GITALY_TESTING_ENABLE_ALL_FEATURE_FLAGS" + // featureFlagsOverride allows to enable all feature flags with a // single environment variable. If the value of // GITALY_TESTING_ENABLE_ALL_FEATURE_FLAGS is set to "true", then all // feature flags will be enabled. This is only used for testing // purposes such that we can run integration tests with feature flags. - featureFlagsOverride, _ = env.GetBool("GITALY_TESTING_ENABLE_ALL_FEATURE_FLAGS", false) + featureFlagsOverride, _ = env.GetBool(EnableAllFeatureFlagsEnvVar, false) flagChecks = promauto.NewCounterVec( prometheus.CounterOpts{ |