Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: check name length for all gk resources #3094

Merged
merged 6 commits into from
Nov 6, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions pkg/mutation/mutators/assign/assign_mutator.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,9 @@ func (m *Mutator) String() string {

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

Expand Down
35 changes: 29 additions & 6 deletions pkg/mutation/mutators/assign/assign_mutator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import (
mutationsunversioned "github.com/open-policy-agent/gatekeeper/v3/apis/mutations/unversioned"
"github.com/open-policy-agent/gatekeeper/v3/pkg/externaldata"
"github.com/open-policy-agent/gatekeeper/v3/pkg/mutation/match"
"github.com/open-policy-agent/gatekeeper/v3/pkg/mutation/mutators/core"
"github.com/open-policy-agent/gatekeeper/v3/pkg/mutation/mutators/testhelpers"
path "github.com/open-policy-agent/gatekeeper/v3/pkg/mutation/path/tester"
"github.com/open-policy-agent/gatekeeper/v3/pkg/mutation/types"
"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -1073,11 +1075,32 @@ func nestedMapSlice(u map[string]interface{}, fields ...string) ([]map[string]in
return out, nil
}

// Tests the Assign mutator MutatorForAssign call with an empty spec for graceful handling.
func Test_Assign_emptySpec(t *testing.T) {
assign := &mutationsunversioned.Assign{}
mutator, err := MutatorForAssign(assign)
func Test_Assign_errors(t *testing.T) {
for _, tt := range []struct {
name string
mut *mutationsunversioned.Assign
errMsg string
}{
{
name: "empty path",
mut: &mutationsunversioned.Assign{},
errMsg: "empty path",
},
{
name: "name > 64",
mut: &mutationsunversioned.Assign{
ObjectMeta: metav1.ObjectMeta{
Name: testhelpers.BigName(),
},
},
errMsg: core.ErrNameLength.Error(),
},
} {
t.Run(tt.name, func(t *testing.T) {
mutator, err := MutatorForAssign(tt.mut)

require.ErrorContains(t, err, "empty path")
require.Nil(t, mutator)
require.ErrorContains(t, err, tt.errMsg)
require.Nil(t, mutator)
})
}
}
4 changes: 4 additions & 0 deletions pkg/mutation/mutators/assignimage/assignimage_mutator.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,10 @@ func (m *Mutator) String() string {
// MutatorForAssignImage returns a mutator built from
// the given assignImage instance.
func MutatorForAssignImage(assignImage *mutationsunversioned.AssignImage) (*Mutator, error) {
if err := core.ValidateName(assignImage.Name); err != nil {
return nil, err
}

// This is not always set by the kubernetes API server
assignImage.SetGroupVersionKind(runtimeschema.GroupVersionKind{Group: mutationsv1beta1.GroupVersion.Group, Kind: "AssignImage"})

Expand Down
33 changes: 33 additions & 0 deletions pkg/mutation/mutators/assignimage/assignimage_mutator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,10 @@ import (

mutationsunversioned "github.com/open-policy-agent/gatekeeper/v3/apis/mutations/unversioned"
"github.com/open-policy-agent/gatekeeper/v3/pkg/mutation/match"
"github.com/open-policy-agent/gatekeeper/v3/pkg/mutation/mutators/core"
"github.com/open-policy-agent/gatekeeper/v3/pkg/mutation/mutators/testhelpers"
"github.com/open-policy-agent/gatekeeper/v3/pkg/mutation/types"
"github.com/stretchr/testify/require"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
Expand Down Expand Up @@ -392,3 +395,33 @@ func TestMutatorForAssignImage(t *testing.T) {
})
}
}

func Test_AssignImage_errors(t *testing.T) {
for _, tt := range []struct {
name string
mut *mutationsunversioned.AssignImage
errMsg string
}{
{
name: "empty path",
mut: &mutationsunversioned.AssignImage{},
errMsg: "empty path",
},
{
name: "name > 64",
acpana marked this conversation as resolved.
Show resolved Hide resolved
mut: &mutationsunversioned.AssignImage{
ObjectMeta: metav1.ObjectMeta{
Name: testhelpers.BigName(),
},
},
errMsg: core.ErrNameLength.Error(),
},
} {
t.Run(tt.name, func(t *testing.T) {
mutator, err := MutatorForAssignImage(tt.mut)

require.ErrorContains(t, err, tt.errMsg)
require.Nil(t, mutator)
})
}
}
4 changes: 4 additions & 0 deletions pkg/mutation/mutators/assignmeta/assignmeta_mutator.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,10 @@ func (m *Mutator) String() string {

// MutatorForAssignMetadata builds a Mutator from the given AssignMetadata object.
func MutatorForAssignMetadata(assignMeta *mutationsunversioned.AssignMetadata) (*Mutator, error) {
if err := core.ValidateName(assignMeta.Name); err != nil {
return nil, err
}

// This is not always set by the kubernetes API server
assignMeta.SetGroupVersionKind(runtimeschema.GroupVersionKind{Group: mutationsv1beta1.GroupVersion.Group, Kind: "AssignMetadata"})

Expand Down
35 changes: 29 additions & 6 deletions pkg/mutation/mutators/assignmeta/assignmeta_mutator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import (

"github.com/open-policy-agent/gatekeeper/v3/apis/mutations/unversioned"
"github.com/open-policy-agent/gatekeeper/v3/pkg/externaldata"
"github.com/open-policy-agent/gatekeeper/v3/pkg/mutation/mutators/core"
"github.com/open-policy-agent/gatekeeper/v3/pkg/mutation/mutators/testhelpers"
"github.com/open-policy-agent/gatekeeper/v3/pkg/mutation/types"
"github.com/stretchr/testify/require"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -109,11 +111,32 @@ func TestAssignMetadata(t *testing.T) {
}
}

// Tests the AssignMeta mutator MutatorForAssignMetadata call with an empty spec for graceful handling.
func Test_AssignMeta_emptySpec(t *testing.T) {
assignMeta := &unversioned.AssignMetadata{}
mutator, err := MutatorForAssignMetadata(assignMeta)
func Test_AssignMetadata_errors(t *testing.T) {
for _, tt := range []struct {
name string
mut *unversioned.AssignMetadata
errMsg string
}{
{
name: "empty spec",
mut: &unversioned.AssignMetadata{},
errMsg: "invalid location for assignmetadata",
},
{
name: "name > 64",
mut: &unversioned.AssignMetadata{
ObjectMeta: metav1.ObjectMeta{
Name: testhelpers.BigName(),
},
},
errMsg: core.ErrNameLength.Error(),
},
} {
t.Run(tt.name, func(t *testing.T) {
mutator, err := MutatorForAssignMetadata(tt.mut)

require.ErrorContains(t, err, "invalid location for assignmetadat")
require.Nil(t, mutator)
require.ErrorContains(t, err, tt.errMsg)
require.Nil(t, mutator)
})
}
}
5 changes: 4 additions & 1 deletion pkg/mutation/mutators/core/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,7 @@ import "errors"

// ErrNonKeyedSetter occurs when a setter that doesn't understand keyed lists
// is called against a keyed list.
var ErrNonKeyedSetter = errors.New("mutator does not understand keyed lists")
var (
ErrNonKeyedSetter = errors.New("mutator does not understand keyed lists")
ErrNameLength = errors.New("maximum length for name is 63 char")
acpana marked this conversation as resolved.
Show resolved Hide resolved
)
10 changes: 9 additions & 1 deletion pkg/mutation/mutators/core/mutator.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import (
"k8s.io/apimachinery/pkg/runtime/schema"
)

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

return matches, nil
}

func ValidateName(name string) error {
if len(name) > 63 {
return ErrNameLength
}

return nil
}
4 changes: 4 additions & 0 deletions pkg/mutation/mutators/modifyset/modify_set_mutator.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,10 @@ func (m *Mutator) String() string {
// MutatorForModifySet returns an Mutator built from
// the given modifyset instance.
func MutatorForModifySet(modifySet *mutationsunversioned.ModifySet) (*Mutator, error) {
if err := core.ValidateName(modifySet.Name); err != nil {
return nil, err
}

// This is not always set by the kubernetes API server
modifySet.SetGroupVersionKind(runtimeschema.GroupVersionKind{Group: mutationsv1beta1.GroupVersion.Group, Kind: "ModifySet"})

Expand Down
36 changes: 30 additions & 6 deletions pkg/mutation/mutators/modifyset/modify_set_mutator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,38 @@ import (
"testing"

mutationsunversioned "github.com/open-policy-agent/gatekeeper/v3/apis/mutations/unversioned"
"github.com/open-policy-agent/gatekeeper/v3/pkg/mutation/mutators/core"
"github.com/open-policy-agent/gatekeeper/v3/pkg/mutation/mutators/testhelpers"
"github.com/stretchr/testify/require"
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

// Tests the ModifySet mutator MutatorForModifySet call with an empty spec for graceful handling.
func Test_ModifySet_emptySpec(t *testing.T) {
modifySet := &mutationsunversioned.ModifySet{}
mutator, err := MutatorForModifySet(modifySet)
func Test_ModifySet_errors(t *testing.T) {
for _, tt := range []struct {
name string
mut *mutationsunversioned.ModifySet
errMsg string
}{
{
name: "empty spec",
mut: &mutationsunversioned.ModifySet{},
errMsg: "applyTo required for ModifySet mutator",
},
{
name: "name > 64",
mut: &mutationsunversioned.ModifySet{
ObjectMeta: v1.ObjectMeta{
Name: testhelpers.BigName(),
},
},
errMsg: core.ErrNameLength.Error(),
},
} {
t.Run(tt.name, func(t *testing.T) {
mutator, err := MutatorForModifySet(tt.mut)

require.ErrorContains(t, err, "applyTo required for ModifySet mutator")
require.Nil(t, mutator)
require.ErrorContains(t, err, tt.errMsg)
require.Nil(t, mutator)
})
}
}
6 changes: 6 additions & 0 deletions pkg/mutation/mutators/testhelpers/dummy_mutator.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package testhelpers

import (
"reflect"
"strings"

"github.com/open-policy-agent/gatekeeper/v3/pkg/mutation/match"
"github.com/open-policy-agent/gatekeeper/v3/pkg/mutation/mutators/core"
Expand Down Expand Up @@ -65,3 +66,8 @@ func NewDummyMutator(name, path string, value interface{}) *DummyMutator {
}
return &DummyMutator{name: name, path: p, value: value}
}

// BigName returns a 64-length string.
func BigName() string {
return strings.Repeat("abigname", len("abigname")) // 8 X 8 = 64
acpana marked this conversation as resolved.
Show resolved Hide resolved
}
14 changes: 11 additions & 3 deletions pkg/webhook/policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -338,12 +338,20 @@ func (h *validationHandler) getValidationMessages(res []*rtypes.Result, req *adm
func (h *validationHandler) validateGatekeeperResources(ctx context.Context, req *admission.Request) (bool, error) {
gvk := req.AdmissionRequest.Kind

// for resources that don't have a name validation
acpana marked this conversation as resolved.
Show resolved Hide resolved
validateWithName := func(ctx context.Context, req *admission.Request, specificValidator func(ctx context.Context, req *admission.Request) (bool, error)) (bool, error) {
if len(req.Name) > 63 {
return false, fmt.Errorf("resource cannot have metadata.name larger than 63 char; length: %d", len(req.Name))
}
return specificValidator(ctx, req)
}
switch {
case gvk.Group == "templates.gatekeeper.sh" && gvk.Kind == "ConstraintTemplate":
return h.validateTemplate(ctx, req)
return validateWithName(ctx, req, h.validateTemplate)
case gvk.Group == "expansion.gatekeeper.sh" && gvk.Kind == "ExpansionTemplate":
return h.validateExpansionTemplate(req)
case gvk.Group == "constraints.gatekeeper.sh":
// constraint name is restricted to 63 at schema creation time
return h.validateConstraint(req)
case gvk.Group == "config.gatekeeper.sh" && gvk.Kind == "Config":
if err := h.validateConfigResource(req); err != nil {
Expand All @@ -358,7 +366,7 @@ func (h *validationHandler) validateGatekeeperResources(ctx context.Context, req
case req.AdmissionRequest.Kind.Group == mutationsGroup && req.AdmissionRequest.Kind.Kind == "AssignImage":
return h.validateAssignImage(req)
case req.AdmissionRequest.Kind.Group == externalDataGroup && req.AdmissionRequest.Kind.Kind == "Provider":
return h.validateProvider(req)
return validateWithName(ctx, req, h.validateProvider)
}

return false, nil
Expand Down Expand Up @@ -523,7 +531,7 @@ func (h *validationHandler) validateModifySet(req *admission.Request) (bool, err
return false, nil
}

func (h *validationHandler) validateProvider(req *admission.Request) (bool, error) {
func (h *validationHandler) validateProvider(_ context.Context, req *admission.Request) (bool, error) {
obj, _, err := deserializer.Decode(req.AdmissionRequest.Object.Raw, nil, nil)
if err != nil {
return false, err
Expand Down
Loading
Loading