Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: improved resource referencing and throw errors for invalid expressions #54

Merged
merged 6 commits into from
Mar 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions examples/04-extras/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ containers:
hello:
image: nginx
volumes:
- source: ${resources.data}
- source: data
target: /usr/share/nginx/html
readOnly: true

Expand Down Expand Up @@ -50,7 +50,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)).

Expand Down
2 changes: 1 addition & 1 deletion examples/04-extras/score.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ containers:
hello:
image: nginx
volumes:
- source: ${resources.data}
- source: data
target: /usr/share/nginx/html
readOnly: true

Expand Down
4 changes: 2 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +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/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
Expand All @@ -20,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
Expand Down
7 changes: 2 additions & 5 deletions internal/command/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
4 changes: 4 additions & 0 deletions internal/command/run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
55 changes: 44 additions & 11 deletions internal/compose/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,7 @@ 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) {
workloadName, ok := spec.Metadata["name"].(string)
if !ok || len(workloadName) == 0 {
return nil, nil, errors.New("workload metadata is missing a name")
Expand All @@ -37,7 +32,29 @@ func ConvertSpec(spec *score.Workload) (*compose.Project, ExternalVariables, err
Services: make(compose.Services, 0, len(spec.Containers)),
}

externalVars := ExternalVariables(ctx.ListEnvVars())
// 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 {
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 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)
}
}

ctx, err := buildContext(spec.Metadata, resources)
if err != nil {
return nil, nil, fmt.Errorf("preparing context: %w", err)
}

var ports []compose.ServicePortConfig
if spec.Service != nil && len(spec.Service.Ports) > 0 {
Expand Down Expand Up @@ -70,8 +87,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
Expand All @@ -87,9 +107,22 @@ 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"))
}

// 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: ctx.Substitute(vol.Source),
Source: resolvedVolumeSource,
Target: vol.Target,
ReadOnly: DerefOr(vol.ReadOnly, false),
}
Expand Down Expand Up @@ -119,5 +152,5 @@ func ConvertSpec(spec *score.Workload) (*compose.Project, ExternalVariables, err

project.Services = append(project.Services, svc)
}
return &project, externalVars, nil
return &project, envVarTracker, nil
}
54 changes: 41 additions & 13 deletions internal/compose/convert_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -93,7 +93,6 @@ func TestScoreConvert(t *testing.T) {
},
},
},
Vars: ExternalVariables{},
},
{
Name: "Should convert all resources references",
Expand All @@ -112,7 +111,7 @@ func TestScoreConvert(t *testing.T) {
},
Volumes: []score.ContainerVolumesElem{
{
Source: "${resources.data}",
Source: "data",
Target: "/mnt/data",
ReadOnly: Ref(true),
},
Expand All @@ -126,7 +125,7 @@ func TestScoreConvert(t *testing.T) {
"app-db": {
Type: "postgress",
},
"dns": {
"some-dns": {
Type: "dns",
},
"data": {
Expand All @@ -142,7 +141,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{
Expand All @@ -156,11 +155,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": "",
},
},
{
Expand Down Expand Up @@ -213,7 +213,6 @@ func TestScoreConvert(t *testing.T) {
},
},
},
Vars: ExternalVariables{},
},

// Errors handling
Expand All @@ -229,7 +228,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),
Expand All @@ -245,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 {
Expand All @@ -260,7 +288,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())
}
})
}
Expand Down
73 changes: 73 additions & 0 deletions internal/compose/envvar_tracker.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
package compose

import (
"maps"
"os"
"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 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 (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, "_"))

// 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 {
e.accessed[envVarKey] = ""
}
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
}

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...)
}
31 changes: 31 additions & 0 deletions internal/compose/resources.go
Original file line number Diff line number Diff line change
@@ -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)
Loading
Loading