diff --git a/CHANGELOG.md b/CHANGELOG.md index e67e22731e..b84895e632 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,9 @@ +## [unreleased] + +### Bug Fixes + +1. [#6145](https://github.com/influxdata/chronograf/pull/6145): Show missing retention policies on the Databases page + ## v1.10.7 [2025-04-15] ### Bug Fixes diff --git a/influx/databases.go b/influx/databases.go index 17ab09fcec..10ff3f4a45 100644 --- a/influx/databases.go +++ b/influx/databases.go @@ -197,7 +197,7 @@ func (c *Client) showRetentionPolicies(ctx context.Context, db string) ([]chrono return nil, err } - return results.RetentionPolicies(), nil + return results.RetentionPolicies(c.Logger), nil } func (c *Client) showMeasurements(ctx context.Context, db string, limit, offset int) ([]chronograf.Measurement, error) { diff --git a/influx/permissions.go b/influx/permissions.go index ec0474407a..90ef01402a 100644 --- a/influx/permissions.go +++ b/influx/permissions.go @@ -45,7 +45,7 @@ func (c *Client) Permissions(context.Context) chronograf.Permissions { // showResults is used to deserialize InfluxQL SHOW commands type showResults []struct { Series []struct { - Values [][]interface{} `json:"values"` + Values []value `json:"values"` } `json:"series"` } @@ -93,37 +93,72 @@ func (r *showResults) Databases() []chronograf.Database { return res } -func (r *showResults) RetentionPolicies() []chronograf.RetentionPolicy { - res := []chronograf.RetentionPolicy{} +func (r *showResults) RetentionPolicies(logger chronograf.Logger) []chronograf.RetentionPolicy { + var res []chronograf.RetentionPolicy for _, u := range *r { for _, s := range u.Series { for _, v := range s.Values { - if name, ok := v[0].(string); !ok { - continue - } else if duration, ok := v[1].(string); !ok { - continue - } else if sduration, ok := v[2].(string); !ok { - continue - } else if replication, ok := v[3].(float64); !ok { - continue - } else if def, ok := v[4].(bool); !ok { - continue - } else { - d := chronograf.RetentionPolicy{ - Name: name, - Duration: duration, - ShardDuration: sduration, - Replication: int32(replication), - Default: def, + rp, err := parseRetentionPolicy(v) + if err != nil { + if logger != nil { + types := make([]string, len(v)) + for i, val := range v { + types[i] = fmt.Sprintf("%T", val) + } + logger. + WithField("values", fmt.Sprintf("%v", v)). + WithField("types", fmt.Sprintf("%v", types)). + WithField("error", err.Error()). + Error("Unsupported retention policy format") } - res = append(res, d) + continue } + res = append(res, rp) } } } return res } +// parseRetentionPolicy validates and parses a retention policy row +func parseRetentionPolicy(v []interface{}) (chronograf.RetentionPolicy, error) { + columns := len(v) + if columns < 5 { + return chronograf.RetentionPolicy{}, fmt.Errorf("insufficient columns: expected at least 5, got %d", columns) + } else if name, ok := v[0].(string); !ok { + return chronograf.RetentionPolicy{}, fmt.Errorf("column 0 (name) is not a string") + } else if duration, ok := v[1].(string); !ok { + return chronograf.RetentionPolicy{}, fmt.Errorf("column 1 (duration) is not a string") + } else if sduration, ok := v[2].(string); !ok { + return chronograf.RetentionPolicy{}, fmt.Errorf("column 2 (shardDuration) is not a string") + } else if replication, ok := v[3].(float64); !ok { + return chronograf.RetentionPolicy{}, fmt.Errorf("column 3 (replication) is not a float64") + } else { + var def bool + if columns == 5 { + // 5-column format: [name, duration, shardGroupDuration, replicaN, default] + if def, ok = v[4].(bool); !ok { + return chronograf.RetentionPolicy{}, fmt.Errorf("column 4 (default) is not a bool") + } + } else if columns == 7 { + // 7-column format: [name, duration, shardGroupDuration, replicaN, futureWriteLimit, pastWriteLimit, default] + if def, ok = v[6].(bool); !ok { + return chronograf.RetentionPolicy{}, fmt.Errorf("column 6 (default) is not a bool") + } + } else { + return chronograf.RetentionPolicy{}, fmt.Errorf("unexpected number of columns: %d", columns) + } + + return chronograf.RetentionPolicy{ + Name: name, + Duration: duration, + ShardDuration: sduration, + Replication: int32(replication), + Default: def, + }, nil + } +} + // Measurements converts SHOW MEASUREMENTS to chronograf Measurement func (r *showResults) Measurements() []chronograf.Measurement { res := []chronograf.Measurement{} diff --git a/influx/permissions_test.go b/influx/permissions_test.go index 9aca7aa74e..c9629d1446 100644 --- a/influx/permissions_test.go +++ b/influx/permissions_test.go @@ -2,6 +2,8 @@ package influx import ( "encoding/json" + "fmt" + "io" "reflect" "testing" @@ -302,6 +304,349 @@ func TestToRevoke(t *testing.T) { } } +// mockLoggerWithError is a simple logger that captures error messages passed via WithField +type mockLoggerWithError struct { + errorMsg string + fields map[string]interface{} +} + +func (m *mockLoggerWithError) Debug(...interface{}) {} +func (m *mockLoggerWithError) Info(...interface{}) {} +func (m *mockLoggerWithError) Error(_ ...interface{}) {} +func (m *mockLoggerWithError) WithField(key string, value interface{}) chronograf.Logger { + if m.fields == nil { + m.fields = make(map[string]interface{}) + } + m.fields[key] = value + if key == "error" { + m.errorMsg = fmt.Sprintf("%v", value) + } + return m +} +func (m *mockLoggerWithError) Writer() *io.PipeWriter { + _, w := io.Pipe() + return w +} + +func TestRetentionPolicies(t *testing.T) { + tests := []struct { + name string + input showResults + expected []chronograf.RetentionPolicy + expectedErr string + }{ + { + name: "5-column format with two retention policies", + input: showResults{ + { + Series: []struct { + Values []value `json:"values"` + }{ + { + Values: []value{ + { + "autogen", // name + "2160h0m0s", // duration + "168h0m0s", // shardGroupDuration + float64(3), // replicaN + true, // default + }, + { + "quarterly", // name + "1560h0m0s", // duration + "24h0m0s", // shardGroupDuration + float64(1), // replicaN + false, // default + }, + }, + }, + }, + }, + }, + expected: []chronograf.RetentionPolicy{ + { + Name: "autogen", + Duration: "2160h0m0s", + ShardDuration: "168h0m0s", + Replication: 3, + Default: true, + }, + { + Name: "quarterly", + Duration: "1560h0m0s", + ShardDuration: "24h0m0s", + Replication: 1, + Default: false, + }, + }, + }, + { + name: "7-column format with two retention policies", + input: showResults{ + { + Series: []struct { + Values []value `json:"values"` + }{ + { + Values: []value{ + { + "autogen", // name + "2160h0m0s", // duration + "168h0m0s", // shardGroupDuration + float64(3), // replicaN + "0s", // futureWriteLimit + "0s", // pastWriteLimit + true, // default + }, + { + "quarterly", // name + "1560h0m0s", // duration + "24h0m0s", // shardGroupDuration + float64(1), // replicaN + "1h", // futureWriteLimit + "30m", // pastWriteLimit + false, // default + }, + }, + }, + }, + }, + }, + expected: []chronograf.RetentionPolicy{ + { + Name: "autogen", + Duration: "2160h0m0s", + ShardDuration: "168h0m0s", + Replication: 3, + Default: true, + }, + { + Name: "quarterly", + Duration: "1560h0m0s", + ShardDuration: "24h0m0s", + Replication: 1, + Default: false, + }, + }, + }, + { + name: "empty input", + input: showResults{ + { + Series: []struct { + Values []value `json:"values"` + }{ + { + Values: []value{}, + }, + }, + }, + }, + expected: []chronograf.RetentionPolicy{}, + }, + { + name: "insufficient columns (3 columns)", + input: showResults{ + { + Series: []struct { + Values []value `json:"values"` + }{ + { + Values: []value{ + {"autogen", "2160h0m0s", "168h0m0s"}, // Only 3 columns + }, + }, + }, + }, + }, + expected: []chronograf.RetentionPolicy{}, + expectedErr: "insufficient columns: expected at least 5, got 3", + }, + { + name: "wrong type for name (int instead of string)", + input: showResults{ + { + Series: []struct { + Values []value `json:"values"` + }{ + { + Values: []value{ + {123, "2160h0m0s", "168h0m0s", float64(3), true}, + }, + }, + }, + }, + }, + expected: []chronograf.RetentionPolicy{}, + expectedErr: "column 0 (name) is not a string", + }, + { + name: "wrong type for duration (int instead of string)", + input: showResults{ + { + Series: []struct { + Values []value `json:"values"` + }{ + { + Values: []value{ + {"autogen", 2160, "168h0m0s", float64(3), true}, + }, + }, + }, + }, + }, + expected: []chronograf.RetentionPolicy{}, + expectedErr: "column 1 (duration) is not a string", + }, + { + name: "wrong type for replication (string instead of float64)", + input: showResults{ + { + Series: []struct { + Values []value `json:"values"` + }{ + { + Values: []value{ + {"autogen", "2160h0m0s", "168h0m0s", "3", true}, + }, + }, + }, + }, + }, + expected: []chronograf.RetentionPolicy{}, + expectedErr: "column 3 (replication) is not a float64", + }, + { + name: "wrong type for default in 5-column format (string instead of bool)", + input: showResults{ + { + Series: []struct { + Values []value `json:"values"` + }{ + { + Values: []value{ + {"autogen", "2160h0m0s", "168h0m0s", float64(3), "true"}, + }, + }, + }, + }, + }, + expected: []chronograf.RetentionPolicy{}, + expectedErr: "column 4 (default) is not a bool", + }, + { + name: "wrong type for default in 7-column format (string instead of bool)", + input: showResults{ + { + Series: []struct { + Values []value `json:"values"` + }{ + { + Values: []value{ + {"autogen", "2160h0m0s", "168h0m0s", float64(3), "0s", "0s", "true"}, + }, + }, + }, + }, + }, + expected: []chronograf.RetentionPolicy{}, + expectedErr: "column 6 (default) is not a bool", + }, + { + name: "invalid column count (6 columns)", + input: showResults{ + { + Series: []struct { + Values []value `json:"values"` + }{ + { + Values: []value{ + {"autogen", "2160h0m0s", "168h0m0s", float64(3), "0s", true}, + }, + }, + }, + }, + }, + expected: []chronograf.RetentionPolicy{}, + expectedErr: "unexpected number of columns: 6", + }, + { + name: "mixed valid and invalid entries", + input: showResults{ + { + Series: []struct { + Values []value `json:"values"` + }{ + { + Values: []value{ + {"autogen", "2160h0m0s", "168h0m0s", float64(3), true}, // valid + {"invalid", "2160h0m0s", "168h0m0s"}, // insufficient columns + {"quarterly", "1560h0m0s", "24h0m0s", float64(1), false}, // valid + }, + }, + }, + }, + }, + expected: []chronograf.RetentionPolicy{ + { + Name: "autogen", + Duration: "2160h0m0s", + ShardDuration: "168h0m0s", + Replication: 3, + Default: true, + }, + { + Name: "quarterly", + Duration: "1560h0m0s", + ShardDuration: "24h0m0s", + Replication: 1, + Default: false, + }, + }, + expectedErr: "insufficient columns: expected at least 5, got 3", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Use mock logger to capture error messages + logger := &mockLoggerWithError{} + result := tt.input.RetentionPolicies(logger) + + // Check the returned policies match expected + if !equalRetentionPolicies(result, tt.expected) { + t.Errorf("RetentionPolicies() = %v, want %v", result, tt.expected) + } + + // Check the error message if one is expected + if tt.expectedErr != "" { + if logger.errorMsg != tt.expectedErr { + t.Errorf("RetentionPolicies() error = %v, want %v", logger.errorMsg, tt.expectedErr) + } + } else if logger.errorMsg != "" { + t.Errorf("RetentionPolicies() unexpected error = %v", logger.errorMsg) + } + }) + } +} + +// equalRetentionPolicies compares two slices of RetentionPolicy for equality +func equalRetentionPolicies(a, b []chronograf.RetentionPolicy) bool { + if len(a) != len(b) { + return false + } + for i := range a { + if a[i].Name != b[i].Name || + a[i].Duration != b[i].Duration || + a[i].ShardDuration != b[i].ShardDuration || + a[i].Replication != b[i].Replication || + a[i].Default != b[i].Default { + return false + } + } + return true +} + func Test_showResults_Users(t *testing.T) { t.Parallel() tests := []struct {