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

fix(database): add limit and eviction policy for deployment builds #355

Merged
merged 1 commit into from
Feb 6, 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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions constants/limit.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,7 @@ const (

// TopicsMaxSize defines the maximum size in characters for repo topics. Ex: GitHub has a 20-topic, 50-char limit.
TopicsMaxSize = 1020

// DeployBuildsMaxSize defines the maximum size in characters for deployment builds.
DeployBuildsMaxSize = 500
)
17 changes: 17 additions & 0 deletions database/deployment.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"errors"
"fmt"

"github.com/go-vela/types/constants"
"github.com/go-vela/types/library"
"github.com/go-vela/types/raw"
"github.com/lib/pq"
Expand Down Expand Up @@ -152,6 +153,22 @@ func (d *Deployment) Validate() error {
d.Target = sql.NullString{String: sanitize(d.Target.String), Valid: d.Target.Valid}
d.Description = sql.NullString{String: sanitize(d.Description.String), Valid: d.Description.Valid}

// calculate total size of builds
total := 0
for _, b := range d.Builds {
total += len(b)
}

// verify the Builds field is within the database constraints and evict if not
// len is to factor in number of comma separators included in the database field,
// removing 1 due to the last item not having an appended comma
if diff := (total + len(d.Builds) - 1) - constants.DeployBuildsMaxSize; diff > 0 {
for diff > 0 {
diff = diff - (len(d.Builds[0]) + 1)
d.Builds = d.Builds[1:]
}
}

return nil
}

Expand Down
47 changes: 47 additions & 0 deletions database/deployment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ func TestDatabase_Deployment_Validate(t *testing.T) {
tests := []struct {
failure bool
deployment *Deployment
want *Deployment
}{
{
failure: false,
Expand All @@ -140,13 +141,47 @@ func TestDatabase_Deployment_Validate(t *testing.T) {
ID: sql.NullInt64{Int64: 1, Valid: true},
RepoID: sql.NullInt64{Int64: 1, Valid: true},
},
want: &Deployment{
ID: sql.NullInt64{Int64: 1, Valid: true},
RepoID: sql.NullInt64{Int64: 1, Valid: true},
},
},
{ // no repoID set for deployment
failure: true,
deployment: &Deployment{
ID: sql.NullInt64{Int64: 1, Valid: true},
Number: sql.NullInt64{Int64: 1, Valid: true},
},
want: &Deployment{
ID: sql.NullInt64{Int64: 1, Valid: true},
RepoID: sql.NullInt64{Int64: 1, Valid: true},
},
},
{ // too many builds
failure: true,
deployment: &Deployment{
ID: sql.NullInt64{Int64: 1, Valid: true},
Number: sql.NullInt64{Int64: 1, Valid: true},
Builds: generateBuilds(100),
},
want: &Deployment{
ID: sql.NullInt64{Int64: 1, Valid: true},
RepoID: sql.NullInt64{Int64: 1, Valid: true},
Builds: generateBuilds(50),
},
},
{ // acceptable builds
failure: true,
deployment: &Deployment{
ID: sql.NullInt64{Int64: 1, Valid: true},
Number: sql.NullInt64{Int64: 1, Valid: true},
Builds: generateBuilds(30),
},
want: &Deployment{
ID: sql.NullInt64{Int64: 1, Valid: true},
RepoID: sql.NullInt64{Int64: 1, Valid: true},
Builds: generateBuilds(30),
},
},
}

Expand Down Expand Up @@ -253,3 +288,15 @@ func testDeployment() *Deployment {
Builds: pq.StringArray{"1"},
}
}

// generateBuilds returns a list of valid builds that exceed the maximum size.
func generateBuilds(amount int) []string {
// initialize empty builds
builds := []string{}

for i := 0; i < amount; i++ {
builds = append(builds, "123456789")
}

return builds
}