[v0.36.x] Update env priority (#571)

* [v0.36.x] Update env priority

* changelog

* add template env update

changelog/v0.36.7/env-template-sources.yaml

@@ -0,0 +1,13 @@
+changelog:
+  - type: FIX
+    issueLink: https://github.com/solo-io/skv2/issues/565
+    description: >
+      Add support for other sources in templated env vars field.
+    skipCI: "false"
+  - type: NON_USER_FACING
+    description: >
+      Reorder the priority of the environment variables to be loaded in the following order:
+      1. Templated environment variables
+      2. Environment variables
+      3. Extra environment variables
+    skipCI: "false"

ci/oss_compliance/osa_provided.md

@@ -51,7 +51,7 @@ Name|Version|License
 [config/v1alpha1](https://k8s.io/component-base/config/v1alpha1)|v0.28.3|Apache License 2.0
 [v2/internal](https://k8s.io/klog/v2/internal)|v2.100.1|Apache License 2.0
 [kube-openapi/pkg](https://k8s.io/kube-openapi/pkg)|v0.0.0-20230717233707-2695361300d9|Apache License 2.0
-[k8s.io/utils](https://k8s.io/utils)|v0.0.0-20230406110748-d93618cff8a2|Apache License 2.0
+[k8s.io/utils](https://k8s.io/utils)|v0.0.0-20240502163921-fe8a2dddb1d0|Apache License 2.0
 [controller-runtime/pkg](https://sigs.k8s.io/controller-runtime/pkg)|v0.16.3|Apache License 2.0
 [encoding/json](https://sigs.k8s.io/json/internal/golang/encoding/json)|v0.0.0-20221116044647-bc3834ca7abd|Apache License 2.0
 [structured-merge-diff/v4](https://sigs.k8s.io/structured-merge-diff/v4)|v4.2.3|Apache License 2.0

codegen/cmd_test.go

@@ -10,27 +10,26 @@ import (
 	"reflect"
 	"strings"
 
-	goyaml "gopkg.in/yaml.v3"
-	rbacv1 "k8s.io/api/rbac/v1"
-	v12 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
-	"k8s.io/utils/pointer"
-
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
-	. "github.com/solo-io/skv2/codegen"
-	"github.com/solo-io/skv2/codegen/model"
-	. "github.com/solo-io/skv2/codegen/model"
-	"github.com/solo-io/skv2/codegen/skv2_anyvendor"
-	"github.com/solo-io/skv2/codegen/util"
-	"github.com/solo-io/skv2/contrib"
+	goyaml "gopkg.in/yaml.v3"
 	appsv1 "k8s.io/api/apps/v1"
 	v1 "k8s.io/api/core/v1"
+	rbacv1 "k8s.io/api/rbac/v1"
+	v12 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/util/intstr"
 	kubeyaml "k8s.io/apimachinery/pkg/util/yaml"
+	"k8s.io/utils/ptr"
 	"sigs.k8s.io/yaml"
+
+	. "github.com/solo-io/skv2/codegen"
+	. "github.com/solo-io/skv2/codegen/model"
+	"github.com/solo-io/skv2/codegen/skv2_anyvendor"
+	"github.com/solo-io/skv2/codegen/util"
+	"github.com/solo-io/skv2/contrib"
 )
 
 var _ = Describe("Cmd", func() {
@@ -42,6 +41,66 @@ var _ = Describe("Cmd", func() {
 	skv2Imports.External["github.com/solo-io/cue"] = []string{
 		"encoding/protobuf/cue/cue.proto",
 	}
+	It("env variable priority", func() {
+		cmd := &Command{
+			Chart: &Chart{
+				Data: Data{
+					ApiVersion:  "v1",
+					Description: "",
+					Name:        "Painting Operator",
+					Version:     "v0.0.1",
+					Home:        "https://docs.solo.io/skv2/latest",
+					Sources: []string{
+						"https://github.com/solo-io/skv2",
+					},
+				},
+				Operators: []Operator{{
+					Name: "painter",
+					Deployment: Deployment{
+						Container: Container{
+							Image: Image{Repository: "painter", Tag: "v0.0.1"},
+							Env:   []v1.EnvVar{{Name: "ENV_VAR", Value: "default"}},
+							TemplateEnvVars: []TemplateEnvVar{
+								{
+									Condition: "$.Values.secret",
+									Name:      "ENV_VAR",
+									Value:     "templated",
+								},
+							},
+						},
+					},
+				}},
+			},
+			ManifestRoot: "codegen/test/chart/env-priority",
+		}
+		Expect(cmd.Execute()).NotTo(HaveOccurred(), "failed to execute command")
+
+		manifests := helmTemplate("./test/chart/env-priority", map[string]any{"painter": map[string]any{"enabled": true}, "secret": true})
+		var renderedDeployment *appsv1.Deployment
+		decoder := kubeyaml.NewYAMLOrJSONDecoder(bytes.NewBuffer(manifests), 4096)
+		for {
+			obj := &unstructured.Unstructured{}
+			err := decoder.Decode(obj)
+			if err != nil {
+				break
+			}
+			if obj.GetName() != "painter" || obj.GetKind() != "Deployment" {
+				continue
+			}
+
+			bytes, err := obj.MarshalJSON()
+			Expect(err).NotTo(HaveOccurred())
+			renderedDeployment = &appsv1.Deployment{}
+			err = json.Unmarshal(bytes, renderedDeployment)
+			Expect(err).NotTo(HaveOccurred())
+		}
+		Expect(renderedDeployment).NotTo(BeNil())
+
+		Expect(renderedDeployment.Spec.Template.Spec.Containers[0].Env).To(HaveLen(2))
+		Expect(renderedDeployment.Spec.Template.Spec.Containers[0].Env[0]).To(Equal(v1.EnvVar{Name: "ENV_VAR", Value: "templated"}))
+		Expect(renderedDeployment.Spec.Template.Spec.Containers[0].Env[1]).To(Equal(v1.EnvVar{Name: "ENV_VAR", Value: "default"}))
+	})
+
 	It("install conditional sidecars", func() {
 		agentConditional := "and ($.Values.glooAgent.enabled) ($.Values.glooAgent.runAsSidecar)"
 
@@ -111,6 +170,30 @@ var _ = Describe("Cmd", func() {
 									Repository: "gloo-mesh-mgmt-server",
 									Tag:        "0.0.1",
 								},
+								TemplateEnvVars: []TemplateEnvVar{
+									{
+										Name: "USERNAME",
+										ValueFrom: v1.EnvVarSource{
+											SecretKeyRef: &v1.SecretKeySelector{
+												LocalObjectReference: v1.LocalObjectReference{
+													Name: "{{ $.Values.someSecret }}",
+												},
+												Key: "{{ $.Values.usernameKey }}",
+											},
+										},
+									},
+									{
+										Name: "PASSWORD",
+										ValueFrom: v1.EnvVarSource{
+											ConfigMapKeyRef: &v1.ConfigMapKeySelector{
+												LocalObjectReference: v1.LocalObjectReference{
+													Name: "{{ $.Values.someConfigMap }}",
+												},
+												Key: "{{ $.Values.passwordKey }}",
+											},
+										},
+									},
+								},
 								ContainerPorts: []ContainerPort{{
 									Name: "stats",
 									Port: "{{ $Values.glooMgmtServer.statsPort }}",
@@ -155,6 +238,11 @@ var _ = Describe("Cmd", func() {
 		Expect(deployment).To(ContainSubstring("name: agent-volume"))
 		Expect(deployment).To(ContainSubstring(`{{ index $glooAgent "ports" "grpc" }}`))
 		Expect(deployment).To(ContainSubstring("{{ $Values.glooMgmtServer.statsPort }}"))
+
+		Expect(deployment).To(ContainSubstring("{{ $.Values.usernameKey }}"))
+		Expect(deployment).To(ContainSubstring("{{ $.Values.passwordKey }}"))
+		Expect(deployment).To(ContainSubstring("{{ $.Values.someSecret }}"))
+		Expect(deployment).To(ContainSubstring("{{ $.Values.someConfigMap }}"))
 	})
 	It("generates conditional crds", func() {
 		cmd := &Command{
@@ -772,13 +860,11 @@ var _ = Describe("Cmd", func() {
 			}
 			Expect(renderedDeployment).NotTo(BeNil())
 
-			pointerBool := func(b bool) *bool { return &b }
-			pointerInt64 := func(i int64) *int64 { return &i }
 			defaultSecurityContext := v1.SecurityContext{
-				RunAsNonRoot:             pointerBool(true),
-				RunAsUser:                pointerInt64(10101),
-				ReadOnlyRootFilesystem:   pointerBool(true),
-				AllowPrivilegeEscalation: pointerBool(false),
+				RunAsNonRoot:             ptr.To(true),
+				RunAsUser:                ptr.To[int64](10101),
+				ReadOnlyRootFilesystem:   ptr.To(true),
+				AllowPrivilegeEscalation: ptr.To(false),
 				Capabilities: &v1.Capabilities{
 					Drop: []v1.Capability{"ALL"},
 				},
@@ -798,8 +884,8 @@ var _ = Describe("Cmd", func() {
 		Entry("renders empty map for container security context when set as false via helm cli", nil, true),
 		Entry("overrides container security context with empty map", &v1.SecurityContext{}, false),
 		Entry("overrides container security context", &v1.SecurityContext{
-			RunAsNonRoot: func(b bool) *bool { return &b }(true),
-			RunAsUser:    func(i int64) *int64 { return &i }(20202),
+			RunAsNonRoot: ptr.To(true),
+			RunAsUser:    ptr.To[int64](20202),
 		}, false),
 	)
 
@@ -1928,7 +2014,7 @@ roleRef:
 	)
 
 	DescribeTable("rendering conditional deployment strategy",
-		func(values map[string]any, conditionalStrategy []model.ConditionalStrategy, expectedStrategy appsv1.DeploymentStrategy) {
+		func(values map[string]any, conditionalStrategy []ConditionalStrategy, expectedStrategy appsv1.DeploymentStrategy) {
 			cmd := &Command{
 				Chart: &Chart{
 					Operators: []Operator{
@@ -1999,7 +2085,7 @@ roleRef:
 		),
 		Entry("when the condition is true",
 			map[string]any{"enabled": true, "condition": true},
-			[]model.ConditionalStrategy{
+			[]ConditionalStrategy{
 				{
 					Condition: "$.Values.painter.condition",
 					Strategy: appsv1.DeploymentStrategy{
@@ -2019,7 +2105,7 @@ roleRef:
 		),
 		Entry("when the condition is false",
 			map[string]any{"enabled": true, "condition": false},
-			[]model.ConditionalStrategy{
+			[]ConditionalStrategy{
 				{
 					Condition: "$.Values.painter.condition",
 					Strategy: appsv1.DeploymentStrategy{
@@ -2114,23 +2200,23 @@ roleRef:
 			map[string]interface{}{"fsGroup": 1000},
 			nil,
 			&v1.PodSecurityContext{
-				FSGroup: pointer.Int64(1000),
+				FSGroup: ptr.To[int64](1000),
 			}),
 		Entry("when PodSecurityContext is defined only in the operator",
 			nil,
 			&v1.PodSecurityContext{
-				FSGroup: pointer.Int64(1000),
+				FSGroup: ptr.To[int64](1000),
 			},
 			&v1.PodSecurityContext{
-				FSGroup: pointer.Int64(1000),
+				FSGroup: ptr.To[int64](1000),
 			}),
 		Entry("when PodSecurityContext is defined in both values and the operator",
 			map[string]interface{}{"fsGroup": 1024},
 			&v1.PodSecurityContext{
-				FSGroup: pointer.Int64(1000),
+				FSGroup: ptr.To[int64](1000),
 			},
 			&v1.PodSecurityContext{
-				FSGroup: pointer.Int64(1024), // should override the value defined in the operator
+				FSGroup: ptr.To[int64](1024), // should override the value defined in the operator
 			}),
 	)
 
@@ -2229,7 +2315,9 @@ roleRef:
 						Value: "{{ $.Values.featureGates.Foo | quote }}",
 					},
 				},
-				nil),
+				[]v1.EnvVar{
+					{Name: "FEATURE_ENABLE_FOO", Value: "true"},
+				}),
 			Entry("when Env and TemplateEnvVar are specified, true value",
 				map[string]string{"Foo": "true"},
 				[]v1.EnvVar{
@@ -2318,7 +2406,7 @@ roleRef:
 		})
 
 		DescribeTable("validation",
-			func(values map[string]any, defaultVolumes []v1.Volume, conditionalVolumes []model.ConditionalVolume, expected []v1.Volume) {
+			func(values map[string]any, defaultVolumes []v1.Volume, conditionalVolumes []ConditionalVolume, expected []v1.Volume) {
 				cmd := &Command{
 					Chart: &Chart{
 						Operators: []Operator{
@@ -2412,7 +2500,7 @@ roleRef:
 					"condition": "true",
 				},
 				nil,
-				[]model.ConditionalVolume{
+				[]ConditionalVolume{
 					{
 						Condition: "$.Values.painter.condition",
 						Volume: v1.Volume{
@@ -2432,7 +2520,7 @@ roleRef:
 					"condition": "true",
 				},
 				nil,
-				[]model.ConditionalVolume{
+				[]ConditionalVolume{
 					{
 						Condition: "$.Values.painter.invalidCondition",
 						Volume: v1.Volume{
@@ -2452,7 +2540,7 @@ roleRef:
 						Name: "vol-1",
 					},
 				},
-				[]model.ConditionalVolume{
+				[]ConditionalVolume{
 					{
 						Condition: "$.Values.painter.condition",
 						Volume: v1.Volume{
@@ -2484,7 +2572,7 @@ roleRef:
 		})
 
 		DescribeTable("validation",
-			func(values map[string]any, defaultMounts []v1.VolumeMount, conditionalMounts []model.ConditionalVolumeMount, expected []v1.VolumeMount) {
+			func(values map[string]any, defaultMounts []v1.VolumeMount, conditionalMounts []ConditionalVolumeMount, expected []v1.VolumeMount) {
 				cmd := &Command{
 					Chart: &Chart{
 						Operators: []Operator{
@@ -2580,7 +2668,7 @@ roleRef:
 					"condition": "true",
 				},
 				nil,
-				[]model.ConditionalVolumeMount{
+				[]ConditionalVolumeMount{
 					{
 						Condition: "$.Values.painter.condition",
 						VolumeMount: v1.VolumeMount{
@@ -2600,7 +2688,7 @@ roleRef:
 					"condition": "true",
 				},
 				nil,
-				[]model.ConditionalVolumeMount{
+				[]ConditionalVolumeMount{
 					{
 						Condition: "$.Values.painter.invalidCondition",
 						VolumeMount: v1.VolumeMount{
@@ -2620,7 +2708,7 @@ roleRef:
 						Name: "vol-1",
 					},
 				},
-				[]model.ConditionalVolumeMount{
+				[]ConditionalVolumeMount{
 					{
 						Condition: "$.Values.painter.condition",
 						VolumeMount: v1.VolumeMount{

codegen/model/chart.go

@@ -173,6 +173,9 @@ type TemplateEnvVar struct {
 	// Helm value
 	// E.g. {{ .Values.foo.bar }}
 	Value string
+
+	//
+	ValueFrom corev1.EnvVarSource
 }
 
 type ContainerPort struct {

codegen/templates/chart/operator-deployment.yamltmpl

@@ -140,25 +140,34 @@ spec:
           containerPort: [[ $port.Port ]]
         [[- end ]]
 [[- end ]]
-{{- if [[ $containerVar ]].env }}
+[[- if or $container.Env $container.TemplateEnvVars ]]
+        env:
+[[- else ]]
+  {{- if or [[ $containerVar ]].env [[ $containerVar ]].extraEnvs }}
         env:
-{{ toYaml [[ $containerVar ]].env | indent 10 }}
+  {{- end }}
+[[- end ]]
 [[- range $f := $container.TemplateEnvVars ]]
-[[- if $f.Condition ]]
-{{- if [[ $f.Condition ]] }}
-[[- end]]
+  [[- if $f.Condition ]]
+  {{- if [[ $f.Condition ]] }}
+  [[- end ]]
+        [[- if $f.Value ]]
           - name: [[ $f.Name ]]
             value: [[ $f.Value ]]
-[[- if $f.Condition ]]
-{{- end }}
-[[- end]]
+        [[- else if $f.ValueFrom ]]
+          - name: [[ $f.Name ]]
+            valueFrom: [[ $f.ValueFrom | toYaml | nindent 14 ]]
+        [[- end ]]
+  [[- if $f.Condition ]]
+  {{- end }}
+  [[- end ]]
 [[- end ]]
-{{- else if [[ $containerVar ]].extraEnvs }}
-        env:
+{{- if [[ $containerVar ]].env }}
+{{- toYaml [[ $containerVar ]].env | nindent 10 }}
 {{- end }}
 {{- range $name, $item := [[ $containerVar ]].extraEnvs }}
           - name: {{ $name }}
-{{- $item | toYaml | nindent 12 }}
+  {{- $item | toYaml | nindent 12 }}
 {{- end }}
 [[- if $container.VolumeMounts ]]
         volumeMounts: