Skip to content

Commit bb339e1

Browse files
committed
server: enable TestServerControllerLoginLogout for secondary tenants
Fixes: #110002 Epic: CRDB-38970 Release note: none
1 parent 2b29b9c commit bb339e1

File tree

1 file changed

+30
-13
lines changed

1 file changed

+30
-13
lines changed

pkg/ccl/serverccl/server_controller_test.go

Lines changed: 30 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -563,16 +563,18 @@ func TestServerControllerLoginLogout(t *testing.T) {
563563

564564
ctx := context.Background()
565565

566-
srv := serverutils.StartServerOnly(t, base.TestServerArgs{
567-
DefaultTestTenant: base.TestIsForStuffThatShouldWorkWithSecondaryTenantsButDoesntYet(110002),
568-
})
566+
srv := serverutils.StartServerOnly(t, base.TestServerArgs{})
569567
defer srv.Stopper().Stop(ctx)
570568
s := srv.ApplicationLayer()
569+
isExternal := srv.DeploymentMode().IsExternal()
571570

572-
client, err := s.GetAuthenticatedHTTPClient(false, serverutils.SingleTenantSession)
571+
sessionType := serverutils.SingleTenantSession
572+
client, err := s.GetAuthenticatedHTTPClient(false, sessionType)
573573
require.NoError(t, err)
574574

575-
resp, err := client.Post(s.AdminURL().WithPath("/logout").String(), "", nil)
575+
// Using `Get` here instead of `Post` because in external process mode, the
576+
// server returns a `StatusMethodNotAllowed` error when using `Post`.
577+
resp, err := client.Get(s.AdminURL().WithPath("/logout").String())
576578
require.NoError(t, err)
577579
defer resp.Body.Close()
578580

@@ -582,18 +584,33 @@ func TestServerControllerLoginLogout(t *testing.T) {
582584
for i, c := range resp.Cookies() {
583585
cookieNames[i] = c.Name
584586
cookieValues[i] = c.Value
585-
require.True(t, c.Secure)
587+
// Secure isn't set in external-mode but it is set in other modes.
588+
// See https://github.com/cockroachdb/cockroach/pull/143354#pullrequestreview-2751413632.
589+
if !isExternal {
590+
require.True(t, c.Secure)
591+
}
586592
if c.Name == "session" {
587593
require.True(t, c.HttpOnly)
588594
}
589595
}
590-
require.ElementsMatch(t, []string{"session", "tenant"}, cookieNames)
591-
require.ElementsMatch(t, []string{"", ""}, cookieValues)
592-
596+
expectedCookies := []string{"session"}
597+
expectedValues := []string{""}
598+
if !isExternal {
599+
expectedCookies = append(expectedCookies, "tenant")
600+
expectedValues = append(expectedValues, "")
601+
}
602+
require.ElementsMatch(t, expectedCookies, cookieNames)
603+
require.ElementsMatch(t, expectedValues, cookieValues)
604+
605+
// This part of the test doesn't run in external-process mode since we can't
606+
// set a session cookie with name `tenant`—that's only supported by HTTP
607+
// servers in the system tenant, which also handle routing for secondary
608+
// tenants.
609+
if isExternal {
610+
return
611+
}
593612
// Need a new server because the HTTP Client is memoized.
594-
srv2 := serverutils.StartServerOnly(t, base.TestServerArgs{
595-
DefaultTestTenant: base.TestIsForStuffThatShouldWorkWithSecondaryTenantsButDoesntYet(110002),
596-
})
613+
srv2 := serverutils.StartServerOnly(t, base.TestServerArgs{})
597614
defer srv2.Stopper().Stop(ctx)
598615
s2 := srv2.ApplicationLayer()
599616

@@ -623,7 +640,7 @@ func TestServerControllerLoginLogout(t *testing.T) {
623640
require.NoError(t, err)
624641
cookieJar.SetCookies(s2.AdminURL().URL, []*http.Cookie{
625642
{
626-
Name: "multitenant-session",
643+
Name: "tenant",
627644
Value: "abc-123",
628645
},
629646
})

0 commit comments

Comments
 (0)