only test west cluster with rate limits; extend test cases

input_test.go

@@ -66,7 +66,7 @@ var rateLimits = QuotaRequest{
 	"object-store": ServiceQuotaRequest{
 		Rates: RateQuotaRequest{
 			"object/account/container": {
-				"create": {Value: 1000, Unit: UnitRequestsPerSeconds},
+				"create": {Value: 1000, Unit: UnitRequestsPerSecond},
 			},
 		},
 		Resources: ResourceQuotaRequest{},

pkg/api/api_test.go

@@ -29,7 +29,7 @@ import (
 	"strings"
 	"testing"
 
-	policy "github.com/databus23/goslo.policy"
+	"github.com/databus23/goslo.policy"
 	"github.com/sapcc/go-bits/assert"
 	"github.com/sapcc/limes"
 	"github.com/sapcc/limes/pkg/core"
@@ -92,7 +92,7 @@ func setupTest(t *testing.T, clusterName, startData string) (*core.Cluster, http
 
 	quotaPlugins["shared"].(*test.Plugin).WithExternallyManagedResource = true
 
-	clusterConfig := &core.ClusterConfiguration{
+	westClusterConfig := &core.ClusterConfiguration{
 		Auth: &core.AuthParameters{},
 		Services: []core.ServiceConfiguration{
 			{
@@ -174,15 +174,15 @@ func setupTest(t *testing.T, clusterName, startData string) (*core.Cluster, http
 				QuotaPlugins:     quotaPlugins,
 				CapacityPlugins:  map[string]core.CapacityPlugin{},
 				QuotaConstraints: &westConstraintSet,
-				Config:           clusterConfig,
+				Config:           westClusterConfig,
 			},
 			"east": {
 				ID:              "east",
 				ServiceTypes:    serviceTypes,
 				IsServiceShared: isServiceShared,
 				QuotaPlugins:    quotaPlugins,
 				CapacityPlugins: map[string]core.CapacityPlugin{},
-				Config:          clusterConfig,
+				Config:          &core.ClusterConfiguration{Auth: &core.AuthParameters{}},
 			},
 			"cloud": {
 				ID:              "cloud",
@@ -191,7 +191,7 @@ func setupTest(t *testing.T, clusterName, startData string) (*core.Cluster, http
 				DiscoveryPlugin: test.NewDiscoveryPlugin(),
 				QuotaPlugins:    quotaPlugins,
 				CapacityPlugins: map[string]core.CapacityPlugin{},
-				Config:          clusterConfig,
+				Config:          &core.ClusterConfiguration{Auth: &core.AuthParameters{}},
 			},
 		},
 	}
@@ -1448,22 +1448,25 @@ func Test_ProjectOperations(t *testing.T) {
 	}
 
 	//Check PUT ../project with rate limits.
-	plugin.SetQuotaFails = false
+	//Attempt setting a rate limit for which no default exists should fail.
 	assert.HTTPRequest{
 		Method:       "PUT",
 		Path:         "/v1/domains/uuid-for-germany/projects/uuid-for-berlin",
-		ExpectStatus: 202,
+		ExpectStatus: 403,
+		ExpectBody: assert.StringData(
+			"cannot change shared/service/shared/notexistent/bogus rate limits: user is not allowed to create new rate limits\n",
+		),
 		Body: assert.JSONObject{
 			"project": assert.JSONObject{
 				"services": []assert.JSONObject{
 					{
 						"type": "shared",
 						"rates": []assert.JSONObject{
 							{
-								"target_type_uri": "service/shared/things",
+								"target_type_uri": "service/shared/notexistent",
 								"actions": []assert.JSONObject{
 									{
-										"name":  "read/list",
+										"name":  "bogus",
 										"limit": 1,
 										"unit":  "r/h",
 									},
@@ -1480,24 +1483,63 @@ func Test_ProjectOperations(t *testing.T) {
 		actualUnit  limes.Unit
 	)
 	err = db.DB.QueryRow(`
-		SELECT prl.rate_limit, prl.unit FROM project_rate_limits pr
-		JOIN project_services ps ON ps.id = pr.service_id
+		SELECT prl.rate_limit, prl.unit FROM project_rate_limits prl
+		JOIN project_services ps ON ps.id = prl.service_id
+		JOIN projects p ON p.id = ps.project_id
+		WHERE p.name = $1 AND ps.type = $2 AND prl.target_type_uri = $3 AND prl.action = $4`,
+		"berlin", "shared", "service/shared/notexistent", "bogus").Scan(&actualLimit, &actualUnit)
+	//There shouldn't be anything in the DB.
+	if err.Error() != "sql: no rows in result set" {
+		t.Fatalf("expected error %v but got %v", "sql: no rows in result set", err)
+	}
+
+	//Attempt setting a rate limit for which a default exists should be successful.
+	assert.HTTPRequest{
+		Method:       "PUT",
+		Path:         "/v1/domains/uuid-for-germany/projects/uuid-for-berlin",
+		ExpectStatus: 202,
+		Body: assert.JSONObject{
+			"project": assert.JSONObject{
+				"services": []assert.JSONObject{
+					{
+						"type": "shared",
+						"rates": []assert.JSONObject{
+							{
+								"target_type_uri": "service/shared/objects",
+								"actions": []assert.JSONObject{
+									{
+										"name":  "create",
+										"limit": 100,
+										"unit":  "r/s",
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+		},
+	}.Check(t, router)
+
+	err = db.DB.QueryRow(`
+		SELECT prl.rate_limit, prl.unit FROM project_rate_limits prl
+		JOIN project_services ps ON ps.id = prl.service_id
 		JOIN projects p ON p.id = ps.project_id
 		WHERE p.name = $1 AND ps.type = $2 AND prl.target_type_uri = $3 AND prl.action = $4`,
-		"berlin", "shared", "service/shared/things", "read/list").Scan(&actualLimit, &actualUnit)
+		"berlin", "shared", "service/shared/objects", "create").Scan(&actualLimit, &actualUnit)
 	if err != nil {
 		t.Fatal(err)
 	}
-	if actualLimit != 1 {
+	if actualLimit != 100 {
 		t.Errorf(
 			"rate limit was not updated in database for %s %s. expected limit %d but got %d",
-			"service/shared/things", "read/list", 1, actualLimit,
+			"service/shared/objects", "create", 100, actualLimit,
 		)
 	}
-	if actualUnit != limes.UnitRequestsPerHour {
+	if actualUnit != limes.UnitRequestsPerSecond {
 		t.Errorf(
 			"rate limit was not updated in database for %s %s. expected unit %s but got %s",
-			"service/shared/things", "read/list", limes.UnitRequestsPerHour, actualUnit,
+			"service/shared/objects", "create", limes.UnitRequestsPerSecond, actualUnit,
 		)
 	}
 

pkg/api/audit.go

@@ -203,8 +203,8 @@ func (t rateLimitEventTarget) Render() cadf.Resource {
 				Name:    "payload",
 				TypeURI: "mime:application/json",
 				Content: targetAttachmentContent{
-					OldQuota:     t.OldLimit,
-					NewQuota:     t.NewLimit,
+					OldLimit:     t.OldLimit,
+					NewLimit:     t.NewLimit,
 					OldUnit:      t.OldUnit,
 					NewUnit:      t.NewUnit,
 					RejectReason: t.RejectReason,

pkg/api/projects.go

@@ -347,7 +347,6 @@ func (p *v1Provider) putOrSimulatePutProjectQuotas(w http.ResponseWriter, r *htt
 		return
 	}
 
-	//TODO: @auhlig this bypasses the quotaUpdater.validateRateLimit(..)
 	//Update the DB with the new rate limits.
 	stmt, err := dbi.Prepare(`UPDATE project_rate_limits SET rate_limit = $1, unit = $2 WHERE service_id = $3 AND target_type_uri = $4 AND action = $5`)
 	if respondwith.ErrorText(w, err) {

pkg/api/quota_updater.go

@@ -167,46 +167,71 @@ func (u *QuotaUpdater) ValidateInput(input limes.QuotaRequest, dbi db.Interface)
 		}
 	}
 
-	//Go through all services and validate the requested rate limits.
-	u.RateLimitRequests = make(map[string]RateLimitRequests)
-	for _, svc := range u.Cluster.Config.Services {
-		u.RateLimitRequests[svc.Type] = RateLimitRequests{}
-		for _, req := range input {
-			for targetTypeURI, requests := range req.Rates {
+	//Rate limits are only available on project level.
+	if u.Project != nil {
+		u.RateLimitRequests = make(map[string]RateLimitRequests)
+
+		//Go through all services and validate the requested rate limits.
+		for svcType, in := range input {
+			svcConfig, err := u.Cluster.Config.GetServiceConfigurationForType(svcType)
+			if err != nil {
+				//Skip service if not configured.
+				continue
+			}
+			if _, ok := u.RateLimitRequests[svcType]; !ok {
+				u.RateLimitRequests[svcType] = make(RateLimitRequests)
+			}
+
+			for targetTypeURI, requests := range in.Rates {
 				for action, newRateLimit := range requests {
-					var rlActRep *limes.ProjectRateLimitActionReport
+					if _, ok := u.RateLimitRequests[svcType][targetTypeURI]; !ok {
+						u.RateLimitRequests[svcType][targetTypeURI] = make(map[string]QuotaRequest)
+					}
 
-					if u.Project != nil {
-						if projectService, exists := projectReport.Services[svc.Type]; exists {
-							projRes, exists := projectService.Rates[targetTypeURI]
-							if !exists {
-								//No report for rate limit.
-								continue
-							}
+					req := QuotaRequest{
+						NewValue: newRateLimit.Value,
+						NewUnit:  newRateLimit.Unit,
+					}
 
-							aRep, exists := projRes.Actions[action]
-							if !exists {
-								continue
-							}
-							rlActRep = aRep
+					//Allow only setting rate limits for which a default exists.
+					defaultLimit, defaultUnit, err := svcConfig.Rates.GetProjectDefaultRateLimit(targetTypeURI, action)
+					if err != nil {
+						req.ValidationError = &core.QuotaValidationError{
+							Status:  http.StatusForbidden,
+							Message: "user is not allowed to create new rate limits",
 						}
+						u.RateLimitRequests[svcType][targetTypeURI][action] = req
+						continue
 					}
 
-					req := QuotaRequest{
-						OldValue: rlActRep.Limit,
-						Unit:     rlActRep.Unit,
+					var rlActRep *limes.ProjectRateLimitActionReport
+					if projectService, exists := projectReport.Services[svcType]; exists {
+						projectRateLimit, exists := projectService.Rates[targetTypeURI]
+						if !exists {
+							projectRateLimit = &limes.ProjectRateLimitReport{Actions: make(limes.ProjectRateLimitActionReports)}
+						}
+
+						actionRateLimit, exists := projectRateLimit.Actions[action]
+						if !exists {
+							actionRateLimit = &limes.ProjectRateLimitActionReport{
+								Limit: defaultLimit,
+								Unit:  limes.Unit(defaultUnit),
+							}
+						}
+						rlActRep = actionRateLimit
 					}
 
+					req.OldValue = rlActRep.Limit
+					req.Unit = rlActRep.Unit
+
 					//Skip if rate limit value and unit were not changed.
 					if req.OldValue == newRateLimit.Value && req.Unit == newRateLimit.Unit {
 						continue
 					}
 
 					//Add to the list of rate limit requests as the value and/or unit changed.
-					req.NewValue = newRateLimit.Value
-					req.NewUnit = newRateLimit.Unit
-					req.ValidationError = u.validateRateLimit(u.Cluster.InfoForService(svc.Type), targetTypeURI, rlActRep, req.NewValue, req.NewUnit)
-					u.RateLimitRequests[svc.Type][targetTypeURI][action] = req
+					req.ValidationError = u.validateRateLimit(u.Cluster.InfoForService(svcType), targetTypeURI, rlActRep, req.NewValue, req.NewUnit)
+					u.RateLimitRequests[svcType][targetTypeURI][action] = req
 				}
 			}
 		}
@@ -284,22 +309,6 @@ func (u QuotaUpdater) validateQuota(srv limes.ServiceInfo, res limes.ResourceInf
 }
 
 func (u QuotaUpdater) validateRateLimit(srv limes.ServiceInfo, targetTypeURI string, rateLimit *limes.ProjectRateLimitActionReport, newRateLimitValue uint64, newRateLimitUnit limes.Unit) *core.QuotaValidationError {
-
-	//Allow only setting rate limits for which a default was configured.
-	svcConfig, err := u.Cluster.Config.GetServiceConfigurationForType(srv.Type)
-	if err != nil {
-		return &core.QuotaValidationError{
-			Status:  http.StatusForbidden,
-			Message: "user is not allowed to create new rate limits",
-		}
-	}
-	if _, _, err := svcConfig.Rates.GetProjectDefaultRateLimit(targetTypeURI, rateLimit.Name); err != nil {
-		return &core.QuotaValidationError{
-			Status:  http.StatusForbidden,
-			Message: "user is not allowed to create new rate limits",
-		}
-	}
-
 	return u.validateAuthorizationRateLimit(rateLimit.Limit, newRateLimitValue, rateLimit.Unit, newRateLimitUnit)
 }
 
@@ -414,6 +423,15 @@ func (u QuotaUpdater) IsValid() bool {
 			}
 		}
 	}
+	for _, rlRequests := range u.RateLimitRequests {
+		for _, actions := range rlRequests {
+			for _, req := range actions {
+				if req.ValidationError != nil {
+					return false
+				}
+			}
+		}
+	}
 	return true
 }
 
@@ -538,14 +556,13 @@ func (u QuotaUpdater) WritePutErrorResponse(w http.ResponseWriter) {
 		}
 	}
 	for srvType, rateLimits := range u.RateLimitRequests {
-		for ttu, requests := range rateLimits {
+		for targetTypeURI, requests := range rateLimits {
 			for action, req := range requests {
-				err := req.ValidationError
-				if err != nil {
+				if err := req.ValidationError; err != nil {
 					hasSubstatus[err.Status] = true
 					lines = append(
 						lines,
-						fmt.Sprintf("cannot change %s/%s/%s rate limits", srvType, ttu, action),
+						fmt.Sprintf("cannot change %s/%s/%s rate limits: %s", srvType, targetTypeURI, action, err.Message),
 					)
 				}
 			}

pkg/reports/project.go

@@ -101,7 +101,7 @@ func GetProjects(cluster *core.Cluster, domain db.Domain, projectID *int64, dbi
 				}
 			}
 
-			if resReport != nil && resourceName != nil {
+			if resReport != nil {
 				subresourcesValue := ""
 				if subresources != nil {
 					subresourcesValue = *subresources

report_cluster_test.go

@@ -121,7 +121,7 @@ var clusterServicesOnlyRates = &ClusterServiceReports{
 					"create": &ClusterRateLimitActionReport{
 						Name:  "create",
 						Limit: 5000,
-						Unit:  UnitRequestsPerSeconds,
+						Unit:  UnitRequestsPerSecond,
 					},
 				},
 			},

report_project_test.go

@@ -176,12 +176,12 @@ var projectMockServicesRateLimit = &ProjectServiceReports{
 					"create": &ProjectRateLimitActionReport{
 						Name:  "create",
 						Limit: 1000,
-						Unit:  UnitRequestsPerSeconds,
+						Unit:  UnitRequestsPerSecond,
 					},
 					"delete": &ProjectRateLimitActionReport{
 						Name:  "delete",
 						Limit: 1000,
-						Unit:  UnitRequestsPerSeconds,
+						Unit:  UnitRequestsPerSecond,
 					},
 				},
 			},
@@ -214,16 +214,16 @@ var projectMockServicesRateLimitDeviatingFromDefaults = &ProjectServiceReports{
 					"create": &ProjectRateLimitActionReport{
 						Name:         "create",
 						Limit:        1000,
-						Unit:         UnitRequestsPerSeconds,
+						Unit:         UnitRequestsPerSecond,
 						DefaultLimit: 500,
-						DefaultUnit:  UnitRequestsPerSeconds,
+						DefaultUnit:  UnitRequestsPerSecond,
 					},
 					"delete": &ProjectRateLimitActionReport{
 						Name:         "delete",
 						Limit:        1000,
-						Unit:         UnitRequestsPerSeconds,
+						Unit:         UnitRequestsPerSecond,
 						DefaultLimit: 500,
-						DefaultUnit:  UnitRequestsPerSeconds,
+						DefaultUnit:  UnitRequestsPerSecond,
 					},
 				},
 			},