Skip to content

Commit bc4ae53

Browse files
spikecurtisspikecurtis
authored
chore: refactor Appearance to an interface callable by AGPL code (#11769)
The new Agent API needs an interface for ServiceBanners, so this PR creates it and refactors the AGPL and Enterprise code to achieve it. Before we depended on the fact that the HTTP endpoint was missing to serve an empty ServiceBanner on AGPL deployments, but that won't work with dRPC, so we need a real interface to call.
1 parent aacb4a2 commit bc4ae53

File tree

6 files changed

+130
-87
lines changed

6 files changed

+130
-87
lines changed

‎coderd/appearance/appearance.go

+39
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
package appearance
2+
3+
import (
4+
"context"
5+
6+
"github.com/coder/coder/v2/codersdk"
7+
)
8+
9+
type Fetcher interface {
10+
Fetch(ctx context.Context) (codersdk.AppearanceConfig, error)
11+
}
12+
13+
var DefaultSupportLinks = []codersdk.LinkConfig{
14+
{
15+
Name: "Documentation",
16+
Target: "https://coder.com/docs/coder-oss",
17+
Icon: "docs",
18+
},
19+
{
20+
Name: "Report a bug",
21+
Target: "https://github.com/coder/coder/issues/new?labels=needs+grooming&body={CODER_BUILD_INFO}",
22+
Icon: "bug",
23+
},
24+
{
25+
Name: "Join the Coder Discord",
26+
Target: "https://coder.com/chat?utm_source=coder&utm_medium=coder&utm_campaign=server-footer",
27+
Icon: "chat",
28+
},
29+
}
30+
31+
type AGPLFetcher struct{}
32+
33+
func (AGPLFetcher) Fetch(context.Context) (codersdk.AppearanceConfig, error) {
34+
return codersdk.AppearanceConfig{
35+
SupportLinks: DefaultSupportLinks,
36+
}, nil
37+
}
38+
39+
var DefaultFetcher Fetcher = AGPLFetcher{}

‎coderd/coderd.go

+19-18
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,6 @@ import (
1717
"sync/atomic"
1818
"time"
1919

20-
"github.com/coder/coder/v2/coderd/prometheusmetrics"
21-
22-
agentproto "github.com/coder/coder/v2/agent/proto"
23-
2420
"github.com/andybalholm/brotli"
2521
"github.com/go-chi/chi/v5"
2622
"github.com/go-chi/chi/v5/middleware"
@@ -40,11 +36,12 @@ import (
4036
"tailscale.com/util/singleflight"
4137

4238
"cdr.dev/slog"
39+
40+
agentproto "github.com/coder/coder/v2/agent/proto"
4341
"github.com/coder/coder/v2/buildinfo"
4442
"github.com/coder/coder/v2/cli/clibase"
45-
46-
// Used for swagger docs.
47-
_ "github.com/coder/coder/v2/coderd/apidoc"
43+
_ "github.com/coder/coder/v2/coderd/apidoc" // Used for swagger docs.
44+
"github.com/coder/coder/v2/coderd/appearance"
4845
"github.com/coder/coder/v2/coderd/audit"
4946
"github.com/coder/coder/v2/coderd/awsidentity"
5047
"github.com/coder/coder/v2/coderd/batchstats"
@@ -59,6 +56,7 @@ import (
5956
"github.com/coder/coder/v2/coderd/httpapi"
6057
"github.com/coder/coder/v2/coderd/httpmw"
6158
"github.com/coder/coder/v2/coderd/metricscache"
59+
"github.com/coder/coder/v2/coderd/prometheusmetrics"
6260
"github.com/coder/coder/v2/coderd/provisionerdserver"
6361
"github.com/coder/coder/v2/coderd/rbac"
6462
"github.com/coder/coder/v2/coderd/schedule"
@@ -357,16 +355,6 @@ func New(options *Options) *API {
357355
OIDC: options.OIDCConfig,
358356
}
359357

360-
staticHandler := site.New(&site.Options{
361-
BinFS: binFS,
362-
BinHashes: binHashes,
363-
Database: options.Database,
364-
SiteFS: site.FS(),
365-
OAuth2Configs: oauthConfigs,
366-
DocsURL: options.DeploymentValues.DocsURL.String(),
367-
})
368-
staticHandler.Experiments.Store(&experiments)
369-
370358
ctx, cancel := context.WithCancel(context.Background())
371359
r := chi.NewRouter()
372360

@@ -383,7 +371,6 @@ func New(options *Options) *API {
383371
ID: uuid.New(),
384372
Options: options,
385373
RootHandler: r,
386-
SiteHandler: staticHandler,
387374
HTTPAuth: &HTTPAuthorizer{
388375
Authorizer: options.Authorizer,
389376
Logger: options.Logger,
@@ -412,6 +399,19 @@ func New(options *Options) *API {
412399
options.Database,
413400
options.Pubsub),
414401
}
402+
403+
api.AppearanceFetcher.Store(&appearance.DefaultFetcher)
404+
api.SiteHandler = site.New(&site.Options{
405+
BinFS: binFS,
406+
BinHashes: binHashes,
407+
Database: options.Database,
408+
SiteFS: site.FS(),
409+
OAuth2Configs: oauthConfigs,
410+
DocsURL: options.DeploymentValues.DocsURL.String(),
411+
AppearanceFetcher: &api.AppearanceFetcher,
412+
})
413+
api.SiteHandler.Experiments.Store(&experiments)
414+
415415
if options.UpdateCheckOptions != nil {
416416
api.updateChecker = updatecheck.New(
417417
options.Database,
@@ -1089,6 +1089,7 @@ type API struct {
10891089
TailnetCoordinator atomic.Pointer[tailnet.Coordinator]
10901090
TailnetClientService *tailnet.ClientService
10911091
QuotaCommitter atomic.Pointer[proto.QuotaCommitter]
1092+
AppearanceFetcher atomic.Pointer[appearance.Fetcher]
10921093
// WorkspaceProxyHostsFn returns the hosts of healthy workspace proxies
10931094
// for header reasons.
10941095
WorkspaceProxyHostsFn atomic.Pointer[func() []string]

‎enterprise/coderd/appearance.go

+20-33
Original file line numberDiff line numberDiff line change
@@ -11,29 +11,13 @@ import (
1111
"golang.org/x/sync/errgroup"
1212
"golang.org/x/xerrors"
1313

14+
agpl "github.com/coder/coder/v2/coderd/appearance"
15+
"github.com/coder/coder/v2/coderd/database"
1416
"github.com/coder/coder/v2/coderd/httpapi"
1517
"github.com/coder/coder/v2/coderd/rbac"
1618
"github.com/coder/coder/v2/codersdk"
1719
)
1820

19-
var DefaultSupportLinks = []codersdk.LinkConfig{
20-
{
21-
Name: "Documentation",
22-
Target: "https://coder.com/docs/coder-oss",
23-
Icon: "docs",
24-
},
25-
{
26-
Name: "Report a bug",
27-
Target: "https://github.com/coder/coder/issues/new?labels=needs+grooming&body={CODER_BUILD_INFO}",
28-
Icon: "bug",
29-
},
30-
{
31-
Name: "Join the Coder Discord",
32-
Target: "https://coder.com/chat?utm_source=coder&utm_medium=coder&utm_campaign=server-footer",
33-
Icon: "chat",
34-
},
35-
}
36-
3721
// @Summary Get appearance
3822
// @ID get-appearance
3923
// @Security CoderSessionToken
@@ -42,7 +26,8 @@ var DefaultSupportLinks = []codersdk.LinkConfig{
4226
// @Success 200 {object} codersdk.AppearanceConfig
4327
// @Router /appearance [get]
4428
func (api *API) appearance(rw http.ResponseWriter, r *http.Request) {
45-
cfg, err := api.fetchAppearanceConfig(r.Context())
29+
af := *api.AGPL.AppearanceFetcher.Load()
30+
cfg, err := af.Fetch(r.Context())
4631
if err != nil {
4732
httpapi.Write(r.Context(), rw, http.StatusInternalServerError, codersdk.Response{
4833
Message: "Failed to fetch appearance config.",
@@ -54,37 +39,39 @@ func (api *API) appearance(rw http.ResponseWriter, r *http.Request) {
5439
httpapi.Write(r.Context(), rw, http.StatusOK, cfg)
5540
}
5641

57-
func (api *API) fetchAppearanceConfig(ctx context.Context) (codersdk.AppearanceConfig, error) {
58-
api.entitlementsMu.RLock()
59-
isEntitled := api.entitlements.Features[codersdk.FeatureAppearance].Entitlement == codersdk.EntitlementEntitled
60-
api.entitlementsMu.RUnlock()
42+
type appearanceFetcher struct {
43+
database database.Store
44+
supportLinks []codersdk.LinkConfig
45+
}
6146

62-
if !isEntitled {
63-
return codersdk.AppearanceConfig{
64-
SupportLinks: DefaultSupportLinks,
65-
}, nil
47+
func newAppearanceFetcher(store database.Store, links []codersdk.LinkConfig) agpl.Fetcher {
48+
return &appearanceFetcher{
49+
database: store,
50+
supportLinks: links,
6651
}
52+
}
6753

54+
func (f *appearanceFetcher) Fetch(ctx context.Context) (codersdk.AppearanceConfig, error) {
6855
var eg errgroup.Group
6956
var applicationName string
7057
var logoURL string
7158
var serviceBannerJSON string
7259
eg.Go(func() (err error) {
73-
applicationName, err = api.Database.GetApplicationName(ctx)
60+
applicationName, err = f.database.GetApplicationName(ctx)
7461
if err != nil && !errors.Is(err, sql.ErrNoRows) {
7562
return xerrors.Errorf("get application name: %w", err)
7663
}
7764
return nil
7865
})
7966
eg.Go(func() (err error) {
80-
logoURL, err = api.Database.GetLogoURL(ctx)
67+
logoURL, err = f.database.GetLogoURL(ctx)
8168
if err != nil && !errors.Is(err, sql.ErrNoRows) {
8269
return xerrors.Errorf("get logo url: %w", err)
8370
}
8471
return nil
8572
})
8673
eg.Go(func() (err error) {
87-
serviceBannerJSON, err = api.Database.GetServiceBanner(ctx)
74+
serviceBannerJSON, err = f.database.GetServiceBanner(ctx)
8875
if err != nil && !errors.Is(err, sql.ErrNoRows) {
8976
return xerrors.Errorf("get service banner: %w", err)
9077
}
@@ -108,10 +95,10 @@ func (api *API) fetchAppearanceConfig(ctx context.Context) (codersdk.AppearanceC
10895
}
10996
}
11097

111-
if len(api.DeploymentValues.Support.Links.Value) == 0 {
112-
cfg.SupportLinks = DefaultSupportLinks
98+
if len(f.supportLinks) == 0 {
99+
cfg.SupportLinks = agpl.DefaultSupportLinks
113100
} else {
114-
cfg.SupportLinks = api.DeploymentValues.Support.Links.Value
101+
cfg.SupportLinks = f.supportLinks
115102
}
116103

117104
return cfg, nil

‎enterprise/coderd/appearance_test.go

+8-10
Original file line numberDiff line numberDiff line change
@@ -6,19 +6,17 @@ import (
66
"net/http"
77
"testing"
88

9-
"github.com/coder/coder/v2/coderd/database/dbtestutil"
10-
11-
"github.com/coder/coder/v2/coderd/database"
12-
"github.com/coder/coder/v2/coderd/database/dbfake"
13-
149
"github.com/stretchr/testify/assert"
1510
"github.com/stretchr/testify/require"
1611

1712
"github.com/coder/coder/v2/cli/clibase"
13+
"github.com/coder/coder/v2/coderd/appearance"
1814
"github.com/coder/coder/v2/coderd/coderdtest"
15+
"github.com/coder/coder/v2/coderd/database"
16+
"github.com/coder/coder/v2/coderd/database/dbfake"
17+
"github.com/coder/coder/v2/coderd/database/dbtestutil"
1918
"github.com/coder/coder/v2/codersdk"
2019
"github.com/coder/coder/v2/codersdk/agentsdk"
21-
"github.com/coder/coder/v2/enterprise/coderd"
2220
"github.com/coder/coder/v2/enterprise/coderd/coderdenttest"
2321
"github.com/coder/coder/v2/enterprise/coderd/license"
2422
"github.com/coder/coder/v2/testutil"
@@ -214,9 +212,9 @@ func TestCustomSupportLinks(t *testing.T) {
214212
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitMedium)
215213
defer cancel()
216214

217-
appearance, err := anotherClient.Appearance(ctx)
215+
appr, err := anotherClient.Appearance(ctx)
218216
require.NoError(t, err)
219-
require.Equal(t, supportLinks, appearance.SupportLinks)
217+
require.Equal(t, supportLinks, appr.SupportLinks)
220218
}
221219

222220
func TestDefaultSupportLinks(t *testing.T) {
@@ -229,7 +227,7 @@ func TestDefaultSupportLinks(t *testing.T) {
229227
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitMedium)
230228
defer cancel()
231229

232-
appearance, err := anotherClient.Appearance(ctx)
230+
appr, err := anotherClient.Appearance(ctx)
233231
require.NoError(t, err)
234-
require.Equal(t, coderd.DefaultSupportLinks, appearance.SupportLinks)
232+
require.Equal(t, appearance.DefaultSupportLinks, appr.SupportLinks)
235233
}

‎enterprise/coderd/coderd.go

+14-1
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ import (
1414
"sync"
1515
"time"
1616

17+
"github.com/coder/coder/v2/coderd/appearance"
18+
1719
"golang.org/x/xerrors"
1820
"tailscale.com/tailcfg"
1921

@@ -118,7 +120,6 @@ func New(ctx context.Context, options *Options) (_ *API, err error) {
118120
}
119121
api.AGPL.Options.SetUserGroups = api.setUserGroups
120122
api.AGPL.Options.SetUserSiteRoles = api.setUserSiteRoles
121-
api.AGPL.SiteHandler.AppearanceFetcher = api.fetchAppearanceConfig
122123
api.AGPL.SiteHandler.RegionsFetcher = func(ctx context.Context) (any, error) {
123124
// If the user can read the workspace proxy resource, return that.
124125
// If not, always default to the regions.
@@ -677,6 +678,18 @@ func (api *API) updateEntitlements(ctx context.Context) error {
677678
api.AGPL.AccessControlStore.Store(&acs)
678679
}
679680

681+
if initial, changed, enabled := featureChanged(codersdk.FeatureAppearance); shouldUpdate(initial, changed, enabled) {
682+
if enabled {
683+
f := newAppearanceFetcher(
684+
api.Database,
685+
api.DeploymentValues.Support.Links.Value,
686+
)
687+
api.AGPL.AppearanceFetcher.Store(&f)
688+
} else {
689+
api.AGPL.AppearanceFetcher.Store(&appearance.DefaultFetcher)
690+
}
691+
}
692+
680693
// External token encryption is soft-enforced
681694
featureExternalTokenEncryption := entitlements.Features[codersdk.FeatureExternalTokenEncryption]
682695
featureExternalTokenEncryption.Enabled = len(api.ExternalTokenEncryption) > 0

0 commit comments

Comments
 (0)