Skip to content
This repository has been archived by the owner on Oct 9, 2023. It is now read-only.

Added validation check to impose limits on stored, free-form strings #180

Merged
merged 13 commits into from
Aug 18, 2021
2 changes: 2 additions & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ require (
github.com/pquerna/cachecontrol v0.0.0-20201205024021-ac21108117ac // indirect
github.com/prometheus/client_golang v1.9.0
github.com/prometheus/common v0.19.0 // indirect
github.com/qor/qor v1.2.0 // indirect
github.com/qor/validations v0.0.0-20171228122639-f364bca61b46
github.com/sendgrid/rest v2.6.4+incompatible // indirect
github.com/sendgrid/sendgrid-go v3.10.0+incompatible
github.com/sirupsen/logrus v1.8.1 // indirect
Expand Down
54 changes: 51 additions & 3 deletions go.sum

Large diffs are not rendered by default.

2 changes: 2 additions & 0 deletions pkg/repositories/config/postgres.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (

"github.com/jinzhu/gorm"
_ "github.com/jinzhu/gorm/dialects/postgres" // Required to import database driver.
"github.com/qor/validations"
)

const Postgres = "postgres"
Expand Down Expand Up @@ -77,5 +78,6 @@ func OpenDbConnection(config DbConnectionConfigProvider) *gorm.DB {
panic(err)
}
db.LogMode(config.IsDebug())
validations.RegisterCallbacks(db)
return db
}
22 changes: 11 additions & 11 deletions pkg/repositories/models/execution.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,19 @@ import (

// Execution primary key
type ExecutionKey struct {
Project string `gorm:"primary_key;column:execution_project"`
Domain string `gorm:"primary_key;column:execution_domain"`
Name string `gorm:"primary_key;column:execution_name"`
Project string `gorm:"primary_key;column:execution_project" valid:"length(0|255)"`
Domain string `gorm:"primary_key;column:execution_domain" valid:"length(0|255)"`
Name string `gorm:"primary_key;column:execution_name" valid:"length(0|255)"`
}

// Database model to encapsulate a (workflow) execution.
type Execution struct {
BaseModel
ExecutionKey
LaunchPlanID uint `gorm:"index"`
WorkflowID uint `gorm:"index"`
TaskID uint `gorm:"index"`
Phase string
LaunchPlanID uint `gorm:"index"`
WorkflowID uint `gorm:"index"`
TaskID uint `gorm:"index"`
Phase string `valid:"length(0|255)"`
Closure []byte
Spec []byte `gorm:"not null"`
StartedAt *time.Time
Expand All @@ -37,7 +37,7 @@ type Execution struct {
ExecutionEvents []ExecutionEvent
// In the case of an aborted execution this string may be non-empty.
// It should be ignored for any other value of phase other than aborted.
AbortCause string
AbortCause string `valid:"length(0|255)"`
// Corresponds to the execution mode used to trigger this execution
Mode int32
// The "parent" execution (if there is one) that is related to this execution.
Expand All @@ -47,16 +47,16 @@ type Execution struct {
// The parent node execution if this was launched by a node
ParentNodeExecutionID uint
// Cluster where execution was triggered
Cluster string
Cluster string `valid:"length(0|255)"`
// Offloaded location of inputs LiteralMap. These are the inputs evaluated and contain applied defaults.
InputsURI storage.DataReference
// User specified inputs. This map might be incomplete and not include defaults applied
UserInputsURI storage.DataReference
// Execution Error Kind. nullable
ErrorKind *string `gorm:"index"`
// Execution Error Code nullable
ErrorCode *string
ErrorCode *string `valid:"length(0|255)"`
// The user responsible for launching this execution.
// This is also stored in the spec but promoted as a column for filtering.
User string `gorm:"index"`
User string `gorm:"index" valid:"length(0|255)"`
}
2 changes: 1 addition & 1 deletion pkg/repositories/models/execution_event.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (
type ExecutionEvent struct {
BaseModel
ExecutionKey
RequestID string
RequestID string `valid:"length(0|255)"`
OccurredAt time.Time
Phase string `gorm:"primary_key"`
}
8 changes: 4 additions & 4 deletions pkg/repositories/models/launch_plan.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@ package models

// Launch plan primary key
type LaunchPlanKey struct {
Project string `gorm:"primary_key;index:lp_project_domain_name_idx,lp_project_domain_idx"`
Domain string `gorm:"primary_key;index:lp_project_domain_name_idx,lp_project_domain_idx"`
Name string `gorm:"primary_key;index:lp_project_domain_name_idx"`
Version string `gorm:"primary_key"`
Project string `gorm:"primary_key;index:lp_project_domain_name_idx,lp_project_domain_idx" valid:"length(0|255)"`
Domain string `gorm:"primary_key;index:lp_project_domain_name_idx,lp_project_domain_idx" valid:"length(0|255)"`
Name string `gorm:"primary_key;index:lp_project_domain_name_idx" valid:"length(0|255)"`
Version string `gorm:"primary_key" valid:"length(0|255)"`
}

type LaunchPlanScheduleType string
Expand Down
14 changes: 7 additions & 7 deletions pkg/repositories/models/named_entity.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@ import (

// NamedEntityMetadata primary key
type NamedEntityMetadataKey struct {
ResourceType core.ResourceType `gorm:"primary_key;index:named_entity_metadata_type_project_domain_name_idx"`
Project string `gorm:"primary_key;index:named_entity_metadata_type_project_domain_name_idx"`
Domain string `gorm:"primary_key;index:named_entity_metadata_type_project_domain_name_idx"`
Name string `gorm:"primary_key;index:named_entity_metadata_type_project_domain_name_idx"`
ResourceType core.ResourceType `gorm:"primary_key;index:named_entity_metadata_type_project_domain_name_idx" valid:"length(0|255)"`
Project string `gorm:"primary_key;index:named_entity_metadata_type_project_domain_name_idx" valid:"length(0|255)"`
Domain string `gorm:"primary_key;index:named_entity_metadata_type_project_domain_name_idx" valid:"length(0|255)"`
Name string `gorm:"primary_key;index:named_entity_metadata_type_project_domain_name_idx" valid:"length(0|255)"`
}

// Fields to be composed into any named entity
Expand All @@ -30,9 +30,9 @@ type NamedEntityMetadata struct {
// fields here should match the ones in NamedEntityMetadataKey.
type NamedEntityKey struct {
ResourceType core.ResourceType
Project string
Domain string
Name string
Project string `valid:"length(0|255)"`
Domain string `valid:"length(0|255)"`
Name string `valid:"length(0|255)"`
}

// Composes an identifier (NamedEntity) and its associated metadata fields
Expand Down
2 changes: 1 addition & 1 deletion pkg/repositories/models/node_execution.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (

type NodeExecutionKey struct {
ExecutionKey
NodeID string `gorm:"primary_key;index"`
NodeID string `gorm:"primary_key;index" valid:"length(0|255)"`
}

// By convention, gorm foreign key references are of the form {ModelName}ID
Expand Down
2 changes: 1 addition & 1 deletion pkg/repositories/models/project.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package models
type Project struct {
BaseModel
Identifier string `gorm:"primary_key"`
Name string // Human-readable name, not a unique identifier.
Name string `valid:"length(0|255)"` // Human-readable name, not a unique identifier.
Description string `gorm:"type:varchar(300)"`
Labels []byte
// GORM doesn't save the zero value for ints, so we use a pointer for the State field
Expand Down
10 changes: 5 additions & 5 deletions pkg/repositories/models/resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,11 @@ type Resource struct {
CreatedAt time.Time
UpdatedAt time.Time
DeletedAt *time.Time `sql:"index"`
Project string `gorm:"unique_index:resource_idx"`
Domain string `gorm:"unique_index:resource_idx"`
Workflow string `gorm:"unique_index:resource_idx"`
LaunchPlan string `gorm:"unique_index:resource_idx"`
ResourceType string `gorm:"unique_index:resource_idx"`
Project string `gorm:"unique_index:resource_idx" valid:"length(0|255)"`
Domain string `gorm:"unique_index:resource_idx" valid:"length(0|255)"`
Workflow string `gorm:"unique_index:resource_idx" valid:"length(0|255)"`
LaunchPlan string `gorm:"unique_index:resource_idx" valid:"length(0|255)"`
ResourceType string `gorm:"unique_index:resource_idx" valid:"length(0|255)"`
Priority ResourcePriority
// Serialized flyteidl.admin.MatchingAttributes.
Attributes []byte
Expand Down
10 changes: 5 additions & 5 deletions pkg/repositories/models/task.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@ package models

// Task primary key
type TaskKey struct {
Project string `gorm:"primary_key;index:task_project_domain_name_idx,task_project_domain_idx"`
Domain string `gorm:"primary_key;index:task_project_domain_name_idx,task_project_domain_idx"`
Name string `gorm:"primary_key;index:task_project_domain_name_idx"`
Version string `gorm:"primary_key"`
Project string `gorm:"primary_key;index:task_project_domain_name_idx,task_project_domain_idx" valid:"length(0|255)"`
Domain string `gorm:"primary_key;index:task_project_domain_name_idx,task_project_domain_idx" valid:"length(0|255)"`
Name string `gorm:"primary_key;index:task_project_domain_name_idx" valid:"length(0|255)"`
Version string `gorm:"primary_key" valid:"length(0|255)"`
}

// Database model to encapsulate a task.
Expand All @@ -19,5 +19,5 @@ type Task struct {
// Hash of the compiled task closure
Digest []byte
// Task type (also stored in the closure put promoted as a column for filtering).
Type string
Type string `valid:"length(0|255)"`
}
4 changes: 2 additions & 2 deletions pkg/repositories/models/task_execution.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@ type TaskExecutionKey struct {
type TaskExecution struct {
BaseModel
TaskExecutionKey
Phase string
Phase string `valid:"length(0|255)"`
PhaseVersion uint32
InputURI string
InputURI string `valid:"length(0|255)"`
Closure []byte
StartedAt *time.Time
// Corresponds to the CreatedAt field in the TaskExecution closure
Expand Down
8 changes: 4 additions & 4 deletions pkg/repositories/models/workflow.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ package models

// Workflow primary key
type WorkflowKey struct {
Project string `gorm:"primary_key;index:workflow_project_domain_name_idx,workflow_project_domain_idx"`
Domain string `gorm:"primary_key;index:workflow_project_domain_name_idx,workflow_project_domain_idx"`
Name string `gorm:"primary_key;index:workflow_project_domain_name_idx"`
Project string `gorm:"primary_key;index:workflow_project_domain_name_idx,workflow_project_domain_idx" valid:"length(0|255)"`
Domain string `gorm:"primary_key;index:workflow_project_domain_name_idx,workflow_project_domain_idx" valid:"length(0|255)"`
Name string `gorm:"primary_key;index:workflow_project_domain_name_idx" valid:"length(0|255)"`
Version string `gorm:"primary_key"`
}

Expand All @@ -13,7 +13,7 @@ type Workflow struct {
BaseModel
WorkflowKey
TypedInterface []byte
RemoteClosureIdentifier string `gorm:"not null"`
RemoteClosureIdentifier string `gorm:"not null" valid:"length(0|255)"`
LaunchPlans []LaunchPlan
Executions []Execution
// Hash of the compiled workflow closure
Expand Down
9 changes: 9 additions & 0 deletions tests/attributes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package tests
import (
"context"
"testing"
"fmt"

"github.com/golang/protobuf/proto"

Expand Down Expand Up @@ -42,13 +43,15 @@ func TestUpdateProjectDomainAttributes(t *testing.T) {
}

_, err := client.UpdateProjectDomainAttributes(ctx, &req)
fmt.Println(err)
assert.Nil(t, err)

response, err := client.GetProjectDomainAttributes(ctx, &admin.ProjectDomainAttributesGetRequest{
Project: "admintests",
Domain: "development",
ResourceType: admin.MatchableResource_TASK_RESOURCE,
})
fmt.Println(err)
assert.Nil(t, err)
assert.True(t, proto.Equal(&admin.ProjectDomainAttributesGetResponse{
Attributes: &admin.ProjectDomainAttributes{
Expand All @@ -64,6 +67,7 @@ func TestUpdateProjectDomainAttributes(t *testing.T) {
Workflow: "workflow",
ResourceType: admin.MatchableResource_TASK_RESOURCE,
})
fmt.Println(err)
assert.Nil(t, err)
// Testing that if overrides are not set at workflow level, the one from Project-Domain is returned
assert.True(t, proto.Equal(&admin.WorkflowAttributesGetResponse{
Expand All @@ -80,13 +84,15 @@ func TestUpdateProjectDomainAttributes(t *testing.T) {
Domain: "development",
ResourceType: admin.MatchableResource_TASK_RESOURCE,
})
fmt.Println(err)
assert.Nil(t, err)

response, err = client.GetProjectDomainAttributes(ctx, &admin.ProjectDomainAttributesGetRequest{
Project: "admintests",
Domain: "development",
ResourceType: admin.MatchableResource_TASK_RESOURCE,
})
fmt.Println(err)
assert.Nil(t, response)
assert.EqualError(t, err, "rpc error: code = NotFound desc = Resource [{Project:admintests Domain:development Workflow: LaunchPlan: ResourceType:TASK_RESOURCE}] not found")
}
Expand All @@ -110,6 +116,7 @@ func TestUpdateWorkflowAttributes(t *testing.T) {
}

_, err := client.UpdateWorkflowAttributes(ctx, &req)
fmt.Println(err)
assert.Nil(t, err)

response, err := client.GetWorkflowAttributes(ctx, &admin.WorkflowAttributesGetRequest{
Expand All @@ -118,6 +125,7 @@ func TestUpdateWorkflowAttributes(t *testing.T) {
Workflow: "workflow",
ResourceType: admin.MatchableResource_TASK_RESOURCE,
})
fmt.Println(err)
assert.Nil(t, err)
assert.True(t, proto.Equal(&admin.WorkflowAttributesGetResponse{
Attributes: &admin.WorkflowAttributes{
Expand All @@ -134,6 +142,7 @@ func TestUpdateWorkflowAttributes(t *testing.T) {
Workflow: "workflow",
ResourceType: admin.MatchableResource_TASK_RESOURCE,
})
fmt.Println(err)
assert.Nil(t, err)

_, err = client.GetWorkflowAttributes(ctx, &admin.WorkflowAttributesGetRequest{
Expand Down