refactor: consistent env var management in traits

pkg/client/cmd/install.go

@@ -19,10 +19,10 @@ package cmd
 
 import (
 	"fmt"
-	"github.com/operator-framework/operator-sdk/pkg/k8sclient"
 	"time"
 
 	"github.com/apache/camel-k/pkg/install"
+	"github.com/operator-framework/operator-sdk/pkg/k8sclient"
 	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
 	k8serrors "k8s.io/apimachinery/pkg/api/errors"

pkg/client/cmd/root.go

@@ -19,6 +19,7 @@ package cmd
 
 import (
 	"context"
+
 	"github.com/apache/camel-k/pkg/util/kubernetes"
 	"github.com/pkg/errors"
 	"github.com/spf13/cobra"

pkg/trait/builder.go

@@ -18,12 +18,13 @@ limitations under the License.
 package trait
 
 import (
+	"regexp"
+
 	"github.com/apache/camel-k/pkg/apis/camel/v1alpha1"
 	"github.com/apache/camel-k/pkg/builder"
 	"github.com/apache/camel-k/pkg/builder/kaniko"
 	"github.com/apache/camel-k/pkg/builder/s2i"
 	"github.com/apache/camel-k/pkg/platform"
-	"regexp"
 )
 
 const (
@@ -103,5 +104,4 @@ func (t *builderTrait) ReplaceHost(ctx *builder.Context) error {
 func getImageWithOpenShiftHost(image string) string {
 	pattern := regexp.MustCompile(`^[0-9]+\.[0-9]+\.[0-9]+\.[0-9]+([:/].*)`)
 	return pattern.ReplaceAllString(image, openshiftDockerRegistryHost+"$1")
-	return image
 }

pkg/trait/builder_test.go

@@ -20,6 +20,8 @@ package trait
 import (
 	"testing"
 
+	"k8s.io/api/core/v1"
+
 	"github.com/apache/camel-k/pkg/builder"
 
 	"github.com/apache/camel-k/pkg/util/kubernetes"
@@ -136,7 +138,7 @@ func createBuilderTestEnv(cluster v1alpha1.IntegrationPlatformCluster, strategy
 				},
 			},
 		},
-		EnvVars:        make(map[string]string),
+		EnvVars:        make([]v1.EnvVar, 0),
 		ExecutedTraits: make([]Trait, 0),
 		Resources:      kubernetes.NewCollection(),
 	}
@@ -150,4 +152,4 @@ func TestIPReplacement(t *testing.T) {
 	assert.Equal(t, "10.0.2.3.4/camel-k", getImageWithOpenShiftHost("10.0.2.3.4/camel-k"))
 	assert.Equal(t, "gcr.io/camel-k/camel-k:latest", getImageWithOpenShiftHost("gcr.io/camel-k/camel-k:latest"))
 	assert.Equal(t, "docker.io/camel-k:latest", getImageWithOpenShiftHost("docker.io/camel-k:latest"))
-}
+}

pkg/trait/catalog.go

@@ -86,8 +86,8 @@ func (c *Catalog) traitsFor(environment *Environment) []Trait {
 			c.tDependencies,
 			c.tBuilder,
 			c.tSpringBoot,
-			c.tDeployment,
 			c.tEnvironment,
+			c.tDeployment,
 			c.tService,
 			c.tRoute,
 			c.tOwner,
@@ -98,8 +98,8 @@ func (c *Catalog) traitsFor(environment *Environment) []Trait {
 			c.tDependencies,
 			c.tBuilder,
 			c.tSpringBoot,
-			c.tDeployment,
 			c.tEnvironment,
+			c.tDeployment,
 			c.tService,
 			c.tIngress,
 			c.tOwner,
@@ -110,9 +110,9 @@ func (c *Catalog) traitsFor(environment *Environment) []Trait {
 			c.tDependencies,
 			c.tBuilder,
 			c.tSpringBoot,
+			c.tEnvironment,
 			c.tKnative,
 			c.tDeployment,
-			c.tEnvironment,
 			c.tIstio,
 			c.tOwner,
 		}

pkg/trait/debug.go

@@ -19,6 +19,7 @@ package trait
 
 import (
 	"github.com/apache/camel-k/pkg/apis/camel/v1alpha1"
+	"github.com/apache/camel-k/pkg/util/envvar"
 )
 
 type debugTrait struct {
@@ -43,7 +44,7 @@ func (t *debugTrait) Configure(e *Environment) (bool, error) {
 
 func (t *debugTrait) Apply(e *Environment) error {
 	// this is all that's needed as long as the base image is `fabric8/s2i-java` look into builder/builder.go
-	e.EnvVars["JAVA_DEBUG"] = True
+	envvar.SetVal(&e.EnvVars, "JAVA_DEBUG", True)
 
 	return nil
 }

pkg/trait/debug_test.go

@@ -20,6 +20,10 @@ package trait
 import (
 	"testing"
 
+	"github.com/apache/camel-k/pkg/util/envvar"
+
+	"k8s.io/api/core/v1"
+
 	"github.com/apache/camel-k/pkg/apis/camel/v1alpha1"
 	"github.com/stretchr/testify/assert"
 )
@@ -40,7 +44,8 @@ func TestDebugTraitApplicability(t *testing.T) {
 				},
 			},
 		},
-		EnvVars: make(map[string]string)}
+		EnvVars: make([]v1.EnvVar, 0),
+	}
 
 	trait := newDebugTrait()
 
@@ -56,7 +61,7 @@ func TestDebugTraitApplicability(t *testing.T) {
 }
 
 func TestApplyDebugTrait(t *testing.T) {
-	env := Environment{
+	environment := Environment{
 		Integration: &v1alpha1.Integration{
 			Status: v1alpha1.IntegrationStatus{
 				Phase: v1alpha1.IntegrationPhaseDeploying,
@@ -71,10 +76,12 @@ func TestApplyDebugTrait(t *testing.T) {
 				},
 			},
 		},
-		EnvVars: make(map[string]string)}
+		EnvVars: make([]v1.EnvVar, 0),
+	}
 
 	trait := newDebugTrait()
 
-	assert.Nil(t, trait.Apply(&env))
-	assert.Equal(t, True, env.EnvVars["JAVA_DEBUG"])
+	assert.Nil(t, trait.Apply(&environment))
+	assert.NotNil(t, envvar.Get(environment.EnvVars, "JAVA_DEBUG"))
+	assert.Equal(t, True, envvar.Get(environment.EnvVars, "JAVA_DEBUG").Value)
 }

pkg/trait/deployment.go

@@ -23,6 +23,8 @@ import (
 	"strconv"
 	"strings"
 
+	"github.com/apache/camel-k/pkg/util/envvar"
+
 	"github.com/apache/camel-k/pkg/apis/camel/v1alpha1"
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
@@ -96,7 +98,11 @@ func (t *deploymentTrait) getConfigMapsFor(e *Environment) []runtime.Object {
 
 	// combine properties of integration with context, integration
 	// properties have the priority
-	properties := CombineConfigurationAsMap("property", e.Context, e.Integration)
+	properties := ""
+
+	VisitKeyValConfigurations("property", e.Context, e.Integration, func(key string, val string) {
+		properties += fmt.Sprintf("%s=%s\n", key, val)
+	})
 
 	maps = append(
 		maps,
@@ -113,7 +119,7 @@ func (t *deploymentTrait) getConfigMapsFor(e *Environment) []runtime.Object {
 				},
 			},
 			Data: map[string]string{
-				"properties": PropertiesString(properties),
+				"properties": properties,
 			},
 		},
 	)
@@ -194,28 +200,32 @@ func (t *deploymentTrait) getSources(e *Environment) []string {
 func (t *deploymentTrait) getDeploymentFor(e *Environment) *appsv1.Deployment {
 	sources := t.getSources(e)
 
+	environment := make([]corev1.EnvVar, 0)
+
 	// combine Environment of integration with context, integration
 	// Environment has the priority
-	environment := CombineConfigurationAsMap("env", e.Context, e.Integration)
+	VisitKeyValConfigurations("env", e.Context, e.Integration, func(key string, value string) {
+		envvar.SetVal(&environment, key, value)
+	})
 
 	// set env vars needed by the runtime
-	environment["JAVA_MAIN_CLASS"] = "org.apache.camel.k.jvm.Application"
+	envvar.SetVal(&environment, "JAVA_MAIN_CLASS", "org.apache.camel.k.jvm.Application")
 
 	// camel-k runtime
-	environment["CAMEL_K_ROUTES"] = strings.Join(sources, ",")
-	environment["CAMEL_K_CONF"] = "/etc/camel/conf/application.properties"
-	environment["CAMEL_K_CONF_D"] = "/etc/camel/conf.d"
+	envvar.SetVal(&environment, "CAMEL_K_ROUTES", strings.Join(sources, ","))
+	envvar.SetVal(&environment, "CAMEL_K_CONF", "/etc/camel/conf/application.properties")
+	envvar.SetVal(&environment, "CAMEL_K_CONF_D", "/etc/camel/conf.d")
 
 	// add a dummy env var to trigger deployment if everything but the code
 	// has been changed
-	environment["CAMEL_K_DIGEST"] = e.Integration.Status.Digest
+	envvar.SetVal(&environment, "CAMEL_K_DIGEST", e.Integration.Status.Digest)
 
 	// optimizations
-	environment["AB_JOLOKIA_OFF"] = True
+	envvar.SetVal(&environment, "AB_JOLOKIA_OFF", True)
 
 	// add env vars from traits
-	for k, v := range e.EnvVars {
-		environment[k] = v
+	for _, envVar := range e.EnvVars {
+		envvar.SetVar(&environment, envVar)
 	}
 
 	labels := map[string]string{
@@ -248,7 +258,7 @@ func (t *deploymentTrait) getDeploymentFor(e *Environment) *appsv1.Deployment {
 						{
 							Name:  e.Integration.Name,
 							Image: e.Integration.Status.Image,
-							Env:   EnvironmentAsEnvVarSlice(environment),
+							Env:   environment,
 						},
 					},
 				},
@@ -329,8 +339,7 @@ func (t *deploymentTrait) getDeploymentFor(e *Environment) *appsv1.Deployment {
 	// Volumes :: Additional ConfigMaps
 	//
 
-	cmList := CombineConfigurationAsSlice("configmap", e.Context, e.Integration)
-	for _, cmName := range cmList {
+	VisitConfigurations("configmap", e.Context, e.Integration, func(cmName string) {
 		cnt++
 
 		vols = append(vols, corev1.Volume{
@@ -348,14 +357,13 @@ func (t *deploymentTrait) getDeploymentFor(e *Environment) *appsv1.Deployment {
 			Name:      "integration-cm-" + cmName,
 			MountPath: fmt.Sprintf("/etc/camel/conf.d/%03d_%s", cnt, cmName),
 		})
-	}
+	})
 
 	//
 	// Volumes :: Additional Secrets
 	//
 
-	secretList := CombineConfigurationAsSlice("secret", e.Context, e.Integration)
-	for _, secretName := range secretList {
+	VisitConfigurations("secret", e.Context, e.Integration, func(secretName string) {
 		cnt++
 
 		vols = append(vols, corev1.Volume{
@@ -371,7 +379,7 @@ func (t *deploymentTrait) getDeploymentFor(e *Environment) *appsv1.Deployment {
 			Name:      "integration-secret-" + secretName,
 			MountPath: fmt.Sprintf("/etc/camel/conf.d/%03d_%s", cnt, secretName),
 		})
-	}
+	})
 
 	//
 	// Volumes

pkg/trait/environment.go

@@ -19,9 +19,8 @@ package trait
 
 import (
 	"github.com/apache/camel-k/pkg/apis/camel/v1alpha1"
+	"github.com/apache/camel-k/pkg/util/envvar"
 	"github.com/apache/camel-k/version"
-	appsv1 "k8s.io/api/apps/v1"
-	"k8s.io/api/core/v1"
 )
 
 type environmentTrait struct {
@@ -53,35 +52,12 @@ func (t *environmentTrait) Configure(e *Environment) (bool, error) {
 }
 
 func (t *environmentTrait) Apply(e *Environment) error {
-	e.Resources.VisitDeployment(func(deployment *appsv1.Deployment) {
-		for i := 0; i < len(deployment.Spec.Template.Spec.Containers); i++ {
-			c := &deployment.Spec.Template.Spec.Containers[i]
+	envvar.SetVal(&e.EnvVars, envVarCamelKVersion, version.Version)
 
-			c.Env = append(c.Env, v1.EnvVar{
-				Name:  envVarCamelKVersion,
-				Value: version.Version,
-			})
-
-			if t.ContainerMeta {
-				c.Env = append(c.Env, v1.EnvVar{
-					Name: envVarNamespace,
-					ValueFrom: &v1.EnvVarSource{
-						FieldRef: &v1.ObjectFieldSelector{
-							FieldPath: "metadata.namespace",
-						},
-					},
-				})
-				c.Env = append(c.Env, v1.EnvVar{
-					Name: envVarPodName,
-					ValueFrom: &v1.EnvVarSource{
-						FieldRef: &v1.ObjectFieldSelector{
-							FieldPath: "metadata.name",
-						},
-					},
-				})
-			}
-		}
-	})
+	if t.ContainerMeta {
+		envvar.SetValFrom(&e.EnvVars, envVarNamespace, "metadata.namespace")
+		envvar.SetValFrom(&e.EnvVars, envVarPodName, "metadata.name")
+	}
 
 	return nil
 }

pkg/trait/environment_test.go

@@ -20,6 +20,8 @@ package trait
 import (
 	"testing"
 
+	corev1 "k8s.io/api/core/v1"
+
 	"github.com/apache/camel-k/pkg/util/kubernetes"
 
 	"k8s.io/api/apps/v1"
@@ -44,7 +46,7 @@ func TestDefaultEnvironment(t *testing.T) {
 				Cluster: v1alpha1.IntegrationPlatformClusterOpenShift,
 			},
 		},
-		EnvVars:        make(map[string]string),
+		EnvVars:        make([]corev1.EnvVar, 0),
 		ExecutedTraits: make([]Trait, 0),
 		Resources:      kubernetes.NewCollection(),
 	}
@@ -99,7 +101,7 @@ func TestEnabledContainerMetaDataEnvVars(t *testing.T) {
 				Cluster: v1alpha1.IntegrationPlatformClusterOpenShift,
 			},
 		},
-		EnvVars:        make(map[string]string),
+		EnvVars:        make([]corev1.EnvVar, 0),
 		ExecutedTraits: make([]Trait, 0),
 		Resources:      kubernetes.NewCollection(),
 	}
@@ -154,7 +156,7 @@ func TestDisabledContainerMetaDataEnvVars(t *testing.T) {
 				Cluster: v1alpha1.IntegrationPlatformClusterOpenShift,
 			},
 		},
-		EnvVars:        make(map[string]string),
+		EnvVars:        make([]corev1.EnvVar, 0),
 		ExecutedTraits: make([]Trait, 0),
 		Resources:      kubernetes.NewCollection(),
 	}