-
Notifications
You must be signed in to change notification settings - Fork 63
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Rules API: Add e2e test #198
Changes from 13 commits
e392176
23cf4d3
9b1403a
d536281
3907da5
9e4bc8d
9afa2be
58502d2
6a91a73
e0353b3
be5c99f
b51cc81
063a88e
7d50a67
0b60005
c27ab99
ccd9a59
ec05d23
79a0937
01485e6
7b543e0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,170 @@ | ||
// +build integration | ||
|
||
package e2e | ||
|
||
import ( | ||
"bytes" | ||
"io/ioutil" | ||
"net/http" | ||
"testing" | ||
|
||
"github.com/efficientgo/e2e" | ||
"github.com/efficientgo/tools/core/pkg/testutil" | ||
) | ||
|
||
func TestRulesAPI(t *testing.T) { | ||
t.Parallel() | ||
|
||
e, err := e2e.NewDockerEnvironment(envRulesAPIName) | ||
testutil.Ok(t, err) | ||
t.Cleanup(e.Close) | ||
|
||
prepareConfigsAndCerts(t, rules, e) | ||
_, token, rateLimiterAddr := startBaseServices(t, e, rules) | ||
rulesEndpoint := startServicesForRules(t, e) | ||
|
||
api, err := newObservatoriumAPIService( | ||
e, | ||
withRulesEndpoint("http://"+rulesEndpoint), | ||
withRateLimiter(rateLimiterAddr), | ||
) | ||
testutil.Ok(t, err) | ||
testutil.Ok(t, e2e.StartAndWaitReady(api)) | ||
|
||
rulesEndpointURL := "https://" + api.Endpoint("https") + "/api/metrics/v1/test-oidc/api/v1/rules/raw" | ||
tr := &http.Transport{ | ||
TLSClientConfig: getTLSClientConfig(t, e), | ||
} | ||
|
||
client := &http.Client{ | ||
Transport: &tokenRoundTripper{rt: tr, token: token}, | ||
} | ||
|
||
t.Run("read-write-recording-rules", func(t *testing.T) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry to be a pedant with this - but the actual flow is more There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. actually for this test case is more like |
||
// Try to list rules | ||
r, err := http.NewRequest( | ||
http.MethodGet, | ||
rulesEndpointURL, | ||
nil, | ||
) | ||
testutil.Ok(t, err) | ||
|
||
res, err := client.Do(r) | ||
testutil.Ok(t, err) | ||
testutil.Equals(t, http.StatusNotFound, res.StatusCode) | ||
|
||
// Set a file containing a recording rule | ||
recordingRule := []byte(recordingRuleYamlTpl) | ||
r, err = http.NewRequest( | ||
http.MethodPut, | ||
rulesEndpointURL, | ||
bytes.NewReader(recordingRule), | ||
) | ||
testutil.Ok(t, err) | ||
|
||
res, err = client.Do(r) | ||
testutil.Ok(t, err) | ||
testutil.Equals(t, http.StatusOK, res.StatusCode) | ||
|
||
// Check if recording rule is listed | ||
r, err = http.NewRequest( | ||
http.MethodGet, | ||
rulesEndpointURL, | ||
nil, | ||
) | ||
testutil.Ok(t, err) | ||
|
||
res, err = client.Do(r) | ||
defer res.Body.Close() | ||
|
||
testutil.Ok(t, err) | ||
testutil.Equals(t, http.StatusOK, res.StatusCode) | ||
|
||
body, err := ioutil.ReadAll(res.Body) | ||
bodyStr := string(body) | ||
|
||
assertResponse(t, bodyStr, "sum by (job) (http_inprogress_requests)") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It might be a good idea to check for tenant labels here? 🙂 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. good idea! added here: 7d50a67 |
||
}) | ||
|
||
t.Run("read-write-alerting-rules", func(t *testing.T) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ditto - |
||
// Set a file containing an alerting rule | ||
alertingRule := []byte(alertingRuleYamlTpl) | ||
r, err := http.NewRequest( | ||
http.MethodPut, | ||
rulesEndpointURL, | ||
bytes.NewReader(alertingRule), | ||
) | ||
testutil.Ok(t, err) | ||
|
||
res, err := client.Do(r) | ||
testutil.Ok(t, err) | ||
testutil.Equals(t, http.StatusOK, res.StatusCode) | ||
|
||
// Check if the alerting rule is listed | ||
r, err = http.NewRequest( | ||
http.MethodGet, | ||
rulesEndpointURL, | ||
nil, | ||
) | ||
testutil.Ok(t, err) | ||
|
||
res, err = client.Do(r) | ||
defer res.Body.Close() | ||
|
||
testutil.Ok(t, err) | ||
testutil.Equals(t, http.StatusOK, res.StatusCode) | ||
|
||
body, err := ioutil.ReadAll(res.Body) | ||
bodyStr := string(body) | ||
assertResponse(t, bodyStr, "alert: HighRequestLatency") | ||
}) | ||
|
||
t.Run("read-write-recording-and-alerting-rules", func(t *testing.T) { | ||
// Set a file containing both recording and alerting rules | ||
recordAndAlertingRules := []byte(recordAndAlertingRulesYamlTpl) | ||
r, err := http.NewRequest( | ||
http.MethodPut, | ||
rulesEndpointURL, | ||
bytes.NewReader(recordAndAlertingRules), | ||
) | ||
testutil.Ok(t, err) | ||
|
||
res, err := client.Do(r) | ||
testutil.Ok(t, err) | ||
testutil.Equals(t, http.StatusOK, res.StatusCode) | ||
|
||
// Check if both recording and alerting rules are listed | ||
r, err = http.NewRequest( | ||
http.MethodGet, | ||
rulesEndpointURL, | ||
nil, | ||
) | ||
testutil.Ok(t, err) | ||
|
||
res, err = client.Do(r) | ||
defer res.Body.Close() | ||
|
||
testutil.Ok(t, err) | ||
testutil.Equals(t, http.StatusOK, res.StatusCode) | ||
|
||
body, err := ioutil.ReadAll(res.Body) | ||
bodyStr := string(body) | ||
assertResponse(t, bodyStr, "record: job:up:avg") | ||
assertResponse(t, bodyStr, "alert: ManyInstancesDown") | ||
}) | ||
|
||
t.Run("write-invalid-rules", func(t *testing.T) { | ||
// Set an invalid rules file | ||
invalidRules := []byte(invalidRulesYamlTpl) | ||
r, err := http.NewRequest( | ||
http.MethodPut, | ||
rulesEndpointURL, | ||
bytes.NewReader(invalidRules), | ||
) | ||
testutil.Ok(t, err) | ||
res, err := client.Do(r) | ||
//TODO: an error/http status code is not being returned to the API | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we're not fixing this issue in this PR then please let's create a ticket and reference it so we don't loose sight of this problem :)
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sounds good, I've removed the test case for now and will create a ticket by EOD :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added a bug ticket here There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should get fixed by #218. 🙂 |
||
//testutil.NotOk(t, err) | ||
testutil.Equals(t, http.StatusOK, res.StatusCode) // should this be http.StatusBadRequest instead? (from: https://github.com/observatorium/rules-objstore/blob/main/pkg/server/server.go#L80) | ||
}) | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: Missing newline |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the
test-oidc
tenant name defined anywhere else? I just imagine if we change that we'll unexpectedly break this testThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good point..we define it manually everywhere in the tests: https://github.com/observatorium/api/search?q=test-oidc
maybe makes sense to define it as a constant in
configs.go
and use its value? I could open another pr for that or just already change it here (?)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added as a constant here: 0b60005