fix: check name length for all gk resources

Signed-off-by: Alex Pana <[email protected]>

pkg/mutation/mutators/assign/assign_mutator.go

@@ -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"})
 

pkg/mutation/mutators/assign/assign_mutator_test.go

@@ -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"
@@ -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)
+		})
+	}
 }

pkg/mutation/mutators/assignimage/assignimage_mutator.go

@@ -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"})
 

pkg/mutation/mutators/assignimage/assignimage_mutator_test.go

@@ -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"
@@ -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",
+			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)
+		})
+	}
+}

pkg/mutation/mutators/assignmeta/assignmeta_mutator.go

@@ -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"})
 

pkg/mutation/mutators/assignmeta/assignmeta_mutator_test.go

@@ -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"
@@ -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)
+		})
+	}
 }

pkg/mutation/mutators/core/errors.go

@@ -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")
+)

pkg/mutation/mutators/core/mutator.go

@@ -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)
@@ -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
+}

pkg/mutation/mutators/modifyset/modify_set_mutator.go

@@ -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"})
 

pkg/mutation/mutators/modifyset/modify_set_mutator_test.go

@@ -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)
+		})
+	}
 }

pkg/mutation/mutators/testhelpers/dummy_mutator.go

@@ -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"
@@ -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
+}

pkg/webhook/policy.go

@@ -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
+	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 {
@@ -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
@@ -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