Skip to content

Commit

Permalink
handle rule value checks from non resource ids/refs
Browse files Browse the repository at this point in the history
  • Loading branch information
Jordan Singer committed Apr 17, 2024
1 parent 3d5db86 commit dfa9905
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 15 deletions.
33 changes: 21 additions & 12 deletions pkg/engine/operational_rule/operational_step.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,10 @@ func (ctx OperationalRuleContext) HandleOperationalStep(step knowledgebase.Opera
// If we are replacing we want to remove all dependencies and clear the property
// otherwise we want to add dependencies from the property and gather the resources which satisfy the step
var ids []construct.ResourceId
var otherValues []any
if ctx.Property != nil {
var err error
ids, err = ctx.addDependenciesFromProperty(step, resource, ctx.Property.Details().Path)
ids, otherValues, err = ctx.addDependenciesFromProperty(step, resource, ctx.Property.Details().Path)
if err != nil {
return err
}
Expand All @@ -50,7 +51,9 @@ func (ctx OperationalRuleContext) HandleOperationalStep(step knowledgebase.Opera
}
}

if len(ids) >= step.NumNeeded && step.NumNeeded > 0 || resource.Imported {
numValues := len(ids) + len(otherValues)

if numValues >= step.NumNeeded && step.NumNeeded > 0 || resource.Imported {
return nil
}

Expand All @@ -61,7 +64,7 @@ func (ctx OperationalRuleContext) HandleOperationalStep(step knowledgebase.Opera
action := operationalResourceAction{
Step: step,
CurrentIds: ids,
numNeeded: step.NumNeeded - len(ids),
numNeeded: step.NumNeeded - numValues,
ruleCtx: ctx,
}
return action.handleOperationalResourceAction(resource)
Expand Down Expand Up @@ -98,17 +101,19 @@ func (ctx OperationalRuleContext) getResourcesForStep(step knowledgebase.Operati
return resourcesOfType, nil
}

// addDependenciesFromProperty adds dependencies from the property of the resource
// and returns the resource ids and other values that were found in the property
func (ctx OperationalRuleContext) addDependenciesFromProperty(
step knowledgebase.OperationalStep,
resource *construct.Resource,
propertyName string,
) ([]construct.ResourceId, error) {
) ([]construct.ResourceId, []any, error) {
val, err := resource.GetProperty(propertyName)
if err != nil {
return nil, fmt.Errorf("error getting property %s on resource %s: %w", propertyName, resource.ID, err)
return nil, nil, fmt.Errorf("error getting property %s on resource %s: %w", propertyName, resource.ID, err)
}
if val == nil {
return nil, nil
return nil, nil, nil
}

addDep := func(id construct.ResourceId) error {
Expand All @@ -129,20 +134,21 @@ func (ctx OperationalRuleContext) addDependenciesFromProperty(
switch val := val.(type) {
case construct.ResourceId:
if val.IsZero() {
return nil, nil
return nil, nil, nil
}
return []construct.ResourceId{val}, addDep(val)
return []construct.ResourceId{val}, nil, addDep(val)

case []construct.ResourceId:
var errs error
for _, id := range val {
errs = errors.Join(errs, addDep(id))
}
return val, errs
return val, nil, errs

case []any:
var errs error
var ids []construct.ResourceId
var otherValues []any
for _, elem := range val {
switch elem := elem.(type) {
case construct.ResourceId:
Expand All @@ -152,14 +158,17 @@ func (ctx OperationalRuleContext) addDependenciesFromProperty(
case construct.PropertyRef:
ids = append(ids, elem.Resource)
errs = errors.Join(errs, addDep(elem.Resource))
case any:
otherValues = append(otherValues, elem)
}
}
return ids, errs
return ids, otherValues, errs

case construct.PropertyRef:
return []construct.ResourceId{val.Resource}, addDep(val.Resource)
return []construct.ResourceId{val.Resource}, nil, addDep(val.Resource)
default:
return nil, []any{val}, nil
}
return nil, fmt.Errorf("cannot add dependencies from property %s on resource %s, due to it being type %s", propertyName, resource.ID, reflect.TypeOf(val))
}

func (ctx OperationalRuleContext) clearProperty(step knowledgebase.OperationalStep, resource *construct.Resource, propertyName string) error {
Expand Down
41 changes: 40 additions & 1 deletion pkg/engine/operational_rule/operational_step_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ func Test_addDependenciesFromProperty(t *testing.T) {
initialState []any
want enginetesting.ExpectedGraphs
wantIds []construct.ResourceId
wantVals []interface{}
}{
{
name: "downstream",
Expand Down Expand Up @@ -185,6 +186,43 @@ func Test_addDependenciesFromProperty(t *testing.T) {
graphtest.ParseId(t, "mock:resource2:test2"),
},
},
{
name: "string value",
resource: &construct.Resource{
ID: graphtest.ParseId(t, "a:a:a"),
Properties: map[string]interface{}{
"Res4": "resource4",
},
},
propertyName: "Res4",
step: knowledgebase.OperationalStep{Direction: knowledgebase.DirectionDownstream},
want: enginetesting.ExpectedGraphs{},
wantVals: []interface{}{
"resource4",
},
},
{
name: "mixed array",
resource: &construct.Resource{
ID: graphtest.ParseId(t, "a:a:a"),
Properties: map[string]interface{}{
"Res2s": []any{MockResource2("test").ID, "resource2"},
},
},
propertyName: "Res2s",
initialState: []any{"mock:resource2:test", "a:a:a"},
step: knowledgebase.OperationalStep{Direction: knowledgebase.DirectionDownstream},
want: enginetesting.ExpectedGraphs{
Dataflow: []any{"mock:resource2:test", "a:a:a", "a:a:a -> mock:resource2:test"},
Deployment: []any{"mock:resource2:test", "a:a:a", "a:a:a -> mock:resource2:test"},
},
wantIds: []construct.ResourceId{
graphtest.ParseId(t, "mock:resource2:test"),
},
wantVals: []interface{}{
"resource2",
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand All @@ -196,12 +234,13 @@ func Test_addDependenciesFromProperty(t *testing.T) {
Solution: testSol,
}

ids, err := ctx.addDependenciesFromProperty(tt.step, tt.resource, tt.propertyName)
ids, otherVals, err := ctx.addDependenciesFromProperty(tt.step, tt.resource, tt.propertyName)
if !assert.NoError(err) {
return
}
tt.want.AssertEqual(t, testSol)
assert.ElementsMatch(tt.wantIds, ids)
assert.ElementsMatch(tt.wantVals, otherVals)
})
}
}
Expand Down
3 changes: 1 addition & 2 deletions pkg/templates/aws/resources/ecs_task_definition.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,7 @@ properties:
- aws:ecr_image:{{ .Self.Name }}-{{ if pathAncestorExists .Path 1}}{{ if hasField (printf "%s.Name" (pathAncestor .Path 1)) .Self}}{{ fieldValue (printf "%s.Name" (pathAncestor .Path 1)) .Self}}{{else}}image{{end}}{{else}}image{{end}}
unique: true
use_property_ref: ImageName
description: Reference to an Amazon Elastic Container Registry (ECR) image that
will be used for the container within the task
description: Reference to the imageName that is used to pull the image from the image repository
LogConfiguration:
type: map
properties:
Expand Down

0 comments on commit dfa9905

Please sign in to comment.