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.
This commit is contained in:
Spike Curtis 2024-01-29 12:17:31 +04:00 committed by GitHub
parent aacb4a2b4c
commit bc4ae53261
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 130 additions and 87 deletions

View File

@ -0,0 +1,39 @@
package appearance
import (
"context"
"github.com/coder/coder/v2/codersdk"
)
type Fetcher interface {
Fetch(ctx context.Context) (codersdk.AppearanceConfig, error)
}
var DefaultSupportLinks = []codersdk.LinkConfig{
{
Name: "Documentation",
Target: "https://coder.com/docs/coder-oss",
Icon: "docs",
},
{
Name: "Report a bug",
Target: "https://github.com/coder/coder/issues/new?labels=needs+grooming&body={CODER_BUILD_INFO}",
Icon: "bug",
},
{
Name: "Join the Coder Discord",
Target: "https://coder.com/chat?utm_source=coder&utm_medium=coder&utm_campaign=server-footer",
Icon: "chat",
},
}
type AGPLFetcher struct{}
func (AGPLFetcher) Fetch(context.Context) (codersdk.AppearanceConfig, error) {
return codersdk.AppearanceConfig{
SupportLinks: DefaultSupportLinks,
}, nil
}
var DefaultFetcher Fetcher = AGPLFetcher{}

View File

@ -17,10 +17,6 @@ import (
"sync/atomic"
"time"
"github.com/coder/coder/v2/coderd/prometheusmetrics"
agentproto "github.com/coder/coder/v2/agent/proto"
"github.com/andybalholm/brotli"
"github.com/go-chi/chi/v5"
"github.com/go-chi/chi/v5/middleware"
@ -40,11 +36,12 @@ import (
"tailscale.com/util/singleflight"
"cdr.dev/slog"
agentproto "github.com/coder/coder/v2/agent/proto"
"github.com/coder/coder/v2/buildinfo"
"github.com/coder/coder/v2/cli/clibase"
// Used for swagger docs.
_ "github.com/coder/coder/v2/coderd/apidoc"
_ "github.com/coder/coder/v2/coderd/apidoc" // Used for swagger docs.
"github.com/coder/coder/v2/coderd/appearance"
"github.com/coder/coder/v2/coderd/audit"
"github.com/coder/coder/v2/coderd/awsidentity"
"github.com/coder/coder/v2/coderd/batchstats"
@ -59,6 +56,7 @@ import (
"github.com/coder/coder/v2/coderd/httpapi"
"github.com/coder/coder/v2/coderd/httpmw"
"github.com/coder/coder/v2/coderd/metricscache"
"github.com/coder/coder/v2/coderd/prometheusmetrics"
"github.com/coder/coder/v2/coderd/provisionerdserver"
"github.com/coder/coder/v2/coderd/rbac"
"github.com/coder/coder/v2/coderd/schedule"
@ -357,16 +355,6 @@ func New(options *Options) *API {
OIDC: options.OIDCConfig,
}
staticHandler := site.New(&site.Options{
BinFS: binFS,
BinHashes: binHashes,
Database: options.Database,
SiteFS: site.FS(),
OAuth2Configs: oauthConfigs,
DocsURL: options.DeploymentValues.DocsURL.String(),
})
staticHandler.Experiments.Store(&experiments)
ctx, cancel := context.WithCancel(context.Background())
r := chi.NewRouter()
@ -383,7 +371,6 @@ func New(options *Options) *API {
ID: uuid.New(),
Options: options,
RootHandler: r,
SiteHandler: staticHandler,
HTTPAuth: &HTTPAuthorizer{
Authorizer: options.Authorizer,
Logger: options.Logger,
@ -412,6 +399,19 @@ func New(options *Options) *API {
options.Database,
options.Pubsub),
}
api.AppearanceFetcher.Store(&appearance.DefaultFetcher)
api.SiteHandler = site.New(&site.Options{
BinFS: binFS,
BinHashes: binHashes,
Database: options.Database,
SiteFS: site.FS(),
OAuth2Configs: oauthConfigs,
DocsURL: options.DeploymentValues.DocsURL.String(),
AppearanceFetcher: &api.AppearanceFetcher,
})
api.SiteHandler.Experiments.Store(&experiments)
if options.UpdateCheckOptions != nil {
api.updateChecker = updatecheck.New(
options.Database,
@ -1089,6 +1089,7 @@ type API struct {
TailnetCoordinator atomic.Pointer[tailnet.Coordinator]
TailnetClientService *tailnet.ClientService
QuotaCommitter atomic.Pointer[proto.QuotaCommitter]
AppearanceFetcher atomic.Pointer[appearance.Fetcher]
// WorkspaceProxyHostsFn returns the hosts of healthy workspace proxies
// for header reasons.
WorkspaceProxyHostsFn atomic.Pointer[func() []string]

View File

@ -11,29 +11,13 @@ import (
"golang.org/x/sync/errgroup"
"golang.org/x/xerrors"
agpl "github.com/coder/coder/v2/coderd/appearance"
"github.com/coder/coder/v2/coderd/database"
"github.com/coder/coder/v2/coderd/httpapi"
"github.com/coder/coder/v2/coderd/rbac"
"github.com/coder/coder/v2/codersdk"
)
var DefaultSupportLinks = []codersdk.LinkConfig{
{
Name: "Documentation",
Target: "https://coder.com/docs/coder-oss",
Icon: "docs",
},
{
Name: "Report a bug",
Target: "https://github.com/coder/coder/issues/new?labels=needs+grooming&body={CODER_BUILD_INFO}",
Icon: "bug",
},
{
Name: "Join the Coder Discord",
Target: "https://coder.com/chat?utm_source=coder&utm_medium=coder&utm_campaign=server-footer",
Icon: "chat",
},
}
// @Summary Get appearance
// @ID get-appearance
// @Security CoderSessionToken
@ -42,7 +26,8 @@ var DefaultSupportLinks = []codersdk.LinkConfig{
// @Success 200 {object} codersdk.AppearanceConfig
// @Router /appearance [get]
func (api *API) appearance(rw http.ResponseWriter, r *http.Request) {
cfg, err := api.fetchAppearanceConfig(r.Context())
af := *api.AGPL.AppearanceFetcher.Load()
cfg, err := af.Fetch(r.Context())
if err != nil {
httpapi.Write(r.Context(), rw, http.StatusInternalServerError, codersdk.Response{
Message: "Failed to fetch appearance config.",
@ -54,37 +39,39 @@ func (api *API) appearance(rw http.ResponseWriter, r *http.Request) {
httpapi.Write(r.Context(), rw, http.StatusOK, cfg)
}
func (api *API) fetchAppearanceConfig(ctx context.Context) (codersdk.AppearanceConfig, error) {
api.entitlementsMu.RLock()
isEntitled := api.entitlements.Features[codersdk.FeatureAppearance].Entitlement == codersdk.EntitlementEntitled
api.entitlementsMu.RUnlock()
type appearanceFetcher struct {
database database.Store
supportLinks []codersdk.LinkConfig
}
if !isEntitled {
return codersdk.AppearanceConfig{
SupportLinks: DefaultSupportLinks,
}, nil
func newAppearanceFetcher(store database.Store, links []codersdk.LinkConfig) agpl.Fetcher {
return &appearanceFetcher{
database: store,
supportLinks: links,
}
}
func (f *appearanceFetcher) Fetch(ctx context.Context) (codersdk.AppearanceConfig, error) {
var eg errgroup.Group
var applicationName string
var logoURL string
var serviceBannerJSON string
eg.Go(func() (err error) {
applicationName, err = api.Database.GetApplicationName(ctx)
applicationName, err = f.database.GetApplicationName(ctx)
if err != nil && !errors.Is(err, sql.ErrNoRows) {
return xerrors.Errorf("get application name: %w", err)
}
return nil
})
eg.Go(func() (err error) {
logoURL, err = api.Database.GetLogoURL(ctx)
logoURL, err = f.database.GetLogoURL(ctx)
if err != nil && !errors.Is(err, sql.ErrNoRows) {
return xerrors.Errorf("get logo url: %w", err)
}
return nil
})
eg.Go(func() (err error) {
serviceBannerJSON, err = api.Database.GetServiceBanner(ctx)
serviceBannerJSON, err = f.database.GetServiceBanner(ctx)
if err != nil && !errors.Is(err, sql.ErrNoRows) {
return xerrors.Errorf("get service banner: %w", err)
}
@ -108,10 +95,10 @@ func (api *API) fetchAppearanceConfig(ctx context.Context) (codersdk.AppearanceC
}
}
if len(api.DeploymentValues.Support.Links.Value) == 0 {
cfg.SupportLinks = DefaultSupportLinks
if len(f.supportLinks) == 0 {
cfg.SupportLinks = agpl.DefaultSupportLinks
} else {
cfg.SupportLinks = api.DeploymentValues.Support.Links.Value
cfg.SupportLinks = f.supportLinks
}
return cfg, nil

View File

@ -6,19 +6,17 @@ import (
"net/http"
"testing"
"github.com/coder/coder/v2/coderd/database/dbtestutil"
"github.com/coder/coder/v2/coderd/database"
"github.com/coder/coder/v2/coderd/database/dbfake"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/coder/coder/v2/cli/clibase"
"github.com/coder/coder/v2/coderd/appearance"
"github.com/coder/coder/v2/coderd/coderdtest"
"github.com/coder/coder/v2/coderd/database"
"github.com/coder/coder/v2/coderd/database/dbfake"
"github.com/coder/coder/v2/coderd/database/dbtestutil"
"github.com/coder/coder/v2/codersdk"
"github.com/coder/coder/v2/codersdk/agentsdk"
"github.com/coder/coder/v2/enterprise/coderd"
"github.com/coder/coder/v2/enterprise/coderd/coderdenttest"
"github.com/coder/coder/v2/enterprise/coderd/license"
"github.com/coder/coder/v2/testutil"
@ -214,9 +212,9 @@ func TestCustomSupportLinks(t *testing.T) {
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitMedium)
defer cancel()
appearance, err := anotherClient.Appearance(ctx)
appr, err := anotherClient.Appearance(ctx)
require.NoError(t, err)
require.Equal(t, supportLinks, appearance.SupportLinks)
require.Equal(t, supportLinks, appr.SupportLinks)
}
func TestDefaultSupportLinks(t *testing.T) {
@ -229,7 +227,7 @@ func TestDefaultSupportLinks(t *testing.T) {
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitMedium)
defer cancel()
appearance, err := anotherClient.Appearance(ctx)
appr, err := anotherClient.Appearance(ctx)
require.NoError(t, err)
require.Equal(t, coderd.DefaultSupportLinks, appearance.SupportLinks)
require.Equal(t, appearance.DefaultSupportLinks, appr.SupportLinks)
}

View File

@ -14,6 +14,8 @@ import (
"sync"
"time"
"github.com/coder/coder/v2/coderd/appearance"
"golang.org/x/xerrors"
"tailscale.com/tailcfg"
@ -118,7 +120,6 @@ func New(ctx context.Context, options *Options) (_ *API, err error) {
}
api.AGPL.Options.SetUserGroups = api.setUserGroups
api.AGPL.Options.SetUserSiteRoles = api.setUserSiteRoles
api.AGPL.SiteHandler.AppearanceFetcher = api.fetchAppearanceConfig
api.AGPL.SiteHandler.RegionsFetcher = func(ctx context.Context) (any, error) {
// If the user can read the workspace proxy resource, return that.
// If not, always default to the regions.
@ -677,6 +678,18 @@ func (api *API) updateEntitlements(ctx context.Context) error {
api.AGPL.AccessControlStore.Store(&acs)
}
if initial, changed, enabled := featureChanged(codersdk.FeatureAppearance); shouldUpdate(initial, changed, enabled) {
if enabled {
f := newAppearanceFetcher(
api.Database,
api.DeploymentValues.Support.Links.Value,
)
api.AGPL.AppearanceFetcher.Store(&f)
} else {
api.AGPL.AppearanceFetcher.Store(&appearance.DefaultFetcher)
}
}
// External token encryption is soft-enforced
featureExternalTokenEncryption := entitlements.Features[codersdk.FeatureExternalTokenEncryption]
featureExternalTokenEncryption.Enabled = len(api.ExternalTokenEncryption) > 0

View File

@ -35,6 +35,7 @@ import (
"golang.org/x/xerrors"
"github.com/coder/coder/v2/buildinfo"
"github.com/coder/coder/v2/coderd/appearance"
"github.com/coder/coder/v2/coderd/database"
"github.com/coder/coder/v2/coderd/database/db2sdk"
"github.com/coder/coder/v2/coderd/database/dbauthz"
@ -61,15 +62,21 @@ func init() {
}
type Options struct {
BinFS http.FileSystem
BinHashes map[string]string
Database database.Store
SiteFS fs.FS
OAuth2Configs *httpmw.OAuth2Configs
DocsURL string
BinFS http.FileSystem
BinHashes map[string]string
Database database.Store
SiteFS fs.FS
OAuth2Configs *httpmw.OAuth2Configs
DocsURL string
AppearanceFetcher *atomic.Pointer[appearance.Fetcher]
}
func New(opts *Options) *Handler {
if opts.AppearanceFetcher == nil {
daf := atomic.Pointer[appearance.Fetcher]{}
daf.Store(&appearance.DefaultFetcher)
opts.AppearanceFetcher = &daf
}
handler := &Handler{
opts: opts,
secureHeaders: secureHeaders(),
@ -147,7 +154,6 @@ type Handler struct {
buildInfoJSON string
AppearanceFetcher func(ctx context.Context) (codersdk.AppearanceConfig, error)
// RegionsFetcher will attempt to fetch the more detailed WorkspaceProxy data, but will fall back to the
// regions if the user does not have the correct permissions.
RegionsFetcher func(ctx context.Context) (any, error)
@ -291,6 +297,7 @@ func execTmpl(tmpl *template.Template, state htmlState) ([]byte, error) {
// renderWithState will render the file using the given nonce if the file exists
// as a template. If it does not, it will return an error.
func (h *Handler) renderHTMLWithState(r *http.Request, filePath string, state htmlState) ([]byte, error) {
af := *(h.opts.AppearanceFetcher.Load())
if filePath == "" {
filePath = "index.html"
}
@ -317,11 +324,9 @@ func (h *Handler) renderHTMLWithState(r *http.Request, filePath string, state ht
})
if !ok || apiKey == nil || actor == nil {
var cfg codersdk.AppearanceConfig
if h.AppearanceFetcher != nil {
// nolint:gocritic // User is not expected to be signed in.
ctx := dbauthz.AsSystemRestricted(r.Context())
cfg, _ = h.AppearanceFetcher(ctx)
}
// nolint:gocritic // User is not expected to be signed in.
ctx := dbauthz.AsSystemRestricted(r.Context())
cfg, _ = af.Fetch(ctx)
state.ApplicationName = applicationNameOrDefault(cfg)
state.LogoURL = cfg.LogoURL
return execTmpl(tmpl, state)
@ -370,21 +375,21 @@ func (h *Handler) renderHTMLWithState(r *http.Request, filePath string, state ht
}
}()
}
if h.AppearanceFetcher != nil {
wg.Add(1)
go func() {
defer wg.Done()
cfg, err := h.AppearanceFetcher(ctx)
wg.Add(1)
go func() {
defer wg.Done()
cfg, err := af.Fetch(ctx)
if err == nil {
appr, err := json.Marshal(cfg)
if err == nil {
appearance, err := json.Marshal(cfg)
if err == nil {
state.Appearance = html.EscapeString(string(appearance))
state.ApplicationName = applicationNameOrDefault(cfg)
state.LogoURL = cfg.LogoURL
}
state.Appearance = html.EscapeString(string(appr))
state.ApplicationName = applicationNameOrDefault(cfg)
state.LogoURL = cfg.LogoURL
}
}()
}
}
}()
if h.RegionsFetcher != nil {
wg.Add(1)
go func() {