diff --git a/cmd/collectors/rest/plugins/volume/volume.go b/cmd/collectors/rest/plugins/volume/volume.go index d6083e727..65aeec4f8 100644 --- a/cmd/collectors/rest/plugins/volume/volume.go +++ b/cmd/collectors/rest/plugins/volume/volume.go @@ -5,7 +5,7 @@ package volume import ( - "github.com/hashicorp/go-version" + "fmt" "github.com/netapp/harvest/v2/cmd/collectors" "github.com/netapp/harvest/v2/cmd/poller/plugin" "github.com/netapp/harvest/v2/cmd/tools/rest" @@ -85,7 +85,10 @@ func (v *Volume) Init() error { // Read template to decide inclusion of flexgroup constituents v.includeConstituents = collectors.ReadPluginKey(v.Params, "include_constituents") // ARW feature is supported from 9.10 onwards, If we ask this field in Rest call in plugin, then it will be failed. - v.isArwSupportedVersion = v.versionHigherThan(ARWSupportedVersion) + v.isArwSupportedVersion, err = util.VersionAtLeast(v.client.Cluster().GetVersion(), ARWSupportedVersion) + if err != nil { + return fmt.Errorf("unable to get version %w", err) + } return nil } @@ -302,15 +305,3 @@ func (v *Volume) updateAggrMap(disks []gjson.Result) { } } } - -func (v *Volume) versionHigherThan(minVersion string) bool { - currentVersion, err := version.NewVersion(v.client.Cluster().GetVersion()) - if err != nil { - return false - } - minSupportedVersion, err := version.NewVersion(minVersion) - if err != nil { - return false - } - return currentVersion.GreaterThanOrEqual(minSupportedVersion) -} diff --git a/cmd/collectors/rest/rest.go b/cmd/collectors/rest/rest.go index adda3849b..c593858e3 100644 --- a/cmd/collectors/rest/rest.go +++ b/cmd/collectors/rest/rest.go @@ -31,17 +31,25 @@ import ( "github.com/netapp/harvest/v2/pkg/util" "github.com/tidwall/gjson" "os" + "regexp" "sort" "strconv" "strings" "time" ) +// Regular expression to match dot notation or single word without dot +// It allows one or more alphanumeric characters or underscores, optionally followed by a dot and more characters +// This pattern can repeat any number of times +// This version does not allow numeric-only segments +var validPropRegex = regexp.MustCompile(`^([a-zA-Z_]\w*\.)*[a-zA-Z_]\w*$`) + type Rest struct { *collector.AbstractCollector - Client *rest.Client - Prop *prop - endpoints []*endPoint + Client *rest.Client + Prop *prop + endpoints []*endPoint + isIgnoreUnknownFieldsEnabled bool } type endPoint struct { @@ -59,8 +67,10 @@ type prop struct { Counters map[string]string ReturnTimeOut *int Fields []string - APIType string // public, private + HiddenFields []string + IsPublic bool Filter []string + Href string } type Metric struct { @@ -85,8 +95,38 @@ func (r *Rest) query(p *endPoint) string { return p.prop.Query } -func (r *Rest) fields(p *endPoint) []string { - return p.prop.Fields +func (r *Rest) isValidFormat(prop *prop) bool { + for _, str := range prop.Fields { + if !validPropRegex.MatchString(str) { + return false + } + } + return true +} + +func (r *Rest) Fields(prop *prop) []string { + fields := prop.Fields + if prop.IsPublic { + // applicable for public API only + if !r.isIgnoreUnknownFieldsEnabled || !r.isValidFormat(prop) { + fields = []string{"*"} + } + } + if len(prop.HiddenFields) > 0 { + fieldsMap := make(map[string]bool) + for _, field := range fields { + fieldsMap[field] = true + } + + // append hidden fields + for _, hiddenField := range prop.HiddenFields { + if _, exists := fieldsMap[hiddenField]; !exists { + fields = append(fields, hiddenField) + fieldsMap[hiddenField] = true + } + } + } + return fields } func (r *Rest) filter(p *endPoint) []string { @@ -126,6 +166,7 @@ func (r *Rest) Init(a *collector.AbstractCollector) error { if err = r.InitMatrix(); err != nil { return err } + r.Logger.Debug(). Int("numMetrics", len(r.Prop.Metrics)). Str("timeout", r.Client.Timeout.String()). @@ -223,26 +264,26 @@ func (r *Rest) initEndPoints() error { n := line.GetNameS() e := endPoint{name: n} - prop := prop{} + p := prop{} - prop.InstanceKeys = make([]string, 0) - prop.InstanceLabels = make(map[string]string) - prop.Counters = make(map[string]string) - prop.Metrics = make(map[string]*Metric) - prop.APIType = "public" - prop.ReturnTimeOut = r.Prop.ReturnTimeOut - prop.TemplatePath = r.Prop.TemplatePath + p.InstanceKeys = make([]string, 0) + p.InstanceLabels = make(map[string]string) + p.Counters = make(map[string]string) + p.Metrics = make(map[string]*Metric) + p.IsPublic = true + p.ReturnTimeOut = r.Prop.ReturnTimeOut + p.TemplatePath = r.Prop.TemplatePath for _, line1 := range line.GetChildren() { if line1.GetNameS() == "query" { - prop.Query = line1.GetContentS() - prop.APIType = checkQueryType(prop.Query) + p.Query = line1.GetContentS() + p.IsPublic = util.IsPublicAPI(p.Query) } if line1.GetNameS() == "counters" { - r.ParseRestCounters(line1, &prop) + r.ParseRestCounters(line1, &p) } } - e.prop = &prop + e.prop = &p r.endpoints = append(r.endpoints, &e) } } @@ -286,6 +327,57 @@ func getFieldName(source string, parent string) []string { return res } +// PollCounter performs daily tasks such as updating the cluster info and caching href. +func (r *Rest) PollCounter() (map[string]*matrix.Matrix, error) { + + startTime := time.Now() + // Update the cluster info to track if customer version is updated + err := r.Client.UpdateClusterInfo(5) + if err != nil { + return nil, err + } + apiD := time.Since(startTime) + + startTime = time.Now() + v, err := util.VersionAtLeast(r.Client.Cluster().GetVersion(), "9.11.1") + if err != nil { + return nil, err + } + // Check the version if it is 9.11.1 then pass relevant fields and not * + if v { + r.isIgnoreUnknownFieldsEnabled = true + } else { + r.isIgnoreUnknownFieldsEnabled = false + } + r.updateHref() + parseD := time.Since(startTime) + + // update metadata for collector logs + _ = r.Metadata.LazySetValueInt64("api_time", "counter", apiD.Microseconds()) + _ = r.Metadata.LazySetValueInt64("parse_time", "counter", parseD.Microseconds()) + return nil, nil +} + +func (r *Rest) updateHref() { + r.Prop.Href = rest.NewHrefBuilder(). + APIPath(r.Prop.Query). + Fields(r.Fields(r.Prop)). + Filter(r.Prop.Filter). + ReturnTimeout(r.Prop.ReturnTimeOut). + IsIgnoreUnknownFieldsEnabled(r.isIgnoreUnknownFieldsEnabled). + Build() + + for _, e := range r.endpoints { + e.prop.Href = rest.NewHrefBuilder(). + APIPath(r.query(e)). + Fields(r.Fields(e.prop)). + Filter(r.filter(e)). + ReturnTimeout(r.Prop.ReturnTimeOut). + IsIgnoreUnknownFieldsEnabled(r.isIgnoreUnknownFieldsEnabled). + Build() + } +} + func (r *Rest) PollData() (map[string]*matrix.Matrix, error) { var ( @@ -296,19 +388,16 @@ func (r *Rest) PollData() (map[string]*matrix.Matrix, error) { r.Logger.Trace().Msg("starting data poll") + if err != nil { + return nil, err + } + r.Matrix[r.Object].Reset() r.Client.Metadata.Reset() startTime = time.Now() - href := rest.NewHrefBuilder(). - APIPath(r.Prop.Query). - Fields(r.Prop.Fields). - Filter(r.Prop.Filter). - ReturnTimeout(r.Prop.ReturnTimeOut). - Build() - - if records, err = r.GetRestData(href); err != nil { + if records, err = r.GetRestData(r.Prop.Href); err != nil { return nil, err } @@ -357,15 +446,8 @@ func (r *Rest) pollData( } func (r *Rest) processEndPoint(e *endPoint) ([]gjson.Result, time.Duration, error) { - href := rest.NewHrefBuilder(). - APIPath(r.query(e)). - Fields(r.fields(e)). - Filter(r.filter(e)). - ReturnTimeout(r.Prop.ReturnTimeOut). - Build() - now := time.Now() - data, err := r.GetRestData(href) + data, err := r.GetRestData(e.prop.Href) if err != nil { return nil, 0, err } @@ -403,14 +485,6 @@ func (r *Rest) processEndPoints(endpointFunc func(e *endPoint) ([]gjson.Result, return count, totalAPID } -// returns private if api endpoint has private keyword in it else public -func checkQueryType(query string) string { - if strings.Contains(query, "private") { - return "private" - } - return "public" -} - func (r *Rest) LoadPlugin(kind string, abc *plugin.AbstractPlugin) plugin.Plugin { switch kind { case "Disk": diff --git a/cmd/collectors/rest/rest_test.go b/cmd/collectors/rest/rest_test.go index c185441bf..5f0ff8d44 100644 --- a/cmd/collectors/rest/rest_test.go +++ b/cmd/collectors/rest/rest_test.go @@ -8,6 +8,7 @@ import ( "github.com/netapp/harvest/v2/pkg/matrix" "github.com/tidwall/gjson" "os" + "reflect" "strings" "testing" "time" @@ -161,3 +162,278 @@ func newRest(object string, path string) *Rest { } return &r } + +func TestIsValidFormat(t *testing.T) { + + tests := []struct { + name string + r *Rest + p *prop + expectedResult bool + }{ + { + name: "Test with valid fields 1", + r: &Rest{}, + p: &prop{ + Fields: []string{ + "uuid", + "block_storage.primary.disk_type", + "block_storage.primary.raid_type", + }, + IsPublic: true, + }, + expectedResult: true, + }, + + { + name: "Test with invalid fields 2", + r: &Rest{}, + p: &prop{ + Fields: []string{ + "uuid", + "cloud_storage.stores.#.cloud_store.name", + "block_storage.primary.raid_type", + }, + IsPublic: true, + }, + expectedResult: false, + }, + { + name: "Test with invalid fields 3", + r: &Rest{}, + p: &prop{ + Fields: []string{ + "uuid", + "cloud_storage.stores.0.cloud_store.name", + "block_storage.primary.raid_type", + }, + IsPublic: true, + }, + expectedResult: false, + }, + { + name: "Test with invalid fields 4", + r: &Rest{}, + p: &prop{ + Fields: []string{ + "uuid", + "{interfaces.#.name,interfaces.#.ip.address}", + }, + IsPublic: true, + }, + expectedResult: false, + }, + { + name: "Test with invalid fields 5", + r: &Rest{}, + p: &prop{ + Fields: []string{ + "uuid", + "friends.#(last==\"Murphy\")#.first", + }, + IsPublic: true, + }, + expectedResult: false, + }, + { + name: "Test with invalid fields 6", + r: &Rest{}, + p: &prop{ + Fields: []string{ + "uuid", + "children|@case:upper", + }, + IsPublic: true, + }, + expectedResult: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := tt.r.isValidFormat(tt.p) + if result != tt.expectedResult { + t.Errorf("Expected %v, got %v", tt.expectedResult, result) + } + }) + } +} + +func TestFields(t *testing.T) { + tests := []struct { + name string + r *Rest + p *prop + expectedResult []string + }{ + { + name: "Test with valid fields and no hidden fields", + r: &Rest{ + isIgnoreUnknownFieldsEnabled: true, + }, + p: &prop{ + Fields: []string{ + "uuid", + "block_storage.primary.disk_type", + "block_storage.primary.raid_type", + }, + IsPublic: true, + }, + expectedResult: []string{ + "uuid", + "block_storage.primary.disk_type", + "block_storage.primary.raid_type", + }, + }, + { + name: "Test with invalid fields and no hidden fields", + r: &Rest{ + isIgnoreUnknownFieldsEnabled: true, + }, + p: &prop{ + Fields: []string{ + "uuid", + "cloud_storage.stores.0.cloud_store.name", + "block_storage.primary.raid_type", + }, + IsPublic: true, + }, + expectedResult: []string{"*"}, + }, + { + name: "Test with valid fields and hidden fields", + r: &Rest{ + isIgnoreUnknownFieldsEnabled: true, + }, + p: &prop{ + Fields: []string{ + "uuid", + "block_storage.primary.disk_type", + "block_storage.primary.raid_type", + }, + HiddenFields: []string{ + "hidden_field1", + "hidden_field2", + }, + IsPublic: true, + }, + expectedResult: []string{ + "uuid", + "block_storage.primary.disk_type", + "block_storage.primary.raid_type", + "hidden_field1", + "hidden_field2", + }, + }, + { + name: "Test with valid fields and hidden fields and prior versions to 9.11.1", + r: &Rest{ + isIgnoreUnknownFieldsEnabled: false, + }, + p: &prop{ + Fields: []string{ + "uuid", + "block_storage.primary.disk_type", + "block_storage.primary.raid_type", + }, + HiddenFields: []string{ + "hidden_field1", + "hidden_field2", + }, + IsPublic: true, + }, + expectedResult: []string{ + "*", + "hidden_field1", + "hidden_field2", + }, + }, + { + name: "Test with valid fields and no hidden fields for private API", + r: &Rest{ + isIgnoreUnknownFieldsEnabled: false, + }, + p: &prop{ + Fields: []string{ + "uuid", + "block_storage.primary.disk_type", + "block_storage.primary.raid_type", + }, + IsPublic: false, + }, + expectedResult: []string{ + "uuid", + "block_storage.primary.disk_type", + "block_storage.primary.raid_type", + }, + }, + { + name: "Test with invalid fields and no hidden fields, ignore unknown fields disabled", + r: &Rest{ + isIgnoreUnknownFieldsEnabled: false, + }, + p: &prop{ + Fields: []string{ + "uuid", + "cloud_storage.stores.0.cloud_store.name", + "block_storage.primary.raid_type", + }, + IsPublic: true, + }, + expectedResult: []string{"*"}, + }, + { + name: "Test with invalid fields and no hidden fields for private API", + r: &Rest{ + isIgnoreUnknownFieldsEnabled: true, + }, + p: &prop{ + Fields: []string{ + "uuid", + "cloud_storage.stores.0.cloud_store.name", + "block_storage.primary.raid_type", + }, + IsPublic: false, + }, + expectedResult: []string{ + "uuid", + "cloud_storage.stores.0.cloud_store.name", + "block_storage.primary.raid_type", + }, + }, + { + name: "Test with valid fields and hidden fields for private API", + r: &Rest{ + isIgnoreUnknownFieldsEnabled: true, + }, + p: &prop{ + Fields: []string{ + "uuid", + "block_storage.primary.disk_type", + "block_storage.primary.raid_type", + }, + HiddenFields: []string{ + "hidden_field1", + "hidden_field2", + }, + IsPublic: true, + }, + expectedResult: []string{ + "uuid", + "block_storage.primary.disk_type", + "block_storage.primary.raid_type", + "hidden_field1", + "hidden_field2", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := tt.r.Fields(tt.p) + if !reflect.DeepEqual(result, tt.expectedResult) { + t.Errorf("Expected %v, got %v", tt.expectedResult, result) + } + }) + } +} diff --git a/cmd/collectors/rest/templating.go b/cmd/collectors/rest/templating.go index 7990bfba8..1f59e52f0 100644 --- a/cmd/collectors/rest/templating.go +++ b/cmd/collectors/rest/templating.go @@ -59,9 +59,9 @@ func (r *Rest) InitCache() error { // private end point do not support * as fields. We need to pass fields in endpoint query := r.Params.GetChildS("query") - r.Prop.APIType = "public" + r.Prop.IsPublic = true if query != nil { - r.Prop.APIType = checkQueryType(query.GetContentS()) + r.Prop.IsPublic = util.IsPublicAPI(query.GetContentS()) } r.ParseRestCounters(counters, r.Prop) @@ -195,29 +195,23 @@ func (r *Rest) ParseRestCounters(counter *node.Node, prop *prop) { prop.InstanceKeys = append(prop.InstanceKeys, instanceKeys[k]) } - if prop.APIType == "private" { - counterKey := make([]string, len(prop.Counters)) - i := 0 - for k := range prop.Counters { - counterKey[i] = k - i++ - } - prop.Fields = counterKey - if counter != nil { - if x := counter.GetChildS("filter"); x != nil { - prop.Filter = append(prop.Filter, x.GetAllChildContentS()...) - } + counterKey := make([]string, len(prop.Counters)) + i := 0 + for k := range prop.Counters { + counterKey[i] = k + i++ + } + prop.Fields = counterKey + if counter != nil { + if x := counter.GetChildS("filter"); x != nil { + prop.Filter = append(prop.Filter, x.GetAllChildContentS()...) } } - if prop.APIType == "public" { - prop.Fields = []string{"*"} + if prop.IsPublic { if counter != nil { if x := counter.GetChildS("hidden_fields"); x != nil { - prop.Fields = append(prop.Fields, x.GetAllChildContentS()...) - } - if x := counter.GetChildS("filter"); x != nil { - prop.Filter = append(prop.Filter, x.GetAllChildContentS()...) + prop.HiddenFields = append(prop.HiddenFields, x.GetAllChildContentS()...) } } } diff --git a/cmd/collectors/restperf/restperf.go b/cmd/collectors/restperf/restperf.go index 0cb67e7bc..45b141e49 100644 --- a/cmd/collectors/restperf/restperf.go +++ b/cmd/collectors/restperf/restperf.go @@ -677,7 +677,7 @@ func (r *RestPerf) PollData() (map[string]*matrix.Matrix, error) { href := rest.NewHrefBuilder(). APIPath(dataQuery). - Fields(r.Prop.Fields). + Fields([]string{"*"}). ReturnTimeout(r.Prop.ReturnTimeOut). Build() diff --git a/cmd/tools/rest/client.go b/cmd/tools/rest/client.go index 68803bdee..367cf4b58 100644 --- a/cmd/tools/rest/client.go +++ b/cmd/tools/rest/client.go @@ -305,8 +305,7 @@ func downloadSwagger(poller *conf.Poller, path string, url string, verbose bool) return n, nil } -func (c *Client) Init(retries int) error { - +func (c *Client) UpdateClusterInfo(retries int) error { var ( err error content []byte @@ -338,6 +337,10 @@ func (c *Client) Init(retries int) error { return err } +func (c *Client) Init(retries int) error { + return c.UpdateClusterInfo(retries) +} + func (c *Client) Cluster() Cluster { return c.cluster } diff --git a/cmd/tools/rest/href_builder.go b/cmd/tools/rest/href_builder.go index 374d4c0d9..b2f866c01 100644 --- a/cmd/tools/rest/href_builder.go +++ b/cmd/tools/rest/href_builder.go @@ -6,14 +6,15 @@ import ( ) type HrefBuilder struct { - apiPath string - fields string - counterSchema string - filter []string - queryFields string - queryValue string - maxRecords *int - returnTimeout *int + apiPath string + fields []string + counterSchema string + filter []string + queryFields string + queryValue string + maxRecords *int + returnTimeout *int + isIgnoreUnknownFieldsEnabled bool } func NewHrefBuilder() *HrefBuilder { @@ -26,7 +27,7 @@ func (b *HrefBuilder) APIPath(apiPath string) *HrefBuilder { } func (b *HrefBuilder) Fields(fields []string) *HrefBuilder { - b.fields = strings.Join(fields, ",") + b.fields = fields return b } @@ -60,6 +61,11 @@ func (b *HrefBuilder) ReturnTimeout(returnTimeout *int) *HrefBuilder { return b } +func (b *HrefBuilder) IsIgnoreUnknownFieldsEnabled(isIgnoreUnknownFieldsEnabled bool) *HrefBuilder { + b.isIgnoreUnknownFieldsEnabled = isIgnoreUnknownFieldsEnabled + return b +} + func (b *HrefBuilder) Build() string { href := strings.Builder{} if !strings.HasPrefix(b.apiPath, "api/") { @@ -68,7 +74,7 @@ func (b *HrefBuilder) Build() string { href.WriteString(b.apiPath) href.WriteString("?return_records=true") - addArg(&href, "&fields=", b.fields) + addArg(&href, "&fields=", strings.Join(b.fields, ",")) addArg(&href, "&counter_schemas=", b.counterSchema) for _, f := range b.filter { addArg(&href, "&", f) @@ -81,6 +87,9 @@ func (b *HrefBuilder) Build() string { if b.returnTimeout != nil { addArg(&href, "&return_timeout=", strconv.Itoa(*b.returnTimeout)) } + if b.isIgnoreUnknownFieldsEnabled { + addArg(&href, "&ignore_unknown_fields=", "true") + } return href.String() } diff --git a/conf/rest/9.12.0/namespace.yaml b/conf/rest/9.12.0/namespace.yaml index 221def2e5..2c17f69d3 100644 --- a/conf/rest/9.12.0/namespace.yaml +++ b/conf/rest/9.12.0/namespace.yaml @@ -16,6 +16,8 @@ counters: - space.block_size => block_size - space.size => size - space.used => size_used + - hidden_fields: + - subsystem_map plugins: - MetricAgent: diff --git a/conf/rest/default.yaml b/conf/rest/default.yaml index dc9c2f4b2..6676c43b8 100644 --- a/conf/rest/default.yaml +++ b/conf/rest/default.yaml @@ -2,6 +2,7 @@ collector: Rest schedule: + - counter: 24h # This handles cases such as cluster upgrades or collector cache updates. - data: 3m # See https://github.com/NetApp/harvest/blob/main/docs/architecture/rest-strategy.md diff --git a/pkg/util/util.go b/pkg/util/util.go index 4d3943d84..77bf0af13 100644 --- a/pkg/util/util.go +++ b/pkg/util/util.go @@ -7,6 +7,7 @@ package util import ( "errors" "fmt" + "github.com/hashicorp/go-version" "github.com/rs/zerolog" "github.com/shirou/gopsutil/v3/process" "golang.org/x/exp/maps" @@ -410,3 +411,26 @@ func GetURLWithoutHost(r *http.Request) string { } return urlWithoutHost } + +// VersionAtLeast checks if the provided currentVersion of the cluster +// is greater than or equal to the provided minimum version (minVersion). +func VersionAtLeast(currentVersion string, minVersion string) (bool, error) { + parsedClusterVersion, err := version.NewVersion(currentVersion) + if err != nil { + return false, fmt.Errorf("invalid current version: %w", err) + } + + minSupportedVersion, err := version.NewVersion(minVersion) + if err != nil { + return false, fmt.Errorf("invalid minimum version: %w", err) + } + + // Check if the current version is greater than or equal to the minimum version + // and return the result + return parsedClusterVersion.GreaterThanOrEqual(minSupportedVersion), nil +} + +// IsPublicAPI returns false if api endpoint has private keyword in it else true +func IsPublicAPI(query string) bool { + return !strings.Contains(query, "private") +} diff --git a/pkg/util/util_test.go b/pkg/util/util_test.go index a3ba1ca11..2fdd3ced0 100644 --- a/pkg/util/util_test.go +++ b/pkg/util/util_test.go @@ -123,3 +123,28 @@ func TestHasDuplicates(t *testing.T) { } } } + +func TestVersionAtLeast(t *testing.T) { + + testCases := []struct { + clusterVersion string + minVersion string + expected bool + }{ + {"9.11.1", "9.11.1", true}, // equal versions + {"9.11.1", "9.10.1", true}, // cluster version higher + {"9.10.1", "9.11.1", false}, // cluster version lower + {"9.11.1", "bad", false}, // bad minimum version + {"bad", "9.11.1", false}, // bad cluster version + } + + for _, tc := range testCases { + result, err := VersionAtLeast(tc.clusterVersion, tc.minVersion) + if err != nil && tc.expected { + t.Errorf("versionAtLeast(%q, %q) returned error: %v", tc.clusterVersion, tc.minVersion, err) + } + if result != tc.expected { + t.Errorf("versionAtLeast(%q, %q) = %v; want %v", tc.clusterVersion, tc.minVersion, result, tc.expected) + } + } +}