Skip to content

Commit c0a802c

Browse files
authored
[v0.34.x] Update env priority (#572)
* [v0.34.x] Update env priority * changelog * add template env update * changelog
1 parent 4a88fbe commit c0a802c

File tree

24 files changed

+472
-136
lines changed

24 files changed

+472
-136
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
changelog:
2+
- type: FIX
3+
issueLink: https://github.com/solo-io/skv2/issues/565
4+
description: >
5+
Add support for other sources in templated env vars field.
6+
skipCI: "false"
7+
- type: NON_USER_FACING
8+
description: >
9+
Reorder the priority of the environment variables to be loaded in the following order:
10+
1. Templated environment variables
11+
2. Environment variables
12+
3. Extra environment variables
13+
skipCI: "false"

ci/oss_compliance/osa_provided.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ Name|Version|License
5252
[config/v1alpha1](https://k8s.io/component-base/config/v1alpha1)|v0.27.3|Apache License 2.0
5353
[v2/internal](https://k8s.io/klog/v2/internal)|v2.90.1|Apache License 2.0
5454
[kube-openapi/pkg](https://k8s.io/kube-openapi/pkg)|v0.0.0-20230501164219-8b0f38b5fd1f|Apache License 2.0
55-
[k8s.io/utils](https://k8s.io/utils)|v0.0.0-20230209194617-a36077c30491|Apache License 2.0
55+
[k8s.io/utils](https://k8s.io/utils)|v0.0.0-20240502163921-fe8a2dddb1d0|Apache License 2.0
5656
[controller-runtime/pkg](https://sigs.k8s.io/controller-runtime/pkg)|v0.15.0|Apache License 2.0
5757
[encoding/json](https://sigs.k8s.io/json/internal/golang/encoding/json)|v0.0.0-20221116044647-bc3834ca7abd|Apache License 2.0
5858
[structured-merge-diff/v4](https://sigs.k8s.io/structured-merge-diff/v4)|v4.2.3|Apache License 2.0

codegen/cmd_test.go

+124-36
Original file line numberDiff line numberDiff line change
@@ -10,27 +10,26 @@ import (
1010
"reflect"
1111
"strings"
1212

13-
goyaml "gopkg.in/yaml.v3"
14-
rbacv1 "k8s.io/api/rbac/v1"
15-
v12 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
16-
"k8s.io/utils/pointer"
17-
1813
. "github.com/onsi/ginkgo/v2"
1914
. "github.com/onsi/gomega"
20-
. "github.com/solo-io/skv2/codegen"
21-
"github.com/solo-io/skv2/codegen/model"
22-
. "github.com/solo-io/skv2/codegen/model"
23-
"github.com/solo-io/skv2/codegen/skv2_anyvendor"
24-
"github.com/solo-io/skv2/codegen/util"
25-
"github.com/solo-io/skv2/contrib"
15+
goyaml "gopkg.in/yaml.v3"
2616
appsv1 "k8s.io/api/apps/v1"
2717
v1 "k8s.io/api/core/v1"
18+
rbacv1 "k8s.io/api/rbac/v1"
19+
v12 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
2820
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2921
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
3022
"k8s.io/apimachinery/pkg/runtime/schema"
3123
"k8s.io/apimachinery/pkg/util/intstr"
3224
kubeyaml "k8s.io/apimachinery/pkg/util/yaml"
25+
"k8s.io/utils/ptr"
3326
"sigs.k8s.io/yaml"
27+
28+
. "github.com/solo-io/skv2/codegen"
29+
. "github.com/solo-io/skv2/codegen/model"
30+
"github.com/solo-io/skv2/codegen/skv2_anyvendor"
31+
"github.com/solo-io/skv2/codegen/util"
32+
"github.com/solo-io/skv2/contrib"
3433
)
3534

3635
var _ = Describe("Cmd", func() {
@@ -42,6 +41,66 @@ var _ = Describe("Cmd", func() {
4241
skv2Imports.External["github.com/solo-io/cue"] = []string{
4342
"encoding/protobuf/cue/cue.proto",
4443
}
44+
It("env variable priority", func() {
45+
cmd := &Command{
46+
Chart: &Chart{
47+
Data: Data{
48+
ApiVersion: "v1",
49+
Description: "",
50+
Name: "Painting Operator",
51+
Version: "v0.0.1",
52+
Home: "https://docs.solo.io/skv2/latest",
53+
Sources: []string{
54+
"https://github.com/solo-io/skv2",
55+
},
56+
},
57+
Operators: []Operator{{
58+
Name: "painter",
59+
Deployment: Deployment{
60+
Container: Container{
61+
Image: Image{Repository: "painter", Tag: "v0.0.1"},
62+
Env: []v1.EnvVar{{Name: "ENV_VAR", Value: "default"}},
63+
TemplateEnvVars: []TemplateEnvVar{
64+
{
65+
Condition: "$.Values.secret",
66+
Name: "ENV_VAR",
67+
Value: "templated",
68+
},
69+
},
70+
},
71+
},
72+
}},
73+
},
74+
ManifestRoot: "codegen/test/chart/env-priority",
75+
}
76+
Expect(cmd.Execute()).NotTo(HaveOccurred(), "failed to execute command")
77+
78+
manifests := helmTemplate("./test/chart/env-priority", map[string]any{"painter": map[string]any{"enabled": true}, "secret": true})
79+
var renderedDeployment *appsv1.Deployment
80+
decoder := kubeyaml.NewYAMLOrJSONDecoder(bytes.NewBuffer(manifests), 4096)
81+
for {
82+
obj := &unstructured.Unstructured{}
83+
err := decoder.Decode(obj)
84+
if err != nil {
85+
break
86+
}
87+
if obj.GetName() != "painter" || obj.GetKind() != "Deployment" {
88+
continue
89+
}
90+
91+
bytes, err := obj.MarshalJSON()
92+
Expect(err).NotTo(HaveOccurred())
93+
renderedDeployment = &appsv1.Deployment{}
94+
err = json.Unmarshal(bytes, renderedDeployment)
95+
Expect(err).NotTo(HaveOccurred())
96+
}
97+
Expect(renderedDeployment).NotTo(BeNil())
98+
99+
Expect(renderedDeployment.Spec.Template.Spec.Containers[0].Env).To(HaveLen(2))
100+
Expect(renderedDeployment.Spec.Template.Spec.Containers[0].Env[0]).To(Equal(v1.EnvVar{Name: "ENV_VAR", Value: "templated"}))
101+
Expect(renderedDeployment.Spec.Template.Spec.Containers[0].Env[1]).To(Equal(v1.EnvVar{Name: "ENV_VAR", Value: "default"}))
102+
})
103+
45104
It("install conditional sidecars", func() {
46105
agentConditional := "and ($.Values.glooAgent.enabled) ($.Values.glooAgent.runAsSidecar)"
47106

@@ -111,6 +170,30 @@ var _ = Describe("Cmd", func() {
111170
Repository: "gloo-mesh-mgmt-server",
112171
Tag: "0.0.1",
113172
},
173+
TemplateEnvVars: []TemplateEnvVar{
174+
{
175+
Name: "USERNAME",
176+
ValueFrom: v1.EnvVarSource{
177+
SecretKeyRef: &v1.SecretKeySelector{
178+
LocalObjectReference: v1.LocalObjectReference{
179+
Name: "{{ $.Values.someSecret }}",
180+
},
181+
Key: "{{ $.Values.usernameKey }}",
182+
},
183+
},
184+
},
185+
{
186+
Name: "PASSWORD",
187+
ValueFrom: v1.EnvVarSource{
188+
ConfigMapKeyRef: &v1.ConfigMapKeySelector{
189+
LocalObjectReference: v1.LocalObjectReference{
190+
Name: "{{ $.Values.someConfigMap }}",
191+
},
192+
Key: "{{ $.Values.passwordKey }}",
193+
},
194+
},
195+
},
196+
},
114197
ContainerPorts: []ContainerPort{{
115198
Name: "stats",
116199
Port: "{{ $Values.glooMgmtServer.statsPort }}",
@@ -155,6 +238,11 @@ var _ = Describe("Cmd", func() {
155238
Expect(deployment).To(ContainSubstring("name: agent-volume"))
156239
Expect(deployment).To(ContainSubstring(`{{ index $glooAgent "ports" "grpc" }}`))
157240
Expect(deployment).To(ContainSubstring("{{ $Values.glooMgmtServer.statsPort }}"))
241+
242+
Expect(deployment).To(ContainSubstring("{{ $.Values.usernameKey }}"))
243+
Expect(deployment).To(ContainSubstring("{{ $.Values.passwordKey }}"))
244+
Expect(deployment).To(ContainSubstring("{{ $.Values.someSecret }}"))
245+
Expect(deployment).To(ContainSubstring("{{ $.Values.someConfigMap }}"))
158246
})
159247
It("generates conditional crds", func() {
160248
cmd := &Command{
@@ -774,13 +862,11 @@ var _ = Describe("Cmd", func() {
774862
}
775863
Expect(renderedDeployment).NotTo(BeNil())
776864

777-
pointerBool := func(b bool) *bool { return &b }
778-
pointerInt64 := func(i int64) *int64 { return &i }
779865
defaultSecurityContext := v1.SecurityContext{
780-
RunAsNonRoot: pointerBool(true),
781-
RunAsUser: pointerInt64(10101),
782-
ReadOnlyRootFilesystem: pointerBool(true),
783-
AllowPrivilegeEscalation: pointerBool(false),
866+
RunAsNonRoot: ptr.To(true),
867+
RunAsUser: ptr.To[int64](10101),
868+
ReadOnlyRootFilesystem: ptr.To(true),
869+
AllowPrivilegeEscalation: ptr.To(false),
784870
Capabilities: &v1.Capabilities{
785871
Drop: []v1.Capability{"ALL"},
786872
},
@@ -800,8 +886,8 @@ var _ = Describe("Cmd", func() {
800886
Entry("renders empty map for container security context when set as false via helm cli", nil, true),
801887
Entry("overrides container security context with empty map", &v1.SecurityContext{}, false),
802888
Entry("overrides container security context", &v1.SecurityContext{
803-
RunAsNonRoot: func(b bool) *bool { return &b }(true),
804-
RunAsUser: func(i int64) *int64 { return &i }(20202),
889+
RunAsNonRoot: ptr.To(true),
890+
RunAsUser: ptr.To[int64](20202),
805891
}, false),
806892
)
807893

@@ -1926,7 +2012,7 @@ roleRef:
19262012
)
19272013

19282014
DescribeTable("rendering conditional deployment strategy",
1929-
func(values map[string]any, conditionalStrategy []model.ConditionalStrategy, expectedStrategy appsv1.DeploymentStrategy) {
2015+
func(values map[string]any, conditionalStrategy []ConditionalStrategy, expectedStrategy appsv1.DeploymentStrategy) {
19302016
cmd := &Command{
19312017
Chart: &Chart{
19322018
Operators: []Operator{
@@ -1997,7 +2083,7 @@ roleRef:
19972083
),
19982084
Entry("when the condition is true",
19992085
map[string]any{"enabled": true, "condition": true},
2000-
[]model.ConditionalStrategy{
2086+
[]ConditionalStrategy{
20012087
{
20022088
Condition: "$.Values.painter.condition",
20032089
Strategy: appsv1.DeploymentStrategy{
@@ -2017,7 +2103,7 @@ roleRef:
20172103
),
20182104
Entry("when the condition is false",
20192105
map[string]any{"enabled": true, "condition": false},
2020-
[]model.ConditionalStrategy{
2106+
[]ConditionalStrategy{
20212107
{
20222108
Condition: "$.Values.painter.condition",
20232109
Strategy: appsv1.DeploymentStrategy{
@@ -2112,23 +2198,23 @@ roleRef:
21122198
map[string]interface{}{"fsGroup": 1000},
21132199
nil,
21142200
&v1.PodSecurityContext{
2115-
FSGroup: pointer.Int64(1000),
2201+
FSGroup: ptr.To[int64](1000),
21162202
}),
21172203
Entry("when PodSecurityContext is defined only in the operator",
21182204
nil,
21192205
&v1.PodSecurityContext{
2120-
FSGroup: pointer.Int64(1000),
2206+
FSGroup: ptr.To[int64](1000),
21212207
},
21222208
&v1.PodSecurityContext{
2123-
FSGroup: pointer.Int64(1000),
2209+
FSGroup: ptr.To[int64](1000),
21242210
}),
21252211
Entry("when PodSecurityContext is defined in both values and the operator",
21262212
map[string]interface{}{"fsGroup": 1024},
21272213
&v1.PodSecurityContext{
2128-
FSGroup: pointer.Int64(1000),
2214+
FSGroup: ptr.To[int64](1000),
21292215
},
21302216
&v1.PodSecurityContext{
2131-
FSGroup: pointer.Int64(1024), // should override the value defined in the operator
2217+
FSGroup: ptr.To[int64](1024), // should override the value defined in the operator
21322218
}),
21332219
)
21342220

@@ -2227,7 +2313,9 @@ roleRef:
22272313
Value: "{{ $.Values.featureGates.Foo | quote }}",
22282314
},
22292315
},
2230-
nil),
2316+
[]v1.EnvVar{
2317+
{Name: "FEATURE_ENABLE_FOO", Value: "true"},
2318+
}),
22312319
Entry("when Env and TemplateEnvVar are specified, true value",
22322320
map[string]string{"Foo": "true"},
22332321
[]v1.EnvVar{
@@ -2316,7 +2404,7 @@ roleRef:
23162404
})
23172405

23182406
DescribeTable("validation",
2319-
func(values map[string]string, defaultVolumes []v1.Volume, conditionalVolumes []model.ConditionalVolume, expected []v1.Volume) {
2407+
func(values map[string]string, defaultVolumes []v1.Volume, conditionalVolumes []ConditionalVolume, expected []v1.Volume) {
23202408
cmd := &Command{
23212409
Chart: &Chart{
23222410
Operators: []Operator{
@@ -2409,7 +2497,7 @@ roleRef:
24092497
"condition": "true",
24102498
},
24112499
nil,
2412-
[]model.ConditionalVolume{
2500+
[]ConditionalVolume{
24132501
{
24142502
Condition: "$.Values.painter.condition",
24152503
Volume: v1.Volume{
@@ -2428,7 +2516,7 @@ roleRef:
24282516
"condition": "true",
24292517
},
24302518
nil,
2431-
[]model.ConditionalVolume{
2519+
[]ConditionalVolume{
24322520
{
24332521
Condition: "$.Values.painter.invalidCondition",
24342522
Volume: v1.Volume{
@@ -2447,7 +2535,7 @@ roleRef:
24472535
Name: "vol-1",
24482536
},
24492537
},
2450-
[]model.ConditionalVolume{
2538+
[]ConditionalVolume{
24512539
{
24522540
Condition: "$.Values.painter.condition",
24532541
Volume: v1.Volume{
@@ -2479,7 +2567,7 @@ roleRef:
24792567
})
24802568

24812569
DescribeTable("validation",
2482-
func(values map[string]string, defaultMounts []v1.VolumeMount, conditionalMounts []model.ConditionalVolumeMount, expected []v1.VolumeMount) {
2570+
func(values map[string]string, defaultMounts []v1.VolumeMount, conditionalMounts []ConditionalVolumeMount, expected []v1.VolumeMount) {
24832571
cmd := &Command{
24842572
Chart: &Chart{
24852573
Operators: []Operator{
@@ -2574,7 +2662,7 @@ roleRef:
25742662
"condition": "true",
25752663
},
25762664
nil,
2577-
[]model.ConditionalVolumeMount{
2665+
[]ConditionalVolumeMount{
25782666
{
25792667
Condition: "$.Values.painter.condition",
25802668
VolumeMount: v1.VolumeMount{
@@ -2593,7 +2681,7 @@ roleRef:
25932681
"condition": "true",
25942682
},
25952683
nil,
2596-
[]model.ConditionalVolumeMount{
2684+
[]ConditionalVolumeMount{
25972685
{
25982686
Condition: "$.Values.painter.invalidCondition",
25992687
VolumeMount: v1.VolumeMount{
@@ -2612,7 +2700,7 @@ roleRef:
26122700
Name: "vol-1",
26132701
},
26142702
},
2615-
[]model.ConditionalVolumeMount{
2703+
[]ConditionalVolumeMount{
26162704
{
26172705
Condition: "$.Values.painter.condition",
26182706
VolumeMount: v1.VolumeMount{

codegen/model/chart.go

+3
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,9 @@ type TemplateEnvVar struct {
173173
// Helm value
174174
// E.g. {{ .Values.foo.bar }}
175175
Value string
176+
177+
//
178+
ValueFrom corev1.EnvVarSource
176179
}
177180

178181
type ContainerPort struct {

codegen/templates/chart/operator-deployment.yamltmpl

+20-11
Original file line numberDiff line numberDiff line change
@@ -140,25 +140,34 @@ spec:
140140
containerPort: [[ $port.Port ]]
141141
[[- end ]]
142142
[[- end ]]
143-
{{- if [[ $containerVar ]].env }}
143+
[[- if or $container.Env $container.TemplateEnvVars ]]
144+
env:
145+
[[- else ]]
146+
{{- if or [[ $containerVar ]].env [[ $containerVar ]].extraEnvs }}
144147
env:
145-
{{ toYaml [[ $containerVar ]].env | indent 10 }}
148+
{{- end }}
149+
[[- end ]]
146150
[[- range $f := $container.TemplateEnvVars ]]
147-
[[- if $f.Condition ]]
148-
{{- if [[ $f.Condition ]] }}
149-
[[- end]]
151+
[[- if $f.Condition ]]
152+
{{- if [[ $f.Condition ]] }}
153+
[[- end ]]
154+
[[- if $f.Value ]]
150155
- name: [[ $f.Name ]]
151156
value: [[ $f.Value ]]
152-
[[- if $f.Condition ]]
153-
{{- end }}
154-
[[- end]]
157+
[[- else if $f.ValueFrom ]]
158+
- name: [[ $f.Name ]]
159+
valueFrom: [[ $f.ValueFrom | toYaml | nindent 14 ]]
160+
[[- end ]]
161+
[[- if $f.Condition ]]
162+
{{- end }}
163+
[[- end ]]
155164
[[- end ]]
156-
{{- else if [[ $containerVar ]].extraEnvs }}
157-
env:
165+
{{- if [[ $containerVar ]].env }}
166+
{{- toYaml [[ $containerVar ]].env | nindent 10 }}
158167
{{- end }}
159168
{{- range $name, $item := [[ $containerVar ]].extraEnvs }}
160169
- name: {{ $name }}
161-
{{- $item | toYaml | nindent 12 }}
170+
{{- $item | toYaml | nindent 12 }}
162171
{{- end }}
163172
[[- if $container.VolumeMounts ]]
164173
volumeMounts:

0 commit comments

Comments
 (0)