Skip to content

chore: refactor Appearance to an interface callable by AGPL code #11769

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 39 additions & 0 deletions coderd/appearance/appearance.go
Original file line number Diff line number Diff line change
@@ -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{}
37 changes: 19 additions & 18 deletions coderd/coderd.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
Expand All @@ -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"
Expand Down Expand Up @@ -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()

Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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]
Expand Down
53 changes: 20 additions & 33 deletions enterprise/coderd/appearance.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.",
Expand All @@ -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)
}
Expand All @@ -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
Expand Down
18 changes: 8 additions & 10 deletions enterprise/coderd/appearance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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) {
Expand All @@ -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)
}
15 changes: 14 additions & 1 deletion enterprise/coderd/coderd.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import (
"sync"
"time"

"github.com/coder/coder/v2/coderd/appearance"

"golang.org/x/xerrors"
"tailscale.com/tailcfg"

Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down
Loading