From f398253cf565df506c46c021d8fc3d0408b2b97c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Charles-Edouard=20Br=C3=A9t=C3=A9ch=C3=A9?= Date: Tue, 5 Mar 2024 14:03:08 +0100 Subject: [PATCH 1/2] fix: add unit tests and config as forbidden binding name (#1043) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: add unit tests and config as forbidden binding name Signed-off-by: Charles-Edouard Brétéché * release notes Signed-off-by: Charles-Edouard Brétéché --------- Signed-off-by: Charles-Edouard Brétéché --- .release-notes/main.md | 6 +- pkg/apis/v1alpha1/binding.go | 2 +- pkg/apis/v1alpha1/binding_test.go | 99 +++++++++++++++++++++++++++++++ 3 files changed, 104 insertions(+), 3 deletions(-) create mode 100644 pkg/apis/v1alpha1/binding_test.go diff --git a/.release-notes/main.md b/.release-notes/main.md index df877731f..c59dcb399 100644 --- a/.release-notes/main.md +++ b/.release-notes/main.md @@ -13,9 +13,11 @@ Release notes for `TODO`. ## ⛵ Tutorials ⛵ -## 🔧 Fixes 🔧 - ## 📚 Docs 📚 ## 🎸 Misc 🎸 --> + +## 🔧 Fixes 🔧 + +- Added `config` in the list of forbidden binding names \ No newline at end of file diff --git a/pkg/apis/v1alpha1/binding.go b/pkg/apis/v1alpha1/binding.go index b6eb183e8..330439edc 100644 --- a/pkg/apis/v1alpha1/binding.go +++ b/pkg/apis/v1alpha1/binding.go @@ -10,7 +10,7 @@ import ( var ( identifier = regexp.MustCompile(`^\w+$`) - forbiddenNames = []string{"namespace", "client", "error", "values", "stdout", "stderr"} + forbiddenNames = []string{"namespace", "client", "config", "error", "values", "stdout", "stderr"} forbidden = sets.New(forbiddenNames...) ) diff --git a/pkg/apis/v1alpha1/binding_test.go b/pkg/apis/v1alpha1/binding_test.go new file mode 100644 index 000000000..1630e5537 --- /dev/null +++ b/pkg/apis/v1alpha1/binding_test.go @@ -0,0 +1,99 @@ +package v1alpha1 + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestBinding_CheckName(t *testing.T) { + tests := []struct { + name string + bindingName string + bindingValue Any + wantErr bool + }{{ + name: "empty", + wantErr: true, + }, { + name: "simple", + bindingName: "simple", + wantErr: false, + }, { + name: "with dollar", + bindingName: "$simple", + wantErr: true, + }, { + name: "with space", + bindingName: "simple one", + wantErr: true, + }, { + name: "with dot", + bindingName: "simple.one", + wantErr: true, + }, { + name: "forbidden", + bindingName: "config", + wantErr: true, + }} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + b := Binding{ + Name: tt.bindingName, + Value: tt.bindingValue, + } + err := b.CheckName() + if tt.wantErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + }) + } +} + +func TestBinding_CheckEnvName(t *testing.T) { + tests := []struct { + name string + bindingName string + bindingValue Any + wantErr bool + }{{ + name: "empty", + wantErr: true, + }, { + name: "simple", + bindingName: "simple", + wantErr: false, + }, { + name: "with dollar", + bindingName: "$simple", + wantErr: true, + }, { + name: "with space", + bindingName: "simple one", + wantErr: true, + }, { + name: "with dot", + bindingName: "simple.one", + wantErr: true, + }, { + name: "forbidden", + bindingName: "config", + wantErr: false, + }} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + b := Binding{ + Name: tt.bindingName, + Value: tt.bindingValue, + } + err := b.CheckEnvName() + if tt.wantErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + }) + } +} From 7da4d799f03027f1a34a8a8f86ca63a3ec2a2276 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Charles-Edouard=20Br=C3=A9t=C3=A9ch=C3=A9?= Date: Tue, 5 Mar 2024 14:43:11 +0100 Subject: [PATCH 2/2] chore: add validation unit tests (#1044) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Charles-Edouard Brétéché --- pkg/validation/test/binding_test.go | 91 +++++++++++++++++++ pkg/validation/test/for_test.go | 74 +++++++++++++++ .../test/resource_reference_test.go | 87 ++++++++++++++++++ pkg/validation/test/test_spec_test.go | 34 +++++++ pkg/validation/test/test_test.go | 27 +++--- 5 files changed, 297 insertions(+), 16 deletions(-) create mode 100644 pkg/validation/test/binding_test.go create mode 100644 pkg/validation/test/for_test.go create mode 100644 pkg/validation/test/resource_reference_test.go create mode 100644 pkg/validation/test/test_spec_test.go diff --git a/pkg/validation/test/binding_test.go b/pkg/validation/test/binding_test.go new file mode 100644 index 000000000..163140dbc --- /dev/null +++ b/pkg/validation/test/binding_test.go @@ -0,0 +1,91 @@ +package test + +import ( + "testing" + + "github.com/kyverno/chainsaw/pkg/apis/v1alpha1" + "github.com/stretchr/testify/assert" + "k8s.io/apimachinery/pkg/util/validation/field" +) + +func TestValidateBinding(t *testing.T) { + tests := []struct { + name string + path *field.Path + obj v1alpha1.Binding + want field.ErrorList + }{{ + name: "empty", + path: field.NewPath("foo"), + want: field.ErrorList{ + &field.Error{ + Type: field.ErrorTypeInvalid, + Field: "foo.name", + BadValue: "", + Detail: "invalid name ", + }, + }, + }, { + name: "valid name", + path: field.NewPath("foo"), + obj: v1alpha1.Binding{ + Name: "foo", + }, + }, { + name: "invalid name", + path: field.NewPath("foo"), + obj: v1alpha1.Binding{ + Name: "$foo", + }, + want: field.ErrorList{ + &field.Error{ + Type: field.ErrorTypeInvalid, + Field: "foo.name", + BadValue: "$foo", + Detail: "invalid name $foo", + }, + }, + }} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := ValidateBinding(tt.path, tt.obj) + assert.Equal(t, tt.want, got) + }) + } +} + +func TestValidateBindings(t *testing.T) { + tests := []struct { + name string + path *field.Path + objs []v1alpha1.Binding + want field.ErrorList + }{{ + name: "null", + }, { + name: "empty", + objs: []v1alpha1.Binding{}, + }, { + name: "valid", + objs: []v1alpha1.Binding{{ + Name: "foo", + }}, + }, { + name: "invalid", + objs: []v1alpha1.Binding{{}}, + want: field.ErrorList{ + &field.Error{ + Type: field.ErrorTypeInvalid, + Field: "[0].name", + BadValue: "", + Detail: "invalid name ", + }, + }, + }} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := ValidateBindings(tt.path, tt.objs...) + assert.Equal(t, tt.want, got) + }) + } +} diff --git a/pkg/validation/test/for_test.go b/pkg/validation/test/for_test.go new file mode 100644 index 000000000..f98684ca2 --- /dev/null +++ b/pkg/validation/test/for_test.go @@ -0,0 +1,74 @@ +package test + +import ( + "testing" + + "github.com/kyverno/chainsaw/pkg/apis/v1alpha1" + "github.com/stretchr/testify/assert" + "k8s.io/apimachinery/pkg/util/validation/field" +) + +func TestValidateFor(t *testing.T) { + tests := []struct { + name string + path *field.Path + obj *v1alpha1.For + want field.ErrorList + }{{ + name: "null", + }, { + name: "empty", + path: field.NewPath("for"), + obj: &v1alpha1.For{}, + want: field.ErrorList{ + &field.Error{ + Type: field.ErrorTypeInvalid, + Field: "for", + BadValue: &v1alpha1.For{}, + Detail: "either a deletion or a condition must be specified", + }, + }, + }, { + name: "no condition name", + path: field.NewPath("for"), + obj: &v1alpha1.For{ + Condition: &v1alpha1.Condition{}, + }, + want: field.ErrorList{ + &field.Error{ + Type: field.ErrorTypeInvalid, + Field: "for.condition.name", + BadValue: &v1alpha1.For{Condition: &v1alpha1.Condition{}}, + Detail: "a condition name must be specified", + }, + }, + }, { + name: "both condition and deletion", + path: field.NewPath("for"), + obj: &v1alpha1.For{ + Condition: &v1alpha1.Condition{ + Name: "foo", + }, + Deletion: &v1alpha1.Deletion{}, + }, + want: field.ErrorList{ + &field.Error{ + Type: field.ErrorTypeInvalid, + Field: "for", + BadValue: &v1alpha1.For{ + Condition: &v1alpha1.Condition{ + Name: "foo", + }, + Deletion: &v1alpha1.Deletion{}, + }, + Detail: "a deletion or a condition must be specified (found both)", + }, + }, + }} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := ValidateFor(tt.path, tt.obj) + assert.Equal(t, tt.want, got) + }) + } +} diff --git a/pkg/validation/test/resource_reference_test.go b/pkg/validation/test/resource_reference_test.go new file mode 100644 index 000000000..71914377d --- /dev/null +++ b/pkg/validation/test/resource_reference_test.go @@ -0,0 +1,87 @@ +package test + +import ( + "testing" + + "github.com/kyverno/chainsaw/pkg/apis/v1alpha1" + "github.com/stretchr/testify/assert" + "k8s.io/apimachinery/pkg/util/validation/field" +) + +func TestValidateResourceReference(t *testing.T) { + tests := []struct { + name string + path *field.Path + obj v1alpha1.ResourceReference + want field.ErrorList + }{{ + name: "empty", + path: field.NewPath("foo"), + want: field.ErrorList{ + &field.Error{ + Type: field.ErrorTypeInvalid, + Field: "foo", + BadValue: v1alpha1.ResourceReference{}, + Detail: "kind or resource must be specified", + }, + }, + }, { + name: "both kind and resource", + path: field.NewPath("foo"), + obj: v1alpha1.ResourceReference{ + Resource: "foo", + Kind: "bar", + }, + want: field.ErrorList{ + &field.Error{ + Type: field.ErrorTypeInvalid, + Field: "foo", + BadValue: v1alpha1.ResourceReference{ + Resource: "foo", + Kind: "bar", + }, + Detail: "kind or resource must be specified (found both)", + }, + }, + }, { + name: "resource and apiVersion", + path: field.NewPath("foo"), + obj: v1alpha1.ResourceReference{ + APIVersion: "v1", + Resource: "foo", + }, + want: field.ErrorList{ + &field.Error{ + Type: field.ErrorTypeInvalid, + Field: "foo.apiVersion", + BadValue: v1alpha1.ResourceReference{ + APIVersion: "v1", + Resource: "foo", + }, + Detail: "apiVersion must not be specified when resource is set", + }, + }, + }, { + name: "kind and no apiVersion", + path: field.NewPath("foo"), + obj: v1alpha1.ResourceReference{ + Kind: "foo", + }, + want: field.ErrorList{ + &field.Error{ + Type: field.ErrorTypeInvalid, + Field: "foo.apiVersion", + BadValue: v1alpha1.ResourceReference{ + Kind: "foo", + }, + Detail: "apiVersion must be specified when kind is set", + }, + }, + }} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := ValidateResourceReference(tt.path, tt.obj) + assert.Equal(t, tt.want, got) + }) + } +} diff --git a/pkg/validation/test/test_spec_test.go b/pkg/validation/test/test_spec_test.go new file mode 100644 index 000000000..93071780c --- /dev/null +++ b/pkg/validation/test/test_spec_test.go @@ -0,0 +1,34 @@ +package test + +import ( + "testing" + + "github.com/kyverno/chainsaw/pkg/apis/v1alpha1" + "github.com/stretchr/testify/assert" + "k8s.io/apimachinery/pkg/util/validation/field" +) + +func TestValidateTestSpec(t *testing.T) { + tests := []struct { + name string + path *field.Path + obj v1alpha1.TestSpec + want field.ErrorList + }{{ + name: "empty", + }, { + name: "invalid catch", + obj: v1alpha1.TestSpec{ + Catch: []v1alpha1.Catch{{}}, + }, + want: field.ErrorList{ + field.Invalid(field.NewPath("catch").Index(0), v1alpha1.Catch{}, "no statement found in operation"), + }, + }} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := ValidateTestSpec(tt.path, tt.obj) + assert.Equal(t, tt.want, got) + }) + } +} diff --git a/pkg/validation/test/test_test.go b/pkg/validation/test/test_test.go index bd734f3b2..f046cd735 100644 --- a/pkg/validation/test/test_test.go +++ b/pkg/validation/test/test_test.go @@ -56,28 +56,23 @@ func TestValidateTest(t *testing.T) { }, }, } - tests := []struct { name string input *v1alpha1.Test expectErr bool - }{ - { - name: "Valid TestSpec", - input: &v1alpha1.Test{ - Spec: validTestSpec, - }, - expectErr: false, + }{{ + name: "Valid TestSpec", + input: &v1alpha1.Test{ + Spec: validTestSpec, }, - { - name: "Invalid TestSpec", - input: &v1alpha1.Test{ - Spec: invalidTestSpec, - }, - expectErr: true, + expectErr: false, + }, { + name: "Invalid TestSpec", + input: &v1alpha1.Test{ + Spec: invalidTestSpec, }, - } - + expectErr: true, + }} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { errs := ValidateTest(tt.input)