Merge pull request #366 from chris-crone/settings-refactor

Expose settings functions externally

Author: silvin-lubecki

cmd/docker-app/validate.go

@@ -2,7 +2,7 @@ package main
 
 import (
 	"github.com/docker/app/internal/packager"
-	"github.com/docker/app/internal/validator"
+	"github.com/docker/app/render"
 	"github.com/docker/app/types"
 	"github.com/docker/cli/cli"
 	cliopts "github.com/docker/cli/opts"
@@ -28,7 +28,8 @@ func validateCmd() *cobra.Command {
 			}
 			defer app.Cleanup()
 			argSettings := cliopts.ConvertKVStringsToMap(validateEnv)
-			return validator.Validate(app, argSettings)
+			_, err = render.Render(app, argSettings)
+			return err
 		},
 	}
 	cmd.Flags().StringArrayVarP(&validateSettingsFile, "settings-files", "f", []string{}, "Override settings files")

internal/helm/helm.go

@@ -13,12 +13,12 @@ import (
 	"github.com/docker/app/internal/helm/templateconversion"
 	"github.com/docker/app/internal/helm/templateloader"
 	"github.com/docker/app/internal/helm/templatev1beta2"
-	"github.com/docker/app/internal/settings"
 	"github.com/docker/app/internal/slices"
 	"github.com/docker/app/internal/yaml"
 	"github.com/docker/app/render"
 	"github.com/docker/app/types"
 	"github.com/docker/app/types/metadata"
+	"github.com/docker/app/types/settings"
 	"github.com/docker/cli/cli/command/stack/kubernetes"
 	"github.com/docker/cli/cli/compose/loader"
 	"github.com/docker/cli/kubernetes/compose/v1beta1"
@@ -49,8 +49,8 @@ func Helm(app *types.App, env map[string]string, shouldRender bool, stackVersion
 	if err := os.MkdirAll(targetDir, 0755); err != nil {
 		return errors.Wrap(err, "failed to create Chart directory")
 	}
-	err := makeChart(app.Metadata(), targetDir)
-	if err != nil {
+	meta := app.Metadata()
+	if err := makeChart(&meta, targetDir); err != nil {
 		return err
 	}
 	if shouldRender {
@@ -70,8 +70,7 @@ func Helm(app *types.App, env map[string]string, shouldRender bool, stackVersion
 	for k := range vars {
 		variables = append(variables, k)
 	}
-	err = makeStack(app.Name, targetDir, data, stackVersion)
-	if err != nil {
+	if err := makeStack(app.Name, targetDir, data, stackVersion); err != nil {
 		return err
 	}
 	return makeValues(app, targetDir, env, variables)
@@ -80,11 +79,8 @@ func Helm(app *types.App, env map[string]string, shouldRender bool, stackVersion
 // makeValues updates helm values.yaml with used variables from settings and env
 func makeValues(app *types.App, targetDir string, env map[string]string, variables []string) error {
 	// merge our variables into Values.yaml
-	s, err := settings.LoadMultiple(app.Settings())
-	if err != nil {
-		return err
-	}
-	metaPrefixed, err := settings.Load(app.Metadata(), settings.WithPrefix("app"))
+	s := app.Settings()
+	metaPrefixed, err := settings.Load(app.MetadataRaw(), settings.WithPrefix("app"))
 	if err != nil {
 		return err
 	}
@@ -210,13 +206,8 @@ func helmRender(app *types.App, targetDir string, env map[string]string, stackVe
 	return ioutil.WriteFile(filepath.Join(targetDir, "templates", "stack.yaml"), stackData, 0644)
 }
 
-func makeChart(data []byte, targetDir string) error {
-	var meta metadata.AppMetadata
-	err := yaml.Unmarshal(data, &meta)
-	if err != nil {
-		return errors.Wrap(err, "failed to parse application metadata")
-	}
-	hmeta, err := toHelmMeta(&meta)
+func makeChart(meta *metadata.AppMetadata, targetDir string) error {
+	hmeta, err := toHelmMeta(meta)
 	if err != nil {
 		return errors.Wrap(err, "failed to convert application metadata")
 	}

internal/inspect/inspect.go

@@ -6,29 +6,17 @@ import (
 	"sort"
 	"text/tabwriter"
 
-	"github.com/docker/app/internal/settings"
-	"github.com/docker/app/internal/yaml"
 	"github.com/docker/app/types"
-	"github.com/docker/app/types/metadata"
-	"github.com/pkg/errors"
 )
 
 // Inspect dumps the metadata of an app
 func Inspect(out io.Writer, app *types.App) error {
-	var meta metadata.AppMetadata
-	err := yaml.Unmarshal(app.Metadata(), &meta)
-	if err != nil {
-		return errors.Wrap(err, "failed to parse application metadata")
-	}
+	meta := app.Metadata()
 	// extract settings
-	s, err := settings.LoadMultiple(app.Settings())
-	if err != nil {
-		return errors.Wrap(err, "failed to load application settings")
-	}
-	fs := s.Flatten()
+	settings := app.Settings().Flatten()
 	// sort the keys to get consistent output
 	var settingsKeys []string
-	for k := range fs {
+	for k := range settings {
 		settingsKeys = append(settingsKeys, k)
 	}
 	sort.Slice(settingsKeys, func(i, j int) bool { return settingsKeys[i] < settingsKeys[j] })
@@ -47,7 +35,7 @@ func Inspect(out io.Writer, app *types.App) error {
 	fmt.Fprintln(w, "Setting\tDefault")
 	fmt.Fprintln(w, "-------\t-------")
 	for _, k := range settingsKeys {
-		fmt.Fprintf(w, "%s\t%s\n", k, fs[k])
+		fmt.Fprintf(w, "%s\t%s\n", k, settings[k])
 	}
 	w.Flush()
 	return nil

internal/inspect/inspect_test.go

@@ -3,13 +3,11 @@ package inspect
 import (
 	"bytes"
 	"fmt"
-	"io/ioutil"
 	"testing"
 
 	"github.com/docker/app/internal"
 	"github.com/docker/app/types"
 	"gotest.tools/assert"
-	is "gotest.tools/assert/cmp"
 	"gotest.tools/fs"
 	"gotest.tools/golden"
 )
@@ -22,32 +20,6 @@ services:
     image: nginx`
 )
 
-func TestInspectErrorsOnFiles(t *testing.T) {
-	dir := fs.NewDir(t, "inspect-errors",
-		fs.WithDir("unparseable-metadata-app",
-			fs.WithFile(internal.ComposeFileName, composeYAML),
-			fs.WithFile(internal.MetadataFileName, `something is wrong`),
-			fs.WithFile(internal.SettingsFileName, "foo"),
-		),
-		fs.WithDir("unparseable-settings-app",
-			fs.WithFile(internal.ComposeFileName, composeYAML),
-			fs.WithFile(internal.MetadataFileName, `{}`),
-			fs.WithFile(internal.SettingsFileName, "foo"),
-		),
-	)
-	defer dir.Remove()
-
-	for appname, expectedError := range map[string]string{
-		"unparseable-metadata-app": "failed to parse application metadat",
-		"unparseable-settings-app": "failed to load application settings",
-	} {
-		app, err := types.NewAppFromDefaultFiles(dir.Join(appname))
-		assert.NilError(t, err)
-		err = Inspect(ioutil.Discard, app)
-		assert.Check(t, is.ErrorContains(err, expectedError))
-	}
-}
-
 func TestInspect(t *testing.T) {
 	dir := fs.NewDir(t, "inspect",
 		fs.WithDir("no-maintainers",

internal/packager/init.go

@@ -237,12 +237,10 @@ func newMetadata(name string, description string, maintainers []string) metadata
 		Description: description,
 	}
 	if len(maintainers) == 0 {
-		var userName string
 		userData, _ := user.Current()
-		if userData != nil {
-			userName = userData.Username
+		if userData != nil && userData.Username != "" {
+			res.Maintainers = []metadata.Maintainer{{Name: userData.Username}}
 		}
-		res.Maintainers = []metadata.Maintainer{{Name: userName}}
 	} else {
 		res.Maintainers = parseMaintainersData(maintainers)
 	}

internal/packager/registry.go

@@ -8,10 +8,8 @@ import (
 	"strings"
 
 	"github.com/docker/app/internal"
-	"github.com/docker/app/internal/yaml"
 	"github.com/docker/app/pkg/resto"
 	"github.com/docker/app/types"
-	"github.com/docker/app/types/metadata"
 	"github.com/docker/distribution/reference"
 	"github.com/pkg/errors"
 	log "github.com/sirupsen/logrus"
@@ -70,14 +68,11 @@ func Pull(repotag string, outputDir string) (string, error) {
 // Push pushes an app to a registry. Returns the image digest.
 func Push(app *types.App, namespace, tag string) (string, error) {
 	payload := make(map[string]string)
-	payload[internal.MetadataFileName] = string(app.Metadata())
+	payload[internal.MetadataFileName] = string(app.MetadataRaw())
 	payload[internal.ComposeFileName] = string(app.Composes()[0])
-	payload[internal.SettingsFileName] = string(app.Settings()[0])
+	payload[internal.SettingsFileName] = string(app.SettingsRaw()[0])
 	if namespace == "" || tag == "" {
-		var metadata metadata.AppMetadata
-		if err := yaml.Unmarshal(app.Metadata(), &metadata); err != nil {
-			return "", errors.Wrap(err, "failed to parse application metadata")
-		}
+		metadata := app.Metadata()
 		if namespace == "" {
 			namespace = metadata.Namespace
 		}

internal/packager/split.go

@@ -16,17 +16,17 @@ func Split(app *types.App, outputDir string) error {
 	if len(app.Composes()) > 1 {
 		return errors.New("split: multiple compose files is not supported")
 	}
-	if len(app.Settings()) > 1 {
+	if len(app.SettingsRaw()) > 1 {
 		return errors.New("split: multiple setting files is not supported")
 	}
 	err := os.MkdirAll(outputDir, 0755)
 	if err != nil {
 		return err
 	}
 	for file, data := range map[string][]byte{
-		internal.MetadataFileName: app.Metadata(),
+		internal.MetadataFileName: app.MetadataRaw(),
 		internal.ComposeFileName:  app.Composes()[0],
-		internal.SettingsFileName: app.Settings()[0],
+		internal.SettingsFileName: app.SettingsRaw()[0],
 	} {
 		if err := ioutil.WriteFile(filepath.Join(outputDir, file), data, 0644); err != nil {
 			return err
@@ -40,15 +40,15 @@ func Merge(app *types.App, target io.Writer) error {
 	if len(app.Composes()) > 1 {
 		return errors.New("merge: multiple compose files is not supported")
 	}
-	if len(app.Settings()) > 1 {
+	if len(app.SettingsRaw()) > 1 {
 		return errors.New("merge: multiple setting files is not supported")
 	}
 	for _, data := range [][]byte{
-		app.Metadata(),
+		app.MetadataRaw(),
 		[]byte(types.SingleFileSeparator),
 		app.Composes()[0],
 		[]byte(types.SingleFileSeparator),
-		app.Settings()[0],
+		app.SettingsRaw()[0],
 	} {
 		if _, err := target.Write(data); err != nil {
 			return err