Skip to content
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

extract some bits from #356 #363

Merged
merged 3 commits into from Mar 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
51 changes: 31 additions & 20 deletions internal/api/keppel/accounts.go
Expand Up @@ -183,6 +183,32 @@ func renderReplicationPolicy(dbAccount keppel.Account) *ReplicationPolicy {
return nil
}

func (r ReplicationPolicy) ApplyToAccount(db *keppel.DB, dbAccount *keppel.Account) (httpStatus int, err error) {
switch r.Strategy {
case "on_first_use":
peerCount, err := db.SelectInt(`SELECT COUNT(*) FROM peers WHERE hostname = $1`, r.UpstreamPeerHostName)
if err != nil {
return http.StatusInternalServerError, err
}

if peerCount == 0 {
return http.StatusUnprocessableEntity, fmt.Errorf(`unknown peer registry: %q`, r.UpstreamPeerHostName)
}
dbAccount.UpstreamPeerHostName = r.UpstreamPeerHostName
case "from_external_on_first_use":
if r.ExternalPeer.URL == "" {
return http.StatusUnprocessableEntity, errors.New(`missing upstream URL for "from_external_on_first_use" replication`)
}
dbAccount.ExternalPeerURL = r.ExternalPeer.URL
dbAccount.ExternalPeerUserName = r.ExternalPeer.UserName
dbAccount.ExternalPeerPassword = r.ExternalPeer.Password
default:
return http.StatusUnprocessableEntity, fmt.Errorf("strategy %s is unsupported", r.Strategy)
}

return http.StatusOK, nil
}

func renderValidationPolicy(dbAccount keppel.Account) *ValidationPolicy {
if dbAccount.RequiredLabels == "" {
return nil
Expand Down Expand Up @@ -348,27 +374,12 @@ func (a *API) handlePutAccount(w http.ResponseWriter, r *http.Request) {
if req.Account.ReplicationPolicy != nil {
rp := *req.Account.ReplicationPolicy

switch rp.Strategy {
case "on_first_use":
peerCount, err := a.db.SelectInt(`SELECT COUNT(*) FROM peers WHERE hostname = $1`, rp.UpstreamPeerHostName)
if respondwith.ErrorText(w, err) {
return
}
if peerCount == 0 {
http.Error(w, fmt.Sprintf(`unknown peer registry: %q`, rp.UpstreamPeerHostName), http.StatusUnprocessableEntity)
return
}
accountToCreate.UpstreamPeerHostName = rp.UpstreamPeerHostName
case "from_external_on_first_use":
if rp.ExternalPeer.URL == "" {
http.Error(w, `missing upstream URL for "from_external_on_first_use" replication`, http.StatusUnprocessableEntity)
return
}
accountToCreate.ExternalPeerURL = rp.ExternalPeer.URL
accountToCreate.ExternalPeerUserName = rp.ExternalPeer.UserName
accountToCreate.ExternalPeerPassword = rp.ExternalPeer.Password
//NOTE: There are some delayed checks below which require the existing account to be loaded from the DB first.
httpStatus, err := rp.ApplyToAccount(a.db, &accountToCreate)
if err != nil {
http.Error(w, err.Error(), httpStatus)
return
}
//NOTE: There are some delayed checks below which require the existing account to be loaded from the DB first.
}

// validate validation policy
Expand Down
42 changes: 39 additions & 3 deletions internal/api/keppel/accounts_test.go
Expand Up @@ -377,7 +377,7 @@ func TestAccountsAPI(t *testing.T) {
}
}

// check editing of RBAC policies
// check editing of metadata and RBAC policies
newRBACPoliciesJSON := []assert.JSONObject{
// rbacPoliciesJSON[0] is deleted
// rbacPoliciesJSON[1] is updated as follows:
Expand All @@ -393,14 +393,20 @@ func TestAccountsAPI(t *testing.T) {
"permissions": []string{"pull", "delete"},
},
}
newMetadataJSON := assert.JSONObject{
"foo": "bingo",
"bar": "buz",
}
assert.HTTPRequest{
Method: "PUT",
Path: "/keppel/v1/accounts/second",
Header: map[string]string{"X-Test-Perms": "change:tenant1"},
Body: assert.JSONObject{
"account": assert.JSONObject{
"auth_tenant_id": "tenant1",
"rbac_policies": newRBACPoliciesJSON,
// add metadata
"metadata": newMetadataJSON,
"rbac_policies": newRBACPoliciesJSON,
},
},
ExpectStatus: http.StatusOK,
Expand All @@ -409,7 +415,7 @@ func TestAccountsAPI(t *testing.T) {
"name": "second",
"auth_tenant_id": "tenant1",
"in_maintenance": false,
"metadata": assert.JSONObject{},
"metadata": newMetadataJSON,
"rbac_policies": newRBACPoliciesJSON,
},
},
Expand Down Expand Up @@ -603,6 +609,19 @@ func TestPutAccountErrorCases(t *testing.T) {
ExpectBody: assert.StringData("account names with the prefix \"keppel\" are reserved for internal use\n"),
}.Check(t, h)

assert.HTTPRequest{
Method: "PUT",
Path: "/keppel/v1/accounts/v1",
Header: map[string]string{"X-Test-Perms": "change:tenant1"},
Body: assert.JSONObject{
"account": assert.JSONObject{
"auth_tenant_id": "tenant1",
},
},
ExpectStatus: http.StatusUnprocessableEntity,
ExpectBody: assert.StringData("account names that look like API versions are reserved for internal use\n"),
}.Check(t, h)

assert.HTTPRequest{
Method: "PUT",
Path: "/keppel/v1/accounts/first",
Expand Down Expand Up @@ -695,6 +714,23 @@ func TestPutAccountErrorCases(t *testing.T) {
}.Check(t, h)
s.SD.ForbidNewAccounts = false

// test setting up invalid required_labels
assert.HTTPRequest{
Method: "PUT",
Path: "/keppel/v1/accounts/second",
Header: map[string]string{"X-Test-Perms": "change:tenant1"},
Body: assert.JSONObject{
"account": assert.JSONObject{
"auth_tenant_id": "tenant1",
"validation": assert.JSONObject{
"required_labels": []string{"foo,", ",bar"},
},
},
},
ExpectStatus: http.StatusUnprocessableEntity,
ExpectBody: assert.StringData("invalid label name: \"foo,\"\n"),
}.Check(t, h)

// test malformed GC policies
gcPolicyTestcases := []struct {
GCPolicyJSON assert.JSONObject
Expand Down
4 changes: 2 additions & 2 deletions internal/drivers/basic/ratelimit.go
Expand Up @@ -65,10 +65,10 @@ func init() {
})
}

// PluginTypeID implements the keppel.FederationDriver interface.
// PluginTypeID implements the keppel.RateLimitDriver interface.
func (d RateLimitDriver) PluginTypeID() string { return "basic" }

// Init implements the keppel.FederationDriver interface.
// Init implements the keppel.RateLimitDriver interface.
func (d RateLimitDriver) Init(ad keppel.AuthDriver, cfg keppel.Configuration) error {
for action, envVars := range envVars {
rate, err := parseRateLimit(envVars.RateLimit)
Expand Down