From 53778dd1026f134ab61432a1a28b62e491e41085 Mon Sep 17 00:00:00 2001 From: Andreas Gkizas Date: Tue, 16 Apr 2024 11:12:48 +0300 Subject: [PATCH 1/2] adding validate flag for GenerateHints --- CHANGELOG.md | 14 ++++- utils/hints.go | 132 +++++++++++++++++++++++--------------------- utils/hints_test.go | 104 +++++++++++++++++++++++++++++++++- 3 files changed, 183 insertions(+), 67 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4e8ab4b2a..fff2d7e35 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,11 +54,21 @@ This project adheres to [Semantic Versioning](http://semver.org/). [0.6.9]: https://github.com/elastic/elastic-agent-autodiscover/compare/v0.6.8...v0.6.9 -## [0.6.11] +## [0.6.12] ### Changed - Enhance GenerateHints function to check supported list of hints for multiple datastreams and metricsets -[0.6.10]: https://github.com/elastic/elastic-agent-autodiscover/compare/v0.6.10...v0.6.11 +[0.6.10]: https://github.com/elastic/elastic-agent-autodiscover/compare/v0.6.10...v0.6.12 + + +## [0.6.13] + +### Changed + +- Enhance GenerateHints function with validate flag to enable the validation of hints + + +[0.6.13]: https://github.com/elastic/elastic-agent-autodiscover/compare/v0.6.12...v0.6.13 \ No newline at end of file diff --git a/utils/hints.go b/utils/hints.go index 41e79996d..d0ca569e2 100644 --- a/utils/hints.go +++ b/utils/hints.go @@ -203,7 +203,9 @@ func IsDisabled(hints mapstr.M, key string) bool { } // GenerateHints parses annotations based on a prefix and sets up hints that can be picked up by individual Beats. -func GenerateHints(annotations mapstr.M, container, prefix string, allSupportedHints []string) (mapstr.M, []string) { +// Arguments: annotations: include the annotatons defined in the container, container: is the container name, +// prefix: the prefix of the annotation to check, validate: boolean variable that enables the validation of hints format and allSupportedHints: list of supported annotations to validate against +func GenerateHints(annotations mapstr.M, container, prefix string, validate bool, allSupportedHints []string) (mapstr.M, []string) { hints := mapstr.M{} var incorrecthints []string var incorrecthint string @@ -212,22 +214,23 @@ func GenerateHints(annotations mapstr.M, container, prefix string, allSupportedH if rawEntries, err := annotations.GetValue(prefix); err == nil { if entries, ok := rawEntries.(mapstr.M); ok { - //Start of Annotation Check: whether the annotation follows the supported format and vocabulary. The check happens for annotations that have prefix co.elastic - datastreamlist := GetHintAsList(entries, logName+"/"+"data_streams", "") - // We check if multiple data_streams are defined and we retrieve the hints per data_stream. Only applicable in elastic-agent - // See Metrics_apache_package_and_specific_config_per_datastream test case in hints_test.go - for _, stream := range datastreamlist { - allSupportedHints = append(allSupportedHints, stream) - incorrecthints = checkSupportedHintsSets(annotations, prefix, stream, logName, allSupportedHints, incorrecthints) - } - metricsetlist := GetHintAsList(entries, "metrics"+"/"+"metricsets", "") - // We check if multiple metrcisets are defined and we retrieve the hints per metricset. Only applicable in beats - //See Metrics_istio_module_and_specific_config_per_metricset test case in hints_test.go - for _, metric := range metricsetlist { - allSupportedHints = append(allSupportedHints, metric) - incorrecthints = checkSupportedHintsSets(annotations, prefix, metric, "metrics", allSupportedHints, incorrecthints) - } - //End of Annotation Check + if validate { + //Start of Annotation Check: whether the annotation follows the supported format and vocabulary. The check happens for annotations that have prefix co.elastic + datastreamlist := GetHintAsList(entries, logName+"/"+"data_streams", "") + // We check if multiple data_streams are defined and we retrieve the hints per data_stream. Only applicable in elastic-agent + // See Metrics_apache_package_and_specific_config_per_datastream test case in hints_test.go + for _, stream := range datastreamlist { + allSupportedHints = append(allSupportedHints, stream) + incorrecthints = checkSupportedHintsSets(annotations, prefix, stream, logName, allSupportedHints, incorrecthints) + } + metricsetlist := GetHintAsList(entries, "metrics"+"/"+"metricsets", "") + // We check if multiple metrcisets are defined and we retrieve the hints per metricset. Only applicable in beats + //See Metrics_istio_module_and_specific_config_per_metricset test case in hints_test.go + for _, metric := range metricsetlist { + allSupportedHints = append(allSupportedHints, metric) + incorrecthints = checkSupportedHintsSets(annotations, prefix, metric, "metrics", allSupportedHints, incorrecthints) + } + } //End of Annotation Check for key, rawValue := range entries { enumeratedmodules := []string{} @@ -237,32 +240,33 @@ func GenerateHints(annotations mapstr.M, container, prefix string, allSupportedH if len(parts) == 2 { hintKey := fmt.Sprintf("%s.%s", parts[0], parts[1]) - checkdigit := digitCheck.MatchString(parts[1]) // With this regex we check if enumeration for modules is provided - if checkdigit { - allSupportedHints = append(allSupportedHints, parts[1]) - - specificlist, _ := entries.GetValue(key) - if specificentries, ok := specificlist.(mapstr.M); ok { - for keyspec := range specificentries { - // enumeratedmodules will be populated only in cases we have module enumeration, like: - // "co.elastic.metrics/1.module": "prometheus", - // "co.elastic.metrics/2.module": "istiod", - enumeratedmodules = append(enumeratedmodules, keyspec) + if validate { + checkdigit := digitCheck.MatchString(parts[1]) // With this regex we check if enumeration for modules is provided + if checkdigit { + allSupportedHints = append(allSupportedHints, parts[1]) + + specificlist, _ := entries.GetValue(key) + if specificentries, ok := specificlist.(mapstr.M); ok { + for keyspec := range specificentries { + // enumeratedmodules will be populated only in cases we have module enumeration, like: + // "co.elastic.metrics/1.module": "prometheus", + // "co.elastic.metrics/2.module": "istiod", + enumeratedmodules = append(enumeratedmodules, keyspec) + } } } - } + // We check if multiple metrcisets are defined and we retrieve the hints per metricset. Only applicable in beats + // See Metrics_multiple_modules_and_specific_config_per_module test case in hints_test.go + for _, metric := range enumeratedmodules { + _, incorrecthint = checkSupportedHints(metric, fmt.Sprintf("%s.%s", key, metric), allSupportedHints) + if incorrecthint != "" { + incorrecthints = append(incorrecthints, incorrecthint) + } - // We check if multiple metrcisets are defined and we retrieve the hints per metricset. Only applicable in beats - // See Metrics_multiple_modules_and_specific_config_per_module test case in hints_test.go - for _, metric := range enumeratedmodules { - _, incorrecthint = checkSupportedHints(metric, fmt.Sprintf("%s.%s", key, metric), allSupportedHints) - if incorrecthint != "" { - incorrecthints = append(incorrecthints, incorrecthint) } - - } - //We check whether the provided annotation follows the supported format and vocabulary. The check happens for annotations that have prefix co.elastic - _, incorrecthint = checkSupportedHints(parts[1], key, allSupportedHints) + //We check whether the provided annotation follows the supported format and vocabulary. The check happens for annotations that have prefix co.elastic + _, incorrecthint = checkSupportedHints(parts[1], key, allSupportedHints) + } //end of check // Insert only if there is no entry already. container level annotations take // higher priority. @@ -286,34 +290,34 @@ func GenerateHints(annotations mapstr.M, container, prefix string, allSupportedH // Split the key to get part[1] to be the hint parts := strings.Split(hintKey, "/") - checkdigit := digitCheck.MatchString(parts[1]) // With this regex we check if enumeration for modules is provided - if checkdigit { - allSupportedHints = append(allSupportedHints, parts[1]) - - specificlist, _ := entries.GetValue(key) - if specificentries, ok := specificlist.(mapstr.M); ok { - for keyspec := range specificentries { - // enumeratedmodules will be populated only in cases we have module enumeration, like: - // "co.elastic.metrics/1.module": "prometheus", - // "co.elastic.metrics/2.module": "istiod", - enumeratedmodules = append(enumeratedmodules, keyspec) + if validate { + checkdigit := digitCheck.MatchString(parts[1]) // With this regex we check if enumeration for modules is provided + if checkdigit { + allSupportedHints = append(allSupportedHints, parts[1]) + + specificlist, _ := entries.GetValue(key) + if specificentries, ok := specificlist.(mapstr.M); ok { + for keyspec := range specificentries { + // enumeratedmodules will be populated only in cases we have module enumeration, like: + // "co.elastic.metrics/1.module": "prometheus", + // "co.elastic.metrics/2.module": "istiod", + enumeratedmodules = append(enumeratedmodules, keyspec) + } } } - } - - // We check if multiple metrcisets are defined and we retrieve the hints per metricset. Only applicable in beats - // See Metrics_multiple_modules_and_specific_config_per_module test case in hints_test.go - for _, metric := range enumeratedmodules { - _, incorrecthint = checkSupportedHints(metric, fmt.Sprintf("%s.%s", key, metric), allSupportedHints) - if incorrecthint != "" { - incorrecthints = append(incorrecthints, incorrecthint) - } - } - //We check whether the provided annotation follows the supported format and vocabulary. The check happens for annotations that have prefix co.elastic - _, incorrecthint = checkSupportedHints(parts[1], key, allSupportedHints) + // We check if multiple metrcisets are defined and we retrieve the hints per metricset. Only applicable in beats + // See Metrics_multiple_modules_and_specific_config_per_module test case in hints_test.go + for _, metric := range enumeratedmodules { + _, incorrecthint = checkSupportedHints(metric, fmt.Sprintf("%s.%s", key, metric), allSupportedHints) + if incorrecthint != "" { + incorrecthints = append(incorrecthints, incorrecthint) + } - //end of check + } + //We check whether the provided annotation follows the supported format and vocabulary. The check happens for annotations that have prefix co.elastic + _, incorrecthint = checkSupportedHints(parts[1], key, allSupportedHints) + } //end of check if len(parts) == 2 { // key will be the hint type @@ -326,7 +330,7 @@ func GenerateHints(annotations mapstr.M, container, prefix string, allSupportedH } } } - if incorrecthint != "" { + if validate && incorrecthint != "" { incorrecthints = append(incorrecthints, incorrecthint) } } diff --git a/utils/hints_test.go b/utils/hints_test.go index 0f48cc0e9..e83418842 100644 --- a/utils/hints_test.go +++ b/utils/hints_test.go @@ -311,11 +311,113 @@ func TestGenerateHints(t *testing.T) { } } - generateHints, incorrectHints := GenerateHints(annMap, "foobar", "co.elastic", allSupportedHints) + generateHints, incorrectHints := GenerateHints(annMap, "foobar", "co.elastic", true, allSupportedHints) assert.Equal(t, test.expectedIncorrectHints, len(incorrectHints)) // We validate how many incorrect hints are provided per test case. assert.Equal(t, test.result, generateHints) } } + +func TestGenerateHintsWithValidatedisabled(t *testing.T) { + + var allSupportedHints = []string{"enabled", "package", "module", "integration", "data_streams", "metricsets", "host", "period", "timeout", "metrics_path", "username", "password", "stream", "processors", "multiline", "json", "disable"} + + tests := []struct { + name string + annotations map[string]string + result mapstr.M + expectedIncorrectHints int // We set the number of hints that will be marked as incorrect and wont be included in the acceptable supported list + }{ + + // Scenarios being tested: + // have co.elastic.hints/package set. + // Define multiple co.elastic.hints/data_streams and also specific configuration for each one + // Typo errors introduced for "co.elastic.hints/access.streams" and "co.elastic.hints/error.streams" + { + name: "Metrics_apache_package_and_specific_config_per_datastream", + annotations: map[string]string{ + "co.elastic.hints/package": "apache", + "co.elastic.hints/data_streams": "access,error", + "co.elastic.hints/access.period": "5m", + "co.elastic.hints/access.streamssssssssss": "stdout", // On purpose this added with typo + "co.elastic.hints/error.period": "5m", + "co.elastic.hints/error.streamssssssssss": "stderr", // On purpose this added with typo + }, + result: mapstr.M{ + "hints": mapstr.M{ + "data_streams": "access,error", + "access": mapstr.M{"period": "5m", "streamssssssssss": "stdout"}, + "error": mapstr.M{"period": "5m", "streamssssssssss": "stderr"}, + "package": "apache", + }}, + expectedIncorrectHints: 0, // Validate flag= false in GenerateHints + }, + // Scenarios being tested: + // have co.elastic.metrics/module set. + // Define multiple co.elastic.hints/data_streams and also specific configuration for each one + // A typo error introduced for "co.elastic.metrics/istiod.streams" + { + name: "Metrics_istio_module_and_specific_config_per_metricset", + annotations: map[string]string{ + "co.elastic.metrics/module": "istio", + "co.elastic.metrics/metricsets": "istiod,proxy", + "co.elastic.metrics/istiod.period": "5m", + "co.elastic.metrics/istiod.streamssssssssss": "stdout", // On purpose this added with typo + "co.elastic.metrics/proxy.period": "5m", + "co.elastic.metrics/proxy.stream": "stderr", + }, + result: mapstr.M{ + "metrics": mapstr.M{ + "metricsets": "istiod,proxy", + "istiod": mapstr.M{"period": "5m", "streamssssssssss": "stdout"}, + "proxy": mapstr.M{"period": "5m", "stream": "stderr"}, + "module": "istio", + }}, + expectedIncorrectHints: 0, // Validate flag= false in GenerateHints + }, + // Scenarios being tested: + // have co.elastic.metrics/module set for multiple enumerations. + // Define different hints for each one enumeration + // A typo error introduced for "co.elastic.metrics/1.periods" and "co.elastic.metrics/2.streams" + { + name: "Metrics_multiple_modules_and_specific_config_per_module", + annotations: map[string]string{ + "co.elastic.metrics/1.module": "prometheus", + "co.elastic.metrics/1.periodssssssssss": "15s", // On purpose this added with typo + "co.elastic.metrics/2.module": "istiod", + "co.elastic.metrics/2.period": "15s", + "co.elastic.metrics/2.streamssssssssss": "stderr", // On purpose this added with typo + }, + result: mapstr.M{ + "metrics": mapstr.M{ + "1": mapstr.M{ + "module": "prometheus", + "periodssssssssss": "15s", + }, + "2": mapstr.M{ + "module": "istiod", + "period": "15s", + "streamssssssssss": "stderr", + }, + }}, + expectedIncorrectHints: 0, // Validate flag= false in GenerateHints + }, + } + + for _, test := range tests { + annMap := mapstr.M{} + for k, v := range test.annotations { + _, err := annMap.Put(k, v) + if err != nil { + continue + } + } + + generateHints, incorrectHints := GenerateHints(annMap, "foobar", "co.elastic", false, allSupportedHints) + assert.Equal(t, test.expectedIncorrectHints, len(incorrectHints)) // We validate how many incorrect hints are provided per test case. + assert.Equal(t, test.result, generateHints) + } +} + func TestGetHintsAsList(t *testing.T) { tests := []struct { input mapstr.M From 2b6b17ba903bec8aea1ea50dc83eb3e6cabad5e0 Mon Sep 17 00:00:00 2001 From: Andreas Gkizas Date: Tue, 16 Apr 2024 11:18:19 +0300 Subject: [PATCH 2/2] adding comments --- utils/hints.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/utils/hints.go b/utils/hints.go index d0ca569e2..2d23c2ceb 100644 --- a/utils/hints.go +++ b/utils/hints.go @@ -230,7 +230,7 @@ func GenerateHints(annotations mapstr.M, container, prefix string, validate bool allSupportedHints = append(allSupportedHints, metric) incorrecthints = checkSupportedHintsSets(annotations, prefix, metric, "metrics", allSupportedHints, incorrecthints) } - } //End of Annotation Check + } //end of annotation check for key, rawValue := range entries { enumeratedmodules := []string{} @@ -266,7 +266,7 @@ func GenerateHints(annotations mapstr.M, container, prefix string, validate bool } //We check whether the provided annotation follows the supported format and vocabulary. The check happens for annotations that have prefix co.elastic _, incorrecthint = checkSupportedHints(parts[1], key, allSupportedHints) - } //end of check + } //end of annotation check // Insert only if there is no entry already. container level annotations take // higher priority. @@ -317,7 +317,7 @@ func GenerateHints(annotations mapstr.M, container, prefix string, validate bool } //We check whether the provided annotation follows the supported format and vocabulary. The check happens for annotations that have prefix co.elastic _, incorrecthint = checkSupportedHints(parts[1], key, allSupportedHints) - } //end of check + } //end of annotation check if len(parts) == 2 { // key will be the hint type