Merge pull request #247 from endophage/coverage
minor refactor + tests for app.go just to improve test coverage.
This commit is contained in:
commit
cbdc3621cf
2 changed files with 105 additions and 31 deletions
|
@ -304,37 +304,7 @@ func (app *App) authorized(w http.ResponseWriter, r *http.Request, context *Cont
|
|||
var accessRecords []auth.Access
|
||||
|
||||
if repo != "" {
|
||||
resource := auth.Resource{
|
||||
Type: "repository",
|
||||
Name: repo,
|
||||
}
|
||||
|
||||
switch r.Method {
|
||||
case "GET", "HEAD":
|
||||
accessRecords = append(accessRecords,
|
||||
auth.Access{
|
||||
Resource: resource,
|
||||
Action: "pull",
|
||||
})
|
||||
case "POST", "PUT", "PATCH":
|
||||
accessRecords = append(accessRecords,
|
||||
auth.Access{
|
||||
Resource: resource,
|
||||
Action: "pull",
|
||||
},
|
||||
auth.Access{
|
||||
Resource: resource,
|
||||
Action: "push",
|
||||
})
|
||||
case "DELETE":
|
||||
// DELETE access requires full admin rights, which is represented
|
||||
// as "*". This may not be ideal.
|
||||
accessRecords = append(accessRecords,
|
||||
auth.Access{
|
||||
Resource: resource,
|
||||
Action: "*",
|
||||
})
|
||||
}
|
||||
accessRecords = appendAccessRecords(accessRecords, r.Method, repo)
|
||||
} else {
|
||||
// Only allow the name not to be set on the base route.
|
||||
if app.nameRequired(r) {
|
||||
|
@ -411,3 +381,39 @@ func apiBase(w http.ResponseWriter, r *http.Request) {
|
|||
|
||||
fmt.Fprint(w, emptyJSON)
|
||||
}
|
||||
|
||||
// appendAccessRecords checks the method and adds the appropriate Access records to the records list.
|
||||
func appendAccessRecords(records []auth.Access, method string, repo string) []auth.Access {
|
||||
resource := auth.Resource{
|
||||
Type: "repository",
|
||||
Name: repo,
|
||||
}
|
||||
|
||||
switch method {
|
||||
case "GET", "HEAD":
|
||||
records = append(records,
|
||||
auth.Access{
|
||||
Resource: resource,
|
||||
Action: "pull",
|
||||
})
|
||||
case "POST", "PUT", "PATCH":
|
||||
records = append(records,
|
||||
auth.Access{
|
||||
Resource: resource,
|
||||
Action: "pull",
|
||||
},
|
||||
auth.Access{
|
||||
Resource: resource,
|
||||
Action: "push",
|
||||
})
|
||||
case "DELETE":
|
||||
// DELETE access requires full admin rights, which is represented
|
||||
// as "*". This may not be ideal.
|
||||
records = append(records,
|
||||
auth.Access{
|
||||
Resource: resource,
|
||||
Action: "*",
|
||||
})
|
||||
}
|
||||
return records
|
||||
}
|
||||
|
|
|
@ -5,10 +5,12 @@ import (
|
|||
"net/http"
|
||||
"net/http/httptest"
|
||||
"net/url"
|
||||
"reflect"
|
||||
"testing"
|
||||
|
||||
"github.com/docker/distribution/configuration"
|
||||
"github.com/docker/distribution/registry/api/v2"
|
||||
"github.com/docker/distribution/registry/auth"
|
||||
_ "github.com/docker/distribution/registry/auth/silly"
|
||||
"github.com/docker/distribution/registry/storage"
|
||||
"github.com/docker/distribution/registry/storage/driver/inmemory"
|
||||
|
@ -200,3 +202,69 @@ func TestNewApp(t *testing.T) {
|
|||
t.Fatalf("unexpected error code: %v != %v", errs.Errors[0].Code, v2.ErrorCodeUnauthorized)
|
||||
}
|
||||
}
|
||||
|
||||
// Test the access record accumulator
|
||||
func TestAppendAccessRecords(t *testing.T) {
|
||||
repo := "testRepo"
|
||||
|
||||
expectedResource := auth.Resource{
|
||||
Type: "repository",
|
||||
Name: repo,
|
||||
}
|
||||
|
||||
expectedPullRecord := auth.Access{
|
||||
Resource: expectedResource,
|
||||
Action: "pull",
|
||||
}
|
||||
expectedPushRecord := auth.Access{
|
||||
Resource: expectedResource,
|
||||
Action: "push",
|
||||
}
|
||||
expectedAllRecord := auth.Access{
|
||||
Resource: expectedResource,
|
||||
Action: "*",
|
||||
}
|
||||
|
||||
records := []auth.Access{}
|
||||
result := appendAccessRecords(records, "GET", repo)
|
||||
expectedResult := []auth.Access{expectedPullRecord}
|
||||
if ok := reflect.DeepEqual(result, expectedResult); !ok {
|
||||
t.Fatalf("Actual access record differs from expected")
|
||||
}
|
||||
|
||||
records = []auth.Access{}
|
||||
result = appendAccessRecords(records, "HEAD", repo)
|
||||
expectedResult = []auth.Access{expectedPullRecord}
|
||||
if ok := reflect.DeepEqual(result, expectedResult); !ok {
|
||||
t.Fatalf("Actual access record differs from expected")
|
||||
}
|
||||
|
||||
records = []auth.Access{}
|
||||
result = appendAccessRecords(records, "POST", repo)
|
||||
expectedResult = []auth.Access{expectedPullRecord, expectedPushRecord}
|
||||
if ok := reflect.DeepEqual(result, expectedResult); !ok {
|
||||
t.Fatalf("Actual access record differs from expected")
|
||||
}
|
||||
|
||||
records = []auth.Access{}
|
||||
result = appendAccessRecords(records, "PUT", repo)
|
||||
expectedResult = []auth.Access{expectedPullRecord, expectedPushRecord}
|
||||
if ok := reflect.DeepEqual(result, expectedResult); !ok {
|
||||
t.Fatalf("Actual access record differs from expected")
|
||||
}
|
||||
|
||||
records = []auth.Access{}
|
||||
result = appendAccessRecords(records, "PATCH", repo)
|
||||
expectedResult = []auth.Access{expectedPullRecord, expectedPushRecord}
|
||||
if ok := reflect.DeepEqual(result, expectedResult); !ok {
|
||||
t.Fatalf("Actual access record differs from expected")
|
||||
}
|
||||
|
||||
records = []auth.Access{}
|
||||
result = appendAccessRecords(records, "DELETE", repo)
|
||||
expectedResult = []auth.Access{expectedAllRecord}
|
||||
if ok := reflect.DeepEqual(result, expectedResult); !ok {
|
||||
t.Fatalf("Actual access record differs from expected")
|
||||
}
|
||||
|
||||
}
|
||||
|
|
Loading…
Reference in a new issue