Skip to content

Commit

Permalink
validate fields for property refs and produce config errors repoty
Browse files Browse the repository at this point in the history
  • Loading branch information
Jordan Singer committed Dec 6, 2023
1 parent 9d5e75a commit 5b19a6b
Show file tree
Hide file tree
Showing 30 changed files with 572 additions and 80 deletions.
12 changes: 12 additions & 0 deletions pkg/engine2/cli.go
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,18 @@ func (em *EngineMain) RunEngine(cmd *cobra.Command, args []string) error {
},
)

configErrors := em.Engine.getPropertyValidation(context.Solutions[0])
if len(configErrors) > 0 {
configErrorData, err := json.Marshal(configErrors)
if err != nil {
return errors.Errorf("failed to marshal config errors: %s", err.Error())
}
files = append(files, &io.RawFile{
FPath: "config-errors.json",
Content: configErrorData,
})
}

err = io.OutputTo(files, architectureEngineCfg.outputDir)
if err != nil {
return errors.Errorf("failed to write output files: %s", err.Error())
Expand Down
13 changes: 13 additions & 0 deletions pkg/engine2/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,3 +43,16 @@ func (e *Engine) Run(context *EngineContext) error {
context.Solutions = append(context.Solutions, solutionCtx)
return err
}

func (e *Engine) getPropertyValidation(ctx solution_context.SolutionContext) []solution_context.PropertyValidationDecision {
decisions := ctx.GetDecisions().GetRecords()
validationDecisions := make([]solution_context.PropertyValidationDecision, 0)
for _, decision := range decisions {
if validation, ok := decision.(solution_context.PropertyValidationDecision); ok {
if validation.Error != nil {
validationDecisions = append(validationDecisions, validation)
}
}
}
return validationDecisions
}
5 changes: 5 additions & 0 deletions pkg/engine2/enginetesting/mock_solution.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,11 @@ func (m *MockSolution) RecordDecision(d solution_context.SolveDecision) {
m.Called(d)
}

func (m *MockSolution) GetDecisions() solution_context.DecisionRecords {
args := m.Called()
return args.Get(0).(solution_context.DecisionRecords)
}

func (m *MockSolution) DataflowGraph() construct.Graph {
args := m.Called()
return args.Get(0).(construct.Graph)
Expand Down
4 changes: 4 additions & 0 deletions pkg/engine2/enginetesting/test_solution.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,10 @@ func (sol *TestSolution) Constraints() *constraints.Constraints {

func (sol *TestSolution) RecordDecision(d solution_context.SolveDecision) {}

func (sol *TestSolution) GetDecisions() solution_context.DecisionRecords {
return nil
}

func (sol *TestSolution) DataflowGraph() construct.Graph {
return sol.dataflow
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/engine2/operational_eval/vertex_path_expand.go
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ func (v *pathExpandVertex) addDepsFromProps(
continue
}
// if this dependency could pass validation for the resources property, consider it as a dependent vertex
if err := prop.Validate(resource, dep); err == nil {
if err := prop.Validate(resource, dep, solution_context.DynamicCtx(eval.Solution)); err == nil {
changes.addEdge(v.Key(), Key{Ref: ref})
}
}
Expand Down
12 changes: 12 additions & 0 deletions pkg/engine2/operational_eval/vertex_property.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,18 @@ func (v *propertyVertex) Evaluate(eval *Evaluator) error {
return eval.AddResources(res)
}

// Now that the vertex is evaluated, we will check it for validity and record our decision
val, err := res.GetProperty(v.Ref.Property)
if err != nil {
return fmt.Errorf("error while validating resource property: could not get property %s on resource %s: %w", v.Ref.Property, v.Ref.Resource, err)
}
err = v.Template.Validate(res, val, solution_context.DynamicCtx(eval.Solution))
eval.Solution.RecordDecision(solution_context.PropertyValidationDecision{
Resource: v.Ref.Resource,
Property: v.Template,
Value: val,
Error: err,
})
return nil
}

Expand Down
36 changes: 30 additions & 6 deletions pkg/engine2/solution_context/decisions.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package solution_context

import (
"fmt"

construct "github.com/klothoplatform/klotho/pkg/construct2"
knowledgebase "github.com/klothoplatform/klotho/pkg/knowledge_base2"
)
Expand All @@ -18,6 +20,7 @@ type (
// FindDecision(decision SolveDecision) []KV
// // FindContext returns the various decisions (the what) for a given context (the why)
// FindContext(key string, value any) []SolveDecision
GetRecords() []SolveDecision
}

SolveDecision interface {
Expand Down Expand Up @@ -50,14 +53,35 @@ type (
Value any
}

ResourceConfigurationError struct {
PropertyValidationDecision struct {
Resource construct.ResourceId
Property knowledgebase.Property
Value any
Error error
}
)

func (d AddResourceDecision) internal() {}
func (d AddDependencyDecision) internal() {}
func (d RemoveResourceDecision) internal() {}
func (d RemoveDependencyDecision) internal() {}
func (d SetPropertyDecision) internal() {}
func (d AddResourceDecision) internal() {}
func (d AddDependencyDecision) internal() {}
func (d RemoveResourceDecision) internal() {}
func (d RemoveDependencyDecision) internal() {}
func (d SetPropertyDecision) internal() {}
func (d PropertyValidationDecision) internal() {}

func (d PropertyValidationDecision) MarshalJSON() ([]byte, error) {
if d.Value != nil {
stringVal := `{
"resource": "%s",
"property": "%s",
"value": "%s",
"error": "%s"
}`
return []byte(fmt.Sprintf(stringVal, d.Resource, d.Property.Details().Path, d.Value, d.Error)), nil
}
stringVal := `{
"resource": "%s",
"property": "%s",
"error": "%s"
}`
return []byte(fmt.Sprintf(stringVal, d.Resource, d.Property.Details().Path, d.Error)), nil
}
1 change: 1 addition & 0 deletions pkg/engine2/solution_context/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ type (
KnowledgeBase() knowledgebase.TemplateKB
Constraints() *constraints.Constraints
RecordDecision(d SolveDecision)
GetDecisions() DecisionRecords

DataflowGraph() construct.Graph
DeploymentGraph() construct.Graph
Expand Down
8 changes: 8 additions & 0 deletions pkg/engine2/solution_context/memory_record.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,11 @@ type (
func (m *MemoryRecord) AddRecord(context []KV, decision SolveDecision) {
m.records = append(m.records, record{context: context, decision: decision})
}

func (m *MemoryRecord) GetRecords() []SolveDecision {
var decisions []SolveDecision
for _, record := range m.records {
decisions = append(decisions, record.decision)
}
return decisions
}
7 changes: 6 additions & 1 deletion pkg/knowledge_base2/properties/any_property.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package properties

import (
"fmt"

construct "github.com/klothoplatform/klotho/pkg/construct2"
knowledgebase "github.com/klothoplatform/klotho/pkg/knowledge_base2"
)
Expand Down Expand Up @@ -107,7 +109,10 @@ func (a *AnyProperty) Type() string {
return "any"
}

func (a *AnyProperty) Validate(resource *construct.Resource, value any) error {
func (a *AnyProperty) Validate(resource *construct.Resource, value any, ctx knowledgebase.DynamicContext) error {
if a.Required && value == nil {
return fmt.Errorf(knowledgebase.ErrRequiredProperty, a.Path, resource.ID)
}
return nil
}

Expand Down
8 changes: 7 additions & 1 deletion pkg/knowledge_base2/properties/bool_property.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,13 @@ func (b *BoolProperty) Type() string {
return "bool"
}

func (b *BoolProperty) Validate(resource *construct.Resource, value any) error {
func (b *BoolProperty) Validate(resource *construct.Resource, value any, ctx knowledgebase.DynamicContext) error {
if value == nil {
if b.Required {
return fmt.Errorf(knowledgebase.ErrRequiredProperty, b.Path, resource.ID)
}
return nil
}
if _, ok := value.(bool); !ok {
return fmt.Errorf("invalid bool value %v", value)
}
Expand Down
26 changes: 21 additions & 5 deletions pkg/knowledge_base2/properties/bool_property_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@ import (
"testing"

construct "github.com/klothoplatform/klotho/pkg/construct2"
"github.com/klothoplatform/klotho/pkg/engine2/enginetesting"
knowledgebase "github.com/klothoplatform/klotho/pkg/knowledge_base2"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
)

func Test_SetBoolProperty(t *testing.T) {
Expand Down Expand Up @@ -268,10 +270,12 @@ func Test_BoolPropertyType(t *testing.T) {

func Test_BoolPropertyValidate(t *testing.T) {
tests := []struct {
name string
property *BoolProperty
value any
wantErr bool
name string
property *BoolProperty
testResources []*construct.Resource
mockKBCalls []mock.Call
value any
wantErr bool
}{
{
name: "bool property",
Expand All @@ -298,7 +302,19 @@ func Test_BoolPropertyValidate(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
assert := assert.New(t)
resource := &construct.Resource{}
err := tt.property.Validate(resource, tt.value)
graph := construct.NewGraph()
for _, r := range tt.testResources {
graph.AddVertex(r)
}
mockKB := &enginetesting.MockKB{}
for _, call := range tt.mockKBCalls {
mockKB.On(call.Method, call.Arguments...).Return(call.ReturnArguments...)
}
ctx := knowledgebase.DynamicValueContext{
Graph: graph,
KnowledgeBase: mockKB,
}
err := tt.property.Validate(resource, tt.value, ctx)
assert.Equal(tt.wantErr, err != nil)
})
}
Expand Down
8 changes: 7 additions & 1 deletion pkg/knowledge_base2/properties/float_property.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,13 @@ func (f *FloatProperty) Type() string {
return "float"
}

func (f *FloatProperty) Validate(resource *construct.Resource, value any) error {
func (f *FloatProperty) Validate(resource *construct.Resource, value any, ctx knowledgebase.DynamicContext) error {
if value == nil {
if f.Required {
return fmt.Errorf(knowledgebase.ErrRequiredProperty, f.Path, resource.ID)
}
return nil
}
floatVal, ok := value.(float64)
if !ok {
return fmt.Errorf("invalid int value %v", value)
Expand Down
27 changes: 22 additions & 5 deletions pkg/knowledge_base2/properties/float_property_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,11 @@ import (
"testing"

construct "github.com/klothoplatform/klotho/pkg/construct2"
"github.com/klothoplatform/klotho/pkg/engine2/enginetesting"
knowledgebase "github.com/klothoplatform/klotho/pkg/knowledge_base2"
knowledgebase2 "github.com/klothoplatform/klotho/pkg/knowledge_base2"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
)

func Test_SetFloatProperty(t *testing.T) {
Expand Down Expand Up @@ -196,10 +199,12 @@ func Test_ParseFloatValue(t *testing.T) {

func Test_FloatProperty_Validate(t *testing.T) {
tests := []struct {
name string
property *FloatProperty
value any
wantErr bool
name string
property *FloatProperty
testResources []*construct.Resource
mockKBCalls []mock.Call
value any
wantErr bool
}{
{
name: "float value",
Expand All @@ -225,7 +230,19 @@ func Test_FloatProperty_Validate(t *testing.T) {
t.Run(test.name, func(t *testing.T) {
assert := assert.New(t)
resource := &construct.Resource{}
err := test.property.Validate(resource, test.value)
graph := construct.NewGraph()
for _, r := range test.testResources {
graph.AddVertex(r)
}
mockKB := &enginetesting.MockKB{}
for _, call := range test.mockKBCalls {
mockKB.On(call.Method, call.Arguments...).Return(call.ReturnArguments...)
}
ctx := knowledgebase.DynamicValueContext{
Graph: graph,
KnowledgeBase: mockKB,
}
err := test.property.Validate(resource, test.value, ctx)
if test.wantErr {
assert.Error(err)
return
Expand Down
8 changes: 7 additions & 1 deletion pkg/knowledge_base2/properties/int_property.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,13 @@ func (i *IntProperty) Type() string {
return "int"
}

func (i *IntProperty) Validate(resource *construct.Resource, value any) error {
func (i *IntProperty) Validate(resource *construct.Resource, value any, ctx knowledgebase.DynamicContext) error {
if value == nil {
if i.Required {
return fmt.Errorf(knowledgebase.ErrRequiredProperty, i.Path, resource.ID)
}
return nil
}
intVal, ok := value.(int)
if !ok {
return fmt.Errorf("invalid int value %v", value)
Expand Down
27 changes: 22 additions & 5 deletions pkg/knowledge_base2/properties/int_property_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,11 @@ import (
"testing"

construct "github.com/klothoplatform/klotho/pkg/construct2"
"github.com/klothoplatform/klotho/pkg/engine2/enginetesting"
knowledgebase "github.com/klothoplatform/klotho/pkg/knowledge_base2"
knowledgebase2 "github.com/klothoplatform/klotho/pkg/knowledge_base2"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
)

func Test_SetIntPropertyProperty(t *testing.T) {
Expand Down Expand Up @@ -188,10 +191,12 @@ func Test_IntProperty_Validate(t *testing.T) {
upperBound := 10
lowerBound := 0
tests := []struct {
name string
property *IntProperty
value any
wantErr bool
name string
property *IntProperty
testResources []*construct.Resource
mockKBCalls []mock.Call
value any
wantErr bool
}{
{
name: "int property",
Expand Down Expand Up @@ -252,7 +257,19 @@ func Test_IntProperty_Validate(t *testing.T) {
t.Run(test.name, func(t *testing.T) {
assert := assert.New(t)
resource := &construct.Resource{}
err := test.property.Validate(resource, test.value)
graph := construct.NewGraph()
for _, r := range test.testResources {
graph.AddVertex(r)
}
mockKB := &enginetesting.MockKB{}
for _, call := range test.mockKBCalls {
mockKB.On(call.Method, call.Arguments...).Return(call.ReturnArguments...)
}
ctx := knowledgebase.DynamicValueContext{
Graph: graph,
KnowledgeBase: mockKB,
}
err := test.property.Validate(resource, test.value, ctx)
if test.wantErr {
assert.Error(err)
return
Expand Down
Loading

0 comments on commit 5b19a6b

Please sign in to comment.