From 43a1c9eaebb644bfd1060aaf54463c94836b9f3d Mon Sep 17 00:00:00 2001 From: Spike Curtis Date: Tue, 23 Jan 2024 14:18:18 +0400 Subject: [PATCH] feat: agentapi uses appearance.Fetcher --- agent/proto/convert.go | 16 ++++ coderd/agentapi/api.go | 4 +- coderd/agentapi/servicebanner.go | 33 ++------ .../agentapi/servicebanner_internal_test.go | 72 ++++++++++++++++ coderd/agentapi/servicebanner_test.go | 84 ------------------- coderd/workspaceagentsrpc.go | 1 + enterprise/coderd/appearance_test.go | 19 +++++ 7 files changed, 120 insertions(+), 109 deletions(-) create mode 100644 coderd/agentapi/servicebanner_internal_test.go delete mode 100644 coderd/agentapi/servicebanner_test.go diff --git a/agent/proto/convert.go b/agent/proto/convert.go index 3d2397c8d35f8..67578caabeb9b 100644 --- a/agent/proto/convert.go +++ b/agent/proto/convert.go @@ -104,3 +104,19 @@ func SDKAppFromProto(protoApp *WorkspaceApp) (codersdk.WorkspaceApp, error) { Health: health, }, nil } + +func SDKServiceBannerFromProto(sbp *ServiceBanner) codersdk.ServiceBannerConfig { + return codersdk.ServiceBannerConfig{ + Enabled: sbp.GetEnabled(), + Message: sbp.GetMessage(), + BackgroundColor: sbp.GetBackgroundColor(), + } +} + +func ServiceBannerFromSDK(sb codersdk.ServiceBannerConfig) *ServiceBanner { + return &ServiceBanner{ + Enabled: sb.Enabled, + Message: sb.Message, + BackgroundColor: sb.BackgroundColor, + } +} diff --git a/coderd/agentapi/api.go b/coderd/agentapi/api.go index e3c7b1d0672b9..b76d5ac4cf65f 100644 --- a/coderd/agentapi/api.go +++ b/coderd/agentapi/api.go @@ -17,6 +17,7 @@ import ( "cdr.dev/slog" agentproto "github.com/coder/coder/v2/agent/proto" + "github.com/coder/coder/v2/coderd/appearance" "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/pubsub" "github.com/coder/coder/v2/coderd/externalauth" @@ -61,6 +62,7 @@ type Options struct { TailnetCoordinator *atomic.Pointer[tailnet.Coordinator] TemplateScheduleStore *atomic.Pointer[schedule.TemplateScheduleStore] StatsBatcher StatsBatcher + AppearanceFetcher *atomic.Pointer[appearance.Fetcher] PublishWorkspaceUpdateFn func(ctx context.Context, workspaceID uuid.UUID) PublishWorkspaceAgentLogsUpdateFn func(ctx context.Context, workspaceAgentID uuid.UUID, msg agentsdk.LogsNotifyMessage) @@ -98,7 +100,7 @@ func New(opts Options) *API { } api.ServiceBannerAPI = &ServiceBannerAPI{ - Database: opts.Database, + appearanceFetcher: opts.AppearanceFetcher, } api.StatsAPI = &StatsAPI{ diff --git a/coderd/agentapi/servicebanner.go b/coderd/agentapi/servicebanner.go index b123e78fc179e..a4bc1f457d9f0 100644 --- a/coderd/agentapi/servicebanner.go +++ b/coderd/agentapi/servicebanner.go @@ -2,37 +2,22 @@ package agentapi import ( "context" - "database/sql" - "encoding/json" + "sync/atomic" "golang.org/x/xerrors" - agentproto "github.com/coder/coder/v2/agent/proto" - "github.com/coder/coder/v2/coderd/database" - "github.com/coder/coder/v2/codersdk" + "github.com/coder/coder/v2/agent/proto" + "github.com/coder/coder/v2/coderd/appearance" ) type ServiceBannerAPI struct { - Database database.Store + appearanceFetcher *atomic.Pointer[appearance.Fetcher] } -func (a *ServiceBannerAPI) GetServiceBanner(ctx context.Context, _ *agentproto.GetServiceBannerRequest) (*agentproto.ServiceBanner, error) { - serviceBannerJSON, err := a.Database.GetServiceBanner(ctx) - if err != nil && !xerrors.Is(err, sql.ErrNoRows) { - return nil, xerrors.Errorf("get service banner: %w", err) +func (a *ServiceBannerAPI) GetServiceBanner(ctx context.Context, _ *proto.GetServiceBannerRequest) (*proto.ServiceBanner, error) { + cfg, err := (*a.appearanceFetcher.Load()).Fetch(ctx) + if err != nil { + return nil, xerrors.Errorf("fetch appearance: %w", err) } - - var cfg codersdk.ServiceBannerConfig - if serviceBannerJSON != "" { - err = json.Unmarshal([]byte(serviceBannerJSON), &cfg) - if err != nil { - return nil, xerrors.Errorf("unmarshal json: %w, raw: %s", err, serviceBannerJSON) - } - } - - return &agentproto.ServiceBanner{ - Enabled: cfg.Enabled, - Message: cfg.Message, - BackgroundColor: cfg.BackgroundColor, - }, nil + return proto.ServiceBannerFromSDK(cfg.ServiceBanner), nil } diff --git a/coderd/agentapi/servicebanner_internal_test.go b/coderd/agentapi/servicebanner_internal_test.go new file mode 100644 index 0000000000000..d559bf08604b2 --- /dev/null +++ b/coderd/agentapi/servicebanner_internal_test.go @@ -0,0 +1,72 @@ +package agentapi + +import ( + "context" + "sync/atomic" + "testing" + + "golang.org/x/xerrors" + + agentproto "github.com/coder/coder/v2/agent/proto" + "github.com/coder/coder/v2/coderd/appearance" + "github.com/coder/coder/v2/codersdk" + "github.com/stretchr/testify/require" +) + +func TestGetServiceBanner(t *testing.T) { + t.Parallel() + + t.Run("OK", func(t *testing.T) { + t.Parallel() + + cfg := codersdk.ServiceBannerConfig{ + Enabled: true, + Message: "hello world", + BackgroundColor: "#000000", + } + + var ff appearance.Fetcher = fakeFetcher{cfg: codersdk.AppearanceConfig{ServiceBanner: cfg}} + ptr := atomic.Pointer[appearance.Fetcher]{} + ptr.Store(&ff) + + api := &ServiceBannerAPI{ + appearanceFetcher: &ptr, + } + + resp, err := api.GetServiceBanner(context.Background(), &agentproto.GetServiceBannerRequest{}) + require.NoError(t, err) + + require.Equal(t, &agentproto.ServiceBanner{ + Enabled: cfg.Enabled, + Message: cfg.Message, + BackgroundColor: cfg.BackgroundColor, + }, resp) + }) + + t.Run("FetchError", func(t *testing.T) { + t.Parallel() + + expectedErr := xerrors.New("badness") + var ff appearance.Fetcher = fakeFetcher{err: expectedErr} + ptr := atomic.Pointer[appearance.Fetcher]{} + ptr.Store(&ff) + + api := &ServiceBannerAPI{ + appearanceFetcher: &ptr, + } + + resp, err := api.GetServiceBanner(context.Background(), &agentproto.GetServiceBannerRequest{}) + require.Error(t, err) + require.ErrorIs(t, err, expectedErr) + require.Nil(t, resp) + }) +} + +type fakeFetcher struct { + cfg codersdk.AppearanceConfig + err error +} + +func (f fakeFetcher) Fetch(context.Context) (codersdk.AppearanceConfig, error) { + return f.cfg, f.err +} diff --git a/coderd/agentapi/servicebanner_test.go b/coderd/agentapi/servicebanner_test.go deleted file mode 100644 index 902af7395e54d..0000000000000 --- a/coderd/agentapi/servicebanner_test.go +++ /dev/null @@ -1,84 +0,0 @@ -package agentapi_test - -import ( - "context" - "database/sql" - "encoding/json" - "testing" - - "github.com/stretchr/testify/require" - "go.uber.org/mock/gomock" - - agentproto "github.com/coder/coder/v2/agent/proto" - "github.com/coder/coder/v2/coderd/agentapi" - "github.com/coder/coder/v2/coderd/database/dbmock" - "github.com/coder/coder/v2/codersdk" -) - -func TestGetServiceBanner(t *testing.T) { - t.Parallel() - - t.Run("OK", func(t *testing.T) { - t.Parallel() - - cfg := codersdk.ServiceBannerConfig{ - Enabled: true, - Message: "hello world", - BackgroundColor: "#000000", - } - cfgJSON, err := json.Marshal(cfg) - require.NoError(t, err) - - dbM := dbmock.NewMockStore(gomock.NewController(t)) - dbM.EXPECT().GetServiceBanner(gomock.Any()).Return(string(cfgJSON), nil) - - api := &agentapi.ServiceBannerAPI{ - Database: dbM, - } - - resp, err := api.GetServiceBanner(context.Background(), &agentproto.GetServiceBannerRequest{}) - require.NoError(t, err) - - require.Equal(t, &agentproto.ServiceBanner{ - Enabled: cfg.Enabled, - Message: cfg.Message, - BackgroundColor: cfg.BackgroundColor, - }, resp) - }) - - t.Run("None", func(t *testing.T) { - t.Parallel() - - dbM := dbmock.NewMockStore(gomock.NewController(t)) - dbM.EXPECT().GetServiceBanner(gomock.Any()).Return("", sql.ErrNoRows) - - api := &agentapi.ServiceBannerAPI{ - Database: dbM, - } - - resp, err := api.GetServiceBanner(context.Background(), &agentproto.GetServiceBannerRequest{}) - require.NoError(t, err) - - require.Equal(t, &agentproto.ServiceBanner{ - Enabled: false, - Message: "", - BackgroundColor: "", - }, resp) - }) - - t.Run("BadJSON", func(t *testing.T) { - t.Parallel() - - dbM := dbmock.NewMockStore(gomock.NewController(t)) - dbM.EXPECT().GetServiceBanner(gomock.Any()).Return("hi", nil) - - api := &agentapi.ServiceBannerAPI{ - Database: dbM, - } - - resp, err := api.GetServiceBanner(context.Background(), &agentproto.GetServiceBannerRequest{}) - require.Error(t, err) - require.ErrorContains(t, err, "unmarshal json") - require.Nil(t, resp) - }) -} diff --git a/coderd/workspaceagentsrpc.go b/coderd/workspaceagentsrpc.go index 8e02fc878e243..e5398b271570e 100644 --- a/coderd/workspaceagentsrpc.go +++ b/coderd/workspaceagentsrpc.go @@ -130,6 +130,7 @@ func (api *API) workspaceAgentRPC(rw http.ResponseWriter, r *http.Request) { DerpMapFn: api.DERPMap, TailnetCoordinator: &api.TailnetCoordinator, TemplateScheduleStore: api.TemplateScheduleStore, + AppearanceFetcher: &api.AppearanceFetcher, StatsBatcher: api.statsBatcher, PublishWorkspaceUpdateFn: api.publishWorkspaceUpdate, PublishWorkspaceAgentLogsUpdateFn: api.publishWorkspaceAgentLogsUpdate, diff --git a/enterprise/coderd/appearance_test.go b/enterprise/coderd/appearance_test.go index bb15a700ce716..41e2e8c6f0d64 100644 --- a/enterprise/coderd/appearance_test.go +++ b/enterprise/coderd/appearance_test.go @@ -9,6 +9,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/coder/coder/v2/agent/proto" "github.com/coder/coder/v2/cli/clibase" "github.com/coder/coder/v2/coderd/appearance" "github.com/coder/coder/v2/coderd/coderdtest" @@ -159,6 +160,8 @@ func TestServiceBanners(t *testing.T) { banner, err := agentClient.GetServiceBanner(ctx) require.NoError(t, err) require.Equal(t, cfg.ServiceBanner, banner) + banner = requireGetServiceBannerV2(ctx, t, agentClient) + require.Equal(t, cfg.ServiceBanner, banner) // Create an AGPL Coderd against the same database agplClient := coderdtest.New(t, &coderdtest.Options{Database: store, Pubsub: ps}) @@ -167,6 +170,8 @@ func TestServiceBanners(t *testing.T) { banner, err = agplAgentClient.GetServiceBanner(ctx) require.NoError(t, err) require.Equal(t, codersdk.ServiceBannerConfig{}, banner) + banner = requireGetServiceBannerV2(ctx, t, agplAgentClient) + require.Equal(t, codersdk.ServiceBannerConfig{}, banner) // No license means no banner. err = client.DeleteLicense(ctx, lic.ID) @@ -174,9 +179,23 @@ func TestServiceBanners(t *testing.T) { banner, err = agentClient.GetServiceBanner(ctx) require.NoError(t, err) require.Equal(t, codersdk.ServiceBannerConfig{}, banner) + banner = requireGetServiceBannerV2(ctx, t, agentClient) + require.Equal(t, codersdk.ServiceBannerConfig{}, banner) }) } +func requireGetServiceBannerV2(ctx context.Context, t *testing.T, client *agentsdk.Client) codersdk.ServiceBannerConfig { + cc, err := client.Listen(ctx) + require.NoError(t, err) + defer func() { + _ = cc.Close() + }() + aAPI := proto.NewDRPCAgentClient(cc) + sbp, err := aAPI.GetServiceBanner(ctx, &proto.GetServiceBannerRequest{}) + require.NoError(t, err) + return proto.SDKServiceBannerFromProto(sbp) +} + func TestCustomSupportLinks(t *testing.T) { t.Parallel()