Skip to content

Commit 067854e

Browse files
author
Paulo Gomes
committed
Add validation to TargetPath and ValuesKey
Formalises the API requirements around TargetPath and ValuesKey, which were the two fields missing validation within ValuesReference. In both cases the validation was introduced at CRD level, so that the apiserver will enforce it. ValuesKey must be a valid Data Key. Therefore the same logic used by upstream Kubernetes is reused here to ensure a valid key is being used. For TargetPath a loose regex is being used to largely represent the expected format. A max length of 250 is now being enforced. This is a breaking change, as invalid TargetPath and ValuesKey will now be rejected by the apiserver, instead of being accepted and potentially failing at reconciliation time. Signed-off-by: Paulo Gomes <paulo.gomes@weave.works>
1 parent 2195310 commit 067854e

File tree

7 files changed

+205
-47
lines changed

7 files changed

+205
-47
lines changed

‎api/v2beta1/reference_types.go

+6
Original file line numberDiff line numberDiff line change
@@ -59,12 +59,18 @@ type ValuesReference struct {
5959

6060
// ValuesKey is the data key where the values.yaml or a specific value can be
6161
// found at. Defaults to 'values.yaml'.
62+
// When set, must be a valid Data Key, consisting of alphanumeric characters,
63+
// '-', '_' or '.'.
64+
// +kubebuilder:validation:MaxLength=253
65+
// +kubebuilder:validation:Pattern=`^[\-._a-zA-Z0-9]+$`
6266
// +optional
6367
ValuesKey string `json:"valuesKey,omitempty"`
6468

6569
// TargetPath is the YAML dot notation path the value should be merged at. When
6670
// set, the ValuesKey is expected to be a single flat value. Defaults to 'None',
6771
// which results in the values getting merged at the root.
72+
// +kubebuilder:validation:MaxLength=250
73+
// +kubebuilder:validation:Pattern=`^([a-zA-Z0-9_\-.\\\/]|\[[0-9]{1,5}\])+$`
6874
// +optional
6975
TargetPath string `json:"targetPath,omitempty"`
7076

‎config/crd/bases/helm.toolkit.fluxcd.io_helmreleases.yaml

+6
Original file line numberDiff line numberDiff line change
@@ -697,10 +697,16 @@ spec:
697697
should be merged at. When set, the ValuesKey is expected to
698698
be a single flat value. Defaults to 'None', which results
699699
in the values getting merged at the root.
700+
maxLength: 250
701+
pattern: ^([a-zA-Z0-9_\-.\\\/]|\[[0-9]{1,5}\])+$
700702
type: string
701703
valuesKey:
702704
description: ValuesKey is the data key where the values.yaml
703705
or a specific value can be found at. Defaults to 'values.yaml'.
706+
When set, must be a valid Data Key, consisting of alphanumeric
707+
characters, '-', '_' or '.'.
708+
maxLength: 253
709+
pattern: ^[\-._a-zA-Z0-9]+$
704710
type: string
705711
required:
706712
- kind

‎controllers/helmrelease_controller_test.go

+171
Original file line numberDiff line numberDiff line change
@@ -19,14 +19,17 @@ package controllers
1919
import (
2020
"context"
2121
"reflect"
22+
"strings"
2223
"testing"
24+
"time"
2325

2426
"github.com/go-logr/logr"
2527
"helm.sh/helm/v3/pkg/chartutil"
2628
corev1 "k8s.io/api/core/v1"
2729
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
2830
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2931
"k8s.io/apimachinery/pkg/runtime"
32+
"sigs.k8s.io/controller-runtime/pkg/client"
3033
"sigs.k8s.io/controller-runtime/pkg/client/fake"
3134
"sigs.k8s.io/yaml"
3235

@@ -278,6 +281,174 @@ invalid`,
278281
}
279282
}
280283

284+
func TestValuesReferenceValidation(t *testing.T) {
285+
tests := []struct {
286+
name string
287+
references []v2.ValuesReference
288+
values string
289+
wantErr bool
290+
}{
291+
{
292+
name: "valid ValuesKey",
293+
references: []v2.ValuesReference{
294+
{
295+
Kind: "Secret",
296+
Name: "values",
297+
ValuesKey: "any-key_na.me",
298+
},
299+
},
300+
wantErr: false,
301+
},
302+
{
303+
name: "valid ValuesKey: empty",
304+
references: []v2.ValuesReference{
305+
{
306+
Kind: "Secret",
307+
Name: "values",
308+
ValuesKey: "",
309+
},
310+
},
311+
wantErr: false,
312+
},
313+
{
314+
name: "valid ValuesKey: long",
315+
references: []v2.ValuesReference{
316+
{
317+
Kind: "Secret",
318+
Name: "values",
319+
ValuesKey: strings.Repeat("a", 253),
320+
},
321+
},
322+
wantErr: false,
323+
},
324+
{
325+
name: "invalid ValuesKey",
326+
references: []v2.ValuesReference{
327+
{
328+
Kind: "Secret",
329+
Name: "values",
330+
ValuesKey: "a($&^%b",
331+
},
332+
},
333+
wantErr: true,
334+
},
335+
{
336+
name: "invalid ValuesKey: too long",
337+
references: []v2.ValuesReference{
338+
{
339+
Kind: "Secret",
340+
Name: "values",
341+
ValuesKey: strings.Repeat("a", 254),
342+
},
343+
},
344+
wantErr: true,
345+
},
346+
{
347+
name: "valid target path: empty",
348+
references: []v2.ValuesReference{
349+
{
350+
Kind: "Secret",
351+
Name: "values",
352+
TargetPath: "",
353+
},
354+
},
355+
wantErr: false,
356+
},
357+
{
358+
name: "valid target path",
359+
references: []v2.ValuesReference{
360+
{
361+
Kind: "Secret",
362+
Name: "values",
363+
TargetPath: "list_with.nested-values.and.index[0]",
364+
},
365+
},
366+
wantErr: false,
367+
},
368+
{
369+
name: "valid target path: long",
370+
references: []v2.ValuesReference{
371+
{
372+
Kind: "Secret",
373+
Name: "values",
374+
TargetPath: func() string { return strings.Repeat("a", 250) }(),
375+
},
376+
},
377+
wantErr: false,
378+
},
379+
{
380+
name: "invalid target path: too long",
381+
references: []v2.ValuesReference{
382+
{
383+
Kind: "Secret",
384+
Name: "values",
385+
TargetPath: func() string { return strings.Repeat("a", 251) }(),
386+
},
387+
},
388+
wantErr: true,
389+
},
390+
{
391+
name: "invalid target path: opened index",
392+
references: []v2.ValuesReference{
393+
{
394+
Kind: "Secret",
395+
Name: "values",
396+
ValuesKey: "single",
397+
TargetPath: "a[",
398+
},
399+
},
400+
wantErr: true,
401+
},
402+
{
403+
name: "invalid target path: incorrect index syntax",
404+
references: []v2.ValuesReference{
405+
{
406+
Kind: "Secret",
407+
Name: "values",
408+
ValuesKey: "single",
409+
TargetPath: "a]0[",
410+
},
411+
},
412+
wantErr: true,
413+
},
414+
}
415+
416+
for _, tt := range tests {
417+
t.Run(tt.name, func(t *testing.T) {
418+
var values *apiextensionsv1.JSON
419+
if tt.values != "" {
420+
v, _ := yaml.YAMLToJSON([]byte(tt.values))
421+
values = &apiextensionsv1.JSON{Raw: v}
422+
}
423+
424+
hr := v2.HelmRelease{
425+
ObjectMeta: metav1.ObjectMeta{
426+
Name: "test",
427+
Namespace: "default",
428+
},
429+
Spec: v2.HelmReleaseSpec{
430+
Interval: metav1.Duration{Duration: 5 * time.Minute},
431+
Chart: v2.HelmChartTemplate{
432+
Spec: v2.HelmChartTemplateSpec{
433+
SourceRef: v2.CrossNamespaceObjectReference{
434+
Name: "something",
435+
},
436+
},
437+
},
438+
ValuesFrom: tt.references,
439+
Values: values,
440+
},
441+
}
442+
443+
err := k8sClient.Create(context.TODO(), &hr, client.DryRunAll)
444+
if (err != nil) != tt.wantErr {
445+
t.Errorf("composeValues() error = %v, wantErr %v", err, tt.wantErr)
446+
return
447+
}
448+
})
449+
}
450+
}
451+
281452
func valuesSecret(name string, data map[string][]byte) *corev1.Secret {
282453
return &corev1.Secret{
283454
ObjectMeta: metav1.ObjectMeta{Name: name},

‎controllers/suite_test.go

+19-38
Original file line numberDiff line numberDiff line change
@@ -17,67 +17,48 @@ limitations under the License.
1717
package controllers
1818

1919
import (
20+
"fmt"
21+
"os"
2022
"path/filepath"
2123
"testing"
2224

23-
. "github.com/onsi/ginkgo"
24-
. "github.com/onsi/gomega"
25+
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
2526
"k8s.io/client-go/kubernetes/scheme"
2627
"k8s.io/client-go/rest"
2728
"sigs.k8s.io/controller-runtime/pkg/client"
2829
"sigs.k8s.io/controller-runtime/pkg/envtest"
29-
"sigs.k8s.io/controller-runtime/pkg/envtest/printer"
30-
logf "sigs.k8s.io/controller-runtime/pkg/log"
31-
"sigs.k8s.io/controller-runtime/pkg/log/zap"
3230

3331
"github.com/fluxcd/helm-controller/api/v2beta1"
3432
// +kubebuilder:scaffold:imports
3533
)
3634

37-
// These tests use Ginkgo (BDD-style Go testing framework). Refer to
38-
// http://onsi.github.io/ginkgo/ to learn more about Ginkgo.
39-
4035
var cfg *rest.Config
4136
var k8sClient client.Client
4237
var testEnv *envtest.Environment
4338

44-
func TestAPIs(t *testing.T) {
45-
RegisterFailHandler(Fail)
46-
47-
RunSpecsWithDefaultAndCustomReporters(t,
48-
"Controller Suite",
49-
[]Reporter{printer.NewlineReporter{}})
50-
}
51-
52-
var _ = BeforeSuite(func(done Done) {
53-
logf.SetLogger(
54-
zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true)),
55-
)
56-
57-
By("bootstrapping test environment")
39+
func TestMain(m *testing.M) {
5840
testEnv = &envtest.Environment{
5941
CRDDirectoryPaths: []string{filepath.Join("..", "config", "crd", "bases")},
6042
}
6143

6244
var err error
6345
cfg, err = testEnv.Start()
64-
Expect(err).ToNot(HaveOccurred())
65-
Expect(cfg).ToNot(BeNil())
66-
67-
err = v2beta1.AddToScheme(scheme.Scheme)
68-
Expect(err).NotTo(HaveOccurred())
69-
70-
// +kubebuilder:scaffold:scheme
46+
if err != nil {
47+
panic(fmt.Errorf("failed to start testenv: %v", err))
48+
}
7149

50+
utilruntime.Must(v2beta1.AddToScheme(scheme.Scheme))
7251
k8sClient, err = client.New(cfg, client.Options{Scheme: scheme.Scheme})
73-
Expect(err).ToNot(HaveOccurred())
74-
Expect(k8sClient).ToNot(BeNil())
52+
if err != nil {
53+
panic(fmt.Errorf("failed to create k8s client: %v", err))
54+
}
7555

76-
close(done)
77-
}, 60)
56+
code := m.Run()
7857

79-
var _ = AfterSuite(func() {
80-
By("tearing down the test environment")
81-
err := testEnv.Stop()
82-
Expect(err).ToNot(HaveOccurred())
83-
})
58+
err = testEnv.Stop()
59+
if err != nil {
60+
panic(fmt.Errorf("failed to stop testenv: %v", err))
61+
}
62+
63+
os.Exit(code)
64+
}

‎docs/api/helmrelease.md

+3-1
Original file line numberDiff line numberDiff line change
@@ -2092,7 +2092,9 @@ string
20922092
<td>
20932093
<em>(Optional)</em>
20942094
<p>ValuesKey is the data key where the values.yaml or a specific value can be
2095-
found at. Defaults to &lsquo;values.yaml&rsquo;.</p>
2095+
found at. Defaults to &lsquo;values.yaml&rsquo;.
2096+
When set, must be a valid Data Key, consisting of alphanumeric characters,
2097+
&lsquo;-&rsquo;, &lsquo;_&rsquo; or &lsquo;.&rsquo;.</p>
20962098
</td>
20972099
</tr>
20982100
<tr>

‎go.mod

-3
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ require (
1313
github.com/fluxcd/source-controller/api v0.25.9
1414
github.com/go-logr/logr v1.2.3
1515
github.com/hashicorp/go-retryablehttp v0.7.1
16-
github.com/onsi/ginkgo v1.16.5
1716
github.com/onsi/gomega v1.19.0
1817
github.com/spf13/pflag v1.0.5
1918
helm.sh/helm/v3 v3.9.0
@@ -120,7 +119,6 @@ require (
120119
github.com/monochromegane/go-gitignore v0.0.0-20200626010858-205db1a8cc00 // indirect
121120
github.com/morikuni/aec v1.0.0 // indirect
122121
github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect
123-
github.com/nxadm/tail v1.4.8 // indirect
124122
github.com/opencontainers/go-digest v1.0.0 // indirect
125123
github.com/opencontainers/image-spec v1.0.3-0.20211202183452-c5a74bcca799 // indirect
126124
github.com/peterbourgon/diskv v2.0.1+incompatible // indirect
@@ -157,7 +155,6 @@ require (
157155
google.golang.org/grpc v1.43.0 // indirect
158156
google.golang.org/protobuf v1.28.0 // indirect
159157
gopkg.in/inf.v0 v0.9.1 // indirect
160-
gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7 // indirect
161158
gopkg.in/yaml.v2 v2.4.0 // indirect
162159
gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b // indirect
163160
k8s.io/apiserver v0.24.1 // indirect

‎go.sum

-5
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,6 @@ github.com/go-sql-driver/mysql v1.4.1/go.mod h1:zAC/RDZ24gD3HViQzih4MyKcchzm+sOG
259259
github.com/go-sql-driver/mysql v1.5.0 h1:ozyZYNQW3x3HtqT1jira07DN2PArx2v7/mN66gGcHOs=
260260
github.com/go-sql-driver/mysql v1.5.0/go.mod h1:DCzpHaOWr8IXmIStZouvnhqoel9Qv2LBy8hT2VhHyBg=
261261
github.com/go-stack/stack v1.8.0/go.mod h1:v0f6uXyyMGvRgIKkXu+yp6POWl0qKG85gN/melR3HDY=
262-
github.com/go-task/slim-sprig v0.0.0-20210107165309-348f09dbbbc0/go.mod h1:fyg7847qk6SyHyPtNmDHnmrv/HOrqktSC+C9fM+CJOE=
263262
github.com/gobuffalo/logger v1.0.6 h1:nnZNpxYo0zx+Aj9RfMPBm+x9zAU2OayFh/xrAWi34HU=
264263
github.com/gobuffalo/logger v1.0.6/go.mod h1:J31TBEHR1QLV2683OXTAItYIg8pv2JMHnF/quuAbMjs=
265264
github.com/gobuffalo/packd v1.0.1 h1:U2wXfRr4E9DH8IdsDLlRFwTZTK7hLfq9qT/QHXGVe/0=
@@ -550,7 +549,6 @@ github.com/mxk/go-flowrate v0.0.0-20140419014527-cca7078d478f/go.mod h1:ZdcZmHo+
550549
github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e/go.mod h1:zD1mROLANZcx1PVRCS0qkT7pwLkGfwJo4zjcN/Tysno=
551550
github.com/nxadm/tail v1.4.4/go.mod h1:kenIhsEOeOJmVchQTgglprH7qJGnHDVpk1VPCcaMI8A=
552551
github.com/nxadm/tail v1.4.8 h1:nPr65rt6Y5JFSKQO7qToXr7pePgD6Gwiw05lkbyAQTE=
553-
github.com/nxadm/tail v1.4.8/go.mod h1:+ncqLTQzXmGhMZNUePPaPqPvBxHAIsmXswZKocGu+AU=
554552
github.com/oklog/ulid v1.3.1/go.mod h1:CirwcVhetQ6Lv90oh/F+FBtV6XMibvdAFo93nm5qn4U=
555553
github.com/olekukonko/tablewriter v0.0.4/go.mod h1:zq6QwlOf5SlnkVbMSr5EoBv3636FWnp+qbPhuoO21uA=
556554
github.com/olekukonko/tablewriter v0.0.5/go.mod h1:hPp6KlRPjbx+hW8ykQs1w3UBbZlj6HuIJcUGPhkA7kY=
@@ -559,7 +557,6 @@ github.com/onsi/ginkgo v1.6.0/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+W
559557
github.com/onsi/ginkgo v1.12.1/go.mod h1:zj2OWP4+oCPe1qIXoGWkgMRwljMUYCdkwsT2108oapk=
560558
github.com/onsi/ginkgo v1.14.0/go.mod h1:iSB4RoI2tjJc9BBv4NKIKWKya62Rps+oPG/Lv9klQyY=
561559
github.com/onsi/ginkgo v1.16.5 h1:8xi0RTUf59SOSfEtZMvwTvXYMzG4gV23XVHOZiXNtnE=
562-
github.com/onsi/ginkgo v1.16.5/go.mod h1:+E8gABHa3K6zRBolWtd+ROzc/U5bkGt0FwiG042wbpU=
563560
github.com/onsi/ginkgo/v2 v2.1.3 h1:e/3Cwtogj0HA+25nMP1jCMDIf8RtRYbGwGGuBIFztkc=
564561
github.com/onsi/gomega v0.0.0-20170829124025-dcabb60a477c/go.mod h1:C1qb7wdrVGGVU+Z6iS04AVkA3Q65CEZX59MT0QO5uiA=
565562
github.com/onsi/gomega v1.7.1/go.mod h1:XdKZgCCFLUoM/7CFJVPcG8C1xQ1AJ0vpAezJrB7JYyY=
@@ -933,7 +930,6 @@ golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7w
933930
golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
934931
golang.org/x/sys v0.0.0-20201201145000-ef89a241ccb3/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
935932
golang.org/x/sys v0.0.0-20210104204734-6f8348627aad/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
936-
golang.org/x/sys v0.0.0-20210112080510-489259a85091/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
937933
golang.org/x/sys v0.0.0-20210119212857-b64e53b001e4/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
938934
golang.org/x/sys v0.0.0-20210124154548-22da62e12c0c/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
939935
golang.org/x/sys v0.0.0-20210220050731-9a76102bfb43/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
@@ -1034,7 +1030,6 @@ golang.org/x/tools v0.0.0-20200904185747-39188db58858/go.mod h1:Cj7w3i3Rnn0Xh82u
10341030
golang.org/x/tools v0.0.0-20201110124207-079ba7bd75cd/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA=
10351031
golang.org/x/tools v0.0.0-20201201161351-ac6f37ff4c2a/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA=
10361032
golang.org/x/tools v0.0.0-20201208233053-a543418bbed2/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA=
1037-
golang.org/x/tools v0.0.0-20201224043029-2b0845dc783e/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA=
10381033
golang.org/x/tools v0.0.0-20210105154028-b0ab187a4818/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA=
10391034
golang.org/x/tools v0.0.0-20210106214847-113979e3529a/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA=
10401035
golang.org/x/tools v0.1.0/go.mod h1:xkSsbof2nBLbhDlRMhhhyNLN/zl3eTqcnHD5viDpcZ0=

0 commit comments

Comments
 (0)