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

make ecs images be able to be strings #980

Merged
merged 2 commits into from
Apr 19, 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
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
2 changes: 1 addition & 1 deletion pkg/engine/testdata/ecs_rds.expect.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ resources:
- Name: RDS_INSTANCE_2_RDS_USERNAME
Value: aws:rds_instance:rds-instance-2#Username
Essential: true
Image: aws:ecr_image:ecs_service_0-ecs_service_0
Image: aws:ecr_image:ecs_service_0-ecs_service_0#ImageName
LogConfiguration:
LogDriver: awslogs
Options:
Expand Down
2 changes: 1 addition & 1 deletion pkg/engine/testdata/idempotent_constraints.expect.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ resources:
- Name: RDS_INSTANCE_2_RDS_USERNAME
Value: aws:rds_instance:rds-instance-2#Username
Essential: true
Image: aws:ecr_image:ecs_service_0-ecs_service_0
Image: aws:ecr_image:ecs_service_0-ecs_service_0#ImageName
LogConfiguration:
LogDriver: awslogs
Options:
Expand Down
2 changes: 1 addition & 1 deletion pkg/engine/testdata/idempotent_constraints.input.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ resources:
- Name: RDS_INSTANCE_2_RDS_USERNAME
Value: aws:rds_instance:rds-instance-2#Username
Essential: true
Image: aws:ecr_image:ecs_service_0-ecs_service_0
Image: aws:ecr_image:ecs_service_0-ecs_service_0#ImageName
LogConfiguration:
LogDriver: awslogs
Options:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@
},
{{- end }}
{{- if $cd.Image }}
image: {{ getVar $cd.Image }}.imageName,
image: {{ modelCase $cd.Image }},
{{- end }}
{{- if $cd.LogConfiguration }}
logConfiguration: {
Expand Down
6 changes: 3 additions & 3 deletions pkg/templates/aws/resources/ecs_task_definition.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -95,16 +95,16 @@ properties:
min_value: 2
max_value: 60
Image:
type: resource(aws:ecr_image)
type: string
required: true
operational_rule:
step:
direction: downstream
resources:
- 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
description: Reference to an Amazon Elastic Container Registry (ECR) image that
will be used for the container within the task
use_property_ref: ImageName
description: Reference to the imageName that is used to pull the image from the image repository
LogConfiguration:
type: map
properties:
Expand Down
Loading