diff --git a/internal/packager/init.go b/internal/packager/init.go index 221847e48..76a3c02a3 100644 --- a/internal/packager/init.go +++ b/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) diff --git a/internal/packager/init_test.go b/internal/packager/init_test.go index 7e03f8f08..9d208defe 100644 --- a/internal/packager/init_test.go +++ b/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)) } diff --git a/render/render.go b/render/render.go index 861bfdfdd..ca6de0f2f 100644 --- a/render/render.go +++ b/render/render.go @@ -1,6 +1,8 @@ package render import ( + "fmt" + "regexp" "strings" "github.com/deislabs/cnab-go/bundle" @@ -8,7 +10,6 @@ import ( "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%s)|(?P%s{1,})|\{(?P%s)\}|\{(?P%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 { diff --git a/render/render_test.go b/render/render_test.go index 6594d57e0..fcd3adc88 100644 --- a/render/render_test.go +++ b/render/render_test.go @@ -1,13 +1,13 @@ package render import ( + "fmt" "strings" "testing" "github.com/deislabs/cnab-go/bundle" "github.com/docker/app/types" "github.com/docker/app/types/parameters" - composetypes "github.com/docker/cli/cli/compose/types" yaml "gopkg.in/yaml.v2" "gotest.tools/assert" is "gotest.tools/assert/cmp" @@ -18,73 +18,181 @@ const ( name: my-app` ) -func TestRenderMissingValue(t *testing.T) { - configFiles := []composetypes.ConfigFile{ - { - Config: map[string]interface{}{ - "version": "3", - "services": map[string]interface{}{ - "foo": map[string]interface{}{ - "image": "${imageName}:${version}", - }, - }, - }, - }, +func TestSubstituteBracedParams(t *testing.T) { + composeFile := ` +version: "3.6" +services: + front: + ports: + - "${front.port}:80" +` + parameters := map[string]string{ + "front.port": "8080", } - finalEnv := map[string]string{ - "imageName": "foo", + s, err := substituteParams(parameters, composeFile) + assert.NilError(t, err) + assert.Equal(t, s, ` +version: "3.6" +services: + front: + ports: + - "8080:80" +`) +} + +func TestSubstituteNamedParams(t *testing.T) { + composeFile := ` +version: "3.6" +services: + back: + ports: + - "$back.port:90" +` + parameters := map[string]string{ + "back.port": "9000", } - _, err := render("foo.dockerapp", configFiles, finalEnv, nil) - assert.Check(t, err != nil) - assert.Check(t, is.ErrorContains(err, "required variable")) + s, err := substituteParams(parameters, composeFile) + assert.NilError(t, err) + assert.Equal(t, s, ` +version: "3.6" +services: + back: + ports: + - "9000:90" +`) +} + +func checkRenderError(t *testing.T, userParameters map[string]string, composeFile string, expectedError string) { + metadata := strings.NewReader(validMeta) + + app := &types.App{Path: "my-app"} + assert.NilError(t, types.Metadata(metadata)(app)) + assert.NilError(t, types.WithComposes(strings.NewReader(composeFile))(app)) + _, err := Render(app, userParameters, nil) + assert.ErrorContains(t, err, expectedError) } -func TestRender(t *testing.T) { - configFiles := []composetypes.ConfigFile{ - { - Config: map[string]interface{}{ - "version": "3", - "services": map[string]interface{}{ - "foo": map[string]interface{}{ - "image": "busybox:${version}", - "command": []interface{}{"-text", "${foo.bar}"}, - }, - }, - }, - }, +func TestRenderFailOnDefaultParamValueInCompose(t *testing.T) { + composeFile := ` +version: "3.6" +services: + front: + ports: + - "${front.port:-9090}:80" +` + userParameters := map[string]string{ + "front.port": "4242", } - finalEnv := map[string]string{ - "version": "latest", - "foo.bar": "baz", + checkRenderError(t, userParameters, composeFile, "Parameters must not have default values set in compose file. Invalid parameter: ${front.port:-9090}.") + + composeFile = ` +version: "3.6" +services: + front: + ports: + - "${front.port-9090}:80" + ` + checkRenderError(t, userParameters, composeFile, "Parameters must not have default values set in compose file. Invalid parameter: ${front.port-9090}.") + composeFile = ` +version: "3.6" +services: + front: + ports: + - "${front.port:?Error}:80" + ` + checkRenderError(t, userParameters, composeFile, "Parameters must not have default values set in compose file. Invalid parameter: ${front.port:?Error}.") + + composeFile = ` +version: "3.6" +services: + front: + ports: + - "${front.port?Error:unset variable}:80" + ` + checkRenderError(t, userParameters, composeFile, "Parameters must not have default values set in compose file. Invalid parameter: ${front.port?Error:unset variable}.") + +} +func TestSubstituteMixedParams(t *testing.T) { + composeFile := ` +version: "3.6" +services: + front: + ports: + - "${front.port}:80" + deploy: + replicas: ${front.deploy.replicas} + back: + ports: + - "$back.port:90" +` + parameters := map[string]string{ + "front.port": "8080", + "back.port": "9000", + "front.deploy.replicas": "3", } - c, err := render("foo.dockerapp", configFiles, finalEnv, nil) + s, err := substituteParams(parameters, composeFile) assert.NilError(t, err) - assert.Check(t, is.Len(c.Services, 1)) - assert.Check(t, is.Equal(c.Services[0].Image, "busybox:latest")) - assert.Check(t, is.DeepEqual([]string(c.Services[0].Command), []string{"-text", "baz"})) + assert.Equal(t, s, ` +version: "3.6" +services: + front: + ports: + - "8080:80" + deploy: + replicas: 3 + back: + ports: + - "9000:90" +`) +} + +func TestSkipDoubleDollarCase(t *testing.T) { + composeFile := ` + version: "3.7" + services: + front: + command: $$dollar +` + s, err := substituteParams(map[string]string{}, composeFile) + assert.NilError(t, err) + assert.Equal(t, s, ` + version: "3.7" + services: + front: + command: $$dollar +`) +} + +func TestSubstituteMissingParameterValue(t *testing.T) { + composeFile := ` + version: "3.7" + services: + front: + deploy: + replicas: ${myapp.nginx_replicas} + debug: + ports: + - $aport +` + parameters := map[string]string{ + "aport": "10000", + } + _, err := substituteParams(parameters, composeFile) + assert.ErrorContains(t, err, "Failed to set value for myapp.nginx_replicas. Value not found in parameters.") } func TestRenderEnabledFalse(t *testing.T) { - for _, tc := range []interface{}{false, "false", "! ${myapp.debug}"} { - configs := []composetypes.ConfigFile{ - { - Config: map[string]interface{}{ - "version": "3.7", - "services": map[string]interface{}{ - "foo": map[string]interface{}{ - "image": "busybox", - "command": []interface{}{"-text", "foo"}, - "x-enabled": tc, - }, - }, - }, - }, - } - c, err := render("foo.dockerapp", configs, map[string]string{ - "myapp.debug": "true", - }, nil) + for _, tc := range []interface{}{"false", "\"false\"", "\"! true\""} { + composeFile := fmt.Sprintf(` +version: "3.7" +services: + foo: + image: busybox + "x-enabled": %s +`, tc) + c, err := render("foo.dockerapp", composeFile, nil) assert.NilError(t, err) - assert.Check(t, is.Len(c.Services, 0)) + assert.Check(t, is.Len(c.Services, 0), fmt.Sprintf("Failed for %s", tc)) } } @@ -214,19 +322,13 @@ services: } func TestServiceImageOverride(t *testing.T) { - configFiles := []composetypes.ConfigFile{ - { - Config: map[string]interface{}{ - "version": "3", - "services": map[string]interface{}{ - "foo": map[string]interface{}{ - "image": "busybox", - }, - }, - }, - }, - } - c, err := render("foo.dockerapp", configFiles, nil, map[string]bundle.Image{ + composeFile := ` +version: "3.6" +services: + foo: + image: busybox, +` + c, err := render("foo.dockerapp", composeFile, map[string]bundle.Image{ "foo": {BaseImage: bundle.BaseImage{Image: "test"}}, }) assert.NilError(t, err)