Repository: score-spec/score-compose (go)

Issue:
[feature request] support access to `${metadata.annotations.key\.com/key}` annotation placeholders in the substituion routines
We allow `${metadata...} placeholders to access keys and values under the metadata section. While we support escaping .'s for the `key\.com` part, we don't support the forward slash in the supported substitution key format. We should fix that :)



Pull request:
feat: support /-separated annotation keys in metadata reference
This PR expands the set of supported characters inside a `${..}` placeholder and makes the templating system more predictable and reliable.

In the past a placeholder like `${x y z}` would just be returned as `${x y z}` without any error or modification. Now we will steer users towards escaping it instead if this is the desired behavior: `$${x y z}`.

As a side effect this fixes #91 as it now allows the forward slash inside the placeholder.

Patch:
diff --git a/examples/02-environment/README.md b/examples/02-environment/README.md
index cc9a88c..85064c5 100644
--- a/examples/02-environment/README.md
+++ b/examples/02-environment/README.md
@@ -2,7 +2,9 @@
 
 When `docker-compose` spins-up the service, it is possible to pass some information from the host to the container via the environment variables. These variables can then be accessed by programs running in the container.
 
-The `containers.*.variables` section supports placeholder interpolation using `${..}` syntax. This can be used to access outputs from metadata or resources. The `environment` resource to collect them from the current shell:
+The `containers.*.variables` section supports placeholder interpolation using `${..}` syntax. This can be used to access outputs from metadata or resources. The placeholder syntax can be escaped with a double-$ like `$${..}`.
+
+The `environment` resource to collect them from the current shell:
 
 ```yaml
 apiVersion: score.dev/v1b1
diff --git a/examples/06-resource-provisioning/README.md b/examples/06-resource-provisioning/README.md
index 87f8ece..7d36aab 100644
--- a/examples/06-resource-provisioning/README.md
+++ b/examples/06-resource-provisioning/README.md
@@ -28,26 +28,43 @@ resources:
 In some stateful cases, you may need some resources to be shared. This can be done by adding `id: specific-id` to the resource definition. This is unique to the project and shared across workloads. For example, below we share the same cache across both workloads, while an extra independent cache is added to the second workload.
 
 ```yaml
+apiVersion: score.dev/v1b1
 metadata:
   name: workload-one
-...
+containers:
+  example:
+    image: busybox
+    command: ["/bin/sh"]
+    args: ["-c", "while true; do echo $${CONNECTION}; sleep 5; done"]
+    variables:
+      CONNECTION: "redis://${resources.cache-a.username}:${resources.cache-a.password}@${resources.cache-a.host}:${resources.cache-a.port}"
 resources:
-  cacheA:
+  cache-a:
     type: redis
     id: main-cache
     
 ---
+apiVersion: score.dev/v1b1
 metadata:
   name: workload-two
-...
+containers:
+  example:
+    image: busybox
+    command: ["/bin/sh"]
+    args: ["-c", "while true; do echo $${CONNECTION_A}; echo $${CONNECTION_B}; sleep 5; done"]
+    variables:
+      CONNECTION_A: "redis://${resources.cache-b.username}:${resources.cache-b.password}@${resources.cache-b.host}:${resources.cache-b.port}"
+      CONNECTION_B: "redis://${resources.cache-c.username}:${resources.cache-c.password}@${resources.cache-c.host}:${resources.cache-c.port}"
 resources:
-  cacheB:
+  cache-b:
     type: redis
     id: main-cache
-  cacheC:
+  cache-c:
     type: redis
 ```
 
+Notice how we are using the placeholder syntax to access outputs from the resources and pass these through to the workloads. The syntax uses a `.`-separated path to traverse the resource outputs. The `.` can be escaped inside the placeholder with a backslash, for example: `${resources.cache-a.some\.key}`.
+
 When we provision this with `score-compose` we get an output that shows 2 redis services being created and both workloads have access to the connection strings. This supports applications that need to share data within the same caches, databases, or other stateful resources.
 
 ```console
diff --git a/internal/project/substitution.go b/internal/project/substitution.go
index 47bc43a..538bb8a 100644
--- a/internal/project/substitution.go
+++ b/internal/project/substitution.go
@@ -23,7 +23,8 @@ import (
 )
 
 var (
-	placeholderRegEx = regexp.MustCompile(`\$(\$|{([a-zA-Z0-9.\-_\\]+)})`)
+	// placeholderRegEx will search for ${...} with any sequence of characters between them.
+	placeholderRegEx = regexp.MustCompile(`\$(\$|{([^}]*)})`)
 )
 
 func SplitRefParts(ref string) []string {
@@ -50,11 +51,9 @@ func SubstituteString(src string, inner func(string) (string, error)) (string, e
 			return src
 		}
 
-		// EDGE CASE: Captures "$$" sequences and empty templates "${}"
-		if matches[2] == "" {
+		// support escaped dollars
+		if matches[1] == "$" {
 			return matches[1]
-		} else if matches[2] == "$" {
-			return matches[2]
 		}
 
 		result, subErr := inner(matches[2])
@@ -146,7 +145,7 @@ func BuildSubstitutionFunction(metadata map[string]interface{}, resources map[st
 				resolvedValue = rv2
 			}
 		default:
-			return "", fmt.Errorf("invalid ref '%s': unknown reference root", ref)
+			return "", fmt.Errorf("invalid ref '%s': unknown reference root, use $$ to escape the substitution", ref)
 		}
 
 		if asString, ok := resolvedValue.(string); ok {


Test patch:
diff --git a/internal/command/generate_test.go b/internal/command/generate_test.go
index 5d456ee..78f2168 100644
--- a/internal/command/generate_test.go
+++ b/internal/command/generate_test.go
@@ -725,3 +725,50 @@ resources:
 	})
 
 }
+
+func TestInitAndGenerate_with_annotation_ref(t *testing.T) {
+	td := changeToTempDir(t)
+	stdout, _, err := executeAndResetCommand(context.Background(), rootCmd, []string{"init"})
+	assert.NoError(t, err)
+	assert.Equal(t, "", stdout)
+
+	assert.NoError(t, os.WriteFile("score.yaml", []byte(`
+apiVersion: score.dev/v1b1
+metadata:
+  name: example
+  annotations:
+    key.com/foo-bar: thing
+containers:
+  example:
+    image: foo
+    variables:
+      REF: ${metadata.annotations.key\.com/foo-bar}
+`), 0644))
+	// generate
+	stdout, _, err = executeAndResetCommand(context.Background(), rootCmd, []string{"generate", "score.yaml"})
+	assert.NoError(t, err)
+	assert.Equal(t, "", stdout)
+	raw, err := os.ReadFile(filepath.Join(td, "compose.yaml"))
+	assert.NoError(t, err)
+	assert.Equal(t, `name: "001"
+services:
+    example-example:
+        environment:
+            REF: thing
+        image: foo
+`, string(raw))
+
+	t.Run("validate compose spec", func(t *testing.T) {
+		if os.Getenv("NO_DOCKER") != "" {
+			t.Skip("NO_DOCKER is set")
+			return
+		}
+		dockerCmd, err := exec.LookPath("docker")
+		require.NoError(t, err)
+		cmd := exec.Command(dockerCmd, "compose", "-f", "compose.yaml", "convert", "--quiet", "--dry-run")
+		cmd.Dir = td
+		cmd.Stdout = os.Stdout
+		cmd.Stderr = os.Stderr
+		assert.NoError(t, cmd.Run())
+	})
+}
diff --git a/internal/project/substitution_test.go b/internal/project/substitution_test.go
index 40bfa17..ebd6c11 100644
--- a/internal/project/substitution_test.go
+++ b/internal/project/substitution_test.go
@@ -29,6 +29,9 @@ func init() {
 	substitutionFunction = BuildSubstitutionFunction(score.WorkloadMetadata{
 		"name":  "test-name",
 		"other": map[string]interface{}{"key": "value"},
+		"annotations": map[string]interface{}{
+			"key.com/foo-bar": "thing",
+		},
 	}, map[string]OutputLookupFunc{
 		"env": func(keys ...string) (interface{}, error) {
 			if len(keys) != 1 {
@@ -52,13 +55,14 @@ func TestSubstitutionFunction(t *testing.T) {
 		Expected      string
 		ExpectedError string
 	}{
-		{Input: "missing", ExpectedError: "invalid ref 'missing': unknown reference root"},
+		{Input: "missing", ExpectedError: "invalid ref 'missing': unknown reference root, use $$ to escape the substitution"},
 		{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': key 'missing' not found"},
 		{Input: "metadata.name.foo", ExpectedError: "invalid ref 'metadata.name.foo': cannot lookup key 'foo', context is not a map"},
+		{Input: "metadata.annotations.key\\.com/foo-bar", Expected: "thing"},
 		{Input: "resources.env", Expected: "env"},
 		{Input: "resources.env.DEBUG", Expected: "${DEBUG}"},
 		{Input: "resources.missing", ExpectedError: "invalid ref 'resources.missing': no known resource 'missing'"},
@@ -94,8 +98,10 @@ func TestSubstituteString(t *testing.T) {
 		{Input: "$abc", Expected: "$abc"},
 		{Input: "abc $$ abc", Expected: "abc $ abc"},
 		{Input: "$${abc}", Expected: "${abc}"},
+		{Input: "$${abc .4t3298y *(^&(*}", Expected: "${abc .4t3298y *(^&(*}"},
 		{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': key 'thing.two' not found"},
+		{Input: "my name is ${}", ExpectedError: "invalid ref '': unknown reference root, use $$ to escape the substitution"},
 		{
 			Input:    "postgresql://${resources.db.user}:${resources.db.password}@${resources.db.host}:${resources.db.port}/${resources.db.name}",
 			Expected: "postgresql://${DB_USER?required}:${DB_PASSWORD?required}@${DB_HOST?required}:${DB_PORT?required}/${DB_NAME?required}",