From fdf7764dd114d129168fa9b7d3ea9cb48cd5c9b7 Mon Sep 17 00:00:00 2001 From: Jacob MacElroy Date: Tue, 16 Nov 2021 07:43:55 -0700 Subject: [PATCH] Jdm/stack healthcheck marshaling (#1975) * Simplify curl-based healthcheck parsing for stacks. There was at least one issue parsing for the url to transform the stack health check to the data needed for a health check in kubernetes. The issue discovered was an out of bounds on a slice issue when there was no path present. This commit fixes #1974 Signed-off-by: Jacob MacElroy * Removing leftover print statements. --- pkg/model/stack_serializer.go | 86 ++++++++++++++++++------------ pkg/model/stack_serializer_test.go | 75 +++++++++++++++++++++++++- 2 files changed, 126 insertions(+), 35 deletions(-) diff --git a/pkg/model/stack_serializer.go b/pkg/model/stack_serializer.go index 86d65a2ca865..bbefc79607e9 100644 --- a/pkg/model/stack_serializer.go +++ b/pkg/model/stack_serializer.go @@ -15,7 +15,7 @@ package model import ( "fmt" - "regexp" + "net/url" "strconv" "strings" "time" @@ -490,45 +490,63 @@ func validateHealthcheck(healthcheck *HealthCheck) error { } func translateHealtcheckCurlToHTTP(healthcheck *HealthCheck) { - localPortTestRegex := `^curl ((-f|--fail) )?'?((http|https)://)?(localhost|0.0.0.0):\d+(\/\w*)?'?$` - regexp, err := regexp.Compile(localPortTestRegex) - if err != nil { + // Join and then split the strings by space to ensure that + // each element in the string slice is a contiguous string with + // no spaces. + s := strings.Join(healthcheck.Test, " ") + testStrings := strings.Split(s, " ") + + // There should be at least two strings, the curl binary and url. + if len(testStrings) < 2 { return } - testString := strings.Join(healthcheck.Test, " ") - if regexp.MatchString(testString) { - var firstSlashIndex, portStart int - if strings.Contains(testString, "://") { - testStringCopy := testString - for i := 0; i < 3; i++ { - firstSlashIndex += strings.Index(testStringCopy[firstSlashIndex:], "/") + 1 - } - testStringCopy = testString - for i := 0; i < 2; i++ { - portStart += strings.Index(testStringCopy[portStart:], ":") + 1 - } - portStart-- - firstSlashIndex-- - } else { - firstSlashIndex = strings.Index(testString, "/") - portStart = strings.Index(testString, ":") - } - var port, path string - if firstSlashIndex != -1 { - port = testString[portStart+1 : firstSlashIndex] - path = testString[firstSlashIndex:] - } else { - port = testString[portStart+1:] - path = "/" + if testStrings[0] != "curl" { + return + } + + var checkURL *url.URL + for _, possibleURL := range testStrings[1:] { + if possibleURL == "-f" || possibleURL == "--fail" { + continue } - p, err := strconv.Atoi(port) - if err != nil { - return + + u, err := url.ParseRequestURI(possibleURL) + // It's possible to have a healthcheck url without the scheme. If + // that happens then we inject one and attempt parsing again. + if err != nil || u.Host == "" { + u, err = url.ParseRequestURI("https://" + possibleURL) + if err != nil { + u = nil + continue + } } - healthcheck.HTTP = &HTTPHealtcheck{Path: path, Port: int32(p)} - healthcheck.Test = make(HealtcheckTest, 0) + checkURL = u + break } + + if checkURL == nil { + return + } + + p := checkURL.Port() + if p == "" { + return + } + port, err := strconv.Atoi(p) + if err != nil { + return + } + + var path string + if checkURL.Path == "" { + path = "/" + } else { + path = checkURL.Path + } + + healthcheck.HTTP = &HTTPHealtcheck{Path: path, Port: int32(port)} + healthcheck.Test = make(HealtcheckTest, 0) } func getSvcPorts(public bool, rawPorts, rawExpose []PortRaw) (bool, []Port, error) { diff --git a/pkg/model/stack_serializer_test.go b/pkg/model/stack_serializer_test.go index 692d05d6485d..39e3133bfd63 100644 --- a/pkg/model/stack_serializer_test.go +++ b/pkg/model/stack_serializer_test.go @@ -14,6 +14,7 @@ package model import ( + "fmt" "os" "reflect" "testing" @@ -26,6 +27,77 @@ import ( "k8s.io/utils/pointer" ) +func TestTranslateHealthcheckCurlToHttp(t *testing.T) { + for _, tc := range []struct { + name string + test []string + expectedTest HealtcheckTest + expectedHTTP HTTPHealtcheck + }{ + { + name: "RegularCurlWithZeros", + test: []string{"curl http://0.0.0.0:4572"}, + expectedTest: HealtcheckTest{}, + expectedHTTP: HTTPHealtcheck{Path: "/", Port: int32(4572)}, + }, + { + name: "RegularCurlWithLocalhost", + test: []string{"curl http://localhost:4572"}, + expectedTest: HealtcheckTest{}, + expectedHTTP: HTTPHealtcheck{Path: "/", Port: int32(4572)}, + }, + { + name: "RegularCurlTrailingSlash", + test: []string{"curl http://0.0.0.0:4572/"}, + expectedTest: HealtcheckTest{}, + expectedHTTP: HTTPHealtcheck{Path: "/", Port: int32(4572)}, + }, + { + name: "MissingScheme", + test: []string{"curl 0.0.0.0:4572/readiness"}, + expectedTest: HealtcheckTest{}, + expectedHTTP: HTTPHealtcheck{Path: "/readiness", Port: int32(4572)}, + }, + { + name: "MissingSchemeWithF", + test: []string{"curl -f localhost:8080/"}, + expectedTest: HealtcheckTest{}, + expectedHTTP: HTTPHealtcheck{Path: "/", Port: int32(8080)}, + }, + { + name: "NoTest", + test: []string{"curl --fail 0.0.0.0:8080"}, + expectedTest: HealtcheckTest{}, + expectedHTTP: HTTPHealtcheck{Path: "/", Port: int32(8080)}, + }, + { + name: "Readiness", + test: []string{"curl https://0.0.0.0:8080/readiness"}, + expectedTest: HealtcheckTest{}, + expectedHTTP: HTTPHealtcheck{Path: "/readiness", Port: int32(8080)}, + }, + { + name: "RegularCurlWithPath", + test: []string{"curl http://0.0.0.0:4572/a/path/exists"}, + expectedTest: HealtcheckTest{}, + expectedHTTP: HTTPHealtcheck{Path: "/a/path/exists", Port: int32(4572)}, + }, + } { + t.Run(tc.name, func(t *testing.T) { + check := HealthCheck{ + Test: tc.test, + } + translateHealtcheckCurlToHTTP(&check) + if !reflect.DeepEqual(check.Test, tc.expectedTest) { + t.Fatalf("expected %v but got %v", tc.expectedTest, check.Test) + } + if !reflect.DeepEqual(check.Test, tc.expectedTest) { + t.Fatalf("expected %v but got %v", tc.expectedHTTP, check.HTTP) + } + }) + } +} + func Test_DeployReplicasUnmarshalling(t *testing.T) { tests := []struct { name string @@ -324,7 +396,8 @@ func Test_HealthcheckUnmarshalling(t *testing.T) { } if !tt.expectedError { if !reflect.DeepEqual(s.Services["app"].Healtcheck, tt.expected) { - t.Fatalf("Expected %v, but got %v", tt.expected, s.Services["app"].Healtcheck) + fmt.Printf("GOT: %+v", s.Services["app"].Healtcheck.HTTP) + t.Fatalf("Expected %+v, but got %+v", tt.expected, s.Services["app"].Healtcheck) } }