chore: refactor site handler to take cache dir (#21918)

relates to: https://github.com/coder/internal/issues/1300

Refactors the options to the site handler to take the cache directory, rather than expecting the caller to call `ExtractOrReadBinFS` and pass the results.

This is important in this stack because we need direct access to the cache directory for compressed file caching.

coderd/coderd.go

@@ -462,10 +462,6 @@ func New(options *Options) *API {
 	if siteCacheDir != "" {
 		siteCacheDir = filepath.Join(siteCacheDir, "site")
 	}
-	binFS, binHashes, err := site.ExtractOrReadBinFS(siteCacheDir, site.FS())
-	if err != nil {
-		panic(xerrors.Errorf("read site bin failed: %w", err))
-	}
 
 	metricsCache := metricscache.New(
 		options.Database,
@@ -658,9 +654,8 @@ func New(options *Options) *API {
 		WebPushPublicKey:      api.WebpushDispatcher.PublicKey(),
 		Telemetry:             api.Telemetry.Enabled(),
 	}
-	api.SiteHandler = site.New(&site.Options{
-		BinFS:             binFS,
-		BinHashes:         binHashes,
+	api.SiteHandler, err = site.New(&site.Options{
+		CacheDir:          siteCacheDir,
 		Database:          options.Database,
 		SiteFS:            site.FS(),
 		OAuth2Configs:     oauthConfigs,
@@ -672,6 +667,9 @@ func New(options *Options) *API {
 		Logger:            options.Logger.Named("site"),
 		HideAITasks:       options.DeploymentValues.HideAITasks.Value(),
 	})
+	if err != nil {
+		options.Logger.Fatal(ctx, "failed to initialize site handler", slog.Error(err))
+	}
 	api.SiteHandler.Experiments.Store(&experiments)
 
 	if options.UpdateCheckOptions != nil {

site/bin.go

@@ -89,11 +89,15 @@ func (h *binHandler) ServeHTTP(rw http.ResponseWriter, r *http.Request) {
 	http.FileServer(h.binFS).ServeHTTP(rw, r)
 }
 
-func newBinHandler(options *Options) *binHandler {
-	return &binHandler{
-		binFS:         options.BinFS,
-		metadataCache: newBinMetadataCache(options.BinFS, options.BinHashes),
+func newBinHandler(options *Options) (*binHandler, error) {
+	binFS, binHashes, err := ExtractOrReadBinFS(options.CacheDir, options.SiteFS)
+	if err != nil {
+		return nil, xerrors.Errorf("extract or read bin filesystem: %w", err)
 	}
+	return &binHandler{
+		binFS:         binFS,
+		metadataCache: newBinMetadataCache(binFS, binHashes),
+	}, nil
 }
 
 // ExtractOrReadBinFS checks the provided fs for compressed coder binaries and

site/site.go

@@ -68,8 +68,7 @@ func init() {
 }
 
 type Options struct {
-	BinFS             http.FileSystem
-	BinHashes         map[string]string
+	CacheDir          string
 	Database          database.Store
 	SiteFS            fs.FS
 	OAuth2Configs     *httpmw.OAuth2Configs
@@ -82,7 +81,7 @@ type Options struct {
 	HideAITasks       bool
 }
 
-func New(opts *Options) *Handler {
+func New(opts *Options) (*Handler, error) {
 	if opts.AppearanceFetcher == nil {
 		daf := atomic.Pointer[appearance.Fetcher]{}
 		f := appearance.NewDefaultFetcher(opts.DocsURL)
@@ -100,11 +99,16 @@ func New(opts *Options) *Handler {
 	var err error
 	handler.htmlTemplates, err = findAndParseHTMLFiles(opts.SiteFS)
 	if err != nil {
-		panic(fmt.Sprintf("Failed to parse html files: %v", err))
+		return nil, xerrors.Errorf("failed to parse html files: %w", err)
+	}
+
+	binHand, err := newBinHandler(opts)
+	if err != nil {
+		return nil, xerrors.Errorf("create bin handler: %w", err)
 	}
 
 	mux := http.NewServeMux()
-	mux.Handle("/bin/", newBinHandler(opts))
+	mux.Handle("/bin/", binHand)
 	mux.Handle("/", http.FileServer(
 		http.FS(
 			// OnlyFiles is a wrapper around the file system that prevents directory
@@ -116,7 +120,7 @@ func New(opts *Options) *Handler {
 	)
 	buildInfoResponse, err := json.Marshal(opts.BuildInfo)
 	if err != nil {
-		panic("failed to marshal build info: " + err.Error())
+		return nil, xerrors.Errorf("failed to marshal build info: %w", err)
 	}
 	handler.buildInfoJSON = html.EscapeString(string(buildInfoResponse))
 	handler.handler = mux.ServeHTTP
@@ -126,7 +130,7 @@ func New(opts *Options) *Handler {
 		opts.Logger.Warn(context.Background(), "could not parse install.sh, it will be unavailable", slog.Error(err))
 	}
 
-	return handler
+	return handler, nil
 }
 
 type Handler struct {

site/site_test.go

@@ -23,6 +23,7 @@ import (
 	"github.com/google/uuid"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"golang.org/x/exp/maps"
 
 	"github.com/coder/coder/v2/coderd/database"
 	"github.com/coder/coder/v2/coderd/database/db2sdk"
@@ -44,14 +45,13 @@ func TestInjection(t *testing.T) {
 			Data: []byte("{{ .User }}"),
 		},
 	}
-	binFs := http.FS(fstest.MapFS{})
 	db, _ := dbtestutil.NewDB(t)
-	handler := site.New(&site.Options{
+	handler, err := site.New(&site.Options{
 		Telemetry: telemetry.NewNoop(),
-		BinFS:     binFs,
 		Database:  db,
 		SiteFS:    siteFS,
 	})
+	require.NoError(t, err)
 
 	user := dbgen.User(t, db, database.User{})
 	_, token := dbgen.APIKey(t, db, database.APIKey{
@@ -66,7 +66,7 @@ func TestInjection(t *testing.T) {
 	handler.ServeHTTP(rw, r)
 	require.Equal(t, http.StatusOK, rw.Code)
 	var got codersdk.User
-	err := json.Unmarshal([]byte(html.UnescapeString(rw.Body.String())), &got)
+	err = json.Unmarshal([]byte(html.UnescapeString(rw.Body.String())), &got)
 	require.NoError(t, err)
 
 	// This will update as part of the request!
@@ -101,15 +101,13 @@ func TestInjectionFailureProducesCleanHTML(t *testing.T) {
 		OAuthExpiry:       dbtime.Now().Add(-time.Second),
 	})
 
-	binFs := http.FS(fstest.MapFS{})
 	siteFS := fstest.MapFS{
 		"index.html": &fstest.MapFile{
 			Data: []byte("<html>{{ .User }}</html>"),
 		},
 	}
-	handler := site.New(&site.Options{
+	handler, err := site.New(&site.Options{
 		Telemetry: telemetry.NewNoop(),
-		BinFS:     binFs,
 		Database:  db,
 		SiteFS:    siteFS,
 
@@ -119,6 +117,7 @@ func TestInjectionFailureProducesCleanHTML(t *testing.T) {
 			OIDC:   nil,
 		},
 	})
+	require.NoError(t, err)
 
 	r := httptest.NewRequest("GET", "/", nil)
 	r.Header.Set(codersdk.SessionTokenHeader, token)
@@ -153,15 +152,15 @@ func TestCaching(t *testing.T) {
 			Data: []byte("folderFile"),
 		},
 	}
-	binFS := http.FS(fstest.MapFS{})
 
 	db, _ := dbtestutil.NewDB(t)
-	srv := httptest.NewServer(site.New(&site.Options{
+	s, err := site.New(&site.Options{
 		Telemetry: telemetry.NewNoop(),
-		BinFS:     binFS,
 		SiteFS:    rootFS,
 		Database:  db,
-	}))
+	})
+	require.NoError(t, err)
+	srv := httptest.NewServer(s)
 	defer srv.Close()
 
 	// Create a context
@@ -222,15 +221,15 @@ func TestServingFiles(t *testing.T) {
 			Data: []byte("install-sh-bytes"),
 		},
 	}
-	binFS := http.FS(fstest.MapFS{})
 
 	db, _ := dbtestutil.NewDB(t)
-	srv := httptest.NewServer(site.New(&site.Options{
+	handler, err := site.New(&site.Options{
 		Telemetry: telemetry.NewNoop(),
-		BinFS:     binFS,
 		SiteFS:    rootFS,
 		Database:  db,
-	}))
+	})
+	require.NoError(t, err)
+	srv := httptest.NewServer(handler)
 	defer srv.Close()
 	client := &http.Client{}
 
@@ -506,21 +505,20 @@ func TestServingBin(t *testing.T) {
 			t.Parallel()
 
 			dest := t.TempDir()
-			binFS, binHashes, err := site.ExtractOrReadBinFS(dest, tt.fs)
+			testFS := maps.Clone(rootFS)
+			maps.Copy(testFS, tt.fs)
+			handler, err := site.New(&site.Options{
+				Telemetry: telemetry.NewNoop(),
+				SiteFS:    testFS,
+				CacheDir:  dest,
+			})
 			if !tt.wantErr && err != nil {
 				require.NoError(t, err, "extract or read failed")
 			} else if tt.wantErr {
 				require.Error(t, err, "extraction or read did not fail")
 			}
-
-			site := site.New(&site.Options{
-				Telemetry: telemetry.NewNoop(),
-				BinFS:     binFS,
-				BinHashes: binHashes,
-				SiteFS:    rootFS,
-			})
 			compressor := middleware.NewCompressor(1, "text/*", "application/*")
-			srv := httptest.NewServer(compressor.Handler(site))
+			srv := httptest.NewServer(compressor.Handler(handler))
 			defer srv.Close()
 			client := &http.Client{}