Skip to content
This repository was archived by the owner on Jul 18, 2025. It is now read-only.

Commit 7d37627

Browse files
author
Jean-Christophe Sirot
committed
Move render command under image command. Move bundle and installation parameters merging into a specific package to avoid package cycles
Signed-off-by: Jean-Christophe Sirot <jean-christophe.sirot@docker.com>
1 parent 6f5737a commit 7d37627

9 files changed

Lines changed: 83 additions & 65 deletions

File tree

‎e2e/commands_test.go‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -49,15 +49,15 @@ func testRenderApp(appPath string, env ...string) func(*testing.T) {
4949
defer dir.Remove()
5050

5151
// Build the App
52-
cmd.Command = dockerCli.Command("app", "build", ".", "--folder", filepath.Join(appPath, "my.dockerapp"), "--tag", "a-simple-tag", "--no-resolve-image")
52+
cmd.Command = dockerCli.Command("app", "build", ".", "--file", filepath.Join(appPath, "my.dockerapp"), "--tag", "a-simple-tag", "--no-resolve-image")
5353
icmd.RunCmd(cmd).Assert(t, icmd.Success)
5454

5555
// Render the App
5656
envParameters := map[string]string{}
5757
data, err := ioutil.ReadFile(filepath.Join(appPath, "env.yml"))
5858
assert.NilError(t, err)
5959
assert.NilError(t, yaml.Unmarshal(data, &envParameters))
60-
args := dockerCli.Command("app", "render", "a-simple-tag", "--parameters-file", filepath.Join(appPath, "parameters-0.yml"))
60+
args := dockerCli.Command("app", "image", "render", "a-simple-tag", "--parameters-file", filepath.Join(appPath, "parameters-0.yml"))
6161
for k, v := range envParameters {
6262
args = append(args, "--set", fmt.Sprintf("%s=%s", k, v))
6363
}
@@ -80,7 +80,7 @@ func TestRenderAppNotFound(t *testing.T) {
8080
defer cleanup()
8181

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

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

100-
cmd.Command = dockerCli.Command("app", "render", "--formatter", "yaml", "a-simple-tag")
100+
cmd.Command = dockerCli.Command("app", "image", "render", "--formatter", "yaml", "a-simple-tag")
101101
result = icmd.RunCmd(cmd).Assert(t, icmd.Success)
102102
golden.Assert(t, result.Stdout(), "expected-yaml-render.golden")
103103
})

‎e2e/envfile_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ func TestRenderWithEnvFile(t *testing.T) {
1515
cmd.Command = dockerCli.Command("app", "build", "-f", appPath, "--tag", "a-simple-tag", "--no-resolve-image", ".")
1616
icmd.RunCmd(cmd).Assert(t, icmd.Success)
1717

18-
cmd.Command = dockerCli.Command("app", "render", "a-simple-tag")
18+
cmd.Command = dockerCli.Command("app", "image", "render", "a-simple-tag")
1919
icmd.RunCmd(cmd).Assert(t, icmd.Expected{Out: `version: "3.7"
2020
services:
2121
db:

internal/commands/parameters.go renamed to internal/bundle/parameters.go

Lines changed: 21 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
package commands
1+
package bundle
22

33
import (
44
"encoding/json"
@@ -16,16 +16,18 @@ import (
1616
"github.com/pkg/errors"
1717
)
1818

19-
type mergeBundleConfig struct {
19+
// MergeBundleConfig is the actual parameters and bundle parameters to be merged
20+
type MergeBundleConfig struct {
2021
bundle *bundle.Bundle
2122
params map[string]string
2223
stderr io.Writer
2324
}
2425

25-
type mergeBundleOpt func(c *mergeBundleConfig) error
26+
// MergeBundleOpt is a functional option of the bundle parameter merge function
27+
type MergeBundleOpt func(c *MergeBundleConfig) error
2628

27-
func withFileParameters(parametersFiles []string) mergeBundleOpt {
28-
return func(c *mergeBundleConfig) error {
29+
func WithFileParameters(parametersFiles []string) MergeBundleOpt {
30+
return func(c *MergeBundleConfig) error {
2931
p, err := parameters.LoadFiles(parametersFiles)
3032
if err != nil {
3133
return err
@@ -37,8 +39,8 @@ func withFileParameters(parametersFiles []string) mergeBundleOpt {
3739
}
3840
}
3941

40-
func withCommandLineParameters(overrides []string) mergeBundleOpt {
41-
return func(c *mergeBundleConfig) error {
42+
func WithCommandLineParameters(overrides []string) MergeBundleOpt {
43+
return func(c *MergeBundleConfig) error {
4244
d := cliopts.ConvertKVStringsToMap(overrides)
4345
for k, v := range d {
4446
c.params[k] = v
@@ -47,8 +49,8 @@ func withCommandLineParameters(overrides []string) mergeBundleOpt {
4749
}
4850
}
4951

50-
func withLabels(labels []string) mergeBundleOpt {
51-
return func(c *mergeBundleConfig) error {
52+
func WithLabels(labels []string) MergeBundleOpt {
53+
return func(c *MergeBundleConfig) error {
5254
for _, l := range labels {
5355
if strings.HasPrefix(l, internal.Namespace) {
5456
return errors.Errorf("labels cannot start with %q", internal.Namespace)
@@ -68,8 +70,8 @@ func withLabels(labels []string) mergeBundleOpt {
6870
}
6971
}
7072

71-
func withSendRegistryAuth(sendRegistryAuth bool) mergeBundleOpt {
72-
return func(c *mergeBundleConfig) error {
73+
func WithSendRegistryAuth(sendRegistryAuth bool) MergeBundleOpt {
74+
return func(c *MergeBundleConfig) error {
7375
if _, ok := c.bundle.Definitions[internal.ParameterShareRegistryCredsName]; ok {
7476
val := "false"
7577
if sendRegistryAuth {
@@ -81,8 +83,8 @@ func withSendRegistryAuth(sendRegistryAuth bool) mergeBundleOpt {
8183
}
8284
}
8385

84-
func withOrchestratorParameters(orchestrator string, kubeNamespace string) mergeBundleOpt {
85-
return func(c *mergeBundleConfig) error {
86+
func WithOrchestratorParameters(orchestrator string, kubeNamespace string) MergeBundleOpt {
87+
return func(c *MergeBundleConfig) error {
8688
if _, ok := c.bundle.Definitions[internal.ParameterOrchestratorName]; ok {
8789
c.params[internal.ParameterOrchestratorName] = orchestrator
8890
}
@@ -93,20 +95,21 @@ func withOrchestratorParameters(orchestrator string, kubeNamespace string) merge
9395
}
9496
}
9597

96-
func withErrorWriter(w io.Writer) mergeBundleOpt {
97-
return func(c *mergeBundleConfig) error {
98+
func WithErrorWriter(w io.Writer) MergeBundleOpt {
99+
return func(c *MergeBundleConfig) error {
98100
c.stderr = w
99101
return nil
100102
}
101103
}
102104

103-
func mergeBundleParameters(installation *store.Installation, ops ...mergeBundleOpt) error {
105+
// MergeBundleParameters merges current, provided and bundle default parameters
106+
func MergeBundleParameters(installation *store.Installation, ops ...MergeBundleOpt) error {
104107
bndl := installation.Bundle
105108
if installation.Parameters == nil {
106109
installation.Parameters = make(map[string]interface{})
107110
}
108111
userParams := map[string]string{}
109-
cfg := &mergeBundleConfig{
112+
cfg := &MergeBundleConfig{
110113
bundle: bndl,
111114
params: userParams,
112115
stderr: os.Stderr,
@@ -124,7 +127,7 @@ func mergeBundleParameters(installation *store.Installation, ops ...mergeBundleO
124127
return err
125128
}
126129

127-
func matchAndMergeParametersDefinition(currentValues map[string]interface{}, cfg *mergeBundleConfig) (map[string]interface{}, error) {
130+
func matchAndMergeParametersDefinition(currentValues map[string]interface{}, cfg *MergeBundleConfig) (map[string]interface{}, error) {
128131
mergedValues := make(map[string]interface{})
129132
for k, v := range currentValues {
130133
mergedValues[k] = v

internal/commands/parameters_test.go renamed to internal/bundle/parameters_test.go

Lines changed: 26 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
package commands
1+
package bundle
22

33
import (
44
"bytes"
@@ -31,8 +31,8 @@ overridden: bar`))
3131
actual := map[string]string{
3232
"overridden": "foo",
3333
}
34-
err := withFileParameters([]string{tmpDir.Join("params.yaml")})(
35-
&mergeBundleConfig{
34+
err := WithFileParameters([]string{tmpDir.Join("params.yaml")})(
35+
&MergeBundleConfig{
3636
bundle: bundle,
3737
params: actual,
3838
})
@@ -51,8 +51,8 @@ func TestWithCommandLineParameters(t *testing.T) {
5151
"overridden": "foo",
5252
}
5353

54-
err := withCommandLineParameters([]string{"param1.param2=value1", "param3=3", "overridden=bar"})(
55-
&mergeBundleConfig{
54+
err := WithCommandLineParameters([]string{"param1.param2=value1", "param3=3", "overridden=bar"})(
55+
&MergeBundleConfig{
5656
bundle: bundle,
5757
params: actual,
5858
})
@@ -145,7 +145,7 @@ func TestWithOrchestratorParameters(t *testing.T) {
145145
for _, testCase := range testCases {
146146
t.Run(testCase.name, func(t *testing.T) {
147147
actual := map[string]string{}
148-
err := withOrchestratorParameters("kubernetes", "my-namespace")(&mergeBundleConfig{
148+
err := WithOrchestratorParameters("kubernetes", "my-namespace")(&MergeBundleConfig{
149149
bundle: testCase.bundle,
150150
params: actual,
151151
})
@@ -157,17 +157,17 @@ func TestWithOrchestratorParameters(t *testing.T) {
157157

158158
func TestMergeBundleParameters(t *testing.T) {
159159
t.Run("Override Order", func(t *testing.T) {
160-
first := func(c *mergeBundleConfig) error {
160+
first := func(c *MergeBundleConfig) error {
161161
c.params["param"] = "first"
162162
return nil
163163
}
164-
second := func(c *mergeBundleConfig) error {
164+
second := func(c *MergeBundleConfig) error {
165165
c.params["param"] = "second"
166166
return nil
167167
}
168168
bundle := prepareBundle(withParameterAndDefault("param", "string", "default"))
169169
i := &store.Installation{Claim: claim.Claim{Bundle: bundle}}
170-
err := mergeBundleParameters(i,
170+
err := MergeBundleParameters(i,
171171
first,
172172
second,
173173
)
@@ -181,7 +181,7 @@ func TestMergeBundleParameters(t *testing.T) {
181181
t.Run("Default values", func(t *testing.T) {
182182
bundle := prepareBundle(withParameterAndDefault("param", "string", "default"))
183183
i := &store.Installation{Claim: claim.Claim{Bundle: bundle}}
184-
err := mergeBundleParameters(i)
184+
err := MergeBundleParameters(i)
185185
assert.NilError(t, err)
186186
expected := map[string]interface{}{
187187
"param": "default",
@@ -190,13 +190,13 @@ func TestMergeBundleParameters(t *testing.T) {
190190
})
191191

192192
t.Run("Converting values", func(t *testing.T) {
193-
withIntValue := func(c *mergeBundleConfig) error {
193+
withIntValue := func(c *MergeBundleConfig) error {
194194
c.params["param"] = "1"
195195
return nil
196196
}
197197
bundle := prepareBundle(withParameter("param", "integer"))
198198
i := &store.Installation{Claim: claim.Claim{Bundle: bundle}}
199-
err := mergeBundleParameters(i, withIntValue)
199+
err := MergeBundleParameters(i, withIntValue)
200200
assert.NilError(t, err)
201201
expected := map[string]interface{}{
202202
"param": 1,
@@ -207,7 +207,7 @@ func TestMergeBundleParameters(t *testing.T) {
207207
t.Run("Default values", func(t *testing.T) {
208208
bundle := prepareBundle(withParameterAndDefault("param", "string", "default"))
209209
i := &store.Installation{Claim: claim.Claim{Bundle: bundle}}
210-
err := mergeBundleParameters(i)
210+
err := MergeBundleParameters(i)
211211
assert.NilError(t, err)
212212
expected := map[string]interface{}{
213213
"param": "default",
@@ -216,54 +216,54 @@ func TestMergeBundleParameters(t *testing.T) {
216216
})
217217

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

231231
t.Run("Warn on undefined parameter", func(t *testing.T) {
232-
withUndefined := func(c *mergeBundleConfig) error {
232+
withUndefined := func(c *MergeBundleConfig) error {
233233
c.params["param"] = "1"
234234
return nil
235235
}
236236
w := bytes.NewBuffer(nil)
237-
withStdErr := func(c *mergeBundleConfig) error {
237+
withStdErr := func(c *MergeBundleConfig) error {
238238
c.stderr = w
239239
return nil
240240
}
241241
bundle := prepareBundle()
242242
i := &store.Installation{Claim: claim.Claim{Bundle: bundle}}
243-
err := mergeBundleParameters(i, withUndefined, withStdErr)
243+
err := MergeBundleParameters(i, withUndefined, withStdErr)
244244
assert.NilError(t, err)
245245
assert.Equal(t, w.String(), "Warning: parameter \"param\" is not defined in the bundle\n")
246246
})
247247

248248
t.Run("Invalid type is rejected", func(t *testing.T) {
249-
withIntValue := func(c *mergeBundleConfig) error {
249+
withIntValue := func(c *MergeBundleConfig) error {
250250
c.params["param"] = "foo"
251251
return nil
252252
}
253253
bundle := prepareBundle(withParameter("param", "integer"))
254254
i := &store.Installation{Claim: claim.Claim{Bundle: bundle}}
255-
err := mergeBundleParameters(i, withIntValue)
255+
err := MergeBundleParameters(i, withIntValue)
256256
assert.ErrorContains(t, err, "invalid value for parameter")
257257
})
258258

259259
t.Run("Invalid value is rejected", func(t *testing.T) {
260-
withInvalidValue := func(c *mergeBundleConfig) error {
260+
withInvalidValue := func(c *MergeBundleConfig) error {
261261
c.params["param"] = "invalid"
262262
return nil
263263
}
264264
bundle := prepareBundle(withParameterAndValues("param", "string", []interface{}{"valid"}))
265265
i := &store.Installation{Claim: claim.Claim{Bundle: bundle}}
266-
err := mergeBundleParameters(i, withInvalidValue)
266+
err := MergeBundleParameters(i, withInvalidValue)
267267
assert.ErrorContains(t, err, "invalid value for parameter")
268268
})
269269
}
@@ -280,9 +280,9 @@ func TestLabels(t *testing.T) {
280280
labels := []string{
281281
"label=value",
282282
}
283-
op := withLabels(labels)
283+
op := WithLabels(labels)
284284

285-
config := &mergeBundleConfig{
285+
config := &MergeBundleConfig{
286286
bundle: &bundle.Bundle{
287287
Parameters: map[string]bundle.Parameter{
288288
internal.ParameterArgs: {},
@@ -301,7 +301,7 @@ func TestInvalidLabels(t *testing.T) {
301301
labels := []string{
302302
"com.docker.app.label=value",
303303
}
304-
op := withLabels(labels)
305-
err := op(&mergeBundleConfig{})
304+
op := WithLabels(labels)
305+
err := op(&MergeBundleConfig{})
306306
assert.ErrorContains(t, err, fmt.Sprintf("labels cannot start with %q", internal.Namespace))
307307
}

‎internal/commands/image/command.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ func Cmd(dockerCli command.Cli) *cobra.Command {
1717
rmCmd(),
1818
tagCmd(),
1919
inspectCmd(dockerCli),
20+
renderCmd(dockerCli),
2021
)
2122

2223
return cmd

internal/commands/render.go renamed to internal/commands/image/render.go

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,16 @@
1-
package commands
1+
package image
22

33
import (
44
"bytes"
55
"fmt"
66
"io"
77
"os"
88

9+
"github.com/spf13/pflag"
10+
911
"github.com/deislabs/cnab-go/action"
1012
"github.com/docker/app/internal"
13+
bdl "github.com/docker/app/internal/bundle"
1114
"github.com/docker/app/internal/cnab"
1215
appstore "github.com/docker/app/internal/store"
1316
"github.com/docker/cli/cli"
@@ -17,6 +20,16 @@ import (
1720
"github.com/spf13/cobra"
1821
)
1922

23+
type parametersOptions struct {
24+
ParametersFiles []string
25+
Overrides []string
26+
}
27+
28+
func (o *parametersOptions) addFlags(flags *pflag.FlagSet) {
29+
flags.StringArrayVar(&o.ParametersFiles, "parameters-file", []string{}, "Override parameters file")
30+
flags.StringArrayVarP(&o.Overrides, "set", "s", []string{}, "Override parameter value")
31+
}
32+
2033
type renderOptions struct {
2134
parametersOptions
2235
formatDriver string
@@ -86,9 +99,9 @@ func prepareCustomAction(actionName string, dockerCli command.Cli, appname strin
8699
}
87100
installation.Bundle = bundle
88101

89-
if err := mergeBundleParameters(installation,
90-
withFileParameters(paramsOpts.parametersFiles),
91-
withCommandLineParameters(paramsOpts.overrides),
102+
if err := bdl.MergeBundleParameters(installation,
103+
bdl.WithFileParameters(paramsOpts.ParametersFiles),
104+
bdl.WithCommandLineParameters(paramsOpts.Overrides),
92105
); err != nil {
93106
return nil, nil, nil, err
94107
}

‎internal/commands/root.go‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,6 @@ func addCommands(cmd *cobra.Command, dockerCli command.Cli) {
5454
removeCmd(dockerCli),
5555
listCmd(dockerCli),
5656
initCmd(dockerCli),
57-
renderCmd(dockerCli),
5857
validateCmd(),
5958
pushCmd(dockerCli),
6059
pullCmd(dockerCli),

0 commit comments

Comments
 (0)