Skip to content

Commit

Permalink
pr comments
Browse files Browse the repository at this point in the history
  • Loading branch information
Jordan Singer committed Dec 4, 2023
1 parent 7196d26 commit 7858cd7
Show file tree
Hide file tree
Showing 17 changed files with 68 additions and 71 deletions.
5 changes: 2 additions & 3 deletions pkg/engine2/operational_eval/dependency_capture.go
Original file line number Diff line number Diff line change
Expand Up @@ -182,9 +182,8 @@ func (ctx *fauxConfigContext) ExecutePropertyRule(
if propRule.Value != nil {
ctx.ExecuteValue(propRule.Value, data)
}
if len(propRule.Step.Resources) > 0 {
errs = errors.Join(errs, ctx.executeOpStep(data, propRule.Step))
}
errs = errors.Join(errs, ctx.executeOpStep(data, propRule.Step))

return errs
}

Expand Down
3 changes: 0 additions & 3 deletions pkg/engine2/operational_eval/graph.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,9 +144,6 @@ func (eval *Evaluator) resourceVertices(
) (graphChanges, error) {
changes := newChanges()
var errs error
if res.ID.Type == "iam_role" {
res.ID.Type = "iam_role"
}
addProp := func(prop knowledgebase.Property) error {
vertex := &propertyVertex{
Ref: construct.PropertyRef{Resource: res.ID, Property: prop.Details().Path},
Expand Down
6 changes: 3 additions & 3 deletions pkg/engine2/operational_eval/vertex_property.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func (prop *propertyVertex) Dependencies(eval *Evaluator) (graphChanges, error)

// Template can be nil when checking for dependencies from a propertyVertex when adding an edge template
if prop.Template != nil {
_, _ = prop.Template.GetDefaultValue(propCtx.inner, resData)
_, _ = prop.Template.GetDefaultValue(propCtx, resData)
details := prop.Template.Details()
if opRule := details.OperationalRule; opRule != nil {
if err := propCtx.ExecutePropertyRule(resData, *opRule); err != nil {
Expand Down Expand Up @@ -286,12 +286,12 @@ func (v *propertyVertex) Ready(eval *Evaluator) (ReadyPriority, error) {
return ReadyNow, nil
}
ptype := v.Template.Type()
if strings.Contains(ptype, "list") || strings.Contains(ptype, "set") {
if strings.HasPrefix(ptype, "list") || strings.HasPrefix(ptype, "set") {
// never sure when a list/set is ready - it'll just be appended to by edges through
// `v.EdgeRules`
return NotReadyHigh, nil
}
if strings.Contains(ptype, "map") && len(v.Template.SubProperties()) == 0 {
if strings.HasPrefix(ptype, "map") && len(v.Template.SubProperties()) == 0 {
// maps without sub-properties (ie, not objects) are also appended to by edges
return NotReadyHigh, nil
}
Expand Down
3 changes: 1 addition & 2 deletions pkg/engine2/operational_rule/operational_configuration.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ func (ctx OperationalRuleContext) HandleConfigurationRule(config knowledgebase.C
if err != nil {
return fmt.Errorf("resource %s not found: %w", res, err)
}
action := "add"

resolvedField := config.Config.Field
err = dyn.ExecuteDecode(config.Config.Field, ctx.Data, &resolvedField)
Expand All @@ -26,7 +25,7 @@ func (ctx OperationalRuleContext) HandleConfigurationRule(config knowledgebase.C
}
config.Config.Field = resolvedField

err = solution_context.ConfigureResource(ctx.Solution, resource, config.Config, ctx.Data, action)
err = solution_context.ConfigureResource(ctx.Solution, resource, config.Config, ctx.Data, "add")
if err != nil {
return err
}
Expand Down
25 changes: 10 additions & 15 deletions pkg/engine2/operational_rule/operational_step.go
Original file line number Diff line number Diff line change
Expand Up @@ -270,25 +270,20 @@ func (ctx OperationalRuleContext) SetField(resource, fieldResource *construct.Re
return nil
}

removeCurrentValue := func(res any) error {
switch val := res.(type) {
case construct.ResourceId:
if val != fieldResource.ID {
return removeResource(val)
}
case construct.PropertyRef:
if val.Resource != fieldResource.ID {
return removeResource(val.Resource)
}
}
return nil
}

propVal, err := resource.GetProperty(path)
if err != nil {
zap.S().Debugf("property %s not found on resource %s", path, resource.ID)
}
err = removeCurrentValue(propVal)
switch val := propVal.(type) {
case construct.ResourceId:
if val != fieldResource.ID {
err = removeResource(val)
}
case construct.PropertyRef:
if val.Resource != fieldResource.ID {
err = removeResource(val.Resource)
}
}
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/knowledge_base2/properties/any_property.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ func (a *AnyProperty) Clone() knowledgebase.Property {
return &clone
}

func (a *AnyProperty) GetDefaultValue(ctx knowledgebase.DynamicValueContext, data knowledgebase.DynamicValueData) (any, error) {
func (a *AnyProperty) GetDefaultValue(ctx knowledgebase.DynamicContext, data knowledgebase.DynamicValueData) (any, error) {
if a.DefaultValue == nil {
return nil, nil
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/knowledge_base2/properties/bool_property.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func (b *BoolProperty) Details() *knowledgebase.PropertyDetails {
return &b.PropertyDetails
}

func (b *BoolProperty) GetDefaultValue(ctx knowledgebase.DynamicValueContext, data knowledgebase.DynamicValueData) (any, error) {
func (b *BoolProperty) GetDefaultValue(ctx knowledgebase.DynamicContext, data knowledgebase.DynamicValueData) (any, error) {
if b.DefaultValue == nil {
return nil, nil
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/knowledge_base2/properties/float_property.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func (f *FloatProperty) Clone() knowledgebase.Property {
return &clone
}

func (f *FloatProperty) GetDefaultValue(ctx knowledgebase.DynamicValueContext, data knowledgebase.DynamicValueData) (any, error) {
func (f *FloatProperty) GetDefaultValue(ctx knowledgebase.DynamicContext, data knowledgebase.DynamicValueData) (any, error) {
if f.DefaultValue == nil {
return nil, nil
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/knowledge_base2/properties/int_property.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ func (i *IntProperty) Clone() knowledgebase.Property {
return &clone
}

func (i *IntProperty) GetDefaultValue(ctx knowledgebase.DynamicValueContext, data knowledgebase.DynamicValueData) (any, error) {
func (i *IntProperty) GetDefaultValue(ctx knowledgebase.DynamicContext, data knowledgebase.DynamicValueData) (any, error) {
if i.DefaultValue == nil {
return nil, nil
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/knowledge_base2/properties/list_property.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ func (l *ListProperty) Clone() knowledgebase.Property {
return &clone
}

func (list *ListProperty) GetDefaultValue(ctx knowledgebase.DynamicValueContext, data knowledgebase.DynamicValueData) (any, error) {
func (list *ListProperty) GetDefaultValue(ctx knowledgebase.DynamicContext, data knowledgebase.DynamicValueData) (any, error) {
if list.DefaultValue == nil {
return nil, nil
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/knowledge_base2/properties/map_property.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ func (m *MapProperty) Clone() knowledgebase.Property {
return &clone
}

func (m *MapProperty) GetDefaultValue(ctx knowledgebase.DynamicValueContext, data knowledgebase.DynamicValueData) (any, error) {
func (m *MapProperty) GetDefaultValue(ctx knowledgebase.DynamicContext, data knowledgebase.DynamicValueData) (any, error) {
if m.DefaultValue == nil {
return nil, nil
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/knowledge_base2/properties/resource_property.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func (r *ResourceProperty) Clone() knowledgebase.Property {
return &clone
}

func (r *ResourceProperty) GetDefaultValue(ctx knowledgebase.DynamicValueContext, data knowledgebase.DynamicValueData) (any, error) {
func (r *ResourceProperty) GetDefaultValue(ctx knowledgebase.DynamicContext, data knowledgebase.DynamicValueData) (any, error) {
if r.DefaultValue == nil {
return nil, nil
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/knowledge_base2/properties/set_property.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ func (s *SetProperty) Clone() knowledgebase.Property {
return &clone
}

func (s *SetProperty) GetDefaultValue(ctx knowledgebase.DynamicValueContext, data knowledgebase.DynamicValueData) (any, error) {
func (s *SetProperty) GetDefaultValue(ctx knowledgebase.DynamicContext, data knowledgebase.DynamicValueData) (any, error) {
if s.DefaultValue == nil {
return nil, nil
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/knowledge_base2/properties/string_property.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ func (s *StringProperty) Clone() knowledgebase.Property {
return &clone
}

func (s *StringProperty) GetDefaultValue(ctx knowledgebase.DynamicValueContext, data knowledgebase.DynamicValueData) (any, error) {
func (s *StringProperty) GetDefaultValue(ctx knowledgebase.DynamicContext, data knowledgebase.DynamicValueData) (any, error) {
if s.DefaultValue == nil {
return nil, nil
}
Expand Down
3 changes: 3 additions & 0 deletions pkg/knowledge_base2/reader/embed.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ func NewKBFromFs(resources, edges, models fs.FS) (*knowledgebase.KnowledgeBase,
}
kbModels[name] = kbModel
}
if errs != nil {
return nil, errs
}
kb.Models = kbModels
templates, err := TemplatesFromFs(resources, readerModels)
if err != nil {
Expand Down
74 changes: 39 additions & 35 deletions pkg/knowledge_base2/reader/properties.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,11 @@ import (
)

type (
// Properties defines the structure of properties defined in yaml as a part of a template.
Properties map[string]*Property

// Property defines the structure of a property defined in yaml as a part of a template.
// these fields must be exactly the union of all the fields in the different property types.
Property struct {
Name string `json:"name" yaml:"name"`
// Type defines the type of the property
Expand Down Expand Up @@ -97,45 +100,46 @@ func (p *Property) Convert() (knowledgebase.Property, error) {
srcField := srcVal.Field(i)
fieldName := srcVal.Type().Field(i).Name
dstField := dstVal.FieldByName(fieldName)
if dstField.IsValid() && dstField.CanSet() {
// Skip nil pointers
if (srcField.Kind() == reflect.Ptr || srcField.Kind() == reflect.Interface) && srcField.IsNil() {
continue
if !dstField.IsValid() || !dstField.CanSet() {
continue
}
// Skip nil pointers
if (srcField.Kind() == reflect.Ptr || srcField.Kind() == reflect.Interface) && srcField.IsNil() {
continue
}
// Handle sub properties so we can recurse down the tree
if fieldName == "Properties" {
properties, ok := srcField.Interface().(Properties)
if !ok {
return nil, fmt.Errorf("invalid properties")
}
// Handle sub properties so we can recurse down the tree
if fieldName == "Properties" {
properties, ok := srcField.Interface().(Properties)
if !ok {
return nil, fmt.Errorf("invalid properties")
}
var errs error
props := knowledgebase.Properties{}
for name, prop := range properties {
propertyType, err := prop.Convert()
if err != nil {
errs = fmt.Errorf("%w\n%s", errs, err.Error())
continue
}
props[name] = propertyType
}
if errs != nil {
return nil, errs
var errs error
props := knowledgebase.Properties{}
for name, prop := range properties {
propertyType, err := prop.Convert()
if err != nil {
errs = fmt.Errorf("%w\n%s", errs, err.Error())
continue
}
dstField.Set(reflect.ValueOf(props))
continue
props[name] = propertyType
}
if errs != nil {
return nil, errs
}
dstField.Set(reflect.ValueOf(props))
continue
}

if dstField.Type() == srcField.Type() {
dstField.Set(srcField)
} else {
if conversion, found := fieldConversion[fieldName]; found {
err := conversion(srcField, p, propertyType)
if err != nil {
return nil, err
}
} else {
return nil, fmt.Errorf("invalid property type %s", fieldName)
if dstField.Type() == srcField.Type() {
dstField.Set(srcField)
} else {
if conversion, found := fieldConversion[fieldName]; found {
err := conversion(srcField, p, propertyType)
if err != nil {
return nil, err
}
} else {
return nil, fmt.Errorf("invalid property type %s", fieldName)
}
}
}
Expand All @@ -151,7 +155,6 @@ func setChildPaths(property *Property, currPath string) {
path := currPath + "." + name
child.Path = path
setChildPaths(child, path)
property.Properties[name] = child
}
}

Expand All @@ -172,6 +175,7 @@ func (p *Property) Clone() *Property {
return &cloned
}

// fieldConversion is a map providing functionality on how to convert inputs into our internal types if they are not inherently the same structure
var fieldConversion = map[string]func(val reflect.Value, p *Property, kp knowledgebase.Property) error{
"SanitizeTmpl": func(val reflect.Value, p *Property, kp knowledgebase.Property) error {
sanitizeTmpl, ok := val.Interface().(string)
Expand Down
2 changes: 1 addition & 1 deletion pkg/knowledge_base2/resource_template.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ type (
// Type returns the string representation of the type of the property, as it should appear in the resource template
Type() string
// GetDefaultValue returns the default value for the property, pertaining to the specific data being passed in for execution
GetDefaultValue(ctx DynamicValueContext, data DynamicValueData) (any, error)
GetDefaultValue(ctx DynamicContext, data DynamicValueData) (any, error)
// Validate ensures the value is valid for the property and returns an error if it is not
Validate(value any, properties construct.Properties) error
// SubProperties returns the sub properties of the property, if any. This is used for properties that are complex structures,
Expand Down

0 comments on commit 7858cd7

Please sign in to comment.