Repository: score-spec/score-compose
Pull request #59 (2024-03-06 14:38:07)

fix: require env var substitutions to be set
This PR makes it clear that the resource output properties used in placeholders are expected to be set and not optional. The compose spec expects resources to be provisioned and that failures in resource provisioning should prevent the workloads from being deployed. The analogue of this in Docker compose is that the expected resource references are not set. 

NOTE: that this doesn't necessarily apply to the `environment` resource type, whos outputs should be real outputs of the system environment where defaulting to an empty string is expected behavior.

So `${resources.db.user}` would require `$DB_USER` to be set, but `${resources.env.foo}` would allow `$FOO` to default to empty string.

#### Types of changes
- [X] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [ ] New chore (expected functionality to be implemented)

#### Checklist:
- [X] My change requires a change to the documentation.
  - [ ] I have updated the documentation accordingly.
- [X] I've signed off with an email address that matches the commit author.



Patch:
diff --git a/internal/compose/envvar_tracker.go b/internal/compose/envvar_tracker.go
index 0d31f3c..ebb0bda 100644
--- a/internal/compose/envvar_tracker.go
+++ b/internal/compose/envvar_tracker.go
@@ -24,7 +24,7 @@ func (e *EnvVarTracker) Accessed() map[string]string {
 // the env var tracker is a resource itself (an environment resource)
 var _ ResourceWithOutputs = (*EnvVarTracker)(nil)
 
-func (e *EnvVarTracker) LookupOutput(keys ...string) (interface{}, error) {
+func (e *EnvVarTracker) lookupOutput(required bool, keys ...string) (interface{}, error) {
 	if len(keys) == 0 {
 		panic("requires at least 1 key")
 	}
@@ -46,9 +46,16 @@ func (e *EnvVarTracker) LookupOutput(keys ...string) (interface{}, error) {
 	} else {
 		e.accessed[envVarKey] = ""
 	}
+	if required {
+		envVarKey += "?required"
+	}
 	return "${" + envVarKey + "}", nil
 }
 
+func (e *EnvVarTracker) LookupOutput(keys ...string) (interface{}, error) {
+	return e.lookupOutput(false, keys...)
+}
+
 func (e *EnvVarTracker) GenerateResource(resName string) ResourceWithOutputs {
 	return &envVarResourceTracker{
 		inner:  e,
@@ -69,5 +76,5 @@ func (e *envVarResourceTracker) LookupOutput(keys ...string) (interface{}, error
 	for i, k := range keys {
 		next[1+i] = k
 	}
-	return e.inner.LookupOutput(next...)
+	return e.inner.lookupOutput(true, next...)
 }


Test patch:
diff --git a/internal/command/run_test.go b/internal/command/run_test.go
index d2966b0..b2e423b 100644
--- a/internal/command/run_test.go
+++ b/internal/command/run_test.go
@@ -374,7 +374,7 @@ func TestRunExample03(t *testing.T) {
     entrypoint:
       - /bin/sh
     environment:
-      CONNECTION_STRING: postgresql://${DB_USER}:${DB_PASSWORD}@${DB_HOST}:${DB_PORT}/${DB_NAME}
+      CONNECTION_STRING: postgresql://${DB_USER?required}:${DB_PASSWORD?required}@${DB_HOST?required}:${DB_PORT?required}/${DB_NAME?required}
       FRIEND: ${NAME}
     image: busybox
 `
@@ -426,6 +426,7 @@ func TestRunExample03(t *testing.T) {
 		cmd.Dir = td
 		cmd.Stdout = os.Stdout
 		cmd.Stderr = os.Stderr
+		cmd.Env = append(os.Environ(), "DB_HOST=host", "DB_PORT=80", "DB_NAME=default", "DB_USER=myuser", "DB_PASSWORD=password")
 		assert.NoError(t, cmd.Run())
 	})
 }
diff --git a/internal/compose/convert_test.go b/internal/compose/convert_test.go
index effd671..4df240a 100644
--- a/internal/compose/convert_test.go
+++ b/internal/compose/convert_test.go
@@ -141,8 +141,8 @@ func TestScoreConvert(t *testing.T) {
 						Environment: compose.MappingWithEquals{
 							"DEBUG":             stringPtr("${DEBUG}"),
 							"LOGS_LEVEL":        stringPtr("${LOGS_LEVEL}"),
-							"DOMAIN_NAME":       stringPtr("${SOME_DNS_DOMAIN_NAME}"),
-							"CONNECTION_STRING": stringPtr("postgresql://${APP_DB_HOST}:${APP_DB_PORT}/${APP_DB_NAME}"),
+							"DOMAIN_NAME":       stringPtr("${SOME_DNS_DOMAIN_NAME?required}"),
+							"CONNECTION_STRING": stringPtr("postgresql://${APP_DB_HOST?required}:${APP_DB_PORT?required}/${APP_DB_NAME?required}"),
 						},
 						Volumes: []compose.ServiceVolumeConfig{
 							{
diff --git a/internal/compose/templates_test.go b/internal/compose/templates_test.go
index ae51044..3ed67cb 100644
--- a/internal/compose/templates_test.go
+++ b/internal/compose/templates_test.go
@@ -49,10 +49,10 @@ func TestMapVar(t *testing.T) {
 		{Input: "resources.env.DEBUG", Expected: "${DEBUG}"},
 		{Input: "resources.missing", ExpectedError: "invalid ref 'resources.missing': no known resource 'missing'"},
 		{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}"},
-		{Input: "resources.db.name.user", Expected: "${DB_NAME_USER}"},
+		{Input: "resources.db.host", Expected: "${DB_HOST?required}"},
+		{Input: "resources.db.port", Expected: "${DB_PORT?required}"},
+		{Input: "resources.db.name", Expected: "${DB_NAME?required}"},
+		{Input: "resources.db.name.user", Expected: "${DB_NAME_USER?required}"},
 		{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"},
@@ -108,7 +108,7 @@ func TestSubstitute(t *testing.T) {
 		{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}",
+			Expected: "postgresql://${DB_USER?required}:${DB_PASSWORD?required}@${DB_HOST?required}:${DB_PORT?required}/${DB_NAME?required}",
 		},
 	} {
 		t.Run(tc.Input, func(t *testing.T) {