From b64ccc00f01202f67e02ce11665e6dcdf01ebe16 Mon Sep 17 00:00:00 2001 From: Naveen Sreeramachandra Date: Fri, 20 Jun 2025 08:38:04 +0000 Subject: [PATCH 1/5] add input validation to support to support gorouter log level changes at runtime --- server.go | 58 ++++++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 49 insertions(+), 9 deletions(-) diff --git a/server.go b/server.go index e1fbd4e..714db9f 100644 --- a/server.go +++ b/server.go @@ -1,6 +1,7 @@ package debugserver import ( + "errors" "flag" "io" "net/http" @@ -42,6 +43,24 @@ func DebugAddress(flags *flag.FlagSet) string { return dbgFlag.Value.String() } +func validateloglevelrequest(w http.ResponseWriter, r *http.Request, level []byte) error { + // Only POST method is allowed for setting log level. + if r.Method != http.MethodPost { + return errors.New("method not allowed, use POST") + } + + // Only http is allowed for setting log level. + if r.TLS != nil { + return errors.New("invalid scheme, https is not allowed") + } + + // Ensure the log level is not empty. + if len(level) == 0 { + return errors.New("log level cannot be empty") + } + return nil +} + func Runner(address string, sink ReconfigurableSinkInterface) ifrit.Runner { return http_server.New(address, Handler(sink)) } @@ -64,21 +83,42 @@ func Handler(sink ReconfigurableSinkInterface) http.Handler { mux.Handle("/debug/pprof/profile", http.HandlerFunc(pprof.Profile)) mux.Handle("/debug/pprof/symbol", http.HandlerFunc(pprof.Symbol)) mux.Handle("/log-level", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // Read the log level from the request body. level, err := io.ReadAll(r.Body) if err != nil { + http.Error(w, "Failed to read body", http.StatusBadRequest) return } - + // Validate the log level request. + if err = validateloglevelrequest(w, r, level); err != nil { + http.Error(w, err.Error(), http.StatusBadRequest) + return + } + // Set the logLevel based on the input. + // Accepts: debug, info, error, fatal, or their short forms (d, i, e, f) or numeric values. + // If the input is not recognized, return a 400 Bad Request. + var logLevel lager.LogLevel switch string(level) { - case "debug", "DEBUG", "d", strconv.Itoa(int(lager.DEBUG)): - sink.SetMinLevel(lager.DEBUG) - case "info", "INFO", "i", strconv.Itoa(int(lager.INFO)): - sink.SetMinLevel(lager.INFO) - case "error", "ERROR", "e", strconv.Itoa(int(lager.ERROR)): - sink.SetMinLevel(lager.ERROR) - case "fatal", "FATAL", "f", strconv.Itoa(int(lager.FATAL)): - sink.SetMinLevel(lager.FATAL) + case "debug", "d", strconv.Itoa(int(lager.DEBUG)): + logLevel = lager.DEBUG + case "info", "i", strconv.Itoa(int(lager.INFO)): + logLevel = lager.INFO + case "error", "e", strconv.Itoa(int(lager.ERROR)): + logLevel = lager.ERROR + case "fatal", "f", strconv.Itoa(int(lager.FATAL)): + logLevel = lager.FATAL + default: + http.Error(w, "Invalid log level provided: "+string(level), http.StatusBadRequest) + return } + // Set the log level in the sink. + // The SetMinLevel sets the global zapcore conf.level for the logger. + sink.SetMinLevel(logLevel) + + // Respond with a success message. + w.WriteHeader(http.StatusOK) + w.Header().Set("Content-Type", "text/plain") + w.Write([]byte("✅ /log-level was invoked with Level: " + string(level) + "\n")) })) mux.Handle("/block-profile-rate", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { _rate, err := io.ReadAll(r.Body) From f20ee4a57e3f49b11b29b3f3a3f9d80ecf1ccb5b Mon Sep 17 00:00:00 2001 From: Naveen Sreeramachandra Date: Fri, 20 Jun 2025 09:08:59 +0000 Subject: [PATCH 2/5] add uppercase log levels --- server.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/server.go b/server.go index 714db9f..116312b 100644 --- a/server.go +++ b/server.go @@ -99,13 +99,13 @@ func Handler(sink ReconfigurableSinkInterface) http.Handler { // If the input is not recognized, return a 400 Bad Request. var logLevel lager.LogLevel switch string(level) { - case "debug", "d", strconv.Itoa(int(lager.DEBUG)): + case "debug", "DEBUG", "d", strconv.Itoa(int(lager.DEBUG)): logLevel = lager.DEBUG - case "info", "i", strconv.Itoa(int(lager.INFO)): + case "info", "INFO", "i", strconv.Itoa(int(lager.INFO)): logLevel = lager.INFO - case "error", "e", strconv.Itoa(int(lager.ERROR)): + case "error", "ERROR", "e", strconv.Itoa(int(lager.ERROR)): logLevel = lager.ERROR - case "fatal", "f", strconv.Itoa(int(lager.FATAL)): + case "fatal", "FATAL", "f", strconv.Itoa(int(lager.FATAL)): logLevel = lager.FATAL default: http.Error(w, "Invalid log level provided: "+string(level), http.StatusBadRequest) From 3f84006cff916200773d63b132c054800b6b8eb0 Mon Sep 17 00:00:00 2001 From: Naveen Sreeramachandra Date: Thu, 3 Jul 2025 09:25:59 +0000 Subject: [PATCH 3/5] add validation and input normalization logic --- adapter.go | 68 +++++++++++++ cf_debug_server_test.go | 173 +++++++++++++++++++++++++++------- cf_debug_server_testhelper.go | 12 +++ server.go | 105 +++++++++++---------- 4 files changed, 278 insertions(+), 80 deletions(-) create mode 100644 adapter.go create mode 100644 cf_debug_server_testhelper.go diff --git a/adapter.go b/adapter.go new file mode 100644 index 0000000..69f1342 --- /dev/null +++ b/adapter.go @@ -0,0 +1,68 @@ +package debugserver + +import ( + lager "code.cloudfoundry.org/lager/v3" + "errors" + "net/http" + "strings" +) + +// zapLogLevelController is an interface that defines a method to set the minimum log level. +type zapLogLevelController interface { + SetMinLevel(level lager.LogLevel) +} + +// LagerAdapter is an adapter for the ReconfigurableSinkInterface to work with lager.LogLevel. +type LagerAdapter struct { + Sink ReconfigurableSinkInterface +} + +// SetMinLevel sets the minimum log level for the LagerAdapter. +func (l *LagerAdapter) SetMinLevel(level lager.LogLevel) { + l.Sink.SetMinLevel(level) +} + +// normalizeLogLevel returns a single value that represents +// various forms of the same input level. For example: +// "0", "d", "debug", all of these represents debug log level. +func normalizeLogLevel(input string) string { + switch strings.ToLower(strings.TrimSpace(input)) { + case "0", "d", "debug": + return "debug" + case "1", "i", "info": + return "info" + case "2", "w", "warn": + return "warn" + case "3", "e", "error": + return "error" + case "4", "f", "fatal": + return "fatal" + default: + return "" + } +} + +// validateAndNormalize does two things: +// It validates the incoming request is HTTP type, uses POST method and has non-nil level specified. +// It also normalizes the various forms of the same log level type. For ex: 0, d, debug are all same. +func validateAndNormalize(w http.ResponseWriter, r *http.Request, level []byte) (string, error) { + if r.Method != http.MethodPost { + return "", errors.New("❌ method not allowed, use POST") + } + + if r.TLS != nil { + return "", errors.New("❌ invalid scheme, https is not allowed") + } + + if len(level) == 0 { + return "", errors.New("❌ log level cannot be empty") + } + + input := strings.TrimSpace(string(level)) + normalized := normalizeLogLevel(input) + if normalized == "" { + return "", errors.New("❌ invalid log level: " + string(level)) + } + + return normalized, nil +} diff --git a/cf_debug_server_test.go b/cf_debug_server_test.go index 063d8d7..4f50d25 100644 --- a/cf_debug_server_test.go +++ b/cf_debug_server_test.go @@ -1,12 +1,14 @@ package debugserver_test import ( - "bytes" + "crypto/tls" "flag" "fmt" + "io" "net" "net/http" - "strconv" + "net/http/httptest" + "strings" cf_debug_server "code.cloudfoundry.org/debugserver" lager "code.cloudfoundry.org/lager/v3" @@ -114,44 +116,151 @@ var _ = Describe("CF Debug Server", func() { }) }) - Context("checking log-level endpoint", func() { - validForms := map[lager.LogLevel][]string{ - lager.DEBUG: []string{"debug", "DEBUG", "d", strconv.Itoa(int(lager.DEBUG))}, - lager.INFO: []string{"info", "INFO", "i", strconv.Itoa(int(lager.INFO))}, - lager.ERROR: []string{"error", "ERROR", "e", strconv.Itoa(int(lager.ERROR))}, - lager.FATAL: []string{"fatal", "FATAL", "f", strconv.Itoa(int(lager.FATAL))}, - } + // Context("checking log-level endpoint", func() { + // validForms := map[lager.LogLevel][]string{ + // lager.DEBUG: {"debug", "DEBUG", "d", strconv.Itoa(int(lager.DEBUG))}, + // lager.INFO: {"info", "INFO", "i", strconv.Itoa(int(lager.INFO))}, + // lager.ERROR: {"error", "ERROR", "e", strconv.Itoa(int(lager.ERROR))}, + // lager.FATAL: {"fatal", "FATAL", "f", strconv.Itoa(int(lager.FATAL))}, + // } - //This will add another 16 unit tests to the suit - for level, acceptedForms := range validForms { - for _, form := range acceptedForms { - testLevel := level - testForm := form + // //This will add another 16 unit tests to the suit + // for level, acceptedForms := range validForms { + // for _, form := range acceptedForms { + // testLevel := level + // testForm := form - It("can reconfigure the given sink with "+form, func() { - var err error - process, err = cf_debug_server.Run(address, sink) - Expect(err).NotTo(HaveOccurred()) + // It("can reconfigure the given sink with "+form, func() { + // var err error + // process, err = cf_debug_server.Run(address, sink) + // Expect(err).NotTo(HaveOccurred()) - sink.Log(lager.LogFormat{LogLevel: testLevel, Message: "hello before level change"}) - Eventually(logBuf).ShouldNot(gbytes.Say("hello before level change")) + // sink.Log(lager.LogFormat{LogLevel: testLevel, Message: "hello before level change"}) + // Eventually(logBuf).ShouldNot(gbytes.Say("hello before level change")) - request, err := http.NewRequest("PUT", fmt.Sprintf("http://%s/log-level", address), bytes.NewBufferString(testForm)) + // request, err := http.NewRequest("POST", fmt.Sprintf("http://%s/log-level", address), bytes.NewBufferString(testForm)) - Expect(err).NotTo(HaveOccurred()) + // Expect(err).NotTo(HaveOccurred()) - response, err := http.DefaultClient.Do(request) - Expect(err).NotTo(HaveOccurred()) + // response, err := http.DefaultClient.Do(request) + // dump, err := httputil.DumpResponse(response, true) + // fmt.Println("Response: ", string(dump)) + // Expect(err).NotTo(HaveOccurred()) - Expect(response.StatusCode).To(Equal(http.StatusOK)) - response.Body.Close() + // Expect(response.StatusCode).To(Equal(http.StatusOK)) + // response.Body.Close() - sink.Log(lager.LogFormat{LogLevel: testLevel, Message: "Logs sent with log-level " + testForm}) - Eventually(logBuf).Should(gbytes.Say("Logs sent with log-level " + testForm)) - }) - } - } + // sink.Log(lager.LogFormat{LogLevel: testLevel, Message: "Logs sent with log-level " + testForm}) + // Eventually(logBuf).Should(gbytes.Say("Logs sent with log-level " + testForm)) + // }) + // } + // } + // }) + + }) + + Describe("checking log-level endpoint with various inputs", func() { + var ( + req *http.Request + writer *httptest.ResponseRecorder + ) + + BeforeEach(func() { + writer = httptest.NewRecorder() + req = httptest.NewRequest(http.MethodPost, fmt.Sprintf("http://%s/log-level", address), nil) + }) + + Context("valid log levels", func() { + DescribeTable("returns normalized log level", + func(input string, expected string) { + req.Body = io.NopCloser(strings.NewReader(input)) + levelBytes, _ := io.ReadAll(req.Body) + + actual, err := cf_debug_server.ValidateAndNormalize(writer, req, levelBytes) + Expect(err).ToNot(HaveOccurred()) + Expect(actual).To(Equal(expected)) + }, + + // Debug + Entry("debug - 0", "0", "debug"), + Entry("debug - d", "d", "debug"), + Entry("debug - debug", "debug", "debug"), + Entry("debug - DEBUG", "DEBUG", "debug"), + Entry("debug - DeBuG", "DeBuG", "debug"), + + // Info + Entry("info - 1", "1", "info"), + Entry("info - i", "i", "info"), + Entry("info - info", "info", "info"), + Entry("info - INFO", "INFO", "info"), + Entry("info - InFo", "InFo", "info"), + + // Warn + Entry("warn - 2", "2", "warn"), + Entry("warn - w", "w", "warn"), + Entry("warn - warn", "warn", "warn"), + Entry("warn - WARN", "WARN", "warn"), + Entry("warn - wARn", "wARn", "warn"), + + // Error + Entry("error - 3", "3", "error"), + Entry("error - e", "e", "error"), + Entry("error - error", "error", "error"), + Entry("error - ERROR", "ERROR", "error"), + Entry("error - eRroR", "eRroR", "error"), + + // Fatal + Entry("fatal - 4", "4", "fatal"), + Entry("fatal - f", "f", "fatal"), + Entry("fatal - fatal", "fatal", "fatal"), + Entry("fatal - FATAL", "FATAL", "fatal"), + Entry("fatal - FaTaL", "FaTaL", "fatal"), + ) + }) + + Context("invalid log levels", func() { + It("fails on unsupported level", func() { + level := []byte("invalid") + actual, err := cf_debug_server.ValidateAndNormalize(writer, req, level) + Expect(err).To(HaveOccurred()) + Expect(actual).To(BeEmpty()) + }) + + It("fails on empty level", func() { + level := []byte("") + actual, err := cf_debug_server.ValidateAndNormalize(writer, req, level) + Expect(err).To(HaveOccurred()) + Expect(actual).To(BeEmpty()) + }) + }) + + Context("invalid request method", func() { + It("returns error for non-POST", func() { + req.Method = http.MethodGet + actual, err := cf_debug_server.ValidateAndNormalize(writer, req, []byte("info")) + Expect(err).To(MatchError(ContainSubstring("method not allowed"))) + Expect(actual).To(BeEmpty()) + }) + }) + + Context("invalid TLS scheme", func() { + It("returns error if TLS is used", func() { + req.TLS = &tls.ConnectionState{} + actual, err := cf_debug_server.ValidateAndNormalize(writer, req, []byte("debug")) + Expect(err).To(MatchError(ContainSubstring("invalid scheme"))) + Expect(actual).To(BeEmpty()) + }) + }) + + It("returns error if the request is made over HTTPS", func() { + // Simulate HTTPS by assigning a non-nil TLS connection state + req.TLS = &tls.ConnectionState{} + actual, err := cf_debug_server.ValidateAndNormalize(writer, req, []byte("debug")) + + Expect(err).To(MatchError(ContainSubstring("invalid scheme"))) + Expect(actual).To(BeEmpty()) }) }) -}) + +}) \ No newline at end of file diff --git a/cf_debug_server_testhelper.go b/cf_debug_server_testhelper.go new file mode 100644 index 0000000..034ccd9 --- /dev/null +++ b/cf_debug_server_testhelper.go @@ -0,0 +1,12 @@ +//go:build test + +package debugserver + +import ( + "net/http" +) + +// Exported only for tests +func ValidateAndNormalize(w http.ResponseWriter, r *http.Request, level []byte) (string, error) { + return validateAndNormalize(w, r, level) +} diff --git a/server.go b/server.go index 116312b..25224ff 100644 --- a/server.go +++ b/server.go @@ -1,7 +1,6 @@ package debugserver import ( - "errors" "flag" "io" "net/http" @@ -16,6 +15,7 @@ import ( const ( DebugFlag = "debugAddr" + InvalidLagerLogLevel = 99 ) type DebugServerConfig struct { @@ -39,34 +39,19 @@ func DebugAddress(flags *flag.FlagSet) string { if dbgFlag == nil { return "" } - return dbgFlag.Value.String() } -func validateloglevelrequest(w http.ResponseWriter, r *http.Request, level []byte) error { - // Only POST method is allowed for setting log level. - if r.Method != http.MethodPost { - return errors.New("method not allowed, use POST") - } - - // Only http is allowed for setting log level. - if r.TLS != nil { - return errors.New("invalid scheme, https is not allowed") - } - - // Ensure the log level is not empty. - if len(level) == 0 { - return errors.New("log level cannot be empty") - } - return nil -} - -func Runner(address string, sink ReconfigurableSinkInterface) ifrit.Runner { - return http_server.New(address, Handler(sink)) +// Run starts the debug server with the provided address and log controller. +// Run() -> runProcess() -> Runner() -> http_server.New() -> Handler() +func Run(address string, zapCtrl zapLogLevelController) (ifrit.Process, error) { + return runProcess(address, &LagerAdapter{zapCtrl}) } -func Run(address string, sink ReconfigurableSinkInterface) (ifrit.Process, error) { - p := ifrit.Invoke(Runner(address, sink)) +// runProcess starts the debug server and returns the process. +// It invokes the Runner with the provided address and log controller. +func runProcess(address string, zapCtrl zapLogLevelController) (ifrit.Process, error) { + p := ifrit.Invoke(Runner(address, zapCtrl)) select { case <-p.Ready(): case err := <-p.Wait(): @@ -75,7 +60,12 @@ func Run(address string, sink ReconfigurableSinkInterface) (ifrit.Process, error return p, nil } -func Handler(sink ReconfigurableSinkInterface) http.Handler { +// Runner creates an ifrit.Runner for the debug server with the provided address and log controller. +func Runner(address string, zapCtrl zapLogLevelController) ifrit.Runner { + return http_server.New(address, Handler(zapCtrl)) +} + +func Handler(zapCtrl zapLogLevelController) http.Handler { mux := http.NewServeMux() mux.Handle("/debug/pprof/", http.HandlerFunc(pprof.Index)) mux.Handle("/debug/pprof/trace", http.HandlerFunc(pprof.Trace)) @@ -83,6 +73,26 @@ func Handler(sink ReconfigurableSinkInterface) http.Handler { mux.Handle("/debug/pprof/profile", http.HandlerFunc(pprof.Profile)) mux.Handle("/debug/pprof/symbol", http.HandlerFunc(pprof.Symbol)) mux.Handle("/log-level", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // http://<>:<>/log-level -d {0, debug, DEBUG, d} + // calls zapCtrl.SetMinLevel(lagerLogLevel) -> 0 + // calls grlog.SetLoggingLevel(zapLevel.String()) -> "debug" + // calls conf.level.SetLevel(zapcore.DebugLevel) + // http://<>:<>/log-level -d {1, info, INFO, I} + // calls zapCtrl.SetMinLevel(lagerLogLevel) -> 1 + // calls grlog.SetLoggingLevel(zapLevel.String()) -> "info" + // calls conf.level.SetLevel(zapcore.InfoLevel) + // http://<>:<>/log-level -d {2, warn, WARN, w} + // calls zapCtrl.SetMinLevel(lager.LogLevel(InvalidLagerLogLevel)) -> 99 + // calls conf.level.SetLevel(zapcore.WarnLevel) + // http://<>:<>/log-level -d {3, error, ERROR, e} + // calls zapCtrl.SetMinLevel(lagerLogLevel) -> 2 + // calls grlog.SetLoggingLevel(zapLevel.String()) -> "error" + // calls conf.level.SetLevel(zapcore.ErrorLevel) + // http://<>:<>/log-level -d {4, fatal, FATAL, f} + // calls zapCtrl.SetMinLevel(lagerLogLevel) -> 3 + // calls grlog.SetLoggingLevel(zapLevel.String()) -> "fatal" + // calls conf.level.SetLevel(zapcore.FatalLevel) + // Read the log level from the request body. level, err := io.ReadAll(r.Body) if err != nil { @@ -90,35 +100,34 @@ func Handler(sink ReconfigurableSinkInterface) http.Handler { return } // Validate the log level request. - if err = validateloglevelrequest(w, r, level); err != nil { + var normalizedLevel string + if normalizedLevel, err = validateAndNormalize(w, r, level); err != nil { http.Error(w, err.Error(), http.StatusBadRequest) return } - // Set the logLevel based on the input. - // Accepts: debug, info, error, fatal, or their short forms (d, i, e, f) or numeric values. - // If the input is not recognized, return a 400 Bad Request. - var logLevel lager.LogLevel - switch string(level) { - case "debug", "DEBUG", "d", strconv.Itoa(int(lager.DEBUG)): - logLevel = lager.DEBUG - case "info", "INFO", "i", strconv.Itoa(int(lager.INFO)): - logLevel = lager.INFO - case "error", "ERROR", "e", strconv.Itoa(int(lager.ERROR)): - logLevel = lager.ERROR - case "fatal", "FATAL", "f", strconv.Itoa(int(lager.FATAL)): - logLevel = lager.FATAL - default: - http.Error(w, "Invalid log level provided: "+string(level), http.StatusBadRequest) - return + // Convert the log level to lager.LogLevel. + if normalizedLevel == "warn" { + // Note that zapcore.WarnLevel is not directly supported by lager. + // And lager does not have a separate WARN level, it uses INFO for warnings. + // So to set the minimum level to "warn" we send an Invalid log level, + // which hits the default case in the SetMinLevel method. + // This is a workaround to ensure that the log level is set correctly. + zapCtrl.SetMinLevel(lager.LogLevel(InvalidLagerLogLevel)) + } else { + lagerLogLevel, err := lager.LogLevelFromString(normalizedLevel) + if err != nil { + http.Error(w, "Invalid log level: "+err.Error(), http.StatusBadRequest) + return + } + zapCtrl.SetMinLevel(lagerLogLevel) } - // Set the log level in the sink. - // The SetMinLevel sets the global zapcore conf.level for the logger. - sink.SetMinLevel(logLevel) - // Respond with a success message. w.WriteHeader(http.StatusOK) w.Header().Set("Content-Type", "text/plain") - w.Write([]byte("✅ /log-level was invoked with Level: " + string(level) + "\n")) + w.Write([]byte("✅ /log-level was invoked with Level: " + normalizedLevel + "\n")) + if normalizedLevel == "fatal" { + w.Write([]byte("ℹ️ Note: Fatal logs are reported as error logs in the Gorouter logs.\n")) + } })) mux.Handle("/block-profile-rate", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { _rate, err := io.ReadAll(r.Body) @@ -162,4 +171,4 @@ func Handler(sink ReconfigurableSinkInterface) http.Handler { })) return mux -} +} \ No newline at end of file From ec80c8882821a960577a0234dda52ecf472fcf15 Mon Sep 17 00:00:00 2001 From: Naveen Sreeramachandra Date: Thu, 3 Jul 2025 09:30:59 +0000 Subject: [PATCH 4/5] remove old tests and add new ones for checking log-level endpoint --- cf_debug_server_test.go | 42 ----------------------------------------- 1 file changed, 42 deletions(-) diff --git a/cf_debug_server_test.go b/cf_debug_server_test.go index 4f50d25..84f24f3 100644 --- a/cf_debug_server_test.go +++ b/cf_debug_server_test.go @@ -115,48 +115,6 @@ var _ = Describe("CF Debug Server", func() { Expect(netErr.Op).To(Equal("listen")) }) }) - - // Context("checking log-level endpoint", func() { - // validForms := map[lager.LogLevel][]string{ - // lager.DEBUG: {"debug", "DEBUG", "d", strconv.Itoa(int(lager.DEBUG))}, - // lager.INFO: {"info", "INFO", "i", strconv.Itoa(int(lager.INFO))}, - // lager.ERROR: {"error", "ERROR", "e", strconv.Itoa(int(lager.ERROR))}, - // lager.FATAL: {"fatal", "FATAL", "f", strconv.Itoa(int(lager.FATAL))}, - // } - - // //This will add another 16 unit tests to the suit - // for level, acceptedForms := range validForms { - // for _, form := range acceptedForms { - // testLevel := level - // testForm := form - - // It("can reconfigure the given sink with "+form, func() { - // var err error - // process, err = cf_debug_server.Run(address, sink) - // Expect(err).NotTo(HaveOccurred()) - - // sink.Log(lager.LogFormat{LogLevel: testLevel, Message: "hello before level change"}) - // Eventually(logBuf).ShouldNot(gbytes.Say("hello before level change")) - - // request, err := http.NewRequest("POST", fmt.Sprintf("http://%s/log-level", address), bytes.NewBufferString(testForm)) - - // Expect(err).NotTo(HaveOccurred()) - - // response, err := http.DefaultClient.Do(request) - // dump, err := httputil.DumpResponse(response, true) - // fmt.Println("Response: ", string(dump)) - // Expect(err).NotTo(HaveOccurred()) - - // Expect(response.StatusCode).To(Equal(http.StatusOK)) - // response.Body.Close() - - // sink.Log(lager.LogFormat{LogLevel: testLevel, Message: "Logs sent with log-level " + testForm}) - // Eventually(logBuf).Should(gbytes.Say("Logs sent with log-level " + testForm)) - // }) - // } - // } - // }) - }) Describe("checking log-level endpoint with various inputs", func() { From 229ffefb692640cd1a8e7791d9dd48349c2496f6 Mon Sep 17 00:00:00 2001 From: Naveen Sreeramachandra Date: Thu, 3 Jul 2025 18:17:02 +0000 Subject: [PATCH 5/5] removed emojis and some excess comments --- adapter.go | 8 ++++---- server.go | 29 ++++------------------------- 2 files changed, 8 insertions(+), 29 deletions(-) diff --git a/adapter.go b/adapter.go index 69f1342..a38d53d 100644 --- a/adapter.go +++ b/adapter.go @@ -47,21 +47,21 @@ func normalizeLogLevel(input string) string { // It also normalizes the various forms of the same log level type. For ex: 0, d, debug are all same. func validateAndNormalize(w http.ResponseWriter, r *http.Request, level []byte) (string, error) { if r.Method != http.MethodPost { - return "", errors.New("❌ method not allowed, use POST") + return "", errors.New("method not allowed, use POST") } if r.TLS != nil { - return "", errors.New("❌ invalid scheme, https is not allowed") + return "", errors.New("invalid scheme, https is not allowed") } if len(level) == 0 { - return "", errors.New("❌ log level cannot be empty") + return "", errors.New("log level cannot be empty") } input := strings.TrimSpace(string(level)) normalized := normalizeLogLevel(input) if normalized == "" { - return "", errors.New("❌ invalid log level: " + string(level)) + return "", errors.New("invalid log level: " + string(level)) } return normalized, nil diff --git a/server.go b/server.go index 25224ff..d93fab7 100644 --- a/server.go +++ b/server.go @@ -15,7 +15,6 @@ import ( const ( DebugFlag = "debugAddr" - InvalidLagerLogLevel = 99 ) type DebugServerConfig struct { @@ -73,26 +72,6 @@ func Handler(zapCtrl zapLogLevelController) http.Handler { mux.Handle("/debug/pprof/profile", http.HandlerFunc(pprof.Profile)) mux.Handle("/debug/pprof/symbol", http.HandlerFunc(pprof.Symbol)) mux.Handle("/log-level", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - // http://<>:<>/log-level -d {0, debug, DEBUG, d} - // calls zapCtrl.SetMinLevel(lagerLogLevel) -> 0 - // calls grlog.SetLoggingLevel(zapLevel.String()) -> "debug" - // calls conf.level.SetLevel(zapcore.DebugLevel) - // http://<>:<>/log-level -d {1, info, INFO, I} - // calls zapCtrl.SetMinLevel(lagerLogLevel) -> 1 - // calls grlog.SetLoggingLevel(zapLevel.String()) -> "info" - // calls conf.level.SetLevel(zapcore.InfoLevel) - // http://<>:<>/log-level -d {2, warn, WARN, w} - // calls zapCtrl.SetMinLevel(lager.LogLevel(InvalidLagerLogLevel)) -> 99 - // calls conf.level.SetLevel(zapcore.WarnLevel) - // http://<>:<>/log-level -d {3, error, ERROR, e} - // calls zapCtrl.SetMinLevel(lagerLogLevel) -> 2 - // calls grlog.SetLoggingLevel(zapLevel.String()) -> "error" - // calls conf.level.SetLevel(zapcore.ErrorLevel) - // http://<>:<>/log-level -d {4, fatal, FATAL, f} - // calls zapCtrl.SetMinLevel(lagerLogLevel) -> 3 - // calls grlog.SetLoggingLevel(zapLevel.String()) -> "fatal" - // calls conf.level.SetLevel(zapcore.FatalLevel) - // Read the log level from the request body. level, err := io.ReadAll(r.Body) if err != nil { @@ -109,10 +88,10 @@ func Handler(zapCtrl zapLogLevelController) http.Handler { if normalizedLevel == "warn" { // Note that zapcore.WarnLevel is not directly supported by lager. // And lager does not have a separate WARN level, it uses INFO for warnings. - // So to set the minimum level to "warn" we send an Invalid log level, + // So to set the minimum level to "warn" we send an Invalid log level of 99, // which hits the default case in the SetMinLevel method. // This is a workaround to ensure that the log level is set correctly. - zapCtrl.SetMinLevel(lager.LogLevel(InvalidLagerLogLevel)) + zapCtrl.SetMinLevel(lager.LogLevel(99)) } else { lagerLogLevel, err := lager.LogLevelFromString(normalizedLevel) if err != nil { @@ -124,9 +103,9 @@ func Handler(zapCtrl zapLogLevelController) http.Handler { // Respond with a success message. w.WriteHeader(http.StatusOK) w.Header().Set("Content-Type", "text/plain") - w.Write([]byte("✅ /log-level was invoked with Level: " + normalizedLevel + "\n")) + w.Write([]byte("/log-level was invoked with Level: " + normalizedLevel + "\n")) if normalizedLevel == "fatal" { - w.Write([]byte("ℹ️ Note: Fatal logs are reported as error logs in the Gorouter logs.\n")) + w.Write([]byte("Note: Fatal logs are reported as error logs in the Gorouter logs.\n")) } })) mux.Handle("/block-profile-rate", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {