Skip to content

Commit

Permalink
Merge #120707
Browse files Browse the repository at this point in the history
120707: server: http requests always fallback to system tenant r=abarganier a=dhartunian

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

Co-authored-by: David Hartunian <[email protected]>
  • Loading branch information
craig[bot] and dhartunian committed Mar 20, 2024
2 parents 0e6deab + 93ebf15 commit 5c26990
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 5c26990

Please sign in to comment.