Skip to content

Commit bb712d4

Browse files
sgalsalehsgalsaleh
authored
fix: only unmarshal HelmChart custom resources after they are templated (#2700)
1 parent 1da8615 commit bb712d4

File tree

4 files changed

+41
-50
lines changed

4 files changed

+41
-50
lines changed

‎api/internal/managers/app/release/template.go‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -71,18 +71,12 @@ func (m *appReleaseManager) templateHelmChartCRs(configValues types.AppConfigVal
7171
templatedCRs := make([]*kotsv1beta2.HelmChart, 0, len(helmChartCRs))
7272

7373
for _, helmChartCR := range helmChartCRs {
74-
if helmChartCR == nil {
74+
if len(helmChartCR) == 0 {
7575
continue
7676
}
7777

78-
// Marshal the HelmChart CR to YAML for templating
79-
helmChartYAML, err := kyaml.Marshal(helmChartCR)
80-
if err != nil {
81-
return nil, fmt.Errorf("marshal helm chart CR: %w", err)
82-
}
83-
8478
// Parse the YAML as a template
85-
if err := m.templateEngine.Parse(string(helmChartYAML)); err != nil {
79+
if err := m.templateEngine.Parse(string(helmChartCR)); err != nil {
8680
return nil, fmt.Errorf("parse helm chart template: %w", err)
8781
}
8882

‎api/internal/managers/app/release/template_test.go‎

Lines changed: 24 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ func TestAppReleaseManager_ExtractAppPreflightSpec(t *testing.T) {
2222

2323
tests := []struct {
2424
name string
25-
helmChartCRs []*kotsv1beta2.HelmChart
25+
helmChartCRs [][]byte
2626
chartArchives [][]byte
2727
configValues types.AppConfigValues
2828
proxySpec *ecv1beta1.ProxySpec
@@ -32,17 +32,16 @@ func TestAppReleaseManager_ExtractAppPreflightSpec(t *testing.T) {
3232
}{
3333
{
3434
name: "no helm charts returns nil",
35-
helmChartCRs: []*kotsv1beta2.HelmChart{},
35+
helmChartCRs: [][]byte{},
3636
configValues: types.AppConfigValues{},
3737
proxySpec: &ecv1beta1.ProxySpec{},
3838
expectedSpec: nil,
3939
expectError: false,
4040
},
4141
{
4242
name: "single chart with preflight spec and templating",
43-
helmChartCRs: []*kotsv1beta2.HelmChart{
44-
createHelmChartCRFromYAML(`
45-
apiVersion: kots.io/v1beta2
43+
helmChartCRs: [][]byte{
44+
[]byte(`apiVersion: kots.io/v1beta2
4645
kind: HelmChart
4746
metadata:
4847
name: myapp-chart
@@ -114,9 +113,8 @@ spec:
114113
},
115114
{
116115
name: "multiple charts with merged preflight specs and templating",
117-
helmChartCRs: []*kotsv1beta2.HelmChart{
118-
createHelmChartCRFromYAML(`
119-
apiVersion: kots.io/v1beta2
116+
helmChartCRs: [][]byte{
117+
[]byte(`apiVersion: kots.io/v1beta2
120118
kind: HelmChart
121119
metadata:
122120
name: frontend-chart
@@ -126,8 +124,7 @@ spec:
126124
chartVersion: "1.0.0"
127125
values:
128126
versionCheckName: '{{repl ConfigOption "version_check_name"}}'`),
129-
createHelmChartCRFromYAML(`
130-
apiVersion: kots.io/v1beta2
127+
[]byte(`apiVersion: kots.io/v1beta2
131128
kind: HelmChart
132129
metadata:
133130
name: backend-chart
@@ -224,9 +221,8 @@ spec:
224221
},
225222
{
226223
name: "chart with no preflights returns empty spec",
227-
helmChartCRs: []*kotsv1beta2.HelmChart{
228-
createHelmChartCRFromYAML(`
229-
apiVersion: kots.io/v1beta2
224+
helmChartCRs: [][]byte{
225+
[]byte(`apiVersion: kots.io/v1beta2
230226
kind: HelmChart
231227
metadata:
232228
name: simple-chart
@@ -245,9 +241,8 @@ spec:
245241
},
246242
{
247243
name: "chart with proxy template functions",
248-
helmChartCRs: []*kotsv1beta2.HelmChart{
249-
createHelmChartCRFromYAML(`
250-
apiVersion: kots.io/v1beta2
244+
helmChartCRs: [][]byte{
245+
[]byte(`apiVersion: kots.io/v1beta2
251246
kind: HelmChart
252247
metadata:
253248
name: proxy-chart
@@ -369,24 +364,24 @@ spec:
369364
func TestAppReleaseManager_templateHelmChartCRs(t *testing.T) {
370365
tests := []struct {
371366
name string
372-
helmChartCRs []*kotsv1beta2.HelmChart
367+
helmChartCRs [][]byte
373368
configValues types.AppConfigValues
374369
proxySpec *ecv1beta1.ProxySpec
375370
expected []*kotsv1beta2.HelmChart
376371
expectError bool
377372
}{
378373
{
379374
name: "empty helm chart CRs",
380-
helmChartCRs: []*kotsv1beta2.HelmChart{},
375+
helmChartCRs: [][]byte{},
381376
configValues: types.AppConfigValues{},
382377
proxySpec: &ecv1beta1.ProxySpec{},
383378
expected: []*kotsv1beta2.HelmChart{},
384379
expectError: false,
385380
},
386381
{
387382
name: "single helm chart with repl templating",
388-
helmChartCRs: []*kotsv1beta2.HelmChart{
389-
createHelmChartCRFromYAML(`
383+
helmChartCRs: [][]byte{
384+
[]byte(`
390385
apiVersion: kots.io/v1beta2
391386
kind: HelmChart
392387
metadata:
@@ -451,8 +446,8 @@ spec:
451446
},
452447
{
453448
name: "multiple helm charts with mixed templating",
454-
helmChartCRs: []*kotsv1beta2.HelmChart{
455-
createHelmChartCRFromYAML(`
449+
helmChartCRs: [][]byte{
450+
[]byte(`
456451
apiVersion: kots.io/v1beta2
457452
kind: HelmChart
458453
metadata:
@@ -471,7 +466,7 @@ spec:
471466
limits:
472467
memory: 128Mi
473468
`),
474-
createHelmChartCRFromYAML(`
469+
[]byte(`
475470
apiVersion: kots.io/v1beta2
476471
kind: HelmChart
477472
metadata:
@@ -552,9 +547,9 @@ spec:
552547
},
553548
{
554549
name: "skip nil helm chart",
555-
helmChartCRs: []*kotsv1beta2.HelmChart{
550+
helmChartCRs: [][]byte{
556551
nil,
557-
createHelmChartCRFromYAML(`
552+
[]byte(`
558553
apiVersion: kots.io/v1beta2
559554
kind: HelmChart
560555
metadata:
@@ -593,8 +588,8 @@ spec:
593588
},
594589
{
595590
name: "helm chart with proxy template functions",
596-
helmChartCRs: []*kotsv1beta2.HelmChart{
597-
createHelmChartCRFromYAML(`
591+
helmChartCRs: [][]byte{
592+
[]byte(`
598593
apiVersion: kots.io/v1beta2
599594
kind: HelmChart
600595
metadata:
@@ -655,8 +650,8 @@ spec:
655650
},
656651
{
657652
name: "helm chart with empty proxy spec",
658-
helmChartCRs: []*kotsv1beta2.HelmChart{
659-
createHelmChartCRFromYAML(`
653+
helmChartCRs: [][]byte{
654+
[]byte(`
660655
apiVersion: kots.io/v1beta2
661656
kind: HelmChart
662657
metadata:

‎pkg/release/release.go‎

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,9 @@ var (
2727
)
2828

2929
// ReleaseData holds the parsed data from a Kots Release.
30+
//
31+
// Note / TODO: Custom resources (like HelmChart CRs) must be templated before they are parsed
32+
// to avoid unmarshaling errors due to invalid schema that only becomes valid after templating.
3033
type ReleaseData struct {
3134
data []byte
3235
Application *kotsv1beta1.Application
@@ -36,7 +39,7 @@ type ReleaseData struct {
3639
ChannelRelease *ChannelRelease
3740
VeleroBackup *velerov1.Backup
3841
VeleroRestore *velerov1.Restore
39-
HelmChartCRs []*kotsv1beta2.HelmChart
42+
HelmChartCRs [][]byte
4043
HelmChartArchives [][]byte
4144
}
4245

@@ -100,9 +103,9 @@ func GetChannelRelease() *ChannelRelease {
100103

101104
// GetHelmChartCRs reads and returns the HelmChart custom resources embedded as part of the release.
102105
// If no HelmChart CRs are found, returns an empty slice.
103-
func GetHelmChartCRs() []*kotsv1beta2.HelmChart {
106+
func GetHelmChartCRs() [][]byte {
104107
if _releaseData.HelmChartCRs == nil {
105-
return []*kotsv1beta2.HelmChart{}
108+
return [][]byte{}
106109
}
107110
return _releaseData.HelmChartCRs
108111
}
@@ -360,16 +363,10 @@ func (r *ReleaseData) parse() error {
360363

361364
case bytes.Contains(content.Bytes(), []byte("apiVersion: kots.io/v1beta2")):
362365
if bytes.Contains(content.Bytes(), []byte("kind: HelmChart")) {
363-
helmChart, err := parseHelmChartCR(content.Bytes())
364-
if err != nil {
365-
return fmt.Errorf("failed to parse helm chart CR: %w", err)
366-
}
367-
if helmChart != nil {
368-
if r.HelmChartCRs == nil {
369-
r.HelmChartCRs = []*kotsv1beta2.HelmChart{}
370-
}
371-
r.HelmChartCRs = append(r.HelmChartCRs, helmChart)
366+
if r.HelmChartCRs == nil {
367+
r.HelmChartCRs = [][]byte{}
372368
}
369+
r.HelmChartCRs = append(r.HelmChartCRs, content.Bytes())
373370
}
374371

375372
case bytes.Contains(content.Bytes(), []byte("# channel release object")):

‎pkg/release/release_test.go‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,11 @@ metadata:
141141
spec:
142142
chart:
143143
chartVersion: "1.0.0"
144-
name: test-chart`
144+
name: test-chart
145+
optionalValues:
146+
- when: '{{repl not (empty (ConfigOption "test-option")) }}'
147+
recursiveMerge: true
148+
values: repl{{ ConfigOption "test-option" | nindent 8 }}`
145149

146150
chartArchive := []byte("fake-chart-archive-content")
147151

@@ -178,6 +182,7 @@ spec:
178182
// Verify HelmChart CRs
179183
assert.Len(t, release.HelmChartCRs, 1)
180184
assert.NotNil(t, release.HelmChartCRs[0])
185+
assert.Greater(t, len(release.HelmChartCRs[0]), 0)
181186

182187
// Verify chart archives
183188
assert.Len(t, release.HelmChartArchives, 1)

0 commit comments

Comments
 (0)