Fixes from code review.

internal/pkg/agent/transpiler/ast.go

@@ -58,7 +58,8 @@ type Node interface {
 	// Hash compute a sha256 hash of the current node and recursively call any children.
 	Hash() []byte
 
-	// Vars return a list of referenced vars.
+	// Vars adds to the array with the variables identified in the node. Returns the array in-case
+	// the capacity of the array had to be changed.
 	Vars([]string) []string
 
 	// Apply apply the current vars, returning the new value for the node.
@@ -502,35 +503,11 @@ func (s *StrVal) Hash() []byte {
 
 // Vars returns a list of all variables referenced in the string.
 func (s *StrVal) Vars(vars []string) []string {
-	value := s.value
-	matchIdxs := varsRegex.FindAllSubmatchIndex([]byte(value), -1)
-	if !validBrackets(value, matchIdxs) {
-		// brackets are not valid; unable to pull vars (computing the policy will fail)
-		return vars
-	}
-	for _, r := range matchIdxs {
-		for i := 0; i < len(r); i += 4 {
-			if value[r[i]+1] == '$' {
-				// match on an escaped var, this is not a real variable
-				continue
-			}
-			// match on a non-escaped var
-			extractedVars, err := extractVars(value[r[i+2]:r[i+3]])
-			if err != nil {
-				// variable parsing failed (computing the policy will fail)
-				return vars
-			}
-			for _, val := range extractedVars {
-				switch val.(type) {
-				case *constString:
-					// not a variable
-				case *varString:
-					// found variable add it to the array
-					vars = append(vars, val.Value())
-				}
-			}
-		}
-	}
+	// errors are ignored (if there is an error determine the vars it will also error computing the policy)
+	_, _ = replaceVars(s.value, func(variable string) (Node, Processors, bool) {
+		vars = append(vars, variable)
+		return nil, nil, false
+	}, false)
 	return vars
 }
 

internal/pkg/agent/transpiler/ast_test.go

@@ -1019,6 +1019,58 @@ func TestVars(t *testing.T) {
 	}
 }
 
+func TestLookup(t *testing.T) {
+	tests := map[string]struct {
+		ast      *AST
+		selector Selector
+		node     Node
+		ok       bool
+	}{
+		"nil": {
+			ast:      nil,
+			selector: "",
+			node:     nil,
+			ok:       false,
+		},
+		"noroot": {
+			ast:      &AST{},
+			selector: "",
+			node:     nil,
+			ok:       false,
+		},
+		"notfound": {
+			ast: &AST{
+				root: NewDict([]Node{NewKey("entry", NewDict([]Node{
+					NewKey("var1", NewStrVal("value1")),
+					NewKey("var2", NewStrVal("value2")),
+				}))}),
+			},
+			selector: "entry.var3",
+			node:     nil,
+			ok:       false,
+		},
+		"found": {
+			ast: &AST{
+				root: NewDict([]Node{NewKey("entry", NewDict([]Node{
+					NewKey("var1", NewStrVal("value1")),
+					NewKey("var2", NewStrVal("value2")),
+				}))}),
+			},
+			selector: "entry.var2",
+			node:     NewKey("var2", NewStrVal("value2")),
+			ok:       true,
+		},
+	}
+	for name, test := range tests {
+		t.Run(name, func(t *testing.T) {
+			node, ok := Lookup(test.ast, test.selector)
+			if assert.Equal(t, test.ok, ok) {
+				assert.Equal(t, test.node, node)
+			}
+		})
+	}
+}
+
 func mustMakeVars(mapping map[string]interface{}) *Vars {
 	v, err := NewVars("", mapping, nil)
 	if err != nil {

internal/pkg/agent/transpiler/vars.go

@@ -54,6 +54,56 @@ func NewVarsWithProcessorsFromAst(id string, tree *AST, processorKey string, pro
 
 // Replace returns a new value based on variable replacement.
 func (v *Vars) Replace(value string) (Node, error) {
+	return replaceVars(value, func(variable string) (Node, Processors, bool) {
+		var processors Processors
+		node, ok := v.lookupNode(variable)
+		if ok && v.processorsKey != "" && varPrefixMatched(variable, v.processorsKey) {
+			processors = v.processors
+		}
+		return node, processors, ok
+	}, true)
+}
+
+// ID returns the unique ID for the vars.
+func (v *Vars) ID() string {
+	return v.id
+}
+
+// Lookup returns the value from the vars.
+func (v *Vars) Lookup(name string) (interface{}, bool) {
+	// lookup in the AST tree
+	return v.tree.Lookup(name)
+}
+
+// Map transforms the variables into a map[string]interface{} and will abort and return any errors related
+// to type conversion.
+func (v *Vars) Map() (map[string]interface{}, error) {
+	return v.tree.Map()
+}
+
+// lookupNode performs a lookup on the AST, but keeps the result as a `Node`.
+//
+// This is different from `Lookup` which returns the actual type, not the AST type.
+func (v *Vars) lookupNode(name string) (Node, bool) {
+	// check if the value can be retrieved from a FetchContextProvider
+	for providerName, provider := range v.fetchContextProviders {
+		if varPrefixMatched(name, providerName) {
+			fetchProvider, ok := provider.(composable.FetchContextProvider)
+			if !ok {
+				return &StrVal{value: ""}, false
+			}
+			fval, found := fetchProvider.Fetch(name)
+			if found {
+				return &StrVal{value: fval}, true
+			}
+			return &StrVal{value: ""}, false
+		}
+	}
+	// lookup in the AST tree
+	return Lookup(v.tree, name)
+}
+
+func replaceVars(value string, replacer func(variable string) (Node, Processors, bool), reqMatch bool) (Node, error) {
 	var processors Processors
 	matchIdxs := varsRegex.FindAllSubmatchIndex([]byte(value), -1)
 	if !validBrackets(value, matchIdxs) {
@@ -81,11 +131,11 @@ func (v *Vars) Replace(value string) (Node, error) {
 					result += value[lastIndex:r[0]] + val.Value()
 					set = true
 				case *varString:
-					node, ok := v.lookupNode(val.Value())
+					node, nodeProcessors, ok := replacer(val.Value())
 					if ok {
 						node := nodeToValue(node)
-						if v.processorsKey != "" && varPrefixMatched(val.Value(), v.processorsKey) {
-							processors = v.processors
+						if nodeProcessors != nil {
+							processors = nodeProcessors
 						}
 						if r[i] == 0 && r[i+1] == len(value) {
 							// possible for complete replacement of object, because the variable
@@ -100,7 +150,7 @@ func (v *Vars) Replace(value string) (Node, error) {
 					break
 				}
 			}
-			if !set {
+			if !set && reqMatch {
 				return NewStrVal(""), ErrNoMatch
 			}
 			lastIndex = r[1]
@@ -109,45 +159,6 @@ func (v *Vars) Replace(value string) (Node, error) {
 	return NewStrValWithProcessors(result+value[lastIndex:], processors), nil
 }
 
-// ID returns the unique ID for the vars.
-func (v *Vars) ID() string {
-	return v.id
-}
-
-// Lookup returns the value from the vars.
-func (v *Vars) Lookup(name string) (interface{}, bool) {
-	// lookup in the AST tree
-	return v.tree.Lookup(name)
-}
-
-// Map transforms the variables into a map[string]interface{} and will abort and return any errors related
-// to type conversion.
-func (v *Vars) Map() (map[string]interface{}, error) {
-	return v.tree.Map()
-}
-
-// lookupNode performs a lookup on the AST, but keeps the result as a `Node`.
-//
-// This is different from `Lookup` which returns the actual type, not the AST type.
-func (v *Vars) lookupNode(name string) (Node, bool) {
-	// check if the value can be retrieved from a FetchContextProvider
-	for providerName, provider := range v.fetchContextProviders {
-		if varPrefixMatched(name, providerName) {
-			fetchProvider, ok := provider.(composable.FetchContextProvider)
-			if !ok {
-				return &StrVal{value: ""}, false
-			}
-			fval, found := fetchProvider.Fetch(name)
-			if found {
-				return &StrVal{value: fval}, true
-			}
-			return &StrVal{value: ""}, false
-		}
-	}
-	// lookup in the AST tree
-	return Lookup(v.tree, name)
-}
-
 // nodeToValue ensures that the node is an actual value.
 func nodeToValue(node Node) Node {
 	switch n := node.(type) {