From fb9b2efd09db57c200fc10b4d6039e2e47105510 Mon Sep 17 00:00:00 2001 From: Sean Barag Date: Fri, 8 Apr 2022 08:51:13 -0700 Subject: [PATCH] ui: serve files from embedded ./assets/ directory Static assets that are embedded within CRDB were traditionally served from the ./assets/ directory (relative to the root of the embed.FS instance built at compile time). Commit fcd385c204 (ui: serve ETag header and respect If-None-Match req. header for assets, 2022-03-24) served files directly from the root of that embed.FS instance, which caused browser requests for `/bundle.js` to fail with a 404 response code (the correct path would have been `/assets/bundle.js` in that commit). Serve static assets from ./assets/ again, so that browsers can load `/bundle.js` and the admin UI can be successfully loaded. fixes #79657 Release note: None --- pkg/server/server_test.go | 11 ++++------- pkg/server/testdata/assets/bundle.js | 10 ++++++++++ pkg/server/testdata/embedded_assets.go | 17 +++++++++++++++++ pkg/ui/ui.go | 9 +++++++-- 4 files changed, 38 insertions(+), 9 deletions(-) create mode 100644 pkg/server/testdata/assets/bundle.js create mode 100644 pkg/server/testdata/embedded_assets.go diff --git a/pkg/server/server_test.go b/pkg/server/server_test.go index f62a89b0cdfd..d6597561f76a 100644 --- a/pkg/server/server_test.go +++ b/pkg/server/server_test.go @@ -42,6 +42,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/security" "github.com/cockroachdb/cockroach/pkg/server/serverpb" "github.com/cockroachdb/cockroach/pkg/server/status/statuspb" + "github.com/cockroachdb/cockroach/pkg/server/testdata" "github.com/cockroachdb/cockroach/pkg/sql" "github.com/cockroachdb/cockroach/pkg/sql/catalog/catalogkeys" "github.com/cockroachdb/cockroach/pkg/sql/catalog/dbdesc" @@ -68,10 +69,6 @@ import ( "google.golang.org/grpc/status" ) -// embedFsys is a small instance of embed.FS used for testing purposes only -//go:embed doc.go -var embedFsys embed.FS - // TestSelfBootstrap verifies operation when no bootstrap hosts have // been specified. func TestSelfBootstrap(t *testing.T) { @@ -1163,7 +1160,7 @@ Binary built without web UI. linkInFakeUI() defer unlinkFakeUI() - ui.Assets = embedFsys + ui.Assets = testdata.TestAssets // Clear fake asset FS when we're done defer func() { @@ -1196,7 +1193,7 @@ Binary built without web UI. for _, testCase := range cases { t.Run(fmt.Sprintf("bundle caching for %s", testCase.desc), func(t *testing.T) { // Request bundle.js without an If-None-Match header first, to simulate the initial load - uncachedReq, err := http.NewRequestWithContext(ctx, "GET", s.AdminURL()+"/doc.go", nil) + uncachedReq, err := http.NewRequestWithContext(ctx, "GET", s.AdminURL()+"/bundle.js", nil) require.NoError(t, err) uncachedResp, err := testCase.client.Do(uncachedReq) @@ -1208,7 +1205,7 @@ Binary built without web UI. require.NotEmpty(t, etag, "Server must provide ETag response header with asset responses") // Use that ETag header on the next request to simulate a client reload - cachedReq, err := http.NewRequestWithContext(ctx, "GET", s.AdminURL()+"/doc.go", nil) + cachedReq, err := http.NewRequestWithContext(ctx, "GET", s.AdminURL()+"/bundle.js", nil) require.NoError(t, err) cachedReq.Header.Add("If-None-Match", etag) diff --git a/pkg/server/testdata/assets/bundle.js b/pkg/server/testdata/assets/bundle.js new file mode 100644 index 000000000000..af19f0e845a0 --- /dev/null +++ b/pkg/server/testdata/assets/bundle.js @@ -0,0 +1,10 @@ +// Copyright 2022 The Cockroach Authors. +// +// Use of this software is governed by the Business Source License +// included in the file licenses/BSL.txt. +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0, included in the file +// licenses/APL.txt. +console.log("This is a test-only bundle.js, meant for embedding in server_test.go."); diff --git a/pkg/server/testdata/embedded_assets.go b/pkg/server/testdata/embedded_assets.go new file mode 100644 index 000000000000..b3a8f163ef69 --- /dev/null +++ b/pkg/server/testdata/embedded_assets.go @@ -0,0 +1,17 @@ +// Copyright 2022 The Cockroach Authors. +// +// Use of this software is governed by the Business Source License +// included in the file licenses/BSL.txt. +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0, included in the file +// licenses/APL.txt. + +package testdata + +import "embed" + +// TestAssets is a test-only embed.FS, and should not be used for any other purposes. +//go:embed assets +var TestAssets embed.FS diff --git a/pkg/ui/ui.go b/pkg/ui/ui.go index 9ff1b705bf1d..ebba7f4a6725 100644 --- a/pkg/ui/ui.go +++ b/pkg/ui/ui.go @@ -22,6 +22,7 @@ import ( "embed" "fmt" "html/template" + "io/fs" "net/http" "github.com/cockroachdb/cockroach/pkg/base" @@ -113,13 +114,17 @@ type Config struct { // including index.html, which has some login-related variables // templated into it, as well as static assets. func Handler(cfg Config) http.Handler { + // assetsFsys is an io/fs instance rooted at `./assets`, relative to the Assets embedded in + // the CRDB binary. + assetsFsys, _ := fs.Sub(Assets, "assets") + // etags is used to provide a unique per-file checksum for each served file, // which enables client-side caching using Cache-Control and ETag headers. etags := make(map[string]string) if HaveUI { // Only compute hashes for UI-enabled builds - err := httputil.ComputeEtags(Assets, etags) + err := httputil.ComputeEtags(assetsFsys, etags) if err != nil { log.Errorf(context.Background(), "Unable to compute asset hashes: %+v", err) } @@ -128,7 +133,7 @@ func Handler(cfg Config) http.Handler { fileHandlerChain := httputil.EtagHandler( etags, http.FileServer( - http.FS(Assets), + http.FS(assetsFsys), ), ) buildInfo := build.GetInfo()