From 2346a6ee4f3b5d2bb8a6d8d4e8224d40bb2bd75b Mon Sep 17 00:00:00 2001 From: TwiN Date: Tue, 6 Dec 2022 01:37:05 -0500 Subject: [PATCH] fix!: Enforce mandatory space around condition operator (#382) BREAKING CHANGE: The comparator in each condition must now be wrapped by a space (e.g. [STATUS] == 200) or the condition will not be valid. --- core/condition.go | 38 ++++++++++++++++++++++++-------------- core/condition_test.go | 37 +++++++++++++++++++++++++++++++++++++ core/endpoint.go | 15 ++++++++++----- core/endpoint_test.go | 35 ++++++++++++++--------------------- 4 files changed, 85 insertions(+), 40 deletions(-) diff --git a/core/condition.go b/core/condition.go index e8879ecf..c4516b34 100644 --- a/core/condition.go +++ b/core/condition.go @@ -1,6 +1,7 @@ package core import ( + "errors" "fmt" "strconv" "strings" @@ -86,50 +87,59 @@ const ( // Condition is a condition that needs to be met in order for an Endpoint to be considered healthy. type Condition string +// Validate checks if the Condition is valid +func (c Condition) Validate() error { + r := &Result{} + c.evaluate(r, false) + if len(r.Errors) != 0 { + return errors.New(r.Errors[0]) + } + return nil +} + // evaluate the Condition with the Result of the health check -// TODO: Add a mandatory space between each operators (e.g. " == " instead of "==") (BREAKING CHANGE) func (c Condition) evaluate(result *Result, dontResolveFailedConditions bool) bool { condition := string(c) success := false conditionToDisplay := condition - if strings.Contains(condition, "==") { - parameters, resolvedParameters := sanitizeAndResolve(strings.Split(condition, "=="), result) + if strings.Contains(condition, " == ") { + parameters, resolvedParameters := sanitizeAndResolve(strings.Split(condition, " == "), result) success = isEqual(resolvedParameters[0], resolvedParameters[1]) if !success && !dontResolveFailedConditions { conditionToDisplay = prettify(parameters, resolvedParameters, "==") } - } else if strings.Contains(condition, "!=") { - parameters, resolvedParameters := sanitizeAndResolve(strings.Split(condition, "!="), result) + } else if strings.Contains(condition, " != ") { + parameters, resolvedParameters := sanitizeAndResolve(strings.Split(condition, " != "), result) success = !isEqual(resolvedParameters[0], resolvedParameters[1]) if !success && !dontResolveFailedConditions { conditionToDisplay = prettify(parameters, resolvedParameters, "!=") } - } else if strings.Contains(condition, "<=") { - parameters, resolvedParameters := sanitizeAndResolveNumerical(strings.Split(condition, "<="), result) + } else if strings.Contains(condition, " <= ") { + parameters, resolvedParameters := sanitizeAndResolveNumerical(strings.Split(condition, " <= "), result) success = resolvedParameters[0] <= resolvedParameters[1] if !success && !dontResolveFailedConditions { conditionToDisplay = prettifyNumericalParameters(parameters, resolvedParameters, "<=") } - } else if strings.Contains(condition, ">=") { - parameters, resolvedParameters := sanitizeAndResolveNumerical(strings.Split(condition, ">="), result) + } else if strings.Contains(condition, " >= ") { + parameters, resolvedParameters := sanitizeAndResolveNumerical(strings.Split(condition, " >= "), result) success = resolvedParameters[0] >= resolvedParameters[1] if !success && !dontResolveFailedConditions { conditionToDisplay = prettifyNumericalParameters(parameters, resolvedParameters, ">=") } - } else if strings.Contains(condition, ">") { - parameters, resolvedParameters := sanitizeAndResolveNumerical(strings.Split(condition, ">"), result) + } else if strings.Contains(condition, " > ") { + parameters, resolvedParameters := sanitizeAndResolveNumerical(strings.Split(condition, " > "), result) success = resolvedParameters[0] > resolvedParameters[1] if !success && !dontResolveFailedConditions { conditionToDisplay = prettifyNumericalParameters(parameters, resolvedParameters, ">") } - } else if strings.Contains(condition, "<") { - parameters, resolvedParameters := sanitizeAndResolveNumerical(strings.Split(condition, "<"), result) + } else if strings.Contains(condition, " < ") { + parameters, resolvedParameters := sanitizeAndResolveNumerical(strings.Split(condition, " < "), result) success = resolvedParameters[0] < resolvedParameters[1] if !success && !dontResolveFailedConditions { conditionToDisplay = prettifyNumericalParameters(parameters, resolvedParameters, "<") } } else { - result.AddError(fmt.Sprintf("invalid condition '%s' has been provided", condition)) + result.AddError(fmt.Sprintf("invalid condition: %s", condition)) return false } if !success { diff --git a/core/condition_test.go b/core/condition_test.go index 5c63a7b8..06540e65 100644 --- a/core/condition_test.go +++ b/core/condition_test.go @@ -1,11 +1,48 @@ package core import ( + "errors" + "fmt" "strconv" "testing" "time" ) +func TestCondition_Validate(t *testing.T) { + scenarios := []struct { + condition Condition + expectedErr error + }{ + {condition: "[STATUS] == 200", expectedErr: nil}, + {condition: "[STATUS] != 200", expectedErr: nil}, + {condition: "[STATUS] <= 200", expectedErr: nil}, + {condition: "[STATUS] >= 200", expectedErr: nil}, + {condition: "[STATUS] < 200", expectedErr: nil}, + {condition: "[STATUS] > 200", expectedErr: nil}, + {condition: "[STATUS] == any(200, 201, 202, 203)", expectedErr: nil}, + {condition: "[STATUS] == [BODY].status", expectedErr: nil}, + {condition: "[BODY].test == wat", expectedErr: nil}, + {condition: "[BODY].test == wat", expectedErr: nil}, + {condition: "[BODY].test.wat == wat", expectedErr: nil}, + {condition: "[BODY].users[0].id == 1", expectedErr: nil}, + {condition: "len([BODY].users) == 100", expectedErr: nil}, + {condition: "has([BODY].users[0].name) == 100", expectedErr: nil}, + {condition: "raw == raw", expectedErr: nil}, + {condition: "[STATUS] ? 201", expectedErr: errors.New("invalid condition: [STATUS] ? 201")}, + {condition: "[STATUS]==201", expectedErr: errors.New("invalid condition: [STATUS]==201")}, + {condition: "[STATUS] = = 201", expectedErr: errors.New("invalid condition: [STATUS] = = 201")}, + // FIXME: Should return an error, but doesn't because jsonpath isn't evaluated due to body being empty in Condition.Validate() + //{condition: "len([BODY].users == 100", expectedErr: nil}, + } + for _, scenario := range scenarios { + t.Run(string(scenario.condition), func(t *testing.T) { + if err := scenario.condition.Validate(); fmt.Sprint(err) != fmt.Sprint(scenario.expectedErr) { + t.Errorf("expected err %v, got %v", scenario.expectedErr, err) + } + }) + } +} + func TestCondition_evaluate(t *testing.T) { scenarios := []struct { Name string diff --git a/core/endpoint.go b/core/endpoint.go index 538f5850..dd017842 100644 --- a/core/endpoint.go +++ b/core/endpoint.go @@ -5,6 +5,7 @@ import ( "crypto/x509" "encoding/json" "errors" + "fmt" "io" "net" "net/http" @@ -60,6 +61,9 @@ var ( // ErrUnknownEndpointType is the error with which Gatus will panic if an endpoint has an unknown type ErrUnknownEndpointType = errors.New("unknown endpoint type") + // ErrInvalidConditionFormat is the error with which Gatus will panic if a condition has an invalid format + ErrInvalidConditionFormat = errors.New("invalid condition format: does not match ' '") + // ErrInvalidEndpointIntervalForDomainExpirationPlaceholder is the error with which Gatus will panic if an endpoint // has both an interval smaller than 5 minutes and a condition with DomainExpirationPlaceholder. // This is because the free whois service we are using should not be abused, especially considering the fact that @@ -202,11 +206,12 @@ func (endpoint *Endpoint) ValidateAndSetDefaults() error { if len(endpoint.Conditions) == 0 { return ErrEndpointWithNoCondition } - if endpoint.Interval < 5*time.Minute { - for _, condition := range endpoint.Conditions { - if condition.hasDomainExpirationPlaceholder() { - return ErrInvalidEndpointIntervalForDomainExpirationPlaceholder - } + for _, c := range endpoint.Conditions { + if endpoint.Interval < 5*time.Minute && c.hasDomainExpirationPlaceholder() { + return ErrInvalidEndpointIntervalForDomainExpirationPlaceholder + } + if err := c.Validate(); err != nil { + return fmt.Errorf("%v: %w", ErrInvalidConditionFormat, err) } } if endpoint.DNS != nil { diff --git a/core/endpoint_test.go b/core/endpoint_test.go index 2ce3178b..f12fd8f4 100644 --- a/core/endpoint_test.go +++ b/core/endpoint_test.go @@ -346,7 +346,9 @@ func TestEndpoint_ValidateAndSetDefaults(t *testing.T) { Conditions: []Condition{Condition("[STATUS] == 200")}, Alerts: []*alert.Alert{{Type: alert.TypePagerDuty}}, } - endpoint.ValidateAndSetDefaults() + if err := endpoint.ValidateAndSetDefaults(); err != nil { + t.Errorf("Expected no error, got %v", err) + } if endpoint.ClientConfig == nil { t.Error("client configuration should've been set to the default configuration") } else { @@ -383,6 +385,17 @@ func TestEndpoint_ValidateAndSetDefaults(t *testing.T) { } } +func TestEndpoint_ValidateAndSetDefaultsWithInvalidCondition(t *testing.T) { + endpoint := Endpoint{ + Name: "invalid-condition", + URL: "https://twin.sh/health", + Conditions: []Condition{"[STATUS] invalid 200"}, + } + if err := endpoint.ValidateAndSetDefaults(); err == nil { + t.Error("endpoint validation should've returned an error, but didn't") + } +} + func TestEndpoint_ValidateAndSetDefaultsWithClientConfig(t *testing.T) { endpoint := Endpoint{ Name: "website-health", @@ -605,26 +618,6 @@ func TestIntegrationEvaluateHealth(t *testing.T) { } } -func TestIntegrationEvaluateHealthWithInvalidCondition(t *testing.T) { - condition := Condition("[STATUS] invalid 200") - endpoint := Endpoint{ - Name: "invalid-condition", - URL: "https://twin.sh/health", - Conditions: []Condition{condition}, - } - if err := endpoint.ValidateAndSetDefaults(); err != nil { - // XXX: Should this really not return an error? After all, the condition is not valid and conditions are part of the endpoint... - t.Error("endpoint validation should've been successful, but wasn't") - } - result := endpoint.EvaluateHealth() - if result.Success { - t.Error("Because one of the conditions was invalid, result.Success should have been false") - } - if len(result.Errors) == 0 { - t.Error("There should've been an error") - } -} - func TestIntegrationEvaluateHealthWithErrorAndHideURL(t *testing.T) { endpoint := Endpoint{ Name: "invalid-url",