Skip to content

Commit

Permalink
ui: serve files from embedded ./assets/ directory
Browse files Browse the repository at this point in the history
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 fcd385c (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
  • Loading branch information
sjbarag authored and celiala committed Apr 8, 2022
1 parent eafda96 commit fb9b2ef
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 9 deletions.
11 changes: 4 additions & 7 deletions pkg/server/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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) {
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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)
Expand All @@ -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)

Expand Down
10 changes: 10 additions & 0 deletions pkg/server/testdata/assets/bundle.js
Original file line number Diff line number Diff line change
@@ -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.");
17 changes: 17 additions & 0 deletions pkg/server/testdata/embedded_assets.go
Original file line number Diff line number Diff line change
@@ -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
9 changes: 7 additions & 2 deletions pkg/ui/ui.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"embed"
"fmt"
"html/template"
"io/fs"
"net/http"

"github.com/cockroachdb/cockroach/pkg/base"
Expand Down Expand Up @@ -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)
}
Expand All @@ -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()
Expand Down

0 comments on commit fb9b2ef

Please sign in to comment.