Skip to content

Commit

Permalink
query rejection - address comments
Browse files Browse the repository at this point in the history 
Signed-off-by: Erlan Zholdubai uulu <erlanz@amazon.com>
  • Loading branch information
@erlan-z
erlan-z committed Jul 3, 2024
1 parent 572c8ca commit c0d9115
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 17 deletions.
17 changes: 11 additions & 6 deletions docs/configuration/config-file-reference.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5393,15 +5393,20 @@ time_window:
# checked.
[end: <int> | default = 0]
# Query time range should be within this limit to match. If not set, it won't be
# checked.
# Query time range should be within this limit to match. Depending on where it
# was used, in most of the use-cases, either min or max value will be used. If
# not set, it won't be checked.
time_range_limit:
# Query time range should be above or equal to this value to match. If set to
# 0, it won't be checked.
# This will be duration (12h, 1d, 15d etc.). Query time range should be above
# or equal to this value to match. Ex: if this value is 20d, then queries
# whose range is bigger than or equal to 20d will match. If set to 0, it won't
# be checked.
[min: <int> | default = 0]
# Query time range should be below or equal to this value to match. If set to
# 0, it won't be checked.
# This will be duration (12h, 1d, 15d etc.). Query time range should be below
# or equal to this value to match. Ex: if this value is 24h, then queries
# whose range is smaller than or equal to 24h will match.If set to 0, it won't
# be checked.
[max: <int> | default = 0]
# If query step provided should be within this limit to match. If not set, it
Expand Down
16 changes: 8 additions & 8 deletions integration/query_frontend_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -662,13 +662,13 @@ func TestQueryFrontendQueryRejection(t *testing.T) {
now := time.Now()
// We expect request to be rejected, as it matches query_attribute of query_rejection (contains rate, contains dashboard header dash123). step limit is ignored for instant queries
// Query shouldn't be checked against attributes that is not provided in query_attribute config(time_window, time_range_limit, user_agent_regex, panel_id)
resp, body, err := c.QueryRaw(`min_over_time( rate(http_requests_total[5m])[30m:5s] )`, now, map[string]string{"X-Dashboard-Uid": "dash123", "User-Agent": "grafana-agent/v0.19.0"})
resp, body, err := c.QueryRaw(`min_over_time( rate(http_requests_total[5m])[30m:5s] )`, now, map[string]string{"X-Dashboard-Uid": "dash123", "User-Agent": "grafana"})
require.NoError(t, err)
require.Equal(t, http.StatusUnprocessableEntity, resp.StatusCode)
require.Contains(t, string(body), tripperware.QueryRejectErrorMessage)

// We expect request not to be rejected, as it doesn't match api_type
resp, body, err = c.QueryRangeRaw(`min_over_time( rate(http_requests_total[5m])[30m:5s] )`, now.Add(-11*time.Hour), now.Add(-8*time.Hour), 25*time.Minute, map[string]string{"X-Dashboard-Uid": "dash123", "User-Agent": "grafana-agent/v0.19.0"})
resp, body, err = c.QueryRangeRaw(`min_over_time( rate(http_requests_total[5m])[30m:5s] )`, now.Add(-11*time.Hour), now.Add(-8*time.Hour), 25*time.Minute, map[string]string{"X-Dashboard-Uid": "dash123", "User-Agent": "grafana"})
require.NoError(t, err)
require.Equal(t, http.StatusOK, resp.StatusCode)
require.NotContains(t, string(body), tripperware.QueryRejectErrorMessage)
Expand All @@ -695,31 +695,31 @@ func TestQueryFrontendQueryRejection(t *testing.T) {
time.Sleep(2 * time.Second)

// We expect request to be rejected, as it matches query_attribute (contains 'rate', within time_window(11h-8h), within time range(3h), within step limit(25m>22m), contains dashboard header(dash123) and user-agent matches regex).
resp, body, err = c.QueryRangeRaw(`rate(test[1m])`, now.Add(-11*time.Hour), now.Add(-8*time.Hour), 25*time.Minute, map[string]string{"X-Dashboard-Uid": "dash123", "User-Agent": "grafana-agent/v0.19.0"})
resp, body, err = c.QueryRangeRaw(`rate(test[1m])`, now.Add(-11*time.Hour), now.Add(-8*time.Hour), 25*time.Minute, map[string]string{"X-Dashboard-Uid": "dash123", "User-Agent": "grafana"})
require.NoError(t, err)
require.Equal(t, http.StatusUnprocessableEntity, resp.StatusCode)
require.Contains(t, string(body), tripperware.QueryRejectErrorMessage)

// We expect request not to be rejected, as it doesn't match query step limit (min is 22m, and actual step is 20m)
resp, body, err = c.QueryRangeRaw(`rate(test[1m])`, now.Add(-11*time.Hour), now.Add(-8*time.Hour), 20*time.Minute, map[string]string{"X-Dashboard-Uid": "dash123", "User-Agent": "grafana-agent/v0.19.0"})
resp, body, err = c.QueryRangeRaw(`rate(test[1m])`, now.Add(-11*time.Hour), now.Add(-8*time.Hour), 20*time.Minute, map[string]string{"X-Dashboard-Uid": "dash123", "User-Agent": "grafana"})
require.NoError(t, err)
require.Equal(t, http.StatusOK, resp.StatusCode)
require.NotContains(t, string(body), tripperware.QueryRejectErrorMessage)

// We expect request not to be rejected, as it goes beyond time_window(-15h is outside of 12h-0h window)
resp, body, err = c.QueryRangeRaw(`rate(test[1m])`, now.Add(-15*time.Hour), now.Add(-8*time.Hour), 25*time.Minute, map[string]string{"X-Dashboard-Uid": "dash123", "User-Agent": "grafana-agent/v0.19.0"})
resp, body, err = c.QueryRangeRaw(`rate(test[1m])`, now.Add(-15*time.Hour), now.Add(-8*time.Hour), 25*time.Minute, map[string]string{"X-Dashboard-Uid": "dash123", "User-Agent": "grafana"})
require.NoError(t, err)
require.Equal(t, http.StatusOK, resp.StatusCode)
require.NotContains(t, string(body), tripperware.QueryRejectErrorMessage)

// We expect request not to be rejected as it goes beyond time-range(9h is bigger than max time range of 6h)
resp, body, err = c.QueryRangeRaw(`rate(test[1m])`, now.Add(-11*time.Hour), now.Add(-2*time.Hour), 25*time.Minute, map[string]string{"X-Dashboard-Uid": "dash123", "User-Agent": "grafana-agent/v0.19.0"})
resp, body, err = c.QueryRangeRaw(`rate(test[1m])`, now.Add(-11*time.Hour), now.Add(-2*time.Hour), 25*time.Minute, map[string]string{"X-Dashboard-Uid": "dash123", "User-Agent": "grafana"})
require.NoError(t, err)
require.Equal(t, http.StatusOK, resp.StatusCode)
require.NotContains(t, string(body), tripperware.QueryRejectErrorMessage)

// We expect request not to be rejected, as it doesn't match regex (doesn't contain 'rate')
resp, body, err = c.QueryRangeRaw(`increase(test[1m])`, now.Add(-11*time.Hour), now.Add(-8*time.Hour), 25*time.Minute, map[string]string{"X-Dashboard-Uid": "dash123", "User-Agent": "grafana-agent/v0.19.0"})
resp, body, err = c.QueryRangeRaw(`increase(test[1m])`, now.Add(-11*time.Hour), now.Add(-8*time.Hour), 25*time.Minute, map[string]string{"X-Dashboard-Uid": "dash123", "User-Agent": "grafana"})
require.NoError(t, err)
require.Equal(t, http.StatusOK, resp.StatusCode)
require.NotContains(t, string(body), tripperware.QueryRejectErrorMessage)
Expand All @@ -731,7 +731,7 @@ func TestQueryFrontendQueryRejection(t *testing.T) {
require.NotContains(t, string(body), tripperware.QueryRejectErrorMessage)

// We expect request not to be rejected, as it doesn't match grafana dashboard uid ('dash123' != 'new-dashboard')
resp, body, err = c.QueryRangeRaw(`rate(test[1m])`, now.Add(-11*time.Hour), now.Add(-8*time.Hour), 25*time.Minute, map[string]string{"X-Dashboard-Uid": "new-dashboard", "User-Agent": "grafana-agent/v0.19.0"})
resp, body, err = c.QueryRangeRaw(`rate(test[1m])`, now.Add(-11*time.Hour), now.Add(-8*time.Hour), 25*time.Minute, map[string]string{"X-Dashboard-Uid": "new-dashboard", "User-Agent": "grafana"})
require.NoError(t, err)
require.Equal(t, http.StatusOK, resp.StatusCode)
require.NotContains(t, string(body), tripperware.QueryRejectErrorMessage)
Expand Down
6 changes: 3 additions & 3 deletions pkg/util/validation/limits.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ type QueryAttribute struct {
ApiType string `yaml:"api_type" json:"api_type" doc:"nocli|description=API type for the query. Should be one of the query, query_range, series, labels, label_values. If not set, it won't be checked."`
Regex string `yaml:"regex" json:"regex" doc:"nocli|description=Regex that the query string (or at least one of the matchers in metadata query) should match. If not set, it won't be checked."`
TimeWindow TimeWindow `yaml:"time_window" json:"time_window" doc:"nocli|description=Overall data select time window (including range selectors, modifiers and lookback delta) that the query should be within. If not set, it won't be checked."`
TimeRangeLimit TimeRangeLimit `yaml:"time_range_limit" json:"time_range_limit" doc:"nocli|description=Query time range should be within this limit to match. If not set, it won't be checked."`
TimeRangeLimit TimeRangeLimit `yaml:"time_range_limit" json:"time_range_limit" doc:"nocli|description=Query time range should be within this limit to match. Depending on where it was used, in most of the use-cases, either min or max value will be used. If not set, it won't be checked."`
QueryStepLimit QueryStepLimit `yaml:"query_step_limit" json:"query_step_limit" doc:"nocli|description=If query step provided should be within this limit to match. If not set, it won't be checked. This property only applied to range queries and ignored for other types of queries."`
UserAgentRegex string `yaml:"user_agent_regex" json:"user_agent_regex" doc:"nocli|description=Regex that User-Agent header of the request should match. If not set, it won't be checked."`
DashboardUID string `yaml:"dashboard_uid" json:"dashboard_uid" doc:"nocli|description=Grafana includes X-Dashboard-Uid header in query requests. If this field is provided then X-Dashboard-Uid header of request should match this value. If not set, it won't be checked. This property won't be applied to metadata queries."`
Expand All @@ -90,8 +90,8 @@ type TimeWindow struct {
}

type TimeRangeLimit struct {
Min model.Duration `yaml:"min" json:"min" doc:"nocli|description=Query time range should be above or equal to this value to match. If set to 0, it won't be checked.|default=0"`
Max model.Duration `yaml:"max" json:"max" doc:"nocli|description=Query time range should be below or equal to this value to match. If set to 0, it won't be checked.|default=0"`
Min model.Duration `yaml:"min" json:"min" doc:"nocli|description=This will be duration (12h, 1d, 15d etc.). Query time range should be above or equal to this value to match. Ex: if this value is 20d, then queries whose range is bigger than or equal to 20d will match. If set to 0, it won't be checked.|default=0"`
Max model.Duration `yaml:"max" json:"max" doc:"nocli|description=This will be duration (12h, 1d, 15d etc.). Query time range should be below or equal to this value to match. Ex: if this value is 24h, then queries whose range is smaller than or equal to 24h will match.If set to 0, it won't be checked.|default=0"`
}

type QueryStepLimit struct {
Expand Down

0 comments on commit c0d9115

Please sign in to comment.