Skip to content

Commit 2055dcc

Browse files
acpanaacpana
authored
fix: check name length for all gk resources (#3094)
Signed-off-by: Alex Pana <8968914+acpana@users.noreply.github.com> Signed-off-by: alex <8968914+acpana@users.noreply.github.com>
1 parent 384150f commit 2055dcc

File tree

13 files changed

+244
-62
lines changed

13 files changed

+244
-62
lines changed

‎pkg/mutation/mutators/assign/assign_mutator.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,9 @@ func (m *Mutator) String() string {
118118

119119
// MutatorForAssign returns a mutator built from the given assign instance.
120120
func MutatorForAssign(assign *mutationsunversioned.Assign) (*Mutator, error) {
121+
if err := core.ValidateName(assign.Name); err != nil {
122+
return nil, err
123+
}
121124
// This is not always set by the kubernetes API server
122125
assign.SetGroupVersionKind(runtimeschema.GroupVersionKind{Group: mutationsv1beta1.GroupVersion.Group, Kind: "Assign"})
123126

‎pkg/mutation/mutators/assign/assign_mutator_test.go

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ import (
1111
mutationsunversioned "github.com/open-policy-agent/gatekeeper/v3/apis/mutations/unversioned"
1212
"github.com/open-policy-agent/gatekeeper/v3/pkg/externaldata"
1313
"github.com/open-policy-agent/gatekeeper/v3/pkg/mutation/match"
14+
"github.com/open-policy-agent/gatekeeper/v3/pkg/mutation/mutators/core"
15+
"github.com/open-policy-agent/gatekeeper/v3/pkg/mutation/mutators/testhelpers"
1416
path "github.com/open-policy-agent/gatekeeper/v3/pkg/mutation/path/tester"
1517
"github.com/open-policy-agent/gatekeeper/v3/pkg/mutation/types"
1618
"github.com/stretchr/testify/require"
@@ -1073,11 +1075,32 @@ func nestedMapSlice(u map[string]interface{}, fields ...string) ([]map[string]in
10731075
return out, nil
10741076
}
10751077

1076-
// Tests the Assign mutator MutatorForAssign call with an empty spec for graceful handling.
1077-
func Test_Assign_emptySpec(t *testing.T) {
1078-
assign := &mutationsunversioned.Assign{}
1079-
mutator, err := MutatorForAssign(assign)
1078+
func Test_Assign_errors(t *testing.T) {
1079+
for _, tt := range []struct {
1080+
name string
1081+
mut *mutationsunversioned.Assign
1082+
errMsg string
1083+
}{
1084+
{
1085+
name: "empty path",
1086+
mut: &mutationsunversioned.Assign{},
1087+
errMsg: "empty path",
1088+
},
1089+
{
1090+
name: "name > 63",
1091+
mut: &mutationsunversioned.Assign{
1092+
ObjectMeta: metav1.ObjectMeta{
1093+
Name: testhelpers.BigName(),
1094+
},
1095+
},
1096+
errMsg: core.ErrNameLength.Error(),
1097+
},
1098+
} {
1099+
t.Run(tt.name, func(t *testing.T) {
1100+
mutator, err := MutatorForAssign(tt.mut)
10801101

1081-
require.ErrorContains(t, err, "empty path")
1082-
require.Nil(t, mutator)
1102+
require.ErrorContains(t, err, tt.errMsg)
1103+
require.Nil(t, mutator)
1104+
})
1105+
}
10831106
}

‎pkg/mutation/mutators/assignimage/assignimage_mutator.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,10 @@ func (m *Mutator) String() string {
115115
// MutatorForAssignImage returns a mutator built from
116116
// the given assignImage instance.
117117
func MutatorForAssignImage(assignImage *mutationsunversioned.AssignImage) (*Mutator, error) {
118+
if err := core.ValidateName(assignImage.Name); err != nil {
119+
return nil, err
120+
}
121+
118122
// This is not always set by the kubernetes API server
119123
assignImage.SetGroupVersionKind(runtimeschema.GroupVersionKind{Group: mutationsv1beta1.GroupVersion.Group, Kind: "AssignImage"})
120124

‎pkg/mutation/mutators/assignimage/assignimage_mutator_test.go

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,10 @@ import (
88

99
mutationsunversioned "github.com/open-policy-agent/gatekeeper/v3/apis/mutations/unversioned"
1010
"github.com/open-policy-agent/gatekeeper/v3/pkg/mutation/match"
11+
"github.com/open-policy-agent/gatekeeper/v3/pkg/mutation/mutators/core"
12+
"github.com/open-policy-agent/gatekeeper/v3/pkg/mutation/mutators/testhelpers"
1113
"github.com/open-policy-agent/gatekeeper/v3/pkg/mutation/types"
14+
"github.com/stretchr/testify/require"
1215
corev1 "k8s.io/api/core/v1"
1316
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1417
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
@@ -392,3 +395,33 @@ func TestMutatorForAssignImage(t *testing.T) {
392395
})
393396
}
394397
}
398+
399+
func Test_AssignImage_errors(t *testing.T) {
400+
for _, tt := range []struct {
401+
name string
402+
mut *mutationsunversioned.AssignImage
403+
errMsg string
404+
}{
405+
{
406+
name: "empty path",
407+
mut: &mutationsunversioned.AssignImage{},
408+
errMsg: "empty path",
409+
},
410+
{
411+
name: "name > 63",
412+
mut: &mutationsunversioned.AssignImage{
413+
ObjectMeta: metav1.ObjectMeta{
414+
Name: testhelpers.BigName(),
415+
},
416+
},
417+
errMsg: core.ErrNameLength.Error(),
418+
},
419+
} {
420+
t.Run(tt.name, func(t *testing.T) {
421+
mutator, err := MutatorForAssignImage(tt.mut)
422+
423+
require.ErrorContains(t, err, tt.errMsg)
424+
require.Nil(t, mutator)
425+
})
426+
}
427+
}

‎pkg/mutation/mutators/assignmeta/assignmeta_mutator.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,10 @@ func (m *Mutator) String() string {
124124

125125
// MutatorForAssignMetadata builds a Mutator from the given AssignMetadata object.
126126
func MutatorForAssignMetadata(assignMeta *mutationsunversioned.AssignMetadata) (*Mutator, error) {
127+
if err := core.ValidateName(assignMeta.Name); err != nil {
128+
return nil, err
129+
}
130+
127131
// This is not always set by the kubernetes API server
128132
assignMeta.SetGroupVersionKind(runtimeschema.GroupVersionKind{Group: mutationsv1beta1.GroupVersion.Group, Kind: "AssignMetadata"})
129133

‎pkg/mutation/mutators/assignmeta/assignmeta_mutator_test.go

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ import (
66

77
"github.com/open-policy-agent/gatekeeper/v3/apis/mutations/unversioned"
88
"github.com/open-policy-agent/gatekeeper/v3/pkg/externaldata"
9+
"github.com/open-policy-agent/gatekeeper/v3/pkg/mutation/mutators/core"
10+
"github.com/open-policy-agent/gatekeeper/v3/pkg/mutation/mutators/testhelpers"
911
"github.com/open-policy-agent/gatekeeper/v3/pkg/mutation/types"
1012
"github.com/stretchr/testify/require"
1113
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -109,11 +111,32 @@ func TestAssignMetadata(t *testing.T) {
109111
}
110112
}
111113

112-
// Tests the AssignMeta mutator MutatorForAssignMetadata call with an empty spec for graceful handling.
113-
func Test_AssignMeta_emptySpec(t *testing.T) {
114-
assignMeta := &unversioned.AssignMetadata{}
115-
mutator, err := MutatorForAssignMetadata(assignMeta)
114+
func Test_AssignMetadata_errors(t *testing.T) {
115+
for _, tt := range []struct {
116+
name string
117+
mut *unversioned.AssignMetadata
118+
errMsg string
119+
}{
120+
{
121+
name: "empty spec",
122+
mut: &unversioned.AssignMetadata{},
123+
errMsg: "invalid location for assignmetadata",
124+
},
125+
{
126+
name: "name > 63",
127+
mut: &unversioned.AssignMetadata{
128+
ObjectMeta: metav1.ObjectMeta{
129+
Name: testhelpers.BigName(),
130+
},
131+
},
132+
errMsg: core.ErrNameLength.Error(),
133+
},
134+
} {
135+
t.Run(tt.name, func(t *testing.T) {
136+
mutator, err := MutatorForAssignMetadata(tt.mut)
116137

117-
require.ErrorContains(t, err, "invalid location for assignmetadat")
118-
require.Nil(t, mutator)
138+
require.ErrorContains(t, err, tt.errMsg)
139+
require.Nil(t, mutator)
140+
})
141+
}
119142
}

‎pkg/mutation/mutators/core/errors.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,4 +4,7 @@ import "errors"
44

55
// ErrNonKeyedSetter occurs when a setter that doesn't understand keyed lists
66
// is called against a keyed list.
7-
var ErrNonKeyedSetter = errors.New("mutator does not understand keyed lists")
7+
var (
8+
ErrNonKeyedSetter = errors.New("mutator does not understand keyed lists")
9+
ErrNameLength = errors.New("maximum name length is 63 characters")
10+
)

‎pkg/mutation/mutators/core/mutator.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import (
1414
"k8s.io/apimachinery/pkg/runtime/schema"
1515
)
1616

17-
// NewTester returns a patht.Tester for the given object name, kind path and
17+
// NewTester returns a path.Tester for the given object name, kind path and
1818
// pathtests.
1919
func NewTester(name string, kind string, path parser.Path, ptests []unversioned.PathTest) (*patht.Tester, error) {
2020
pathTests, err := gatherPathTests(name, kind, ptests)
@@ -141,3 +141,11 @@ func MatchWithApplyTo(mut *types.Mutable, applies []match.ApplyTo, mat *match.Ma
141141

142142
return matches, nil
143143
}
144+
145+
func ValidateName(name string) error {
146+
if len(name) > 63 {
147+
return ErrNameLength
148+
}
149+
150+
return nil
151+
}

‎pkg/mutation/mutators/modifyset/modify_set_mutator.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,10 @@ func (m *Mutator) String() string {
126126
// MutatorForModifySet returns an Mutator built from
127127
// the given modifyset instance.
128128
func MutatorForModifySet(modifySet *mutationsunversioned.ModifySet) (*Mutator, error) {
129+
if err := core.ValidateName(modifySet.Name); err != nil {
130+
return nil, err
131+
}
132+
129133
// This is not always set by the kubernetes API server
130134
modifySet.SetGroupVersionKind(runtimeschema.GroupVersionKind{Group: mutationsv1beta1.GroupVersion.Group, Kind: "ModifySet"})
131135

‎pkg/mutation/mutators/modifyset/modify_set_mutator_test.go

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,38 @@ import (
44
"testing"
55

66
mutationsunversioned "github.com/open-policy-agent/gatekeeper/v3/apis/mutations/unversioned"
7+
"github.com/open-policy-agent/gatekeeper/v3/pkg/mutation/mutators/core"
8+
"github.com/open-policy-agent/gatekeeper/v3/pkg/mutation/mutators/testhelpers"
79
"github.com/stretchr/testify/require"
10+
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
811
)
912

10-
// Tests the ModifySet mutator MutatorForModifySet call with an empty spec for graceful handling.
11-
func Test_ModifySet_emptySpec(t *testing.T) {
12-
modifySet := &mutationsunversioned.ModifySet{}
13-
mutator, err := MutatorForModifySet(modifySet)
13+
func Test_ModifySet_errors(t *testing.T) {
14+
for _, tt := range []struct {
15+
name string
16+
mut *mutationsunversioned.ModifySet
17+
errMsg string
18+
}{
19+
{
20+
name: "empty spec",
21+
mut: &mutationsunversioned.ModifySet{},
22+
errMsg: "applyTo required for ModifySet mutator",
23+
},
24+
{
25+
name: "name > 63",
26+
mut: &mutationsunversioned.ModifySet{
27+
ObjectMeta: v1.ObjectMeta{
28+
Name: testhelpers.BigName(),
29+
},
30+
},
31+
errMsg: core.ErrNameLength.Error(),
32+
},
33+
} {
34+
t.Run(tt.name, func(t *testing.T) {
35+
mutator, err := MutatorForModifySet(tt.mut)
1436

15-
require.ErrorContains(t, err, "applyTo required for ModifySet mutator")
16-
require.Nil(t, mutator)
37+
require.ErrorContains(t, err, tt.errMsg)
38+
require.Nil(t, mutator)
39+
})
40+
}
1741
}

0 commit comments

Comments
 (0)