conf: only allow updates to store via client
Created by: bobheadxi
conf.client
is typically started with a goroutine that polls the frontend API for configuration updates. If an update is detected, conf.store
is updated with a cached value of the new configuration. Consumers of configuration get values from conf.store
instead of the frontend API
In the frontend itself, however, we start a conf.Server
and a conf.client
to be backed by a ConfigurationSource
, and both poll this ConfigurationSource
for updates. Both hold references to the same conf.store
, and both write the updates they receive from ConfigurationSource
to the same conf.store
. The key difference is that conf.client
will notify subscribers of conf.Watch
, and conf.Server
will not. This leads to the following scenario:
- Edit the site config in the UI
- GraphQL API calls
conf.Server.Write
-
conf.Server
has a channel internally that immediately procs its source poller, which updatesconf.store
with the new configuration -
conf.client
pollsConfigurationSource
and gets the new update. However, it then compares it toconf.store
, which has the new value fromconf.Server
, and detects no diff, causing watchers to not get notified.
This fix:
- removes
conf.Server
's polling routine entirely - removes
conf.Server
's access toconf.store
entirely - to preserve the fast-update behaviour (docstrings indicate this is important for tests),
conf.client
can now have a channel set,sourceUpdates
, that also procs its polling if non-nil - adds some more robust logging about
conf.client
updates
Closes https://github.com/sourcegraph/sourcegraph/issues/39079
Test plan
Main-dry-run: https://buildkite.com/sourcegraph/sourcegraph/builds?branch=main-dry-run/conf-only-update-store-via-client
Unit tests, added logging + local test of updating configuration: