Skip to content

Commit c0016d4

Browse files
committed
oc new-app --build-env doesn't work on templates
Fixes #14906
1 parent c7d9fb8 commit c0016d4

File tree

3 files changed

+43
-3
lines changed

3 files changed

+43
-3
lines changed

pkg/build/util/util.go

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -349,6 +349,22 @@ func SafeForLoggingS2IConfig(config *s2iapi.Config) *s2iapi.Config {
349349
return &newConfig
350350
}
351351

352+
// GetBuildConfigEnv gets the buildconfig strategy environment
353+
func GetBuildConfigEnv(buildConfig *buildapi.BuildConfig) []kapi.EnvVar {
354+
switch {
355+
case buildConfig.Spec.Strategy.SourceStrategy != nil:
356+
return buildConfig.Spec.Strategy.SourceStrategy.Env
357+
case buildConfig.Spec.Strategy.DockerStrategy != nil:
358+
return buildConfig.Spec.Strategy.DockerStrategy.Env
359+
case buildConfig.Spec.Strategy.CustomStrategy != nil:
360+
return buildConfig.Spec.Strategy.CustomStrategy.Env
361+
case buildConfig.Spec.Strategy.JenkinsPipelineStrategy != nil:
362+
return buildConfig.Spec.Strategy.JenkinsPipelineStrategy.Env
363+
default:
364+
return nil
365+
}
366+
}
367+
352368
// GetBuildEnv gets the build strategy environment
353369
func GetBuildEnv(build *buildapi.Build) []kapi.EnvVar {
354370
switch {
@@ -365,6 +381,23 @@ func GetBuildEnv(build *buildapi.Build) []kapi.EnvVar {
365381
}
366382
}
367383

384+
// SetBuildConfigEnv replaces the current buildconfig environment
385+
func SetBuildConfigEnv(buildConfig *buildapi.BuildConfig, env []kapi.EnvVar) {
386+
var oldEnv *[]kapi.EnvVar
387+
388+
switch {
389+
case buildConfig.Spec.Strategy.SourceStrategy != nil:
390+
oldEnv = &buildConfig.Spec.Strategy.SourceStrategy.Env
391+
case buildConfig.Spec.Strategy.DockerStrategy != nil:
392+
oldEnv = &buildConfig.Spec.Strategy.DockerStrategy.Env
393+
case buildConfig.Spec.Strategy.CustomStrategy != nil:
394+
oldEnv = &buildConfig.Spec.Strategy.CustomStrategy.Env
395+
case buildConfig.Spec.Strategy.JenkinsPipelineStrategy != nil:
396+
oldEnv = &buildConfig.Spec.Strategy.JenkinsPipelineStrategy.Env
397+
}
398+
*oldEnv = env
399+
}
400+
368401
// SetBuildEnv replaces the current build environment
369402
func SetBuildEnv(build *buildapi.Build, env []kapi.EnvVar) {
370403
var oldEnv *[]kapi.EnvVar
@@ -380,7 +413,6 @@ func SetBuildEnv(build *buildapi.Build, env []kapi.EnvVar) {
380413
oldEnv = &build.Spec.Strategy.JenkinsPipelineStrategy.Env
381414
}
382415
*oldEnv = env
383-
384416
}
385417

386418
// UpdateBuildEnv updates the strategy environment

pkg/generate/app/cmd/newapp.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -466,7 +466,7 @@ func (c *AppConfig) buildPipelines(components app.ComponentReferences, environme
466466
}
467467

468468
// buildTemplates converts a set of resolved, valid references into references to template objects.
469-
func (c *AppConfig) buildTemplates(components app.ComponentReferences, parameters app.Environment, environment app.Environment) (string, []runtime.Object, error) {
469+
func (c *AppConfig) buildTemplates(components app.ComponentReferences, parameters app.Environment, environment app.Environment, buildEnvironment app.Environment) (string, []runtime.Object, error) {
470470
objects := []runtime.Object{}
471471
name := ""
472472
for _, ref := range components {
@@ -485,6 +485,12 @@ func (c *AppConfig) buildTemplates(components app.ComponentReferences, parameter
485485
// if environment variables were passed in, let's apply the environment variables
486486
// to every pod template object
487487
for i := range result.Objects {
488+
switch result.Objects[i].(type) {
489+
case *buildapi.BuildConfig:
490+
buildEnv := buildutil.GetBuildConfigEnv(result.Objects[i].(*buildapi.BuildConfig))
491+
buildEnv = app.JoinEnvironment(buildEnv, buildEnvironment.List())
492+
buildutil.SetBuildConfigEnv(result.Objects[i].(*buildapi.BuildConfig), buildEnv)
493+
}
488494
podSpec, _, err := ometa.GetPodSpec(result.Objects[i])
489495
if err == nil {
490496
for ii := range podSpec.Containers {
@@ -805,7 +811,7 @@ func (c *AppConfig) Run() (*AppResult, error) {
805811

806812
objects = app.AddServices(objects, false)
807813

808-
templateName, templateObjects, err := c.buildTemplates(components.TemplateComponentRefs(), parameters, env)
814+
templateName, templateObjects, err := c.buildTemplates(components.TemplateComponentRefs(), parameters, env, buildenv)
809815
if err != nil {
810816
return nil, err
811817
}

test/cmd/newapp.sh

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,8 @@ os::cmd::expect_success_and_text 'oc new-app -f test/testdata/template-with-name
7171
# non-string parameterized values should be stripped
7272
os::cmd::expect_failure_and_text 'oc new-app -f test/testdata/template-with-namespaces.json -o jsonpath="{.items[?(@.metadata.name==\"route-edge-refstripped\")].metadata.namespace}"' 'namespace is not found'
7373
os::cmd::expect_failure_and_text 'oc new-app -f test/testdata/template-with-namespaces.json -o jsonpath="{.items[?(@.metadata.name==\"route-edge-prefix-refstripped\")].metadata.namespace}"' 'namespace is not found'
74+
# ensure --build-env environment variables get added to the buildconfig
75+
os::cmd::expect_success_and_text 'oc new-app -f test/testdata/template-with-app-label.json --build-env FOO=bar -o yaml' 'FOO'
7476
# ensure the objects can actually get created with a namespace specified
7577
project=$(oc project -q)
7678
os::cmd::expect_success 'oc new-project template-substitute'

0 commit comments

Comments
 (0)