Skip to content

conf: only allow updates to store via client

Warren Gifford requested to merge conf-only-update-store-via-client into main

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 updates conf.store with the new configuration
  • conf.client polls ConfigurationSource and gets the new update. However, it then compares it to conf.store, which has the new value from conf.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 to conf.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:

image

Merge request reports

Loading