Skip to content

Commit f99ce98

Browse files
hiddecohiddeco
authored
Merge pull request #114 from fluxcd/compose-values-bug
Declare and assign result Values before composing
2 parents ebc3178 + 3fccbb9 commit f99ce98

File tree

3 files changed

+250
-1
lines changed

3 files changed

+250
-1
lines changed

‎controllers/helmrelease_controller.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -462,7 +462,7 @@ func (r *HelmReleaseReconciler) checkDependencies(hr v2.HelmRelease) error {
462462
// and merges them as defined. Referenced resources are only retrieved once
463463
// to ensure a single version is taken into account during the merge.
464464
func (r *HelmReleaseReconciler) composeValues(ctx context.Context, hr v2.HelmRelease) (chartutil.Values, error) {
465-
var result chartutil.Values
465+
result := chartutil.Values{}
466466

467467
configMaps := make(map[string]*corev1.ConfigMap)
468468
secrets := make(map[string]*corev1.Secret)

‎controllers/helmrelease_controller_test.go‎

Lines changed: 247 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,247 @@
1+
/*
2+
Copyright 2020 The Flux CD contributors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package controllers
18+
19+
import (
20+
"context"
21+
"reflect"
22+
"testing"
23+
24+
"helm.sh/helm/v3/pkg/chartutil"
25+
corev1 "k8s.io/api/core/v1"
26+
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
27+
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
28+
"k8s.io/apimachinery/pkg/runtime"
29+
"sigs.k8s.io/controller-runtime/pkg/client/fake"
30+
"sigs.k8s.io/controller-runtime/pkg/log"
31+
"sigs.k8s.io/yaml"
32+
33+
v2 "github.com/fluxcd/helm-controller/api/v2beta1"
34+
)
35+
36+
func TestHelmReleaseReconciler_composeValues(t *testing.T) {
37+
scheme := runtime.NewScheme()
38+
_ = corev1.AddToScheme(scheme)
39+
_ = v2.AddToScheme(scheme)
40+
41+
tests := []struct {
42+
name string
43+
resources []runtime.Object
44+
references []v2.ValuesReference
45+
values string
46+
want chartutil.Values
47+
wantErr bool
48+
}{
49+
{
50+
name: "merges",
51+
resources: []runtime.Object{
52+
valuesConfigMap("values", map[string]string{
53+
"values.yaml": `flat: value
54+
nested:
55+
configuration: value
56+
`,
57+
}),
58+
valuesSecret("values", map[string][]byte{
59+
"values.yaml": []byte(`flat:
60+
nested: value
61+
nested: value
62+
`),
63+
}),
64+
},
65+
references: []v2.ValuesReference{
66+
{
67+
Kind: "ConfigMap",
68+
Name: "values",
69+
},
70+
{
71+
Kind: "Secret",
72+
Name: "values",
73+
},
74+
},
75+
values: `
76+
other: values
77+
`,
78+
want: chartutil.Values{
79+
"flat": map[string]interface{}{
80+
"nested": "value",
81+
},
82+
"nested": "value",
83+
"other": "values",
84+
},
85+
},
86+
{
87+
name: "target path",
88+
resources: []runtime.Object{
89+
valuesSecret("values", map[string][]byte{"single": []byte("value")}),
90+
},
91+
references: []v2.ValuesReference{
92+
{
93+
Kind: "Secret",
94+
Name: "values",
95+
ValuesKey: "single",
96+
TargetPath: "merge.at.specific.path",
97+
},
98+
},
99+
want: chartutil.Values{
100+
"merge": map[string]interface{}{
101+
"at": map[string]interface{}{
102+
"specific": map[string]interface{}{
103+
"path": "value",
104+
},
105+
},
106+
},
107+
},
108+
},
109+
{
110+
name: "values reference to non existing secret",
111+
references: []v2.ValuesReference{
112+
{
113+
Kind: "Secret",
114+
Name: "missing",
115+
},
116+
},
117+
wantErr: true,
118+
},
119+
{
120+
name: "optional values reference to non existing secret",
121+
references: []v2.ValuesReference{
122+
{
123+
Kind: "Secret",
124+
Name: "missing",
125+
Optional: true,
126+
},
127+
},
128+
want: chartutil.Values{},
129+
wantErr: false,
130+
},
131+
{
132+
name: "values reference to non existing config map",
133+
references: []v2.ValuesReference{
134+
{
135+
Kind: "ConfigMap",
136+
Name: "missing",
137+
},
138+
},
139+
wantErr: true,
140+
},
141+
{
142+
name: "optional values reference to non existing config map",
143+
references: []v2.ValuesReference{
144+
{
145+
Kind: "ConfigMap",
146+
Name: "missing",
147+
Optional: true,
148+
},
149+
},
150+
want: chartutil.Values{},
151+
wantErr: false,
152+
},
153+
{
154+
name: "missing secret key",
155+
resources: []runtime.Object{
156+
valuesSecret("values", nil),
157+
},
158+
references: []v2.ValuesReference{
159+
{
160+
Kind: "Secret",
161+
Name: "values",
162+
ValuesKey: "nonexisting",
163+
},
164+
},
165+
wantErr: true,
166+
},
167+
{
168+
name: "missing config map key",
169+
resources: []runtime.Object{
170+
valuesConfigMap("values", nil),
171+
},
172+
references: []v2.ValuesReference{
173+
{
174+
Kind: "ConfigMap",
175+
Name: "values",
176+
ValuesKey: "nonexisting",
177+
},
178+
},
179+
wantErr: true,
180+
},
181+
{
182+
name: "unsupported values reference kind",
183+
references: []v2.ValuesReference{
184+
{
185+
Kind: "Unsupported",
186+
},
187+
},
188+
wantErr: true,
189+
},
190+
{
191+
name: "invalid values",
192+
resources: []runtime.Object{
193+
valuesConfigMap("values", map[string]string{
194+
"values.yaml": `
195+
invalid`,
196+
}),
197+
},
198+
references: []v2.ValuesReference{
199+
{
200+
Kind: "ConfigMap",
201+
Name: "values",
202+
},
203+
},
204+
wantErr: true,
205+
},
206+
}
207+
208+
for _, tt := range tests {
209+
t.Run(tt.name, func(t *testing.T) {
210+
c := fake.NewFakeClientWithScheme(scheme, tt.resources...)
211+
r := &HelmReleaseReconciler{Client: c, Log: log.NullLogger{}}
212+
var values *apiextensionsv1.JSON
213+
if tt.values != "" {
214+
v, _ := yaml.YAMLToJSON([]byte(tt.values))
215+
values = &apiextensionsv1.JSON{Raw: v}
216+
}
217+
hr := v2.HelmRelease{
218+
Spec: v2.HelmReleaseSpec{
219+
ValuesFrom: tt.references,
220+
Values: values,
221+
},
222+
}
223+
got, err := r.composeValues(context.TODO(), hr)
224+
if (err != nil) != tt.wantErr {
225+
t.Errorf("composeValues() error = %v, wantErr %v", err, tt.wantErr)
226+
return
227+
}
228+
if !reflect.DeepEqual(got, tt.want) {
229+
t.Errorf("composeValues() got = %v, want %v", got, tt.want)
230+
}
231+
})
232+
}
233+
}
234+
235+
func valuesSecret(name string, data map[string][]byte) *corev1.Secret {
236+
return &corev1.Secret{
237+
ObjectMeta: v1.ObjectMeta{Name: name},
238+
Data: data,
239+
}
240+
}
241+
242+
func valuesConfigMap(name string, data map[string]string) *corev1.ConfigMap {
243+
return &corev1.ConfigMap{
244+
ObjectMeta: v1.ObjectMeta{Name: name},
245+
Data: data,
246+
}
247+
}

‎go.mod‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,11 @@ require (
1515
go.uber.org/zap v1.13.0
1616
helm.sh/helm/v3 v3.3.4
1717
k8s.io/api v0.18.9
18+
k8s.io/apiextensions-apiserver v0.18.9
1819
k8s.io/apimachinery v0.18.9
1920
k8s.io/cli-runtime v0.18.9
2021
k8s.io/client-go v0.18.9
2122
rsc.io/letsencrypt v0.0.3 // indirect
2223
sigs.k8s.io/controller-runtime v0.6.3
24+
sigs.k8s.io/yaml v1.2.0
2325
)

0 commit comments

Comments
 (0)