Repository: score-spec/score-compose
Pull request #55 (2024-03-04 13:55:03)

fix: reject unsupported features and log ignored ones
Users face a log of confusion when features in the score file are ignored without any notice or error.

In this PR we do things to improve this:

1. Update the logger to the modern log/slog package which allows log levels to be tracked. We use a simple handler here that ignores structured fields and just writes out the message with the level. We don't need timestamps here since the execution of the problem is fast and deterministic at this time.
2. Reject the files element which isn't supported yet.
3. Log warnings for unsupported resources when falling back to the environment variable references.
4. Log warnings for the ignored spec sections: probes and resources.

For example we get the following output out of the big RunExample test:

```
$ ./score-compose run -f ../schema/samples/score-full.yaml
WARN: resources.resource-two2: 'Resource-Two.default' is not directly supported in score-compose, references will be converted to environment variables
WARN: resources.resource-one1: 'Resource-One.default' is not directly supported in score-compose, references will be converted to environment variables
Error: building docker-compose configuration: containers.container-one1.volumes[0].path: can't mount named volume with sub path '/sub/path': not supported
```

And fixing any issues while running with verbose on:

```
$ ./score-compose run -f ../schema/samples/score-full.yaml  --verbose
INFO: Reading Score file '../schema/samples/score-full.yaml'
INFO: Parsing Score specification
INFO: Loading Score overrides file './overrides.score.yaml'
INFO: Validating final Score specification
INFO: Building docker-compose configuration
WARN: resources.resource-one1: 'Resource-One.default' is not directly supported in score-compose, references will be converted to environment variables
WARN: resources.resource-two2: 'Resource-Two.default' is not directly supported in score-compose, references will be converted to environment variables
WARN: containers.container-one1.resources.requests: not supported - ignoring
WARN: containers.container-one1.resources.limits: not supported - ignoring
WARN: containers.container-one1.readinessProbe: not supported - ignoring
WARN: containers.container-one1.livenessProbe: not supported - ignoring
...
```

#### 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:
- [ ] 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/compose.yaml b/compose.yaml
index 8e31e23..a0fa5c1 100644
--- a/compose.yaml
+++ b/compose.yaml
@@ -10,10 +10,8 @@ services:
         published: "80"
     environment:
       FOO: bar
-  my-project-worker:
-    command:
-      - /bin/sh
-      - -c
-      - sleep 2600
-    image: debian
-    network_mode: service:my-project-api
+
+    deploy:
+      resources:
+        limits:
+          cpus: '0.5M'
\ No newline at end of file
diff --git a/internal/command/run.go b/internal/command/run.go
index fe02055..6500999 100644
--- a/internal/command/run.go
+++ b/internal/command/run.go
@@ -11,7 +11,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"io"
-	"log"
+	"log/slog"
 	"os"
 	"sort"
 	"strings"
@@ -23,6 +23,7 @@ import (
 	"gopkg.in/yaml.v3"
 
 	"github.com/score-spec/score-compose/internal/compose"
+	"github.com/score-spec/score-compose/internal/logging"
 
 	loader "github.com/score-spec/score-go/loader"
 	schema "github.com/score-spec/score-go/schema"
@@ -74,13 +75,15 @@ func run(cmd *cobra.Command, args []string) error {
 	// Silence usage message if args are parsed correctly
 	cmd.SilenceUsage = true
 
-	if !verbose {
-		log.SetOutput(io.Discard)
+	logLevel := slog.LevelWarn
+	if verbose {
+		logLevel = slog.LevelDebug
 	}
+	slog.SetDefault(slog.New(&logging.SimpleHandler{Level: logLevel, Writer: cmd.ErrOrStderr()}))
 
 	// Open source file
 	//
-	log.Printf("Reading '%s'...\n", scoreFile)
+	slog.Info(fmt.Sprintf("Reading Score file '%s'", scoreFile))
 	var err error
 	var src *os.File
 	if src, err = os.Open(scoreFile); err != nil {
@@ -90,7 +93,7 @@ func run(cmd *cobra.Command, args []string) error {
 
 	// Parse SCORE spec
 	//
-	log.Print("Parsing SCORE spec...\n")
+	slog.Info("Parsing Score specification")
 	var srcMap map[string]interface{}
 	if err = loader.ParseYAML(&srcMap, src); err != nil {
 		return err
@@ -99,15 +102,15 @@ func run(cmd *cobra.Command, args []string) error {
 	// Apply overrides from file (optional)
 	//
 	if overridesFile != "" {
-		log.Printf("Checking '%s'...\n", overridesFile)
+		slog.Info(fmt.Sprintf("Loading Score overrides file '%s'", overridesFile))
 		if ovr, err := os.Open(overridesFile); err == nil {
 			defer ovr.Close()
 
-			log.Print("Applying SCORE overrides...\n")
 			var ovrMap map[string]interface{}
 			if err = loader.ParseYAML(&ovrMap, ovr); err != nil {
 				return err
 			}
+			slog.Info("Applying Score overrides")
 			if err := mergo.MergeWithOverwrite(&srcMap, ovrMap); err != nil {
 				return fmt.Errorf("applying overrides fom '%s': %w", overridesFile, err)
 			}
@@ -119,8 +122,6 @@ func run(cmd *cobra.Command, args []string) error {
 	// Apply overrides from command line (optional)
 	//
 	for _, pstr := range overrideParams {
-		log.Print("Applying SCORE properties overrides...\n")
-
 		jsonBytes, err := json.Marshal(srcMap)
 		if err != nil {
 			return fmt.Errorf("marshalling score spec: %w", err)
@@ -129,7 +130,7 @@ func run(cmd *cobra.Command, args []string) error {
 		pmap := strings.SplitN(pstr, "=", 2)
 		if len(pmap) <= 1 {
 			var path = pmap[0]
-			log.Printf("removing '%s'", path)
+			slog.Info(fmt.Sprintf("Applying Score properties override: removing '%s'", path))
 			if jsonBytes, err = sjson.DeleteBytes(jsonBytes, path); err != nil {
 				return fmt.Errorf("removing '%s': %w", path, err)
 			}
@@ -140,7 +141,7 @@ func run(cmd *cobra.Command, args []string) error {
 				val = pmap[1]
 			}
 
-			log.Printf("overriding '%s' = '%s' (%T)", path, val, val)
+			slog.Info(fmt.Sprintf("Applying Score properties override: overriding '%s' = '%s' (%T)", path, val, val))
 			if jsonBytes, err = sjson.SetBytes(jsonBytes, path, val); err != nil {
 				return fmt.Errorf("overriding '%s': %w", path, err)
 			}
@@ -156,14 +157,14 @@ func run(cmd *cobra.Command, args []string) error {
 		return fmt.Errorf("failed to upgrade spec: %w", err)
 	} else if len(changes) > 0 {
 		for _, change := range changes {
-			log.Printf("Applying upgrade to specification: %s\n", change)
+			slog.Info(fmt.Sprintf("Applying upgrade to specification: %s", change))
 		}
 	}
 
 	// Validate SCORE spec
 	//
 	if !skipValidation {
-		log.Print("Validating SCORE spec...\n")
+		slog.Info("Validating final Score specification")
 		if err := schema.Validate(srcMap); err != nil {
 			return fmt.Errorf("validating workload spec: %w", err)
 		}
@@ -178,7 +179,7 @@ func run(cmd *cobra.Command, args []string) error {
 
 	// Build docker-compose configuration
 	//
-	log.Print("Building docker-compose configuration...\n")
+	slog.Info("Building docker-compose configuration")
 	proj, vars, err := compose.ConvertSpec(&spec)
 	if err != nil {
 		return fmt.Errorf("building docker-compose configuration: %w", err)
@@ -187,7 +188,7 @@ func run(cmd *cobra.Command, args []string) error {
 	// Override 'image' reference with 'build' instructions
 	//
 	if buildCtx != "" {
-		log.Printf("Applying build instructions: '%s'...\n", buildCtx)
+		slog.Info(fmt.Sprintf("Applying build context '%s' for service images", buildCtx))
 		// We add the build context to all services and containers here and make a big assumption that all are
 		// using the image we are building here and now. If this is unexpected, users should use a more complex
 		// overrides file.
@@ -201,7 +202,7 @@ func run(cmd *cobra.Command, args []string) error {
 	//
 	var dest = cmd.OutOrStdout()
 	if outFile != "" {
-		log.Printf("Creating '%s'...\n", outFile)
+		slog.Info(fmt.Sprintf("Writing output compose file '%s'", outFile))
 		destFile, err := os.Create(outFile)
 		if err != nil {
 			return err
@@ -213,7 +214,6 @@ func run(cmd *cobra.Command, args []string) error {
 
 	// Write docker-compose spec
 	//
-	log.Print("Writing docker-compose configuration...\n")
 	if err = compose.WriteYAML(dest, proj); err != nil {
 		return err
 	}
@@ -221,7 +221,7 @@ func run(cmd *cobra.Command, args []string) error {
 	if envFile != "" {
 		// Open .env file
 		//
-		log.Printf("Creating '%s'...\n", envFile)
+		slog.Info(fmt.Sprintf("Writing output .env file '%s'", envFile))
 		dest, err := os.Create(envFile)
 		if err != nil {
 			return err
@@ -230,8 +230,6 @@ func run(cmd *cobra.Command, args []string) error {
 
 		// Write .env file
 		//
-		log.Print("Writing .env file template...\n")
-
 		envVars := make([]string, 0)
 		for key, val := range vars.Accessed() {
 			var envVar = fmt.Sprintf("%s=%v\n", key, val)
diff --git a/internal/compose/convert.go b/internal/compose/convert.go
index cb9cf31..49395ad 100644
--- a/internal/compose/convert.go
+++ b/internal/compose/convert.go
@@ -10,6 +10,7 @@ package compose
 import (
 	"errors"
 	"fmt"
+	"log/slog"
 	"sort"
 	"strings"
 
@@ -38,14 +39,16 @@ func ConvertSpec(spec *score.Workload) (*compose.Project, *EnvVarTracker, error)
 	// 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 {
+		resClass := DerefOr(resourceSpec.Class, "default")
 		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)
+			if resClass != "default" {
+				return nil, nil, fmt.Errorf("resources.%s: '%s.%s' is not supported in score-compose", resourceName, resourceSpec.Type, resClass)
 			}
 			resources[resourceName] = envVarTracker
-		} else if resourceSpec.Type == "volume" && DerefOr(resourceSpec.Class, "default") == "default" {
+		} else if resourceSpec.Type == "volume" && resClass == "default" {
 			resources[resourceName] = resourceWithStaticOutputs{}
 		} else {
+			slog.Warn(fmt.Sprintf("resources.%s: '%s.%s' is not directly supported in score-compose, references will be converted to environment variables", resourceName, resourceSpec.Type, resClass))
 			// 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)
 		}
@@ -105,13 +108,15 @@ func ConvertSpec(spec *score.Workload) (*compose.Project, *EnvVarTracker, error)
 			volumes = make([]compose.ServiceVolumeConfig, len(cSpec.Volumes))
 			for idx, vol := range cSpec.Volumes {
 				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"))
+					return nil, nil, fmt.Errorf("containers.%s.volumes[%d].path: can't mount named volume with sub path '%s': not supported", containerName, idx, *vol.Path)
 				}
 
 				// 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)
+				} else if resolvedVolumeSource != vol.Source {
+					slog.Warn(fmt.Sprintf("containers.%s.volumes[%d].source: interpolation will be deprecated in the future", containerName, idx))
 				}
 
 				if res, ok := spec.Resources[resolvedVolumeSource]; !ok {
@@ -134,6 +139,29 @@ func ConvertSpec(spec *score.Workload) (*compose.Project, *EnvVarTracker, error)
 		})
 		// END (NOTE)
 
+		// Files are not supported just yet
+		if len(cSpec.Files) > 0 {
+			return nil, nil, fmt.Errorf("containers.%s.files: not supported", containerName)
+		}
+
+		// Docker compose without swarm/stack mode doesn't really support resource limits. There are optimistic
+		// workarounds but they vary between specific versions of the CLI. Better to just ignore.
+		if cSpec.Resources != nil {
+			if cSpec.Resources.Requests != nil {
+				slog.Warn(fmt.Sprintf("containers.%s.resources.requests: not supported - ignoring", containerName))
+			}
+			if cSpec.Resources.Limits != nil {
+				slog.Warn(fmt.Sprintf("containers.%s.resources.limits: not supported - ignoring", containerName))
+			}
+		}
+
+		if cSpec.ReadinessProbe != nil {
+			slog.Warn(fmt.Sprintf("containers.%s.readinessProbe: not supported - ignoring", containerName))
+		}
+		if cSpec.LivenessProbe != nil {
+			slog.Warn(fmt.Sprintf("containers.%s.livenessProbe: not supported - ignoring", containerName))
+		}
+
 		var svc = compose.ServiceConfig{
 			Name:        workloadName + "-" + containerName,
 			Image:       cSpec.Image,
diff --git a/internal/logging/logging.go b/internal/logging/logging.go
new file mode 100644
index 0000000..b70e55f
--- /dev/null
+++ b/internal/logging/logging.go
@@ -0,0 +1,39 @@
+package logging
+
+import (
+	"context"
+	"fmt"
+	"io"
+	"log/slog"
+	"sync"
+)
+
+type SimpleHandler struct {
+	Writer io.Writer
+	Level  slog.Leveler
+
+	mu sync.Mutex
+}
+
+func (h *SimpleHandler) Enabled(ctx context.Context, level slog.Level) bool {
+	return level >= h.Level.Level()
+}
+
+func (h *SimpleHandler) Handle(ctx context.Context, record slog.Record) error {
+	h.mu.Lock()
+	defer h.mu.Unlock()
+	_, err := h.Writer.Write([]byte(fmt.Sprintf("%s: %s\n", record.Level.String(), record.Message)))
+	return err
+}
+
+func (h *SimpleHandler) WithAttrs(attrs []slog.Attr) slog.Handler {
+	// no support for attrs here
+	return h
+}
+
+func (h *SimpleHandler) WithGroup(name string) slog.Handler {
+	// no support for attrs here
+	return h
+}
+
+var _ slog.Handler = (*SimpleHandler)(nil)


Test patch:
diff --git a/internal/command/run_test.go b/internal/command/run_test.go
index 4132aeb..056f055 100644
--- a/internal/command/run_test.go
+++ b/internal/command/run_test.go
@@ -146,7 +146,17 @@ resources:
 	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)
 	assert.NotEqual(t, "", stdout)
-	assert.Equal(t, "", stderr)
+	for _, l := range []string{
+		"WARN: resources.resource-one1: 'Resource-One.default' is not directly supported in score-compose, references will be converted to environment variables\n",
+		"WARN: resources.resource-two2: 'Resource-Two.default' is not directly supported in score-compose, references will be converted to environment variables\n",
+		"WARN: containers.container-one1.resources.requests: not supported - ignoring\n",
+		"WARN: containers.container-one1.resources.limits: not supported - ignoring\n",
+		"WARN: containers.container-one1.readinessProbe: not supported - ignoring\n",
+		"WARN: containers.container-one1.livenessProbe: not supported - ignoring\n",
+	} {
+		assert.Contains(t, stderr, l)
+	}
+
 	rawComposeContent, err := os.ReadFile(filepath.Join(td, "compose.yaml"))
 	require.NoError(t, err)
 	var actualComposeContent map[string]interface{}
@@ -227,7 +237,26 @@ containers:
       path: /sub/path
 `), 0600))
 	stdout, stderr, err := executeAndResetCommand(context.Background(), rootCmd, []string{"run", "--file", filepath.Join(td, "score.yaml"), "--output", filepath.Join(td, "compose.yaml")})
-	assert.EqualError(t, err, "building docker-compose configuration: can't mount named volume with sub path '/sub/path': not supported")
+	assert.EqualError(t, err, "building docker-compose configuration: containers.container-one1.volumes[0].path: can't mount named volume with sub path '/sub/path': not supported")
+	assert.Equal(t, "", stdout)
+	assert.Equal(t, "", stderr)
+}
+
+func TestFilesNotSupported(t *testing.T) {
+	td := t.TempDir()
+	assert.NoError(t, os.WriteFile(filepath.Join(td, "score.yaml"), []byte(`
+apiVersion: score.dev/v1b1
+metadata:
+  name: example-workload-name123
+containers:
+  container-one1:
+    image: localhost:4000/repo/my-image:tag
+    files:
+    - target: /mnt/something
+      content: bananas
+`), 0600))
+	stdout, stderr, err := executeAndResetCommand(context.Background(), rootCmd, []string{"run", "--file", filepath.Join(td, "score.yaml"), "--output", filepath.Join(td, "compose.yaml")})
+	assert.EqualError(t, err, "building docker-compose configuration: containers.container-one1.files: not supported")
 	assert.Equal(t, "", stdout)
 	assert.Equal(t, "", stderr)
 }
@@ -347,7 +376,12 @@ func TestRunExample03(t *testing.T) {
 `
 
 		assert.Equal(t, expectedOutput, stdout)
-		assert.Equal(t, "", stderr)
+		for _, l := range []string{
+			"WARN: resources.db: 'postgres.default' is not directly supported in score-compose, references will be converted to environment variables\n",
+			"WARN: resources.service-b: 'service.default' is not directly supported in score-compose, references will be converted to environment variables\n",
+		} {
+			assert.Contains(t, stderr, l)
+		}
 		rawComposeContent, err := os.ReadFile(filepath.Join(td, "compose-a.yaml"))
 		require.NoError(t, err)
 		assert.Equal(t, expectedOutput, string(rawComposeContent))