Merge #78936

78936: Revert #78460 r=erikgrinaker a=tbg This reverts commit c178f3d. This commit caused `./dev build cockroach` to produce a binary that would panic upon invocation: > panic: file does not exist in `pkg/ui/distccl/distccl.go:40`. Reverting while we fix that. Investigated [here]. [here]: https://cockroachlabs.slack.com/archives/C023S0V4YEB/p1648536535710149 Release note: None Co-authored-by: Tobias Grieger <[email protected]>
cockroachdb · Mar 29, 2022 · 327f886 · 327f886
2 parents a662489 + 39a600e
commit 327f886
Show file tree

Hide file tree

Showing 16 changed files with 4 additions and 504 deletions.
diff --git a/pkg/BUILD.bazel b/pkg/BUILD.bazel
@@ -408,7 +408,6 @@ ALL_TESTS = [
     "//pkg/util/goschedstats:goschedstats_test",
     "//pkg/util/grpcutil:grpcutil_test",
     "//pkg/util/hlc:hlc_test",
-    "//pkg/util/httputil:httputil_test",
     "//pkg/util/humanizeutil:humanizeutil_test",
     "//pkg/util/interval/generic:generic_test",
     "//pkg/util/interval:interval_test",

diff --git a/pkg/server/server_test.go b/pkg/server/server_test.go
@@ -26,7 +26,6 @@ import (
 	"regexp"
 	"strings"
 	"testing"
-	"testing/fstest"
 	"time"
 
 	"github.com/cockroachdb/cockroach/pkg/base"
@@ -957,88 +956,6 @@ Binary built without web UI.
 			})
 		}
 	})
-
-	t.Run("Client-side caching", func(t *testing.T) {
-		linkInFakeUI()
-		defer unlinkFakeUI()
-
-		// Set up fake asset FS with hashes
-		mapfs := fstest.MapFS{
-			"bundle.js": &fstest.MapFile{
-				Data: []byte("console.log('hello world');"),
-			},
-		}
-		fsys, err := mapfs.Sub(".")
-		require.NoError(t, err)
-		ui.Assets = fsys
-		ui.AssetHashes = map[string]string{
-			"/bundle.js": "ad43b0d7fb055db16583c156c5507ed58c157e9d",
-		}
-
-		// Clear fake asset FS and hashes when we're done
-		defer func() {
-			ui.Assets = nil
-			ui.AssetHashes = nil
-		}()
-
-		s, _, _ := serverutils.StartServer(t, base.TestServerArgs{})
-		defer s.Stopper().Stop(ctx)
-		tsrv := s.(*TestServer)
-
-		loggedInClient, err := tsrv.GetAdminHTTPClient()
-		require.NoError(t, err)
-		loggedOutClient, err := tsrv.GetUnauthenticatedHTTPClient()
-		require.NoError(t, err)
-
-		cases := []struct {
-			desc   string
-			client http.Client
-		}{
-			{
-				desc:   "unauthenticated user",
-				client: loggedOutClient,
-			},
-			{
-				desc:   "authenticated user",
-				client: loggedInClient,
-			},
-		}
-
-		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()+"/bundle.js", nil)
-				require.NoError(t, err)
-
-				uncachedResp, err := testCase.client.Do(uncachedReq)
-				require.NoError(t, err)
-				defer uncachedResp.Body.Close()
-				require.Equal(t, 200, uncachedResp.StatusCode)
-
-				etag := uncachedResp.Header.Get("ETag")
-				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()+"/bundle.js", nil)
-				require.NoError(t, err)
-				cachedReq.Header.Add("If-None-Match", etag)
-
-				cachedResp, err := testCase.client.Do(cachedReq)
-				require.NoError(t, err)
-				defer cachedResp.Body.Close()
-				require.Equal(t, 304, cachedResp.StatusCode)
-
-				respBytes, err := ioutil.ReadAll(cachedResp.Body)
-				require.NoError(t, err)
-				require.Empty(t, respBytes, "Server must provide empty body for cached response")
-
-				etagFromEmptyResp := cachedResp.Header.Get("ETag")
-				require.NotEmpty(t, etag, "Server must provide ETag response header with asset responses")
-
-				require.Equal(t, etag, etagFromEmptyResp, "Server must provide consistent ETag response headers")
-			})
-		}
-	})
 }
 
 func TestGWRuntimeMarshalProto(t *testing.T) {

diff --git a/pkg/ui/BUILD.bazel b/pkg/ui/BUILD.bazel
@@ -19,7 +19,6 @@ go_library(
     deps = [
         "//pkg/base",
         "//pkg/build",
-        "//pkg/util/httputil",
         "//pkg/util/log",
         "@com_github_cockroachdb_errors//:errors",
     ],

diff --git a/pkg/ui/buildutil/BUILD.bazel b/pkg/ui/buildutil/BUILD.bazel
diff --git a/pkg/ui/buildutil/hash_files.go b/pkg/ui/buildutil/hash_files.go
diff --git a/pkg/ui/buildutil/hash_files_test.go b/pkg/ui/buildutil/hash_files_test.go
diff --git a/pkg/ui/distccl/BUILD.bazel b/pkg/ui/distccl/BUILD.bazel
@@ -26,7 +26,6 @@ go_library(
     visibility = ["//visibility:public"],
     deps = [
         "//pkg/ui",
-        "//pkg/ui/buildutil",
         "//pkg/util/targz",
     ],
 )
diff --git a/pkg/ui/distccl/distccl.go b/pkg/ui/distccl/distccl.go
@@ -19,7 +19,6 @@ import (
 	_ "embed"
 
 	"github.com/cockroachdb/cockroach/pkg/ui"
-	"github.com/cockroachdb/cockroach/pkg/ui/buildutil"
 	"github.com/cockroachdb/cockroach/pkg/util/targz"
 )
 
@@ -33,11 +32,4 @@ func init() {
 	}
 	ui.Assets = fs
 	ui.HaveUI = true
-
-	assetHashes := make(map[string]string)
-	err = buildutil.HashFilesInDir(&assetHashes, ui.Assets)
-	if err != nil {
-		panic(err)
-	}
-	ui.AssetHashes = assetHashes
 }
diff --git a/pkg/ui/distccl/distccl_no_bazel.go b/pkg/ui/distccl/distccl_no_bazel.go
@@ -19,7 +19,6 @@ import (
 	"io/fs"
 
 	"github.com/cockroachdb/cockroach/pkg/ui"
-	"github.com/cockroachdb/cockroach/pkg/ui/buildutil"
 )
 
 //go:embed assets/*
@@ -32,11 +31,4 @@ func init() {
 		panic(err)
 	}
 	ui.HaveUI = true
-
-	assetHashes := make(map[string]string)
-	err = buildutil.HashFilesInDir(&assetHashes, ui.Assets)
-	if err != nil {
-		panic(err)
-	}
-	ui.AssetHashes = assetHashes
 }
diff --git a/pkg/ui/distoss/BUILD.bazel b/pkg/ui/distoss/BUILD.bazel
@@ -26,7 +26,6 @@ go_library(
     visibility = ["//visibility:public"],
     deps = [
         "//pkg/ui",
-        "//pkg/ui/buildutil",
         "//pkg/util/targz",
     ],
 )
diff --git a/pkg/ui/distoss/distoss.go b/pkg/ui/distoss/distoss.go
@@ -21,7 +21,6 @@ import (
 	_ "embed"
 
 	"github.com/cockroachdb/cockroach/pkg/ui"
-	"github.com/cockroachdb/cockroach/pkg/ui/buildutil"
 	"github.com/cockroachdb/cockroach/pkg/util/targz"
 )
 
@@ -35,11 +34,4 @@ func init() {
 	}
 	ui.Assets = fs
 	ui.HaveUI = true
-
-	assetHashes := make(map[string]string)
-	err = buildutil.HashFilesInDir(&assetHashes, ui.Assets)
-	if err != nil {
-		panic(err)
-	}
-	ui.AssetHashes = assetHashes
 }
diff --git a/pkg/ui/distoss/distoss_no_bazel.go b/pkg/ui/distoss/distoss_no_bazel.go
@@ -21,24 +21,16 @@ import (
 	"io/fs"
 
 	"github.com/cockroachdb/cockroach/pkg/ui"
-	"github.com/cockroachdb/cockroach/pkg/ui/buildutil"
 )
 
 //go:embed assets/*
 var assets embed.FS
 
 func init() {
 	var err error
-	ui.HaveUI = true
 	ui.Assets, err = fs.Sub(assets, "assets")
 	if err != nil {
 		panic(err)
 	}
-
-	assetHashes := make(map[string]string)
-	err = buildutil.HashFilesInDir(&assetHashes, ui.Assets)
-	if err != nil {
-		panic(err)
-	}
-	ui.AssetHashes = assetHashes
+	ui.HaveUI = true
 }