Skip to content
This repository has been archived by the owner on Jun 13, 2021. It is now read-only.

Commit

Permalink
Merge pull request #726 from jcsirot/move-render-cmd-to-image
Browse files Browse the repository at this point in the history
Move render command under image command
  • Loading branch information
silvin-lubecki authored Nov 6, 2019
2 parents 9042ab2 + 3c05dee commit f141d47
Show file tree
Hide file tree
Showing 11 changed files with 107 additions and 88 deletions.
8 changes: 4 additions & 4 deletions e2e/commands_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func testRenderApp(appPath string, env ...string) func(*testing.T) {
data, err := ioutil.ReadFile(filepath.Join(appPath, "env.yml"))
assert.NilError(t, err)
assert.NilError(t, yaml.Unmarshal(data, &envParameters))
args := dockerCli.Command("app", "render", "a-simple-tag", "--parameters-file", filepath.Join(appPath, "parameters-0.yml"))
args := dockerCli.Command("app", "image", "render", "a-simple-tag", "--parameters-file", filepath.Join(appPath, "parameters-0.yml"))
for k, v := range envParameters {
args = append(args, "--set", fmt.Sprintf("%s=%s", k, v))
}
Expand All @@ -80,7 +80,7 @@ func TestRenderAppNotFound(t *testing.T) {
defer cleanup()

appName := "non_existing_app:some_tag"
cmd.Command = dockerCli.Command("app", "render", appName)
cmd.Command = dockerCli.Command("app", "image", "render", appName)
checkContains(t, icmd.RunCmd(cmd).Assert(t, icmd.Expected{ExitCode: 1}).Combined(),
[]string{fmt.Sprintf("could not render %q: no such App image", appName)})
}
Expand All @@ -93,11 +93,11 @@ func TestRenderFormatters(t *testing.T) {
cmd.Command = dockerCli.Command("app", "build", "--tag", "a-simple-tag", "--no-resolve-image", contextPath)
icmd.RunCmd(cmd).Assert(t, icmd.Success)

cmd.Command = dockerCli.Command("app", "render", "--formatter", "json", "a-simple-tag")
cmd.Command = dockerCli.Command("app", "image", "render", "--formatter", "json", "a-simple-tag")
result := icmd.RunCmd(cmd).Assert(t, icmd.Success)
golden.Assert(t, result.Stdout(), "expected-json-render.golden")

cmd.Command = dockerCli.Command("app", "render", "--formatter", "yaml", "a-simple-tag")
cmd.Command = dockerCli.Command("app", "image", "render", "--formatter", "yaml", "a-simple-tag")
result = icmd.RunCmd(cmd).Assert(t, icmd.Success)
golden.Assert(t, result.Stdout(), "expected-yaml-render.golden")
})
Expand Down
2 changes: 1 addition & 1 deletion e2e/envfile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ func TestRenderWithEnvFile(t *testing.T) {
cmd.Command = dockerCli.Command("app", "build", "-f", appPath, "--tag", "a-simple-tag", "--no-resolve-image", ".")
icmd.RunCmd(cmd).Assert(t, icmd.Success)

cmd.Command = dockerCli.Command("app", "render", "a-simple-tag")
cmd.Command = dockerCli.Command("app", "image", "render", "a-simple-tag")
icmd.RunCmd(cmd).Assert(t, icmd.Expected{Out: `version: "3.7"
services:
db:
Expand Down
39 changes: 21 additions & 18 deletions internal/commands/parameters.go → internal/bundle/parameters.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package commands
package bundle

import (
"encoding/json"
Expand All @@ -16,16 +16,18 @@ import (
"github.com/pkg/errors"
)

type mergeBundleConfig struct {
// MergeBundleConfig is the actual parameters and bundle parameters to be merged
type MergeBundleConfig struct {
bundle *bundle.Bundle
params map[string]string
stderr io.Writer
}

type mergeBundleOpt func(c *mergeBundleConfig) error
// MergeBundleOpt is a functional option of the bundle parameter merge function
type MergeBundleOpt func(c *MergeBundleConfig) error

func withFileParameters(parametersFiles []string) mergeBundleOpt {
return func(c *mergeBundleConfig) error {
func WithFileParameters(parametersFiles []string) MergeBundleOpt {
return func(c *MergeBundleConfig) error {
p, err := parameters.LoadFiles(parametersFiles)
if err != nil {
return err
Expand All @@ -37,8 +39,8 @@ func withFileParameters(parametersFiles []string) mergeBundleOpt {
}
}

func withCommandLineParameters(overrides []string) mergeBundleOpt {
return func(c *mergeBundleConfig) error {
func WithCommandLineParameters(overrides []string) MergeBundleOpt {
return func(c *MergeBundleConfig) error {
d := cliopts.ConvertKVStringsToMap(overrides)
for k, v := range d {
c.params[k] = v
Expand All @@ -47,8 +49,8 @@ func withCommandLineParameters(overrides []string) mergeBundleOpt {
}
}

func withLabels(labels []string) mergeBundleOpt {
return func(c *mergeBundleConfig) error {
func WithLabels(labels []string) MergeBundleOpt {
return func(c *MergeBundleConfig) error {
for _, l := range labels {
if strings.HasPrefix(l, internal.Namespace) {
return errors.Errorf("labels cannot start with %q", internal.Namespace)
Expand All @@ -68,8 +70,8 @@ func withLabels(labels []string) mergeBundleOpt {
}
}

func withSendRegistryAuth(sendRegistryAuth bool) mergeBundleOpt {
return func(c *mergeBundleConfig) error {
func WithSendRegistryAuth(sendRegistryAuth bool) MergeBundleOpt {
return func(c *MergeBundleConfig) error {
if _, ok := c.bundle.Definitions[internal.ParameterShareRegistryCredsName]; ok {
val := "false"
if sendRegistryAuth {
Expand All @@ -81,8 +83,8 @@ func withSendRegistryAuth(sendRegistryAuth bool) mergeBundleOpt {
}
}

func withOrchestratorParameters(orchestrator string, kubeNamespace string) mergeBundleOpt {
return func(c *mergeBundleConfig) error {
func WithOrchestratorParameters(orchestrator string, kubeNamespace string) MergeBundleOpt {
return func(c *MergeBundleConfig) error {
if _, ok := c.bundle.Definitions[internal.ParameterOrchestratorName]; ok {
c.params[internal.ParameterOrchestratorName] = orchestrator
}
Expand All @@ -93,20 +95,21 @@ func withOrchestratorParameters(orchestrator string, kubeNamespace string) merge
}
}

func withErrorWriter(w io.Writer) mergeBundleOpt {
return func(c *mergeBundleConfig) error {
func WithErrorWriter(w io.Writer) MergeBundleOpt {
return func(c *MergeBundleConfig) error {
c.stderr = w
return nil
}
}

func mergeBundleParameters(installation *store.Installation, ops ...mergeBundleOpt) error {
// MergeBundleParameters merges current, provided and bundle default parameters
func MergeBundleParameters(installation *store.Installation, ops ...MergeBundleOpt) error {
bndl := installation.Bundle
if installation.Parameters == nil {
installation.Parameters = make(map[string]interface{})
}
userParams := map[string]string{}
cfg := &mergeBundleConfig{
cfg := &MergeBundleConfig{
bundle: bndl,
params: userParams,
stderr: os.Stderr,
Expand All @@ -124,7 +127,7 @@ func mergeBundleParameters(installation *store.Installation, ops ...mergeBundleO
return err
}

func matchAndMergeParametersDefinition(currentValues map[string]interface{}, cfg *mergeBundleConfig) (map[string]interface{}, error) {
func matchAndMergeParametersDefinition(currentValues map[string]interface{}, cfg *MergeBundleConfig) (map[string]interface{}, error) {
mergedValues := make(map[string]interface{})
for k, v := range currentValues {
mergedValues[k] = v
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package commands
package bundle

import (
"bytes"
Expand Down Expand Up @@ -31,8 +31,8 @@ overridden: bar`))
actual := map[string]string{
"overridden": "foo",
}
err := withFileParameters([]string{tmpDir.Join("params.yaml")})(
&mergeBundleConfig{
err := WithFileParameters([]string{tmpDir.Join("params.yaml")})(
&MergeBundleConfig{
bundle: bundle,
params: actual,
})
Expand All @@ -51,8 +51,8 @@ func TestWithCommandLineParameters(t *testing.T) {
"overridden": "foo",
}

err := withCommandLineParameters([]string{"param1.param2=value1", "param3=3", "overridden=bar"})(
&mergeBundleConfig{
err := WithCommandLineParameters([]string{"param1.param2=value1", "param3=3", "overridden=bar"})(
&MergeBundleConfig{
bundle: bundle,
params: actual,
})
Expand Down Expand Up @@ -145,7 +145,7 @@ func TestWithOrchestratorParameters(t *testing.T) {
for _, testCase := range testCases {
t.Run(testCase.name, func(t *testing.T) {
actual := map[string]string{}
err := withOrchestratorParameters("kubernetes", "my-namespace")(&mergeBundleConfig{
err := WithOrchestratorParameters("kubernetes", "my-namespace")(&MergeBundleConfig{
bundle: testCase.bundle,
params: actual,
})
Expand All @@ -157,17 +157,17 @@ func TestWithOrchestratorParameters(t *testing.T) {

func TestMergeBundleParameters(t *testing.T) {
t.Run("Override Order", func(t *testing.T) {
first := func(c *mergeBundleConfig) error {
first := func(c *MergeBundleConfig) error {
c.params["param"] = "first"
return nil
}
second := func(c *mergeBundleConfig) error {
second := func(c *MergeBundleConfig) error {
c.params["param"] = "second"
return nil
}
bundle := prepareBundle(withParameterAndDefault("param", "string", "default"))
i := &store.Installation{Claim: claim.Claim{Bundle: bundle}}
err := mergeBundleParameters(i,
err := MergeBundleParameters(i,
first,
second,
)
Expand All @@ -181,7 +181,7 @@ func TestMergeBundleParameters(t *testing.T) {
t.Run("Default values", func(t *testing.T) {
bundle := prepareBundle(withParameterAndDefault("param", "string", "default"))
i := &store.Installation{Claim: claim.Claim{Bundle: bundle}}
err := mergeBundleParameters(i)
err := MergeBundleParameters(i)
assert.NilError(t, err)
expected := map[string]interface{}{
"param": "default",
Expand All @@ -190,13 +190,13 @@ func TestMergeBundleParameters(t *testing.T) {
})

t.Run("Converting values", func(t *testing.T) {
withIntValue := func(c *mergeBundleConfig) error {
withIntValue := func(c *MergeBundleConfig) error {
c.params["param"] = "1"
return nil
}
bundle := prepareBundle(withParameter("param", "integer"))
i := &store.Installation{Claim: claim.Claim{Bundle: bundle}}
err := mergeBundleParameters(i, withIntValue)
err := MergeBundleParameters(i, withIntValue)
assert.NilError(t, err)
expected := map[string]interface{}{
"param": 1,
Expand All @@ -207,7 +207,7 @@ func TestMergeBundleParameters(t *testing.T) {
t.Run("Default values", func(t *testing.T) {
bundle := prepareBundle(withParameterAndDefault("param", "string", "default"))
i := &store.Installation{Claim: claim.Claim{Bundle: bundle}}
err := mergeBundleParameters(i)
err := MergeBundleParameters(i)
assert.NilError(t, err)
expected := map[string]interface{}{
"param": "default",
Expand All @@ -216,54 +216,54 @@ func TestMergeBundleParameters(t *testing.T) {
})

t.Run("Undefined parameter throws warning", func(t *testing.T) {
withUndefined := func(c *mergeBundleConfig) error {
withUndefined := func(c *MergeBundleConfig) error {
c.params["param"] = "1"
return nil
}
bundle := prepareBundle()
i := &store.Installation{Claim: claim.Claim{Bundle: bundle}}
buf := new(bytes.Buffer)
err := mergeBundleParameters(i, withUndefined, withErrorWriter(buf))
err := MergeBundleParameters(i, withUndefined, WithErrorWriter(buf))
assert.NilError(t, err)
assert.Assert(t, strings.Contains(buf.String(), "is not defined in the bundle"))
})

t.Run("Warn on undefined parameter", func(t *testing.T) {
withUndefined := func(c *mergeBundleConfig) error {
withUndefined := func(c *MergeBundleConfig) error {
c.params["param"] = "1"
return nil
}
w := bytes.NewBuffer(nil)
withStdErr := func(c *mergeBundleConfig) error {
withStdErr := func(c *MergeBundleConfig) error {
c.stderr = w
return nil
}
bundle := prepareBundle()
i := &store.Installation{Claim: claim.Claim{Bundle: bundle}}
err := mergeBundleParameters(i, withUndefined, withStdErr)
err := MergeBundleParameters(i, withUndefined, withStdErr)
assert.NilError(t, err)
assert.Equal(t, w.String(), "Warning: parameter \"param\" is not defined in the bundle\n")
})

t.Run("Invalid type is rejected", func(t *testing.T) {
withIntValue := func(c *mergeBundleConfig) error {
withIntValue := func(c *MergeBundleConfig) error {
c.params["param"] = "foo"
return nil
}
bundle := prepareBundle(withParameter("param", "integer"))
i := &store.Installation{Claim: claim.Claim{Bundle: bundle}}
err := mergeBundleParameters(i, withIntValue)
err := MergeBundleParameters(i, withIntValue)
assert.ErrorContains(t, err, "invalid value for parameter")
})

t.Run("Invalid value is rejected", func(t *testing.T) {
withInvalidValue := func(c *mergeBundleConfig) error {
withInvalidValue := func(c *MergeBundleConfig) error {
c.params["param"] = "invalid"
return nil
}
bundle := prepareBundle(withParameterAndValues("param", "string", []interface{}{"valid"}))
i := &store.Installation{Claim: claim.Claim{Bundle: bundle}}
err := mergeBundleParameters(i, withInvalidValue)
err := MergeBundleParameters(i, withInvalidValue)
assert.ErrorContains(t, err, "invalid value for parameter")
})
}
Expand All @@ -280,9 +280,9 @@ func TestLabels(t *testing.T) {
labels := []string{
"label=value",
}
op := withLabels(labels)
op := WithLabels(labels)

config := &mergeBundleConfig{
config := &MergeBundleConfig{
bundle: &bundle.Bundle{
Parameters: map[string]bundle.Parameter{
internal.ParameterArgs: {},
Expand All @@ -301,7 +301,7 @@ func TestInvalidLabels(t *testing.T) {
labels := []string{
"com.docker.app.label=value",
}
op := withLabels(labels)
err := op(&mergeBundleConfig{})
op := WithLabels(labels)
err := op(&MergeBundleConfig{})
assert.ErrorContains(t, err, fmt.Sprintf("labels cannot start with %q", internal.Namespace))
}
17 changes: 17 additions & 0 deletions internal/cliopts/parameters.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package cliopts

import (
"github.com/spf13/pflag"
)

// ParametersOptions are shared CLI options about docker app parameters
type ParametersOptions struct {
ParametersFiles []string
Overrides []string
}

// AddFlags adds the shared CLI flags to the given flag set
func (o *ParametersOptions) AddFlags(flags *pflag.FlagSet) {
flags.StringArrayVar(&o.ParametersFiles, "parameters-file", []string{}, "Override parameters file")
flags.StringArrayVarP(&o.Overrides, "set", "s", []string{}, "Override parameter value")
}
1 change: 1 addition & 0 deletions internal/commands/image/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ func Cmd(dockerCli command.Cli) *cobra.Command {
rmCmd(),
tagCmd(),
inspectCmd(dockerCli),
renderCmd(dockerCli),
)

return cmd
Expand Down
Loading

0 comments on commit f141d47

Please sign in to comment.