From 5c92688a121ee2b6782b328e4b617b8a0c76dd4e Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Wed, 28 Aug 2024 17:04:53 +0200 Subject: [PATCH 01/19] Implementation Signed-off-by: Danny Kopping --- coderd/runtimeconfig/config.go | 97 ++++++++++++++++++++++ coderd/runtimeconfig/config_test.go | 123 ++++++++++++++++++++++++++++ coderd/runtimeconfig/mutator.go | 40 +++++++++ coderd/runtimeconfig/resolver.go | 63 ++++++++++++++ coderd/runtimeconfig/spec.go | 25 ++++++ coderd/runtimeconfig/store.go | 53 ++++++++++++ coderd/runtimeconfig/util.go | 17 ++++ 7 files changed, 418 insertions(+) create mode 100644 coderd/runtimeconfig/config.go create mode 100644 coderd/runtimeconfig/config_test.go create mode 100644 coderd/runtimeconfig/mutator.go create mode 100644 coderd/runtimeconfig/resolver.go create mode 100644 coderd/runtimeconfig/spec.go create mode 100644 coderd/runtimeconfig/store.go create mode 100644 coderd/runtimeconfig/util.go diff --git a/coderd/runtimeconfig/config.go b/coderd/runtimeconfig/config.go new file mode 100644 index 0000000000000..27ebb203e9507 --- /dev/null +++ b/coderd/runtimeconfig/config.go @@ -0,0 +1,97 @@ +package runtimeconfig + +import ( + "context" + "errors" + "reflect" + + "github.com/spf13/pflag" + "golang.org/x/xerrors" +) + +// TODO: comment +type Value pflag.Value + +type Entry[T Value] struct { + val T + key string +} + +func New[T Value](key, val string) (out Entry[T], err error) { + out.Init(key) + + if err = out.Set(val); err != nil { + return out, err + } + + return out, nil +} + +func MustNew[T Value](key, val string) Entry[T] { + out, err := New[T](key, val) + if err != nil { + panic(err) + } + return out +} + +func (o *Entry[T]) Init(key string) { + o.val = create[T]() + o.key = key +} + +func (o *Entry[T]) Set(s string) error { + if reflect.ValueOf(o.val).IsNil() { + return xerrors.Errorf("instance of %T is uninitialized", o.val) + } + return o.val.Set(s) +} + +func (o *Entry[T]) Type() string { + return o.val.Type() +} + +func (o *Entry[T]) String() string { + return o.val.String() +} + +func (o *Entry[T]) StartupValue() T { + return o.val +} + +func (o *Entry[T]) Resolve(ctx context.Context, r Resolver) (T, error) { + return o.resolve(ctx, r) +} + +func (o *Entry[T]) resolve(ctx context.Context, r Resolver) (T, error) { + var zero T + + val, err := r.ResolveByKey(ctx, o.key) + 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 +} + +func (o *Entry[T]) Save(ctx context.Context, m Mutator, val T) error { + return m.MutateByKey(ctx, o.key, val.String()) +} + +func (o *Entry[T]) Coalesce(ctx context.Context, r Resolver) (T, error) { + var zero T + + resolved, err := o.resolve(ctx, r) + if err != nil { + if errors.Is(err, EntryNotFound) { + return o.StartupValue(), nil + } + return zero, err + } + + return resolved, nil +} diff --git a/coderd/runtimeconfig/config_test.go b/coderd/runtimeconfig/config_test.go new file mode 100644 index 0000000000000..2cce6b2854a0d --- /dev/null +++ b/coderd/runtimeconfig/config_test.go @@ -0,0 +1,123 @@ +package runtimeconfig_test + +import ( + "testing" + + "github.com/coder/serpent" + "github.com/stretchr/testify/require" + + "github.com/coder/coder/v2/coderd/coderdtest" + "github.com/coder/coder/v2/coderd/runtimeconfig" + "github.com/coder/coder/v2/codersdk" + "github.com/coder/coder/v2/enterprise/coderd/coderdenttest" + "github.com/coder/coder/v2/enterprise/coderd/license" + "github.com/coder/coder/v2/testutil" +) + +// TestConfig demonstrates creating org-level overrides for deployment-level settings. +func TestConfig(t *testing.T) { + t.Parallel() + + vals := coderdtest.DeploymentValues(t) + vals.Experiments = []string{string(codersdk.ExperimentMultiOrganization)} + adminClient, _, _, _ := coderdenttest.NewWithAPI(t, &coderdenttest.Options{ + Options: &coderdtest.Options{DeploymentValues: vals}, + LicenseOptions: &coderdenttest.LicenseOptions{ + Features: license.Features{ + codersdk.FeatureMultipleOrganizations: 1, + }, + }, + }) + altOrg := coderdenttest.CreateOrganization(t, adminClient, coderdenttest.CreateOrganizationOptions{}) + + t.Run("panics unless initialized", func(t *testing.T) { + t.Parallel() + + field := runtimeconfig.Entry[*serpent.String]{} + require.Panics(t, func() { + field.StartupValue().String() + }) + + field.Init("my-field") + require.NotPanics(t, func() { + field.StartupValue().String() + }) + }) + + t.Run("simple", func(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitShort) + store := runtimeconfig.NewInMemoryStore() + resolver := runtimeconfig.NewOrgResolver(altOrg.ID, runtimeconfig.NewStoreResolver(store)) + mutator := runtimeconfig.NewOrgMutator(altOrg.ID, runtimeconfig.NewStoreMutator(store)) + + var ( + base = serpent.String("system@dev.coder.com") + override = serpent.String("dogfood@dev.coder.com") + ) + + field := runtimeconfig.Entry[*serpent.String]{} + field.Init("my-field") + // Check that no default has been set. + require.Empty(t, field.StartupValue().String()) + // Initialize the value. + require.NoError(t, field.Set(base.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, resolver) + require.ErrorIs(t, err, runtimeconfig.EntryNotFound) + // Coalesce returns the deployment-wide value. + val, err := field.Coalesce(ctx, resolver) + require.NoError(t, err) + require.Equal(t, base.String(), val.String()) + // Set an org-level override. + require.NoError(t, field.Save(ctx, mutator, &override)) + // Coalesce now returns the org-level value. + val, err = field.Coalesce(ctx, resolver) + 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) + store := runtimeconfig.NewInMemoryStore() + resolver := runtimeconfig.NewOrgResolver(altOrg.ID, runtimeconfig.NewStoreResolver(store)) + mutator := runtimeconfig.NewOrgMutator(altOrg.ID, runtimeconfig.NewStoreMutator(store)) + + field := runtimeconfig.Entry[*serpent.Struct[map[string]string]]{} + field.Init("my-field") + 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", + }, + } + ) + + // Check that no default has been set. + require.Empty(t, field.StartupValue().Value) + // Initialize the value. + require.NoError(t, field.Set(base.String())) + // Validate that there is no org-level override right now. + _, err := field.Resolve(ctx, resolver) + require.ErrorIs(t, err, runtimeconfig.EntryNotFound) + // Coalesce returns the deployment-wide value. + val, err := field.Coalesce(ctx, resolver) + require.NoError(t, err) + require.Equal(t, base.Value, val.Value) + // Set an org-level override. + require.NoError(t, field.Save(ctx, mutator, &override)) + // Coalesce now returns the org-level value. + structVal, err := field.Resolve(ctx, resolver) + require.NoError(t, err) + require.Equal(t, override.Value, structVal.Value) + }) +} diff --git a/coderd/runtimeconfig/mutator.go b/coderd/runtimeconfig/mutator.go new file mode 100644 index 0000000000000..1f377874caf05 --- /dev/null +++ b/coderd/runtimeconfig/mutator.go @@ -0,0 +1,40 @@ +package runtimeconfig + +import ( + "context" + + "github.com/google/uuid" + "golang.org/x/xerrors" +) + +type StoreMutator struct { + store Store +} + +func NewStoreMutator(store Store) *StoreMutator { + if store == nil { + panic("developer error: store is nil") + } + return &StoreMutator{store} +} + +func (s *StoreMutator) MutateByKey(ctx context.Context, key, val string) error { + err := s.store.UpsertRuntimeSetting(ctx, key, val) + if err != nil { + return xerrors.Errorf("update %q: %w", err) + } + return nil +} + +type OrgMutator struct { + inner Mutator + orgID uuid.UUID +} + +func NewOrgMutator(orgID uuid.UUID, inner Mutator) *OrgMutator { + return &OrgMutator{inner: inner, orgID: orgID} +} + +func (m OrgMutator) MutateByKey(ctx context.Context, key, val string) error { + return m.inner.MutateByKey(ctx, orgKey(m.orgID, key), val) +} diff --git a/coderd/runtimeconfig/resolver.go b/coderd/runtimeconfig/resolver.go new file mode 100644 index 0000000000000..7eb47833697c4 --- /dev/null +++ b/coderd/runtimeconfig/resolver.go @@ -0,0 +1,63 @@ +package runtimeconfig + +import ( + "context" + "database/sql" + "errors" + + "github.com/google/uuid" + "golang.org/x/xerrors" +) + +type StoreResolver struct { + store Store +} + +func NewStoreResolver(store Store) *StoreResolver { + return &StoreResolver{store} +} + +func (s StoreResolver) ResolveByKey(ctx context.Context, key string) (string, error) { + if s.store == nil { + panic("developer error: store must be set") + } + + val, err := s.store.GetRuntimeSetting(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 +} + +type OrgResolver struct { + inner Resolver + orgID uuid.UUID +} + +func NewOrgResolver(orgID uuid.UUID, inner Resolver) *OrgResolver { + if inner == nil { + panic("developer error: resolver is nil") + } + + return &OrgResolver{inner: inner, orgID: orgID} +} + +func (r OrgResolver) ResolveByKey(ctx context.Context, key string) (string, error) { + return r.inner.ResolveByKey(ctx, orgKey(r.orgID, key)) +} + +// NoopResolver will always fail to resolve the given key. +// Useful in tests where you just want to look up the startup value of configs, and are not concerned with runtime config. +type NoopResolver struct {} + +func NewNoopResolver() *NoopResolver { + return &NoopResolver{} +} + +func (n NoopResolver) ResolveByKey(context.Context, string) (string, error) { + return "", EntryNotFound +} diff --git a/coderd/runtimeconfig/spec.go b/coderd/runtimeconfig/spec.go new file mode 100644 index 0000000000000..b3e6db23c06d7 --- /dev/null +++ b/coderd/runtimeconfig/spec.go @@ -0,0 +1,25 @@ +package runtimeconfig + +import "context" + +type Initializer interface { + Init(key string) +} + +// type RuntimeConfigResolver[T Value] interface { +// StartupValue() T +// Resolve(r Resolver) (T, error) +// Coalesce(r Resolver) (T, error) +// } +// +// type RuntimeConfigMutator[T Value] interface { +// Save(m Mutator, val T) error +// } + +type Resolver interface { + ResolveByKey(ctx context.Context, key string) (string, error) +} + +type Mutator interface { + MutateByKey(ctx context.Context, key, val string) error +} \ No newline at end of file diff --git a/coderd/runtimeconfig/store.go b/coderd/runtimeconfig/store.go new file mode 100644 index 0000000000000..781563b523331 --- /dev/null +++ b/coderd/runtimeconfig/store.go @@ -0,0 +1,53 @@ +package runtimeconfig + +import ( + "context" + "sync" + + "golang.org/x/xerrors" +) + +var EntryNotFound = xerrors.New("entry not found") + +type Store interface { + GetRuntimeSetting(ctx context.Context, key string) (string, error) + UpsertRuntimeSetting(ctx context.Context, key, value string) error + DeleteRuntimeSetting(ctx context.Context, key string) error +} + +type InMemoryStore struct { + mu sync.Mutex + store map[string]string +} + +func NewInMemoryStore() *InMemoryStore { + return &InMemoryStore{store: make(map[string]string)} +} + +func (s *InMemoryStore) GetRuntimeSetting(_ context.Context, key string) (string, error) { + s.mu.Lock() + defer s.mu.Unlock() + + val, ok := s.store[key] + if !ok { + return "", EntryNotFound + } + + return val, nil +} + +func (s *InMemoryStore) UpsertRuntimeSetting(_ context.Context, key, value string) error { + s.mu.Lock() + defer s.mu.Unlock() + + s.store[key] = value + return nil +} + +func (s *InMemoryStore) DeleteRuntimeSetting(_ context.Context, key string) error { + s.mu.Lock() + defer s.mu.Unlock() + + delete(s.store, key) + return nil +} diff --git a/coderd/runtimeconfig/util.go b/coderd/runtimeconfig/util.go new file mode 100644 index 0000000000000..9281736159211 --- /dev/null +++ b/coderd/runtimeconfig/util.go @@ -0,0 +1,17 @@ +package runtimeconfig + +import ( + "fmt" + "reflect" + + "github.com/google/uuid" +) + +func create[T any]() T { + var zero T + return reflect.New(reflect.TypeOf(zero).Elem()).Interface().(T) +} + +func orgKey(orgID uuid.UUID, key string) string { + return fmt.Sprintf("%s:%s", orgID.String(), key) +} \ No newline at end of file From 03ecbac3f4ae8c72ce33ff4dbae79d6ed4e1e7d9 Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Tue, 3 Sep 2024 12:14:44 +0200 Subject: [PATCH 02/19] Support zero values Signed-off-by: Danny Kopping --- coderd/runtimeconfig/config.go | 82 +++++++++++++++++++---------- coderd/runtimeconfig/config_test.go | 54 ++++++++++++------- coderd/runtimeconfig/mutator.go | 10 ++++ coderd/runtimeconfig/spec.go | 10 ---- 4 files changed, 99 insertions(+), 57 deletions(-) diff --git a/coderd/runtimeconfig/config.go b/coderd/runtimeconfig/config.go index 27ebb203e9507..17de13e9eb7d7 100644 --- a/coderd/runtimeconfig/config.go +++ b/coderd/runtimeconfig/config.go @@ -9,18 +9,20 @@ import ( "golang.org/x/xerrors" ) +var ErrKeyNotSet = xerrors.New("key is not set") + // TODO: comment type Value pflag.Value type Entry[T Value] struct { - val T - key string + k string + v T } func New[T Value](key, val string) (out Entry[T], err error) { - out.Init(key) + out.k = key - if err = out.Set(val); err != nil { + if err = out.SetStartupValue(val); err != nil { return out, err } @@ -35,38 +37,66 @@ func MustNew[T Value](key, val string) Entry[T] { return out } -func (o *Entry[T]) Init(key string) { - o.val = create[T]() - o.key = key +func (e *Entry[T]) val() T { + if reflect.ValueOf(e.v).IsNil() { + e.v = create[T]() + } + return e.v } -func (o *Entry[T]) Set(s string) error { - if reflect.ValueOf(o.val).IsNil() { - return xerrors.Errorf("instance of %T is uninitialized", o.val) +func (e *Entry[T]) key() (string, error) { + if e.k == "" { + return "", ErrKeyNotSet } - return o.val.Set(s) + + return e.k, nil } -func (o *Entry[T]) Type() string { - return o.val.Type() +func (e *Entry[T]) SetKey(k string) { + e.k = k } -func (o *Entry[T]) String() string { - return o.val.String() +func (e *Entry[T]) SetStartupValue(s string) error { + return e.val().Set(s) } -func (o *Entry[T]) StartupValue() T { - return o.val +func (e *Entry[T]) MustSet(s string) { + err := e.val().Set(s) + if err != nil { + panic(err) + } } -func (o *Entry[T]) Resolve(ctx context.Context, r Resolver) (T, error) { - return o.resolve(ctx, r) +func (e *Entry[T]) Type() string { + return e.val().Type() } -func (o *Entry[T]) resolve(ctx context.Context, r Resolver) (T, error) { +func (e *Entry[T]) String() string { + return e.val().String() +} + +func (e *Entry[T]) StartupValue() T { + return e.val() +} + +func (e *Entry[T]) SetRuntimeValue(ctx context.Context, m Mutator, val T) error { + key, err := e.key() + if err != nil { + return err + } + + return m.MutateByKey(ctx, key, val.String()) +} + +func (e *Entry[T]) Resolve(ctx context.Context, r Resolver) (T, error) { var zero T - val, err := r.ResolveByKey(ctx, o.key) + key, err := e.key() + if err != nil { + return zero, err + } + + val, err := r.ResolveByKey(ctx, key) if err != nil { return zero, err } @@ -78,17 +108,13 @@ func (o *Entry[T]) resolve(ctx context.Context, r Resolver) (T, error) { return inst, nil } -func (o *Entry[T]) Save(ctx context.Context, m Mutator, val T) error { - return m.MutateByKey(ctx, o.key, val.String()) -} - -func (o *Entry[T]) Coalesce(ctx context.Context, r Resolver) (T, error) { +func (e *Entry[T]) Coalesce(ctx context.Context, r Resolver) (T, error) { var zero T - resolved, err := o.resolve(ctx, r) + resolved, err := e.Resolve(ctx, r) if err != nil { if errors.Is(err, EntryNotFound) { - return o.StartupValue(), nil + return e.StartupValue(), nil } return zero, err } diff --git a/coderd/runtimeconfig/config_test.go b/coderd/runtimeconfig/config_test.go index 2cce6b2854a0d..56a4ee43ff3c2 100644 --- a/coderd/runtimeconfig/config_test.go +++ b/coderd/runtimeconfig/config_test.go @@ -1,6 +1,7 @@ package runtimeconfig_test import ( + "context" "testing" "github.com/coder/serpent" @@ -8,6 +9,7 @@ import ( "github.com/coder/coder/v2/coderd/coderdtest" "github.com/coder/coder/v2/coderd/runtimeconfig" + "github.com/coder/coder/v2/coderd/util/ptr" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/enterprise/coderd/coderdenttest" "github.com/coder/coder/v2/enterprise/coderd/license" @@ -30,20 +32,39 @@ func TestConfig(t *testing.T) { }) altOrg := coderdenttest.CreateOrganization(t, adminClient, coderdenttest.CreateOrganizationOptions{}) - t.Run("panics unless initialized", func(t *testing.T) { + t.Run("new", func(t *testing.T) { t.Parallel() - field := runtimeconfig.Entry[*serpent.String]{} require.Panics(t, func() { - field.StartupValue().String() + // "hello" cannot be set on a *serpent.Float64 field. + runtimeconfig.MustNew[*serpent.Float64]("key", "hello") }) - field.Init("my-field") require.NotPanics(t, func() { - field.StartupValue().String() + runtimeconfig.MustNew[*serpent.Float64]("key", "91.1234") }) }) + t.Run("zero", func(t *testing.T) { + t.Parallel() + + // A zero-value declaration of a runtimeconfig.Entry should behave as a zero value of the generic type. + // NB! A key has not been set for this entry. + 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(), runtimeconfig.NewNoopResolver()) + require.ErrorIs(t, err, runtimeconfig.ErrKeyNotSet) + // But attempting to set the runtime value will produce an error. + val := serpent.BoolOf(ptr.Ref(true)) + require.ErrorIs(t, field.SetRuntimeValue(context.Background(), runtimeconfig.NewNoopMutator(), val), runtimeconfig.ErrKeyNotSet) + }) + t.Run("simple", func(t *testing.T) { t.Parallel() @@ -57,12 +78,9 @@ func TestConfig(t *testing.T) { override = serpent.String("dogfood@dev.coder.com") ) - field := runtimeconfig.Entry[*serpent.String]{} - field.Init("my-field") - // Check that no default has been set. - require.Empty(t, field.StartupValue().String()) - // Initialize the value. - require.NoError(t, field.Set(base.String())) + 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. @@ -73,7 +91,7 @@ func TestConfig(t *testing.T) { require.NoError(t, err) require.Equal(t, base.String(), val.String()) // Set an org-level override. - require.NoError(t, field.Save(ctx, mutator, &override)) + require.NoError(t, field.SetRuntimeValue(ctx, mutator, &override)) // Coalesce now returns the org-level value. val, err = field.Coalesce(ctx, resolver) require.NoError(t, err) @@ -88,8 +106,6 @@ func TestConfig(t *testing.T) { resolver := runtimeconfig.NewOrgResolver(altOrg.ID, runtimeconfig.NewStoreResolver(store)) mutator := runtimeconfig.NewOrgMutator(altOrg.ID, runtimeconfig.NewStoreMutator(store)) - field := runtimeconfig.Entry[*serpent.Struct[map[string]string]]{} - field.Init("my-field") var ( base = serpent.Struct[map[string]string]{ Value: map[string]string{"access_type": "offline"}, @@ -102,10 +118,10 @@ func TestConfig(t *testing.T) { } ) - // Check that no default has been set. - require.Empty(t, field.StartupValue().Value) - // Initialize the value. - require.NoError(t, field.Set(base.String())) + 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, resolver) require.ErrorIs(t, err, runtimeconfig.EntryNotFound) @@ -114,7 +130,7 @@ func TestConfig(t *testing.T) { require.NoError(t, err) require.Equal(t, base.Value, val.Value) // Set an org-level override. - require.NoError(t, field.Save(ctx, mutator, &override)) + require.NoError(t, field.SetRuntimeValue(ctx, mutator, &override)) // Coalesce now returns the org-level value. structVal, err := field.Resolve(ctx, resolver) require.NoError(t, err) diff --git a/coderd/runtimeconfig/mutator.go b/coderd/runtimeconfig/mutator.go index 1f377874caf05..bb76e0c42641d 100644 --- a/coderd/runtimeconfig/mutator.go +++ b/coderd/runtimeconfig/mutator.go @@ -38,3 +38,13 @@ func NewOrgMutator(orgID uuid.UUID, inner Mutator) *OrgMutator { func (m OrgMutator) MutateByKey(ctx context.Context, key, val string) error { return m.inner.MutateByKey(ctx, orgKey(m.orgID, key), val) } + +type NoopMutator struct{} + +func (n *NoopMutator) MutateByKey(ctx context.Context, key, val string) error { + return nil +} + +func NewNoopMutator() *NoopMutator { + return &NoopMutator{} +} diff --git a/coderd/runtimeconfig/spec.go b/coderd/runtimeconfig/spec.go index b3e6db23c06d7..7a1f7e70fcfd3 100644 --- a/coderd/runtimeconfig/spec.go +++ b/coderd/runtimeconfig/spec.go @@ -6,16 +6,6 @@ type Initializer interface { Init(key string) } -// type RuntimeConfigResolver[T Value] interface { -// StartupValue() T -// Resolve(r Resolver) (T, error) -// Coalesce(r Resolver) (T, error) -// } -// -// type RuntimeConfigMutator[T Value] interface { -// Save(m Mutator, val T) error -// } - type Resolver interface { ResolveByKey(ctx context.Context, key string) (string, error) } From f97ca897c681cb8c74a85c163d950c8f569a24a3 Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Tue, 3 Sep 2024 14:19:38 +0200 Subject: [PATCH 03/19] Add database queries against site_configs Signed-off-by: Danny Kopping --- coderd/database/dbauthz/dbauthz.go | 15 +++++++++ coderd/database/dbmem/dbmem.go | 36 ++++++++++++++++++++ coderd/database/dbmetrics/dbmetrics.go | 21 ++++++++++++ coderd/database/querier.go | 3 ++ coderd/database/queries.sql.go | 36 ++++++++++++++++++++ coderd/database/queries/siteconfig.sql | 11 ++++++ coderd/runtimeconfig/config_test.go | 5 +-- coderd/runtimeconfig/mutator.go | 4 ++- coderd/runtimeconfig/resolver.go | 2 +- coderd/runtimeconfig/store.go | 46 +++----------------------- 10 files changed, 134 insertions(+), 45 deletions(-) diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index f6bd03cc50e8b..1520c73f6ad60 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -1183,6 +1183,11 @@ func (q *querier) DeleteReplicasUpdatedBefore(ctx context.Context, updatedAt tim return q.db.DeleteReplicasUpdatedBefore(ctx, updatedAt) } +func (q *querier) DeleteRuntimeConfig(ctx context.Context, key string) error { + // TODO: auth + return q.db.DeleteRuntimeConfig(ctx, key) +} + func (q *querier) DeleteTailnetAgent(ctx context.Context, arg database.DeleteTailnetAgentParams) (database.DeleteTailnetAgentRow, error) { if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceTailnetCoordinator); err != nil { return database.DeleteTailnetAgentRow{}, err @@ -1856,6 +1861,11 @@ func (q *querier) GetReplicasUpdatedAfter(ctx context.Context, updatedAt time.Ti return q.db.GetReplicasUpdatedAfter(ctx, updatedAt) } +func (q *querier) GetRuntimeConfig(ctx context.Context, key string) (string, error) { + // TODO: auth + return q.db.GetRuntimeConfig(ctx, key) +} + func (q *querier) GetTailnetAgents(ctx context.Context, id uuid.UUID) ([]database.TailnetAgent, error) { if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceTailnetCoordinator); err != nil { return nil, err @@ -3906,6 +3916,11 @@ func (q *querier) UpsertProvisionerDaemon(ctx context.Context, arg database.Upse return q.db.UpsertProvisionerDaemon(ctx, arg) } +func (q *querier) UpsertRuntimeConfig(ctx context.Context, arg database.UpsertRuntimeConfigParams) error { + // TODO: auth + return q.db.UpsertRuntimeConfig(ctx, arg) +} + func (q *querier) UpsertTailnetAgent(ctx context.Context, arg database.UpsertTailnetAgentParams) (database.TailnetAgent, error) { if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceTailnetCoordinator); err != nil { return database.TailnetAgent{}, err diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go index b1d2178e66a29..583526af0e49a 100644 --- a/coderd/database/dbmem/dbmem.go +++ b/coderd/database/dbmem/dbmem.go @@ -22,6 +22,7 @@ import ( "golang.org/x/xerrors" "github.com/coder/coder/v2/coderd/notifications/types" + "github.com/coder/coder/v2/coderd/runtimeconfig" "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/dbtime" @@ -84,6 +85,7 @@ func New() database.Store { workspaceProxies: make([]database.WorkspaceProxy, 0), customRoles: make([]database.CustomRole, 0), locks: map[int64]struct{}{}, + runtimeConfig: map[string]string{}, }, } // Always start with a default org. Matching migration 198. @@ -194,6 +196,7 @@ type data struct { workspaces []database.Workspace workspaceProxies []database.WorkspaceProxy customRoles []database.CustomRole + runtimeConfig map[string]string // Locks is a map of lock names. Any keys within the map are currently // locked. locks map[int64]struct{} @@ -1928,6 +1931,14 @@ func (q *FakeQuerier) DeleteReplicasUpdatedBefore(_ context.Context, before time return nil } +func (q *FakeQuerier) DeleteRuntimeConfig(_ context.Context, key string) error { + q.mutex.Lock() + defer q.mutex.Unlock() + + delete(q.runtimeConfig, key) + return nil +} + func (*FakeQuerier) DeleteTailnetAgent(context.Context, database.DeleteTailnetAgentParams) (database.DeleteTailnetAgentRow, error) { return database.DeleteTailnetAgentRow{}, ErrUnimplemented } @@ -3505,6 +3516,18 @@ func (q *FakeQuerier) GetReplicasUpdatedAfter(_ context.Context, updatedAt time. return replicas, nil } +func (q *FakeQuerier) GetRuntimeConfig(_ context.Context, key string) (string, error) { + q.mutex.Lock() + defer q.mutex.Unlock() + + val, ok := q.runtimeConfig[key] + if !ok { + return "", runtimeconfig.EntryNotFound + } + + return val, nil +} + func (*FakeQuerier) GetTailnetAgents(context.Context, uuid.UUID) ([]database.TailnetAgent, error) { return nil, ErrUnimplemented } @@ -9186,6 +9209,19 @@ func (q *FakeQuerier) UpsertProvisionerDaemon(_ context.Context, arg database.Up return d, nil } +func (q *FakeQuerier) UpsertRuntimeConfig(ctx context.Context, arg database.UpsertRuntimeConfigParams) error { + err := validateDatabaseType(arg) + if err != nil { + return err + } + + q.mutex.Lock() + defer q.mutex.Unlock() + + q.runtimeConfig[arg.Key] = arg.Value + return nil +} + func (*FakeQuerier) UpsertTailnetAgent(context.Context, database.UpsertTailnetAgentParams) (database.TailnetAgent, error) { return database.TailnetAgent{}, ErrUnimplemented } diff --git a/coderd/database/dbmetrics/dbmetrics.go b/coderd/database/dbmetrics/dbmetrics.go index 38289c143bfd9..5aa3a0c8d8cfb 100644 --- a/coderd/database/dbmetrics/dbmetrics.go +++ b/coderd/database/dbmetrics/dbmetrics.go @@ -347,6 +347,13 @@ func (m metricsStore) DeleteReplicasUpdatedBefore(ctx context.Context, updatedAt return err } +func (m metricsStore) DeleteRuntimeConfig(ctx context.Context, key string) error { + start := time.Now() + r0 := m.s.DeleteRuntimeConfig(ctx, key) + m.queryLatencies.WithLabelValues("DeleteRuntimeConfig").Observe(time.Since(start).Seconds()) + return r0 +} + func (m metricsStore) DeleteTailnetAgent(ctx context.Context, arg database.DeleteTailnetAgentParams) (database.DeleteTailnetAgentRow, error) { start := time.Now() r0, r1 := m.s.DeleteTailnetAgent(ctx, arg) @@ -991,6 +998,13 @@ func (m metricsStore) GetReplicasUpdatedAfter(ctx context.Context, updatedAt tim return replicas, err } +func (m metricsStore) GetRuntimeConfig(ctx context.Context, key string) (string, error) { + start := time.Now() + r0, r1 := m.s.GetRuntimeConfig(ctx, key) + m.queryLatencies.WithLabelValues("GetRuntimeConfig").Observe(time.Since(start).Seconds()) + return r0, r1 +} + func (m metricsStore) GetTailnetAgents(ctx context.Context, id uuid.UUID) ([]database.TailnetAgent, error) { start := time.Now() r0, r1 := m.s.GetTailnetAgents(ctx, id) @@ -2454,6 +2468,13 @@ func (m metricsStore) UpsertProvisionerDaemon(ctx context.Context, arg database. return r0, r1 } +func (m metricsStore) UpsertRuntimeConfig(ctx context.Context, arg database.UpsertRuntimeConfigParams) error { + start := time.Now() + r0 := m.s.UpsertRuntimeConfig(ctx, arg) + m.queryLatencies.WithLabelValues("UpsertRuntimeConfig").Observe(time.Since(start).Seconds()) + return r0 +} + func (m metricsStore) UpsertTailnetAgent(ctx context.Context, arg database.UpsertTailnetAgentParams) (database.TailnetAgent, error) { start := time.Now() r0, r1 := m.s.UpsertTailnetAgent(ctx, arg) diff --git a/coderd/database/querier.go b/coderd/database/querier.go index c614a03834a9b..3432bac7dada1 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -96,6 +96,7 @@ type sqlcQuerier interface { DeleteOrganizationMember(ctx context.Context, arg DeleteOrganizationMemberParams) error DeleteProvisionerKey(ctx context.Context, id uuid.UUID) error DeleteReplicasUpdatedBefore(ctx context.Context, updatedAt time.Time) error + DeleteRuntimeConfig(ctx context.Context, key string) error DeleteTailnetAgent(ctx context.Context, arg DeleteTailnetAgentParams) (DeleteTailnetAgentRow, error) DeleteTailnetClient(ctx context.Context, arg DeleteTailnetClientParams) (DeleteTailnetClientRow, error) DeleteTailnetClientSubscription(ctx context.Context, arg DeleteTailnetClientSubscriptionParams) error @@ -199,6 +200,7 @@ type sqlcQuerier interface { GetQuotaConsumedForUser(ctx context.Context, arg GetQuotaConsumedForUserParams) (int64, error) GetReplicaByID(ctx context.Context, id uuid.UUID) (Replica, error) GetReplicasUpdatedAfter(ctx context.Context, updatedAt time.Time) ([]Replica, error) + GetRuntimeConfig(ctx context.Context, key string) (string, error) GetTailnetAgents(ctx context.Context, id uuid.UUID) ([]TailnetAgent, error) GetTailnetClientsForAgent(ctx context.Context, agentID uuid.UUID) ([]TailnetClient, error) GetTailnetPeers(ctx context.Context, id uuid.UUID) ([]TailnetPeer, error) @@ -478,6 +480,7 @@ type sqlcQuerier interface { UpsertNotificationsSettings(ctx context.Context, value string) error UpsertOAuthSigningKey(ctx context.Context, value string) error UpsertProvisionerDaemon(ctx context.Context, arg UpsertProvisionerDaemonParams) (ProvisionerDaemon, error) + UpsertRuntimeConfig(ctx context.Context, arg UpsertRuntimeConfigParams) error UpsertTailnetAgent(ctx context.Context, arg UpsertTailnetAgentParams) (TailnetAgent, error) UpsertTailnetClient(ctx context.Context, arg UpsertTailnetClientParams) (TailnetClient, error) UpsertTailnetClientSubscription(ctx context.Context, arg UpsertTailnetClientSubscriptionParams) error diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index fc388e55247d0..1267449cf3d98 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -6703,6 +6703,16 @@ func (q *sqlQuerier) UpdateCustomRole(ctx context.Context, arg UpdateCustomRoleP return i, err } +const deleteRuntimeConfig = `-- name: DeleteRuntimeConfig :exec +DELETE FROM site_configs +WHERE site_configs.key = $1 +` + +func (q *sqlQuerier) DeleteRuntimeConfig(ctx context.Context, key string) error { + _, err := q.db.ExecContext(ctx, deleteRuntimeConfig, key) + return err +} + const getAnnouncementBanners = `-- name: GetAnnouncementBanners :one SELECT value FROM site_configs WHERE key = 'announcement_banners' ` @@ -6844,6 +6854,17 @@ func (q *sqlQuerier) GetOAuthSigningKey(ctx context.Context) (string, error) { return value, err } +const getRuntimeConfig = `-- name: GetRuntimeConfig :one +SELECT value FROM site_configs WHERE site_configs.key = $1 +` + +func (q *sqlQuerier) GetRuntimeConfig(ctx context.Context, key string) (string, error) { + row := q.db.QueryRowContext(ctx, getRuntimeConfig, key) + var value string + err := row.Scan(&value) + return value, err +} + const insertDERPMeshKey = `-- name: InsertDERPMeshKey :exec INSERT INTO site_configs (key, value) VALUES ('derp_mesh_key', $1) ` @@ -6975,6 +6996,21 @@ func (q *sqlQuerier) UpsertOAuthSigningKey(ctx context.Context, value string) er return err } +const upsertRuntimeConfig = `-- name: UpsertRuntimeConfig :exec +INSERT INTO site_configs (key, value) VALUES ($1, $2) +ON CONFLICT (key) DO UPDATE SET value = $2 WHERE site_configs.key = $1 +` + +type UpsertRuntimeConfigParams struct { + Key string `db:"key" json:"key"` + Value string `db:"value" json:"value"` +} + +func (q *sqlQuerier) UpsertRuntimeConfig(ctx context.Context, arg UpsertRuntimeConfigParams) error { + _, err := q.db.ExecContext(ctx, upsertRuntimeConfig, arg.Key, arg.Value) + return err +} + const cleanTailnetCoordinators = `-- name: CleanTailnetCoordinators :exec DELETE FROM tailnet_coordinators diff --git a/coderd/database/queries/siteconfig.sql b/coderd/database/queries/siteconfig.sql index 877f5ee237122..e8d02372e5a4f 100644 --- a/coderd/database/queries/siteconfig.sql +++ b/coderd/database/queries/siteconfig.sql @@ -96,3 +96,14 @@ SELECT INSERT INTO site_configs (key, value) VALUES ('notifications_settings', $1) ON CONFLICT (key) DO UPDATE SET value = $1 WHERE site_configs.key = 'notifications_settings'; +-- name: GetRuntimeConfig :one +SELECT value FROM site_configs WHERE site_configs.key = $1; + +-- name: UpsertRuntimeConfig :exec +INSERT INTO site_configs (key, value) VALUES ($1, $2) +ON CONFLICT (key) DO UPDATE SET value = $2 WHERE site_configs.key = $1; + +-- name: DeleteRuntimeConfig :exec +DELETE FROM site_configs +WHERE site_configs.key = $1; + diff --git a/coderd/runtimeconfig/config_test.go b/coderd/runtimeconfig/config_test.go index 56a4ee43ff3c2..61049e626e6ed 100644 --- a/coderd/runtimeconfig/config_test.go +++ b/coderd/runtimeconfig/config_test.go @@ -8,6 +8,7 @@ import ( "github.com/stretchr/testify/require" "github.com/coder/coder/v2/coderd/coderdtest" + "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/codersdk" @@ -69,7 +70,7 @@ func TestConfig(t *testing.T) { t.Parallel() ctx := testutil.Context(t, testutil.WaitShort) - store := runtimeconfig.NewInMemoryStore() + store := dbmem.New() resolver := runtimeconfig.NewOrgResolver(altOrg.ID, runtimeconfig.NewStoreResolver(store)) mutator := runtimeconfig.NewOrgMutator(altOrg.ID, runtimeconfig.NewStoreMutator(store)) @@ -102,7 +103,7 @@ func TestConfig(t *testing.T) { t.Parallel() ctx := testutil.Context(t, testutil.WaitShort) - store := runtimeconfig.NewInMemoryStore() + store := dbmem.New() resolver := runtimeconfig.NewOrgResolver(altOrg.ID, runtimeconfig.NewStoreResolver(store)) mutator := runtimeconfig.NewOrgMutator(altOrg.ID, runtimeconfig.NewStoreMutator(store)) diff --git a/coderd/runtimeconfig/mutator.go b/coderd/runtimeconfig/mutator.go index bb76e0c42641d..e33f2de82d193 100644 --- a/coderd/runtimeconfig/mutator.go +++ b/coderd/runtimeconfig/mutator.go @@ -5,6 +5,8 @@ import ( "github.com/google/uuid" "golang.org/x/xerrors" + + "github.com/coder/coder/v2/coderd/database" ) type StoreMutator struct { @@ -19,7 +21,7 @@ func NewStoreMutator(store Store) *StoreMutator { } func (s *StoreMutator) MutateByKey(ctx context.Context, key, val string) error { - err := s.store.UpsertRuntimeSetting(ctx, key, val) + err := s.store.UpsertRuntimeConfig(ctx, database.UpsertRuntimeConfigParams{Key: key, Value: val}) if err != nil { return xerrors.Errorf("update %q: %w", err) } diff --git a/coderd/runtimeconfig/resolver.go b/coderd/runtimeconfig/resolver.go index 7eb47833697c4..b43dcd851ac34 100644 --- a/coderd/runtimeconfig/resolver.go +++ b/coderd/runtimeconfig/resolver.go @@ -22,7 +22,7 @@ func (s StoreResolver) ResolveByKey(ctx context.Context, key string) (string, er panic("developer error: store must be set") } - val, err := s.store.GetRuntimeSetting(ctx, key) + val, err := s.store.GetRuntimeConfig(ctx, key) if err != nil { if errors.Is(err, sql.ErrNoRows) { return "", xerrors.Errorf("%q: %w", key, EntryNotFound) diff --git a/coderd/runtimeconfig/store.go b/coderd/runtimeconfig/store.go index 781563b523331..16f12340bbf68 100644 --- a/coderd/runtimeconfig/store.go +++ b/coderd/runtimeconfig/store.go @@ -2,52 +2,16 @@ package runtimeconfig import ( "context" - "sync" "golang.org/x/xerrors" + + "github.com/coder/coder/v2/coderd/database" ) var EntryNotFound = xerrors.New("entry not found") type Store interface { - GetRuntimeSetting(ctx context.Context, key string) (string, error) - UpsertRuntimeSetting(ctx context.Context, key, value string) error - DeleteRuntimeSetting(ctx context.Context, key string) error -} - -type InMemoryStore struct { - mu sync.Mutex - store map[string]string -} - -func NewInMemoryStore() *InMemoryStore { - return &InMemoryStore{store: make(map[string]string)} -} - -func (s *InMemoryStore) GetRuntimeSetting(_ context.Context, key string) (string, error) { - s.mu.Lock() - defer s.mu.Unlock() - - val, ok := s.store[key] - if !ok { - return "", EntryNotFound - } - - return val, nil -} - -func (s *InMemoryStore) UpsertRuntimeSetting(_ context.Context, key, value string) error { - s.mu.Lock() - defer s.mu.Unlock() - - s.store[key] = value - return nil -} - -func (s *InMemoryStore) DeleteRuntimeSetting(_ context.Context, key string) error { - s.mu.Lock() - defer s.mu.Unlock() - - delete(s.store, key) - return nil + GetRuntimeConfig(ctx context.Context, key string) (string, error) + UpsertRuntimeConfig(ctx context.Context, arg database.UpsertRuntimeConfigParams) error + DeleteRuntimeConfig(ctx context.Context, key string) error } From b2b43c8c56f37d11266d68ff2c025c77c0f493b7 Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Tue, 3 Sep 2024 14:55:34 +0200 Subject: [PATCH 04/19] Document usage in test Signed-off-by: Danny Kopping --- coderd/runtimeconfig/config.go | 20 +++++- coderd/runtimeconfig/config_test.go | 97 +++++++++++++++++++++++++---- coderd/runtimeconfig/mutator.go | 24 +++++-- coderd/runtimeconfig/resolver.go | 8 +-- coderd/runtimeconfig/spec.go | 7 ++- 5 files changed, 129 insertions(+), 27 deletions(-) diff --git a/coderd/runtimeconfig/config.go b/coderd/runtimeconfig/config.go index 17de13e9eb7d7..f4eca40d6af04 100644 --- a/coderd/runtimeconfig/config.go +++ b/coderd/runtimeconfig/config.go @@ -56,6 +56,10 @@ func (e *Entry[T]) SetKey(k string) { e.k = k } +func (e *Entry[T]) Set(s string) error { + return e.SetStartupValue(s) +} + func (e *Entry[T]) SetStartupValue(s string) error { return e.val().Set(s) } @@ -75,6 +79,9 @@ 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() } @@ -85,7 +92,16 @@ func (e *Entry[T]) SetRuntimeValue(ctx context.Context, m Mutator, val T) error return err } - return m.MutateByKey(ctx, key, val.String()) + return m.UpsertRuntimeSetting(ctx, key, val.String()) +} + +func (e *Entry[T]) UnsetRuntimeValue(ctx context.Context, m Mutator) error { + key, err := e.key() + if err != nil { + return err + } + + return m.DeleteRuntimeSetting(ctx, key) } func (e *Entry[T]) Resolve(ctx context.Context, r Resolver) (T, error) { @@ -96,7 +112,7 @@ func (e *Entry[T]) Resolve(ctx context.Context, r Resolver) (T, error) { return zero, err } - val, err := r.ResolveByKey(ctx, key) + val, err := r.GetRuntimeSetting(ctx, key) if err != nil { return zero, err } diff --git a/coderd/runtimeconfig/config_test.go b/coderd/runtimeconfig/config_test.go index 61049e626e6ed..68e78b9011c96 100644 --- a/coderd/runtimeconfig/config_test.go +++ b/coderd/runtimeconfig/config_test.go @@ -17,21 +17,77 @@ import ( "github.com/coder/coder/v2/testutil" ) +func TestUsage(t *testing.T) { + 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() + + _, altOrg := setup(t) + + ctx := testutil.Context(t, testutil.WaitShort) + store := dbmem.New() + resolver := runtimeconfig.NewOrgResolver(altOrg.ID, runtimeconfig.NewStoreResolver(store)) + mutator := runtimeconfig.NewOrgMutator(altOrg.ID, runtimeconfig.NewStoreMutator(store)) + + // 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 key on the runtimeconfig.Entry. + // Attempting to perform any operation which accesses the store will enforce the need for a key. + _, err := field.Resolve(ctx, resolver) + require.ErrorIs(t, err, runtimeconfig.ErrKeyNotSet) + + // Let's see that key. The environment var name is likely to be the most stable. + field.SetKey(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, mutator, &newVal)) + + // ...and we can retrieve the value, as well. + resolved, err := field.Resolve(ctx, resolver) + require.NoError(t, err) + require.Equal(t, newVal.String(), resolved.String()) + + // We can also remove the runtime config. + require.NoError(t, field.UnsetRuntimeValue(ctx, mutator)) + }) +} + // TestConfig demonstrates creating org-level overrides for deployment-level settings. func TestConfig(t *testing.T) { t.Parallel() - vals := coderdtest.DeploymentValues(t) - vals.Experiments = []string{string(codersdk.ExperimentMultiOrganization)} - adminClient, _, _, _ := coderdenttest.NewWithAPI(t, &coderdenttest.Options{ - Options: &coderdtest.Options{DeploymentValues: vals}, - LicenseOptions: &coderdenttest.LicenseOptions{ - Features: license.Features{ - codersdk.FeatureMultipleOrganizations: 1, - }, - }, - }) - altOrg := coderdenttest.CreateOrganization(t, adminClient, coderdenttest.CreateOrganizationOptions{}) + _, altOrg := setup(t) t.Run("new", func(t *testing.T) { t.Parallel() @@ -119,7 +175,7 @@ func TestConfig(t *testing.T) { } ) - field := runtimeconfig.MustNew[*serpent.Struct[map[string]string]]("my-field", base.String()) + 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()) @@ -138,3 +194,20 @@ func TestConfig(t *testing.T) { require.Equal(t, override.Value, structVal.Value) }) } + +// setup creates a new API, enabled notifications + multi-org experiments, and returns the API client and a new org. +func setup(t *testing.T) (*codersdk.Client, codersdk.Organization) { + t.Helper() + + vals := coderdtest.DeploymentValues(t) + vals.Experiments = []string{string(codersdk.ExperimentMultiOrganization)} + adminClient, _, _, _ := coderdenttest.NewWithAPI(t, &coderdenttest.Options{ + Options: &coderdtest.Options{DeploymentValues: vals}, + LicenseOptions: &coderdenttest.LicenseOptions{ + Features: license.Features{ + codersdk.FeatureMultipleOrganizations: 1, + }, + }, + }) + return adminClient, coderdenttest.CreateOrganization(t, adminClient, coderdenttest.CreateOrganizationOptions{}) +} diff --git a/coderd/runtimeconfig/mutator.go b/coderd/runtimeconfig/mutator.go index e33f2de82d193..e66296fae570c 100644 --- a/coderd/runtimeconfig/mutator.go +++ b/coderd/runtimeconfig/mutator.go @@ -20,7 +20,7 @@ func NewStoreMutator(store Store) *StoreMutator { return &StoreMutator{store} } -func (s *StoreMutator) MutateByKey(ctx context.Context, key, val string) error { +func (s StoreMutator) UpsertRuntimeSetting(ctx context.Context, key, val string) error { err := s.store.UpsertRuntimeConfig(ctx, database.UpsertRuntimeConfigParams{Key: key, Value: val}) if err != nil { return xerrors.Errorf("update %q: %w", err) @@ -28,6 +28,10 @@ func (s *StoreMutator) MutateByKey(ctx context.Context, key, val string) error { return nil } +func (s StoreMutator) DeleteRuntimeSetting(ctx context.Context, key string) error { + return s.store.DeleteRuntimeConfig(ctx, key) +} + type OrgMutator struct { inner Mutator orgID uuid.UUID @@ -37,16 +41,24 @@ func NewOrgMutator(orgID uuid.UUID, inner Mutator) *OrgMutator { return &OrgMutator{inner: inner, orgID: orgID} } -func (m OrgMutator) MutateByKey(ctx context.Context, key, val string) error { - return m.inner.MutateByKey(ctx, orgKey(m.orgID, key), val) +func (m OrgMutator) UpsertRuntimeSetting(ctx context.Context, key, val string) error { + return m.inner.UpsertRuntimeSetting(ctx, orgKey(m.orgID, key), val) +} + +func (m OrgMutator) DeleteRuntimeSetting(ctx context.Context, key string) error { + return m.inner.DeleteRuntimeSetting(ctx, key) } type NoopMutator struct{} -func (n *NoopMutator) MutateByKey(ctx context.Context, key, val string) error { +func NewNoopMutator() *NoopMutator { + return &NoopMutator{} +} + +func (n NoopMutator) UpsertRuntimeSetting(context.Context, string, string) error { return nil } -func NewNoopMutator() *NoopMutator { - return &NoopMutator{} +func (n NoopMutator) DeleteRuntimeSetting(context.Context, string) error { + return nil } diff --git a/coderd/runtimeconfig/resolver.go b/coderd/runtimeconfig/resolver.go index b43dcd851ac34..0b1b8a7c5a456 100644 --- a/coderd/runtimeconfig/resolver.go +++ b/coderd/runtimeconfig/resolver.go @@ -17,7 +17,7 @@ func NewStoreResolver(store Store) *StoreResolver { return &StoreResolver{store} } -func (s StoreResolver) ResolveByKey(ctx context.Context, key string) (string, error) { +func (s StoreResolver) GetRuntimeSetting(ctx context.Context, key string) (string, error) { if s.store == nil { panic("developer error: store must be set") } @@ -46,8 +46,8 @@ func NewOrgResolver(orgID uuid.UUID, inner Resolver) *OrgResolver { return &OrgResolver{inner: inner, orgID: orgID} } -func (r OrgResolver) ResolveByKey(ctx context.Context, key string) (string, error) { - return r.inner.ResolveByKey(ctx, orgKey(r.orgID, key)) +func (r OrgResolver) GetRuntimeSetting(ctx context.Context, key string) (string, error) { + return r.inner.GetRuntimeSetting(ctx, orgKey(r.orgID, key)) } // NoopResolver will always fail to resolve the given key. @@ -58,6 +58,6 @@ func NewNoopResolver() *NoopResolver { return &NoopResolver{} } -func (n NoopResolver) ResolveByKey(context.Context, string) (string, error) { +func (n NoopResolver) GetRuntimeSetting(context.Context, string) (string, error) { return "", EntryNotFound } diff --git a/coderd/runtimeconfig/spec.go b/coderd/runtimeconfig/spec.go index 7a1f7e70fcfd3..cefaf2e0d5b77 100644 --- a/coderd/runtimeconfig/spec.go +++ b/coderd/runtimeconfig/spec.go @@ -7,9 +7,10 @@ type Initializer interface { } type Resolver interface { - ResolveByKey(ctx context.Context, key string) (string, error) + GetRuntimeSetting(ctx context.Context, key string) (string, error) } type Mutator interface { - MutateByKey(ctx context.Context, key, val string) error -} \ No newline at end of file + UpsertRuntimeSetting(ctx context.Context, key, val string) error + DeleteRuntimeSetting(ctx context.Context, key string) error +} From e67da12bb14e9f601cc5bda5d34e6c6e183a7d3d Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Tue, 3 Sep 2024 15:22:59 +0200 Subject: [PATCH 05/19] Touchups Signed-off-by: Danny Kopping --- coderd/runtimeconfig/config.go | 30 +++++++++++++++++++++++++----- coderd/runtimeconfig/spec.go | 9 +++++---- 2 files changed, 30 insertions(+), 9 deletions(-) diff --git a/coderd/runtimeconfig/config.go b/coderd/runtimeconfig/config.go index f4eca40d6af04..c003e4307b68b 100644 --- a/coderd/runtimeconfig/config.go +++ b/coderd/runtimeconfig/config.go @@ -11,14 +11,19 @@ import ( var ErrKeyNotSet = xerrors.New("key is not set") -// TODO: comment +// 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. type Entry[T Value] struct { k string v T } +// New creates a new T instance with a defined key and value. func New[T Value](key, val string) (out Entry[T], err error) { out.k = key @@ -29,6 +34,7 @@ func New[T Value](key, val string) (out Entry[T], err error) { return out, nil } +// MustNew is like New but panics if an error occurs. func MustNew[T Value](key, val string) Entry[T] { out, err := New[T](key, val) if err != nil { @@ -37,6 +43,7 @@ func MustNew[T Value](key, val string) Entry[T] { return out } +// 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.v).IsNil() { e.v = create[T]() @@ -44,6 +51,7 @@ func (e *Entry[T]) val() T { return e.v } +// key returns the configured key, or fails with ErrKeyNotSet. func (e *Entry[T]) key() (string, error) { if e.k == "" { return "", ErrKeyNotSet @@ -52,18 +60,17 @@ func (e *Entry[T]) key() (string, error) { return e.k, nil } +// SetKey allows the key to be set. func (e *Entry[T]) SetKey(k string) { e.k = k } +// Set is an alias of SetStartupValue. func (e *Entry[T]) Set(s string) error { return e.SetStartupValue(s) } -func (e *Entry[T]) SetStartupValue(s string) error { - return e.val().Set(s) -} - +// MustSet is like Set but panics on error. func (e *Entry[T]) MustSet(s string) { err := e.val().Set(s) if err != nil { @@ -71,10 +78,18 @@ func (e *Entry[T]) MustSet(s string) { } } +// 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() } @@ -86,6 +101,7 @@ 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 Mutator, val T) error { key, err := e.key() if err != nil { @@ -95,6 +111,7 @@ func (e *Entry[T]) SetRuntimeValue(ctx context.Context, m Mutator, val T) error return m.UpsertRuntimeSetting(ctx, key, val.String()) } +// UnsetRuntimeValue removes the runtime value from the store. func (e *Entry[T]) UnsetRuntimeValue(ctx context.Context, m Mutator) error { key, err := e.key() if err != nil { @@ -104,6 +121,7 @@ func (e *Entry[T]) UnsetRuntimeValue(ctx context.Context, m Mutator) error { return m.DeleteRuntimeSetting(ctx, key) } +// 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 Resolver) (T, error) { var zero T @@ -124,6 +142,8 @@ func (e *Entry[T]) Resolve(ctx context.Context, r Resolver) (T, error) { return inst, nil } +// Coalesce attempts to resolve the runtime value of this field from the store via the given Resolver. Should no runtime +// value be found, the startup value will be used. func (e *Entry[T]) Coalesce(ctx context.Context, r Resolver) (T, error) { var zero T diff --git a/coderd/runtimeconfig/spec.go b/coderd/runtimeconfig/spec.go index cefaf2e0d5b77..e81bf92af0fb4 100644 --- a/coderd/runtimeconfig/spec.go +++ b/coderd/runtimeconfig/spec.go @@ -2,15 +2,16 @@ package runtimeconfig import "context" -type Initializer interface { - Init(key string) -} - +// Resolver is an interface for resolving runtime settings. type Resolver interface { + // GetRuntimeSetting gets a runtime setting by key. GetRuntimeSetting(ctx context.Context, key string) (string, error) } +// Mutator is an interface for mutating runtime settings. type Mutator interface { + // UpsertRuntimeSetting upserts a runtime setting by key. UpsertRuntimeSetting(ctx context.Context, key, val string) error + // DeleteRuntimeSetting deletes a runtime setting by key. DeleteRuntimeSetting(ctx context.Context, key string) error } From 127f1cc21bf98f2e58ad580578b9f8a80f822af2 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 3 Sep 2024 16:52:49 -0500 Subject: [PATCH 06/19] make gen to generate dbmock functions --- coderd/database/dbmock/dbmock.go | 43 ++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/coderd/database/dbmock/dbmock.go b/coderd/database/dbmock/dbmock.go index 1771807f26b2f..6d881cfe6fc1b 100644 --- a/coderd/database/dbmock/dbmock.go +++ b/coderd/database/dbmock/dbmock.go @@ -584,6 +584,20 @@ func (mr *MockStoreMockRecorder) DeleteReplicasUpdatedBefore(arg0, arg1 any) *go return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteReplicasUpdatedBefore", reflect.TypeOf((*MockStore)(nil).DeleteReplicasUpdatedBefore), arg0, arg1) } +// DeleteRuntimeConfig mocks base method. +func (m *MockStore) DeleteRuntimeConfig(arg0 context.Context, arg1 string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "DeleteRuntimeConfig", arg0, arg1) + ret0, _ := ret[0].(error) + return ret0 +} + +// DeleteRuntimeConfig indicates an expected call of DeleteRuntimeConfig. +func (mr *MockStoreMockRecorder) DeleteRuntimeConfig(arg0, arg1 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteRuntimeConfig", reflect.TypeOf((*MockStore)(nil).DeleteRuntimeConfig), arg0, arg1) +} + // DeleteTailnetAgent mocks base method. func (m *MockStore) DeleteTailnetAgent(arg0 context.Context, arg1 database.DeleteTailnetAgentParams) (database.DeleteTailnetAgentRow, error) { m.ctrl.T.Helper() @@ -2019,6 +2033,21 @@ func (mr *MockStoreMockRecorder) GetReplicasUpdatedAfter(arg0, arg1 any) *gomock return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetReplicasUpdatedAfter", reflect.TypeOf((*MockStore)(nil).GetReplicasUpdatedAfter), arg0, arg1) } +// GetRuntimeConfig mocks base method. +func (m *MockStore) GetRuntimeConfig(arg0 context.Context, arg1 string) (string, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetRuntimeConfig", arg0, arg1) + ret0, _ := ret[0].(string) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetRuntimeConfig indicates an expected call of GetRuntimeConfig. +func (mr *MockStoreMockRecorder) GetRuntimeConfig(arg0, arg1 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetRuntimeConfig", reflect.TypeOf((*MockStore)(nil).GetRuntimeConfig), arg0, arg1) +} + // GetTailnetAgents mocks base method. func (m *MockStore) GetTailnetAgents(arg0 context.Context, arg1 uuid.UUID) ([]database.TailnetAgent, error) { m.ctrl.T.Helper() @@ -5151,6 +5180,20 @@ func (mr *MockStoreMockRecorder) UpsertProvisionerDaemon(arg0, arg1 any) *gomock return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpsertProvisionerDaemon", reflect.TypeOf((*MockStore)(nil).UpsertProvisionerDaemon), arg0, arg1) } +// UpsertRuntimeConfig mocks base method. +func (m *MockStore) UpsertRuntimeConfig(arg0 context.Context, arg1 database.UpsertRuntimeConfigParams) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "UpsertRuntimeConfig", arg0, arg1) + ret0, _ := ret[0].(error) + return ret0 +} + +// UpsertRuntimeConfig indicates an expected call of UpsertRuntimeConfig. +func (mr *MockStoreMockRecorder) UpsertRuntimeConfig(arg0, arg1 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpsertRuntimeConfig", reflect.TypeOf((*MockStore)(nil).UpsertRuntimeConfig), arg0, arg1) +} + // UpsertTailnetAgent mocks base method. func (m *MockStore) UpsertTailnetAgent(arg0 context.Context, arg1 database.UpsertTailnetAgentParams) (database.TailnetAgent, error) { m.ctrl.T.Helper() From 43e9e85cce856420f2b79f20e438f2b9f02dd1e0 Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Wed, 4 Sep 2024 09:47:10 +0200 Subject: [PATCH 07/19] make lint/fmt Signed-off-by: Danny Kopping --- coderd/runtimeconfig/resolver.go | 2 +- coderd/runtimeconfig/util.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/coderd/runtimeconfig/resolver.go b/coderd/runtimeconfig/resolver.go index 0b1b8a7c5a456..a8e5d9cb9d21a 100644 --- a/coderd/runtimeconfig/resolver.go +++ b/coderd/runtimeconfig/resolver.go @@ -52,7 +52,7 @@ func (r OrgResolver) GetRuntimeSetting(ctx context.Context, key string) (string, // NoopResolver will always fail to resolve the given key. // Useful in tests where you just want to look up the startup value of configs, and are not concerned with runtime config. -type NoopResolver struct {} +type NoopResolver struct{} func NewNoopResolver() *NoopResolver { return &NoopResolver{} diff --git a/coderd/runtimeconfig/util.go b/coderd/runtimeconfig/util.go index 9281736159211..2899b2947dee7 100644 --- a/coderd/runtimeconfig/util.go +++ b/coderd/runtimeconfig/util.go @@ -14,4 +14,4 @@ func create[T any]() T { func orgKey(orgID uuid.UUID, key string) string { return fmt.Sprintf("%s:%s", orgID.String(), key) -} \ No newline at end of file +} From fa7193eb8a61fce19658cfe70773acec179fe4a9 Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Wed, 4 Sep 2024 09:57:55 +0200 Subject: [PATCH 08/19] Initializer interface, rename "key" to "name" Signed-off-by: Danny Kopping --- coderd/runtimeconfig/config.go | 56 +++++++++++++++-------------- coderd/runtimeconfig/config_test.go | 20 +++++------ coderd/runtimeconfig/spec.go | 16 +++++---- 3 files changed, 49 insertions(+), 43 deletions(-) diff --git a/coderd/runtimeconfig/config.go b/coderd/runtimeconfig/config.go index c003e4307b68b..e67eb786dc342 100644 --- a/coderd/runtimeconfig/config.go +++ b/coderd/runtimeconfig/config.go @@ -9,7 +9,7 @@ import ( "golang.org/x/xerrors" ) -var ErrKeyNotSet = xerrors.New("key is not set") +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. @@ -18,14 +18,15 @@ 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 { - k string - v T + n string + inner T } -// New creates a new T instance with a defined key and value. -func New[T Value](key, val string) (out Entry[T], err error) { - out.k = key +// 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 @@ -35,34 +36,35 @@ func New[T Value](key, val string) (out Entry[T], err error) { } // MustNew is like New but panics if an error occurs. -func MustNew[T Value](key, val string) Entry[T] { - out, err := New[T](key, val) +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.v).IsNil() { - e.v = create[T]() + if reflect.ValueOf(e.inner).IsNil() { + e.inner = create[T]() } - return e.v + return e.inner } -// key returns the configured key, or fails with ErrKeyNotSet. -func (e *Entry[T]) key() (string, error) { - if e.k == "" { - return "", ErrKeyNotSet +// name returns the configured name, or fails with ErrNameNotSet. +func (e *Entry[T]) name() (string, error) { + if e.n == "" { + return "", ErrNameNotSet } - return e.k, nil -} - -// SetKey allows the key to be set. -func (e *Entry[T]) SetKey(k string) { - e.k = k + return e.n, nil } // Set is an alias of SetStartupValue. @@ -103,34 +105,34 @@ func (e *Entry[T]) StartupValue() T { // 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 Mutator, val T) error { - key, err := e.key() + name, err := e.name() if err != nil { return err } - return m.UpsertRuntimeSetting(ctx, key, val.String()) + return m.UpsertRuntimeSetting(ctx, name, val.String()) } // UnsetRuntimeValue removes the runtime value from the store. func (e *Entry[T]) UnsetRuntimeValue(ctx context.Context, m Mutator) error { - key, err := e.key() + name, err := e.name() if err != nil { return err } - return m.DeleteRuntimeSetting(ctx, key) + 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 Resolver) (T, error) { var zero T - key, err := e.key() + name, err := e.name() if err != nil { return zero, err } - val, err := r.GetRuntimeSetting(ctx, key) + val, err := r.GetRuntimeSetting(ctx, name) if err != nil { return zero, err } diff --git a/coderd/runtimeconfig/config_test.go b/coderd/runtimeconfig/config_test.go index 68e78b9011c96..844f817f2ddd8 100644 --- a/coderd/runtimeconfig/config_test.go +++ b/coderd/runtimeconfig/config_test.go @@ -61,13 +61,13 @@ func TestUsage(t *testing.T) { // 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 key on the runtimeconfig.Entry. - // Attempting to perform any operation which accesses the store will enforce the need for a key. + // 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, resolver) - require.ErrorIs(t, err, runtimeconfig.ErrKeyNotSet) + require.ErrorIs(t, err, runtimeconfig.ErrNameNotSet) - // Let's see that key. The environment var name is likely to be the most stable. - field.SetKey(opt.Env) + // 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. @@ -94,11 +94,11 @@ func TestConfig(t *testing.T) { require.Panics(t, func() { // "hello" cannot be set on a *serpent.Float64 field. - runtimeconfig.MustNew[*serpent.Float64]("key", "hello") + runtimeconfig.MustNew[*serpent.Float64]("my-field", "hello") }) require.NotPanics(t, func() { - runtimeconfig.MustNew[*serpent.Float64]("key", "91.1234") + runtimeconfig.MustNew[*serpent.Float64]("my-field", "91.1234") }) }) @@ -106,7 +106,7 @@ func TestConfig(t *testing.T) { t.Parallel() // A zero-value declaration of a runtimeconfig.Entry should behave as a zero value of the generic type. - // NB! A key has not been set for this entry. + // 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()) @@ -116,10 +116,10 @@ func TestConfig(t *testing.T) { // But attempting to resolve will produce an error. _, err := field.Resolve(context.Background(), runtimeconfig.NewNoopResolver()) - require.ErrorIs(t, err, runtimeconfig.ErrKeyNotSet) + 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(), runtimeconfig.NewNoopMutator(), val), runtimeconfig.ErrKeyNotSet) + require.ErrorIs(t, field.SetRuntimeValue(context.Background(), runtimeconfig.NewNoopMutator(), val), runtimeconfig.ErrNameNotSet) }) t.Run("simple", func(t *testing.T) { diff --git a/coderd/runtimeconfig/spec.go b/coderd/runtimeconfig/spec.go index e81bf92af0fb4..f9c61d9429f07 100644 --- a/coderd/runtimeconfig/spec.go +++ b/coderd/runtimeconfig/spec.go @@ -2,16 +2,20 @@ package runtimeconfig import "context" +type Initializer interface { + Initialize(name string) +} + // Resolver is an interface for resolving runtime settings. type Resolver interface { - // GetRuntimeSetting gets a runtime setting by key. - GetRuntimeSetting(ctx context.Context, key string) (string, error) + // GetRuntimeSetting gets a runtime setting by name. + GetRuntimeSetting(ctx context.Context, name string) (string, error) } // Mutator is an interface for mutating runtime settings. type Mutator interface { - // UpsertRuntimeSetting upserts a runtime setting by key. - UpsertRuntimeSetting(ctx context.Context, key, val string) error - // DeleteRuntimeSetting deletes a runtime setting by key. - DeleteRuntimeSetting(ctx context.Context, key 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 } From 31f8cbfe6a73425bedd7d1d95f5e2444ccba77c7 Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Wed, 4 Sep 2024 10:54:12 +0200 Subject: [PATCH 09/19] Manager interface Signed-off-by: Danny Kopping --- coderd/runtimeconfig/spec.go | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/coderd/runtimeconfig/spec.go b/coderd/runtimeconfig/spec.go index f9c61d9429f07..2dd2ca83b79c1 100644 --- a/coderd/runtimeconfig/spec.go +++ b/coderd/runtimeconfig/spec.go @@ -19,3 +19,26 @@ type Mutator interface { // DeleteRuntimeSetting deletes a runtime setting by name. DeleteRuntimeSetting(ctx context.Context, name string) error } + +type Manager interface { + Resolver + Mutator +} + +type NoopManager struct {} + +func NewNoopManager() *NoopManager { + return &NoopManager{} +} + +func (n NoopManager) GetRuntimeSetting(context.Context, string) (string, error) { + return "", EntryNotFound +} + +func (n NoopManager) UpsertRuntimeSetting(context.Context, string, string) error { + return EntryNotFound +} + +func (n NoopManager) DeleteRuntimeSetting(context.Context, string) error { + return EntryNotFound +} From 048506db004189ef15fde138c7e18e8755651a05 Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Wed, 4 Sep 2024 14:04:41 +0200 Subject: [PATCH 10/19] Manager, with scoping Signed-off-by: Danny Kopping --- coderd/runtimeconfig/config.go | 10 +-- coderd/runtimeconfig/config_test.go | 115 ++++++++++++++++------------ coderd/runtimeconfig/manager.go | 80 +++++++++++++++++++ coderd/runtimeconfig/mutator.go | 64 ---------------- coderd/runtimeconfig/resolver.go | 63 --------------- coderd/runtimeconfig/spec.go | 37 ++------- 6 files changed, 160 insertions(+), 209 deletions(-) create mode 100644 coderd/runtimeconfig/manager.go delete mode 100644 coderd/runtimeconfig/mutator.go delete mode 100644 coderd/runtimeconfig/resolver.go diff --git a/coderd/runtimeconfig/config.go b/coderd/runtimeconfig/config.go index e67eb786dc342..25017e663dc1a 100644 --- a/coderd/runtimeconfig/config.go +++ b/coderd/runtimeconfig/config.go @@ -104,7 +104,7 @@ func (e *Entry[T]) StartupValue() T { } // 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 Mutator, val T) error { +func (e *Entry[T]) SetRuntimeValue(ctx context.Context, m Manager, val T) error { name, err := e.name() if err != nil { return err @@ -114,7 +114,7 @@ func (e *Entry[T]) SetRuntimeValue(ctx context.Context, m Mutator, val T) error } // UnsetRuntimeValue removes the runtime value from the store. -func (e *Entry[T]) UnsetRuntimeValue(ctx context.Context, m Mutator) error { +func (e *Entry[T]) UnsetRuntimeValue(ctx context.Context, m Manager) error { name, err := e.name() if err != nil { return err @@ -124,7 +124,7 @@ func (e *Entry[T]) UnsetRuntimeValue(ctx context.Context, m Mutator) error { } // 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 Resolver) (T, error) { +func (e *Entry[T]) Resolve(ctx context.Context, r Manager) (T, error) { var zero T name, err := e.name() @@ -144,9 +144,9 @@ func (e *Entry[T]) Resolve(ctx context.Context, r Resolver) (T, error) { return inst, nil } -// Coalesce attempts to resolve the runtime value of this field from the store via the given Resolver. Should no runtime +// 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 Resolver) (T, error) { +func (e *Entry[T]) Coalesce(ctx context.Context, r Manager) (T, error) { var zero T resolved, err := e.Resolve(ctx, r) diff --git a/coderd/runtimeconfig/config_test.go b/coderd/runtimeconfig/config_test.go index 844f817f2ddd8..dd5f36ab21bec 100644 --- a/coderd/runtimeconfig/config_test.go +++ b/coderd/runtimeconfig/config_test.go @@ -4,16 +4,14 @@ import ( "context" "testing" - "github.com/coder/serpent" + "github.com/google/uuid" "github.com/stretchr/testify/require" - "github.com/coder/coder/v2/coderd/coderdtest" + "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/codersdk" - "github.com/coder/coder/v2/enterprise/coderd/coderdenttest" - "github.com/coder/coder/v2/enterprise/coderd/license" "github.com/coder/coder/v2/testutil" ) @@ -38,12 +36,8 @@ func TestUsage(t *testing.T) { t.Run("deployment value with runtimeconfig", func(t *testing.T) { t.Parallel() - _, altOrg := setup(t) - ctx := testutil.Context(t, testutil.WaitShort) - store := dbmem.New() - resolver := runtimeconfig.NewOrgResolver(altOrg.ID, runtimeconfig.NewStoreResolver(store)) - mutator := runtimeconfig.NewOrgMutator(altOrg.ID, runtimeconfig.NewStoreMutator(store)) + mgr := runtimeconfig.NewStoreManager(dbmem.New()) // NOTE: this field is now wrapped var field runtimeconfig.Entry[*serpent.HostPort] @@ -63,7 +57,7 @@ func TestUsage(t *testing.T) { // 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, resolver) + _, 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. @@ -71,15 +65,15 @@ func TestUsage(t *testing.T) { 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, mutator, &newVal)) + require.NoError(t, field.SetRuntimeValue(ctx, mgr, &newVal)) // ...and we can retrieve the value, as well. - resolved, err := field.Resolve(ctx, resolver) + 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, mutator)) + require.NoError(t, field.UnsetRuntimeValue(ctx, mgr)) }) } @@ -87,8 +81,6 @@ func TestUsage(t *testing.T) { func TestConfig(t *testing.T) { t.Parallel() - _, altOrg := setup(t) - t.Run("new", func(t *testing.T) { t.Parallel() @@ -105,6 +97,8 @@ func TestConfig(t *testing.T) { 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] @@ -115,20 +109,18 @@ func TestConfig(t *testing.T) { require.NoError(t, field.SetStartupValue("true")) // But attempting to resolve will produce an error. - _, err := field.Resolve(context.Background(), runtimeconfig.NewNoopResolver()) + _, 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(), runtimeconfig.NewNoopMutator(), val), runtimeconfig.ErrNameNotSet) + 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) - store := dbmem.New() - resolver := runtimeconfig.NewOrgResolver(altOrg.ID, runtimeconfig.NewStoreResolver(store)) - mutator := runtimeconfig.NewOrgMutator(altOrg.ID, runtimeconfig.NewStoreMutator(store)) + mgr := runtimeconfig.NewStoreManager(dbmem.New()) var ( base = serpent.String("system@dev.coder.com") @@ -141,16 +133,16 @@ func TestConfig(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, resolver) + _, err := field.Resolve(ctx, mgr) require.ErrorIs(t, err, runtimeconfig.EntryNotFound) // Coalesce returns the deployment-wide value. - val, err := field.Coalesce(ctx, resolver) + 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, mutator, &override)) + require.NoError(t, field.SetRuntimeValue(ctx, mgr, &override)) // Coalesce now returns the org-level value. - val, err = field.Coalesce(ctx, resolver) + val, err = field.Coalesce(ctx, mgr) require.NoError(t, err) require.Equal(t, override.String(), val.String()) }) @@ -159,9 +151,7 @@ func TestConfig(t *testing.T) { t.Parallel() ctx := testutil.Context(t, testutil.WaitShort) - store := dbmem.New() - resolver := runtimeconfig.NewOrgResolver(altOrg.ID, runtimeconfig.NewStoreResolver(store)) - mutator := runtimeconfig.NewOrgMutator(altOrg.ID, runtimeconfig.NewStoreMutator(store)) + mgr := runtimeconfig.NewStoreManager(dbmem.New()) var ( base = serpent.Struct[map[string]string]{ @@ -180,34 +170,65 @@ func TestConfig(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, resolver) + _, err := field.Resolve(ctx, mgr) require.ErrorIs(t, err, runtimeconfig.EntryNotFound) // Coalesce returns the deployment-wide value. - val, err := field.Coalesce(ctx, resolver) + 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, mutator, &override)) + require.NoError(t, field.SetRuntimeValue(ctx, mgr, &override)) // Coalesce now returns the org-level value. - structVal, err := field.Resolve(ctx, resolver) + structVal, err := field.Resolve(ctx, mgr) require.NoError(t, err) require.Equal(t, override.Value, structVal.Value) }) } -// setup creates a new API, enabled notifications + multi-org experiments, and returns the API client and a new org. -func setup(t *testing.T) (*codersdk.Client, codersdk.Organization) { - t.Helper() - - vals := coderdtest.DeploymentValues(t) - vals.Experiments = []string{string(codersdk.ExperimentMultiOrganization)} - adminClient, _, _, _ := coderdenttest.NewWithAPI(t, &coderdenttest.Options{ - Options: &coderdtest.Options{DeploymentValues: vals}, - LicenseOptions: &coderdenttest.LicenseOptions{ - Features: license.Features{ - codersdk.FeatureMultipleOrganizations: 1, - }, - }, - }) - return adminClient, coderdenttest.CreateOrganization(t, adminClient, coderdenttest.CreateOrganizationOptions{}) +func TestScoped(t *testing.T) { + 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/manager.go b/coderd/runtimeconfig/manager.go new file mode 100644 index 0000000000000..4100fbf2be82c --- /dev/null +++ b/coderd/runtimeconfig/manager.go @@ -0,0 +1,80 @@ +package runtimeconfig + +import ( + "context" + "database/sql" + "errors" + "fmt" + + "golang.org/x/xerrors" + + "github.com/coder/coder/v2/coderd/database" +) + +type NoopManager struct{} + +func NewNoopManager() *NoopManager { + return &NoopManager{} +} + +func (n NoopManager) GetRuntimeSetting(context.Context, string) (string, error) { + return "", EntryNotFound +} + +func (n NoopManager) UpsertRuntimeSetting(context.Context, string, string) error { + return EntryNotFound +} + +func (n NoopManager) DeleteRuntimeSetting(context.Context, string) error { + return EntryNotFound +} + +func (n NoopManager) Scoped(string) Manager { + return n +} + +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} +} + +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) + } + + 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", 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 StoreManager) namespacedKey(k string) string { + return fmt.Sprintf("%s:%s", m.ns, k) +} diff --git a/coderd/runtimeconfig/mutator.go b/coderd/runtimeconfig/mutator.go deleted file mode 100644 index e66296fae570c..0000000000000 --- a/coderd/runtimeconfig/mutator.go +++ /dev/null @@ -1,64 +0,0 @@ -package runtimeconfig - -import ( - "context" - - "github.com/google/uuid" - "golang.org/x/xerrors" - - "github.com/coder/coder/v2/coderd/database" -) - -type StoreMutator struct { - store Store -} - -func NewStoreMutator(store Store) *StoreMutator { - if store == nil { - panic("developer error: store is nil") - } - return &StoreMutator{store} -} - -func (s StoreMutator) UpsertRuntimeSetting(ctx context.Context, key, val string) error { - err := s.store.UpsertRuntimeConfig(ctx, database.UpsertRuntimeConfigParams{Key: key, Value: val}) - if err != nil { - return xerrors.Errorf("update %q: %w", err) - } - return nil -} - -func (s StoreMutator) DeleteRuntimeSetting(ctx context.Context, key string) error { - return s.store.DeleteRuntimeConfig(ctx, key) -} - -type OrgMutator struct { - inner Mutator - orgID uuid.UUID -} - -func NewOrgMutator(orgID uuid.UUID, inner Mutator) *OrgMutator { - return &OrgMutator{inner: inner, orgID: orgID} -} - -func (m OrgMutator) UpsertRuntimeSetting(ctx context.Context, key, val string) error { - return m.inner.UpsertRuntimeSetting(ctx, orgKey(m.orgID, key), val) -} - -func (m OrgMutator) DeleteRuntimeSetting(ctx context.Context, key string) error { - return m.inner.DeleteRuntimeSetting(ctx, key) -} - -type NoopMutator struct{} - -func NewNoopMutator() *NoopMutator { - return &NoopMutator{} -} - -func (n NoopMutator) UpsertRuntimeSetting(context.Context, string, string) error { - return nil -} - -func (n NoopMutator) DeleteRuntimeSetting(context.Context, string) error { - return nil -} diff --git a/coderd/runtimeconfig/resolver.go b/coderd/runtimeconfig/resolver.go deleted file mode 100644 index a8e5d9cb9d21a..0000000000000 --- a/coderd/runtimeconfig/resolver.go +++ /dev/null @@ -1,63 +0,0 @@ -package runtimeconfig - -import ( - "context" - "database/sql" - "errors" - - "github.com/google/uuid" - "golang.org/x/xerrors" -) - -type StoreResolver struct { - store Store -} - -func NewStoreResolver(store Store) *StoreResolver { - return &StoreResolver{store} -} - -func (s StoreResolver) GetRuntimeSetting(ctx context.Context, key string) (string, error) { - if s.store == nil { - panic("developer error: store must be set") - } - - val, err := s.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) - } - - return val, nil -} - -type OrgResolver struct { - inner Resolver - orgID uuid.UUID -} - -func NewOrgResolver(orgID uuid.UUID, inner Resolver) *OrgResolver { - if inner == nil { - panic("developer error: resolver is nil") - } - - return &OrgResolver{inner: inner, orgID: orgID} -} - -func (r OrgResolver) GetRuntimeSetting(ctx context.Context, key string) (string, error) { - return r.inner.GetRuntimeSetting(ctx, orgKey(r.orgID, key)) -} - -// NoopResolver will always fail to resolve the given key. -// Useful in tests where you just want to look up the startup value of configs, and are not concerned with runtime config. -type NoopResolver struct{} - -func NewNoopResolver() *NoopResolver { - return &NoopResolver{} -} - -func (n NoopResolver) GetRuntimeSetting(context.Context, string) (string, error) { - return "", EntryNotFound -} diff --git a/coderd/runtimeconfig/spec.go b/coderd/runtimeconfig/spec.go index 2dd2ca83b79c1..80015f56a5ca1 100644 --- a/coderd/runtimeconfig/spec.go +++ b/coderd/runtimeconfig/spec.go @@ -1,44 +1,21 @@ package runtimeconfig -import "context" +import ( + "context" +) type Initializer interface { Initialize(name string) } -// Resolver is an interface for resolving runtime settings. -type Resolver interface { +type Manager interface { // GetRuntimeSetting gets a runtime setting by name. GetRuntimeSetting(ctx context.Context, name string) (string, error) -} - -// Mutator is an interface for mutating runtime settings. -type Mutator interface { // 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 -} - -type Manager interface { - Resolver - Mutator -} - -type NoopManager struct {} - -func NewNoopManager() *NoopManager { - return &NoopManager{} -} - -func (n NoopManager) GetRuntimeSetting(context.Context, string) (string, error) { - return "", EntryNotFound -} - -func (n NoopManager) UpsertRuntimeSetting(context.Context, string, string) error { - return EntryNotFound -} - -func (n NoopManager) DeleteRuntimeSetting(context.Context, string) error { - return EntryNotFound + // 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 1940ca02cab4c940046ec579022904082bf471f6 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 4 Sep 2024 09:18:54 -0500 Subject: [PATCH 11/19] linting errors --- coderd/runtimeconfig/config_test.go | 8 ++++++-- coderd/runtimeconfig/manager.go | 8 ++++---- coderd/runtimeconfig/util.go | 8 +------- 3 files changed, 11 insertions(+), 13 deletions(-) diff --git a/coderd/runtimeconfig/config_test.go b/coderd/runtimeconfig/config_test.go index dd5f36ab21bec..4f045dbdd237d 100644 --- a/coderd/runtimeconfig/config_test.go +++ b/coderd/runtimeconfig/config_test.go @@ -16,6 +16,8 @@ import ( ) func TestUsage(t *testing.T) { + t.Parallel() + t.Run("deployment value without runtimeconfig", func(t *testing.T) { t.Parallel() @@ -186,7 +188,9 @@ func TestConfig(t *testing.T) { } func TestScoped(t *testing.T) { - orgId := uuid.New() + t.Parallel() + + orgID := uuid.New() ctx := testutil.Context(t, testutil.WaitShort) @@ -209,7 +213,7 @@ func TestScoped(t *testing.T) { require.Equal(t, host, val.Host) require.Equal(t, port, val.Port) - orgMgr := mgr.Scoped(orgId.String()) + orgMgr := mgr.Scoped(orgID.String()) // Using the org scope, nothing will be returned. _, err = field.Resolve(ctx, orgMgr) require.ErrorIs(t, err, runtimeconfig.EntryNotFound) diff --git a/coderd/runtimeconfig/manager.go b/coderd/runtimeconfig/manager.go index 4100fbf2be82c..b3eaeded024ef 100644 --- a/coderd/runtimeconfig/manager.go +++ b/coderd/runtimeconfig/manager.go @@ -17,15 +17,15 @@ func NewNoopManager() *NoopManager { return &NoopManager{} } -func (n NoopManager) GetRuntimeSetting(context.Context, string) (string, error) { +func (NoopManager) GetRuntimeSetting(context.Context, string) (string, error) { return "", EntryNotFound } -func (n NoopManager) UpsertRuntimeSetting(context.Context, string, string) error { +func (NoopManager) UpsertRuntimeSetting(context.Context, string, string) error { return EntryNotFound } -func (n NoopManager) DeleteRuntimeSetting(context.Context, string) error { +func (NoopManager) DeleteRuntimeSetting(context.Context, string) error { return EntryNotFound } @@ -62,7 +62,7 @@ func (m StoreManager) GetRuntimeSetting(ctx context.Context, key string) (string 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", err) + return xerrors.Errorf("update %q: %w", key, err) } return nil } diff --git a/coderd/runtimeconfig/util.go b/coderd/runtimeconfig/util.go index 2899b2947dee7..73af53cb8aeee 100644 --- a/coderd/runtimeconfig/util.go +++ b/coderd/runtimeconfig/util.go @@ -1,17 +1,11 @@ package runtimeconfig import ( - "fmt" "reflect" - - "github.com/google/uuid" ) func create[T any]() T { var zero T + //nolint:forcetypeassert return reflect.New(reflect.TypeOf(zero).Elem()).Interface().(T) } - -func orgKey(orgID uuid.UUID, key string) string { - return fmt.Sprintf("%s:%s", orgID.String(), key) -} From d8ad4d4fc4e2ab20c9ef47219376f486b26594f4 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 4 Sep 2024 09:47:43 -0500 Subject: [PATCH 12/19] chore: add runtimeconfig manager to options for plumbing (#14562) --- cli/server.go | 9 +++++++++ coderd/coderd.go | 2 ++ coderd/coderdtest/coderdtest.go | 6 ++++++ 3 files changed, 17 insertions(+) diff --git a/cli/server.go b/cli/server.go index 94f1518fa13a1..a4fcf660225a1 100644 --- a/cli/server.go +++ b/cli/server.go @@ -56,6 +56,7 @@ import ( "cdr.dev/slog" "cdr.dev/slog/sloggers/sloghuman" "github.com/coder/coder/v2/coderd/entitlements" + "github.com/coder/coder/v2/coderd/runtimeconfig" "github.com/coder/pretty" "github.com/coder/quartz" "github.com/coder/retry" @@ -820,6 +821,14 @@ 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(options.Database) + // 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/coderd.go b/coderd/coderd.go index 20ce616eab5ba..37e39d4e03c84 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -39,6 +39,7 @@ import ( "cdr.dev/slog" "github.com/coder/coder/v2/coderd/entitlements" "github.com/coder/coder/v2/coderd/idpsync" + "github.com/coder/coder/v2/coderd/runtimeconfig" "github.com/coder/quartz" "github.com/coder/serpent" @@ -135,6 +136,7 @@ type Options struct { Logger slog.Logger Database database.Store Pubsub pubsub.Pubsub + RuntimeConfig runtimeconfig.Manager // CacheDir is used for caching files served by the API. CacheDir string diff --git a/coderd/coderdtest/coderdtest.go b/coderd/coderdtest/coderdtest.go index 57d2a876de125..0e3c049c16511 100644 --- a/coderd/coderdtest/coderdtest.go +++ b/coderd/coderdtest/coderdtest.go @@ -67,6 +67,7 @@ import ( "github.com/coder/coder/v2/coderd/notifications" "github.com/coder/coder/v2/coderd/rbac" "github.com/coder/coder/v2/coderd/rbac/policy" + "github.com/coder/coder/v2/coderd/runtimeconfig" "github.com/coder/coder/v2/coderd/schedule" "github.com/coder/coder/v2/coderd/telemetry" "github.com/coder/coder/v2/coderd/unhanger" @@ -254,6 +255,10 @@ 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) options.Database = dbauthz.New(options.Database, options.Authorizer, *options.Logger, accessControlStore) // Some routes expect a deployment ID, so just make sure one exists. @@ -482,6 +487,7 @@ func NewOptions(t testing.TB, options *Options) (func(http.Handler), context.Can AppHostnameRegex: appHostnameRegex, Logger: *options.Logger, CacheDir: t.TempDir(), + RuntimeConfig: runtimeManager, Database: options.Database, Pubsub: options.Pubsub, ExternalAuthConfigs: options.ExternalAuthConfigs, From f44f1c76ba11233296aa9aa2225b421d2a5f45a3 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 5 Sep 2024 09:40:39 -0500 Subject: [PATCH 13/19] linting, remove unused ctx --- coderd/database/dbmem/dbmem.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go index 583526af0e49a..98eb35b4240ad 100644 --- a/coderd/database/dbmem/dbmem.go +++ b/coderd/database/dbmem/dbmem.go @@ -9209,7 +9209,7 @@ func (q *FakeQuerier) UpsertProvisionerDaemon(_ context.Context, arg database.Up return d, nil } -func (q *FakeQuerier) UpsertRuntimeConfig(ctx context.Context, arg database.UpsertRuntimeConfigParams) error { +func (q *FakeQuerier) UpsertRuntimeConfig(_ context.Context, arg database.UpsertRuntimeConfigParams) error { err := validateDatabaseType(arg) if err != nil { return err From 7452a23d82d73fbd4aed6253d6cb669990240312 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 5 Sep 2024 16:12:08 -0500 Subject: [PATCH 14/19] chore: refactor entry into deployment and runtime (#14575) * chore: refactor entry into deployment and runtime RuntimeEntry has no startup value, and omits functions required to be serpent compatible. Co-authored-by: Danny Kopping --------- Co-authored-by: Danny Kopping --- cli/server.go | 8 +- coderd/coderdtest/coderdtest.go | 5 +- coderd/database/dbauthz/dbauthz.go | 12 +- coderd/database/dbauthz/dbauthz_test.go | 16 ++ coderd/runtimeconfig/config.go | 161 ---------------- coderd/runtimeconfig/config_test.go | 238 ------------------------ coderd/runtimeconfig/deploymententry.go | 88 +++++++++ coderd/runtimeconfig/entry.go | 94 ++++++++++ coderd/runtimeconfig/entry_test.go | 135 ++++++++++++++ coderd/runtimeconfig/manager.go | 83 +++------ coderd/runtimeconfig/resolver.go | 139 ++++++++++++++ coderd/runtimeconfig/spec.go | 12 +- 12 files changed, 519 insertions(+), 472 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 create mode 100644 coderd/runtimeconfig/resolver.go diff --git a/cli/server.go b/cli/server.go index a4fcf660225a1..0846f65624b0e 100644 --- a/cli/server.go +++ b/cli/server.go @@ -821,13 +821,7 @@ 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(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/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..d23bb48184b61 100644 --- a/coderd/database/dbauthz/dbauthz_test.go +++ b/coderd/database/dbauthz/dbauthz_test.go @@ -2696,6 +2696,22 @@ 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) { + _ = 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(database.UpsertRuntimeConfigParams{ + Key: "test", + Value: "value", + }).Asserts(rbac.ResourceSystem, policy.ActionCreate) + })) } func (s *MethodTestSuite) TestNotifications() { 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..0ae23de970fc4 --- /dev/null +++ b/coderd/runtimeconfig/deploymententry.go @@ -0,0 +1,88 @@ +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) + _ 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 Resolver) (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..3204936a76384 --- /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 Resolver, 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 Resolver) 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 Resolver) (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..064f4d2800596 --- /dev/null +++ b/coderd/runtimeconfig/entry_test.go @@ -0,0 +1,135 @@ +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() + + 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". + 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(), 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(), rlv, val), runtimeconfig.ErrNameNotSet) + }) + + t.Run("simple", func(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitShort) + mgr := runtimeconfig.NewStoreManager() + db := 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.DeploymentResolver(db)) + require.ErrorIs(t, err, runtimeconfig.EntryNotFound) + // Coalesce returns the deployment-wide value. + 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.DeploymentResolver(db), &override)) + // Coalesce now returns the org-level value. + val, err = field.Coalesce(ctx, mgr.DeploymentResolver(db)) + 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() + db := 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.DeploymentResolver(db)) + require.ErrorIs(t, err, runtimeconfig.EntryNotFound) + // Coalesce returns the deployment-wide value. + 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.DeploymentResolver(db), &override)) + // Coalesce now returns the org-level value. + 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 b3eaeded024ef..293b04cbd108f 100644 --- a/coderd/runtimeconfig/manager.go +++ b/coderd/runtimeconfig/manager.go @@ -1,80 +1,51 @@ 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{} +// StoreManager is the shared singleton that produces resolvers for runtime configuration. +type StoreManager struct{} -func NewNoopManager() *NoopManager { - return &NoopManager{} +func NewStoreManager() Manager { + return &StoreManager{} } -func (NoopManager) GetRuntimeSetting(context.Context, string) (string, error) { - return "", EntryNotFound +func (*StoreManager) DeploymentResolver(db Store) Resolver { + return NewStoreResolver(db) } -func (NoopManager) UpsertRuntimeSetting(context.Context, string, string) error { - return EntryNotFound +func (*StoreManager) OrganizationResolver(db Store, orgID uuid.UUID) Resolver { + return OrganizationResolver(orgID, NewStoreResolver(db)) } -func (NoopManager) DeleteRuntimeSetting(context.Context, string) error { - return EntryNotFound +type cacheEntry struct { + value string + lastUpdated time.Time } -func (n NoopManager) Scoped(string) Manager { - return n +// 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] } -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} -} - -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() *MemoryCacheManager { + return &MemoryCacheManager{ + cache: syncmap.New[string, cacheEntry](), } - - 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, NewStoreResolver(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, NewStoreResolver(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..ad78455dc8964 100644 --- a/coderd/runtimeconfig/spec.go +++ b/coderd/runtimeconfig/spec.go @@ -2,20 +2,26 @@ package runtimeconfig import ( "context" + + "github.com/google/uuid" ) type Initializer interface { Initialize(name string) } +// TODO: We should probably remove the manager interface and only support +// 1 implementation. type Manager interface { + DeploymentResolver(db Store) Resolver + OrganizationResolver(db Store, orgID uuid.UUID) 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 13ea99a4b46238fd73dad53a501d02c365881904 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 6 Sep 2024 08:53:00 -0500 Subject: [PATCH 15/19] chore: comments, cleanup, error fixes --- coderd/database/dbmem/dbmem.go | 3 +- coderd/runtimeconfig/deploymententry.go | 3 +- coderd/runtimeconfig/deploymententry_test.go | 132 +++++++++++++++++++ coderd/runtimeconfig/doc.go | 10 ++ coderd/runtimeconfig/entry.go | 5 +- coderd/runtimeconfig/entry_test.go | 20 +-- coderd/runtimeconfig/manager.go | 43 ++---- coderd/runtimeconfig/resolver.go | 61 +-------- coderd/runtimeconfig/spec.go | 22 +++- coderd/runtimeconfig/store.go | 17 --- 10 files changed, 196 insertions(+), 120 deletions(-) create mode 100644 coderd/runtimeconfig/deploymententry_test.go create mode 100644 coderd/runtimeconfig/doc.go delete mode 100644 coderd/runtimeconfig/store.go diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go index 98eb35b4240ad..445c0c9b4e58d 100644 --- a/coderd/database/dbmem/dbmem.go +++ b/coderd/database/dbmem/dbmem.go @@ -22,7 +22,6 @@ import ( "golang.org/x/xerrors" "github.com/coder/coder/v2/coderd/notifications/types" - "github.com/coder/coder/v2/coderd/runtimeconfig" "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/dbtime" @@ -3522,7 +3521,7 @@ func (q *FakeQuerier) GetRuntimeConfig(_ context.Context, key string) (string, e val, ok := q.runtimeConfig[key] if !ok { - return "", runtimeconfig.EntryNotFound + return "", sql.ErrNoRows } return val, nil diff --git a/coderd/runtimeconfig/deploymententry.go b/coderd/runtimeconfig/deploymententry.go index 0ae23de970fc4..2b5d835f2624e 100644 --- a/coderd/runtimeconfig/deploymententry.go +++ b/coderd/runtimeconfig/deploymententry.go @@ -21,6 +21,7 @@ type SerpentEntry interface { // DeploymentEntry extends a runtime entry with a startup value. // This allows for a single entry to source its value from startup or runtime. +// DeploymentEntry will never return ErrEntryNotFound, as it will always return a value. type DeploymentEntry[T SerpentEntry] struct { RuntimeEntry[T] startupValue T @@ -52,7 +53,7 @@ func (e *DeploymentEntry[T]) Coalesce(ctx context.Context, r Resolver) (T, error resolved, err := e.Resolve(ctx, r) if err != nil { - if errors.Is(err, EntryNotFound) { + if errors.Is(err, ErrEntryNotFound) { return e.StartupValue(), nil } return zero, err diff --git a/coderd/runtimeconfig/deploymententry_test.go b/coderd/runtimeconfig/deploymententry_test.go new file mode 100644 index 0000000000000..6994e0bde4771 --- /dev/null +++ b/coderd/runtimeconfig/deploymententry_test.go @@ -0,0 +1,132 @@ +package runtimeconfig_test + +import ( + "context" + "fmt" + "testing" + + "github.com/google/uuid" + "github.com/stretchr/testify/require" + + "github.com/coder/coder/v2/coderd/database/dbmem" + "github.com/coder/coder/v2/coderd/database/dbtestutil" + "github.com/coder/coder/v2/coderd/runtimeconfig" + "github.com/coder/coder/v2/coderd/util/ptr" + "github.com/coder/coder/v2/codersdk" + "github.com/coder/coder/v2/testutil" + "github.com/coder/serpent" +) + +func ExampleDeploymentValues() { + ctx := context.Background() + db := dbmem.New() + st := runtimeconfig.NewStoreManager() + + // Define the field, this will usually live on Deployment Values. + var stringField runtimeconfig.DeploymentEntry[*serpent.String] + // All fields need to be initialized with their "key". This will be used + // to uniquely identify the field in the store. + stringField.Initialize("string-field") + + // The startup value configured by the deployment env vars + // This acts as a default value if no runtime value is set. + // Can be used to support migrating a value from startup to runtime. + _ = stringField.SetStartupValue("default") + + // Runtime values take priority over startup values. + _ = stringField.SetRuntimeValue(ctx, st.Resolver(db), serpent.StringOf(ptr.Ref("hello world"))) + + // Resolve the value of the field. + val, err := stringField.Resolve(ctx, st.Resolver(db)) + if err != nil { + panic(err) + } + fmt.Println(val) + // Output: hello world +} + +// TestSerpentDeploymentEntry uses the package as the serpent options will use it. +// Some of the usage might feel awkward, since the serpent package values come from +// the serpent parsing (strings), not manual assignment. +func TestSerpentDeploymentEntry(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitMedium) + db, _ := dbtestutil.NewDB(t) + st := runtimeconfig.NewStoreManager() + + // TestEntries is how entries are defined in deployment values. + type TestEntries struct { + String runtimeconfig.DeploymentEntry[*serpent.String] + Bool runtimeconfig.DeploymentEntry[*serpent.Bool] + // codersdk.Feature is arbitrary, just using an actual struct to test. + Struct runtimeconfig.DeploymentEntry[*serpent.Struct[codersdk.Feature]] + } + + var entries TestEntries + // Init fields + entries.String.Initialize("string-field") + entries.Bool.Initialize("bool-field") + entries.Struct.Initialize("struct-field") + + // When using Coalesce, the default value is the empty value + stringVal, err := entries.String.Coalesce(ctx, st.Resolver(db)) + require.NoError(t, err) + require.Equal(t, "", stringVal.String()) + + // Set some defaults for some + _ = entries.String.SetStartupValue("default") + _ = entries.Struct.SetStartupValue((&serpent.Struct[codersdk.Feature]{ + Value: codersdk.Feature{ + Entitlement: codersdk.EntitlementEntitled, + Enabled: false, + Limit: ptr.Ref(int64(100)), + Actual: nil, + }, + }).String()) + + // Retrieve startup values + stringVal, err = entries.String.Coalesce(ctx, st.Resolver(db)) + require.NoError(t, err) + require.Equal(t, "default", stringVal.String()) + + structVal, err := entries.Struct.Coalesce(ctx, st.Resolver(db)) + require.NoError(t, err) + require.Equal(t, structVal.Value.Entitlement, codersdk.EntitlementEntitled) + require.Equal(t, structVal.Value.Limit, ptr.Ref(int64(100))) + + // Override some defaults + err = entries.String.SetRuntimeValue(ctx, st.Resolver(db), serpent.StringOf(ptr.Ref("hello world"))) + require.NoError(t, err) + + err = entries.Struct.SetRuntimeValue(ctx, st.Resolver(db), &serpent.Struct[codersdk.Feature]{ + Value: codersdk.Feature{ + Entitlement: codersdk.EntitlementGracePeriod, + }, + }) + require.NoError(t, err) + + // Retrieve runtime values + stringVal, err = entries.String.Coalesce(ctx, st.Resolver(db)) + require.NoError(t, err) + require.Equal(t, "hello world", stringVal.String()) + + structVal, err = entries.Struct.Coalesce(ctx, st.Resolver(db)) + require.NoError(t, err) + require.Equal(t, structVal.Value.Entitlement, codersdk.EntitlementGracePeriod) + + // Test using org scoped resolver + orgID := uuid.New() + orgResolver := st.OrganizationResolver(db, orgID) + // No org runtime set + stringVal, err = entries.String.Coalesce(ctx, orgResolver) + require.NoError(t, err) + require.Equal(t, "default", stringVal.String()) + // Update org runtime + err = entries.String.SetRuntimeValue(ctx, orgResolver, serpent.StringOf(ptr.Ref("hello organizations"))) + require.NoError(t, err) + // Verify org runtime + stringVal, err = entries.String.Coalesce(ctx, orgResolver) + require.NoError(t, err) + require.Equal(t, "hello organizations", stringVal.String()) +} diff --git a/coderd/runtimeconfig/doc.go b/coderd/runtimeconfig/doc.go new file mode 100644 index 0000000000000..a0e42b1390ddf --- /dev/null +++ b/coderd/runtimeconfig/doc.go @@ -0,0 +1,10 @@ +// Package runtimeconfig contains logic for managing runtime configuration values +// stored in the database. Each coderd should have a Manager singleton instance +// that can create a Resolver for runtime configuration CRUD. +// +// TODO: Implement a caching layer for the Resolver so that we don't hit the +// database on every request. Configuration values are not expected to change +// frequently, so we should use pubsub to notify for updates. +// When implemented, the runtimeconfig will essentially be an in memory lookup +// with a database for persistence. +package runtimeconfig diff --git a/coderd/runtimeconfig/entry.go b/coderd/runtimeconfig/entry.go index 3204936a76384..e9ae3b3a746f5 100644 --- a/coderd/runtimeconfig/entry.go +++ b/coderd/runtimeconfig/entry.go @@ -7,8 +7,9 @@ import ( "golang.org/x/xerrors" ) -var ErrNameNotSet = xerrors.New("name is not set") - +// EntryMarshaller requires all entries to marshal to and from a string. +// The final store value is a database `text` column. +// This also is compatible with serpent values. type EntryMarshaller interface { fmt.Stringer } diff --git a/coderd/runtimeconfig/entry_test.go b/coderd/runtimeconfig/entry_test.go index 064f4d2800596..6d95037822c11 100644 --- a/coderd/runtimeconfig/entry_test.go +++ b/coderd/runtimeconfig/entry_test.go @@ -79,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.DeploymentResolver(db)) - require.ErrorIs(t, err, runtimeconfig.EntryNotFound) + _, err := field.Resolve(ctx, mgr.Resolver(db)) + require.ErrorIs(t, err, runtimeconfig.ErrEntryNotFound) // Coalesce returns the deployment-wide value. - val, err := field.Coalesce(ctx, mgr.DeploymentResolver(db)) + val, err := field.Coalesce(ctx, mgr.Resolver(db)) require.NoError(t, err) require.Equal(t, base.String(), val.String()) // Set an org-level override. - require.NoError(t, field.SetRuntimeValue(ctx, mgr.DeploymentResolver(db), &override)) + require.NoError(t, field.SetRuntimeValue(ctx, mgr.Resolver(db), &override)) // Coalesce now returns the org-level value. - val, err = field.Coalesce(ctx, mgr.DeploymentResolver(db)) + val, err = field.Coalesce(ctx, mgr.Resolver(db)) require.NoError(t, err) require.Equal(t, override.String(), val.String()) }) @@ -119,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.DeploymentResolver(db)) - require.ErrorIs(t, err, runtimeconfig.EntryNotFound) + _, err := field.Resolve(ctx, mgr.Resolver(db)) + require.ErrorIs(t, err, runtimeconfig.ErrEntryNotFound) // Coalesce returns the deployment-wide value. - val, err := field.Coalesce(ctx, mgr.DeploymentResolver(db)) + val, err := field.Coalesce(ctx, mgr.Resolver(db)) require.NoError(t, err) require.Equal(t, base.Value, val.Value) // Set an org-level override. - require.NoError(t, field.SetRuntimeValue(ctx, mgr.DeploymentResolver(db), &override)) + require.NoError(t, field.SetRuntimeValue(ctx, mgr.Resolver(db), &override)) // Coalesce now returns the org-level value. - structVal, err := field.Resolve(ctx, mgr.DeploymentResolver(db)) + structVal, err := field.Resolve(ctx, mgr.Resolver(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 293b04cbd108f..b974f73b569e6 100644 --- a/coderd/runtimeconfig/manager.go +++ b/coderd/runtimeconfig/manager.go @@ -1,51 +1,28 @@ package runtimeconfig import ( - "time" - "github.com/google/uuid" - - "github.com/coder/coder/v2/coderd/util/syncmap" ) -// StoreManager is the shared singleton that produces resolvers for runtime configuration. +// StoreManager is the singleton that produces resolvers for runtime configuration. +// TODO: Implement caching layer. type StoreManager struct{} func NewStoreManager() Manager { return &StoreManager{} } -func (*StoreManager) DeploymentResolver(db Store) Resolver { +// Resolver is the deployment wide namespace for runtime configuration. +// If you are trying to namespace a configuration, orgs for example, use +// OrganizationResolver. +func (*StoreManager) Resolver(db Store) Resolver { return NewStoreResolver(db) } +// OrganizationResolver will namespace all runtime configuration to the provided +// organization ID. Configuration values stored with a given organization ID require +// that the organization ID be provided to retrieve the value. +// No values set here will ever be returned by the call to 'Resolver()'. func (*StoreManager) OrganizationResolver(db Store, orgID uuid.UUID) Resolver { return OrganizationResolver(orgID, NewStoreResolver(db)) } - -type cacheEntry struct { - value string - 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'. -// TODO: Handle pubsub-based cache invalidation. -type MemoryCacheManager struct { - cache *syncmap.Map[string, cacheEntry] -} - -func NewMemoryCacheManager() *MemoryCacheManager { - return &MemoryCacheManager{ - cache: syncmap.New[string, cacheEntry](), - } -} - -func (m *MemoryCacheManager) DeploymentResolver(db Store) Resolver { - return NewMemoryCachedResolver(m.cache, NewStoreResolver(db)) -} - -func (m *MemoryCacheManager) OrganizationResolver(db Store, orgID uuid.UUID) Resolver { - return OrganizationResolver(orgID, NewMemoryCachedResolver(m.cache, NewStoreResolver(db))) -} diff --git a/coderd/runtimeconfig/resolver.go b/coderd/runtimeconfig/resolver.go index 222641eda3ba9..1832aca937187 100644 --- a/coderd/runtimeconfig/resolver.go +++ b/coderd/runtimeconfig/resolver.go @@ -5,15 +5,14 @@ import ( "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" ) +// NoopResolver is a useful test device. type NoopResolver struct{} func NewNoopResolver() *NoopResolver { @@ -21,15 +20,15 @@ func NewNoopResolver() *NoopResolver { } func (NoopResolver) GetRuntimeSetting(context.Context, string) (string, error) { - return "", EntryNotFound + return "", ErrEntryNotFound } func (NoopResolver) UpsertRuntimeSetting(context.Context, string, string) error { - return EntryNotFound + return ErrEntryNotFound } func (NoopResolver) DeleteRuntimeSetting(context.Context, string) error { - return EntryNotFound + return ErrEntryNotFound } // StoreResolver uses the database as the underlying store for runtime settings. @@ -45,7 +44,7 @@ func (m StoreResolver) GetRuntimeSetting(ctx context.Context, key string) (strin 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("%q: %w", key, ErrEntryNotFound) } return "", xerrors.Errorf("fetch %q: %w", key, err) } @@ -65,6 +64,8 @@ func (m StoreResolver) DeleteRuntimeSetting(ctx context.Context, key string) err return m.db.DeleteRuntimeConfig(ctx, key) } +// NamespacedResolver prefixes all keys with a namespace. +// Then defers to the underlying resolver for the actual operations. type NamespacedResolver struct { ns string wrapped Resolver @@ -89,51 +90,3 @@ func (m NamespacedResolver) DeleteRuntimeSetting(ctx context.Context, key string 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 ad78455dc8964..de8e0984e0839 100644 --- a/coderd/runtimeconfig/spec.go +++ b/coderd/runtimeconfig/spec.go @@ -4,6 +4,19 @@ import ( "context" "github.com/google/uuid" + "golang.org/x/xerrors" + + "github.com/coder/coder/v2/coderd/database" +) + +var ( + // ErrEntryNotFound is returned when a runtime entry is not saved in the + // store. It is essentially a 'sql.ErrNoRows'. + ErrEntryNotFound = xerrors.New("entry not found") + // ErrNameNotSet is returned when a runtime entry is created without a name. + // This is more likely to happen on DeploymentEntry that has not called + // Initialize(). + ErrNameNotSet = xerrors.New("name is not set") ) type Initializer interface { @@ -13,7 +26,7 @@ type Initializer interface { // TODO: We should probably remove the manager interface and only support // 1 implementation. type Manager interface { - DeploymentResolver(db Store) Resolver + Resolver(db Store) Resolver OrganizationResolver(db Store, orgID uuid.UUID) Resolver } @@ -25,3 +38,10 @@ type Resolver interface { // DeleteRuntimeSetting deletes a runtime setting by name. DeleteRuntimeSetting(ctx context.Context, name string) error } + +// Store is a subset of database.Store +type Store interface { + GetRuntimeConfig(ctx context.Context, key string) (string, error) + UpsertRuntimeConfig(ctx context.Context, arg database.UpsertRuntimeConfigParams) error + DeleteRuntimeConfig(ctx context.Context, key string) error +} diff --git a/coderd/runtimeconfig/store.go b/coderd/runtimeconfig/store.go deleted file mode 100644 index 16f12340bbf68..0000000000000 --- a/coderd/runtimeconfig/store.go +++ /dev/null @@ -1,17 +0,0 @@ -package runtimeconfig - -import ( - "context" - - "golang.org/x/xerrors" - - "github.com/coder/coder/v2/coderd/database" -) - -var EntryNotFound = xerrors.New("entry not found") - -type Store interface { - GetRuntimeConfig(ctx context.Context, key string) (string, error) - UpsertRuntimeConfig(ctx context.Context, arg database.UpsertRuntimeConfigParams) error - DeleteRuntimeConfig(ctx context.Context, key string) error -} From 7d082553724c218b7ef3bb02b83684d2ede0ceea Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 6 Sep 2024 09:19:26 -0500 Subject: [PATCH 16/19] increase test coverage: --- coderd/runtimeconfig/deploymententry_test.go | 66 ++++++++++++++++++++ coderd/runtimeconfig/entry_test.go | 9 ++- 2 files changed, 73 insertions(+), 2 deletions(-) diff --git a/coderd/runtimeconfig/deploymententry_test.go b/coderd/runtimeconfig/deploymententry_test.go index 6994e0bde4771..8fd1b1e323a52 100644 --- a/coderd/runtimeconfig/deploymententry_test.go +++ b/coderd/runtimeconfig/deploymententry_test.go @@ -7,8 +7,11 @@ import ( "github.com/google/uuid" "github.com/stretchr/testify/require" + "go.uber.org/mock/gomock" + "golang.org/x/xerrors" "github.com/coder/coder/v2/coderd/database/dbmem" + "github.com/coder/coder/v2/coderd/database/dbmock" "github.com/coder/coder/v2/coderd/database/dbtestutil" "github.com/coder/coder/v2/coderd/runtimeconfig" "github.com/coder/coder/v2/coderd/util/ptr" @@ -45,6 +48,50 @@ func ExampleDeploymentValues() { // Output: hello world } +// TestResolveDBError ensures a db error that is not a sql.ErrNoRows +// will bubble up using Coalesce. The error should not be ignored and replaced +// with the startup value. +func TestResolveDBError(t *testing.T) { + t.Parallel() + + dbErr := xerrors.Errorf("some db error") + ctrl := gomock.NewController(t) + mDB := dbmock.NewMockStore(ctrl) + // Error on fetch + mDB.EXPECT(). + GetRuntimeConfig(gomock.Any(), gomock.Any()). + Times(1). + Return("", dbErr) + + // Error on upsert + mDB.EXPECT(). + UpsertRuntimeConfig(gomock.Any(), gomock.Any()). + Times(1). + Return(dbErr) + + // Error on delete + mDB.EXPECT(). + DeleteRuntimeConfig(gomock.Any(), gomock.Any()). + Times(1). + Return(dbErr) + + st := runtimeconfig.NewStoreManager() + var stringField runtimeconfig.DeploymentEntry[*serpent.String] + stringField.Initialize("string-field") + stringField.SetStartupValue("default") + + ctx := testutil.Context(t, testutil.WaitMedium) + // Resolve + _, err := stringField.Coalesce(ctx, st.Resolver(mDB)) + require.ErrorIs(t, err, dbErr) + // Set + err = stringField.SetRuntimeValue(ctx, st.Resolver(mDB), serpent.StringOf(ptr.Ref("hello world"))) + require.ErrorIs(t, err, dbErr) + // Unset + err = stringField.UnsetRuntimeValue(ctx, st.Resolver(mDB)) + require.ErrorIs(t, err, dbErr) +} + // TestSerpentDeploymentEntry uses the package as the serpent options will use it. // Some of the usage might feel awkward, since the serpent package values come from // the serpent parsing (strings), not manual assignment. @@ -69,6 +116,11 @@ func TestSerpentDeploymentEntry(t *testing.T) { entries.Bool.Initialize("bool-field") entries.Struct.Initialize("struct-field") + // Check the Type() methods are unchanged + require.Equal(t, entries.String.Type(), (serpent.String("")).Type()) + require.Equal(t, entries.Bool.Type(), (serpent.Bool(false)).Type()) + require.Equal(t, entries.Struct.Type(), (&serpent.Struct[codersdk.Feature]{}).Type()) + // When using Coalesce, the default value is the empty value stringVal, err := entries.String.Coalesce(ctx, st.Resolver(db)) require.NoError(t, err) @@ -115,6 +167,13 @@ func TestSerpentDeploymentEntry(t *testing.T) { require.NoError(t, err) require.Equal(t, structVal.Value.Entitlement, codersdk.EntitlementGracePeriod) + // Test unset + err = entries.String.UnsetRuntimeValue(ctx, st.Resolver(db)) + require.NoError(t, err) + stringVal, err = entries.String.Coalesce(ctx, st.Resolver(db)) + require.NoError(t, err) + require.Equal(t, "default", stringVal.String()) + // Test using org scoped resolver orgID := uuid.New() orgResolver := st.OrganizationResolver(db, orgID) @@ -129,4 +188,11 @@ func TestSerpentDeploymentEntry(t *testing.T) { stringVal, err = entries.String.Coalesce(ctx, orgResolver) require.NoError(t, err) require.Equal(t, "hello organizations", stringVal.String()) + // Unset org runtime + err = entries.String.UnsetRuntimeValue(ctx, orgResolver) + require.NoError(t, err) + // Verify org runtime is back to default + stringVal, err = entries.String.Coalesce(ctx, orgResolver) + require.NoError(t, err) + require.Equal(t, "default", stringVal.String()) } diff --git a/coderd/runtimeconfig/entry_test.go b/coderd/runtimeconfig/entry_test.go index 6d95037822c11..3edf884399445 100644 --- a/coderd/runtimeconfig/entry_test.go +++ b/coderd/runtimeconfig/entry_test.go @@ -51,10 +51,15 @@ func TestEntry(t *testing.T) { // Setting a value will not produce an error. require.NoError(t, field.SetStartupValue("true")) - // But attempting to resolve will produce an error. + // Attempting to resolve will produce an error. _, err := field.Resolve(context.Background(), rlv) require.ErrorIs(t, err, runtimeconfig.ErrNameNotSet) - // But attempting to set the runtime value will produce an error. + + // Attempting to unset + err = field.UnsetRuntimeValue(context.Background(), rlv) + require.ErrorIs(t, err, runtimeconfig.ErrNameNotSet) + + // Attempting to set val := serpent.BoolOf(ptr.Ref(true)) require.ErrorIs(t, field.SetRuntimeValue(context.Background(), rlv, val), runtimeconfig.ErrNameNotSet) }) From 4ec16cd71f645148b29174a3bfbdc7628df26841 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 6 Sep 2024 09:26:24 -0500 Subject: [PATCH 17/19] linting complained about an unhandled Println error --- coderd/runtimeconfig/deploymententry_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coderd/runtimeconfig/deploymententry_test.go b/coderd/runtimeconfig/deploymententry_test.go index 8fd1b1e323a52..62506376d906b 100644 --- a/coderd/runtimeconfig/deploymententry_test.go +++ b/coderd/runtimeconfig/deploymententry_test.go @@ -44,7 +44,7 @@ func ExampleDeploymentValues() { if err != nil { panic(err) } - fmt.Println(val) + _, _ = fmt.Println(val) // Output: hello world } From 9f546d0e1a61ea51458781d72e9338627e4a81b3 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 6 Sep 2024 10:19:13 -0500 Subject: [PATCH 18/19] drop manager interface, wrap errors --- coderd/coderd.go | 2 +- coderd/runtimeconfig/entry.go | 14 +++++++------- coderd/runtimeconfig/manager.go | 12 ++++++------ coderd/runtimeconfig/resolver.go | 24 ++++++++++++------------ coderd/runtimeconfig/spec.go | 20 ++++++-------------- 5 files changed, 32 insertions(+), 40 deletions(-) diff --git a/coderd/coderd.go b/coderd/coderd.go index 37e39d4e03c84..51b6780e4dc47 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -136,7 +136,7 @@ type Options struct { Logger slog.Logger Database database.Store Pubsub pubsub.Pubsub - RuntimeConfig runtimeconfig.Manager + RuntimeConfig *runtimeconfig.Manager // CacheDir is used for caching files served by the API. CacheDir string diff --git a/coderd/runtimeconfig/entry.go b/coderd/runtimeconfig/entry.go index e9ae3b3a746f5..780138a89d03b 100644 --- a/coderd/runtimeconfig/entry.go +++ b/coderd/runtimeconfig/entry.go @@ -48,20 +48,20 @@ func MustNew[T EntryValue](name string) RuntimeEntry[T] { func (e *RuntimeEntry[T]) SetRuntimeValue(ctx context.Context, m Resolver, val T) error { name, err := e.name() if err != nil { - return err + return xerrors.Errorf("set runtime: %w", err) } - return m.UpsertRuntimeSetting(ctx, name, val.String()) + return m.UpsertRuntimeConfig(ctx, name, val.String()) } // UnsetRuntimeValue removes the runtime value from the store. func (e *RuntimeEntry[T]) UnsetRuntimeValue(ctx context.Context, m Resolver) error { name, err := e.name() if err != nil { - return err + return xerrors.Errorf("unset runtime: %w", err) } - return m.DeleteRuntimeSetting(ctx, name) + return m.DeleteRuntimeConfig(ctx, name) } // Resolve attempts to resolve the runtime value of this field from the store via the given Resolver. @@ -70,12 +70,12 @@ func (e *RuntimeEntry[T]) Resolve(ctx context.Context, r Resolver) (T, error) { name, err := e.name() if err != nil { - return zero, err + return zero, xerrors.Errorf("resolve, name issue: %w", err) } - val, err := r.GetRuntimeSetting(ctx, name) + val, err := r.GetRuntimeConfig(ctx, name) if err != nil { - return zero, err + return zero, xerrors.Errorf("resolve runtime: %w", err) } inst := create[T]() diff --git a/coderd/runtimeconfig/manager.go b/coderd/runtimeconfig/manager.go index b974f73b569e6..232f30e3e1f5d 100644 --- a/coderd/runtimeconfig/manager.go +++ b/coderd/runtimeconfig/manager.go @@ -4,18 +4,18 @@ import ( "github.com/google/uuid" ) -// StoreManager is the singleton that produces resolvers for runtime configuration. +// Manager is the singleton that produces resolvers for runtime configuration. // TODO: Implement caching layer. -type StoreManager struct{} +type Manager struct{} -func NewStoreManager() Manager { - return &StoreManager{} +func NewStoreManager() *Manager { + return &Manager{} } // Resolver is the deployment wide namespace for runtime configuration. // If you are trying to namespace a configuration, orgs for example, use // OrganizationResolver. -func (*StoreManager) Resolver(db Store) Resolver { +func (*Manager) Resolver(db Store) Resolver { return NewStoreResolver(db) } @@ -23,6 +23,6 @@ func (*StoreManager) Resolver(db Store) Resolver { // organization ID. Configuration values stored with a given organization ID require // that the organization ID be provided to retrieve the value. // No values set here will ever be returned by the call to 'Resolver()'. -func (*StoreManager) OrganizationResolver(db Store, orgID uuid.UUID) Resolver { +func (*Manager) OrganizationResolver(db Store, orgID uuid.UUID) Resolver { return OrganizationResolver(orgID, NewStoreResolver(db)) } diff --git a/coderd/runtimeconfig/resolver.go b/coderd/runtimeconfig/resolver.go index 1832aca937187..d899680f034a4 100644 --- a/coderd/runtimeconfig/resolver.go +++ b/coderd/runtimeconfig/resolver.go @@ -19,15 +19,15 @@ func NewNoopResolver() *NoopResolver { return &NoopResolver{} } -func (NoopResolver) GetRuntimeSetting(context.Context, string) (string, error) { +func (NoopResolver) GetRuntimeConfig(context.Context, string) (string, error) { return "", ErrEntryNotFound } -func (NoopResolver) UpsertRuntimeSetting(context.Context, string, string) error { +func (NoopResolver) UpsertRuntimeConfig(context.Context, string, string) error { return ErrEntryNotFound } -func (NoopResolver) DeleteRuntimeSetting(context.Context, string) error { +func (NoopResolver) DeleteRuntimeConfig(context.Context, string) error { return ErrEntryNotFound } @@ -40,7 +40,7 @@ func NewStoreResolver(db Store) *StoreResolver { return &StoreResolver{db: db} } -func (m StoreResolver) GetRuntimeSetting(ctx context.Context, key string) (string, error) { +func (m StoreResolver) GetRuntimeConfig(ctx context.Context, key string) (string, error) { val, err := m.db.GetRuntimeConfig(ctx, key) if err != nil { if errors.Is(err, sql.ErrNoRows) { @@ -52,7 +52,7 @@ func (m StoreResolver) GetRuntimeSetting(ctx context.Context, key string) (strin return val, nil } -func (m StoreResolver) UpsertRuntimeSetting(ctx context.Context, key, val string) error { +func (m StoreResolver) UpsertRuntimeConfig(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) @@ -60,7 +60,7 @@ func (m StoreResolver) UpsertRuntimeSetting(ctx context.Context, key, val string return nil } -func (m StoreResolver) DeleteRuntimeSetting(ctx context.Context, key string) error { +func (m StoreResolver) DeleteRuntimeConfig(ctx context.Context, key string) error { return m.db.DeleteRuntimeConfig(ctx, key) } @@ -75,16 +75,16 @@ 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) GetRuntimeConfig(ctx context.Context, key string) (string, error) { + return m.wrapped.GetRuntimeConfig(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) UpsertRuntimeConfig(ctx context.Context, key, val string) error { + return m.wrapped.UpsertRuntimeConfig(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) DeleteRuntimeConfig(ctx context.Context, key string) error { + return m.wrapped.DeleteRuntimeConfig(ctx, m.namespacedKey(key)) } func (m NamespacedResolver) namespacedKey(k string) string { diff --git a/coderd/runtimeconfig/spec.go b/coderd/runtimeconfig/spec.go index de8e0984e0839..04451131c252a 100644 --- a/coderd/runtimeconfig/spec.go +++ b/coderd/runtimeconfig/spec.go @@ -3,7 +3,6 @@ package runtimeconfig import ( "context" - "github.com/google/uuid" "golang.org/x/xerrors" "github.com/coder/coder/v2/coderd/database" @@ -23,20 +22,13 @@ type Initializer interface { Initialize(name string) } -// TODO: We should probably remove the manager interface and only support -// 1 implementation. -type Manager interface { - Resolver(db Store) Resolver - OrganizationResolver(db Store, orgID uuid.UUID) 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 + // GetRuntimeConfig gets a runtime setting by name. + GetRuntimeConfig(ctx context.Context, name string) (string, error) + // UpsertRuntimeConfig upserts a runtime setting by name. + UpsertRuntimeConfig(ctx context.Context, name, val string) error + // DeleteRuntimeConfig deletes a runtime setting by name. + DeleteRuntimeConfig(ctx context.Context, name string) error } // Store is a subset of database.Store From 56f4df29478eaa40c2d99cc783f38125cefdf2e4 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 6 Sep 2024 10:40:20 -0500 Subject: [PATCH 19/19] rename NewStoreManager -> NewManager --- cli/server.go | 2 +- coderd/coderdtest/coderdtest.go | 2 +- coderd/runtimeconfig/deploymententry_test.go | 6 +++--- coderd/runtimeconfig/entry_test.go | 4 ++-- coderd/runtimeconfig/manager.go | 2 +- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/cli/server.go b/cli/server.go index 0846f65624b0e..4e3b1e16a1482 100644 --- a/cli/server.go +++ b/cli/server.go @@ -821,7 +821,7 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd. return err } - options.RuntimeConfig = runtimeconfig.NewStoreManager() + options.RuntimeConfig = runtimeconfig.NewManager() // 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 cc904b8fc92f7..bd7945541556f 100644 --- a/coderd/coderdtest/coderdtest.go +++ b/coderd/coderdtest/coderdtest.go @@ -255,7 +255,7 @@ func NewOptions(t testing.TB, options *Options) (func(http.Handler), context.Can var acs dbauthz.AccessControlStore = dbauthz.AGPLTemplateAccessControlStore{} accessControlStore.Store(&acs) - runtimeManager := runtimeconfig.NewStoreManager() + runtimeManager := runtimeconfig.NewManager() 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_test.go b/coderd/runtimeconfig/deploymententry_test.go index 62506376d906b..b1c0f8ac69bd5 100644 --- a/coderd/runtimeconfig/deploymententry_test.go +++ b/coderd/runtimeconfig/deploymententry_test.go @@ -23,7 +23,7 @@ import ( func ExampleDeploymentValues() { ctx := context.Background() db := dbmem.New() - st := runtimeconfig.NewStoreManager() + st := runtimeconfig.NewManager() // Define the field, this will usually live on Deployment Values. var stringField runtimeconfig.DeploymentEntry[*serpent.String] @@ -75,7 +75,7 @@ func TestResolveDBError(t *testing.T) { Times(1). Return(dbErr) - st := runtimeconfig.NewStoreManager() + st := runtimeconfig.NewManager() var stringField runtimeconfig.DeploymentEntry[*serpent.String] stringField.Initialize("string-field") stringField.SetStartupValue("default") @@ -100,7 +100,7 @@ func TestSerpentDeploymentEntry(t *testing.T) { ctx := testutil.Context(t, testutil.WaitMedium) db, _ := dbtestutil.NewDB(t) - st := runtimeconfig.NewStoreManager() + st := runtimeconfig.NewManager() // TestEntries is how entries are defined in deployment values. type TestEntries struct { diff --git a/coderd/runtimeconfig/entry_test.go b/coderd/runtimeconfig/entry_test.go index 3edf884399445..483172d063417 100644 --- a/coderd/runtimeconfig/entry_test.go +++ b/coderd/runtimeconfig/entry_test.go @@ -68,7 +68,7 @@ func TestEntry(t *testing.T) { t.Parallel() ctx := testutil.Context(t, testutil.WaitShort) - mgr := runtimeconfig.NewStoreManager() + mgr := runtimeconfig.NewManager() db := dbmem.New() var ( @@ -102,7 +102,7 @@ func TestEntry(t *testing.T) { t.Parallel() ctx := testutil.Context(t, testutil.WaitShort) - mgr := runtimeconfig.NewStoreManager() + mgr := runtimeconfig.NewManager() db := dbmem.New() var ( diff --git a/coderd/runtimeconfig/manager.go b/coderd/runtimeconfig/manager.go index 232f30e3e1f5d..f7861b34bd8cd 100644 --- a/coderd/runtimeconfig/manager.go +++ b/coderd/runtimeconfig/manager.go @@ -8,7 +8,7 @@ import ( // TODO: Implement caching layer. type Manager struct{} -func NewStoreManager() *Manager { +func NewManager() *Manager { return &Manager{} }