refactor: make vars a map

Author: FabianKramm

cmd/init.go

@@ -277,10 +277,13 @@ func (cmd *InitCmd) Run(f factory.Factory) error {
 
 		if config.Images != nil && config.Images[imageName] != nil {
 			// Move full image name to variables
-			config.Vars = append(config.Vars, &latest.Variable{
+			if config.Vars == nil {
+				config.Vars = map[string]*latest.Variable{}
+			}
+			config.Vars[imageVarName] = &latest.Variable{
 				Name:  imageVarName,
 				Value: config.Images[imageName].Image,
-			})
+			}
 
 			// Use variable in images section
 			config.Images[imageName].Image = imageVar

cmd/set/var.go

@@ -82,14 +82,19 @@ func (cmd *varCmd) RunSetVar(f factory.Factory, cobraCmd *cobra.Command, args []
 			return errors.Errorf("Unexpected variable format. Expected key=value, got %s", v)
 		} else if variable.IsPredefinedVariable(splitted[0]) {
 			return errors.Errorf("cannot set predefined variable %s", splitted[0])
-		} else if !variableParser.Used[splitted[0]] {
-			allowedVarsArr := []string{}
-			for k := range variableParser.Used {
-				if variable.IsPredefinedVariable(k) {
-					continue
-				}
+		}
 
-				allowedVarsArr = append(allowedVarsArr, k)
+		found := false
+		for _, u := range variableParser.Used {
+			if u.Name == splitted[0] {
+				found = true
+				break
+			}
+		}
+		if !found {
+			allowedVarsArr := []string{}
+			for _, v := range variableParser.Used {
+				allowedVarsArr = append(allowedVarsArr, v.Name)
 			}
 
 			return errors.Errorf("variable %s is not allowed. Allowed vars: %+v", splitted[0], allowedVarsArr)
@@ -105,7 +110,7 @@ func (cmd *varCmd) RunSetVar(f factory.Factory, cobraCmd *cobra.Command, args []
 		}
 
 		// only overwrite it if the flag is true and value is not set yet
-		_, found := c.LocalCache().GetVar(splitted[0])
+		_, found = c.LocalCache().GetVar(splitted[0])
 		if cmd.Overwrite || !found {
 			c.LocalCache().SetVar(splitted[0], splitted[1])
 		} else {
@@ -124,8 +129,8 @@ func (cmd *varCmd) RunSetVar(f factory.Factory, cobraCmd *cobra.Command, args []
 }
 
 type variableParser struct {
-	Definitions []*latest.Variable
-	Used        map[string]bool
+	Definitions map[string]*latest.Variable
+	Used        []*latest.Variable
 }
 
 func (v *variableParser) Parse(ctx context.Context, originalRawConfig map[string]interface{}, rawConfig map[string]interface{}, resolver variable.Resolver, log log.Logger) (*latest.Config, map[string]interface{}, error) {

e2e/tests/config/config.go

@@ -1864,7 +1864,7 @@ var _ = DevSpaceDescribe("config", func() {
 		framework.ExpectEqual(deployment1.Name, "development1")
 		framework.ExpectEqual(deployment1.Helm.CleanupOnFail, false)
 		framework.ExpectEqual(deployment1.Helm.Timeout, "1000s")
-		gomega.Expect(*deployment1.Helm.ComponentChart).To(gomega.BeTrue())
+		gomega.Expect(deployment1.Helm).ToNot(gomega.BeNil())
 	})
 
 	ginkgo.It("should apply patch to some deployments using wildcard profile patches", func() {
@@ -1882,7 +1882,7 @@ var _ = DevSpaceDescribe("config", func() {
 		deployment1 := config.Config().Deployments["test"]
 		framework.ExpectEqual(deployment1.Name, "test")
 		gomega.Expect(deployment1.Kubectl).To(gomega.BeNil())
-		gomega.Expect(*deployment1.Helm.ComponentChart).To(gomega.BeTrue())
+		gomega.Expect(deployment1.Helm).ToNot(gomega.BeNil())
 
 		deployment2 := config.Config().Deployments["test2"]
 		framework.ExpectEqual(deployment2.Name, "test2")
@@ -1906,7 +1906,7 @@ var _ = DevSpaceDescribe("config", func() {
 		deployment1 := config.Config().Deployments["backend"]
 		framework.ExpectEqual(deployment1.Name, "backend")
 		gomega.Expect(deployment1.Kubectl).To(gomega.BeNil())
-		gomega.Expect(*deployment1.Helm.ComponentChart).To(gomega.BeTrue())
+		gomega.Expect(deployment1.Helm).ToNot(gomega.BeNil())
 	})
 
 	ginkgo.It("should apply patch even value is empty", func() {
@@ -1924,7 +1924,7 @@ var _ = DevSpaceDescribe("config", func() {
 		deployment1 := config.Config().Deployments["test-sigsegv"]
 		framework.ExpectEqual(deployment1.Name, "test-sigsegv")
 		gomega.Expect(deployment1.Kubectl).To(gomega.BeNil())
-		gomega.Expect(*deployment1.Helm.ComponentChart).To(gomega.BeTrue())
+		gomega.Expect(deployment1.Helm).ToNot(gomega.BeNil())
 	})
 
 	// regression test for issue: https://github.com/loft-sh/devspace/issues/1835
@@ -1953,6 +1953,6 @@ var _ = DevSpaceDescribe("config", func() {
 		framework.ExpectEqual(v["name"], "replace-0")
 
 		gomega.Expect(deployment.Kubectl).To(gomega.BeNil())
-		gomega.Expect(*deployment.Helm.ComponentChart).To(gomega.BeTrue())
+		gomega.Expect(deployment.Helm).ToNot(gomega.BeNil())
 	})
 })

examples/pipelines/devspace.yaml

@@ -1,17 +1,13 @@
 version: v2beta1
 name: pipelines
 
+vars:
+  MELLO: $(echo "TEST")
+  DELLO: ${MELLO}-${CHELLO}
+  HELLO: ${devspace.namespace}-${DELLO}
+
 pipelines:
   dev:
     steps:
       - run: |-
-          create_deployments test --set helm.componentChart=true \
-                                  --set helm.values.containers[0].image=nginx
-
-          start_dev test --set imageSelector=nginx \
-                         --set logs.disabled=false \
-                         --set command[0]=sh \
-                         --set command[1]=-c \
-                         --set 'command[2]=while true; do sleep 2 && echo Hello; done' \
-                         --set sync[0].containerPath=/app \
-                         --set sync[0].onUpload.restartContainer=true
+          echo ${HELLO}

pkg/devspace/config/loader/imports.go

@@ -14,6 +14,7 @@ import (
 )
 
 var ImportSections = []string{
+	"vars",
 	"dev",
 	"deployments",
 	"images",

pkg/devspace/config/loader/loader.go

@@ -572,7 +572,7 @@ func (l *configLoader) applyProfiles(ctx context.Context, data map[string]interf
 	return data, nil
 }
 
-func (l *configLoader) newVariableResolver(localCache localcache.Cache, remoteCache remotecache.Cache, client kubectl.Client, options *ConfigOptions, vars []*latest.Variable, log log.Logger) variable.Resolver {
+func (l *configLoader) newVariableResolver(localCache localcache.Cache, remoteCache remotecache.Cache, client kubectl.Client, options *ConfigOptions, vars map[string]*latest.Variable, log log.Logger) variable.Resolver {
 	return variable.NewResolver(localCache, remoteCache, &variable.PredefinedVariableOptions{
 		ConfigPath: l.absConfigPath,
 		KubeClient: client,

pkg/devspace/config/loader/validate.go

@@ -81,20 +81,13 @@ func Validate(config *latest.Config) error {
 	return nil
 }
 
-func validateVars(vars []*latest.Variable) error {
+func validateVars(vars map[string]*latest.Variable) error {
 	for i, v := range vars {
-		if v.Name == "" {
+		if i == "" {
 			return fmt.Errorf("vars[*].name has to be specified")
 		}
 		if encoding.IsUnsafeUpperName(v.Name) {
-			return fmt.Errorf("vars[%d].name %s has to match the following regex: %v", i, v.Name, encoding.UnsafeUpperNameRegEx.String())
-		}
-
-		// make sure is unique
-		for j, v2 := range vars {
-			if i != j && v.Name == v2.Name {
-				return fmt.Errorf("multiple definitions for variable %s found", v.Name)
-			}
+			return fmt.Errorf("vars[%s].name %s has to match the following regex: %v", i, v.Name, encoding.UnsafeUpperNameRegEx.String())
 		}
 	}
 

pkg/devspace/config/loader/variable/resolver.go

@@ -7,6 +7,7 @@ import (
 	"github.com/loft-sh/devspace/pkg/devspace/config/loader/variable/runtime"
 	"github.com/loft-sh/devspace/pkg/devspace/config/localcache"
 	"github.com/loft-sh/devspace/pkg/devspace/config/remotecache"
+	"github.com/loft-sh/devspace/pkg/devspace/dependency/graph"
 	"path/filepath"
 	"regexp"
 	"strings"
@@ -21,7 +22,7 @@ import (
 var AlwaysResolvePredefinedVars = []string{"devspace.version", "devspace.random", "devspace.profile", "devspace.userHome", "devspace.timestamp", "devspace.context", "devspace.namespace"}
 
 // NewResolver creates a new resolver that caches resolved variables in memory and in the provided cache
-func NewResolver(localCache localcache.Cache, remoteCache remotecache.Cache, predefinedVariableOptions *PredefinedVariableOptions, vars []*latest.Variable, log log.Logger) Resolver {
+func NewResolver(localCache localcache.Cache, remoteCache remotecache.Cache, predefinedVariableOptions *PredefinedVariableOptions, vars map[string]*latest.Variable, log log.Logger) Resolver {
 	return &resolver{
 		memoryCache: map[string]interface{}{},
 		localCache:  localCache,
@@ -33,7 +34,7 @@ func NewResolver(localCache localcache.Cache, remoteCache remotecache.Cache, pre
 }
 
 type resolver struct {
-	vars        []*latest.Variable
+	vars        map[string]*latest.Variable
 	memoryCache map[string]interface{}
 	localCache  localcache.Cache
 	remoteCache remotecache.Cache
@@ -45,11 +46,11 @@ func varMatchFn(key, value string) bool {
 	return varspkg.VarMatchRegex.MatchString(value)
 }
 
-func (r *resolver) DefinedVars() []*latest.Variable {
+func (r *resolver) DefinedVars() map[string]*latest.Variable {
 	return r.vars
 }
 
-func (r *resolver) UpdateVars(vars []*latest.Variable) {
+func (r *resolver) UpdateVars(vars map[string]*latest.Variable) {
 	r.vars = vars
 }
 
@@ -96,10 +97,9 @@ func (r *resolver) replaceString(ctx context.Context, str string) (interface{},
 	})
 }
 
-func (r *resolver) FindVariables(haystack interface{}) (map[string]bool, error) {
+func (r *resolver) FindVariables(haystack interface{}) ([]*latest.Variable, error) {
 	// find out what vars are really used
 	varsUsed := map[string]bool{}
-
 	switch t := haystack.(type) {
 	case string:
 		_, _ = varspkg.ParseString(t, func(v string) (interface{}, error) {
@@ -120,22 +120,90 @@ func (r *resolver) FindVariables(haystack interface{}) (map[string]bool, error)
 		}
 	}
 
-	// find out what vars are used within other vars definition
+	// add always resolve variables
 	for _, v := range r.vars {
-		varsUsedInDefinition := r.findVariablesInDefinition(v)
-		for usedVar := range varsUsedInDefinition {
-			varsUsed[usedVar] = true
+		if v.AlwaysResolve {
+			varsUsed[v.Name] = true
 		}
 	}
 
 	// filter out runtime environment variables
 	for k := range varsUsed {
-		if strings.HasPrefix(k, "runtime.") {
+		if r.vars[k] == nil || IsPredefinedVariable(k) || strings.HasPrefix(k, "runtime.") {
 			delete(varsUsed, k)
 		}
 	}
 
-	return varsUsed, nil
+	return r.orderVariables(varsUsed)
+}
+
+func (r *resolver) orderVariables(vars map[string]bool) ([]*latest.Variable, error) {
+	root := graph.NewNode("root", nil)
+	g := graph.NewGraphOf(root, "variable")
+	for name := range vars {
+		// check if has definition
+		definition, ok := r.vars[name]
+		if !ok {
+			continue
+		}
+
+		err := r.insertVariableGraph(g, definition)
+		if err != nil {
+			return nil, err
+		}
+	}
+
+	// now get all the leaf nodes
+	retVars := []*latest.Variable{}
+	for {
+		nextLeaf := g.GetNextLeaf(root)
+		if nextLeaf == root {
+			break
+		}
+
+		retVars = append(retVars, nextLeaf.Data.(*latest.Variable))
+		err := g.RemoveNode(nextLeaf.ID)
+		if err != nil {
+			return nil, err
+		}
+	}
+
+	// reverse the slice
+	for i, j := 0, len(retVars)-1; i < j; i, j = i+1, j-1 {
+		retVars[i], retVars[j] = retVars[j], retVars[i]
+	}
+	return retVars, nil
+}
+
+func (r *resolver) insertVariableGraph(g *graph.Graph, node *latest.Variable) error {
+	if _, ok := g.Nodes[node.Name]; !ok {
+		_, err := g.InsertNodeAt("root", node.Name, node)
+		if err != nil {
+			return err
+		}
+	}
+
+	parents := r.findVariablesInDefinition(node)
+	for parent := range parents {
+		parentDefinition, ok := r.vars[parent]
+		if !ok {
+			continue
+		}
+
+		if _, ok := g.Nodes[parentDefinition.Name]; !ok {
+			err := r.insertVariableGraph(g, parentDefinition)
+			if err != nil {
+				return err
+			}
+		}
+
+		err := g.AddEdge(parent, node.Name)
+		if err != nil {
+			return err
+		}
+	}
+
+	return nil
 }
 
 func (r *resolver) FillVariablesExclude(ctx context.Context, haystack interface{}, excludedPaths []string) (interface{}, error) {
@@ -178,19 +246,6 @@ func (r *resolver) findAndFillVariables(ctx context.Context, haystack interface{
 		return nil, err
 	}
 
-	// resolve used defined variables
-	for _, v := range r.vars {
-		if v.AlwaysResolve || varsUsed[strings.TrimSpace(v.Name)] {
-			name := strings.TrimSpace(v.Name)
-
-			// resolve the variable with definition
-			_, err := r.resolve(ctx, name, v)
-			if err != nil {
-				return nil, err
-			}
-		}
-	}
-
 	// try resolving predefined variables
 	for _, name := range AlwaysResolvePredefinedVars {
 		// ignore errors here as those variables are probably not used anyways
@@ -200,6 +255,14 @@ func (r *resolver) findAndFillVariables(ctx context.Context, haystack interface{
 		}
 	}
 
+	// resolve used defined variables
+	for _, v := range varsUsed {
+		_, err := r.resolve(ctx, v.Name, v)
+		if err != nil {
+			return nil, err
+		}
+	}
+
 	return r.fillVariables(ctx, haystack, exclude)
 }
 
@@ -316,6 +379,13 @@ func (r *resolver) findVariablesInDefinition(definition *latest.Variable) map[st
 		}
 	}
 
+	// filter out runtime environment variables and non existing ones
+	for k := range varsUsed {
+		if r.vars[k] == nil || IsPredefinedVariable(k) || strings.HasPrefix(k, "runtime.") {
+			delete(varsUsed, k)
+		}
+	}
+
 	return varsUsed
 }
 
@@ -393,6 +463,10 @@ func (r *resolver) resolveDefinitionString(ctx context.Context, str string, defi
 			// check if its a predefined variable
 			variable, err := NewPredefinedVariable(varName, r.options)
 			if err != nil {
+				if r.vars[varName] == nil {
+					return "${" + varName + "}", nil
+				}
+
 				return nil, errors.Errorf("variable '%s' was not resolved yet, however is used in the definition of variable '%s' as '%s'. Please make sure you define '%s' before '%s' in the vars array", varName, definition.Name, str, varName, definition.Name)
 			}
 

pkg/devspace/config/loader/variable/types.go

@@ -27,13 +27,13 @@ type Resolver interface {
 	ConvertFlags(flags []string) (map[string]interface{}, error)
 
 	// DefinedVars returns the defined variables
-	DefinedVars() []*latest.Variable
+	DefinedVars() map[string]*latest.Variable
 
 	// UpdateVars sets the defined variables to use in the resolver
-	UpdateVars(vars []*latest.Variable)
+	UpdateVars(vars map[string]*latest.Variable)
 
 	// FindVariables returns all variable names that were found in the given map
-	FindVariables(haystack interface{}) (map[string]bool, error)
+	FindVariables(haystack interface{}) ([]*latest.Variable, error)
 
 	// FillVariables finds the used variables first and then fills in those in the haystack
 	FillVariables(ctx context.Context, haystack interface{}) (interface{}, error)