Skip to content

Commit c3f6ee7

Browse files
relurelu
authored
Merge pull request #244 from fluxcd/fix-values-file-path-support
Fix HelmChart valuesFile chart path restriction
2 parents a5032a0 + a55c502 commit c3f6ee7

File tree

6 files changed

+214
-132
lines changed

6 files changed

+214
-132
lines changed

‎config/testdata/helmchart-valuesfile/helmchart_gitrepository.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,4 +8,4 @@ spec:
88
kind: GitRepository
99
name: podinfo
1010
chart: charts/podinfo
11-
valuesFile: values-prod.yaml
11+
valuesFile: charts/podinfo/values-prod.yaml

‎controllers/helmchart_controller.go‎

Lines changed: 33 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
"io/ioutil"
2323
"net/url"
2424
"os"
25+
"path/filepath"
2526
"regexp"
2627
"strings"
2728
"time"
@@ -378,8 +379,18 @@ func (r *HelmChartReconciler) reconcileFromHelmRepository(ctx context.Context,
378379
return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err
379380
}
380381

382+
// Find override file and retrieve contents
383+
var valuesData []byte
384+
cfn := filepath.Clean(chart.Spec.ValuesFile)
385+
for _, f := range helmChart.Files {
386+
if f.Name == cfn {
387+
valuesData = f.Data
388+
break
389+
}
390+
}
391+
381392
// Overwrite values file
382-
if changed, err := helm.OverwriteChartDefaultValues(helmChart, chart.Spec.ValuesFile); err != nil {
393+
if changed, err := helm.OverwriteChartDefaultValues(helmChart, valuesData); err != nil {
383394
return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPackageFailedReason, err.Error()), err
384395
} else if !changed {
385396
// No changes, skip to write original package to storage
@@ -483,9 +494,27 @@ func (r *HelmChartReconciler) reconcileFromTarballArtifact(ctx context.Context,
483494
// or write the chart directly to storage.
484495
pkgPath := chartPath
485496
isValuesFileOverriden := false
486-
if chart.Spec.ValuesFile != "" && chart.Spec.ValuesFile != chartutil.ValuesfileName {
487-
// Overwrite default values if configured
488-
isValuesFileOverriden, err = helm.OverwriteChartDefaultValues(helmChart, chart.Spec.ValuesFile)
497+
if chart.Spec.ValuesFile != "" {
498+
srcPath, err := securejoin.SecureJoin(tmpDir, chart.Spec.ValuesFile)
499+
if err != nil {
500+
return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err
501+
}
502+
if f, err := os.Stat(srcPath); os.IsNotExist(err) || !f.Mode().IsRegular() {
503+
err = fmt.Errorf("invalid values file path: %s", chart.Spec.ValuesFile)
504+
return chart, err
505+
}
506+
src, err := os.Open(srcPath)
507+
if err != nil {
508+
err = fmt.Errorf("failed to open values file '%s': %w", chart.Spec.ValuesFile, err)
509+
return chart, err
510+
}
511+
defer src.Close()
512+
513+
var valuesData []byte
514+
if _, err := src.Read(valuesData); err == nil {
515+
isValuesFileOverriden, err = helm.OverwriteChartDefaultValues(helmChart, valuesData)
516+
}
517+
489518
if err != nil {
490519
return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPackageFailedReason, err.Error()), err
491520
}

‎controllers/helmchart_controller_test.go‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -693,6 +693,31 @@ var _ = Describe("HelmChartReconciler", func() {
693693
return got.Status.Artifact != nil &&
694694
storage.ArtifactExist(*got.Status.Artifact)
695695
}, timeout, interval).Should(BeTrue())
696+
697+
When("Setting valid valuesFile attribute", func() {
698+
updated := &sourcev1.HelmChart{}
699+
Expect(k8sClient.Get(context.Background(), key, updated)).To(Succeed())
700+
chart.Spec.ValuesFile = "./charts/helmchart/override.yaml"
701+
Expect(k8sClient.Update(context.Background(), updated)).To(Succeed())
702+
got := &sourcev1.HelmChart{}
703+
Eventually(func() bool {
704+
_ = k8sClient.Get(context.Background(), key, got)
705+
return got.Status.Artifact != nil &&
706+
storage.ArtifactExist(*got.Status.Artifact)
707+
}, timeout, interval).Should(BeTrue())
708+
})
709+
710+
When("Setting invalid valuesFile attribute", func() {
711+
updated := &sourcev1.HelmChart{}
712+
Expect(k8sClient.Get(context.Background(), key, updated)).To(Succeed())
713+
chart.Spec.ValuesFile = "invalid.yaml"
714+
Expect(k8sClient.Update(context.Background(), updated)).To(Succeed())
715+
got := &sourcev1.HelmChart{}
716+
Eventually(func() bool {
717+
_ = k8sClient.Get(context.Background(), key, got)
718+
return got.Status.Artifact != nil && got.Status.Artifact.Revision == updated.Status.Artifact.Revision
719+
}, timeout, interval).Should(BeTrue())
720+
})
696721
})
697722
})
698723

@@ -936,6 +961,31 @@ var _ = Describe("HelmChartReconciler", func() {
936961
return got.Status.Artifact != nil &&
937962
storage.ArtifactExist(*got.Status.Artifact)
938963
}, timeout, interval).Should(BeTrue())
964+
965+
When("Setting valid valuesFile attribute", func() {
966+
updated := &sourcev1.HelmChart{}
967+
Expect(k8sClient.Get(context.Background(), key, updated)).To(Succeed())
968+
chart.Spec.ValuesFile = "override.yaml"
969+
Expect(k8sClient.Update(context.Background(), updated)).To(Succeed())
970+
got := &sourcev1.HelmChart{}
971+
Eventually(func() bool {
972+
_ = k8sClient.Get(context.Background(), key, got)
973+
return got.Status.Artifact != nil &&
974+
storage.ArtifactExist(*got.Status.Artifact)
975+
}, timeout, interval).Should(BeTrue())
976+
})
977+
978+
When("Setting invalid valuesFile attribute", func() {
979+
updated := &sourcev1.HelmChart{}
980+
Expect(k8sClient.Get(context.Background(), key, updated)).To(Succeed())
981+
chart.Spec.ValuesFile = "./charts/helmchart/override.yaml"
982+
Expect(k8sClient.Update(context.Background(), updated)).To(Succeed())
983+
got := &sourcev1.HelmChart{}
984+
Eventually(func() bool {
985+
_ = k8sClient.Get(context.Background(), key, got)
986+
return got.Status.Artifact != nil && got.Status.Artifact.Revision == updated.Status.Artifact.Revision
987+
}, timeout, interval).Should(BeTrue())
988+
})
939989
})
940990
})
941991
})

‎controllers/testdata/charts/helmchart/override.yaml‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
# Override values for helmchart.
2+
# This is a YAML-formatted file.
3+
# Declare variables to be passed into your templates.
4+
5+
replicaCount: 3
6+
7+
image:
8+
repository: nginx
9+
pullPolicy: IfNotPresent
10+
11+
imagePullSecrets: []
12+
nameOverride: ""
13+
fullnameOverride: ""
14+
15+
serviceAccount:
16+
# Specifies whether a service account should be created
17+
create: true
18+
# The name of the service account to use.
19+
# If not set and create is true, a name is generated using the fullname template
20+
name:
21+
22+
podSecurityContext: {}
23+
# fsGroup: 2000
24+
25+
securityContext: {}
26+
# capabilities:
27+
# drop:
28+
# - ALL
29+
# readOnlyRootFilesystem: true
30+
# runAsNonRoot: true
31+
# runAsUser: 1000
32+
33+
service:
34+
type: ClusterIP
35+
port: 80
36+
37+
ingress:
38+
enabled: false
39+
annotations: {}
40+
# kubernetes.io/ingress.class: nginx
41+
# kubernetes.io/tls-acme: "true"
42+
hosts:
43+
- host: chart-example.local
44+
paths: []
45+
tls: []
46+
# - secretName: chart-example-tls
47+
# hosts:
48+
# - chart-example.local
49+
50+
resources: {}
51+
# We usually recommend not to specify default resources and to leave this as a conscious
52+
# choice for the user. This also increases chances charts run on environments with little
53+
# resources, such as Minikube. If you do want to specify resources, uncomment the following
54+
# lines, adjust them as necessary, and remove the curly braces after 'resources:'.
55+
# limits:
56+
# cpu: 100m
57+
# memory: 128Mi
58+
# requests:
59+
# cpu: 100m
60+
# memory: 128Mi
61+
62+
nodeSelector: {}
63+
64+
tolerations: []
65+
66+
affinity: {}

‎internal/helm/chart.go‎

Lines changed: 7 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -25,46 +25,30 @@ import (
2525
)
2626

2727
// OverwriteChartDefaultValues overwrites the chart default values file with the
28-
// contents of the given valuesFile.
29-
func OverwriteChartDefaultValues(chart *helmchart.Chart, valuesFile string) (bool, error) {
30-
if valuesFile == "" || valuesFile == chartutil.ValuesfileName {
31-
return false, nil
32-
}
33-
34-
// Find override file and retrieve contents
35-
var valuesData []byte
36-
for _, f := range chart.Files {
37-
if f.Name == valuesFile {
38-
valuesData = f.Data
39-
break
40-
}
41-
}
42-
if valuesData == nil {
43-
return false, fmt.Errorf("failed to locate override values file: %s", valuesFile)
44-
}
45-
28+
// given data.
29+
func OverwriteChartDefaultValues(chart *helmchart.Chart, data []byte) (bool, error) {
4630
// Read override values file data
47-
values, err := chartutil.ReadValues(valuesData)
31+
values, err := chartutil.ReadValues(data)
4832
if err != nil {
49-
return false, fmt.Errorf("failed to parse override values file: %s", valuesFile)
33+
return false, fmt.Errorf("failed to parse provided override values file data")
5034
}
5135

5236
// Replace current values file in Raw field
5337
for _, f := range chart.Raw {
5438
if f.Name == chartutil.ValuesfileName {
5539
// Do nothing if contents are equal
56-
if reflect.DeepEqual(f.Data, valuesData) {
40+
if reflect.DeepEqual(f.Data, data) {
5741
return false, nil
5842
}
5943

6044
// Replace in Files field
6145
for _, f := range chart.Files {
6246
if f.Name == chartutil.ValuesfileName {
63-
f.Data = valuesData
47+
f.Data = data
6448
}
6549
}
6650

67-
f.Data = valuesData
51+
f.Data = data
6852
chart.Values = values
6953
return true, nil
7054
}

0 commit comments

Comments
 (0)