mirror of https://github.com/grafana/loki
Add delete api validations (#6860)
* break out the middleware * Add validation to the API - Time must be in RFC3339 or a 10-digit-unix-seconds timestamp - Start time must always exist * lint * docs * improve tests * cleanup * cleanup * access runtime config via function on validationpull/6872/head
Travis Patterson
3 years ago
committed by
GitHub
parent
b114dc93d6
commit
983ab80e7c
@ -1,132 +1,134 @@ |
|||||||
package deletion |
package deletion |
||||||
|
|
||||||
import ( |
import ( |
||||||
|
"context" |
||||||
|
"fmt" |
||||||
"net/http" |
"net/http" |
||||||
"net/http/httptest" |
"net/http/httptest" |
||||||
"path/filepath" |
"strings" |
||||||
"testing" |
"testing" |
||||||
"time" |
"time" |
||||||
|
|
||||||
|
"github.com/pkg/errors" |
||||||
|
|
||||||
"github.com/prometheus/common/model" |
"github.com/prometheus/common/model" |
||||||
"github.com/stretchr/testify/require" |
"github.com/stretchr/testify/require" |
||||||
"github.com/weaveworks/common/user" |
|
||||||
|
|
||||||
"github.com/grafana/loki/pkg/storage/chunk/client/local" |
"github.com/grafana/loki/pkg/util" |
||||||
"github.com/grafana/loki/pkg/storage/stores/indexshipper/storage" |
|
||||||
"github.com/grafana/loki/pkg/validation" |
"github.com/weaveworks/common/user" |
||||||
) |
) |
||||||
|
|
||||||
func TestDeleteRequestHandlerDeletionMiddleware(t *testing.T) { |
func TestAddDeleteRequestHandler(t *testing.T) { |
||||||
// build the store
|
t.Run("it adds the delete request to the store", func(t *testing.T) { |
||||||
tempDir := t.TempDir() |
store := &mockDeleteRequestsStore{} |
||||||
|
h := NewDeleteRequestHandler(store, time.Second, nil) |
||||||
|
|
||||||
workingDir := filepath.Join(tempDir, "working-dir") |
req := buildRequest("org-id", `{foo="bar"}`, "0000000000", "0000000001") |
||||||
objectStorePath := filepath.Join(tempDir, "object-store") |
|
||||||
|
|
||||||
objectClient, err := local.NewFSObjectClient(local.FSConfig{ |
w := httptest.NewRecorder() |
||||||
Directory: objectStorePath, |
h.AddDeleteRequestHandler(w, req) |
||||||
}) |
|
||||||
require.NoError(t, err) |
|
||||||
testDeleteRequestsStore, err := NewDeleteStore(workingDir, storage.NewIndexStorageClient(objectClient, "")) |
|
||||||
require.NoError(t, err) |
|
||||||
|
|
||||||
// limits
|
|
||||||
fl := &fakeLimits{ |
|
||||||
defaultLimit: retentionLimit{ |
|
||||||
compactorDeletionEnabled: "disabled", |
|
||||||
}, |
|
||||||
perTenant: map[string]retentionLimit{ |
|
||||||
"1": {compactorDeletionEnabled: "filter-only"}, |
|
||||||
"2": {compactorDeletionEnabled: "disabled"}, |
|
||||||
}, |
|
||||||
} |
|
||||||
|
|
||||||
// Setup handler
|
require.Equal(t, w.Code, http.StatusNoContent) |
||||||
drh := NewDeleteRequestHandler(testDeleteRequestsStore, fl, nil) |
|
||||||
middle := drh.deletionMiddleware(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})) |
|
||||||
|
|
||||||
// User that has deletion enabled
|
require.Equal(t, "org-id", store.addedUser) |
||||||
req := httptest.NewRequest(http.MethodGet, "http://www.your-domain.com", nil) |
require.Equal(t, `{foo="bar"}`, store.addedQuery) |
||||||
req = req.WithContext(user.InjectOrgID(req.Context(), "1")) |
require.Equal(t, toTime("0000000000"), store.addedStartTime) |
||||||
|
require.Equal(t, toTime("0000000001"), store.addedEndTime) |
||||||
res := httptest.NewRecorder() |
}) |
||||||
middle.ServeHTTP(res, req) |
|
||||||
|
|
||||||
require.Equal(t, http.StatusOK, res.Result().StatusCode) |
|
||||||
|
|
||||||
// User that does not have deletion enabled
|
t.Run("it works with RFC3339", func(t *testing.T) { |
||||||
req = httptest.NewRequest(http.MethodGet, "http://www.your-domain.com", nil) |
store := &mockDeleteRequestsStore{} |
||||||
req = req.WithContext(user.InjectOrgID(req.Context(), "2")) |
h := NewDeleteRequestHandler(store, time.Second, nil) |
||||||
|
|
||||||
res = httptest.NewRecorder() |
req := buildRequest("org-id", `{foo="bar"}`, "2006-01-02T15:04:05Z", "2006-01-03T15:04:05Z") |
||||||
middle.ServeHTTP(res, req) |
|
||||||
|
|
||||||
require.Equal(t, http.StatusForbidden, res.Result().StatusCode) |
w := httptest.NewRecorder() |
||||||
|
h.AddDeleteRequestHandler(w, req) |
||||||
|
|
||||||
// User without override, this should use the default value which is false
|
require.Equal(t, w.Code, http.StatusNoContent) |
||||||
req = httptest.NewRequest(http.MethodGet, "http://www.your-domain.com", nil) |
|
||||||
req = req.WithContext(user.InjectOrgID(req.Context(), "3")) |
|
||||||
|
|
||||||
res = httptest.NewRecorder() |
require.Equal(t, "org-id", store.addedUser) |
||||||
middle.ServeHTTP(res, req) |
require.Equal(t, `{foo="bar"}`, store.addedQuery) |
||||||
|
require.Equal(t, toTime("1136214245"), store.addedStartTime) |
||||||
|
require.Equal(t, toTime("1136300645"), store.addedEndTime) |
||||||
|
}) |
||||||
|
|
||||||
require.Equal(t, http.StatusForbidden, res.Result().StatusCode) |
t.Run("it fills in end time if blank", func(t *testing.T) { |
||||||
|
store := &mockDeleteRequestsStore{} |
||||||
|
h := NewDeleteRequestHandler(store, time.Second, nil) |
||||||
|
|
||||||
// User without override, after the default value is set to true
|
req := buildRequest("org-id", `{foo="bar"}`, "0000000000", "") |
||||||
fl.defaultLimit.compactorDeletionEnabled = "filter-and-delete" |
|
||||||
|
|
||||||
req = httptest.NewRequest(http.MethodGet, "http://www.your-domain.com", nil) |
w := httptest.NewRecorder() |
||||||
req = req.WithContext(user.InjectOrgID(req.Context(), "3")) |
h.AddDeleteRequestHandler(w, req) |
||||||
|
|
||||||
res = httptest.NewRecorder() |
require.Equal(t, w.Code, http.StatusNoContent) |
||||||
middle.ServeHTTP(res, req) |
|
||||||
|
|
||||||
require.Equal(t, http.StatusOK, res.Result().StatusCode) |
require.Equal(t, "org-id", store.addedUser) |
||||||
|
require.Equal(t, `{foo="bar"}`, store.addedQuery) |
||||||
|
require.Equal(t, toTime("0000000000"), store.addedStartTime) |
||||||
|
require.InDelta(t, int64(model.Now()), int64(store.addedEndTime), 1000) |
||||||
|
}) |
||||||
|
|
||||||
// User header is not given
|
t.Run("it returns 500 when the delete store errors", func(t *testing.T) { |
||||||
req = httptest.NewRequest(http.MethodGet, "http://www.your-domain.com", nil) |
store := &mockDeleteRequestsStore{addErr: errors.New("something bad")} |
||||||
|
h := NewDeleteRequestHandler(store, time.Second, nil) |
||||||
|
|
||||||
res = httptest.NewRecorder() |
req := buildRequest("org-id", `{foo="bar"}`, "0000000000", "0000000001") |
||||||
middle.ServeHTTP(res, req) |
|
||||||
|
|
||||||
require.Equal(t, http.StatusBadRequest, res.Result().StatusCode) |
w := httptest.NewRecorder() |
||||||
} |
h.AddDeleteRequestHandler(w, req) |
||||||
|
require.Equal(t, w.Code, http.StatusInternalServerError) |
||||||
|
}) |
||||||
|
|
||||||
type retentionLimit struct { |
t.Run("Validation", func(t *testing.T) { |
||||||
compactorDeletionEnabled string |
h := NewDeleteRequestHandler(&mockDeleteRequestsStore{}, time.Second, nil) |
||||||
retentionPeriod time.Duration |
|
||||||
streamRetention []validation.StreamRetention |
for _, tc := range []struct { |
||||||
|
orgID, query, startTime, endTime, error string |
||||||
|
}{ |
||||||
|
{"", `{foo="bar"}`, "0000000000", "0000000001", "no org id\n"}, |
||||||
|
{"org-id", "", "0000000000", "0000000001", "query not set\n"}, |
||||||
|
{"org-id", `not a query`, "0000000000", "0000000001", "invalid query expression\n"}, |
||||||
|
{"org-id", `{foo="bar"}`, "", "0000000001", "start time not set\n"}, |
||||||
|
{"org-id", `{foo="bar"}`, "0000000000000", "0000000001", "invalid start time: require unix seconds or RFC3339 format\n"}, |
||||||
|
{"org-id", `{foo="bar"}`, "0000000000", "0000000000001", "invalid end time: require unix seconds or RFC3339 format\n"}, |
||||||
|
{"org-id", `{foo="bar"}`, "0000000000", fmt.Sprint(time.Now().Add(time.Hour).Unix())[:10], "deletes in the future are not allowed\n"}, |
||||||
|
{"org-id", `{foo="bar"}`, "0000000001", "0000000000", "start time can't be greater than end time\n"}, |
||||||
|
} { |
||||||
|
t.Run(strings.TrimSpace(tc.error), func(t *testing.T) { |
||||||
|
req := buildRequest(tc.orgID, tc.query, tc.startTime, tc.endTime) |
||||||
|
|
||||||
|
w := httptest.NewRecorder() |
||||||
|
h.AddDeleteRequestHandler(w, req) |
||||||
|
|
||||||
|
require.Equal(t, w.Code, http.StatusBadRequest) |
||||||
|
require.Equal(t, w.Body.String(), tc.error) |
||||||
|
}) |
||||||
|
} |
||||||
|
}) |
||||||
} |
} |
||||||
|
|
||||||
func (r retentionLimit) convertToValidationLimit() *validation.Limits { |
func buildRequest(orgID, query, start, end string) *http.Request { |
||||||
return &validation.Limits{ |
var req *http.Request |
||||||
DeletionMode: r.compactorDeletionEnabled, |
if orgID == "" { |
||||||
RetentionPeriod: model.Duration(r.retentionPeriod), |
req, _ = http.NewRequest(http.MethodGet, "", nil) |
||||||
StreamRetention: r.streamRetention, |
} else { |
||||||
|
ctx := user.InjectOrgID(context.Background(), orgID) |
||||||
|
req, _ = http.NewRequestWithContext(ctx, http.MethodGet, "", nil) |
||||||
} |
} |
||||||
} |
|
||||||
|
|
||||||
type fakeLimits struct { |
|
||||||
defaultLimit retentionLimit |
|
||||||
perTenant map[string]retentionLimit |
|
||||||
} |
|
||||||
|
|
||||||
func (f fakeLimits) RetentionPeriod(userID string) time.Duration { |
q := req.URL.Query() |
||||||
return f.perTenant[userID].retentionPeriod |
q.Set("query", query) |
||||||
} |
q.Set("start", start) |
||||||
|
q.Set("end", end) |
||||||
|
req.URL.RawQuery = q.Encode() |
||||||
|
|
||||||
func (f fakeLimits) StreamRetention(userID string) []validation.StreamRetention { |
return req |
||||||
return f.perTenant[userID].streamRetention |
|
||||||
} |
} |
||||||
|
|
||||||
func (f fakeLimits) DefaultLimits() *validation.Limits { |
func toTime(t string) model.Time { |
||||||
return f.defaultLimit.convertToValidationLimit() |
modelTime, _ := util.ParseTime(t) |
||||||
} |
return model.Time(modelTime) |
||||||
|
|
||||||
func (f fakeLimits) AllByUserID() map[string]*validation.Limits { |
|
||||||
res := make(map[string]*validation.Limits) |
|
||||||
for userID, ret := range f.perTenant { |
|
||||||
res[userID] = ret.convertToValidationLimit() |
|
||||||
} |
|
||||||
return res |
|
||||||
} |
} |
||||||
|
|||||||
@ -0,0 +1,31 @@ |
|||||||
|
package deletion |
||||||
|
|
||||||
|
import ( |
||||||
|
"net/http" |
||||||
|
|
||||||
|
"github.com/grafana/dskit/tenant" |
||||||
|
) |
||||||
|
|
||||||
|
func TenantMiddleware(limits Limits, next http.Handler) http.Handler { |
||||||
|
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { |
||||||
|
ctx := r.Context() |
||||||
|
userID, err := tenant.TenantID(ctx) |
||||||
|
if err != nil { |
||||||
|
http.Error(w, err.Error(), http.StatusBadRequest) |
||||||
|
return |
||||||
|
} |
||||||
|
|
||||||
|
hasDelete, err := validDeletionLimit(limits, userID) |
||||||
|
if err != nil { |
||||||
|
http.Error(w, err.Error(), http.StatusBadRequest) |
||||||
|
return |
||||||
|
} |
||||||
|
|
||||||
|
if !hasDelete { |
||||||
|
http.Error(w, deletionNotAvailableMsg, http.StatusForbidden) |
||||||
|
return |
||||||
|
} |
||||||
|
|
||||||
|
next.ServeHTTP(w, r) |
||||||
|
}) |
||||||
|
} |
||||||
@ -0,0 +1,83 @@ |
|||||||
|
package deletion |
||||||
|
|
||||||
|
import ( |
||||||
|
"net/http" |
||||||
|
"net/http/httptest" |
||||||
|
"testing" |
||||||
|
"time" |
||||||
|
|
||||||
|
"github.com/grafana/loki/pkg/storage/stores/indexshipper/compactor/retention" |
||||||
|
|
||||||
|
"github.com/prometheus/common/model" |
||||||
|
"github.com/stretchr/testify/require" |
||||||
|
"github.com/weaveworks/common/user" |
||||||
|
|
||||||
|
"github.com/grafana/loki/pkg/validation" |
||||||
|
) |
||||||
|
|
||||||
|
func TestDeleteRequestHandlerDeletionMiddleware(t *testing.T) { |
||||||
|
fl := &fakeLimits{ |
||||||
|
limits: map[string]string{ |
||||||
|
"1": "filter-only", |
||||||
|
"2": "disabled", |
||||||
|
}, |
||||||
|
} |
||||||
|
|
||||||
|
// Setup handler
|
||||||
|
middle := TenantMiddleware(fl, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})) |
||||||
|
|
||||||
|
// User that has deletion enabled
|
||||||
|
req := httptest.NewRequest(http.MethodGet, "http://www.your-domain.com", nil) |
||||||
|
req = req.WithContext(user.InjectOrgID(req.Context(), "1")) |
||||||
|
|
||||||
|
res := httptest.NewRecorder() |
||||||
|
middle.ServeHTTP(res, req) |
||||||
|
|
||||||
|
require.Equal(t, http.StatusOK, res.Result().StatusCode) |
||||||
|
|
||||||
|
// User that does not have deletion enabled
|
||||||
|
req = httptest.NewRequest(http.MethodGet, "http://www.your-domain.com", nil) |
||||||
|
req = req.WithContext(user.InjectOrgID(req.Context(), "2")) |
||||||
|
|
||||||
|
res = httptest.NewRecorder() |
||||||
|
middle.ServeHTTP(res, req) |
||||||
|
|
||||||
|
require.Equal(t, http.StatusForbidden, res.Result().StatusCode) |
||||||
|
|
||||||
|
// User header is not given
|
||||||
|
req = httptest.NewRequest(http.MethodGet, "http://www.your-domain.com", nil) |
||||||
|
|
||||||
|
res = httptest.NewRecorder() |
||||||
|
middle.ServeHTTP(res, req) |
||||||
|
|
||||||
|
require.Equal(t, http.StatusBadRequest, res.Result().StatusCode) |
||||||
|
} |
||||||
|
|
||||||
|
type retentionLimit struct { |
||||||
|
compactorDeletionEnabled string |
||||||
|
retentionPeriod time.Duration |
||||||
|
streamRetention []validation.StreamRetention |
||||||
|
} |
||||||
|
|
||||||
|
func (r retentionLimit) convertToValidationLimit() *validation.Limits { |
||||||
|
return &validation.Limits{ |
||||||
|
DeletionMode: r.compactorDeletionEnabled, |
||||||
|
RetentionPeriod: model.Duration(r.retentionPeriod), |
||||||
|
StreamRetention: r.streamRetention, |
||||||
|
} |
||||||
|
} |
||||||
|
|
||||||
|
type fakeLimits struct { |
||||||
|
retention.Limits |
||||||
|
|
||||||
|
limits map[string]string |
||||||
|
mode string |
||||||
|
} |
||||||
|
|
||||||
|
func (f *fakeLimits) DeletionMode(userID string) string { |
||||||
|
if f.mode != "" { |
||||||
|
return f.mode |
||||||
|
} |
||||||
|
|
||||||
|
return f.limits[userID] |
||||||
|
} |
||||||
Loading…
Reference in new issue