Merge pull request #363 from sapcc/356-extracted

extract some bits from #356

internal/api/keppel/accounts.go

@@ -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
@@ -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

internal/api/keppel/accounts_test.go

@@ -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:
@@ -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,
@@ -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,
 			},
 		},
@@ -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",
@@ -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

internal/drivers/basic/ratelimit.go

@@ -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)