[ARVADOS] updated: 087773f25a3df2d35e402f1afc6c7204a0852727
git at public.curoverse.com
git at public.curoverse.com
Thu May 8 18:05:13 EDT 2014
Summary of changes:
services/keep/src/keep/handler_test.go | 97 +++++++++++++++-----------------
services/keep/src/keep/keep.go | 32 +++++++---
2 files changed, 68 insertions(+), 61 deletions(-)
via 087773f25a3df2d35e402f1afc6c7204a0852727 (commit)
from 2a3f67fa270b9660d4dc17b55c48fa781bdba4bb (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
commit 087773f25a3df2d35e402f1afc6c7204a0852727
Author: Tim Pierce <twp at curoverse.com>
Date: Thu May 8 18:05:49 2014 -0400
2328: simplify tests, permission_ttl variable
Incorporating code review:
Added ExpectStatusCode and ExpectBody to simplify repetitive tests.
permission_ttl is now a time.Duration to reduce confusion about units.
diff --git a/services/keep/src/keep/handler_test.go b/services/keep/src/keep/handler_test.go
index f839611..c02ced1 100644
--- a/services/keep/src/keep/handler_test.go
+++ b/services/keep/src/keep/handler_test.go
@@ -39,12 +39,8 @@ func TestGetHandler(t *testing.T) {
resp := httptest.NewRecorder()
rest.ServeHTTP(resp, req)
- if resp.Code != 200 {
- t.Errorf("bad response code: %v", resp)
- }
- if bytes.Compare(resp.Body.Bytes(), TEST_BLOCK) != 0 {
- t.Errorf("bad response body: %v", resp)
- }
+ ExpectStatusCode(t, "unsigned GET", resp, http.StatusOK)
+ ExpectBody(t, "unsigned GET", resp, string(TEST_BLOCK))
// Enable permissions.
enforce_permissions = true
@@ -59,12 +55,8 @@ func TestGetHandler(t *testing.T) {
req.Header.Set("Authorization", "OAuth "+known_token)
rest.ServeHTTP(resp, req)
- if resp.Code != 200 {
- t.Errorf("signed request: bad response code: %v", resp)
- }
- if bytes.Compare(resp.Body.Bytes(), TEST_BLOCK) != 0 {
- t.Errorf("signed request: bad response body: %v", resp)
- }
+ ExpectStatusCode(t, "signed GET", resp, http.StatusOK)
+ ExpectBody(t, "signed GET", resp, string(TEST_BLOCK))
// Test GET with an unsigned locator.
test_url = "http://localhost:25107/" + TEST_HASH
@@ -73,9 +65,7 @@ func TestGetHandler(t *testing.T) {
req.Header.Set("Authorization", "OAuth "+known_token)
rest.ServeHTTP(resp, req)
- if resp.Code != PermissionError.HTTPCode {
- t.Errorf("unsigned request: bad response code: %v", resp)
- }
+ ExpectStatusCode(t, "unsigned locator", resp, PermissionError.HTTPCode)
// Test GET with a signed locator and an unauthenticated request.
test_url = "http://localhost:25107/" + SignLocator(TEST_HASH, known_token, expiry)
@@ -83,9 +73,7 @@ func TestGetHandler(t *testing.T) {
req, _ = http.NewRequest("GET", test_url, nil)
rest.ServeHTTP(resp, req)
- if resp.Code != PermissionError.HTTPCode {
- t.Errorf("signed locator, unauthenticated request: bad response code: %v", resp)
- }
+ ExpectStatusCode(t, "signed locator", resp, PermissionError.HTTPCode)
// Test GET with an expired, signed locator.
expired_ts := time.Now().Add(-time.Hour)
@@ -95,9 +83,7 @@ func TestGetHandler(t *testing.T) {
req.Header.Set("Authorization", "OAuth "+known_token)
rest.ServeHTTP(resp, req)
- if resp.Code != ExpiredError.HTTPCode {
- t.Errorf("expired signature: bad response code: %v", resp)
- }
+ ExpectStatusCode(t, "expired signature", resp, ExpiredError.HTTPCode)
}
func TestPutHandler(t *testing.T) {
@@ -117,12 +103,8 @@ func TestPutHandler(t *testing.T) {
resp := httptest.NewRecorder()
rest.ServeHTTP(resp, req)
- if resp.Code != 200 {
- t.Errorf("bad response code: %v", resp)
- }
- if resp.Body.String() != TEST_HASH {
- t.Errorf("bad response body: %v", resp)
- }
+ ExpectStatusCode(t, "permissions off", resp, http.StatusOK)
+ ExpectBody(t, "permissions off", resp, TEST_HASH)
// Add a permission key.
// When a permission key is available, the locator returned
@@ -137,11 +119,10 @@ func TestPutHandler(t *testing.T) {
resp = httptest.NewRecorder()
rest.ServeHTTP(resp, req)
- if resp.Code != 200 {
- t.Errorf("bad response code: %v", resp)
- }
+ ExpectStatusCode(t, "authenticated PUT", resp, http.StatusOK)
if !VerifySignature(resp.Body.String(), known_token) {
- t.Errorf("bad response body: %v", resp)
+ t.Errorf("authenticated PUT: response '%s' failed signature check",
+ resp.Body.String())
}
// An unauthenticated PUT request returns an unsigned locator
@@ -152,12 +133,8 @@ func TestPutHandler(t *testing.T) {
resp = httptest.NewRecorder()
rest.ServeHTTP(resp, req)
- if resp.Code != 200 {
- t.Errorf("bad response code: %v", resp)
- }
- if resp.Body.String() != TEST_HASH {
- t.Errorf("bad response body: %v", resp)
- }
+ ExpectStatusCode(t, "anon PUT with server key", resp, http.StatusOK)
+ ExpectBody(t, "anon PUT with server key", resp, TEST_HASH)
}
func TestIndexHandler(t *testing.T) {
@@ -185,7 +162,8 @@ func TestIndexHandler(t *testing.T) {
expected := `^` + TEST_HASH + `\+\d+ \d+\n$`
match, _ := regexp.MatchString(expected, resp.Body.String())
if !match {
- t.Errorf("IndexHandler returned:\n%s", resp.Body.String())
+ t.Errorf("IndexHandler expected: %s, returned:\n%s",
+ expected, resp.Body.String())
}
// Unauthenticated /index requests: fail.
@@ -194,9 +172,7 @@ func TestIndexHandler(t *testing.T) {
resp = httptest.NewRecorder()
rest.ServeHTTP(resp, req)
- if resp.Code != PermissionError.HTTPCode {
- t.Errorf("unauthenticated /index: %+v", resp)
- }
+ ExpectStatusCode(t, "unauthenticated /index", resp, PermissionError.HTTPCode)
// Authenticated /index requests by a non-superuser: also fail.
test_url = "http://localhost:25107/index"
@@ -205,9 +181,7 @@ func TestIndexHandler(t *testing.T) {
resp = httptest.NewRecorder()
rest.ServeHTTP(resp, req)
- if resp.Code != PermissionError.HTTPCode {
- t.Errorf("authenticated /index: %+v", resp)
- }
+ ExpectStatusCode(t, "authenticated /index", resp, PermissionError.HTTPCode)
// Even superuser /index requests fail if enforce_permissions is off!
enforce_permissions = false
@@ -218,9 +192,7 @@ func TestIndexHandler(t *testing.T) {
resp = httptest.NewRecorder()
rest.ServeHTTP(resp, req)
- if resp.Code != PermissionError.HTTPCode {
- t.Errorf("superuser /index (permissions off): %+v", resp)
- }
+ ExpectStatusCode(t, "superuser /index (permissions off)", resp, PermissionError.HTTPCode)
// Superuser /index requests with enforce_permissions set: succeed!
enforce_permissions = true
@@ -231,13 +203,36 @@ func TestIndexHandler(t *testing.T) {
resp = httptest.NewRecorder()
rest.ServeHTTP(resp, req)
- if resp.Code != http.StatusOK {
- t.Errorf("superuser /index: %+v", resp)
- }
+ ExpectStatusCode(t, "superuser /index (permissions on)", resp, http.StatusOK)
expected = `^` + TEST_HASH + `\+\d+ \d+\n` +
TEST_HASH_2 + `\+\d+ \d+\n$`
match, _ = regexp.MatchString(expected, resp.Body.String())
if !match {
- t.Errorf("superuser /index:\n%s", resp.Body.String())
+ t.Errorf("superuser /index: expected %s, got:\n%s",
+ expected, resp.Body.String())
+ }
+}
+
+// ExpectStatusCode checks whether a response has the specified status code,
+// and reports a test failure if not.
+func ExpectStatusCode(
+ t *testing.T,
+ testname string,
+ response *httptest.ResponseRecorder,
+ expected_status int) {
+ if response.Code != expected_status {
+ t.Errorf("%s: expected status %s, got %+v",
+ testname, expected_status, response)
+ }
+}
+
+func ExpectBody(
+ t *testing.T,
+ testname string,
+ response *httptest.ResponseRecorder,
+ expected_body string) {
+ if response.Body.String() != expected_body {
+ t.Errorf("%s: expected response body '%s', got %+v",
+ testname, expected_body, response)
}
}
diff --git a/services/keep/src/keep/keep.go b/services/keep/src/keep/keep.go
index fa27b66..143d290 100644
--- a/services/keep/src/keep/keep.go
+++ b/services/keep/src/keep/keep.go
@@ -28,6 +28,7 @@ import (
// and/or configuration file settings.
// Default TCP address on which to listen for requests.
+// Initialized by the --listen flag.
const DEFAULT_ADDR = ":25107"
// A Keep "block" is 64MB.
@@ -40,19 +41,22 @@ const MIN_FREE_KILOBYTES = BLOCKSIZE / 1024
var PROC_MOUNTS = "/proc/mounts"
// The Keep VolumeManager maintains a list of available volumes.
+// Initialized by the --volumes flag (or by FindKeepVolumes).
var KeepVM VolumeManager
// enforce_permissions controls whether permission signatures
-// should be enforced (affecting GET and DELETE requests)
+// should be enforced (affecting GET and DELETE requests).
+// Initialized by the --enforce-permissions flag.
var enforce_permissions bool
-// permission_ttl is the time duration (in seconds) for which
-// new permission signatures (returned by PUT requests) will be
-// valid.
-var permission_ttl int
+// permission_ttl is the time duration for which new permission
+// signatures (returned by PUT requests) will be valid.
+// Initialized by the --permission-ttl flag.
+var permission_ttl time.Duration
// data_manager_token represents the API token used by the
// Data Manager, and is required on certain privileged operations.
+// Initialized by the --data-manager-token-file flag.
var data_manager_token string
// ==========
@@ -103,8 +107,14 @@ func main() {
// by looking at currently mounted filesystems for /keep top-level
// directories.
- var data_manager_token_file, listen, permission_key_file, volumearg string
- var serialize_io bool
+ var (
+ data_manager_token_file string
+ listen string
+ permission_key_file string
+ permission_ttl_sec int
+ serialize_io bool
+ volumearg string
+ )
flag.StringVar(
&data_manager_token_file,
"data-manager-token-file",
@@ -126,7 +136,7 @@ func main() {
"",
"File containing the secret key for generating and verifying permission signatures.")
flag.IntVar(
- &permission_ttl,
+ &permission_ttl_sec,
"permission-ttl",
300,
"Expiration time (in seconds) for newly generated permission signatures.")
@@ -185,6 +195,9 @@ func main() {
}
}
+ // Initialize permission TTL
+ permission_ttl = time.Duration(permission_ttl_sec) * time.Second
+
// If --enforce-permissions is true, we must have a permission key to continue.
if enforce_permissions && PermissionSecret == nil {
log.Fatal("--enforce-permissions requires a permission key")
@@ -297,8 +310,7 @@ func PutBlockHandler(w http.ResponseWriter, req *http.Request) {
if err := PutBlock(buf, hash); err == nil {
// Success; sign the locator and return it to the client.
api_token := GetApiToken(req)
- expiry := time.Now().Add( // convert permission_ttl to time.Duration
- time.Duration(permission_ttl) * time.Second)
+ expiry := time.Now().Add(permission_ttl)
signed_loc := SignLocator(hash, api_token, expiry)
w.Write([]byte(signed_loc))
} else {
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list