From 9a49b73caa404982962981f9466727b668c3f475 Mon Sep 17 00:00:00 2001 From: Ben Meier Date: Fri, 1 Mar 2024 18:44:12 +0000 Subject: [PATCH 1/6] feat: improved resource referencing predictability Signed-off-by: Ben Meier --- examples/04-extras/README.md | 8 +- examples/04-extras/score.yaml | 2 +- go.mod | 1 + go.sum | 2 + internal/command/run.go | 7 +- internal/compose/convert.go | 53 ++++++---- internal/compose/convert_test.go | 27 ++--- internal/compose/envvar_tracker.go | 61 ++++++++++++ internal/compose/resources.go | 30 ++++++ internal/compose/templates.go | 135 +++++++++++++------------ internal/compose/templates_test.go | 153 ++++++++++++++++++----------- 11 files changed, 310 insertions(+), 169 deletions(-) create mode 100644 internal/compose/envvar_tracker.go create mode 100644 internal/compose/resources.go diff --git a/examples/04-extras/README.md b/examples/04-extras/README.md index ff7a1c2..62e10d3 100644 --- a/examples/04-extras/README.md +++ b/examples/04-extras/README.md @@ -20,13 +20,9 @@ containers: hello: image: nginx volumes: - - source: ${resources.data} + - source: data target: /usr/share/nginx/html readOnly: true - -resources: - data: - type: volume ``` To convert `score.yaml` file into runnable `web-app.compose.yaml` use a `score-compose` CLI tool: @@ -50,7 +46,7 @@ services: target: /usr/share/nginx/html ``` -This compose service configuration references a volume called `data`, that should be available in the target environment by the time service starts. +This compose service configuration references a volume called `data`, that should be available in the target environment by the time the service starts. If running the service with the Docker, the volume can be created manually ([read more](https://docs.docker.com/storage/volumes/#create-and-manage-volumes)). diff --git a/examples/04-extras/score.yaml b/examples/04-extras/score.yaml index c5307be..d7536f6 100644 --- a/examples/04-extras/score.yaml +++ b/examples/04-extras/score.yaml @@ -13,7 +13,7 @@ containers: hello: image: nginx volumes: - - source: ${resources.data} + - source: data target: /usr/share/nginx/html readOnly: true diff --git a/go.mod b/go.mod index ac8a02f..42fd2ea 100644 --- a/go.mod +++ b/go.mod @@ -8,6 +8,7 @@ require ( github.com/compose-spec/compose-go v1.6.0 github.com/imdario/mergo v0.3.13 github.com/mitchellh/mapstructure v1.5.0 + github.com/pkg/errors v0.9.1 github.com/score-spec/score-go v1.1.0 github.com/spf13/cobra v1.6.0 github.com/stretchr/testify v1.8.0 diff --git a/go.sum b/go.sum index 14e3c09..c03a521 100644 --- a/go.sum +++ b/go.sum @@ -22,6 +22,8 @@ github.com/mitchellh/mapstructure v1.5.0 h1:jeMsZIYE/09sWLaz43PL7Gy6RuMjD2eJVyua github.com/mitchellh/mapstructure v1.5.0/go.mod h1:bFUtVrKA4DC2yAKiSyO/QUcy7e+RRV2QTWOzhPopBRo= github.com/opencontainers/go-digest v1.0.0 h1:apOUWs51W5PlhuyGyz9FCeeBIOUDA/6nW8Oi/yOhh5U= github.com/opencontainers/go-digest v1.0.0/go.mod h1:0JzlMkj0TRzQZfJkVvzbP0HBR3IKzErnv2BNG4W4MAM= +github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= +github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= diff --git a/internal/command/run.go b/internal/command/run.go index 1504456..fe02055 100644 --- a/internal/command/run.go +++ b/internal/command/run.go @@ -232,11 +232,8 @@ func run(cmd *cobra.Command, args []string) error { // log.Print("Writing .env file template...\n") - envVars := make([]string, 0, len(vars)) - for key, val := range vars { - if val == nil { - val = "" - } + envVars := make([]string, 0) + for key, val := range vars.Accessed() { var envVar = fmt.Sprintf("%s=%v\n", key, val) envVars = append(envVars, envVar) } diff --git a/internal/compose/convert.go b/internal/compose/convert.go index c092cd5..7576790 100644 --- a/internal/compose/convert.go +++ b/internal/compose/convert.go @@ -18,26 +18,38 @@ import ( ) // ConvertSpec converts SCORE specification into docker-compose configuration. -func ConvertSpec(spec *score.Workload) (*compose.Project, ExternalVariables, error) { - ctx, err := buildContext(spec.Metadata, spec.Resources) - if err != nil { - return nil, nil, fmt.Errorf("preparing context: %w", err) - } +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() - workloadName, ok := spec.Metadata["name"].(string) - if !ok || len(workloadName) == 0 { - return nil, nil, errors.New("workload metadata is missing a name") + var project = compose.Project{ + Services: make(compose.Services, 0, len(spec.Containers)), } - if len(spec.Containers) == 0 { - return nil, nil, errors.New("workload does not have any containers to convert into a compose service") + // this map holds the results of the provisioning process + resources := make(map[string]ResourceWithOutputs) + + // 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! + for resourceName, resourceSpec := range spec.Resources { + if resourceSpec.Type == "environment" { + if DerefOr(resourceSpec.Class, "default") != "default" { + return nil, nil, fmt.Errorf("resources.%s: '%s.%s' is not supported in score-compose", resourceName, resourceSpec.Type, *resourceSpec.Class) + } + resources[resourceName] = envVarTracker + } else { + // TODO: only enable this if the type.class is in an allow-list or the allow-list is '*' - otherwise return an error + resources[resourceName] = envVarTracker.GenerateResource(resourceName) + } } - var project = compose.Project{ - Services: make(compose.Services, 0, len(spec.Containers)), + ctx, err := buildContext(spec.Metadata, resources) + if err != nil { + return nil, nil, fmt.Errorf("preparing context: %w", err) } - externalVars := ExternalVariables(ctx.ListEnvVars()) + // 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 { @@ -70,8 +82,11 @@ func ConvertSpec(spec *score.Workload) (*compose.Project, ExternalVariables, err var env = make(compose.MappingWithEquals, len(cSpec.Variables)) for key, val := range cSpec.Variables { - var envVarVal = ctx.Substitute(val) - env[key] = &envVarVal + resolved, err := ctx.Substitute(val) + if err != nil { + return nil, nil, fmt.Errorf("containers.%s.variables.%s: %w", containerName, key, err) + } + env[key] = &resolved } // NOTE: Sorting is necessary for DeepEqual call within our Unit Tests to work reliably @@ -87,9 +102,13 @@ func ConvertSpec(spec *score.Workload) (*compose.Project, ExternalVariables, err if vol.Path != nil && *vol.Path != "" { return nil, nil, fmt.Errorf("can't mount named volume with sub path '%s': %w", *vol.Path, errors.New("not supported")) } + resolvedVolumeSource, err := ctx.Substitute(vol.Source) + if err != nil { + return nil, nil, fmt.Errorf("containers.%s.volumes[%d].source: %w", containerName, idx, err) + } volumes[idx] = compose.ServiceVolumeConfig{ Type: "volume", - Source: ctx.Substitute(vol.Source), + Source: resolvedVolumeSource, Target: vol.Target, ReadOnly: DerefOr(vol.ReadOnly, false), } @@ -119,5 +138,5 @@ func ConvertSpec(spec *score.Workload) (*compose.Project, ExternalVariables, err project.Services = append(project.Services, svc) } - return &project, externalVars, nil + return &project, envVarTracker, nil } diff --git a/internal/compose/convert_test.go b/internal/compose/convert_test.go index 390dfa0..f534437 100644 --- a/internal/compose/convert_test.go +++ b/internal/compose/convert_test.go @@ -25,7 +25,7 @@ func TestScoreConvert(t *testing.T) { Name string Source *score.Workload Project *compose.Project - Vars ExternalVariables + Vars map[string]string Error error }{ // Success path @@ -93,7 +93,7 @@ func TestScoreConvert(t *testing.T) { }, }, }, - Vars: ExternalVariables{}, + Vars: map[string]string{}, }, { Name: "Should convert all resources references", @@ -112,7 +112,7 @@ func TestScoreConvert(t *testing.T) { }, Volumes: []score.ContainerVolumesElem{ { - Source: "${resources.data}", + Source: "data", Target: "/mnt/data", ReadOnly: Ref(true), }, @@ -126,7 +126,7 @@ func TestScoreConvert(t *testing.T) { "app-db": { Type: "postgress", }, - "dns": { + "some-dns": { Type: "dns", }, "data": { @@ -142,7 +142,7 @@ func TestScoreConvert(t *testing.T) { Environment: compose.MappingWithEquals{ "DEBUG": stringPtr("${DEBUG}"), "LOGS_LEVEL": stringPtr("${LOGS_LEVEL}"), - "DOMAIN_NAME": stringPtr(""), + "DOMAIN_NAME": stringPtr("${SOME_DNS_DOMAIN_NAME}"), "CONNECTION_STRING": stringPtr("postgresql://${APP_DB_HOST}:${APP_DB_PORT}/${APP_DB_NAME}"), }, Volumes: []compose.ServiceVolumeConfig{ @@ -156,11 +156,12 @@ func TestScoreConvert(t *testing.T) { }, }, }, - Vars: ExternalVariables{ - "DEBUG": "", - "APP_DB_HOST": "", - "APP_DB_PORT": "", - "APP_DB_NAME": "", + Vars: map[string]string{ + "DEBUG": "", + "APP_DB_HOST": "", + "APP_DB_PORT": "", + "APP_DB_NAME": "", + "SOME_DNS_DOMAIN_NAME": "", }, }, { @@ -213,7 +214,7 @@ func TestScoreConvert(t *testing.T) { }, }, }, - Vars: ExternalVariables{}, + Vars: map[string]string{}, }, // Errors handling @@ -229,7 +230,7 @@ func TestScoreConvert(t *testing.T) { Image: "busybox", Volumes: []score.ContainerVolumesElem{ { - Source: "${resources.data}", + Source: "data", Target: "/mnt/data", Path: Ref("sub/path"), ReadOnly: Ref(true), @@ -260,7 +261,7 @@ func TestScoreConvert(t *testing.T) { // assert.NoError(t, err) assert.Equal(t, tt.Project, proj) - assert.Equal(t, tt.Vars, vars) + assert.Equal(t, tt.Vars, vars.Accessed()) } }) } diff --git a/internal/compose/envvar_tracker.go b/internal/compose/envvar_tracker.go new file mode 100644 index 0000000..eff45f9 --- /dev/null +++ b/internal/compose/envvar_tracker.go @@ -0,0 +1,61 @@ +package compose + +import ( + "maps" + "os" + "strings" +) + +type EnvVarTracker struct { + lookup func(key string) (string, bool) + 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) +} + +// the env var tracker is a resource itself (an environment resource) +var _ ResourceWithOutputs = (*EnvVarTracker)(nil) + +func (e *EnvVarTracker) LookupOutput(keys ...string) (interface{}, error) { + if len(keys) == 0 { + panic("requires at least 1 key") + } + envVarKey := strings.ToUpper(strings.Join(keys, "_")) + envVarKey = strings.ReplaceAll(envVarKey, "-", "_") + if v, ok := e.lookup(envVarKey); ok { + e.accessed[envVarKey] = v + } else { + e.accessed[envVarKey] = "" + } + return "${" + envVarKey + "}", nil +} + +type envVarResourceTracker struct { + prefix string + inner *EnvVarTracker +} + +func (e *envVarResourceTracker) LookupOutput(keys ...string) (interface{}, error) { + next := make([]string, 1+len(keys)) + next[0] = e.prefix + for i, k := range keys { + next[1+i] = k + } + return e.inner.LookupOutput(next...) +} + +func (e *EnvVarTracker) GenerateResource(resName string) ResourceWithOutputs { + return &envVarResourceTracker{ + inner: e, + prefix: resName, + } +} diff --git a/internal/compose/resources.go b/internal/compose/resources.go new file mode 100644 index 0000000..49d5b50 --- /dev/null +++ b/internal/compose/resources.go @@ -0,0 +1,30 @@ +package compose + +import ( + "fmt" + "strings" +) + +type ResourceWithOutputs interface { + LookupOutput(keys ...string) (interface{}, error) +} + +type StaticResource struct { + Outputs map[string]interface{} +} + +func (sr *StaticResource) LookupOutput(keys ...string) (interface{}, error) { + resolvedValue := interface{}(sr.Outputs) + remainingKeys := keys + for partIndex, part := range remainingKeys { + mapV, ok := resolvedValue.(map[string]interface{}) + if !ok { + return nil, fmt.Errorf("cannot lookup a key in %T", resolvedValue) + } + resolvedValue, ok = mapV[part] + if !ok { + return nil, fmt.Errorf("output '%s' does not exist", strings.Join(keys[:partIndex], ".")) + } + } + return resolvedValue, nil +} diff --git a/internal/compose/templates.go b/internal/compose/templates.go index d5148b8..d89ccf5 100644 --- a/internal/compose/templates.go +++ b/internal/compose/templates.go @@ -8,119 +8,118 @@ The Apache Software Foundation (http://www.apache.org/). package compose import ( + "encoding/json" + "errors" "fmt" - "log" + "maps" "regexp" "strings" - - "github.com/mitchellh/mapstructure" - - score "github.com/score-spec/score-go/types" ) var ( - placeholderRegEx = regexp.MustCompile(`\$(\$|{([a-zA-Z0-9.\-_\[\]"'#]+)})`) + placeholderRegEx = regexp.MustCompile(`\$(\$|{([a-zA-Z0-9.\-_\\]+)})`) ) // templatesContext ia an utility type that provides a context for '${...}' templates substitution type templatesContext struct { meta map[string]interface{} - resources score.WorkloadResources - - // env map is populated dynamically with any refenced variable used by Substitute - env map[string]interface{} + resources map[string]ResourceWithOutputs } // buildContext initializes a new templatesContext instance -func buildContext(metadata score.WorkloadMetadata, resources score.WorkloadResources) (*templatesContext, error) { - var metadataMap = make(map[string]interface{}) - if decoder, err := mapstructure.NewDecoder(&mapstructure.DecoderConfig{ - TagName: "json", - Result: &metadataMap, - }); err != nil { - return nil, err - } else { - decoder.Decode(metadata) - } - +func buildContext(metadata map[string]interface{}, resources map[string]ResourceWithOutputs) (*templatesContext, error) { return &templatesContext{ - meta: metadataMap, - resources: resources, - - env: make(map[string]interface{}), + meta: maps.Clone(metadata), + resources: maps.Clone(resources), }, nil } // Substitute replaces all matching '${...}' templates in a source string -func (ctx *templatesContext) Substitute(src string) string { - return placeholderRegEx.ReplaceAllStringFunc(src, func(str string) string { +func (ctx *templatesContext) Substitute(src string) (string, error) { + var err error + result := placeholderRegEx.ReplaceAllStringFunc(src, func(str string) string { // WORKAROUND: ReplaceAllStringFunc(..) does not provide match details // https://github.com/golang/go/issues/5690 var matches = placeholderRegEx.FindStringSubmatch(str) // SANITY CHECK if len(matches) != 3 { - log.Printf("Error: could not find a proper match in previously captured string fragment") + err = errors.Join(err, fmt.Errorf("could not find a proper match in previously captured string fragment")) return src } // EDGE CASE: Captures "$$" sequences and empty templates "${}" if matches[2] == "" { return matches[1] + } else if matches[2] == "$" { + return matches[2] } - return ctx.mapVar(matches[2]) + result, subErr := ctx.mapVar(matches[2]) + err = errors.Join(err, subErr) + return result }) + return result, err } // MapVar replaces objects and properties references with corresponding values // Returns an empty string if the reference can't be resolved -func (ctx *templatesContext) mapVar(ref string) string { - if ref == "" || ref == "$" { - return ref +func (ctx *templatesContext) mapVar(ref string) (string, error) { + subRef := strings.Replace(ref, `\.`, "\000", -1) + parts := strings.Split(subRef, ".") + for i, part := range parts { + parts[i] = strings.Replace(part, "\000", ".", -1) } - var segments = strings.SplitN(ref, ".", 2) - switch segments[0] { + var resolvedValue interface{} + var remainingParts []string + + switch parts[0] { case "metadata": - if len(segments) == 2 { - if val, exists := ctx.meta[segments[1]]; exists { - return fmt.Sprintf("%v", val) - } + if len(parts) < 2 { + return "", fmt.Errorf("invalid ref '%s': requires at least a metadata key to lookup", ref) } - - case "resources": - if len(segments) == 2 { - segments = strings.SplitN(segments[1], ".", 2) - var resName = segments[0] - if res, exists := ctx.resources[resName]; exists { - if len(segments) == 1 { - return resName - } else { - var propName = segments[1] - - var envVar string - switch res.Type { - case "environment": - envVar = strings.ToUpper(propName) - default: - envVar = strings.ToUpper(fmt.Sprintf("%s_%s", resName, propName)) - } - envVar = strings.Replace(envVar, "-", "_", -1) - envVar = strings.Replace(envVar, ".", "_", -1) - - ctx.env[envVar] = "" - return fmt.Sprintf("${%s}", envVar) + if rv, ok := ctx.meta[parts[1]]; ok { + resolvedValue = rv + remainingParts = parts[2:] + for _, part := range remainingParts { + mapV, ok := resolvedValue.(map[string]interface{}) + if !ok { + return "", fmt.Errorf("invalid ref '%s': cannot lookup a key in %T", ref, resolvedValue) + } + resolvedValue, ok = mapV[part] + if !ok { + return "", fmt.Errorf("invalid ref '%s': key '%s' does not exist", ref, part) } } + } else { + return "", fmt.Errorf("invalid ref '%s': unknown metadata key '%s'", ref, parts[1]) + } + case "resources": + if len(parts) < 2 { + return "", fmt.Errorf("invalid ref '%s': requires at least a resource name to lookup", ref) } + rv, ok := ctx.resources[parts[1]] + if !ok { + return "", fmt.Errorf("invalid ref '%s': no known resource '%s'", ref, parts[1]) + } else if len(parts) == 2 { + return "", fmt.Errorf("invalid ref '%s': an output key is required", ref) + } else if rv2, err := rv.LookupOutput(parts[2:]...); err != nil { + return "", err + } else { + resolvedValue = rv2 + } + default: + return "", fmt.Errorf("invalid ref '%s': unknown reference root", ref) } - log.Printf("Warning: Can not resolve '%s' reference.", ref) - return "" -} - -// ListEnvVars reports all environment variables used by templatesContext -func (ctx *templatesContext) ListEnvVars() map[string]interface{} { - return ctx.env + if asString, ok := resolvedValue.(string); ok { + return asString, nil + } + // TODO: work out how we might support other types here in the future + raw, err := json.Marshal(resolvedValue) + if err != nil { + return "", err + } + return string(raw), nil } diff --git a/internal/compose/templates_test.go b/internal/compose/templates_test.go index 76fd9a8..8f782a5 100644 --- a/internal/compose/templates_test.go +++ b/internal/compose/templates_test.go @@ -16,80 +16,115 @@ import ( func TestMapVar(t *testing.T) { var meta = score.WorkloadMetadata{ - "name": "test-name", + "name": "test-name", + "other": map[string]interface{}{"key": "value"}, } - - var resources = score.WorkloadResources{ - "env": score.Resource{ - Type: "environment", - }, - "db": score.Resource{ - Type: "postgres", - }, + evt := NewEnvVarTracker() + evt.lookup = func(key string) (string, bool) { + if key == "DEBUG" { + return "something", true + } + return "", false } - - ctx, err := buildContext(meta, resources) + ctx, err := buildContext(meta, map[string]ResourceWithOutputs{ + "env": evt, + "db": evt.GenerateResource("db"), + }) assert.NoError(t, err) - assert.Equal(t, "", ctx.mapVar("")) - assert.Equal(t, "$", ctx.mapVar("$")) - - assert.Equal(t, "test-name", ctx.mapVar("metadata.name")) - assert.Equal(t, "", ctx.mapVar("metadata.name.nil")) - assert.Equal(t, "", ctx.mapVar("metadata.nil")) - - assert.Equal(t, "${DEBUG}", ctx.mapVar("resources.env.DEBUG")) - - assert.Equal(t, "db", ctx.mapVar("resources.db")) - assert.Equal(t, "${DB_HOST}", ctx.mapVar("resources.db.host")) - assert.Equal(t, "${DB_PORT}", ctx.mapVar("resources.db.port")) - assert.Equal(t, "${DB_NAME}", ctx.mapVar("resources.db.name")) - assert.Equal(t, "${DB_NAME_USER}", ctx.mapVar("resources.db.name.user")) + for _, tc := range []struct { + Input string + Expected string + ExpectedError string + }{ + {Input: "missing", ExpectedError: "invalid ref 'missing': unknown reference root"}, + {Input: "metadata.name", Expected: "test-name"}, + {Input: "metadata", ExpectedError: "invalid ref 'metadata': requires at least a metadata key to lookup"}, + {Input: "metadata.other", Expected: "{\"key\":\"value\"}"}, + {Input: "metadata.other.key", Expected: "value"}, + {Input: "metadata.missing", ExpectedError: "invalid ref 'metadata.missing': unknown metadata key 'missing'"}, + {Input: "metadata.name.foo", ExpectedError: "invalid ref 'metadata.name.foo': cannot lookup a key in string"}, + {Input: "resources.env", ExpectedError: "invalid ref 'resources.env': an output key is required"}, + {Input: "resources.env.DEBUG", Expected: "${DEBUG}"}, + {Input: "resources.missing", ExpectedError: "invalid ref 'resources.missing': no known resource 'missing'"}, + {Input: "resources.db", ExpectedError: "invalid ref 'resources.db': an output key is required"}, + {Input: "resources.db.host", Expected: "${DB_HOST}"}, + {Input: "resources.db.port", Expected: "${DB_PORT}"}, + {Input: "resources.db.name", Expected: "${DB_NAME}"}, + {Input: "resources.db.name.user", Expected: "${DB_NAME_USER}"}, + } { + t.Run(tc.Input, func(t *testing.T) { + res, err := ctx.mapVar(tc.Input) + if tc.ExpectedError != "" { + assert.EqualError(t, err, tc.ExpectedError) + } else { + assert.NoError(t, err) + assert.Equal(t, tc.Expected, res) + } + }) + } - assert.Equal(t, "", ctx.mapVar("resources.nil")) - assert.Equal(t, "", ctx.mapVar("nil.db.name")) + assert.Equal(t, map[string]string{ + "DEBUG": "something", + "DB_HOST": "", + "DB_NAME": "", + "DB_NAME_USER": "", + "DB_PORT": "", + }, evt.Accessed()) } func TestSubstitute(t *testing.T) { var meta = score.WorkloadMetadata{ "name": "test-name", } - - var resources = score.WorkloadResources{ - "env": score.Resource{ - Type: "environment", - }, - "db": score.Resource{ - Type: "postgres", - }, + evt := NewEnvVarTracker() + evt.lookup = func(key string) (string, bool) { + if key == "DEBUG" { + return "something", true + } + return "", false } - - ctx, err := buildContext(meta, resources) + ctx, err := buildContext(meta, map[string]ResourceWithOutputs{ + "env": evt, + "db": evt.GenerateResource("db"), + }) assert.NoError(t, err) - assert.Empty(t, ctx.ListEnvVars()) - - assert.Equal(t, "", ctx.Substitute("")) - assert.Equal(t, "abc", ctx.Substitute("abc")) - assert.Equal(t, "$abc", ctx.Substitute("$abc")) - assert.Equal(t, "abc $ abc", ctx.Substitute("abc $$ abc")) - assert.Equal(t, "${abc}", ctx.Substitute("$${abc}")) - - assert.Equal(t, "The name is 'test-name'", ctx.Substitute("The name is '${metadata.name}'")) - assert.Equal(t, "The name is ''", ctx.Substitute("The name is '${metadata.nil}'")) - - assert.Equal(t, "resources.badref.DEBUG", ctx.Substitute("resources.badref.DEBUG")) - - assert.Equal(t, "db", ctx.Substitute("${resources.db}")) - assert.Equal(t, - "postgresql://${DB_USER}:${DB_PASSWORD}@${DB_HOST}:${DB_PORT}/${DB_NAME}", - ctx.Substitute("postgresql://${resources.db.user}:${resources.db.password}@${resources.db.host}:${resources.db.port}/${resources.db.name}")) + for _, tc := range []struct { + Input string + Expected string + ExpectedError string + }{ + {Input: "", Expected: ""}, + {Input: "abc", Expected: "abc"}, + {Input: "$abc", Expected: "$abc"}, + {Input: "abc $$ abc", Expected: "abc $ abc"}, + {Input: "$${abc}", Expected: "${abc}"}, + {Input: "my name is ${metadata.name}", Expected: "my name is test-name"}, + {Input: "my name is ${metadata.thing\\.two}", ExpectedError: "invalid ref 'metadata.thing\\.two': unknown metadata key 'thing.two'"}, + { + Input: "postgresql://${resources.db.user}:${resources.db.password}@${resources.db.host}:${resources.db.port}/${resources.db.name}", + Expected: "postgresql://${DB_USER}:${DB_PASSWORD}@${DB_HOST}:${DB_PORT}/${DB_NAME}", + }, + } { + t.Run(tc.Input, func(t *testing.T) { + res, err := ctx.Substitute(tc.Input) + if tc.ExpectedError != "" { + if !assert.EqualError(t, err, tc.ExpectedError) { + assert.Equal(t, "", res) + } + } else { + assert.NoError(t, err) + assert.Equal(t, tc.Expected, res) + } + }) + } - assert.Equal(t, map[string]interface{}{ - "DB_USER": "", - "DB_PASSWORD": "", + assert.Equal(t, map[string]string{ "DB_HOST": "", - "DB_PORT": "", "DB_NAME": "", - }, ctx.ListEnvVars()) + "DB_PASSWORD": "", + "DB_PORT": "", + "DB_USER": "", + }, evt.Accessed()) } From 9218cf11df7ce76ebc60bd20b1b8597ef9ab45a0 Mon Sep 17 00:00:00 2001 From: Ben Meier Date: Fri, 1 Mar 2024 19:35:50 +0000 Subject: [PATCH 2/6] chore: go mod tidy Signed-off-by: Ben Meier --- go.mod | 5 ++--- go.sum | 2 -- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/go.mod b/go.mod index 42fd2ea..4956201 100644 --- a/go.mod +++ b/go.mod @@ -7,10 +7,9 @@ toolchain go1.21.0 require ( github.com/compose-spec/compose-go v1.6.0 github.com/imdario/mergo v0.3.13 - github.com/mitchellh/mapstructure v1.5.0 - github.com/pkg/errors v0.9.1 github.com/score-spec/score-go v1.1.0 github.com/spf13/cobra v1.6.0 + github.com/spf13/pflag v1.0.5 github.com/stretchr/testify v1.8.0 github.com/tidwall/sjson v1.2.5 gopkg.in/yaml.v3 v3.0.1 @@ -21,10 +20,10 @@ require ( github.com/distribution/distribution/v3 v3.0.0-20220725133111-4bf3547399eb // indirect github.com/docker/go-connections v0.4.0 // indirect github.com/inconshreveable/mousetrap v1.0.1 // indirect + github.com/mitchellh/mapstructure v1.5.0 // indirect github.com/opencontainers/go-digest v1.0.0 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect github.com/santhosh-tekuri/jsonschema/v5 v5.3.1 // indirect - github.com/spf13/pflag v1.0.5 // indirect github.com/tidwall/gjson v1.14.4 // indirect github.com/tidwall/match v1.1.1 // indirect github.com/tidwall/pretty v1.2.1 // indirect diff --git a/go.sum b/go.sum index c03a521..14e3c09 100644 --- a/go.sum +++ b/go.sum @@ -22,8 +22,6 @@ github.com/mitchellh/mapstructure v1.5.0 h1:jeMsZIYE/09sWLaz43PL7Gy6RuMjD2eJVyua github.com/mitchellh/mapstructure v1.5.0/go.mod h1:bFUtVrKA4DC2yAKiSyO/QUcy7e+RRV2QTWOzhPopBRo= github.com/opencontainers/go-digest v1.0.0 h1:apOUWs51W5PlhuyGyz9FCeeBIOUDA/6nW8Oi/yOhh5U= github.com/opencontainers/go-digest v1.0.0/go.mod h1:0JzlMkj0TRzQZfJkVvzbP0HBR3IKzErnv2BNG4W4MAM= -github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= -github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= From 767a77e9db979e2ac3317af81fbfa6e7ace5af04 Mon Sep 17 00:00:00 2001 From: Ben Meier Date: Fri, 1 Mar 2024 19:45:52 +0000 Subject: [PATCH 3/6] chore: cleaned up some unecessary complexity Signed-off-by: Ben Meier --- internal/compose/convert.go | 17 +++++++----- internal/compose/convert_test.go | 2 -- internal/compose/envvar_tracker.go | 42 +++++++++++++++++++----------- internal/compose/resources.go | 30 --------------------- internal/compose/templates.go | 8 ++++++ internal/compose/templates_test.go | 4 +-- 6 files changed, 47 insertions(+), 56 deletions(-) delete mode 100644 internal/compose/resources.go diff --git a/internal/compose/convert.go b/internal/compose/convert.go index 7576790..fd29943 100644 --- a/internal/compose/convert.go +++ b/internal/compose/convert.go @@ -19,16 +19,22 @@ 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() + 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 + // Track any uses of the environment resource or resources that are overridden with an env provider using the tracker. + envVarTracker := new(EnvVarTracker) resources := make(map[string]ResourceWithOutputs) - // 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! for resourceName, resourceSpec := range spec.Resources { @@ -48,9 +54,6 @@ func ConvertSpec(spec *score.Workload) (*compose.Project, *EnvVarTracker, error) 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{} diff --git a/internal/compose/convert_test.go b/internal/compose/convert_test.go index f534437..13f87b1 100644 --- a/internal/compose/convert_test.go +++ b/internal/compose/convert_test.go @@ -93,7 +93,6 @@ func TestScoreConvert(t *testing.T) { }, }, }, - Vars: map[string]string{}, }, { Name: "Should convert all resources references", @@ -214,7 +213,6 @@ func TestScoreConvert(t *testing.T) { }, }, }, - Vars: map[string]string{}, }, // Errors handling diff --git a/internal/compose/envvar_tracker.go b/internal/compose/envvar_tracker.go index eff45f9..0d31f3c 100644 --- a/internal/compose/envvar_tracker.go +++ b/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, - } -} diff --git a/internal/compose/resources.go b/internal/compose/resources.go deleted file mode 100644 index 49d5b50..0000000 --- a/internal/compose/resources.go +++ /dev/null @@ -1,30 +0,0 @@ -package compose - -import ( - "fmt" - "strings" -) - -type ResourceWithOutputs interface { - LookupOutput(keys ...string) (interface{}, error) -} - -type StaticResource struct { - Outputs map[string]interface{} -} - -func (sr *StaticResource) LookupOutput(keys ...string) (interface{}, error) { - resolvedValue := interface{}(sr.Outputs) - remainingKeys := keys - for partIndex, part := range remainingKeys { - mapV, ok := resolvedValue.(map[string]interface{}) - if !ok { - return nil, fmt.Errorf("cannot lookup a key in %T", resolvedValue) - } - resolvedValue, ok = mapV[part] - if !ok { - return nil, fmt.Errorf("output '%s' does not exist", strings.Join(keys[:partIndex], ".")) - } - } - return resolvedValue, nil -} diff --git a/internal/compose/templates.go b/internal/compose/templates.go index d89ccf5..69afff3 100644 --- a/internal/compose/templates.go +++ b/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{} diff --git a/internal/compose/templates_test.go b/internal/compose/templates_test.go index 8f782a5..846df1e 100644 --- a/internal/compose/templates_test.go +++ b/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 From b5dda4adf4df3a7f34bd53d30edc20bb09de9d86 Mon Sep 17 00:00:00 2001 From: Ben Meier Date: Mon, 4 Mar 2024 10:26:56 +0000 Subject: [PATCH 4/6] fix: validate that volume resource exists Signed-off-by: Ben Meier --- examples/04-extras/README.md | 4 ++++ internal/command/run_test.go | 4 ++++ internal/compose/convert.go | 9 +++++++++ internal/compose/templates.go | 3 ++- internal/compose/templates_test.go | 4 ++-- 5 files changed, 21 insertions(+), 3 deletions(-) diff --git a/examples/04-extras/README.md b/examples/04-extras/README.md index 62e10d3..719868d 100644 --- a/examples/04-extras/README.md +++ b/examples/04-extras/README.md @@ -23,6 +23,10 @@ containers: - source: data target: /usr/share/nginx/html readOnly: true + +resources: + data: + type: volume ``` To convert `score.yaml` file into runnable `web-app.compose.yaml` use a `score-compose` CLI tool: diff --git a/internal/command/run_test.go b/internal/command/run_test.go index ec4c28b..4132aeb 100644 --- a/internal/command/run_test.go +++ b/internal/command/run_test.go @@ -138,6 +138,10 @@ resources: data: here resource-two2: type: Resource-Two + volume-name: + type: volume + volume-two: + type: volume `), 0600)) stdout, stderr, err := executeAndResetCommand(context.Background(), rootCmd, []string{"run", "--file", filepath.Join(td, "score.yaml"), "--output", filepath.Join(td, "compose.yaml")}) assert.NoError(t, err) diff --git a/internal/compose/convert.go b/internal/compose/convert.go index fd29943..b9e0859 100644 --- a/internal/compose/convert.go +++ b/internal/compose/convert.go @@ -105,10 +105,19 @@ func ConvertSpec(spec *score.Workload) (*compose.Project, *EnvVarTracker, error) if vol.Path != nil && *vol.Path != "" { return nil, nil, fmt.Errorf("can't mount named volume with sub path '%s': %w", *vol.Path, errors.New("not supported")) } + + // TODO: deprecate this - volume should be linked directly resolvedVolumeSource, err := ctx.Substitute(vol.Source) if err != nil { return nil, nil, fmt.Errorf("containers.%s.volumes[%d].source: %w", containerName, idx, err) } + + if res, ok := spec.Resources[resolvedVolumeSource]; !ok { + return nil, nil, fmt.Errorf("containers.%s.volumes[%d].source: resource '%s' does not exist", containerName, idx, resolvedVolumeSource) + } else if res.Type != "volume" { + return nil, nil, fmt.Errorf("containers.%s.volumes[%d].source: resource '%s' is not a volume", containerName, idx, resolvedVolumeSource) + } + volumes[idx] = compose.ServiceVolumeConfig{ Type: "volume", Source: resolvedVolumeSource, diff --git a/internal/compose/templates.go b/internal/compose/templates.go index 69afff3..116af12 100644 --- a/internal/compose/templates.go +++ b/internal/compose/templates.go @@ -111,7 +111,8 @@ func (ctx *templatesContext) mapVar(ref string) (string, error) { if !ok { return "", fmt.Errorf("invalid ref '%s': no known resource '%s'", ref, parts[1]) } else if len(parts) == 2 { - return "", fmt.Errorf("invalid ref '%s': an output key is required", ref) + // TODO: deprecate this - this is an annoying and nonsensical legacy thing + return parts[1], nil } else if rv2, err := rv.LookupOutput(parts[2:]...); err != nil { return "", err } else { diff --git a/internal/compose/templates_test.go b/internal/compose/templates_test.go index 846df1e..c04a100 100644 --- a/internal/compose/templates_test.go +++ b/internal/compose/templates_test.go @@ -44,10 +44,10 @@ func TestMapVar(t *testing.T) { {Input: "metadata.other.key", Expected: "value"}, {Input: "metadata.missing", ExpectedError: "invalid ref 'metadata.missing': unknown metadata key 'missing'"}, {Input: "metadata.name.foo", ExpectedError: "invalid ref 'metadata.name.foo': cannot lookup a key in string"}, - {Input: "resources.env", ExpectedError: "invalid ref 'resources.env': an output key is required"}, + {Input: "resources.env", Expected: "env"}, {Input: "resources.env.DEBUG", Expected: "${DEBUG}"}, {Input: "resources.missing", ExpectedError: "invalid ref 'resources.missing': no known resource 'missing'"}, - {Input: "resources.db", ExpectedError: "invalid ref 'resources.db': an output key is required"}, + {Input: "resources.db", Expected: "db"}, {Input: "resources.db.host", Expected: "${DB_HOST}"}, {Input: "resources.db.port", Expected: "${DB_PORT}"}, {Input: "resources.db.name", Expected: "${DB_NAME}"}, From 3c3c5af1097af17bfc0ab75faea82ddd60fcf336 Mon Sep 17 00:00:00 2001 From: Ben Meier Date: Mon, 4 Mar 2024 10:29:50 +0000 Subject: [PATCH 5/6] chore: added tests for volume resource validation Signed-off-by: Ben Meier --- internal/compose/convert_test.go | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/internal/compose/convert_test.go b/internal/compose/convert_test.go index 13f87b1..effd671 100644 --- a/internal/compose/convert_test.go +++ b/internal/compose/convert_test.go @@ -244,6 +244,35 @@ func TestScoreConvert(t *testing.T) { }, Error: errors.New("not supported"), }, + + { + Name: "Should report an error for volume that doesn't exist in resources", + Source: &score.Workload{ + Metadata: score.WorkloadMetadata{"name": "test"}, + Containers: score.WorkloadContainers{ + "test": score.Container{ + Image: "busybox", + Volumes: []score.ContainerVolumesElem{{Source: "data", Target: "/mnt/data"}}, + }, + }, + }, + Error: errors.New("containers.test.volumes[0].source: resource 'data' does not exist"), + }, + + { + Name: "Should report an error for volume resource that isn't a volume", + Source: &score.Workload{ + Metadata: score.WorkloadMetadata{"name": "test"}, + Containers: score.WorkloadContainers{ + "test": score.Container{ + Image: "busybox", + Volumes: []score.ContainerVolumesElem{{Source: "data", Target: "/mnt/data"}}, + }, + }, + Resources: map[string]score.Resource{"data": {Type: "thing"}}, + }, + Error: errors.New("containers.test.volumes[0].source: resource 'data' is not a volume"), + }, } for _, tt := range tests { From 724a1fc05f67f4410ce8003092cd7b27b61b9a12 Mon Sep 17 00:00:00 2001 From: Ben Meier Date: Mon, 4 Mar 2024 10:49:57 +0000 Subject: [PATCH 6/6] chore: implement volume as a static resource with no outputs Signed-off-by: Ben Meier --- internal/compose/convert.go | 2 ++ internal/compose/resources.go | 31 ++++++++++++++++++++++++++++++ internal/compose/templates.go | 30 +++++------------------------ internal/compose/templates_test.go | 14 +++++++++----- 4 files changed, 47 insertions(+), 30 deletions(-) create mode 100644 internal/compose/resources.go diff --git a/internal/compose/convert.go b/internal/compose/convert.go index b9e0859..cb9cf31 100644 --- a/internal/compose/convert.go +++ b/internal/compose/convert.go @@ -43,6 +43,8 @@ func ConvertSpec(spec *score.Workload) (*compose.Project, *EnvVarTracker, error) return nil, nil, fmt.Errorf("resources.%s: '%s.%s' is not supported in score-compose", resourceName, resourceSpec.Type, *resourceSpec.Class) } resources[resourceName] = envVarTracker + } else if resourceSpec.Type == "volume" && DerefOr(resourceSpec.Class, "default") == "default" { + resources[resourceName] = resourceWithStaticOutputs{} } else { // TODO: only enable this if the type.class is in an allow-list or the allow-list is '*' - otherwise return an error resources[resourceName] = envVarTracker.GenerateResource(resourceName) diff --git a/internal/compose/resources.go b/internal/compose/resources.go new file mode 100644 index 0000000..b498dbd --- /dev/null +++ b/internal/compose/resources.go @@ -0,0 +1,31 @@ +package compose + +import "fmt" + +// 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) +} + +type resourceWithStaticOutputs map[string]interface{} + +func (r resourceWithStaticOutputs) LookupOutput(keys ...string) (interface{}, error) { + var resolvedValue interface{} + resolvedValue = (map[string]interface{})(r) + for _, k := range keys { + mapV, ok := resolvedValue.(map[string]interface{}) + if !ok { + return "", fmt.Errorf("cannot lookup key '%s', context is not a map", k) + } + resolvedValue, ok = mapV[k] + if !ok { + return "", fmt.Errorf("key '%s' not found", k) + } + } + return resolvedValue, nil +} + +var _ ResourceWithOutputs = (resourceWithStaticOutputs)(nil) diff --git a/internal/compose/templates.go b/internal/compose/templates.go index 116af12..a5388a6 100644 --- a/internal/compose/templates.go +++ b/internal/compose/templates.go @@ -20,17 +20,9 @@ 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{} + meta resourceWithStaticOutputs resources map[string]ResourceWithOutputs } @@ -80,28 +72,16 @@ func (ctx *templatesContext) mapVar(ref string) (string, error) { } var resolvedValue interface{} - var remainingParts []string switch parts[0] { case "metadata": if len(parts) < 2 { return "", fmt.Errorf("invalid ref '%s': requires at least a metadata key to lookup", ref) } - if rv, ok := ctx.meta[parts[1]]; ok { - resolvedValue = rv - remainingParts = parts[2:] - for _, part := range remainingParts { - mapV, ok := resolvedValue.(map[string]interface{}) - if !ok { - return "", fmt.Errorf("invalid ref '%s': cannot lookup a key in %T", ref, resolvedValue) - } - resolvedValue, ok = mapV[part] - if !ok { - return "", fmt.Errorf("invalid ref '%s': key '%s' does not exist", ref, part) - } - } + if rv, err := ctx.meta.LookupOutput(parts[1:]...); err != nil { + return "", fmt.Errorf("invalid ref '%s': %w", ref, err) } else { - return "", fmt.Errorf("invalid ref '%s': unknown metadata key '%s'", ref, parts[1]) + resolvedValue = rv } case "resources": if len(parts) < 2 { @@ -114,7 +94,7 @@ func (ctx *templatesContext) mapVar(ref string) (string, error) { // TODO: deprecate this - this is an annoying and nonsensical legacy thing return parts[1], nil } else if rv2, err := rv.LookupOutput(parts[2:]...); err != nil { - return "", err + return "", fmt.Errorf("invalid ref '%s': %w", ref, err) } else { resolvedValue = rv2 } diff --git a/internal/compose/templates_test.go b/internal/compose/templates_test.go index c04a100..ae51044 100644 --- a/internal/compose/templates_test.go +++ b/internal/compose/templates_test.go @@ -27,8 +27,9 @@ func TestMapVar(t *testing.T) { return "", false } ctx, err := buildContext(meta, map[string]ResourceWithOutputs{ - "env": evt, - "db": evt.GenerateResource("db"), + "env": evt, + "db": evt.GenerateResource("db"), + "static": resourceWithStaticOutputs{"x": "a"}, }) assert.NoError(t, err) @@ -42,8 +43,8 @@ func TestMapVar(t *testing.T) { {Input: "metadata", ExpectedError: "invalid ref 'metadata': requires at least a metadata key to lookup"}, {Input: "metadata.other", Expected: "{\"key\":\"value\"}"}, {Input: "metadata.other.key", Expected: "value"}, - {Input: "metadata.missing", ExpectedError: "invalid ref 'metadata.missing': unknown metadata key 'missing'"}, - {Input: "metadata.name.foo", ExpectedError: "invalid ref 'metadata.name.foo': cannot lookup a key in string"}, + {Input: "metadata.missing", ExpectedError: "invalid ref 'metadata.missing': key 'missing' not found"}, + {Input: "metadata.name.foo", ExpectedError: "invalid ref 'metadata.name.foo': cannot lookup key 'foo', context is not a map"}, {Input: "resources.env", Expected: "env"}, {Input: "resources.env.DEBUG", Expected: "${DEBUG}"}, {Input: "resources.missing", ExpectedError: "invalid ref 'resources.missing': no known resource 'missing'"}, @@ -52,6 +53,9 @@ func TestMapVar(t *testing.T) { {Input: "resources.db.port", Expected: "${DB_PORT}"}, {Input: "resources.db.name", Expected: "${DB_NAME}"}, {Input: "resources.db.name.user", Expected: "${DB_NAME_USER}"}, + {Input: "resources.static", Expected: "static"}, + {Input: "resources.static.x", Expected: "a"}, + {Input: "resources.static.y", ExpectedError: "invalid ref 'resources.static.y': key 'y' not found"}, } { t.Run(tc.Input, func(t *testing.T) { res, err := ctx.mapVar(tc.Input) @@ -101,7 +105,7 @@ func TestSubstitute(t *testing.T) { {Input: "abc $$ abc", Expected: "abc $ abc"}, {Input: "$${abc}", Expected: "${abc}"}, {Input: "my name is ${metadata.name}", Expected: "my name is test-name"}, - {Input: "my name is ${metadata.thing\\.two}", ExpectedError: "invalid ref 'metadata.thing\\.two': unknown metadata key 'thing.two'"}, + {Input: "my name is ${metadata.thing\\.two}", ExpectedError: "invalid ref 'metadata.thing\\.two': key 'thing.two' not found"}, { Input: "postgresql://${resources.db.user}:${resources.db.password}@${resources.db.host}:${resources.db.port}/${resources.db.name}", Expected: "postgresql://${DB_USER}:${DB_PASSWORD}@${DB_HOST}:${DB_PORT}/${DB_NAME}",