Skip to content

Commit

Permalink
server: http requests always fallback to system tenant
Browse files Browse the repository at this point in the history
Previously, when a default tenant was configured via the cluster
setting, if that tenant was unavailable, all HTTP requests would fail
if they did not specify an explicit tenant.

This change allows HTTP requests to always fallback to the system
tenant if the default tenant is not available, which allows for DB
Console to be reached in these scenarios.

Resolves: #120084
Epic: None

Release note: None
  • Loading branch information
dhartunian committed Mar 19, 2024
1 parent 802ce03 commit 93ebf15
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 2 deletions.
26 changes: 26 additions & 0 deletions pkg/ccl/serverccl/server_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -534,6 +534,32 @@ func TestServerStartStop(t *testing.T) {
s.Stopper().Stop(ctx)
}

func TestServerControllerSystemTenantHTTPFallback(t *testing.T) {
defer leaktest.AfterTest(t)()
defer log.Scope(t).Close(t)

ctx := context.Background()

srv, db, _ := serverutils.StartServer(t, base.TestServerArgs{
DefaultTestTenant: base.TestControlsTenantsExplicitly,
})
defer srv.Stopper().Stop(ctx)

_, err := db.Exec("CREATE VIRTUAL CLUSTER 'demo'")
require.NoError(t, err)
_, err = db.Exec("SET CLUSTER SETTING server.controller.default_target_cluster = 'demo'")
require.NoError(t, err)

s := srv.ApplicationLayer()
c, err := s.GetUnauthenticatedHTTPClient()
require.NoError(t, err)

resp, err := c.Get(s.AdminURL().String())
require.NoError(t, err)
defer resp.Body.Close()
require.Equal(t, 200, resp.StatusCode)
}

func TestServerControllerLoginLogout(t *testing.T) {
defer leaktest.AfterTest(t)()
defer log.Scope(t).Close(t)
Expand Down
17 changes: 15 additions & 2 deletions pkg/server/server_controller_http.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/server/authserver"
"github.com/cockroachdb/cockroach/pkg/settings"
"github.com/cockroachdb/cockroach/pkg/settings/cluster"
"github.com/cockroachdb/cockroach/pkg/sql/sem/catconstants"
"github.com/cockroachdb/cockroach/pkg/util/log"
"github.com/cockroachdb/errors"
)
Expand Down Expand Up @@ -107,8 +108,20 @@ func (c *serverController) httpMux(w http.ResponseWriter, r *http.Request) {
defaultTenantName := roachpb.TenantName(multitenant.DefaultTenantSelect.Get(&c.st.SV))
s, _, err = c.getServer(ctx, defaultTenantName)
if err != nil {
log.Warningf(ctx, "unable to find server for default tenant %q: %v", defaultTenantName, err)
w.WriteHeader(http.StatusInternalServerError)
if log.V(1) {
// This could get triggered often if a customer has the default
// tenant set up but not active yet. Every DB Console HTTP
// request will go through this branch in that scenario.
log.Warningf(ctx, "unable to find server for default tenant %q: %v", defaultTenantName, err)
}
sys, _, errSystem := c.getServer(ctx, catconstants.SystemTenantName)
if errSystem != nil {
log.Warningf(ctx, "unable to find server for default tenant %q: %v", defaultTenantName, err)
log.Warningf(ctx, "unable to find server for system tenant: %v", errSystem)
w.WriteHeader(http.StatusInternalServerError)
return
}
sys.getHTTPHandlerFn()(w, r)
return
}
s.getHTTPHandlerFn()(w, r)
Expand Down

0 comments on commit 93ebf15

Please sign in to comment.