From e25a2f5af46b36c01b2c66ece9d31f5a819743cf Mon Sep 17 00:00:00 2001
From: Steven Masley <stevenmasley@gmail.com>
Date: Thu, 5 Sep 2024 10:35:32 -0500
Subject: [PATCH 1/8] chore: refactor entry into deployment and runtime

RuntimeEntry has no startup value, and omits functions required
to be serpent compatible.
---
 coderd/runtimeconfig/config.go          | 161 ----------------
 coderd/runtimeconfig/config_test.go     | 238 ------------------------
 coderd/runtimeconfig/deploymententry.go |  86 +++++++++
 coderd/runtimeconfig/entry.go           |  94 ++++++++++
 coderd/runtimeconfig/entry_test.go      | 133 +++++++++++++
 5 files changed, 313 insertions(+), 399 deletions(-)
 delete mode 100644 coderd/runtimeconfig/config.go
 delete mode 100644 coderd/runtimeconfig/config_test.go
 create mode 100644 coderd/runtimeconfig/deploymententry.go
 create mode 100644 coderd/runtimeconfig/entry.go
 create mode 100644 coderd/runtimeconfig/entry_test.go

diff --git a/coderd/runtimeconfig/config.go b/coderd/runtimeconfig/config.go
deleted file mode 100644
index 25017e663dc1a..0000000000000
--- a/coderd/runtimeconfig/config.go
+++ /dev/null
@@ -1,161 +0,0 @@
-package runtimeconfig
-
-import (
-	"context"
-	"errors"
-	"reflect"
-
-	"github.com/spf13/pflag"
-	"golang.org/x/xerrors"
-)
-
-var ErrNameNotSet = xerrors.New("name is not set")
-
-// Value wraps the type used by the serpent library for its option values.
-// This gives us a seam should serpent ever move away from its current implementation.
-type Value pflag.Value
-
-// Entry is designed to wrap any type which satisfies the Value interface, which currently all serpent.Option instances do.
-// serpent.Option provide configurability to Value instances, and we use this Entry type to extend the functionality of
-// those Value instances.
-// An Entry has a "name" which is used to identify it in the store.
-type Entry[T Value] struct {
-	n     string
-	inner T
-}
-
-// New creates a new T instance with a defined name and value.
-func New[T Value](name, val string) (out Entry[T], err error) {
-	out.n = name
-
-	if err = out.SetStartupValue(val); err != nil {
-		return out, err
-	}
-
-	return out, nil
-}
-
-// MustNew is like New but panics if an error occurs.
-func MustNew[T Value](name, val string) Entry[T] {
-	out, err := New[T](name, val)
-	if err != nil {
-		panic(err)
-	}
-	return out
-}
-
-// Initialize sets the entry's name, and initializes the value.
-func (e *Entry[T]) Initialize(name string) {
-	e.n = name
-	e.val()
-}
-
-// val fronts the T value in the struct, and initializes it should the value be nil.
-func (e *Entry[T]) val() T {
-	if reflect.ValueOf(e.inner).IsNil() {
-		e.inner = create[T]()
-	}
-	return e.inner
-}
-
-// name returns the configured name, or fails with ErrNameNotSet.
-func (e *Entry[T]) name() (string, error) {
-	if e.n == "" {
-		return "", ErrNameNotSet
-	}
-
-	return e.n, nil
-}
-
-// Set is an alias of SetStartupValue.
-func (e *Entry[T]) Set(s string) error {
-	return e.SetStartupValue(s)
-}
-
-// MustSet is like Set but panics on error.
-func (e *Entry[T]) MustSet(s string) {
-	err := e.val().Set(s)
-	if err != nil {
-		panic(err)
-	}
-}
-
-// SetStartupValue sets the value of the wrapped field. This ONLY sets the value locally, not in the store.
-// See SetRuntimeValue.
-func (e *Entry[T]) SetStartupValue(s string) error {
-	return e.val().Set(s)
-}
-
-// Type returns the wrapped value's type.
-func (e *Entry[T]) Type() string {
-	return e.val().Type()
-}
-
-// String returns the wrapper value's string representation.
-func (e *Entry[T]) String() string {
-	return e.val().String()
-}
-
-// StartupValue returns the wrapped type T which represents the state as of the definition of this Entry.
-// This function would've been named Value, but this conflicts with a field named Value on some implementations of T in
-// the serpent library; plus it's just more clear.
-func (e *Entry[T]) StartupValue() T {
-	return e.val()
-}
-
-// SetRuntimeValue attempts to update the runtime value of this field in the store via the given Mutator.
-func (e *Entry[T]) SetRuntimeValue(ctx context.Context, m Manager, val T) error {
-	name, err := e.name()
-	if err != nil {
-		return err
-	}
-
-	return m.UpsertRuntimeSetting(ctx, name, val.String())
-}
-
-// UnsetRuntimeValue removes the runtime value from the store.
-func (e *Entry[T]) UnsetRuntimeValue(ctx context.Context, m Manager) error {
-	name, err := e.name()
-	if err != nil {
-		return err
-	}
-
-	return m.DeleteRuntimeSetting(ctx, name)
-}
-
-// Resolve attempts to resolve the runtime value of this field from the store via the given Resolver.
-func (e *Entry[T]) Resolve(ctx context.Context, r Manager) (T, error) {
-	var zero T
-
-	name, err := e.name()
-	if err != nil {
-		return zero, err
-	}
-
-	val, err := r.GetRuntimeSetting(ctx, name)
-	if err != nil {
-		return zero, err
-	}
-
-	inst := create[T]()
-	if err = inst.Set(val); err != nil {
-		return zero, xerrors.Errorf("instantiate new %T: %w", inst, err)
-	}
-	return inst, nil
-}
-
-// Coalesce attempts to resolve the runtime value of this field from the store via the given Manager. Should no runtime
-// value be found, the startup value will be used.
-func (e *Entry[T]) Coalesce(ctx context.Context, r Manager) (T, error) {
-	var zero T
-
-	resolved, err := e.Resolve(ctx, r)
-	if err != nil {
-		if errors.Is(err, EntryNotFound) {
-			return e.StartupValue(), nil
-		}
-		return zero, err
-	}
-
-	return resolved, nil
-}
diff --git a/coderd/runtimeconfig/config_test.go b/coderd/runtimeconfig/config_test.go
deleted file mode 100644
index 4f045dbdd237d..0000000000000
--- a/coderd/runtimeconfig/config_test.go
+++ /dev/null
@@ -1,238 +0,0 @@
-package runtimeconfig_test
-
-import (
-	"context"
-	"testing"
-
-	"github.com/google/uuid"
-	"github.com/stretchr/testify/require"
-
-	"github.com/coder/serpent"
-
-	"github.com/coder/coder/v2/coderd/database/dbmem"
-	"github.com/coder/coder/v2/coderd/runtimeconfig"
-	"github.com/coder/coder/v2/coderd/util/ptr"
-	"github.com/coder/coder/v2/testutil"
-)
-
-func TestUsage(t *testing.T) {
-	t.Parallel()
-
-	t.Run("deployment value without runtimeconfig", func(t *testing.T) {
-		t.Parallel()
-
-		var field serpent.StringArray
-		opt := serpent.Option{
-			Name:        "my deployment value",
-			Description: "this mimicks an option we'd define in codersdk/deployment.go",
-			Env:         "MY_DEPLOYMENT_VALUE",
-			Default:     "pestle,mortar",
-			Value:       &field,
-		}
-
-		set := serpent.OptionSet{opt}
-		require.NoError(t, set.SetDefaults())
-		require.Equal(t, []string{"pestle", "mortar"}, field.Value())
-	})
-
-	t.Run("deployment value with runtimeconfig", func(t *testing.T) {
-		t.Parallel()
-
-		ctx := testutil.Context(t, testutil.WaitShort)
-		mgr := runtimeconfig.NewStoreManager(dbmem.New())
-
-		// NOTE: this field is now wrapped
-		var field runtimeconfig.Entry[*serpent.HostPort]
-		opt := serpent.Option{
-			Name:        "my deployment value",
-			Description: "this mimicks an option we'd define in codersdk/deployment.go",
-			Env:         "MY_DEPLOYMENT_VALUE",
-			Default:     "localhost:1234",
-			Value:       &field,
-		}
-
-		set := serpent.OptionSet{opt}
-		require.NoError(t, set.SetDefaults())
-
-		// The value has to now be retrieved from a StartupValue() call.
-		require.Equal(t, "localhost:1234", field.StartupValue().String())
-
-		// One new constraint is that we have to set the name on the runtimeconfig.Entry.
-		// Attempting to perform any operation which accesses the store will enforce the need for a name.
-		_, err := field.Resolve(ctx, mgr)
-		require.ErrorIs(t, err, runtimeconfig.ErrNameNotSet)
-
-		// Let's set that name; the environment var name is likely to be the most stable.
-		field.Initialize(opt.Env)
-
-		newVal := serpent.HostPort{Host: "12.34.56.78", Port: "1234"}
-		// Now that we've set it, we can update the runtime value of this field, which modifies given store.
-		require.NoError(t, field.SetRuntimeValue(ctx, mgr, &newVal))
-
-		// ...and we can retrieve the value, as well.
-		resolved, err := field.Resolve(ctx, mgr)
-		require.NoError(t, err)
-		require.Equal(t, newVal.String(), resolved.String())
-
-		// We can also remove the runtime config.
-		require.NoError(t, field.UnsetRuntimeValue(ctx, mgr))
-	})
-}
-
-// TestConfig demonstrates creating org-level overrides for deployment-level settings.
-func TestConfig(t *testing.T) {
-	t.Parallel()
-
-	t.Run("new", func(t *testing.T) {
-		t.Parallel()
-
-		require.Panics(t, func() {
-			// "hello" cannot be set on a *serpent.Float64 field.
-			runtimeconfig.MustNew[*serpent.Float64]("my-field", "hello")
-		})
-
-		require.NotPanics(t, func() {
-			runtimeconfig.MustNew[*serpent.Float64]("my-field", "91.1234")
-		})
-	})
-
-	t.Run("zero", func(t *testing.T) {
-		t.Parallel()
-
-		mgr := runtimeconfig.NewNoopManager()
-
-		// A zero-value declaration of a runtimeconfig.Entry should behave as a zero value of the generic type.
-		// NB! A name has not been set for this entry; it is "uninitialized".
-		var field runtimeconfig.Entry[*serpent.Bool]
-		var zero serpent.Bool
-		require.Equal(t, field.StartupValue().Value(), zero.Value())
-
-		// Setting a value will not produce an error.
-		require.NoError(t, field.SetStartupValue("true"))
-
-		// But attempting to resolve will produce an error.
-		_, err := field.Resolve(context.Background(), mgr)
-		require.ErrorIs(t, err, runtimeconfig.ErrNameNotSet)
-		// But attempting to set the runtime value will produce an error.
-		val := serpent.BoolOf(ptr.Ref(true))
-		require.ErrorIs(t, field.SetRuntimeValue(context.Background(), mgr, val), runtimeconfig.ErrNameNotSet)
-	})
-
-	t.Run("simple", func(t *testing.T) {
-		t.Parallel()
-
-		ctx := testutil.Context(t, testutil.WaitShort)
-		mgr := runtimeconfig.NewStoreManager(dbmem.New())
-
-		var (
-			base     = serpent.String("system@dev.coder.com")
-			override = serpent.String("dogfood@dev.coder.com")
-		)
-
-		field := runtimeconfig.MustNew[*serpent.String]("my-field", base.String())
-		// Check that default has been set.
-		require.Equal(t, base.String(), field.StartupValue().String())
-		// Validate that it returns that value.
-		require.Equal(t, base.String(), field.String())
-		// Validate that there is no org-level override right now.
-		_, err := field.Resolve(ctx, mgr)
-		require.ErrorIs(t, err, runtimeconfig.EntryNotFound)
-		// Coalesce returns the deployment-wide value.
-		val, err := field.Coalesce(ctx, mgr)
-		require.NoError(t, err)
-		require.Equal(t, base.String(), val.String())
-		// Set an org-level override.
-		require.NoError(t, field.SetRuntimeValue(ctx, mgr, &override))
-		// Coalesce now returns the org-level value.
-		val, err = field.Coalesce(ctx, mgr)
-		require.NoError(t, err)
-		require.Equal(t, override.String(), val.String())
-	})
-
-	t.Run("complex", func(t *testing.T) {
-		t.Parallel()
-
-		ctx := testutil.Context(t, testutil.WaitShort)
-		mgr := runtimeconfig.NewStoreManager(dbmem.New())
-
-		var (
-			base = serpent.Struct[map[string]string]{
-				Value: map[string]string{"access_type": "offline"},
-			}
-			override = serpent.Struct[map[string]string]{
-				Value: map[string]string{
-					"a": "b",
-					"c": "d",
-				},
-			}
-		)
-
-		field := runtimeconfig.MustNew[*serpent.Struct[map[string]string]]("my-field", base.String())
-
-		// Check that default has been set.
-		require.Equal(t, base.String(), field.StartupValue().String())
-		// Validate that there is no org-level override right now.
-		_, err := field.Resolve(ctx, mgr)
-		require.ErrorIs(t, err, runtimeconfig.EntryNotFound)
-		// Coalesce returns the deployment-wide value.
-		val, err := field.Coalesce(ctx, mgr)
-		require.NoError(t, err)
-		require.Equal(t, base.Value, val.Value)
-		// Set an org-level override.
-		require.NoError(t, field.SetRuntimeValue(ctx, mgr, &override))
-		// Coalesce now returns the org-level value.
-		structVal, err := field.Resolve(ctx, mgr)
-		require.NoError(t, err)
-		require.Equal(t, override.Value, structVal.Value)
-	})
-}
-
-func TestScoped(t *testing.T) {
-	t.Parallel()
-
-	orgID := uuid.New()
-
-	ctx := testutil.Context(t, testutil.WaitShort)
-
-	// Set up a config manager and a field which will have runtime configs.
-	mgr := runtimeconfig.NewStoreManager(dbmem.New())
-	field := runtimeconfig.MustNew[*serpent.HostPort]("addr", "localhost:3000")
-
-	// No runtime value set at this point, Coalesce will return startup value.
-	_, err := field.Resolve(ctx, mgr)
-	require.ErrorIs(t, err, runtimeconfig.EntryNotFound)
-	val, err := field.Coalesce(ctx, mgr)
-	require.NoError(t, err)
-	require.Equal(t, field.StartupValue().String(), val.String())
-
-	// Set a runtime value which is NOT org-scoped.
-	host, port := "localhost", "1234"
-	require.NoError(t, field.SetRuntimeValue(ctx, mgr, &serpent.HostPort{Host: host, Port: port}))
-	val, err = field.Resolve(ctx, mgr)
-	require.NoError(t, err)
-	require.Equal(t, host, val.Host)
-	require.Equal(t, port, val.Port)
-
-	orgMgr := mgr.Scoped(orgID.String())
-	// Using the org scope, nothing will be returned.
-	_, err = field.Resolve(ctx, orgMgr)
-	require.ErrorIs(t, err, runtimeconfig.EntryNotFound)
-
-	// Now set an org-scoped value.
-	host, port = "localhost", "4321"
-	require.NoError(t, field.SetRuntimeValue(ctx, orgMgr, &serpent.HostPort{Host: host, Port: port}))
-	val, err = field.Resolve(ctx, orgMgr)
-	require.NoError(t, err)
-	require.Equal(t, host, val.Host)
-	require.Equal(t, port, val.Port)
-
-	// Ensure the two runtime configs are NOT equal to each other nor the startup value.
-	global, err := field.Resolve(ctx, mgr)
-	require.NoError(t, err)
-	org, err := field.Resolve(ctx, orgMgr)
-	require.NoError(t, err)
-
-	require.NotEqual(t, global.String(), org.String())
-	require.NotEqual(t, field.StartupValue().String(), global.String())
-	require.NotEqual(t, field.StartupValue().String(), org.String())
-}
diff --git a/coderd/runtimeconfig/deploymententry.go b/coderd/runtimeconfig/deploymententry.go
new file mode 100644
index 0000000000000..896a05caf53f0
--- /dev/null
+++ b/coderd/runtimeconfig/deploymententry.go
@@ -0,0 +1,86 @@
+package runtimeconfig
+
+import (
+	"context"
+	"errors"
+	"reflect"
+
+	"github.com/spf13/pflag"
+)
+
+// Ensure serpent values satisfy the ConfigValue interface for easier usage.
+var _ pflag.Value = SerpentEntry(nil)
+var _ pflag.Value = &DeploymentEntry[SerpentEntry]{}
+
+type SerpentEntry interface {
+	EntryValue
+	Type() string
+}
+
+// DeploymentEntry extends a runtime entry with a startup value.
+// This allows for a single entry to source its value from startup or runtime.
+type DeploymentEntry[T SerpentEntry] struct {
+	RuntimeEntry[T]
+	startupValue T
+}
+
+// Initialize sets the entry's name, and initializes the value.
+func (e *DeploymentEntry[T]) Initialize(name string) {
+	e.n = name
+	e.val()
+}
+
+// SetStartupValue sets the value of the wrapped field. This ONLY sets the value locally, not in the store.
+// See SetRuntimeValue.
+func (e *DeploymentEntry[T]) SetStartupValue(s string) error {
+	return e.val().Set(s)
+}
+
+// StartupValue returns the wrapped type T which represents the state as of the definition of this Entry.
+// This function would've been named Value, but this conflicts with a field named Value on some implementations of T in
+// the serpent library; plus it's just more clear.
+func (e *DeploymentEntry[T]) StartupValue() T {
+	return e.val()
+}
+
+// Coalesce attempts to resolve the runtime value of this field from the store via the given Manager. Should no runtime
+// value be found, the startup value will be used.
+func (e *DeploymentEntry[T]) Coalesce(ctx context.Context, r Manager) (T, error) {
+	var zero T
+
+	resolved, err := e.Resolve(ctx, r)
+	if err != nil {
+		if errors.Is(err, EntryNotFound) {
+			return e.StartupValue(), nil
+		}
+		return zero, err
+	}
+
+	return resolved, nil
+}
+
+// Functions to implement pflag.Value for serpent usage
+
+// Set is an alias of SetStartupValue. Implemented to match the serpent interface
+// such that we can use this Go type in the OptionSet.
+func (e *DeploymentEntry[T]) Set(s string) error {
+	return e.SetStartupValue(s)
+}
+
+// Type returns the wrapped value's type.
+func (e *DeploymentEntry[T]) Type() string {
+	return e.val().Type()
+}
+
+// String returns the wrapper value's string representation.
+func (e *DeploymentEntry[T]) String() string {
+	return e.val().String()
+}
+
+// val fronts the T value in the struct, and initializes it should the value be nil.
+func (e *DeploymentEntry[T]) val() T {
+	if reflect.ValueOf(e.startupValue).IsNil() {
+		e.startupValue = create[T]()
+	}
+	return e.startupValue
+}
diff --git a/coderd/runtimeconfig/entry.go b/coderd/runtimeconfig/entry.go
new file mode 100644
index 0000000000000..ecb8a67a3fe92
--- /dev/null
+++ b/coderd/runtimeconfig/entry.go
@@ -0,0 +1,94 @@
+package runtimeconfig
+
+import (
+	"context"
+	"fmt"
+
+	"golang.org/x/xerrors"
+)
+
+var ErrNameNotSet = xerrors.New("name is not set")
+
+type EntryMarshaller interface {
+	fmt.Stringer
+}
+
+type EntryValue interface {
+	EntryMarshaller
+	Set(string) error
+}
+
+// RuntimeEntry are **only** runtime configurable. They are stored in the
+// database, and have no startup value or default value.
+type RuntimeEntry[T EntryValue] struct {
+	n string
+}
+
+// New creates a new T instance with a defined name and value.
+func New[T EntryValue](name string) (out RuntimeEntry[T], err error) {
+	out.n = name
+	if name == "" {
+		return out, ErrNameNotSet
+	}
+
+	return out, nil
+}
+
+// MustNew is like New but panics if an error occurs.
+func MustNew[T EntryValue](name string) RuntimeEntry[T] {
+	out, err := New[T](name)
+	if err != nil {
+		panic(err)
+	}
+	return out
+}
+
+// SetRuntimeValue attempts to update the runtime value of this field in the store via the given Mutator.
+func (e *RuntimeEntry[T]) SetRuntimeValue(ctx context.Context, m Manager, val T) error {
+	name, err := e.name()
+	if err != nil {
+		return err
+	}
+
+	return m.UpsertRuntimeSetting(ctx, name, val.String())
+}
+
+// UnsetRuntimeValue removes the runtime value from the store.
+func (e *RuntimeEntry[T]) UnsetRuntimeValue(ctx context.Context, m Manager) error {
+	name, err := e.name()
+	if err != nil {
+		return err
+	}
+
+	return m.DeleteRuntimeSetting(ctx, name)
+}
+
+// Resolve attempts to resolve the runtime value of this field from the store via the given Resolver.
+func (e *RuntimeEntry[T]) Resolve(ctx context.Context, r Manager) (T, error) {
+	var zero T
+
+	name, err := e.name()
+	if err != nil {
+		return zero, err
+	}
+
+	val, err := r.GetRuntimeSetting(ctx, name)
+	if err != nil {
+		return zero, err
+	}
+
+	inst := create[T]()
+	if err = inst.Set(val); err != nil {
+		return zero, xerrors.Errorf("instantiate new %T: %w", inst, err)
+	}
+	return inst, nil
+}
+
+// name returns the configured name, or fails with ErrNameNotSet.
+func (e *RuntimeEntry[T]) name() (string, error) {
+	if e.n == "" {
+		return "", ErrNameNotSet
+	}
+
+	return e.n, nil
+}
diff --git a/coderd/runtimeconfig/entry_test.go b/coderd/runtimeconfig/entry_test.go
new file mode 100644
index 0000000000000..a184ff52566b0
--- /dev/null
+++ b/coderd/runtimeconfig/entry_test.go
@@ -0,0 +1,133 @@
+package runtimeconfig_test
+
+import (
+	"context"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+
+	"github.com/coder/coder/v2/coderd/database/dbmem"
+	"github.com/coder/coder/v2/coderd/runtimeconfig"
+	"github.com/coder/coder/v2/coderd/util/ptr"
+	"github.com/coder/coder/v2/testutil"
+	"github.com/coder/serpent"
+)
+
+// TestEntry demonstrates creating org-level overrides for deployment-level settings.
+func TestEntry(t *testing.T) {
+	t.Parallel()
+
+	t.Run("new", func(t *testing.T) {
+		t.Parallel()
+
+		require.Panics(t, func() {
+			// No name should panic
+			runtimeconfig.MustNew[*serpent.Float64]("")
+		})
+
+		require.NotPanics(t, func() {
+			runtimeconfig.MustNew[*serpent.Float64]("my-field")
+		})
+
+		{
+			var field runtimeconfig.DeploymentEntry[*serpent.Float64]
+			field.Initialize("my-field")
+			// "hello" cannot be set on a *serpent.Float64 field.
+			require.Error(t, field.Set("hello"))
+		}
+	})
+
+	t.Run("zero", func(t *testing.T) {
+		t.Parallel()
+
+		mgr := runtimeconfig.NewNoopManager()
+
+		// A zero-value declaration of a runtimeconfig.Entry should behave as a zero value of the generic type.
+		// NB! A name has not been set for this entry; it is "uninitialized".
+		var field runtimeconfig.DeploymentEntry[*serpent.Bool]
+		var zero serpent.Bool
+		require.Equal(t, field.StartupValue().Value(), zero.Value())
+
+		// Setting a value will not produce an error.
+		require.NoError(t, field.SetStartupValue("true"))
+
+		// But attempting to resolve will produce an error.
+		_, err := field.Resolve(context.Background(), mgr)
+		require.ErrorIs(t, err, runtimeconfig.ErrNameNotSet)
+		// But attempting to set the runtime value will produce an error.
+		val := serpent.BoolOf(ptr.Ref(true))
+		require.ErrorIs(t, field.SetRuntimeValue(context.Background(), mgr, val), runtimeconfig.ErrNameNotSet)
+	})
+
+	t.Run("simple", func(t *testing.T) {
+		t.Parallel()
+
+		ctx := testutil.Context(t, testutil.WaitShort)
+		mgr := runtimeconfig.NewStoreManager(dbmem.New())
+
+		var (
+			base     = serpent.String("system@dev.coder.com")
+			override = serpent.String("dogfood@dev.coder.com")
+		)
+
+		var field runtimeconfig.DeploymentEntry[*serpent.String]
+		field.Initialize("my-field")
+		field.SetStartupValue(base.String())
+		// Check that default has been set.
+		require.Equal(t, base.String(), field.StartupValue().String())
+		// Validate that it returns that value.
+		require.Equal(t, base.String(), field.String())
+		// Validate that there is no org-level override right now.
+		_, err := field.Resolve(ctx, mgr)
+		require.ErrorIs(t, err, runtimeconfig.EntryNotFound)
+		// Coalesce returns the deployment-wide value.
+		val, err := field.Coalesce(ctx, mgr)
+		require.NoError(t, err)
+		require.Equal(t, base.String(), val.String())
+		// Set an org-level override.
+		require.NoError(t, field.SetRuntimeValue(ctx, mgr, &override))
+		// Coalesce now returns the org-level value.
+		val, err = field.Coalesce(ctx, mgr)
+		require.NoError(t, err)
+		require.Equal(t, override.String(), val.String())
+	})
+
+	t.Run("complex", func(t *testing.T) {
+		t.Parallel()
+
+		ctx := testutil.Context(t, testutil.WaitShort)
+		mgr := runtimeconfig.NewStoreManager(dbmem.New())
+
+		var (
+			base = serpent.Struct[map[string]string]{
+				Value: map[string]string{"access_type": "offline"},
+			}
+			override = serpent.Struct[map[string]string]{
+				Value: map[string]string{
+					"a": "b",
+					"c": "d",
+				},
+			}
+		)
+
+		var field runtimeconfig.DeploymentEntry[*serpent.Struct[map[string]string]]
+		field.Initialize("my-field")
+		field.SetStartupValue(base.String())
+
+		// Check that default has been set.
+		require.Equal(t, base.String(), field.StartupValue().String())
+		// Validate that there is no org-level override right now.
+		_, err := field.Resolve(ctx, mgr)
+		require.ErrorIs(t, err, runtimeconfig.EntryNotFound)
+		// Coalesce returns the deployment-wide value.
+		val, err := field.Coalesce(ctx, mgr)
+		require.NoError(t, err)
+		require.Equal(t, base.Value, val.Value)
+		// Set an org-level override.
+		require.NoError(t, field.SetRuntimeValue(ctx, mgr, &override))
+		// Coalesce now returns the org-level value.
+		structVal, err := field.Resolve(ctx, mgr)
+		require.NoError(t, err)
+		require.Equal(t, override.Value, structVal.Value)
+	})
+}

From a47156df7030fe98da6fd3b581fe42f5035781b2 Mon Sep 17 00:00:00 2001
From: Steven Masley <stevenmasley@gmail.com>
Date: Thu, 5 Sep 2024 11:12:50 -0500
Subject: [PATCH 2/8] refactor resolvers and managers

---
 coderd/runtimeconfig/cache.go    |  22 +++++
 coderd/runtimeconfig/entry.go    |   6 +-
 coderd/runtimeconfig/manager.go  |  79 ++++++------------
 coderd/runtimeconfig/resolver.go | 139 +++++++++++++++++++++++++++++++
 coderd/runtimeconfig/spec.go     |  13 ++-
 5 files changed, 198 insertions(+), 61 deletions(-)
 create mode 100644 coderd/runtimeconfig/cache.go
 create mode 100644 coderd/runtimeconfig/resolver.go

diff --git a/coderd/runtimeconfig/cache.go b/coderd/runtimeconfig/cache.go
new file mode 100644
index 0000000000000..7798d356be065
--- /dev/null
+++ b/coderd/runtimeconfig/cache.go
@@ -0,0 +1,22 @@
+package runtimeconfig
+
+import (
+	"sync"
+	"time"
+)
+
+type memoryCache struct {
+	stale time.Duration
+	mu    sync.Mutex
+}
+
+func newMemoryCache(stale time.Duration) *memoryCache {
+	return &memoryCache{stale: stale}
+}
+
+type MemoryCacheResolver struct {
+}
+
+func NewMemoryCacheResolver() *MemoryCacheResolver {
+	return &MemoryCacheResolver{}
+}
diff --git a/coderd/runtimeconfig/entry.go b/coderd/runtimeconfig/entry.go
index ecb8a67a3fe92..3204936a76384 100644
--- a/coderd/runtimeconfig/entry.go
+++ b/coderd/runtimeconfig/entry.go
@@ -44,7 +44,7 @@ func MustNew[T EntryValue](name string) RuntimeEntry[T] {
 }
 
 // SetRuntimeValue attempts to update the runtime value of this field in the store via the given Mutator.
-func (e *RuntimeEntry[T]) SetRuntimeValue(ctx context.Context, m Manager, val T) error {
+func (e *RuntimeEntry[T]) SetRuntimeValue(ctx context.Context, m Resolver, val T) error {
 	name, err := e.name()
 	if err != nil {
 		return err
@@ -54,7 +54,7 @@ func (e *RuntimeEntry[T]) SetRuntimeValue(ctx context.Context, m Manager, val T)
 }
 
 // UnsetRuntimeValue removes the runtime value from the store.
-func (e *RuntimeEntry[T]) UnsetRuntimeValue(ctx context.Context, m Manager) error {
+func (e *RuntimeEntry[T]) UnsetRuntimeValue(ctx context.Context, m Resolver) error {
 	name, err := e.name()
 	if err != nil {
 		return err
@@ -64,7 +64,7 @@ func (e *RuntimeEntry[T]) UnsetRuntimeValue(ctx context.Context, m Manager) erro
 }
 
 // Resolve attempts to resolve the runtime value of this field from the store via the given Resolver.
-func (e *RuntimeEntry[T]) Resolve(ctx context.Context, r Manager) (T, error) {
+func (e *RuntimeEntry[T]) Resolve(ctx context.Context, r Resolver) (T, error) {
 	var zero T
 
 	name, err := e.name()
diff --git a/coderd/runtimeconfig/manager.go b/coderd/runtimeconfig/manager.go
index b3eaeded024ef..03b24c0111b3a 100644
--- a/coderd/runtimeconfig/manager.go
+++ b/coderd/runtimeconfig/manager.go
@@ -1,80 +1,49 @@
 package runtimeconfig
 
 import (
-	"context"
-	"database/sql"
-	"errors"
-	"fmt"
+	"time"
 
-	"golang.org/x/xerrors"
+	"github.com/google/uuid"
 
-	"github.com/coder/coder/v2/coderd/database"
+	"github.com/coder/coder/v2/coderd/util/syncmap"
 )
 
-type NoopManager struct{}
-
-func NewNoopManager() *NoopManager {
-	return &NoopManager{}
+type StoreManager struct {
 }
 
-func (NoopManager) GetRuntimeSetting(context.Context, string) (string, error) {
-	return "", EntryNotFound
+func NewStoreManager() *StoreManager {
+	return &StoreManager{}
 }
 
-func (NoopManager) UpsertRuntimeSetting(context.Context, string, string) error {
-	return EntryNotFound
+func (m *StoreManager) DeploymentResolver(db Store) Resolver {
+	return NewStoreResolver(db)
 }
 
-func (NoopManager) DeleteRuntimeSetting(context.Context, string) error {
-	return EntryNotFound
+func (m *StoreManager) OrganizationResolver(db Store, orgID uuid.UUID) Resolver {
+	return OrganizationResolver(orgID, NewStoreResolver(db))
 }
 
-func (n NoopManager) Scoped(string) Manager {
-	return n
+type cacheEntry struct {
+	value       string
+	lastUpdated time.Time
 }
 
-type StoreManager struct {
-	Store
-
-	ns string
-}
-
-func NewStoreManager(store Store) *StoreManager {
-	if store == nil {
-		panic("developer error: store must not be nil")
-	}
-	return &StoreManager{Store: store}
+type MemoryCacheManager struct {
+	cache   *syncmap.Map[string, cacheEntry]
+	wrapped Manager
 }
 
-func (m StoreManager) GetRuntimeSetting(ctx context.Context, key string) (string, error) {
-	key = m.namespacedKey(key)
-	val, err := m.Store.GetRuntimeConfig(ctx, key)
-	if err != nil {
-		if errors.Is(err, sql.ErrNoRows) {
-			return "", xerrors.Errorf("%q: %w", key, EntryNotFound)
-		}
-		return "", xerrors.Errorf("fetch %q: %w", key, err)
+func NewMemoryCacheManager(wrapped Manager) *MemoryCacheManager {
+	return &MemoryCacheManager{
+		cache:   syncmap.New[string, cacheEntry](),
+		wrapped: wrapped,
 	}
-
-	return val, nil
-}
-
-func (m StoreManager) UpsertRuntimeSetting(ctx context.Context, key, val string) error {
-	err := m.Store.UpsertRuntimeConfig(ctx, database.UpsertRuntimeConfigParams{Key: m.namespacedKey(key), Value: val})
-	if err != nil {
-		return xerrors.Errorf("update %q: %w", key, err)
-	}
-	return nil
-}
-
-func (m StoreManager) DeleteRuntimeSetting(ctx context.Context, key string) error {
-	return m.Store.DeleteRuntimeConfig(ctx, m.namespacedKey(key))
 }
 
-func (m StoreManager) Scoped(ns string) Manager {
-	return &StoreManager{Store: m.Store, ns: ns}
+func (m *MemoryCacheManager) DeploymentResolver(db Store) Resolver {
+	return NewMemoryCachedResolver(m.cache, m.wrapped.DeploymentResolver(db))
 }
 
-func (m StoreManager) namespacedKey(k string) string {
-	return fmt.Sprintf("%s:%s", m.ns, k)
+func (m *MemoryCacheManager) OrganizationResolver(db Store, orgID uuid.UUID) Resolver {
+	return OrganizationResolver(orgID, NewMemoryCachedResolver(m.cache, m.wrapped.DeploymentResolver(db)))
 }
diff --git a/coderd/runtimeconfig/resolver.go b/coderd/runtimeconfig/resolver.go
new file mode 100644
index 0000000000000..222641eda3ba9
--- /dev/null
+++ b/coderd/runtimeconfig/resolver.go
@@ -0,0 +1,139 @@
+package runtimeconfig
+
+import (
+	"context"
+	"database/sql"
+	"errors"
+	"fmt"
+	"time"
+
+	"github.com/google/uuid"
+	"golang.org/x/xerrors"
+
+	"github.com/coder/coder/v2/coderd/database"
+	"github.com/coder/coder/v2/coderd/util/syncmap"
+)
+
+type NoopResolver struct{}
+
+func NewNoopResolver() *NoopResolver {
+	return &NoopResolver{}
+}
+
+func (NoopResolver) GetRuntimeSetting(context.Context, string) (string, error) {
+	return "", EntryNotFound
+}
+
+func (NoopResolver) UpsertRuntimeSetting(context.Context, string, string) error {
+	return EntryNotFound
+}
+
+func (NoopResolver) DeleteRuntimeSetting(context.Context, string) error {
+	return EntryNotFound
+}
+
+// StoreResolver uses the database as the underlying store for runtime settings.
+type StoreResolver struct {
+	db Store
+}
+
+func NewStoreResolver(db Store) *StoreResolver {
+	return &StoreResolver{db: db}
+}
+
+func (m StoreResolver) GetRuntimeSetting(ctx context.Context, key string) (string, error) {
+	val, err := m.db.GetRuntimeConfig(ctx, key)
+	if err != nil {
+		if errors.Is(err, sql.ErrNoRows) {
+			return "", xerrors.Errorf("%q: %w", key, EntryNotFound)
+		}
+		return "", xerrors.Errorf("fetch %q: %w", key, err)
+	}
+
+	return val, nil
+}
+
+func (m StoreResolver) UpsertRuntimeSetting(ctx context.Context, key, val string) error {
+	err := m.db.UpsertRuntimeConfig(ctx, database.UpsertRuntimeConfigParams{Key: key, Value: val})
+	if err != nil {
+		return xerrors.Errorf("update %q: %w", key, err)
+	}
+	return nil
+}
+
+func (m StoreResolver) DeleteRuntimeSetting(ctx context.Context, key string) error {
+	return m.db.DeleteRuntimeConfig(ctx, key)
+}
+
+type NamespacedResolver struct {
+	ns      string
+	wrapped Resolver
+}
+
+func OrganizationResolver(orgID uuid.UUID, wrapped Resolver) NamespacedResolver {
+	return NamespacedResolver{ns: orgID.String(), wrapped: wrapped}
+}
+
+func (m NamespacedResolver) GetRuntimeSetting(ctx context.Context, key string) (string, error) {
+	return m.wrapped.GetRuntimeSetting(ctx, m.namespacedKey(key))
+}
+
+func (m NamespacedResolver) UpsertRuntimeSetting(ctx context.Context, key, val string) error {
+	return m.wrapped.UpsertRuntimeSetting(ctx, m.namespacedKey(key), val)
+}
+
+func (m NamespacedResolver) DeleteRuntimeSetting(ctx context.Context, key string) error {
+	return m.wrapped.DeleteRuntimeSetting(ctx, m.namespacedKey(key))
+}
+
+func (m NamespacedResolver) namespacedKey(k string) string {
+	return fmt.Sprintf("%s:%s", m.ns, k)
+}
+
+// MemoryCachedResolver is a super basic implementation of a cache for runtime
+// settings. Essentially, it reuses the shared "cache" that all resolvers should
+// use.
+type MemoryCachedResolver struct {
+	cache *syncmap.Map[string, cacheEntry]
+
+	wrapped Resolver
+}
+
+func NewMemoryCachedResolver(cache *syncmap.Map[string, cacheEntry], wrapped Resolver) *MemoryCachedResolver {
+	return &MemoryCachedResolver{
+		cache:   cache,
+		wrapped: wrapped,
+	}
+}
+
+func (m *MemoryCachedResolver) GetRuntimeSetting(ctx context.Context, key string) (string, error) {
+	cv, ok := m.cache.Load(key)
+	if ok {
+		return cv.value, nil
+	}
+
+	v, err := m.wrapped.GetRuntimeSetting(ctx, key)
+	if err != nil {
+		return "", err
+	}
+	m.cache.Store(key, cacheEntry{value: v, lastUpdated: time.Now()})
+	return v, nil
+}
+
+func (m *MemoryCachedResolver) UpsertRuntimeSetting(ctx context.Context, key, val string) error {
+	err := m.wrapped.UpsertRuntimeSetting(ctx, key, val)
+	if err != nil {
+		return err
+	}
+	m.cache.Store(key, cacheEntry{value: val, lastUpdated: time.Now()})
+	return nil
+}
+
+func (m *MemoryCachedResolver) DeleteRuntimeSetting(ctx context.Context, key string) error {
+	err := m.wrapped.DeleteRuntimeSetting(ctx, key)
+	if err != nil {
+		return err
+	}
+	m.cache.Delete(key)
+	return nil
+}
diff --git a/coderd/runtimeconfig/spec.go b/coderd/runtimeconfig/spec.go
index 80015f56a5ca1..040b37fab516c 100644
--- a/coderd/runtimeconfig/spec.go
+++ b/coderd/runtimeconfig/spec.go
@@ -8,14 +8,21 @@ type Initializer interface {
 	Initialize(name string)
 }
 
+// Manager is just a factory to produce Resolvers.
+// The reason a factory is required, is the Manager can act as a caching
+// layer for runtime settings.
 type Manager interface {
+	// DeploymentResolver returns a Resolver scoped to the deployment.
+	DeploymentResolver(db Store) Resolver
+	// OrganizationResolver returns a Resolver scoped to the organization.
+	OrganizationResolver(db Store, orgID string) Resolver
+}
+
+type Resolver interface {
 	// GetRuntimeSetting gets a runtime setting by name.
 	GetRuntimeSetting(ctx context.Context, name string) (string, error)
 	// UpsertRuntimeSetting upserts a runtime setting by name.
 	UpsertRuntimeSetting(ctx context.Context, name, val string) error
 	// DeleteRuntimeSetting deletes a runtime setting by name.
 	DeleteRuntimeSetting(ctx context.Context, name string) error
-	// Scoped returns a new Manager which is responsible for namespacing all runtime keys during CRUD operations.
-	// This can be used for scoping runtime settings to organizations, for example.
-	Scoped(ns string) Manager
 }

From 9329805cca6ca7310cb9806c19c1ee24c7977ef0 Mon Sep 17 00:00:00 2001
From: Steven Masley <stevenmasley@gmail.com>
Date: Thu, 5 Sep 2024 12:23:00 -0500
Subject: [PATCH 3/8] chore: implement store manager

---
 cli/server.go                           |  2 +-
 coderd/coderdtest/coderdtest.go         |  5 +----
 coderd/runtimeconfig/deploymententry.go |  2 +-
 coderd/runtimeconfig/entry_test.go      | 28 +++++++++++++------------
 coderd/runtimeconfig/manager.go         | 18 +++++++++-------
 coderd/runtimeconfig/spec.go            | 11 +++++-----
 6 files changed, 33 insertions(+), 33 deletions(-)

diff --git a/cli/server.go b/cli/server.go
index a4fcf660225a1..fe9b95a5fed95 100644
--- a/cli/server.go
+++ b/cli/server.go
@@ -827,7 +827,7 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd.
 			// dbauthz is configured in `Coderd.New()`, but we need the manager
 			// at this level for notifications. We might have to move some init
 			// code around.
-			options.RuntimeConfig = runtimeconfig.NewStoreManager(options.Database)
+			options.RuntimeConfig = runtimeconfig.NewStoreManager()
 
 			// This should be output before the logs start streaming.
 			cliui.Infof(inv.Stdout, "\n==> Logs will stream in below (press ctrl+c to gracefully exit):")
diff --git a/coderd/coderdtest/coderdtest.go b/coderd/coderdtest/coderdtest.go
index 0e3c049c16511..cc904b8fc92f7 100644
--- a/coderd/coderdtest/coderdtest.go
+++ b/coderd/coderdtest/coderdtest.go
@@ -255,10 +255,7 @@ func NewOptions(t testing.TB, options *Options) (func(http.Handler), context.Can
 	var acs dbauthz.AccessControlStore = dbauthz.AGPLTemplateAccessControlStore{}
 	accessControlStore.Store(&acs)
 
-	// runtimeManager does not use dbauthz.
-	// TODO: It probably should, but the init code for prod happens before dbauthz
-	// is ready.
-	runtimeManager := runtimeconfig.NewStoreManager(options.Database)
+	runtimeManager := runtimeconfig.NewStoreManager()
 	options.Database = dbauthz.New(options.Database, options.Authorizer, *options.Logger, accessControlStore)
 
 	// Some routes expect a deployment ID, so just make sure one exists.
diff --git a/coderd/runtimeconfig/deploymententry.go b/coderd/runtimeconfig/deploymententry.go
index 896a05caf53f0..c4405a676dd4f 100644
--- a/coderd/runtimeconfig/deploymententry.go
+++ b/coderd/runtimeconfig/deploymententry.go
@@ -45,7 +45,7 @@ func (e *DeploymentEntry[T]) StartupValue() T {
 
 // Coalesce attempts to resolve the runtime value of this field from the store via the given Manager. Should no runtime
 // value be found, the startup value will be used.
-func (e *DeploymentEntry[T]) Coalesce(ctx context.Context, r Manager) (T, error) {
+func (e *DeploymentEntry[T]) Coalesce(ctx context.Context, r Resolver) (T, error) {
 	var zero T
 
 	resolved, err := e.Resolve(ctx, r)
diff --git a/coderd/runtimeconfig/entry_test.go b/coderd/runtimeconfig/entry_test.go
index a184ff52566b0..064f4d2800596 100644
--- a/coderd/runtimeconfig/entry_test.go
+++ b/coderd/runtimeconfig/entry_test.go
@@ -40,7 +40,7 @@ func TestEntry(t *testing.T) {
 	t.Run("zero", func(t *testing.T) {
 		t.Parallel()
 
-		mgr := runtimeconfig.NewNoopManager()
+		rlv := runtimeconfig.NewNoopResolver()
 
 		// A zero-value declaration of a runtimeconfig.Entry should behave as a zero value of the generic type.
 		// NB! A name has not been set for this entry; it is "uninitialized".
@@ -52,18 +52,19 @@ func TestEntry(t *testing.T) {
 		require.NoError(t, field.SetStartupValue("true"))
 
 		// But attempting to resolve will produce an error.
-		_, err := field.Resolve(context.Background(), mgr)
+		_, err := field.Resolve(context.Background(), rlv)
 		require.ErrorIs(t, err, runtimeconfig.ErrNameNotSet)
 		// But attempting to set the runtime value will produce an error.
 		val := serpent.BoolOf(ptr.Ref(true))
-		require.ErrorIs(t, field.SetRuntimeValue(context.Background(), mgr, val), runtimeconfig.ErrNameNotSet)
+		require.ErrorIs(t, field.SetRuntimeValue(context.Background(), rlv, val), runtimeconfig.ErrNameNotSet)
 	})
 
 	t.Run("simple", func(t *testing.T) {
 		t.Parallel()
 
 		ctx := testutil.Context(t, testutil.WaitShort)
-		mgr := runtimeconfig.NewStoreManager(dbmem.New())
+		mgr := runtimeconfig.NewStoreManager()
+		db := dbmem.New()
 
 		var (
 			base     = serpent.String("system@dev.coder.com")
@@ -78,16 +79,16 @@ func TestEntry(t *testing.T) {
 		// Validate that it returns that value.
 		require.Equal(t, base.String(), field.String())
 		// Validate that there is no org-level override right now.
-		_, err := field.Resolve(ctx, mgr)
+		_, err := field.Resolve(ctx, mgr.DeploymentResolver(db))
 		require.ErrorIs(t, err, runtimeconfig.EntryNotFound)
 		// Coalesce returns the deployment-wide value.
-		val, err := field.Coalesce(ctx, mgr)
+		val, err := field.Coalesce(ctx, mgr.DeploymentResolver(db))
 		require.NoError(t, err)
 		require.Equal(t, base.String(), val.String())
 		// Set an org-level override.
-		require.NoError(t, field.SetRuntimeValue(ctx, mgr, &override))
+		require.NoError(t, field.SetRuntimeValue(ctx, mgr.DeploymentResolver(db), &override))
 		// Coalesce now returns the org-level value.
-		val, err = field.Coalesce(ctx, mgr)
+		val, err = field.Coalesce(ctx, mgr.DeploymentResolver(db))
 		require.NoError(t, err)
 		require.Equal(t, override.String(), val.String())
 	})
@@ -96,7 +97,8 @@ func TestEntry(t *testing.T) {
 		t.Parallel()
 
 		ctx := testutil.Context(t, testutil.WaitShort)
-		mgr := runtimeconfig.NewStoreManager(dbmem.New())
+		mgr := runtimeconfig.NewStoreManager()
+		db := dbmem.New()
 
 		var (
 			base = serpent.Struct[map[string]string]{
@@ -117,16 +119,16 @@ func TestEntry(t *testing.T) {
 		// Check that default has been set.
 		require.Equal(t, base.String(), field.StartupValue().String())
 		// Validate that there is no org-level override right now.
-		_, err := field.Resolve(ctx, mgr)
+		_, err := field.Resolve(ctx, mgr.DeploymentResolver(db))
 		require.ErrorIs(t, err, runtimeconfig.EntryNotFound)
 		// Coalesce returns the deployment-wide value.
-		val, err := field.Coalesce(ctx, mgr)
+		val, err := field.Coalesce(ctx, mgr.DeploymentResolver(db))
 		require.NoError(t, err)
 		require.Equal(t, base.Value, val.Value)
 		// Set an org-level override.
-		require.NoError(t, field.SetRuntimeValue(ctx, mgr, &override))
+		require.NoError(t, field.SetRuntimeValue(ctx, mgr.DeploymentResolver(db), &override))
 		// Coalesce now returns the org-level value.
-		structVal, err := field.Resolve(ctx, mgr)
+		structVal, err := field.Resolve(ctx, mgr.DeploymentResolver(db))
 		require.NoError(t, err)
 		require.Equal(t, override.Value, structVal.Value)
 	})
diff --git a/coderd/runtimeconfig/manager.go b/coderd/runtimeconfig/manager.go
index 03b24c0111b3a..00a86d87f541a 100644
--- a/coderd/runtimeconfig/manager.go
+++ b/coderd/runtimeconfig/manager.go
@@ -8,10 +8,11 @@ import (
 	"github.com/coder/coder/v2/coderd/util/syncmap"
 )
 
+// StoreManager is the shared singleton that produces resolvers for runtime configuration.
 type StoreManager struct {
 }
 
-func NewStoreManager() *StoreManager {
+func NewStoreManager() Manager {
 	return &StoreManager{}
 }
 
@@ -28,22 +29,23 @@ type cacheEntry struct {
 	lastUpdated time.Time
 }
 
+// MemoryCacheManager is an example of how a caching layer can be added to the
+// resolver from the manager.
+// TODO: Delete MemoryCacheManager and implement it properly in 'StoreManager'.
 type MemoryCacheManager struct {
-	cache   *syncmap.Map[string, cacheEntry]
-	wrapped Manager
+	cache *syncmap.Map[string, cacheEntry]
 }
 
-func NewMemoryCacheManager(wrapped Manager) *MemoryCacheManager {
+func NewMemoryCacheManager() *MemoryCacheManager {
 	return &MemoryCacheManager{
-		cache:   syncmap.New[string, cacheEntry](),
-		wrapped: wrapped,
+		cache: syncmap.New[string, cacheEntry](),
 	}
 }
 
 func (m *MemoryCacheManager) DeploymentResolver(db Store) Resolver {
-	return NewMemoryCachedResolver(m.cache, m.wrapped.DeploymentResolver(db))
+	return NewMemoryCachedResolver(m.cache, NewStoreResolver(db))
 }
 
 func (m *MemoryCacheManager) OrganizationResolver(db Store, orgID uuid.UUID) Resolver {
-	return OrganizationResolver(orgID, NewMemoryCachedResolver(m.cache, m.wrapped.DeploymentResolver(db)))
+	return OrganizationResolver(orgID, NewMemoryCachedResolver(m.cache, NewStoreResolver(db)))
 }
diff --git a/coderd/runtimeconfig/spec.go b/coderd/runtimeconfig/spec.go
index 040b37fab516c..ad78455dc8964 100644
--- a/coderd/runtimeconfig/spec.go
+++ b/coderd/runtimeconfig/spec.go
@@ -2,20 +2,19 @@ package runtimeconfig
 
 import (
 	"context"
+
+	"github.com/google/uuid"
 )
 
 type Initializer interface {
 	Initialize(name string)
 }
 
-// Manager is just a factory to produce Resolvers.
-// The reason a factory is required, is the Manager can act as a caching
-// layer for runtime settings.
+// TODO: We should probably remove the manager interface and only support
+// 1 implementation.
 type Manager interface {
-	// DeploymentResolver returns a Resolver scoped to the deployment.
 	DeploymentResolver(db Store) Resolver
-	// OrganizationResolver returns a Resolver scoped to the organization.
-	OrganizationResolver(db Store, orgID string) Resolver
+	OrganizationResolver(db Store, orgID uuid.UUID) Resolver
 }
 
 type Resolver interface {

From 0e280ca94104e3dba681be0fec5aa7dd52ccf799 Mon Sep 17 00:00:00 2001
From: Steven Masley <stevenmasley@gmail.com>
Date: Thu, 5 Sep 2024 12:23:54 -0500
Subject: [PATCH 4/8] remove dead code

---
 coderd/runtimeconfig/cache.go | 22 ----------------------
 1 file changed, 22 deletions(-)
 delete mode 100644 coderd/runtimeconfig/cache.go

diff --git a/coderd/runtimeconfig/cache.go b/coderd/runtimeconfig/cache.go
deleted file mode 100644
index 7798d356be065..0000000000000
--- a/coderd/runtimeconfig/cache.go
+++ /dev/null
@@ -1,22 +0,0 @@
-package runtimeconfig
-
-import (
-	"sync"
-	"time"
-)
-
-type memoryCache struct {
-	stale time.Duration
-	mu    sync.Mutex
-}
-
-func newMemoryCache(stale time.Duration) *memoryCache {
-	return &memoryCache{stale: stale}
-}
-
-type MemoryCacheResolver struct {
-}
-
-func NewMemoryCacheResolver() *MemoryCacheResolver {
-	return &MemoryCacheResolver{}
-}

From 62fa3b005c2dd6467736fcf17a55e778bc34a267 Mon Sep 17 00:00:00 2001
From: Steven Masley <stevenmasley@gmail.com>
Date: Thu, 5 Sep 2024 12:29:03 -0500
Subject: [PATCH 5/8] remove comment

---
 cli/server.go | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/cli/server.go b/cli/server.go
index fe9b95a5fed95..0846f65624b0e 100644
--- a/cli/server.go
+++ b/cli/server.go
@@ -821,12 +821,6 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd.
 				return err
 			}
 
-			// TODO: Throw a caching layer infront of the RuntimeConfig to prevent
-			// excessive database queries.
-			// Note: This happens before dbauthz, which is really unfortunate.
-			// dbauthz is configured in `Coderd.New()`, but we need the manager
-			// at this level for notifications. We might have to move some init
-			// code around.
 			options.RuntimeConfig = runtimeconfig.NewStoreManager()
 
 			// This should be output before the logs start streaming.

From f0ce1c4bf1efe947bdb0d5b4faa48eac12027d0d Mon Sep 17 00:00:00 2001
From: Steven Masley <stevenmasley@gmail.com>
Date: Thu, 5 Sep 2024 13:28:58 -0500
Subject: [PATCH 6/8] fixups

---
 coderd/database/dbauthz/dbauthz.go      | 12 +++++++++---
 coderd/database/dbauthz/dbauthz_test.go |  9 +++++++++
 coderd/runtimeconfig/deploymententry.go |  6 ++++--
 coderd/runtimeconfig/manager.go         |  7 +++----
 4 files changed, 25 insertions(+), 9 deletions(-)

diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go
index 1520c73f6ad60..5782bdc8e7155 100644
--- a/coderd/database/dbauthz/dbauthz.go
+++ b/coderd/database/dbauthz/dbauthz.go
@@ -1184,7 +1184,9 @@ func (q *querier) DeleteReplicasUpdatedBefore(ctx context.Context, updatedAt tim
 }
 
 func (q *querier) DeleteRuntimeConfig(ctx context.Context, key string) error {
-	// TODO: auth
+	if err := q.authorizeContext(ctx, policy.ActionDelete, rbac.ResourceSystem); err != nil {
+		return err
+	}
 	return q.db.DeleteRuntimeConfig(ctx, key)
 }
 
@@ -1862,7 +1864,9 @@ func (q *querier) GetReplicasUpdatedAfter(ctx context.Context, updatedAt time.Ti
 }
 
 func (q *querier) GetRuntimeConfig(ctx context.Context, key string) (string, error) {
-	// TODO: auth
+	if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceSystem); err != nil {
+		return "", err
+	}
 	return q.db.GetRuntimeConfig(ctx, key)
 }
 
@@ -3917,7 +3921,9 @@ func (q *querier) UpsertProvisionerDaemon(ctx context.Context, arg database.Upse
 }
 
 func (q *querier) UpsertRuntimeConfig(ctx context.Context, arg database.UpsertRuntimeConfigParams) error {
-	// TODO: auth
+	if err := q.authorizeContext(ctx, policy.ActionCreate, rbac.ResourceSystem); err != nil {
+		return err
+	}
 	return q.db.UpsertRuntimeConfig(ctx, arg)
 }
 
diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go
index e76ea5a3ef28d..8806d44503a83 100644
--- a/coderd/database/dbauthz/dbauthz_test.go
+++ b/coderd/database/dbauthz/dbauthz_test.go
@@ -2696,6 +2696,15 @@ func (s *MethodTestSuite) TestSystemFunctions() {
 			AgentID:     uuid.New(),
 		}).Asserts(tpl, policy.ActionCreate)
 	}))
+	s.Run("DeleteRuntimeConfig", s.Subtest(func(db database.Store, check *expects) {
+		check.Args("test").Asserts(rbac.ResourceSystem, policy.ActionDelete)
+	}))
+	s.Run("GetRuntimeConfig", s.Subtest(func(db database.Store, check *expects) {
+		check.Args("test").Asserts(rbac.ResourceSystem, policy.ActionRead)
+	}))
+	s.Run("UpsertRuntimeConfig", s.Subtest(func(db database.Store, check *expects) {
+		check.Args("test", "value").Asserts(rbac.ResourceSystem, policy.ActionUpdate)
+	}))
 }
 
 func (s *MethodTestSuite) TestNotifications() {
diff --git a/coderd/runtimeconfig/deploymententry.go b/coderd/runtimeconfig/deploymententry.go
index c4405a676dd4f..0ae23de970fc4 100644
--- a/coderd/runtimeconfig/deploymententry.go
+++ b/coderd/runtimeconfig/deploymententry.go
@@ -9,8 +9,10 @@ import (
 )
 
 // Ensure serpent values satisfy the ConfigValue interface for easier usage.
-var _ pflag.Value = SerpentEntry(nil)
-var _ pflag.Value = &DeploymentEntry[SerpentEntry]{}
+var (
+	_ pflag.Value = SerpentEntry(nil)
+	_ pflag.Value = &DeploymentEntry[SerpentEntry]{}
+)
 
 type SerpentEntry interface {
 	EntryValue
diff --git a/coderd/runtimeconfig/manager.go b/coderd/runtimeconfig/manager.go
index 00a86d87f541a..f2e5a942be827 100644
--- a/coderd/runtimeconfig/manager.go
+++ b/coderd/runtimeconfig/manager.go
@@ -9,18 +9,17 @@ import (
 )
 
 // StoreManager is the shared singleton that produces resolvers for runtime configuration.
-type StoreManager struct {
-}
+type StoreManager struct{}
 
 func NewStoreManager() Manager {
 	return &StoreManager{}
 }
 
-func (m *StoreManager) DeploymentResolver(db Store) Resolver {
+func (*StoreManager) DeploymentResolver(db Store) Resolver {
 	return NewStoreResolver(db)
 }
 
-func (m *StoreManager) OrganizationResolver(db Store, orgID uuid.UUID) Resolver {
+func (*StoreManager) OrganizationResolver(db Store, orgID uuid.UUID) Resolver {
 	return OrganizationResolver(orgID, NewStoreResolver(db))
 }
 

From 357438bfc3f5f9d9b79a8e2226e8e2e3b5daf832 Mon Sep 17 00:00:00 2001
From: Steven Masley <stevenmasley@gmail.com>
Date: Thu, 5 Sep 2024 13:41:48 -0500
Subject: [PATCH 7/8] fixup authz test

---
 coderd/database/dbauthz/dbauthz_test.go | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go
index 8806d44503a83..d23bb48184b61 100644
--- a/coderd/database/dbauthz/dbauthz_test.go
+++ b/coderd/database/dbauthz/dbauthz_test.go
@@ -2700,10 +2700,17 @@ func (s *MethodTestSuite) TestSystemFunctions() {
 		check.Args("test").Asserts(rbac.ResourceSystem, policy.ActionDelete)
 	}))
 	s.Run("GetRuntimeConfig", s.Subtest(func(db database.Store, check *expects) {
+		_ = db.UpsertRuntimeConfig(context.Background(), database.UpsertRuntimeConfigParams{
+			Key:   "test",
+			Value: "value",
+		})
 		check.Args("test").Asserts(rbac.ResourceSystem, policy.ActionRead)
 	}))
 	s.Run("UpsertRuntimeConfig", s.Subtest(func(db database.Store, check *expects) {
-		check.Args("test", "value").Asserts(rbac.ResourceSystem, policy.ActionUpdate)
+		check.Args(database.UpsertRuntimeConfigParams{
+			Key:   "test",
+			Value: "value",
+		}).Asserts(rbac.ResourceSystem, policy.ActionCreate)
 	}))
 }
 

From 522cde1094411c4e88b00f93eb2a93ac921fe513 Mon Sep 17 00:00:00 2001
From: Steven Masley <Emyrk@users.noreply.github.com>
Date: Thu, 5 Sep 2024 15:14:51 -0500
Subject: [PATCH 8/8] Update coderd/runtimeconfig/manager.go

Co-authored-by: Danny Kopping <danny@coder.com>
---
 coderd/runtimeconfig/manager.go | 1 +
 1 file changed, 1 insertion(+)

diff --git a/coderd/runtimeconfig/manager.go b/coderd/runtimeconfig/manager.go
index f2e5a942be827..293b04cbd108f 100644
--- a/coderd/runtimeconfig/manager.go
+++ b/coderd/runtimeconfig/manager.go
@@ -31,6 +31,7 @@ type cacheEntry struct {
 // MemoryCacheManager is an example of how a caching layer can be added to the
 // resolver from the manager.
 // TODO: Delete MemoryCacheManager and implement it properly in 'StoreManager'.
+// TODO: Handle pubsub-based cache invalidation.
 type MemoryCacheManager struct {
 	cache *syncmap.Map[string, cacheEntry]
 }