chore: cleaned up some unecessary complexity

Signed-off-by: Ben Meier <[email protected]>

internal/compose/convert.go

@@ -19,15 +19,27 @@ import (
 
 // ConvertSpec converts SCORE specification into docker-compose configuration.
 func ConvertSpec(spec *score.Workload) (*compose.Project, *EnvVarTracker, error) {
-	// Track any uses of the environment resource or resources that are overridden with an env provider using the tracker.
-	envVarTracker := NewEnvVarTracker()
+	resources := make(map[string]ResourceWithOutputs)
+	ctx, err := buildContext(spec.Metadata, resources)
+	if err != nil {
+		return nil, nil, fmt.Errorf("preparing context: %w", err)
+	}
+
+	workloadName, ok := spec.Metadata["name"].(string)
+	if !ok || len(workloadName) == 0 {
+		return nil, nil, errors.New("workload metadata is missing a name")
+	}
+
+	if len(spec.Containers) == 0 {
+		return nil, nil, errors.New("workload does not have any containers to convert into a compose service")
+	}
 
 	var project = compose.Project{
 		Services: make(compose.Services, 0, len(spec.Containers)),
 	}
 
-	// this map holds the results of the provisioning process
-	resources := make(map[string]ResourceWithOutputs)
+	// Track any uses of the environment resource or resources that are overridden with an env provider using the tracker.
+	envVarTracker := new(EnvVarTracker)
 
 	// The first thing we must do is validate or create the resources this workload depends on.
 	// NOTE: this will soon be replaced by a much more sophisticated resource provisioning system!
@@ -43,14 +55,6 @@ func ConvertSpec(spec *score.Workload) (*compose.Project, *EnvVarTracker, error)
 		}
 	}
 
-	ctx, err := buildContext(spec.Metadata, resources)
-	if err != nil {
-		return nil, nil, fmt.Errorf("preparing context: %w", err)
-	}
-
-	// This is already  validated by spec validation
-	workloadName, _ := spec.Metadata["name"].(string)
-
 	var ports []compose.ServicePortConfig
 	if spec.Service != nil && len(spec.Service.Ports) > 0 {
 		ports = []compose.ServicePortConfig{}

internal/compose/envvar_tracker.go

@@ -6,18 +6,17 @@ import (
 	"strings"
 )
 
+// EnvVarTracker is used to provide the `environment` resource type. This tracks what keys are accessed and replaces
+// them with outputs that are environment variable references that docker compose will support.
+// This keeps track of which keys were accessed so that we can produce a reference file or list of keys for the user
+// to understand what inputs docker compose will require at launch time.
 type EnvVarTracker struct {
-	lookup   func(key string) (string, bool)
+	// lookup is an environment variable lookup function, if nil this will be defaulted to os.LookupEnv
+	lookup func(key string) (string, bool)
+	// accessed is the map of accessed environment variables and the value they had at access time
 	accessed map[string]string
 }
 
-func NewEnvVarTracker() *EnvVarTracker {
-	return &EnvVarTracker{
-		lookup:   os.LookupEnv,
-		accessed: make(map[string]string),
-	}
-}
-
 func (e *EnvVarTracker) Accessed() map[string]string {
 	return maps.Clone(e.accessed)
 }
@@ -30,7 +29,18 @@ func (e *EnvVarTracker) LookupOutput(keys ...string) (interface{}, error) {
 		panic("requires at least 1 key")
 	}
 	envVarKey := strings.ToUpper(strings.Join(keys, "_"))
+
+	// in theory we can replace more unexpected characters
 	envVarKey = strings.ReplaceAll(envVarKey, "-", "_")
+	envVarKey = strings.ReplaceAll(envVarKey, ".", "_")
+
+	if e.lookup == nil {
+		e.lookup = os.LookupEnv
+	}
+	if e.accessed == nil {
+		e.accessed = make(map[string]string, 1)
+	}
+
 	if v, ok := e.lookup(envVarKey); ok {
 		e.accessed[envVarKey] = v
 	} else {
@@ -39,6 +49,15 @@ func (e *EnvVarTracker) LookupOutput(keys ...string) (interface{}, error) {
 	return "${" + envVarKey + "}", nil
 }
 
+func (e *EnvVarTracker) GenerateResource(resName string) ResourceWithOutputs {
+	return &envVarResourceTracker{
+		inner:  e,
+		prefix: resName,
+	}
+}
+
+// envVarResourceTracker is a child object of EnvVarTracker and is used as a fallback behavior for resource types
+// that are not supported natively: we treat them like environment variables instead with a prefix of the resource name.
 type envVarResourceTracker struct {
 	prefix string
 	inner  *EnvVarTracker
@@ -52,10 +71,3 @@ func (e *envVarResourceTracker) LookupOutput(keys ...string) (interface{}, error
 	}
 	return e.inner.LookupOutput(next...)
 }
-
-func (e *EnvVarTracker) GenerateResource(resName string) ResourceWithOutputs {
-	return &envVarResourceTracker{
-		inner:  e,
-		prefix: resName,
-	}
-}

internal/compose/templates.go

@@ -20,6 +20,14 @@ var (
 	placeholderRegEx = regexp.MustCompile(`\$(\$|{([a-zA-Z0-9.\-_\\]+)})`)
 )
 
+// ResourceWithOutputs is an interface that resource implementations in the future may provide.
+// The keys here are the parts of a .-separated path traversal down a tree to return some data from the outputs of
+// the provisioned resource. If an error occurs looking up the output, an error should be thrown.
+// nil is a valid result since some resources may return null in their outputs.
+type ResourceWithOutputs interface {
+	LookupOutput(keys ...string) (interface{}, error)
+}
+
 // templatesContext ia an utility type that provides a context for '${...}' templates substitution
 type templatesContext struct {
 	meta      map[string]interface{}

internal/compose/templates_test.go

@@ -19,7 +19,7 @@ func TestMapVar(t *testing.T) {
 		"name":  "test-name",
 		"other": map[string]interface{}{"key": "value"},
 	}
-	evt := NewEnvVarTracker()
+	evt := new(EnvVarTracker)
 	evt.lookup = func(key string) (string, bool) {
 		if key == "DEBUG" {
 			return "something", true
@@ -77,7 +77,7 @@ func TestSubstitute(t *testing.T) {
 	var meta = score.WorkloadMetadata{
 		"name": "test-name",
 	}
-	evt := NewEnvVarTracker()
+	evt := new(EnvVarTracker)
 	evt.lookup = func(key string) (string, bool) {
 		if key == "DEBUG" {
 			return "something", true