APP-179 Simplify templating of docker app (#602)

* parameter substitution using string replace instead of compose functions

Signed-off-by: Anca Iordache <anca.iordache@docker.com>
Co-authored-by: Silvin Lubecki <silvin.lubecki@gmail.com>

Author: aiordache

internal/packager/init.go

@@ -8,6 +8,7 @@ import (
 	"os"
 	"os/user"
 	"path/filepath"
+	"regexp"
 	"strings"
 	"text/template"
 
@@ -163,7 +164,6 @@ func initFromComposeFile(name string, composeFile string) error {
 			}
 		}
 	}
-
 	expandedParams, err := parameters.FromFlatten(params)
 	if err != nil {
 		return errors.Wrap(err, "failed to expand parameters")
@@ -172,6 +172,8 @@ func initFromComposeFile(name string, composeFile string) error {
 	if err != nil {
 		return errors.Wrap(err, "failed to marshal parameters")
 	}
+	// remove parameter default values from compose before saving
+	composeRaw = removeDefaultValuesFromCompose(composeRaw)
 	err = ioutil.WriteFile(filepath.Join(dirName, internal.ComposeFileName), composeRaw, 0644)
 	if err != nil {
 		return errors.Wrap(err, "failed to write docker-compose.yml")
@@ -186,6 +188,20 @@ func initFromComposeFile(name string, composeFile string) error {
 	return nil
 }
 
+func removeDefaultValuesFromCompose(compose []byte) []byte {
+	// find variable names followed by default values/error messages with ':-', '-', ':?' and '?' as separators.
+	rePattern := regexp.MustCompile(`\$\{[a-zA-Z_]+[a-zA-Z0-9_.]*((:-)|(\-)|(:\?)|(\?))(.*)\}`)
+	matches := rePattern.FindAllSubmatch(compose, -1)
+	//remove default value from compose content
+	for _, groups := range matches {
+		variable := groups[0]
+		separator := groups[1]
+		variableName := bytes.SplitN(variable, separator, 2)[0]
+		compose = bytes.ReplaceAll(compose, variable, []byte(fmt.Sprintf("%s}", variableName)))
+	}
+	return compose
+}
+
 func composeFileFromScratch() ([]byte, error) {
 	fileStruct := types.NewInitialComposeFile()
 	return yaml.Marshal(fileStruct)

internal/packager/init_test.go

@@ -49,14 +49,17 @@ func TestInitFromComposeFileWithFlattenedParams(t *testing.T) {
 version: '3.0'
 services:
   service1:
-    image: image1
     ports:
       - ${ports.service1:-9001}
-
   service2:
-    image: image2
     ports:
-      - ${ports.service2:-9002}
+      - ${ports.service2-9002}
+  service3:
+    ports:
+      - ${ports.service3:?'port is unset or empty in the environment'}
+  service4:
+    ports:
+      - ${ports.service4?'port is unset or empty in the environment'}
 `
 	inputDir := fs.NewDir(t, "app_input_",
 		fs.WithFile(internal.ComposeFileName, composeData),
@@ -75,14 +78,31 @@ services:
 	const expectedParameters = `ports:
   service1: 9001
   service2: 9002
+  service3: FILL ME
+  service4: FILL ME
+`
+	const expectedUpdatedComposeData = `
+version: '3.0'
+services:
+  service1:
+    ports:
+      - ${ports.service1}
+  service2:
+    ports:
+      - ${ports.service2}
+  service3:
+    ports:
+      - ${ports.service3}
+  service4:
+    ports:
+      - ${ports.service4}
 `
 	manifest := fs.Expected(
 		t,
 		fs.WithMode(0755),
-		fs.WithFile(internal.ComposeFileName, composeData, fs.WithMode(0644)),
+		fs.WithFile(internal.ComposeFileName, expectedUpdatedComposeData, fs.WithMode(0644)),
 		fs.WithFile(internal.ParametersFileName, expectedParameters, fs.WithMode(0644)),
 	)
-
 	assert.Assert(t, fs.Equal(dir.Join(appName), manifest))
 }
 

render/render.go

@@ -1,14 +1,15 @@
 package render
 
 import (
+	"fmt"
+	"regexp"
 	"strings"
 
 	"github.com/deislabs/cnab-go/bundle"
 	"github.com/docker/app/internal/compose"
 	"github.com/docker/app/types"
 	"github.com/docker/app/types/parameters"
 	"github.com/docker/cli/cli/compose/loader"
-	composetemplate "github.com/docker/cli/cli/compose/template"
 	composetypes "github.com/docker/cli/cli/compose/types"
 	"github.com/pkg/errors"
 
@@ -18,6 +19,23 @@ import (
 	_ "github.com/docker/app/internal/formatter/yaml"
 )
 
+// pattern matching for ${text} and $text substrings (characters allowed: 0-9 a-z _ .)
+const (
+	delimiter = `\$`
+	// variable name must start with at least one of the the following: a-z, A-Z or _
+	substitution = `[a-zA-Z_]+([a-zA-Z0-9_]*(([.]{1}[0-9a-zA-Z_]+)|([0-9a-zA-Z_])))*`
+	// compose files may contain variable names followed by default values/error messages with separators ':-', '-', ':?' and '?'.
+	defaultValuePattern = `[a-zA-Z_]+[a-zA-Z0-9_.]*((:-)|(\-)|(:\?)|(\?)){1}(.*)`
+)
+
+var (
+	patternString = fmt.Sprintf(
+		`%s(?i:(?P<named>%s)|(?P<skip>%s{1,})|\{(?P<braced>%s)\}|\{(?P<fail>%s)\})`,
+		delimiter, substitution, delimiter, substitution, defaultValuePattern,
+	)
+	rePattern = regexp.MustCompile(patternString)
+)
+
 // Render renders the Compose file for this app, merging in parameters files, other compose files, and env
 // appname string, composeFiles []string, parametersFiles []string
 func Render(app *types.App, env map[string]string, imageMap map[string]bundle.Image) (*composetypes.Config, error) {
@@ -37,20 +55,53 @@ func Render(app *types.App, env map[string]string, imageMap map[string]bundle.Im
 	if err != nil {
 		return nil, errors.Wrap(err, "failed to merge parameters")
 	}
-	configFiles, _, err := compose.Load(app.Composes())
+	composeContent := string(app.Composes()[0])
+	composeContent, err = substituteParams(allParameters.Flatten(), composeContent)
 	if err != nil {
-		return nil, errors.Wrap(err, "failed to load composefiles")
+		return nil, err
+	}
+	return render(app.Path, composeContent, imageMap)
+}
+
+func substituteParams(allParameters map[string]string, composeContent string) (string, error) {
+	matches := rePattern.FindAllStringSubmatch(composeContent, -1)
+	if len(matches) == 0 {
+		return composeContent, nil
+	}
+	for _, match := range matches {
+		groups := make(map[string]string)
+		for i, name := range rePattern.SubexpNames()[1:] {
+			groups[name] = match[i+1]
+		}
+		//fail on default values enclosed within {}
+		if fail := groups["fail"]; fail != "" {
+			return "", errors.New(fmt.Sprintf("Parameters must not have default values set in compose file. Invalid parameter: %s.", match[0]))
+		}
+		if skip := groups["skip"]; skip != "" {
+			continue
+		}
+		varString := match[0]
+		val := groups["named"]
+		if val == "" {
+			val = groups["braced"]
+		}
+		if value, ok := allParameters[val]; ok {
+			composeContent = strings.ReplaceAll(composeContent, varString, value)
+		} else {
+			return "", errors.New(fmt.Sprintf("Failed to set value for %s. Value not found in parameters.", val))
+		}
 	}
-	return render(app.Path, configFiles, allParameters.Flatten(), imageMap)
+	return composeContent, nil
 }
 
-func render(appPath string, configFiles []composetypes.ConfigFile, finalEnv map[string]string, imageMap map[string]bundle.Image) (*composetypes.Config, error) {
+func render(appPath string, composeContent string, imageMap map[string]bundle.Image) (*composetypes.Config, error) {
+	configFiles, _, err := compose.Load([][]byte{[]byte(composeContent)})
+	if err != nil {
+		return nil, errors.Wrap(err, "failed to load compose content")
+	}
 	rendered, err := loader.Load(composetypes.ConfigDetails{
 		WorkingDir:  appPath,
 		ConfigFiles: configFiles,
-		Environment: finalEnv,
-	}, func(opts *loader.Options) {
-		opts.Interpolate.Substitute = substitute
 	})
 	if err != nil {
 		return nil, errors.Wrap(err, "failed to load Compose file")
@@ -67,20 +118,6 @@ func render(appPath string, configFiles []composetypes.ConfigFile, finalEnv map[
 	return rendered, nil
 }
 
-func substitute(template string, mapping composetemplate.Mapping) (string, error) {
-	return composetemplate.SubstituteWith(template, mapping, compose.ExtrapolationPattern, errorIfMissing)
-}
-
-func errorIfMissing(substitution string, mapping composetemplate.Mapping) (string, bool, error) {
-	value, found := mapping(substitution)
-	if !found {
-		return "", true, &composetemplate.InvalidTemplateError{
-			Template: "required variable " + substitution + " is missing a value",
-		}
-	}
-	return value, true, nil
-}
-
 func processEnabled(config *composetypes.Config) error {
 	services := []composetypes.ServiceConfig{}
 	for _, service := range config.Services {