Skip to content

Commit

Permalink
Merge pull request #157 from sapcc/refactor-updatedbresource
Browse files Browse the repository at this point in the history 
refactor UpdateDBResource
  • Loading branch information
@majewsky
majewsky authored Apr 13, 2023
2 parents bf9fe03 + 521e68b commit ca095b2
Show file tree
Hide file tree
Showing 7 changed files with 328 additions and 183 deletions.
2 changes: 1 addition & 1 deletion go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ require (
github.com/prometheus/client_golang v1.14.0
github.com/rs/cors v1.8.3
github.com/sapcc/go-api-declarations v1.5.0
github.com/sapcc/go-bits v0.0.0-20230413114037-7dc72a736b5f
github.com/sapcc/go-bits v0.0.0-20230413144132-bee3aa4c8293
github.com/sapcc/gophercloud-sapcc v0.0.0-20230404144305-447dc9e2299b
gopkg.in/yaml.v2 v2.4.0
)
Expand Down
4 changes: 2 additions & 2 deletions go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,8 @@ github.com/rs/cors v1.8.3 h1:O+qNyWn7Z+F9M0ILBHgMVPuB1xTOucVd5gtaYyXBpRo=
github.com/rs/cors v1.8.3/go.mod h1:XyqrcTp5zjWr1wsJ8PIRZssZ8b/WMcMf71DJnit4EMU=
github.com/sapcc/go-api-declarations v1.5.0 h1:inQyaR2khF4z2FE3GMFf5oMrWYgCo0qmuB7l14uIV2o=
github.com/sapcc/go-api-declarations v1.5.0/go.mod h1:EA4x5xxJGyG/3JTteUOYBg6cJLJgsBHrDj3ryfNU4dQ=
github.com/sapcc/go-bits v0.0.0-20230413114037-7dc72a736b5f h1:4uQH504BsF0oTfC/wfRI+CiB24FnjIUWCfkc9KSMhbY=
github.com/sapcc/go-bits v0.0.0-20230413114037-7dc72a736b5f/go.mod h1:VQnfDNRoJLdG/aAykdi24yTt3m/ZmjsythVg29w0Jm4=
github.com/sapcc/go-bits v0.0.0-20230413144132-bee3aa4c8293 h1:onJXQjRl2rTFa7JiSxjYpPwyhJHgnLt0mS8e75mPHaM=
github.com/sapcc/go-bits v0.0.0-20230413144132-bee3aa4c8293/go.mod h1:VQnfDNRoJLdG/aAykdi24yTt3m/ZmjsythVg29w0Jm4=
github.com/sapcc/gophercloud-sapcc v0.0.0-20230404144305-447dc9e2299b h1:er+ILBoB0NqXc24+KU+rTYBA/LoUvNvuc7T2kOarzgQ=
github.com/sapcc/gophercloud-sapcc v0.0.0-20230404144305-447dc9e2299b/go.mod h1:wFayW3xpPIVXviXkjcSkVWk/Wb7523BOL53lVo6EAFo=
github.com/sergi/go-diff v1.3.1 h1:xkr+Oxo4BOQKmkn/B9eMK0g5Kg/983T9DqqPHwYqD+8=
Expand Down
181 changes: 2 additions & 179 deletions internal/api/resources.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,7 @@ package api
import (
"database/sql"
"encoding/json"
"fmt"
"net/http"
"sort"
"strings"
"time"

"github.com/sapcc/go-api-declarations/cadf"
Expand All @@ -37,11 +34,6 @@ import (
"github.com/sapcc/castellum/internal/db"
)

////////////////////////////////////////////////////////////////////////////////
// data types

// Resource is how a db.Resource looks like in the API.

////////////////////////////////////////////////////////////////////////////////
// conversion and validation methods

Expand Down Expand Up @@ -94,172 +86,6 @@ func (h handler) ResourceFromDB(res db.Resource) (castellum.Resource, error) {
return result, nil
}

// UpdateDBResource updates the given db.Resource with the values provided in
// this api.Resource.
func UpdateDBResource(resource castellum.Resource, res *db.Resource, manager core.AssetManager, info core.AssetTypeInfo, maxAssetSize *uint64, existingResources []db.AssetType) (errors []string) {
complain := func(msg string, args ...interface{}) {
if len(args) > 0 {
msg = fmt.Sprintf(msg, args...)
}
errors = append(errors, msg)
}

if resource.ConfigJSON == nil {
res.ConfigJSON = ""
} else {
res.ConfigJSON = string(*resource.ConfigJSON)
}
err := manager.CheckResourceAllowed(res.AssetType, res.ScopeUUID, res.ConfigJSON, existingResources)
if err != nil {
complain(err.Error())
}

if resource.Checked != nil {
complain("resource.checked cannot be set via the API")
}
if resource.AssetCount != 0 {
complain("resource.asset_count cannot be set via the API")
}

//helper function to check the internal consistency of {Low,High,Critical}ThresholdPercent
checkThresholdCommon := func(tType string, vals castellum.UsageValues) {
isMetric := make(map[castellum.UsageMetric]bool)
for _, metric := range info.UsageMetrics {
isMetric[metric] = true
val, exists := vals[metric]
if !exists {
complain("missing %s threshold%s", tType, core.Identifier(metric, " for %s"))
continue
}
if val <= 0 || val > 100 {
complain("%s threshold%s must be above 0%% and below or at 100%% of usage", tType, core.Identifier(metric, " for %s"))
}
}

providedMetrics := make([]string, 0, len(vals))
for metric := range vals {
providedMetrics = append(providedMetrics, string(metric))
}
sort.Strings(providedMetrics) //for deterministic order of error messages in unit test
for _, metric := range providedMetrics {
if !isMetric[castellum.UsageMetric(metric)] {
complain("%s threshold specified for metric %q which is not valid for this asset type", tType, metric)
}
}
}

if resource.LowThreshold == nil {
res.LowThresholdPercent = info.MakeZeroUsageValues()
res.LowDelaySeconds = 0
} else {
res.LowThresholdPercent = resource.LowThreshold.UsagePercent
checkThresholdCommon("low", res.LowThresholdPercent)
res.LowDelaySeconds = resource.LowThreshold.DelaySeconds
if res.LowDelaySeconds == 0 {
complain("delay for low threshold is missing")
}
}

if resource.HighThreshold == nil {
res.HighThresholdPercent = info.MakeZeroUsageValues()
res.HighDelaySeconds = 0
} else {
res.HighThresholdPercent = resource.HighThreshold.UsagePercent
checkThresholdCommon("high", res.HighThresholdPercent)
res.HighDelaySeconds = resource.HighThreshold.DelaySeconds
if res.HighDelaySeconds == 0 {
complain("delay for high threshold is missing")
}
}

if resource.CriticalThreshold == nil {
res.CriticalThresholdPercent = info.MakeZeroUsageValues()
} else {
res.CriticalThresholdPercent = resource.CriticalThreshold.UsagePercent
checkThresholdCommon("critical", res.CriticalThresholdPercent)
if resource.CriticalThreshold.DelaySeconds != 0 {
complain("critical threshold may not have a delay")
}
}

if resource.LowThreshold != nil && resource.HighThreshold != nil {
for _, metric := range info.UsageMetrics {
if res.LowThresholdPercent[metric] > res.HighThresholdPercent[metric] {
complain("low threshold%s must be below high threshold", core.Identifier(metric, " for %s"))
}
}
}
if resource.LowThreshold != nil && resource.CriticalThreshold != nil {
for _, metric := range info.UsageMetrics {
if res.LowThresholdPercent[metric] > res.CriticalThresholdPercent[metric] {
complain("low threshold%s must be below critical threshold", core.Identifier(metric, " for %s"))
}
}
}
if resource.HighThreshold != nil && resource.CriticalThreshold != nil {
for _, metric := range info.UsageMetrics {
if res.HighThresholdPercent[metric] > res.CriticalThresholdPercent[metric] {
complain("high threshold%s must be below critical threshold", core.Identifier(metric, " for %s"))
}
}
}

if resource.LowThreshold == nil && resource.HighThreshold == nil && resource.CriticalThreshold == nil {
complain("at least one threshold must be configured")
}

res.SizeStepPercent = resource.SizeSteps.Percent
res.SingleStep = resource.SizeSteps.Single
if res.SingleStep {
if res.SizeStepPercent != 0 {
complain("percentage-based step may not be configured when single-step resizing is used")
}
} else {
if res.SizeStepPercent == 0 {
complain("size step must be greater than 0%")
}
}

if resource.SizeConstraints == nil {
if maxAssetSize != nil {
complain(fmt.Sprintf("maximum size must be configured for %s", info.AssetType))
}
res.MinimumSize = nil
res.MaximumSize = nil
res.MinimumFreeSize = nil
} else {
res.MinimumSize = resource.SizeConstraints.Minimum
if res.MinimumSize != nil && *res.MinimumSize == 0 {
res.MinimumSize = nil
}

res.MaximumSize = resource.SizeConstraints.Maximum
if res.MaximumSize == nil {
if maxAssetSize != nil {
complain(fmt.Sprintf("maximum size must be configured for %s", info.AssetType))
}
} else {
min := uint64(0)
if res.MinimumSize != nil {
min = *res.MinimumSize
}
if *res.MaximumSize <= min {
complain("maximum size must be greater than minimum size")
}
if maxAssetSize != nil && *res.MaximumSize > *maxAssetSize {
complain(fmt.Sprintf("maximum size must be %d or less", *maxAssetSize))
}
}

res.MinimumFreeSize = resource.SizeConstraints.MinimumFree
if res.MinimumFreeSize != nil && *res.MinimumFreeSize == 0 {
res.MinimumFreeSize = nil
}
}

return
}

////////////////////////////////////////////////////////////////////////////////
// HTTP handlers

Expand Down Expand Up @@ -337,9 +163,6 @@ func (h handler) PutResource(w http.ResponseWriter, r *http.Request) {
return
}

manager, info := h.Team.ForAssetType(dbResource.AssetType)
maxAssetSize := h.Config.MaxAssetSizeFor(info.AssetType)

action := cadf.UpdateAction
if dbResource.ID == 0 {
action = cadf.EnableAction
Expand Down Expand Up @@ -372,10 +195,10 @@ func (h handler) PutResource(w http.ResponseWriter, r *http.Request) {
return
}

errs := UpdateDBResource(input, dbResource, manager, info, maxAssetSize, existingResources)
errs := core.ApplyResourceSpecInto(dbResource, input, existingResources, h.Config, h.Team)
if len(errs) > 0 {
doAudit(http.StatusUnprocessableEntity)
http.Error(w, strings.Join(errs, "\n"), http.StatusUnprocessableEntity)
http.Error(w, errs.Join("\n"), http.StatusUnprocessableEntity)
return
}

Expand Down
Loading

0 comments on commit ca095b2

Please sign in to comment.