From 6da1509e2df7c4e3125c01514550b8a8eebc81c8 Mon Sep 17 00:00:00 2001 From: Bram Gruneir Date: Mon, 21 Aug 2017 17:43:07 -0400 Subject: [PATCH] server, settings, util/log: add a cluster setting to block vmodule requests This required moving the handling of the vmodule endpoint out of logging and into dubug, but it does seem more at home there. Closes #16508. --- pkg/server/debug.go | 25 +++++++++++++++++++ pkg/settings/cluster/settings.go | 8 ++++++ .../logictest/testdata/logic_test/show_source | 1 + pkg/util/log/log.go | 19 ++++---------- 4 files changed, 39 insertions(+), 14 deletions(-) diff --git a/pkg/server/debug.go b/pkg/server/debug.go index 6d468558d042..4f5aa801f245 100644 --- a/pkg/server/debug.go +++ b/pkg/server/debug.go @@ -20,9 +20,11 @@ import ( "strings" // Register the net/trace endpoint with http.DefaultServeMux. + "golang.org/x/net/context" "golang.org/x/net/trace" "github.com/cockroachdb/cockroach/pkg/settings/cluster" + "github.com/cockroachdb/cockroach/pkg/util/log" "github.com/rcrowley/go-metrics" "github.com/rcrowley/go-metrics/exp" @@ -35,6 +37,9 @@ import ( // for access to exported vars and pprof tools. const debugEndpoint = "/debug/" +// vmoduleEndpoint is used to change logging's vmodule settings. +const vmodulePrefix = debugEndpoint + "vmodule/" + // We use the default http mux for the debug endpoint (as pprof and net/trace // register to that via import, and go-metrics registers to that via exp.Exp()) var debugServeMux = http.DefaultServeMux @@ -83,6 +88,24 @@ func authRequest(r *http.Request) (allow, sensitive bool) { return allow, sensitive } +func handleVModule(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "text/plain; charset=utf-8") + if !ClusterSettings.DebugVModule.Get() { + http.Error( + w, + "not allowed (due to the 'server.remote_debugging.vmodule' setting)", + http.StatusForbidden, + ) + return + } + spec := r.RequestURI[len(vmodulePrefix):] + if err := log.SetVModule(spec); err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + } + log.Infof(context.Background(), "vmodule changed to: %s", spec) + fmt.Fprint(w, "ok: "+spec) +} + func init() { traceAuthRequest = trace.AuthRequest @@ -93,6 +116,8 @@ func init() { // TODO(mberhault): properly secure this once we require client certs. trace.AuthRequest = authRequest + debugServeMux.HandleFunc(vmodulePrefix, handleVModule) + debugServeMux.HandleFunc(debugEndpoint, func(w http.ResponseWriter, r *http.Request) { if r.URL.Path != debugEndpoint { http.Redirect(w, r, debugEndpoint, http.StatusMovedPermanently) diff --git a/pkg/settings/cluster/settings.go b/pkg/settings/cluster/settings.go index 00f55cedaefa..beaecc8ea4c2 100644 --- a/pkg/settings/cluster/settings.go +++ b/pkg/settings/cluster/settings.go @@ -224,6 +224,7 @@ type StorageSettings struct { type UISettings struct { WebSessionTimeout *settings.DurationSetting DebugRemote *settings.StringSetting + DebugVModule *settings.BoolSetting } // CCLSettings is the subset of ClusterSettings affecting @@ -694,6 +695,13 @@ func MakeClusterSettings(minVersion, serverVersion roachpb.Version) *Settings { } }, ) + + s.DebugVModule = r.RegisterBoolSetting( + "server.remote_debugging.vmodule", + "set to enable remote control over vmodule (this can severely impact performance)", + false, + ) + s.ClusterOrganization = r.RegisterStringSetting("cluster.organization", "organization name", "") // FIXME(tschottdorf): should be NonNegative? diff --git a/pkg/sql/logictest/testdata/logic_test/show_source b/pkg/sql/logictest/testdata/logic_test/show_source index 3a47488f150a..c3df87f858f9 100644 --- a/pkg/sql/logictest/testdata/logic_test/show_source +++ b/pkg/sql/logictest/testdata/logic_test/show_source @@ -70,6 +70,7 @@ rocksdb.min_wal_sync_interval 0s d minimum server.declined_reservation_timeout 1s d the amount of time to consider the store throttled for up-replication after a reservation was declined server.failed_reservation_timeout 5s d the amount of time to consider the store throttled for up-replication after a failed reservation call server.remote_debugging.mode local s set to enable remote debugging, localhost-only or disable (any, local, off) +server.remote_debugging.vmodule false b set to enable remote control over vmodule (this can severely impact performance) server.time_until_store_dead 5m0s d the time after which if there is no new gossiped information about a store, it is considered dead server.web_session_timeout 168h0m0s d the duration that a newly created web session will be valid sql.defaults.distsql 0 e Default distributed SQL execution mode [off = 0, auto = 1, on = 2] diff --git a/pkg/util/log/log.go b/pkg/util/log/log.go index 0bd10c13fb7c..6bd5e723b9b1 100644 --- a/pkg/util/log/log.go +++ b/pkg/util/log/log.go @@ -17,26 +17,12 @@ package log import ( "fmt" "io" - "net/http" "strings" "golang.org/x/net/context" ) -const httpLogLevelPrefix = "/debug/vmodule/" - -func handleVModule(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", "text/plain; charset=utf-8") - spec := r.RequestURI[len(httpLogLevelPrefix):] - if err := logging.vmodule.Set(spec); err != nil { - http.Error(w, err.Error(), http.StatusInternalServerError) - } - Infof(context.Background(), "vmodule changed to: %s", spec) - fmt.Fprint(w, "ok: "+spec) -} - func init() { - http.Handle(httpLogLevelPrefix, http.HandlerFunc(handleVModule)) copyStandardLogTo("INFO") } @@ -192,3 +178,8 @@ func (e Entry) Format(w io.Writer) error { _, err := w.Write(buf.Bytes()) return err } + +// SetVModule alters the vmodule logging level to the passed in value. +func SetVModule(value string) error { + return logging.vmodule.Set(value) +}