validate fields for property refs and produce config errors repoty #805
Conversation
| 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 |
There was a problem hiding this comment.
This doesn't do any escaping or anything. Might be safer to just return json.Marshal(map[string]any{ ... })
| @@ -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 { | |||
There was a problem hiding this comment.
Considering the infracopilot UI flow, it might not be possible to do required checks until we figure that out (unless that's something you're already doing). In other words, the resource has to be present to configure it in the UI, so if the validation fails on adding the resource, the user can't configure it to resolve that validation.
There was a problem hiding this comment.
yeah validation can fail, but the engine wont fail unless its correlated with an incoming resource constraint. This prevents the user from dragging in a lambda and the engine failing, but still gives us the validation info for the images BaseImage so we can block clicking export IAC until all fields pass
| graph := construct.NewGraph() | ||
| for _, r := range tt.testResources { | ||
| graph.AddVertex(r) | ||
| } |
There was a problem hiding this comment.
nit: Use graphtest and []any to make it easier (no tests use this atm though, so it doesn't matter). Same in a couple of the other tests
There was a problem hiding this comment.
i think its because i needed properties on my resources in some tests, cant remember which test this is
There was a problem hiding this comment.
ah yeah, right now i havent added the test for this, only for string and the isolated property ref tests, but it needs the entire resource because its going to trace back the property ref on the other resource to ensure the value of the field its referencing passes validation
| 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) |
There was a problem hiding this comment.
| return fmt.Errorf("invalid int value %v", value) | |
| return fmt.Errorf("invalid float value %v", value) |
jhsinger-klotho
force-pushed
the
config_errors
branch
from
5b19a6b
to
a93cd6f
Compare
| @@ -12,6 +12,6 @@ resources: | |||
| parent: vpc/vpc-0 | |||
|
|
|||
| ecs_service/ecs_service_0 -> rds_instance/rds-instance-2: | |||
| path: aws:ecs_task_definition:ecs_service_0,aws:iam_role:ecs_service_0-rds-instance-2 | |||
| path: aws:ecs_task_definition:ecs_service_0,aws:iam_role:ecs_service_0-execution-role | |||
There was a problem hiding this comment.
👍 fixed the bug w/ path expand vertex dependencies so this gets set by the op rule instead of expansion
Ensures each type validates that if set with a property ref, that ref is of the correct type and that refs value passes validation (if deploy time then it automatically passes if the correct type)
produces a config_errors.json for failed config
Going to add failure if the failed config matches something that was an input constraint
Standard checks