Skip to content

Commit 5ad6f3f

Browse files
spikecurtisspikecurtis
committed
chore: refactor Appearance to an interface callable by AGPL code
1 parent f168e58 commit 5ad6f3f

File tree

6 files changed

+108
-70
lines changed

6 files changed

+108
-70
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 AppearanceFetcher 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 AGPLAppearance struct{}
32+
33+
func (AGPLAppearance) Fetch(context.Context) (codersdk.AppearanceConfig, error) {
34+
return codersdk.AppearanceConfig{
35+
SupportLinks: DefaultSupportLinks,
36+
}, nil
37+
}
38+
39+
var DefaultAppearanceFetcher AppearanceFetcher = AGPLAppearance{}

‎coderd/coderd.go

+8-7
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"
@@ -523,6 +521,8 @@ func New(options *Options) *API {
523521
DisablePathApps: options.DeploymentValues.DisablePathApps.Value(),
524522
SecureAuthCookie: options.DeploymentValues.SecureAuthCookie.Value(),
525523
}
524+
api.AppearanceFetcher.Store(&appearance.DefaultAppearanceFetcher)
525+
api.SiteHandler.AppearanceFetcher = &api.AppearanceFetcher
526526

527527
apiKeyMiddleware := httpmw.ExtractAPIKeyMW(httpmw.ExtractAPIKeyConfig{
528528
DB: options.Database,
@@ -1087,6 +1087,7 @@ type API struct {
10871087
TailnetCoordinator atomic.Pointer[tailnet.Coordinator]
10881088
TailnetClientService *tailnet.ClientService
10891089
QuotaCommitter atomic.Pointer[proto.QuotaCommitter]
1090+
AppearanceFetcher atomic.Pointer[appearance.AppearanceFetcher]
10901091
// WorkspaceProxyHostsFn returns the hosts of healthy workspace proxies
10911092
// for header reasons.
10921093
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.AppearanceFetcher {
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.DefaultAppearanceFetcher)
690+
}
691+
}
692+
680693
// External token encryption is soft-enforced
681694
featureExternalTokenEncryption := entitlements.Features[codersdk.FeatureExternalTokenEncryption]
682695
featureExternalTokenEncryption.Enabled = len(api.ExternalTokenEncryption) > 0

‎site/site.go

+19-19
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ import (
3535
"golang.org/x/xerrors"
3636

3737
"github.com/coder/coder/v2/buildinfo"
38+
"github.com/coder/coder/v2/coderd/appearance"
3839
"github.com/coder/coder/v2/coderd/database"
3940
"github.com/coder/coder/v2/coderd/database/db2sdk"
4041
"github.com/coder/coder/v2/coderd/database/dbauthz"
@@ -147,7 +148,7 @@ type Handler struct {
147148

148149
buildInfoJSON string
149150

150-
AppearanceFetcher func(ctx context.Context) (codersdk.AppearanceConfig, error)
151+
AppearanceFetcher *atomic.Pointer[appearance.AppearanceFetcher]
151152
// RegionsFetcher will attempt to fetch the more detailed WorkspaceProxy data, but will fall back to the
152153
// regions if the user does not have the correct permissions.
153154
RegionsFetcher func(ctx context.Context) (any, error)
@@ -291,6 +292,7 @@ func execTmpl(tmpl *template.Template, state htmlState) ([]byte, error) {
291292
// renderWithState will render the file using the given nonce if the file exists
292293
// as a template. If it does not, it will return an error.
293294
func (h *Handler) renderHTMLWithState(r *http.Request, filePath string, state htmlState) ([]byte, error) {
295+
af := *(h.AppearanceFetcher.Load())
294296
if filePath == "" {
295297
filePath = "index.html"
296298
}
@@ -317,11 +319,9 @@ func (h *Handler) renderHTMLWithState(r *http.Request, filePath string, state ht
317319
})
318320
if !ok || apiKey == nil || actor == nil {
319321
var cfg codersdk.AppearanceConfig
320-
if h.AppearanceFetcher != nil {
321-
// nolint:gocritic // User is not expected to be signed in.
322-
ctx := dbauthz.AsSystemRestricted(r.Context())
323-
cfg, _ = h.AppearanceFetcher(ctx)
324-
}
322+
// nolint:gocritic // User is not expected to be signed in.
323+
ctx := dbauthz.AsSystemRestricted(r.Context())
324+
cfg, _ = af.Fetch(ctx)
325325
state.ApplicationName = applicationNameOrDefault(cfg)
326326
state.LogoURL = cfg.LogoURL
327327
return execTmpl(tmpl, state)
@@ -370,21 +370,21 @@ func (h *Handler) renderHTMLWithState(r *http.Request, filePath string, state ht
370370
}
371371
}()
372372
}
373-
if h.AppearanceFetcher != nil {
374-
wg.Add(1)
375-
go func() {
376-
defer wg.Done()
377-
cfg, err := h.AppearanceFetcher(ctx)
373+
374+
wg.Add(1)
375+
go func() {
376+
defer wg.Done()
377+
cfg, err := af.Fetch(ctx)
378+
if err == nil {
379+
appr, err := json.Marshal(cfg)
378380
if err == nil {
379-
appearance, err := json.Marshal(cfg)
380-
if err == nil {
381-
state.Appearance = html.EscapeString(string(appearance))
382-
state.ApplicationName = applicationNameOrDefault(cfg)
383-
state.LogoURL = cfg.LogoURL
384-
}
381+
state.Appearance = html.EscapeString(string(appr))
382+
state.ApplicationName = applicationNameOrDefault(cfg)
383+
state.LogoURL = cfg.LogoURL
385384
}
386-
}()
387-
}
385+
}
386+
}()
387+
388388
if h.RegionsFetcher != nil {
389389
wg.Add(1)
390390
go func() {

0 commit comments

Comments
 (0)