From 5f10a92c36a1baefc12d0eb3b1875f85d4fcfab1 Mon Sep 17 00:00:00 2001 From: Michael Engelhardt Date: Sat, 21 Nov 2020 03:42:42 +0100 Subject: [PATCH] simplification and refactorings --- config/config.go | 2 +- config/config_test.go | 44 +++++++++++++++++++++++++++++++++++++++++++ config/web.go | 40 ++++++++++++--------------------------- config/web_test.go | 40 +++++++++++++++++++++++++-------------- main.go | 10 +++++----- 5 files changed, 88 insertions(+), 48 deletions(-) diff --git a/config/config.go b/config/config.go index 95448400..be6f7e37 100644 --- a/config/config.go +++ b/config/config.go @@ -146,7 +146,7 @@ func parseAndValidateConfigBytes(yamlBytes []byte) (config *Config, err error) { func validateWebConfig(config *Config) { if config.Web == nil { - config.Web = &webConfig{Address: DefaultAddress, Port: DefaultPort} + config.Web = &webConfig{Address: DefaultAddress, Port: DefaultPort, ContextRoot: DefaultContextRoot} } else { config.Web.validateAndSetDefaults() } diff --git a/config/config_test.go b/config/config_test.go index ef9602bc..c0a8fe92 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -109,6 +109,9 @@ services: if config.Web.Port != DefaultPort { t.Errorf("Port should have been %d, because it is the default value", DefaultPort) } + if config.Web.ContextRoot != DefaultContextRoot { + t.Errorf("ContextRoot should have been %s, because it is the default value", DefaultContextRoot) + } } func TestParseAndValidateConfigBytesWithAddress(t *testing.T) { @@ -215,6 +218,44 @@ services: } } +func TestParseAndValidateConfigBytesWithPortAndHostAndContextRoot(t *testing.T) { + config, err := parseAndValidateConfigBytes([]byte(` +web: + port: 12345 + address: 127.0.0.1 + context-root: /deeply/nested/down=/their +services: + - name: twinnation + url: https://twinnation.org/health + conditions: + - "[STATUS] == 200" +`)) + if err != nil { + t.Error("No error should've been returned") + } + if config == nil { + t.Fatal("Config shouldn't have been nil") + } + if config.Metrics { + t.Error("Metrics should've been false by default") + } + if config.Services[0].URL != "https://twinnation.org/health" { + t.Errorf("URL should have been %s", "https://twinnation.org/health") + } + if config.Services[0].Interval != 60*time.Second { + t.Errorf("Interval should have been %s, because it is the default value", 60*time.Second) + } + if config.Web.Address != "127.0.0.1" { + t.Errorf("Bind address should have been %s, because it is specified in config", "127.0.0.1") + } + if config.Web.Port != 12345 { + t.Errorf("Port should have been %d, because it is specified in config", 12345) + } + if config.Web.ContextRoot != "/deeply/nested/down=/their/" { + t.Errorf("Port should have been %s, because it is specified in config", "/deeply/nested/down=/their/") + } +} + func TestParseAndValidateConfigBytesWithInvalidPort(t *testing.T) { defer func() { recover() }() _, _ = parseAndValidateConfigBytes([]byte(` @@ -260,6 +301,9 @@ services: if config.Web.Port != DefaultPort { t.Errorf("Port should have been %d, because it is the default value", DefaultPort) } + if config.Web.ContextRoot != DefaultContextRoot { + t.Errorf("ContextRoot should have been %s, because it is the default value", DefaultContextRoot) + } } func TestParseAndValidateConfigBytesWithMetricsAndHostAndPort(t *testing.T) { diff --git a/config/web.go b/config/web.go index f1872021..97c1e074 100644 --- a/config/web.go +++ b/config/web.go @@ -16,9 +16,8 @@ type webConfig struct { // Port to listen on (default to 8080 specified by DefaultPort) Port int `yaml:"port"` + // ContextRoot set the root context for the web application ContextRoot string `yaml:"context-root"` - - safeContextRoot string } // validateAndSetDefaults checks and sets missing values based on the defaults @@ -33,26 +32,16 @@ func (web *webConfig) validateAndSetDefaults() { panic(fmt.Sprintf("port has an invalid: value should be between %d and %d", 0, math.MaxUint16)) } if len(web.ContextRoot) == 0 { - web.safeContextRoot = DefaultContextRoot + web.ContextRoot = DefaultContextRoot } else { - // url.PathEscape escapes all "/", in order to build a secure path - // (1) split into path fragements using "/" as delimiter - // (2) use url.PathEscape() on each fragment - // (3) re-concatinate the path using "/" as join character - const splitJoinChar = "/" - pathes := strings.Split(web.ContextRoot, splitJoinChar) - escapedPathes := make([]string, len(pathes)) - for i, path := range pathes { - escapedPathes[i] = url.PathEscape(path) - } - - web.safeContextRoot = strings.Join(escapedPathes, splitJoinChar) - - // assure that we have still a valid url - _, err := url.Parse(web.safeContextRoot) + url, err := url.Parse(web.ContextRoot) if err != nil { - panic(fmt.Sprintf("Invalid context root %s - Error %s", web.ContextRoot, err)) + panic(fmt.Sprintf("Invalid context root %s - error: %s.", web.ContextRoot, err)) } + if url.Path != web.ContextRoot { + panic(fmt.Sprintf("Invalid context root %s, simple path required.", web.ContextRoot)) + } + web.ContextRoot = strings.TrimRight(url.Path, "/") + "/" } } @@ -61,14 +50,9 @@ func (web *webConfig) SocketAddress() string { return fmt.Sprintf("%s:%d", web.Address, web.Port) } -// CtxRoot returns the context root -func (web *webConfig) CtxRoot() string { - return web.safeContextRoot -} - -// AppendToCtxRoot appends the given string to the context root -// AppendToCtxRoot takes care of having only one "/" character at +// AppendToContexRoot appends the given string to the context root +// AppendToContexRoot takes care of having only one "/" character at // the join point and exactly on "/" at the end -func (web *webConfig) AppendToCtxRoot(fragment string) string { - return strings.TrimSuffix(web.safeContextRoot, "/") + "/" + strings.Trim(fragment, "/") + "/" +func (web *webConfig) AppendToContexRoot(fragment string) string { + return web.ContextRoot + strings.Trim(fragment, "/") + "/" } diff --git a/config/web_test.go b/config/web_test.go index 1218d031..9ab04b29 100644 --- a/config/web_test.go +++ b/config/web_test.go @@ -12,6 +12,20 @@ func TestWebConfig_SocketAddress(t *testing.T) { } } +func TestWebConfig_ContextRootEmpty(t *testing.T) { + const expected = "/" + + web := &webConfig{ + ContextRoot: "", + } + + web.validateAndSetDefaults() + + if web.ContextRoot != expected { + t.Errorf("expected %s, got %s", expected, web.ContextRoot) + } +} + func TestWebConfig_ContextRoot(t *testing.T) { const expected = "/status/" @@ -21,13 +35,13 @@ func TestWebConfig_ContextRoot(t *testing.T) { web.validateAndSetDefaults() - if web.CtxRoot() != expected { - t.Errorf("expected %s, got %s", expected, web.CtxRoot()) + if web.ContextRoot != expected { + t.Errorf("expected %s, got %s", expected, web.ContextRoot) } } -func TestWebConfig_ContextRootWithEscapableChars(t *testing.T) { - const expected = "/s%3F=ta%20t%20u&s/" +func TestWebConfig_ContextRootInvalid(t *testing.T) { + defer func() { recover() }() web := &webConfig{ ContextRoot: "/s?=ta t u&s/", @@ -35,21 +49,19 @@ func TestWebConfig_ContextRootWithEscapableChars(t *testing.T) { web.validateAndSetDefaults() - if web.CtxRoot() != expected { - t.Errorf("expected %s, got %s", expected, web.CtxRoot()) - } + t.Fatal("Should've panicked because the configuration specifies an invalid context root") } func TestWebConfig_ContextRootMultiPath(t *testing.T) { - const expected = "/app/status" + const expected = "/app/status/" web := &webConfig{ ContextRoot: "/app/status", } web.validateAndSetDefaults() - if web.CtxRoot() != expected { - t.Errorf("expected %s, got %s", expected, web.CtxRoot()) + if web.ContextRoot != expected { + t.Errorf("expected %s, got %s", expected, web.ContextRoot) } } @@ -59,8 +71,8 @@ func TestWebConfig_ContextRootAppendWithEmptyContextRoot(t *testing.T) { web.validateAndSetDefaults() - if web.AppendToCtxRoot("/bla/") != expected { - t.Errorf("expected %s, got %s", expected, web.AppendToCtxRoot("/bla/")) + if web.AppendToContexRoot("/bla/") != expected { + t.Errorf("expected %s, got %s", expected, web.AppendToContexRoot("/bla/")) } } @@ -72,7 +84,7 @@ func TestWebConfig_ContextRootAppendWithContext(t *testing.T) { web.validateAndSetDefaults() - if web.AppendToCtxRoot("/bla/") != expected { - t.Errorf("expected %s, got %s", expected, web.AppendToCtxRoot("/bla/")) + if web.AppendToContexRoot("/bla/") != expected { + t.Errorf("expected %s, got %s", expected, web.AppendToContexRoot("/bla/")) } } diff --git a/main.go b/main.go index be8bbef3..259a09f0 100644 --- a/main.go +++ b/main.go @@ -31,14 +31,14 @@ func main() { } // favicon needs to be always served from the root http.HandleFunc("/favicon.ico", favIconHandler) - http.HandleFunc(cfg.Web.AppendToCtxRoot("/api/v1/results"), resultsHandler) - http.HandleFunc(cfg.Web.AppendToCtxRoot("/health"), healthHandler) - http.Handle(cfg.Web.CtxRoot(), GzipHandler(http.StripPrefix(cfg.Web.CtxRoot(), http.FileServer(http.Dir("./static"))))) + http.HandleFunc(cfg.Web.AppendToContexRoot("/api/v1/results"), resultsHandler) + http.HandleFunc(cfg.Web.AppendToContexRoot("/health"), healthHandler) + http.Handle(cfg.Web.ContextRoot, GzipHandler(http.StripPrefix(cfg.Web.ContextRoot, http.FileServer(http.Dir("./static"))))) if cfg.Metrics { - http.Handle(cfg.Web.AppendToCtxRoot("/metrics"), promhttp.Handler()) + http.Handle(cfg.Web.AppendToContexRoot("/metrics"), promhttp.Handler()) } - log.Printf("[main][main] Listening on %s%s\n", cfg.Web.SocketAddress(), cfg.Web.CtxRoot()) + log.Printf("[main][main] Listening on %s%s\n", cfg.Web.SocketAddress(), cfg.Web.ContextRoot) go watchdog.Monitor(cfg) log.Fatal(http.ListenAndServe(cfg.Web.SocketAddress(), nil)) }