From a616f88a7b1aa1fcf8517f99aae73f06e3de41dd Mon Sep 17 00:00:00 2001 From: Larry Hitchon Date: Sun, 1 Apr 2018 08:48:45 -0700 Subject: [PATCH] have CheckAssertion return struct with status and message, add key to isMatch messages --- assertion/assertion.go | 20 ++++++++++----- assertion/assertion_test.go | 10 ++++---- assertion/contains.go | 16 ++++++------ assertion/invoke.go | 13 +++++----- assertion/match.go | 51 +++++++++++++++++-------------------- assertion/match_test.go | 10 ++++++-- assertion/rules.go | 19 +++++++------- assertion/testing.go | 6 ++--- assertion/types.go | 25 +++++++++++++----- 9 files changed, 97 insertions(+), 73 deletions(-) diff --git a/assertion/assertion.go b/assertion/assertion.go index ddbd948..25c1e3d 100644 --- a/assertion/assertion.go +++ b/assertion/assertion.go @@ -9,7 +9,7 @@ func searchAndMatch(assertion Assertion, resource Resource, log LoggingFunction) if err != nil { return matchError(err) } - match, err := isMatch(v, assertion.Op, assertion.Value, assertion.ValueType) + match, err := isMatch(v, assertion) log(fmt.Sprintf("Key: %s Output: %v Looking for %v %v", assertion.Key, v, assertion.Op, assertion.Value)) log(fmt.Sprintf("ResourceID: %s Type: %s %v", resource.ID, @@ -52,7 +52,7 @@ func notExpression(assertions []Assertion, resource Resource, log LoggingFunctio return matchError(err) } if match.Match { - return doesNotMatch("Not expression failsL %s", match.Message) + return doesNotMatch("Not expression fails") // TODO needs more information } } return matches() @@ -182,14 +182,20 @@ func FilterResourceExceptions(rule Rule, resources []Resource) []Resource { } // CheckAssertion validates a single Resource using a single Assertion -func CheckAssertion(rule Rule, assertion Assertion, resource Resource, log LoggingFunction) (string, error) { - status := "OK" +func CheckAssertion(rule Rule, assertion Assertion, resource Resource, log LoggingFunction) (Result, error) { + result := Result{ + Status: "OK", + Message: "", + } match, err := booleanExpression(assertion, resource, log) if err != nil { - return "FAILURE", err + result.Status = "FAILURE" + result.Message = err.Error() + return result, err } if !match.Match { - status = rule.Severity + result.Status = rule.Severity + result.Message = match.Message } - return status, nil + return result, nil } diff --git a/assertion/assertion_test.go b/assertion/assertion_test.go index b732f2e..aab022c 100644 --- a/assertion/assertion_test.go +++ b/assertion/assertion_test.go @@ -317,10 +317,10 @@ func TestCheckAssertion(t *testing.T) { } for k, tc := range testCases { - status, err := CheckAssertion(tc.Rule, tc.Rule.Assertions[0], tc.Resource, TestLogging) + assertionResult, err := CheckAssertion(tc.Rule, tc.Rule.Assertions[0], tc.Resource, TestLogging) FailTestIfError(err, "TestSimple", t) - if status != tc.ExpectedStatus { - t.Errorf("%s Failed Expected '%s' to be '%s'", k, status, tc.ExpectedStatus) + if assertionResult.Status != tc.ExpectedStatus { + t.Errorf("%s Failed Expected '%s' to be '%s'", k, assertionResult.Status, tc.ExpectedStatus) } } } @@ -381,9 +381,9 @@ func TestNestedBooleans(t *testing.T) { if err != nil { t.Error("Error parsing resource JSON") } - status, err := CheckAssertion(rule, rule.Assertions[0], resource, TestLogging) + assertionResult, err := CheckAssertion(rule, rule.Assertions[0], resource, TestLogging) FailTestIfError(err, "TestNestedBoolean", t) - if status != "NOT_COMPLIANT" { + if assertionResult.Status != "NOT_COMPLIANT" { t.Error("Expecting nested boolean to return NOT_COMPLIANT") } } diff --git a/assertion/contains.go b/assertion/contains.go index 24a5dec..d034fc9 100644 --- a/assertion/contains.go +++ b/assertion/contains.go @@ -4,7 +4,7 @@ import ( "strings" ) -func contains(data interface{}, value string) (MatchResult, error) { +func contains(data interface{}, key, value string) (MatchResult, error) { switch v := data.(type) { case []interface{}: for _, element := range v { @@ -14,19 +14,19 @@ func contains(data interface{}, value string) (MatchResult, error) { } } } - return doesNotMatch("does not contain %v", value) + return doesNotMatch("%v does not contain %v", key, value) case []string: for _, stringElement := range v { if stringElement == value { return matches() } } - return doesNotMatch("does not contain %v", value) + return doesNotMatch("%v does not contain %v", key, value) case string: if strings.Contains(v, value) { return matches() } - return doesNotMatch("does not contain %v", value) + return doesNotMatch("%v does not contain %v", key, value) default: searchResult, err := JSONStringify(data) if err != nil { @@ -35,17 +35,17 @@ func contains(data interface{}, value string) (MatchResult, error) { if strings.Contains(searchResult, value) { return matches() } - return doesNotMatch("does not contain %v", value) + return doesNotMatch("%v does not contain %v", key, value) } } -func notContains(data interface{}, value string) (MatchResult, error) { - m, err := contains(data, value) +func notContains(data interface{}, key, value string) (MatchResult, error) { + m, err := contains(data, key, value) if err != nil { return matchError(err) } if m.Match { - return doesNotMatch("should not contain %v", value) + return doesNotMatch("%v should not contain %v", key, value) } return matches() } diff --git a/assertion/invoke.go b/assertion/invoke.go index dda684c..a9148ec 100644 --- a/assertion/invoke.go +++ b/assertion/invoke.go @@ -25,12 +25,13 @@ type StandardExternalRuleInvoker struct { func makeViolation(rule Rule, resource Resource, message string) Violation { return Violation{ - RuleID: rule.ID, - Status: rule.Severity, - ResourceID: resource.ID, - ResourceType: resource.Type, - Filename: resource.Filename, - Message: message, + RuleID: rule.ID, + Status: rule.Severity, + ResourceID: resource.ID, + ResourceType: resource.Type, + Filename: resource.Filename, + RuleMessage: rule.Message, + AssertionMessage: message, } } diff --git a/assertion/match.go b/assertion/match.go index 808f545..9e37bdc 100644 --- a/assertion/match.go +++ b/assertion/match.go @@ -6,14 +6,6 @@ import ( "strings" ) -type ( - // MatchResult has a true/false result, but also includes a message for better reporting - MatchResult struct { - Match bool - Message string - } -) - func matches() (MatchResult, error) { return MatchResult{Match: true, Message: ""}, nil } @@ -32,7 +24,7 @@ func matchError(err error) (MatchResult, error) { }, err } -func isMatch(data interface{}, op string, value string, valueType string) (MatchResult, error) { +func isMatch(data interface{}, assertion Assertion) (MatchResult, error) { // FIXME eliminate searchResult this when all operations converted to use data // individual ops can call JSONStringify as needed searchResult, err := JSONStringify(data) @@ -40,48 +32,53 @@ func isMatch(data interface{}, op string, value string, valueType string) (Match return matchError(err) } searchResult = unquoted(searchResult) + key := assertion.Key + op := assertion.Op + value := assertion.Value + valueType := assertion.ValueType + switch op { case "eq": if compare(data, value, valueType) == 0 { return matches() } - return doesNotMatch("should equal to %v", value) + return doesNotMatch("%v should equal to %v", key, value) case "ne": if compare(data, value, valueType) != 0 { return matches() } - return doesNotMatch("should not equal to %v", value) + return doesNotMatch("%v should not be equal to %v", key, value) case "lt": if compare(data, value, valueType) < 0 { return matches() } - return doesNotMatch("should be less than %v", value) + return doesNotMatch("%v should be less than %v", key, value) case "le": if compare(data, value, valueType) <= 0 { return matches() } - return doesNotMatch("should be less than or equal to %v", value) + return doesNotMatch("%v should be less than or equal to %v", key, value) case "gt": if compare(data, value, valueType) > 0 { return matches() } - return doesNotMatch("should be greater than %v", value) + return doesNotMatch("%v should be greater than %v", key, value) case "ge": if compare(data, value, valueType) >= 0 { return matches() } - return doesNotMatch("should be greater than or equal to %v", value) + return doesNotMatch("%v should be greater than or equal to %v", key, value) case "in": for _, v := range strings.Split(value, ",") { if v == searchResult { return matches() } } - return doesNotMatch("should be in %v", value) + return doesNotMatch("%v should be in %v", key, value) case "not-in": for _, v := range strings.Split(value, ",") { if v == searchResult { - return doesNotMatch("should not be in %v", value) + return doesNotMatch("%v should not be in %v", key, value) } } return matches() @@ -89,41 +86,41 @@ func isMatch(data interface{}, op string, value string, valueType string) (Match if isAbsent(searchResult) { return matches() } - return doesNotMatch("should be absent") + return doesNotMatch("%v should be absent", key) case "present": if isPresent(searchResult) { return matches() } - return doesNotMatch("should be present") + return doesNotMatch("%v should be present", key) case "null": if data == nil { return matches() } - return doesNotMatch("should be null") + return doesNotMatch("%v should be null", key) case "not-null": if data != nil { return matches() } - return doesNotMatch("should not be null") + return doesNotMatch("%v should not be null", key) case "empty": if isEmpty(data) { return matches() } - return doesNotMatch("should be empty") + return doesNotMatch("%v should be empty", key) case "not-empty": if !isEmpty(data) { return matches() } - return doesNotMatch("should not be empty") + return doesNotMatch("%v should not be empty", key) case "intersect": if jsonListsIntersect(searchResult, value) { return matches() } - return doesNotMatch("should intersect", value) + return doesNotMatch("%v should intersect with %v", key, value) case "contains": - return contains(data, value) + return contains(data, key, value) case "not-contains": - return notContains(data, value) + return notContains(data, key, value) case "regex": re, err := regexp.Compile(value) if err != nil { @@ -132,7 +129,7 @@ func isMatch(data interface{}, op string, value string, valueType string) (Match if re.MatchString(searchResult) { return matches() } - return doesNotMatch("should match %v", value) + return doesNotMatch("%v should match %v", key, value) case "has-properties": return hasProperties(data, value) } diff --git a/assertion/match_test.go b/assertion/match_test.go index 556426a..4e4b30c 100644 --- a/assertion/match_test.go +++ b/assertion/match_test.go @@ -104,15 +104,21 @@ func TestIsMatch(t *testing.T) { for k, tc := range testCases { var m MatchResult var err error + assertion := Assertion{ + Key: "key", + Op: tc.Op, + Value: tc.Value, + ValueType: tc.ValueType, + } if s, isString := tc.SearchResult.(string); isString { searchResult, err := unmarshal(s) if err != nil { fmt.Println(err) t.Errorf("Unable to parse %s\n", tc.SearchResult) } - m, err = isMatch(searchResult, tc.Op, tc.Value, tc.ValueType) + m, err = isMatch(searchResult, assertion) } else { - m, err = isMatch(tc.SearchResult, tc.Op, tc.Value, tc.ValueType) + m, err = isMatch(tc.SearchResult, assertion) } if err != nil { t.Errorf("%s Failed with error: %s", k, err.Error()) diff --git a/assertion/rules.go b/assertion/rules.go index c92cb2c..1bdfa22 100644 --- a/assertion/rules.go +++ b/assertion/rules.go @@ -111,19 +111,20 @@ func CheckRule(rule Rule, resource Resource, e ExternalRuleInvoker, log LoggingF } for _, ruleAssertion := range rule.Assertions { log(fmt.Sprintf("Checking resource %s", resource.ID)) - status, err := CheckAssertion(rule, ruleAssertion, resource, log) + assertionResult, err := CheckAssertion(rule, ruleAssertion, resource, log) if err != nil { return "FAILURE", violations, err } - if status != "OK" { - returnStatus = status + if assertionResult.Status != "OK" { + returnStatus = assertionResult.Status v := Violation{ - RuleID: rule.ID, - ResourceID: resource.ID, - ResourceType: resource.Type, - Status: status, - Message: rule.Message, - Filename: resource.Filename, + RuleID: rule.ID, + ResourceID: resource.ID, + ResourceType: resource.Type, + Status: assertionResult.Status, + RuleMessage: rule.Message, + AssertionMessage: assertionResult.Message, + Filename: resource.Filename, } violations = append(violations, v) } diff --git a/assertion/testing.go b/assertion/testing.go index a0efc18..77d56b4 100644 --- a/assertion/testing.go +++ b/assertion/testing.go @@ -55,10 +55,10 @@ func LoadTestCasesFromFixture(filename string, t *testing.T) FixtureTestCases { func RunTestCasesFromFixture(filename string, t *testing.T) { fixture := LoadTestCasesFromFixture(filename, t) for _, testCase := range fixture.TestCases { - status, err := CheckAssertion(testCase.Rule, testCase.Rule.Assertions[0], testCase.Resource, TestLogging) + assertionResult, err := CheckAssertion(testCase.Rule, testCase.Rule.Assertions[0], testCase.Resource, TestLogging) FailTestIfError(err, testCase.Name, t) - if status != testCase.Result { - t.Errorf("Test case %s returned %s expecting %s", testCase.Name, status, testCase.Result) + if assertionResult.Status != testCase.Result { + t.Errorf("Test case %s returned %s expecting %s", testCase.Name, assertionResult.Status, testCase.Result) } } } diff --git a/assertion/types.go b/assertion/types.go index e916c2b..2481fbe 100644 --- a/assertion/types.go +++ b/assertion/types.go @@ -79,12 +79,13 @@ type ( // Violation has details for a failed assertion Violation struct { - RuleID string - ResourceID string - ResourceType string - Status string - Message string - Filename string + RuleID string + ResourceID string + ResourceType string + Status string + RuleMessage string + AssertionMessage string + Filename string } // ValueSource interface to fetch dynamic values @@ -96,4 +97,16 @@ type ( ExternalRuleInvoker interface { Invoke(Rule, Resource) (string, []Violation, error) } + + // MatchResult has a true/false result, but also includes a message for better reporting + MatchResult struct { + Match bool + Message string + } + + // Result returns a status, along with a message + Result struct { + Status string + Message string + } )