Merge pull request #12455 from coreydaley/trello_922_set_env_vars_on_…

…build_config_new_app

Merged by openshift-bot

contrib/completions/bash/oc

@@ -10636,6 +10636,12 @@ _oc_new-app()
     local_nonpersistent_flags+=("--allow-missing-imagestream-tags")
     flags+=("--as-test")
     local_nonpersistent_flags+=("--as-test")
+    flags+=("--build-env=")
+    local_nonpersistent_flags+=("--build-env=")
+    flags+=("--build-env-file=")
+    flags_with_completion+=("--build-env-file")
+    flags_completion+=("_filedir")
+    local_nonpersistent_flags+=("--build-env-file=")
     flags+=("--code=")
     local_nonpersistent_flags+=("--code=")
     flags+=("--context-dir=")

contrib/completions/bash/openshift

@@ -15566,6 +15566,12 @@ _openshift_cli_new-app()
     local_nonpersistent_flags+=("--allow-missing-imagestream-tags")
     flags+=("--as-test")
     local_nonpersistent_flags+=("--as-test")
+    flags+=("--build-env=")
+    local_nonpersistent_flags+=("--build-env=")
+    flags+=("--build-env-file=")
+    flags_with_completion+=("--build-env-file")
+    flags_completion+=("_filedir")
+    local_nonpersistent_flags+=("--build-env-file=")
     flags+=("--code=")
     local_nonpersistent_flags+=("--code=")
     flags+=("--context-dir=")

contrib/completions/zsh/oc

@@ -10784,6 +10784,12 @@ _oc_new-app()
     local_nonpersistent_flags+=("--allow-missing-imagestream-tags")
     flags+=("--as-test")
     local_nonpersistent_flags+=("--as-test")
+    flags+=("--build-env=")
+    local_nonpersistent_flags+=("--build-env=")
+    flags+=("--build-env-file=")
+    flags_with_completion+=("--build-env-file")
+    flags_completion+=("_filedir")
+    local_nonpersistent_flags+=("--build-env-file=")
     flags+=("--code=")
     local_nonpersistent_flags+=("--code=")
     flags+=("--context-dir=")

contrib/completions/zsh/openshift

@@ -15714,6 +15714,12 @@ _openshift_cli_new-app()
     local_nonpersistent_flags+=("--allow-missing-imagestream-tags")
     flags+=("--as-test")
     local_nonpersistent_flags+=("--as-test")
+    flags+=("--build-env=")
+    local_nonpersistent_flags+=("--build-env=")
+    flags+=("--build-env-file=")
+    flags_with_completion+=("--build-env-file")
+    flags_completion+=("_filedir")
+    local_nonpersistent_flags+=("--build-env-file=")
     flags+=("--code=")
     local_nonpersistent_flags+=("--code=")
     flags+=("--context-dir=")

docs/man/man1/oc-new-app.1

@@ -38,6 +38,14 @@ If you provide source code, a new build will be automatically triggered. You can
 \fB\-\-as\-test\fP=false
     If true create this application as a test deployment, which validates that the deployment succeeds and then scales down.
 
+.PP
+\fB\-\-build\-env\fP=[]
+    Specify a key\-value pair for an environment variable to set into each build image.
+
+.PP
+\fB\-\-build\-env\-file\fP=[]
+    File containing key\-value pairs of environment variables to set into each build image.
+
 .PP
 \fB\-\-code\fP=[]
     Source code to use to build this application.

docs/man/man1/openshift-cli-new-app.1

@@ -38,6 +38,14 @@ If you provide source code, a new build will be automatically triggered. You can
 \fB\-\-as\-test\fP=false
     If true create this application as a test deployment, which validates that the deployment succeeds and then scales down.
 
+.PP
+\fB\-\-build\-env\fP=[]
+    Specify a key\-value pair for an environment variable to set into each build image.
+
+.PP
+\fB\-\-build\-env\-file\fP=[]
+    File containing key\-value pairs of environment variables to set into each build image.
+
 .PP
 \fB\-\-code\fP=[]
     Source code to use to build this application.

pkg/cmd/cli/cmd/newapp.go

@@ -171,6 +171,9 @@ func NewCmdNewApplication(name, baseName string, f *clientcmd.Factory, in io.Rea
 	cmd.Flags().StringArrayVarP(&config.Environment, "env", "e", config.Environment, "Specify a key-value pair for an environment variable to set into each container.")
 	cmd.Flags().StringArrayVar(&config.EnvironmentFiles, "env-file", config.EnvironmentFiles, "File containing key-value pairs of environment variables to set into each container.")
 	cmd.MarkFlagFilename("env-file")
+	cmd.Flags().StringArrayVar(&config.BuildEnvironment, "build-env", config.BuildEnvironment, "Specify a key-value pair for an environment variable to set into each build image.")
+	cmd.Flags().StringArrayVar(&config.BuildEnvironmentFiles, "build-env-file", config.BuildEnvironmentFiles, "File containing key-value pairs of environment variables to set into each build image.")
+	cmd.MarkFlagFilename("build-env-file")
 	cmd.Flags().StringVar(&config.Name, "name", "", "Set name to use for generated application artifacts")
 	cmd.Flags().Var(&config.Strategy, "strategy", "Specify the build strategy to use if you don't want to detect (docker|pipeline|source).")
 	cmd.Flags().StringP("labels", "l", "", "Label to set in all resources for this application.")
@@ -214,6 +217,7 @@ func (o *NewAppOptions) Complete(baseName, commandName string, f *clientcmd.Fact
 	o.Config.DryRun = o.Action.DryRun
 
 	cmdutil.WarnAboutCommaSeparation(o.ErrOut, o.Config.Environment, "--env")
+	cmdutil.WarnAboutCommaSeparation(o.ErrOut, o.Config.BuildEnvironment, "--build-env")
 	cmdutil.WarnAboutCommaSeparation(o.ErrOut, o.Config.TemplateParameters, "--param")
 
 	o.CommandPath = c.CommandPath()

pkg/cmd/cli/cmd/newapp_test.go

@@ -66,6 +66,10 @@ func TestNewAppDefaultFlags(t *testing.T) {
 			flagName:   "env",
 			defaultVal: "[" + strings.Join(config.Environment, ",") + "]",
 		},
+		"build-env": {
+			flagName:   "build-env",
+			defaultVal: "[" + strings.Join(config.BuildEnvironment, ",") + "]",
+		},
 		"name": {
 			flagName:   "name",
 			defaultVal: config.Name,
@@ -176,6 +180,18 @@ func TestNewAppRunFailure(t *testing.T) {
 			},
 			expectedErr: "--search can't be used with --env",
 		},
+		"search_with_build_env": {
+			config: &newcmd.AppConfig{
+				AsSearch: true,
+				ComponentInputs: newcmd.ComponentInputs{
+					Components: []string{"mysql"},
+				},
+				GenerationInputs: newcmd.GenerationInputs{
+					BuildEnvironment: []string{"FOO=BAR"},
+				},
+			},
+			expectedErr: "--search can't be used with --build-env",
+		},
 		"search_with_param": {
 			config: &newcmd.AppConfig{
 				AsSearch: true,

pkg/cmd/cli/cmd/newbuild.go

@@ -101,7 +101,6 @@ type NewBuildOptions struct {
 func NewCmdNewBuild(name, baseName string, f *clientcmd.Factory, in io.Reader, out, errout io.Writer) *cobra.Command {
 	config := newcmd.NewAppConfig()
 	config.ExpectToBuild = true
-	config.AddEnvironmentToBuild = true
 	o := &NewBuildOptions{Config: config}
 
 	cmd := &cobra.Command{
@@ -129,8 +128,13 @@ func NewCmdNewBuild(name, baseName string, f *clientcmd.Factory, in io.Reader, o
 	cmd.Flags().StringVar(&config.Name, "name", "", "Set name to use for generated build artifacts.")
 	cmd.Flags().StringVar(&config.To, "to", "", "Push built images to this image stream tag (or Docker image repository if --to-docker is set).")
 	cmd.Flags().BoolVar(&config.OutputDocker, "to-docker", false, "If true, have the build output push to a Docker repository.")
-	cmd.Flags().StringArrayVarP(&config.Environment, "env", "e", config.Environment, "Specify a key-value pair for an environment variable to set into resulting image.")
-	cmd.Flags().StringArrayVar(&config.EnvironmentFiles, "env-file", config.EnvironmentFiles, "File containing key-value pairs of environment variables to set into each container.")
+	cmd.Flags().StringArrayVar(&config.BuildEnvironment, "build-env", config.BuildEnvironment, "Specify a key-value pair for an environment variable to set into resulting image.")
+	cmd.Flags().MarkHidden("build-env")
+	cmd.Flags().StringArrayVarP(&config.BuildEnvironment, "env", "e", config.BuildEnvironment, "Specify a key-value pair for an environment variable to set into resulting image.")
+	cmd.Flags().StringArrayVar(&config.BuildEnvironmentFiles, "build-env-file", config.BuildEnvironmentFiles, "File containing key-value pairs of environment variables to set into each container.")
+	cmd.MarkFlagFilename("build-env-file")
+	cmd.Flags().MarkHidden("build-env-file")
+	cmd.Flags().StringArrayVar(&config.BuildEnvironmentFiles, "env-file", config.BuildEnvironmentFiles, "File containing key-value pairs of environment variables to set into each container.")
 	cmd.MarkFlagFilename("env-file")
 	cmd.Flags().Var(&config.Strategy, "strategy", "Specify the build strategy to use if you don't want to detect (docker|pipeline|source).")
 	cmd.Flags().StringVarP(&config.Dockerfile, "dockerfile", "D", "", "Specify the contents of a Dockerfile to build directly, implies --strategy=docker. Pass '-' to read from STDIN.")
@@ -180,6 +184,7 @@ func (o *NewBuildOptions) Complete(baseName, commandName string, f *clientcmd.Fa
 	o.CommandName = commandName
 
 	cmdutil.WarnAboutCommaSeparation(o.ErrOut, o.Config.Environment, "--env")
+	cmdutil.WarnAboutCommaSeparation(o.ErrOut, o.Config.BuildEnvironment, "--build-env")
 
 	mapper, _ := f.Object()
 	o.PrintObject = cmdutil.VersionedPrintObject(f.PrintObject, c, mapper, out)

pkg/generate/app/cmd/newapp.go

@@ -52,12 +52,12 @@ const (
 type GenerationInputs struct {
 	TemplateParameters []string
 	Environment        []string
+	BuildEnvironment   []string
 	Labels             map[string]string
 
 	TemplateParameterFiles []string
 	EnvironmentFiles       []string
-
-	AddEnvironmentToBuild bool
+	BuildEnvironmentFiles  []string
 
 	InsecureRegistry bool
 
@@ -282,7 +282,7 @@ func validateOutputImageReference(ref string) error {
 // buildPipelines converts a set of resolved, valid references into pipelines.
 func (c *AppConfig) buildPipelines(components app.ComponentReferences, environment app.Environment) (app.PipelineGroup, error) {
 	pipelines := app.PipelineGroup{}
-	pipelineBuilder := app.NewPipelineBuilder(c.Name, c.GetBuildEnvironment(environment), c.OutputDocker).To(c.To)
+	pipelineBuilder := app.NewPipelineBuilder(c.Name, c.GetBuildEnvironment(), c.OutputDocker).To(c.To)
 	for _, group := range components.Group() {
 		glog.V(4).Infof("found group: %v", group)
 		common := app.PipelineGroup{}
@@ -503,7 +503,7 @@ func (c *AppConfig) installComponents(components app.ComponentReferences, env ap
 
 // RunQuery executes the provided config and returns the result of the resolution.
 func (c *AppConfig) RunQuery() (*QueryResult, error) {
-	environment, parameters, err := c.validate()
+	environment, buildEnvironment, parameters, err := c.validate()
 	if err != nil {
 		return nil, err
 	}
@@ -539,6 +539,9 @@ func (c *AppConfig) RunQuery() (*QueryResult, error) {
 	if len(environment) > 0 {
 		errs = append(errs, errors.New("--search can't be used with --env"))
 	}
+	if len(buildEnvironment) > 0 {
+		errs = append(errs, errors.New("--search can't be used with --build-env"))
+	}
 	if len(parameters) > 0 {
 		errs = append(errs, errors.New("--search can't be used with --param"))
 	}
@@ -576,7 +579,7 @@ func (c *AppConfig) RunQuery() (*QueryResult, error) {
 	}, nil
 }
 
-func (c *AppConfig) validate() (app.Environment, app.Environment, error) {
+func (c *AppConfig) validate() (app.Environment, app.Environment, app.Environment, error) {
 	env, err := app.ParseAndCombineEnvironment(c.Environment, c.EnvironmentFiles, c.In, func(key, file string) error {
 		if file == "" {
 			fmt.Fprintf(c.ErrOut, "warning: Environment variable %q was overwritten\n", key)
@@ -586,7 +589,18 @@ func (c *AppConfig) validate() (app.Environment, app.Environment, error) {
 		return nil
 	})
 	if err != nil {
-		return nil, nil, err
+		return nil, nil, nil, err
+	}
+	buildEnv, err := app.ParseAndCombineEnvironment(c.BuildEnvironment, c.BuildEnvironmentFiles, c.In, func(key, file string) error {
+		if file == "" {
+			fmt.Fprintf(c.ErrOut, "warning: Build Environment variable %q was overwritten\n", key)
+		} else {
+			fmt.Fprintf(c.ErrOut, "warning: Build Environment variable %q already defined, ignoring value from file %q\n", key, file)
+		}
+		return nil
+	})
+	if err != nil {
+		return nil, nil, nil, err
 	}
 	params, err := app.ParseAndCombineEnvironment(c.TemplateParameters, c.TemplateParameterFiles, c.In, func(key, file string) error {
 		if file == "" {
@@ -597,14 +611,14 @@ func (c *AppConfig) validate() (app.Environment, app.Environment, error) {
 		return nil
 	})
 	if err != nil {
-		return nil, nil, err
+		return nil, nil, nil, err
 	}
-	return env, params, nil
+	return env, buildEnv, params, nil
 }
 
 // Run executes the provided config to generate objects.
 func (c *AppConfig) Run() (*AppResult, error) {
-	env, parameters, err := c.validate()
+	env, _, parameters, err := c.validate()
 	if err != nil {
 		return nil, err
 	}
@@ -921,11 +935,10 @@ func (c *AppConfig) HasArguments() bool {
 		len(c.TemplateFiles) > 0
 }
 
-func (c *AppConfig) GetBuildEnvironment(environment app.Environment) app.Environment {
-	if c.AddEnvironmentToBuild {
-		return environment
-	}
-	return app.Environment{}
+func (c *AppConfig) GetBuildEnvironment() app.Environment {
+	_, buildEnv, _, _ := c.validate()
+	return buildEnv
+
 }
 
 func optionallyValidateExposedPorts(config *AppConfig, repositories app.SourceRepositories) error {

pkg/generate/app/cmd/newapp_test.go

@@ -31,6 +31,7 @@ func TestValidate(t *testing.T) {
 		componentValues     []string
 		sourceRepoLocations []string
 		env                 map[string]string
+		buildEnv            map[string]string
 		parms               map[string]string
 	}{
 		"components": {
@@ -42,6 +43,7 @@ func TestValidate(t *testing.T) {
 			componentValues:     []string{"one", "two", "three/four"},
 			sourceRepoLocations: []string{},
 			env:                 map[string]string{},
+			buildEnv:            map[string]string{},
 			parms:               map[string]string{},
 		},
 		"envs": {
@@ -53,6 +55,19 @@ func TestValidate(t *testing.T) {
 			componentValues:     []string{},
 			sourceRepoLocations: []string{},
 			env:                 map[string]string{"one": "first", "two": "second", "three": "third"},
+			buildEnv:            map[string]string{},
+			parms:               map[string]string{},
+		},
+		"build-envs": {
+			cfg: AppConfig{
+				GenerationInputs: GenerationInputs{
+					BuildEnvironment: []string{"one=first", "two=second", "three=third"},
+				},
+			},
+			componentValues:     []string{},
+			sourceRepoLocations: []string{},
+			env:                 map[string]string{},
+			buildEnv:            map[string]string{"one": "first", "two": "second", "three": "third"},
 			parms:               map[string]string{},
 		},
 		"component+source": {
@@ -64,6 +79,7 @@ func TestValidate(t *testing.T) {
 			componentValues:     []string{"one"},
 			sourceRepoLocations: []string{"https://server/repo.git"},
 			env:                 map[string]string{},
+			buildEnv:            map[string]string{},
 			parms:               map[string]string{},
 		},
 		"components+source": {
@@ -75,6 +91,7 @@ func TestValidate(t *testing.T) {
 			componentValues:     []string{"mysql", "ruby"},
 			sourceRepoLocations: []string{"git://github.com/namespace/repo.git"},
 			env:                 map[string]string{},
+			buildEnv:            map[string]string{},
 			parms:               map[string]string{},
 		},
 		"components+parms": {
@@ -89,15 +106,13 @@ func TestValidate(t *testing.T) {
 			componentValues:     []string{"ruby-helloworld-sample"},
 			sourceRepoLocations: []string{},
 			env:                 map[string]string{},
-			parms: map[string]string{
-				"one": "first",
-				"two": "second",
-			},
+			buildEnv:            map[string]string{},
+			parms:               map[string]string{"one": "first", "two": "second"},
 		},
 	}
 	for n, c := range tests {
 		b := &app.ReferenceBuilder{}
-		env, parms, err := c.cfg.validate()
+		env, buildEnv, parms, err := c.cfg.validate()
 		if err != nil {
 			t.Errorf("%s: Unexpected error: %v", n, err)
 			continue
@@ -129,6 +144,15 @@ func TestValidate(t *testing.T) {
 				break
 			}
 		}
+		if len(buildEnv) != len(c.buildEnv) {
+			t.Errorf("%s: Environment variables don't match. Expected: %v, Got: %v", n, c.buildEnv, buildEnv)
+		}
+		for e, v := range buildEnv {
+			if c.buildEnv[e] != v {
+				t.Errorf("%s: Environment variables don't match. Expected: %v, Got: %v", n, c.buildEnv, buildEnv)
+				break
+			}
+		}
 		if len(parms) != len(c.parms) {
 			t.Errorf("%s: Template parameters don't match. Expected: %v, Got: %v", n, c.parms, parms)
 		}
@@ -165,7 +189,7 @@ func TestBuildTemplates(t *testing.T) {
 			appCfg.TemplateParameters = append(appCfg.TemplateParameters, fmt.Sprintf("%v=%v", k, v))
 		}
 
-		_, parms, err := appCfg.validate()
+		_, _, parms, err := appCfg.validate()
 		if err != nil {
 			t.Errorf("%s: Unexpected error: %v", n, err)
 			continue