Repository: score-spec/score-compose
Pull request #122 (2024-04-23 08:43:29)

fix: assert volume based on outputs and not type
This PR changes the way that volumes are provisioned in score-compose. Previously volumes HAD TO be type=volume and return a source value, and were limited to just being persistent docker compose volumes.

This PR now removes the restriction on the type field and just requires that the outputs must match one of the 3 kinds of supported docker compose volume mounts: tmpfs, bind, or persistent volume - and we also support the extended attributes https://docs.docker.com/compose/compose-file/compose-file-v3/#volumes

This also modifies the way the 2-part resource-id placeholder is resolved. `${resources.foo}` will not resolve to `type.class#workload.foo` (the resource uid).


Patch:
diff --git a/README.md b/README.md
index 2885f8f..e71b909 100644
--- a/README.md
+++ b/README.md
@@ -38,6 +38,8 @@ The `score.yaml` specification file can be executed against a _Score Implementat
 | route        | default | `host`, `path`, `port` |                                                                                                                                                                 |
 | amqp         | default | (none)                 | `host`, `port`, `vhost`, `username`, `password`                                                                                                                 |
 
+These can be found in the default provisioners file. You are encouraged to write your own provisioners and add them to the `.score-compose` directory (with the `.provisioners.yaml` extension) or contribute them upstream to the [default.provisioners.yaml](internal/command/default.provisioners.yaml) file.
+
 ## ![Installation](docs/images/install.svg) Installation
 
 To install `score-compose`, follow the instructions as described in our [installation guide](https://docs.score.dev/docs/get-started/install/).
diff --git a/examples/05-volume-mounts/README.md b/examples/05-volume-mounts/README.md
index a8f1d43..7d96445 100644
--- a/examples/05-volume-mounts/README.md
+++ b/examples/05-volume-mounts/README.md
@@ -1,14 +1,10 @@
 # 05 - Volume Mounts
 
-Along with `environment`, `volume` is one of the more common resource types supported by score-compose:
+Many Score projects require volume storage. Volume storage may be a tmpfs, a host bind mount, or a persistent and stateful volume. All three can be represented in score-compose depending on the provisioner.
 
-```yaml
-resources:
-  data:
-    type: volume
-```
+A default provisioner exists for a generic `volume` type which provisions a Docker volume and mounts it to the container.
 
-This provisions an empty Docker volume with a random name. This can be mounted into the container volumes section:
+As an example, the Score file may contain:
 
 ```yaml
 containers:
@@ -16,21 +12,49 @@ containers:
     volumes:
       - source: ${resources.data}
         target: /data
+resources:
+  data:
+    type: volume
+```
+
+The `${resources.data}` placeholder resolves to "volume.default#<workload>.data" (the uid of the resource) which is converted into outputs, and then into a Docker compose volume.
+
+The outputs of the volume resource look like:
+
+```yaml
+type: volume
+source: <generated docker volume name>
+```
+
+Since this provisions a stateful Docker volume, the state is persisted across restarts and may also be shared between containers.
+
+## Writing a tmpfs provisioner
+
+If the container does not need state to persist across restarts, we can use `tmpfs` volume. This isn't available as a default provisioner but can be easily added to a `.score-compose/0-custom-provisioners.yaml` file:
+
+```yaml
+- uri: template://custom-empty-dir-volume
+  type: emptyDir
+  outputs: |
+    type: tmpfs
+    tmpfs:
+      size: 10000000
 ```
 
-**NOTE**: the `${resources.data}` placeholder resolves to "data" (the name of the resource) and is linked as the source of the volume.
-
-The volumes are persisted across container starts and stops and can be used for supporting stateful applications.
-
-```console
-$ score-compose init
-$ score-compose generate score.yaml
-...
-$ docker compose up
-[+] Running 3/3
- ✔ Network 05-volume-mounts_default                Created                                                                                                                                                                                                0.1s
- ✔ Volume "hello-world-data-iIlzJ5"                Created                                                                                                                                                                                                0.0s
- ✔ Container 05-volume-mounts-hello-world-first-1  Created                                                                                                                                                                                                0.1s
-Attaching to hello-world-first-1
-...
+This kind of volume cannot be shared across containers.
+
+## Writing a bind provisioner
+
+In some cases you may need to bind mount a volume or file from the host. This could be written as a custom provisioner too!
+
+```yaml
+- uri: template://custom-bind-mount
+  type: host-logs
+  outputs: |
+    type: bind
+    source: /var/log
+    bind:
+      propagation: rprivate
 ```
+
+Note that this would usually have a custom type since the typed-provisioner defines what source directory from the host is mounted.
diff --git a/internal/command/default.provisioners.yaml b/internal/command/default.provisioners.yaml
index ba65459..6d45ae7 100644
--- a/internal/command/default.provisioners.yaml
+++ b/internal/command/default.provisioners.yaml
@@ -26,6 +26,7 @@
     name: {{ dig "name" .Init.randomVolumeName .State }}
   # Return a source value with the volume name. This can be used in volume resource references now.
   outputs: |
+    type: volume
     source: {{ .State.name }}
   # Add a volume to the docker compose file. We assume our name is unique here. We also apply a label to help ensure
   # that we can track the volume back to the workload and resource that created it.
diff --git a/internal/command/init.go b/internal/command/init.go
index 1108794..de730c4 100644
--- a/internal/command/init.go
+++ b/internal/command/init.go
@@ -151,10 +151,20 @@ acts as a namespace when multiple score files and containers are used.
 				return fmt.Errorf("failed to persist new compose project name: %w", err)
 			}
 
+			// create and write the default provisioners file if it doesn't already exist
 			dst := "99-default" + loader.DefaultSuffix
-			slog.Info(fmt.Sprintf("Writing default provisioners yaml file '%s'", dst))
-			if err := os.WriteFile(filepath.Join(sd.Path, dst), []byte(defaultProvisionersContent), 0644); err != nil {
-				return fmt.Errorf("failed to write provisioners: %w", err)
+			if f, err := os.OpenFile(filepath.Join(sd.Path, dst), os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0644); err != nil {
+				if !errors.Is(err, os.ErrExist) {
+					return fmt.Errorf("failed to open default provisioners for writing: %w", err)
+				}
+				slog.Info(fmt.Sprintf("Default provisioners yaml file '%s' already exists, not overwriting it", dst))
+			} else {
+				defer f.Close()
+				slog.Info(fmt.Sprintf("Writing default provisioners yaml file '%s'", dst))
+				if _, err = f.WriteString(defaultProvisionersContent); err != nil {
+					return fmt.Errorf("failed to write provisioners: %w", err)
+				}
+				_ = f.Close()
 			}
 		}
 
diff --git a/internal/compose/convert.go b/internal/compose/convert.go
index d04c062..8958e7a 100644
--- a/internal/compose/convert.go
+++ b/internal/compose/convert.go
@@ -15,6 +15,8 @@
 package compose
 
 import (
+	"bytes"
+	"encoding/json"
 	"errors"
 	"fmt"
 	"log/slog"
@@ -27,6 +29,7 @@ import (
 	compose "github.com/compose-spec/compose-go/v2/types"
 	"github.com/score-spec/score-go/framework"
 	score "github.com/score-spec/score-go/types"
+	"gopkg.in/yaml.v3"
 
 	"github.com/score-spec/score-compose/internal/project"
 	"github.com/score-spec/score-compose/internal/util"
@@ -86,31 +89,11 @@ func ConvertSpec(state *project.State, spec *score.Workload) (*compose.Project,
 					return nil, fmt.Errorf("containers.%s.volumes[%d].path: can't mount named volume with sub path '%s': not supported", containerName, idx, *vol.Path)
 				}
 
-				// The way volumes are linked to a resource is a bit of a special case. The goal is to confirm that the
-				// resource exists and is a volume. We then extract the source property. This only applies to the pattern
-				// of ${resources.my-volume} which can also be written as ${resources.my-volume.source}.
-				resolvedVolumeSource, err := framework.SubstituteString(vol.Source, func(ref string) (string, error) {
-					if parts := framework.SplitRefParts(ref); len(parts) == 2 && parts[0] == "resources" {
-						resName := parts[1]
-						if res, ok := spec.Resources[resName]; !ok {
-							return "", fmt.Errorf("containers.%s.volumes[%d].source: resource '%s' does not exist", containerName, idx, resName)
-						} else if res.Type != "volume" {
-							return "", fmt.Errorf("containers.%s.volumes[%d].source: resource '%s' is not a volume", containerName, idx, resName)
-						}
-						ref += ".source"
-					}
-					return deferredSubstitutionFunction(ref)
-				})
+				cfg, err := convertVolumeSourceIntoVolume(state, deferredSubstitutionFunction, workloadName, vol)
 				if err != nil {
 					return nil, fmt.Errorf("containers.%s.volumes[%d].source: %w", containerName, idx, err)
 				}
-
-				volumes[idx] = compose.ServiceVolumeConfig{
-					Type:     "volume",
-					Source:   resolvedVolumeSource,
-					Target:   vol.Target,
-					ReadOnly: util.DerefOr(vol.ReadOnly, false),
-				}
+				volumes[idx] = *cfg
 			}
 		}
 
@@ -251,3 +234,80 @@ func convertFilesIntoVolumes(state *project.State, workloadName string, containe
 
 	return output, nil
 }
+
+func convertVolumeSourceIntoVolume(state *project.State, substitutionFunction func(string) (string, error), workloadName string, vol score.ContainerVolumesElem) (*compose.ServiceVolumeConfig, error) {
+	spec := state.Workloads[workloadName].Spec
+
+	// The way volumes are linked to a resource is a bit of a special case. The goal is to confirm that the
+	// resource exists and has the outputs that we need.
+	resolvedVolumeSource, err := framework.SubstituteString(vol.Source, func(ref string) (string, error) {
+		if parts := framework.SplitRefParts(ref); len(parts) == 2 && parts[0] == "resources" {
+			resName := parts[1]
+			if res, ok := spec.Resources[resName]; ok {
+				return string(framework.NewResourceUid(workloadName, resName, res.Type, res.Class, res.Id)), nil
+			}
+			return "", fmt.Errorf("resource '%s' does not exist", resName)
+		}
+		return substitutionFunction(ref)
+	})
+	if err != nil {
+		return nil, err
+	}
+
+	outputVolume := &compose.ServiceVolumeConfig{
+		Type:     "volume",
+		Source:   resolvedVolumeSource,
+		Target:   vol.Target,
+		ReadOnly: util.DerefOr(vol.ReadOnly, false),
+	}
+
+	// now if the resolves source is a volume we can check the outputs or throw an error
+	res, ok := state.Resources[framework.ResourceUid(resolvedVolumeSource)]
+	if ok {
+		volType, ok := res.Outputs["type"].(string)
+		if !ok {
+			return nil, fmt.Errorf("resource '%s' has no 'type' output", resolvedVolumeSource)
+		}
+		outputVolume.Type = volType
+		raw, _ := json.Marshal(res.Outputs)
+		dec := yaml.NewDecoder(bytes.NewReader(raw))
+		dec.KnownFields(true)
+		switch volType {
+		case "volume":
+			s := struct {
+				Type   string                       `json:"type"`
+				Source string                       `json:"source"`
+				Volume *compose.ServiceVolumeVolume `json:"volume"`
+			}{}
+			if err := dec.Decode(&s); err != nil {
+				return nil, fmt.Errorf("resource '%s' outputs cannot decode for volume: %w", resolvedVolumeSource, err)
+			}
+			outputVolume.Source = s.Source
+			outputVolume.Volume = s.Volume
+		case "tmpfs":
+			s := struct {
+				Type  string                      `json:"type"`
+				Tmpfs *compose.ServiceVolumeTmpfs `json:"tmpfs"`
+			}{}
+			if err := dec.Decode(&s); err != nil {
+				return nil, fmt.Errorf("resource '%s' outputs cannot decode for tmpfs: %w", resolvedVolumeSource, err)
+			}
+			outputVolume.Tmpfs = s.Tmpfs
+		case "bind":
+			s := struct {
+				Type   string                     `json:"type"`
+				Source string                     `json:"source"`
+				Bind   *compose.ServiceVolumeBind `json:"bind"`
+			}{}
+			if err := dec.Decode(&s); err != nil {
+				return nil, fmt.Errorf("resource '%s' outputs cannot decode for bind: %w", resolvedVolumeSource, err)
+			}
+			outputVolume.Source = s.Source
+			outputVolume.Bind = s.Bind
+		default:
+			return nil, fmt.Errorf("resource '%s' has invalid type '%s'", resolvedVolumeSource, volType)
+		}
+	}
+
+	return outputVolume, nil
+}


Test patch:
diff --git a/internal/command/generate_test.go b/internal/command/generate_test.go
index 6344acc..852033d 100644
--- a/internal/command/generate_test.go
+++ b/internal/command/generate_test.go
@@ -960,3 +960,71 @@ resources:
 		"environment variable 'UNKNOWN_SCORE_VARIABLE' must be resolved",
 	)
 }
+
+func TestInitAndGenerate_with_volume_types(t *testing.T) {
+	td := changeToTempDir(t)
+	stdout, _, err := executeAndResetCommand(context.Background(), rootCmd, []string{"init"})
+	assert.NoError(t, err)
+	assert.Equal(t, "", stdout)
+
+	// write custom providers
+	assert.NoError(t, os.WriteFile(filepath.Join(td, ".score-compose", "00-custom.provisioners.yaml"), []byte(`
+- uri: template://tmpfs-volume
+  type: tmp-volume
+  outputs: |
+    type: tmpfs
+    tmpfs:
+      size: 10000000
+- uri: template://bind-volume
+  type: bind-volume
+  outputs: |
+    type: bind
+    source: /dev/something
+    bind:
+      create_host_path: true
+`), 0644))
+
+	// write custom score file
+	assert.NoError(t, os.WriteFile(filepath.Join(td, "score.yaml"), []byte(`
+apiVersion: score.dev/v1b1
+metadata:
+  name: example
+containers:
+  example:
+    image: busybox
+    volumes:
+    - target: /mnt/v1
+      source: ${resources.v1}
+    - target: /mnt/v2
+      source: ${resources.v2}
+resources:
+  v1:
+    type: tmp-volume
+  v2:
+    type: bind-volume
+`), 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:
+        hostname: example
+        image: busybox
+        volumes:
+            - type: bind
+              source: /dev/something
+              target: /mnt/v2
+              bind:
+                create_host_path: true
+            - type: tmpfs
+              source: tmp-volume.default#example.v1
+              target: /mnt/v1
+              tmpfs:
+                size: "10000000"
+`, string(raw))
+}
diff --git a/internal/command/init_test.go b/internal/command/init_test.go
index cceb152..113bbb8 100644
--- a/internal/command/init_test.go
+++ b/internal/command/init_test.go
@@ -181,16 +181,28 @@ func TestInitNominal_run_twice(t *testing.T) {
 		require.NoError(t, os.Chdir(wd))
 	}()
 
+	// first init
 	stdout, stderr, err := executeAndResetCommand(context.Background(), rootCmd, []string{"init", "--file", "score2.yaml", "--project", "bananas"})
 	assert.NoError(t, err)
 	assert.Equal(t, "", stdout)
 	assert.NotEqual(t, "", strings.TrimSpace(stderr))
 
+	// check default provisioners exists and overwrite it with an empty array
+	dpf, err := os.Stat(filepath.Join(td, ".score-compose", "99-default.provisioners.yaml"))
+	assert.NoError(t, err)
+	assert.NoError(t, os.WriteFile(filepath.Join(td, ".score-compose", dpf.Name()), []byte("[]"), 0644))
+
+	// init again
 	stdout, stderr, err = executeAndResetCommand(context.Background(), rootCmd, []string{"init"})
 	assert.NoError(t, err)
 	assert.Equal(t, "", stdout)
 	assert.NotEqual(t, "", strings.TrimSpace(stderr))
 
+	// verify that default provisioners was not overwritten again
+	dpf, err = os.Stat(filepath.Join(td, ".score-compose", dpf.Name()))
+	assert.NoError(t, err)
+	assert.Equal(t, 2, int(dpf.Size()))
+
 	_, err = os.Stat("score.yaml")
 	assert.NoError(t, err)
 	_, err = os.Stat("score2.yaml")
diff --git a/internal/command/resources_test.go b/internal/command/resources_test.go
index f485b45..5a51cd8 100644
--- a/internal/command/resources_test.go
+++ b/internal/command/resources_test.go
@@ -123,6 +123,6 @@ volume.default#example.vol
 	t.Run("format template", func(t *testing.T) {
 		stdout, _, err := executeAndResetCommand(context.Background(), rootCmd, []string{"resources", "get-outputs", "volume.default#example.vol", "--format", `{{ . | len }}`})
 		assert.NoError(t, err)
-		assert.Equal(t, "1", stdout)
+		assert.Equal(t, "2", stdout)
 	})
 }
diff --git a/internal/compose/convert_test.go b/internal/compose/convert_test.go
index bb5097e..c18bc3f 100644
--- a/internal/compose/convert_test.go
+++ b/internal/compose/convert_test.go
@@ -274,7 +274,7 @@ func TestScoreConvert(t *testing.T) {
 				},
 				Resources: map[string]score.Resource{"data": {Type: "thing"}},
 			},
-			Error: errors.New("containers.test.volumes[0].source: resource 'data' is not a volume"),
+			Error: errors.New("containers.test.volumes[0].source: resource 'data' does not exist"),
 		},
 	}