From 857ba8b39ad604074696a61e6c8a302126f15f9a Mon Sep 17 00:00:00 2001
From: Nikhil Benesch <nikhil.benesch@gmail.com>
Date: Thu, 27 Sep 2018 14:55:46 -0400
Subject: [PATCH] roachtest: teach cluster-init to reproduce startup crash

Teach the cluster-init test to reproduce the crash during bootup
described in issue #25771. The trick is to send requests with a session
cookie, as the admin UI would, while the cluster is waiting for init.

Note that the crash was caused by the authenticationMux, which is used
to protect secure clusters, but this test uses an insecure cluster. This
is relying on a wart: an authenticationMux is installed for UI assets
whether or not the cluster is secure. This could change in the future. A
better test would use a secure cluster, however roachprod/roachtest do
not easily support that at this time, nor is it easy to write a unit
test that exercises the crash more robustly.

Release note: None
---
 pkg/cmd/roachtest/cluster_init.go | 41 ++++++++++++++++++++++++-------
 pkg/server/authentication.go      |  5 ++--
 pkg/server/authentication_test.go |  2 +-
 pkg/server/testserver.go          |  2 +-
 4 files changed, 37 insertions(+), 13 deletions(-)

diff --git a/pkg/cmd/roachtest/cluster_init.go b/pkg/cmd/roachtest/cluster_init.go
index 71cdf74f52e3..08b8d7be6fcb 100644
--- a/pkg/cmd/roachtest/cluster_init.go
+++ b/pkg/cmd/roachtest/cluster_init.go
@@ -23,6 +23,8 @@ import (
 	"strings"
 	"time"
 
+	"github.com/cockroachdb/cockroach/pkg/server"
+	"github.com/cockroachdb/cockroach/pkg/server/serverpb"
 	"github.com/cockroachdb/cockroach/pkg/util/retry"
 	"golang.org/x/sync/errgroup"
 )
@@ -143,16 +145,37 @@ func runClusterInit(ctx context.Context, t *test, c *cluster) {
 				{"/_status/nodes", http.StatusNotFound},
 			}
 			for _, tc := range httpTests {
-				resp, err := http.Get(urlMap[1] + tc.endpoint)
-				if err != nil {
-					t.Fatalf("unexpected error hitting %s endpoint: %v", tc.endpoint, err)
-				}
-				defer resp.Body.Close()
-				if resp.StatusCode != tc.expectedStatus {
-					bodyBytes, _ := ioutil.ReadAll(resp.Body)
-					t.Fatalf("unexpected response code %d (expected %d) hitting %s endpoint: %v",
-						resp.StatusCode, tc.expectedStatus, tc.endpoint, string(bodyBytes))
+				for _, withCookie := range []bool{false, true} {
+					req, err := http.NewRequest("GET", urlMap[1]+tc.endpoint, nil /* body */)
+					if err != nil {
+						t.Fatalf("unexpected error while constructing request for %s: %s", tc.endpoint, err)
+					}
+					if withCookie {
+						// Prevent regression of #25771 by also sending authenticated
+						// requests, like would be sent if an admin UI were open against
+						// this node while it booted.
+						cookie, err := server.EncodeSessionCookie(&serverpb.SessionCookie{
+							// The actual contents of the cookie don't matter; the presence of
+							// a valid encoded cookie is enough to trigger the authentication
+							// code paths.
+						})
+						if err != nil {
+							t.Fatal(err)
+						}
+						req.AddCookie(cookie)
+					}
+					resp, err := http.DefaultClient.Do(req)
+					if err != nil {
+						t.Fatalf("unexpected error hitting %s endpoint: %v", tc.endpoint, err)
+					}
+					defer resp.Body.Close()
+					if resp.StatusCode != tc.expectedStatus {
+						bodyBytes, _ := ioutil.ReadAll(resp.Body)
+						t.Fatalf("unexpected response code %d (expected %d) hitting %s endpoint: %v",
+							resp.StatusCode, tc.expectedStatus, tc.endpoint, string(bodyBytes))
+					}
 				}
+
 			}
 
 			c.Run(ctx, c.Node(initNode),
diff --git a/pkg/server/authentication.go b/pkg/server/authentication.go
index 067e5f6fe31b..982f2186805e 100644
--- a/pkg/server/authentication.go
+++ b/pkg/server/authentication.go
@@ -139,7 +139,7 @@ func (s *authenticationServer) UserLogin(
 		ID:     id,
 		Secret: secret,
 	}
-	cookie, err := encodeSessionCookie(cookieValue)
+	cookie, err := EncodeSessionCookie(cookieValue)
 	if err != nil {
 		return nil, apiInternalError(ctx, err)
 	}
@@ -369,7 +369,8 @@ func (am *authenticationMux) ServeHTTP(w http.ResponseWriter, req *http.Request)
 	am.inner.ServeHTTP(w, req)
 }
 
-func encodeSessionCookie(sessionCookie *serverpb.SessionCookie) (*http.Cookie, error) {
+// EncodeSessionCookie encodes a SessionCookie proto into an http.Cookie.
+func EncodeSessionCookie(sessionCookie *serverpb.SessionCookie) (*http.Cookie, error) {
 	cookieValueBytes, err := protoutil.Marshal(sessionCookie)
 	if err != nil {
 		return nil, errors.Wrap(err, "session cookie could not be encoded")
diff --git a/pkg/server/authentication_test.go b/pkg/server/authentication_test.go
index 5cb326fcd06b..0ce4bfbc8b04 100644
--- a/pkg/server/authentication_test.go
+++ b/pkg/server/authentication_test.go
@@ -551,7 +551,7 @@ func TestLogout(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	encodedCookie, err := encodeSessionCookie(cookie)
+	encodedCookie, err := EncodeSessionCookie(cookie)
 	if err != nil {
 		t.Fatal(err)
 	}
diff --git a/pkg/server/testserver.go b/pkg/server/testserver.go
index e79b65e4f7ca..7c1a673ca27b 100644
--- a/pkg/server/testserver.go
+++ b/pkg/server/testserver.go
@@ -563,7 +563,7 @@ func (ts *TestServer) getAuthenticatedHTTPClientAndCookie() (
 				Secret: secret,
 			}
 			// Encode a session cookie and store it in a cookie jar.
-			cookie, err := encodeSessionCookie(rawCookie)
+			cookie, err := EncodeSessionCookie(rawCookie)
 			if err != nil {
 				return err
 			}