diff options
| author | Patrick Steinhardt <psteinhardt@gitlab.com> | 2021-12-09 16:17:16 +0300 |
|---|---|---|
| committer | Patrick Steinhardt <psteinhardt@gitlab.com> | 2021-12-14 09:54:12 +0300 |
| commit | da9773ba7ad5c047e3d2346a3d348b95542b84ec (patch) | |
| tree | 95099a1ae379f227d3f456517a50bd7459ea2e96 | |
| parent | ef4f2154f36348321aed36ff2117d1dde96a2caa (diff) | |
featureflag: Fix setting incoming feature flags modifying parent context
When setting feature flags on a context, then the expectation is that it
will only be set for the child context which is returned from the
function. This doesn't work as expected though: the metadata we retrieve
from the current context is not a copy, so setting any new values on it
will modify the parent context, as well.
Fix this bug by copying the context before setting the key. Somehow,
this bug only seemed to impact incoming contexts: I wasn't able to
demonstrate that this is broken for outgoing contexts, even though they
should in theory suffer from the exact same problem. This commit fixes
the issue for both incoming and outgoing contexts regardless.
Changelog: fixed
| -rw-r--r-- | internal/metadata/featureflag/context.go | 2 | ||||
| -rw-r--r-- | internal/metadata/featureflag/context_test.go | 3 |
2 files changed, 3 insertions, 2 deletions
diff --git a/internal/metadata/featureflag/context.go b/internal/metadata/featureflag/context.go index 16caa2837..9d43bd744 100644 --- a/internal/metadata/featureflag/context.go +++ b/internal/metadata/featureflag/context.go @@ -33,6 +33,7 @@ func outgoingCtxWithFeatureFlag(ctx context.Context, key string, enabled bool) c md = metadata.New(map[string]string{}) } + md = md.Copy() md.Set(key, strconv.FormatBool(enabled)) return metadata.NewOutgoingContext(ctx, md) @@ -56,6 +57,7 @@ func incomingCtxWithFeatureFlag(ctx context.Context, key string, enabled bool) c md = metadata.New(map[string]string{}) } + md = md.Copy() md.Set(key, strconv.FormatBool(enabled)) return metadata.NewIncomingContext(ctx, md) diff --git a/internal/metadata/featureflag/context_test.go b/internal/metadata/featureflag/context_test.go index a6a93885e..b938aa06f 100644 --- a/internal/metadata/featureflag/context_test.go +++ b/internal/metadata/featureflag/context_test.go @@ -34,8 +34,7 @@ func TestIncomingCtxWithFeatureFlag(t *testing.T) { ctxB := IncomingCtxWithFeatureFlag(ctxA, ffB, true) require.True(t, ffA.IsEnabled(ctxA)) - // This is a bug: setting the feature flag on context B modifies context A. - require.True(t, ffB.IsEnabled(ctxA)) + require.False(t, ffB.IsEnabled(ctxA)) require.True(t, ffA.IsEnabled(ctxB)) require.True(t, ffB.IsEnabled(ctxB)) |