Declarative validation coverage for RoleRef.Name and Subject.Name in RoleBinding#

This commit is contained in:
pranshul gupta 2025-10-19 00:03:16 +05:30
parent 6ae916a577
commit dbb3941cfe
12 changed files with 87 additions and 101 deletions

View file

@ -186,8 +186,9 @@ func Validate_RoleBindingList(ctx context.Context, op operation.Operation, fldPa
// to declarative validation rules in the API schema.
func Validate_RoleRef(ctx context.Context, op operation.Operation, fldPath *field.Path, obj, oldObj *rbacv1.RoleRef) (errs field.ErrorList) {
// field rbacv1.RoleRef.APIGroup has no validation
// field rbacv1.RoleRef.Kind has no validation
// field rbacv1.RoleRef.Kind
// field rbacv1.RoleRef.Name
errs = append(errs,
func(fldPath *field.Path, obj, oldObj *string) (errs field.ErrorList) {
// don't revalidate unchanged data
@ -204,16 +205,18 @@ func Validate_RoleRef(ctx context.Context, op operation.Operation, fldPath *fiel
return // do not proceed
}
return
}(fldPath.Child("kind"), &obj.Kind, safe.Field(oldObj, func(oldObj *rbacv1.RoleRef) *string { return &oldObj.Kind }))...)
}(fldPath.Child("name"), &obj.Name, safe.Field(oldObj, func(oldObj *rbacv1.RoleRef) *string { return &oldObj.Name }))...)
// field rbacv1.RoleRef.Name has no validation
return errs
}
// Validate_Subject validates an instance of Subject according
// to declarative validation rules in the API schema.
func Validate_Subject(ctx context.Context, op operation.Operation, fldPath *field.Path, obj, oldObj *rbacv1.Subject) (errs field.ErrorList) {
// field rbacv1.Subject.Kind
// field rbacv1.Subject.Kind has no validation
// field rbacv1.Subject.APIGroup has no validation
// field rbacv1.Subject.Name
errs = append(errs,
func(fldPath *field.Path, obj, oldObj *string) (errs field.ErrorList) {
// don't revalidate unchanged data
@ -230,10 +233,8 @@ func Validate_Subject(ctx context.Context, op operation.Operation, fldPath *fiel
return // do not proceed
}
return
}(fldPath.Child("kind"), &obj.Kind, safe.Field(oldObj, func(oldObj *rbacv1.Subject) *string { return &oldObj.Kind }))...)
}(fldPath.Child("name"), &obj.Name, safe.Field(oldObj, func(oldObj *rbacv1.Subject) *string { return &oldObj.Name }))...)
// field rbacv1.Subject.APIGroup has no validation
// field rbacv1.Subject.Name has no validation
// field rbacv1.Subject.Namespace has no validation
return errs
}

View file

@ -188,8 +188,9 @@ func Validate_RoleBindingList(ctx context.Context, op operation.Operation, fldPa
// to declarative validation rules in the API schema.
func Validate_RoleRef(ctx context.Context, op operation.Operation, fldPath *field.Path, obj, oldObj *rbacv1alpha1.RoleRef) (errs field.ErrorList) {
// field rbacv1alpha1.RoleRef.APIGroup has no validation
// field rbacv1alpha1.RoleRef.Kind has no validation
// field rbacv1alpha1.RoleRef.Kind
// field rbacv1alpha1.RoleRef.Name
errs = append(errs,
func(fldPath *field.Path, obj, oldObj *string) (errs field.ErrorList) {
// don't revalidate unchanged data
@ -206,16 +207,18 @@ func Validate_RoleRef(ctx context.Context, op operation.Operation, fldPath *fiel
return // do not proceed
}
return
}(fldPath.Child("kind"), &obj.Kind, safe.Field(oldObj, func(oldObj *rbacv1alpha1.RoleRef) *string { return &oldObj.Kind }))...)
}(fldPath.Child("name"), &obj.Name, safe.Field(oldObj, func(oldObj *rbacv1alpha1.RoleRef) *string { return &oldObj.Name }))...)
// field rbacv1alpha1.RoleRef.Name has no validation
return errs
}
// Validate_Subject validates an instance of Subject according
// to declarative validation rules in the API schema.
func Validate_Subject(ctx context.Context, op operation.Operation, fldPath *field.Path, obj, oldObj *rbacv1alpha1.Subject) (errs field.ErrorList) {
// field rbacv1alpha1.Subject.Kind
// field rbacv1alpha1.Subject.Kind has no validation
// field rbacv1alpha1.Subject.APIVersion has no validation
// field rbacv1alpha1.Subject.Name
errs = append(errs,
func(fldPath *field.Path, obj, oldObj *string) (errs field.ErrorList) {
// don't revalidate unchanged data
@ -232,10 +235,8 @@ func Validate_Subject(ctx context.Context, op operation.Operation, fldPath *fiel
return // do not proceed
}
return
}(fldPath.Child("kind"), &obj.Kind, safe.Field(oldObj, func(oldObj *rbacv1alpha1.Subject) *string { return &oldObj.Kind }))...)
}(fldPath.Child("name"), &obj.Name, safe.Field(oldObj, func(oldObj *rbacv1alpha1.Subject) *string { return &oldObj.Name }))...)
// field rbacv1alpha1.Subject.APIVersion has no validation
// field rbacv1alpha1.Subject.Name has no validation
// field rbacv1alpha1.Subject.Namespace has no validation
return errs
}

View file

@ -186,8 +186,9 @@ func Validate_RoleBindingList(ctx context.Context, op operation.Operation, fldPa
// to declarative validation rules in the API schema.
func Validate_RoleRef(ctx context.Context, op operation.Operation, fldPath *field.Path, obj, oldObj *rbacv1beta1.RoleRef) (errs field.ErrorList) {
// field rbacv1beta1.RoleRef.APIGroup has no validation
// field rbacv1beta1.RoleRef.Kind has no validation
// field rbacv1beta1.RoleRef.Kind
// field rbacv1beta1.RoleRef.Name
errs = append(errs,
func(fldPath *field.Path, obj, oldObj *string) (errs field.ErrorList) {
// don't revalidate unchanged data
@ -204,16 +205,18 @@ func Validate_RoleRef(ctx context.Context, op operation.Operation, fldPath *fiel
return // do not proceed
}
return
}(fldPath.Child("kind"), &obj.Kind, safe.Field(oldObj, func(oldObj *rbacv1beta1.RoleRef) *string { return &oldObj.Kind }))...)
}(fldPath.Child("name"), &obj.Name, safe.Field(oldObj, func(oldObj *rbacv1beta1.RoleRef) *string { return &oldObj.Name }))...)
// field rbacv1beta1.RoleRef.Name has no validation
return errs
}
// Validate_Subject validates an instance of Subject according
// to declarative validation rules in the API schema.
func Validate_Subject(ctx context.Context, op operation.Operation, fldPath *field.Path, obj, oldObj *rbacv1beta1.Subject) (errs field.ErrorList) {
// field rbacv1beta1.Subject.Kind
// field rbacv1beta1.Subject.Kind has no validation
// field rbacv1beta1.Subject.APIGroup has no validation
// field rbacv1beta1.Subject.Name
errs = append(errs,
func(fldPath *field.Path, obj, oldObj *string) (errs field.ErrorList) {
// don't revalidate unchanged data
@ -230,10 +233,8 @@ func Validate_Subject(ctx context.Context, op operation.Operation, fldPath *fiel
return // do not proceed
}
return
}(fldPath.Child("kind"), &obj.Kind, safe.Field(oldObj, func(oldObj *rbacv1beta1.Subject) *string { return &oldObj.Kind }))...)
}(fldPath.Child("name"), &obj.Name, safe.Field(oldObj, func(oldObj *rbacv1beta1.Subject) *string { return &oldObj.Name }))...)
// field rbacv1beta1.Subject.APIGroup has no validation
// field rbacv1beta1.Subject.Name has no validation
// field rbacv1beta1.Subject.Namespace has no validation
return errs
}

View file

@ -143,7 +143,7 @@ func ValidateRoleBinding(roleBinding *rbac.RoleBinding) field.ErrorList {
}
if len(roleBinding.RoleRef.Name) == 0 {
allErrs = append(allErrs, field.Required(field.NewPath("roleRef", "name"), ""))
allErrs = append(allErrs, field.Required(field.NewPath("roleRef", "name"), "").MarkCoveredByDeclarative())
} else {
for _, msg := range ValidateRBACName(roleBinding.RoleRef.Name, false) {
allErrs = append(allErrs, field.Invalid(field.NewPath("roleRef", "name"), roleBinding.RoleRef.Name, msg))
@ -187,7 +187,7 @@ func ValidateClusterRoleBinding(roleBinding *rbac.ClusterRoleBinding) field.Erro
}
if len(roleBinding.RoleRef.Name) == 0 {
allErrs = append(allErrs, field.Required(field.NewPath("roleRef", "name"), ""))
allErrs = append(allErrs, field.Required(field.NewPath("roleRef", "name"), "").MarkCoveredByDeclarative())
} else {
for _, msg := range ValidateRBACName(roleBinding.RoleRef.Name, false) {
allErrs = append(allErrs, field.Invalid(field.NewPath("roleRef", "name"), roleBinding.RoleRef.Name, msg))
@ -218,7 +218,7 @@ func ValidateRoleBindingSubject(subject rbac.Subject, isNamespaced bool, fldPath
allErrs := field.ErrorList{}
if len(subject.Name) == 0 {
allErrs = append(allErrs, field.Required(fldPath.Child("name"), ""))
allErrs = append(allErrs, field.Required(fldPath.Child("name"), "").MarkCoveredByDeclarative())
}
switch subject.Kind {

View file

@ -78,7 +78,7 @@ func (strategy) Validate(ctx context.Context, obj runtime.Object) field.ErrorLis
}
allErrs := validation.ValidateClusterRole(clusterRole, opts)
return rest.ValidateDeclarativelyWithMigrationChecks(ctx,legacyscheme.Scheme,clusterRole,nil,allErrs,operation.Create)
return rest.ValidateDeclarativelyWithMigrationChecks(ctx, legacyscheme.Scheme, clusterRole, nil, allErrs, operation.Create)
}
// WarningsOnCreate returns warnings for the creation of the given object.
@ -98,7 +98,7 @@ func (strategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) fie
}
errs := validation.ValidateClusterRoleUpdate(newObj, oldObj, opts)
return rest.ValidateDeclarativelyWithMigrationChecks(ctx,legacyscheme.Scheme,newObj,oldObj,errs,operation.Update)
return rest.ValidateDeclarativelyWithMigrationChecks(ctx, legacyscheme.Scheme, newObj, oldObj, errs, operation.Update)
}
// WarningsOnUpdate returns warnings for the given update.

View file

@ -30,7 +30,9 @@ var apiVersions = []string{"v1", "v1alpha1", "v1beta1"}
func TestDeclarativeValidateForDeclarative(t *testing.T) {
for _, apiVersion := range apiVersions {
testDeclarativeValidateForDeclarative(t, apiVersion)
t.Run(apiVersion, func(t *testing.T) {
testDeclarativeValidateForDeclarative(t, apiVersion)
})
}
}
@ -39,96 +41,65 @@ func testDeclarativeValidateForDeclarative(t *testing.T, apiVersion string) {
APIGroup: "rbac.authorization.k8s.io",
APIVersion: apiVersion,
})
testCases := map[string]struct {
input rbac.RoleBinding
expectedErrs field.ErrorList
}{
"missing roleref name": {
"missing roleRef.name": {
input: rbac.RoleBinding{
ObjectMeta: metav1.ObjectMeta{Name: "test-binding", Namespace: "test-namespace"},
ObjectMeta: metav1.ObjectMeta{Name: "test-binding", Namespace: "test-ns"},
RoleRef: rbac.RoleRef{
APIGroup: "rbac.authorization.k8s.io",
Kind: "ClusterRole",
// Name is intentionally missing here
},
Subjects: []rbac.Subject{
{Kind: rbac.UserKind, APIGroup: rbac.GroupName, Name: "user"},
{Kind: rbac.UserKind, APIGroup: rbac.GroupName, Name: "user1"},
},
},
expectedErrs: field.ErrorList{
field.Required(field.NewPath("roleRef", "name"), "name is required"),
},
},
"valid binding": {
"missing subject.name": {
input: rbac.RoleBinding{
ObjectMeta: metav1.ObjectMeta{Name: "valid-binding", Namespace: "test-namespace"},
ObjectMeta: metav1.ObjectMeta{Name: "test-binding", Namespace: "test-ns"},
RoleRef: rbac.RoleRef{
APIGroup: "rbac.authorization.k8s.io",
Kind: "ClusterRole",
Kind: "Role",
Name: "reader",
},
Subjects: []rbac.Subject{
{Kind: rbac.UserKind, APIGroup: rbac.GroupName},
},
},
expectedErrs: field.ErrorList{
field.Required(field.NewPath("subjects").Index(0).Child("name"), "name is required"),
},
},
"valid binding": {
input: rbac.RoleBinding{
ObjectMeta: metav1.ObjectMeta{Name: "valid-binding", Namespace: "test-ns"},
RoleRef: rbac.RoleRef{
APIGroup: "rbac.authorization.k8s.io",
Kind: "Role",
Name: "admin",
},
Subjects: []rbac.Subject{
{Kind: rbac.UserKind, APIGroup: rbac.GroupName, Name: "user"},
{Kind: rbac.UserKind, APIGroup: rbac.GroupName, Name: "user1"},
},
},
expectedErrs: field.ErrorList{},
},
// TODO: Add more test cases
}
for k, tc := range testCases {
t.Run(k, func(t *testing.T) {
for name, tc := range testCases {
t.Run(name, func(t *testing.T) {
apitesting.VerifyValidationEquivalence(t, ctx, &tc.input, Strategy.Validate, tc.expectedErrs)
})
}
}
func TestValidateUpdateForDeclarative(t *testing.T) {
for _, apiVersion := range apiVersions {
testValidateUpdateForDeclarative(t, apiVersion)
}
}
func testValidateUpdateForDeclarative(t *testing.T, apiVersion string) {
ctx := genericapirequest.WithRequestInfo(genericapirequest.NewDefaultContext(), &genericapirequest.RequestInfo{
APIGroup: "rbac.authorization.k8s.io",
APIVersion: apiVersion,
})
testCases := map[string]struct {
old rbac.RoleBinding
update rbac.RoleBinding
expectedErrs field.ErrorList
}{
"update immutable roleRef Kind": {
old: rbac.RoleBinding{
ObjectMeta: metav1.ObjectMeta{Name: "binding", Namespace: "test-namespace"},
RoleRef: rbac.RoleRef{APIGroup: "rbac.authorization.k8s.io", Kind: "Role", Name: "reader"},
},
update: rbac.RoleBinding{
ObjectMeta: metav1.ObjectMeta{Name: "binding", ResourceVersion: "1", Namespace: "test-namespace"},
RoleRef: rbac.RoleRef{APIGroup: "rbac.authorization.k8s.io", Kind: "ClusterRole", Name: "reader"},
},
expectedErrs: field.ErrorList{
field.Invalid(field.NewPath("roleRef"), rbac.RoleRef{APIGroup: "rbac.authorization.k8s.io", Kind: "ClusterRole", Name: "reader"}, "cannot change roleRef"),
},
},
"valid update": {
old: rbac.RoleBinding{
ObjectMeta: metav1.ObjectMeta{Name: "binding", Namespace: "test-namespace"},
RoleRef: rbac.RoleRef{APIGroup: "rbac.authorization.k8s.io", Kind: "Role", Name: "reader"},
Subjects: []rbac.Subject{{Kind: rbac.UserKind, APIGroup: rbac.GroupName, Name: "user1"}},
},
update: rbac.RoleBinding{
ObjectMeta: metav1.ObjectMeta{Name: "binding", ResourceVersion: "1", Namespace: "test-namespace"},
RoleRef: rbac.RoleRef{APIGroup: "rbac.authorization.k8s.io", Kind: "Role", Name: "reader"},
Subjects: []rbac.Subject{
{Kind: rbac.UserKind, APIGroup: rbac.GroupName, Name: "user1"},
{Kind: rbac.UserKind, APIGroup: rbac.GroupName, Name: "user2"},
},
},
expectedErrs: field.ErrorList{},
},
}
for k, tc := range testCases {
t.Run(k, func(t *testing.T) {
apitesting.VerifyUpdateValidationEquivalence(t, ctx, &tc.update, &tc.old, Strategy.ValidateUpdate, tc.expectedErrs)
})
}
}

View file

@ -185,6 +185,8 @@ message RoleRef {
optional string kind = 2;
// Name is the name of resource being referenced
// +required
// +k8s:required
optional string name = 3;
}
@ -203,6 +205,8 @@ message Subject {
optional string apiGroup = 2;
// Name of the object being referenced.
// +required
// +k8s:required
optional string name = 3;
// Namespace of the referenced object. If the object kind is non-namespace, such as "User" or "Group", and this value is not empty

View file

@ -79,8 +79,6 @@ type PolicyRule struct {
type Subject struct {
// Kind of object being referenced. Values defined by this API group are "User", "Group", and "ServiceAccount".
// If the Authorizer does not recognized the kind value, the Authorizer should report an error.
// +required
// +k8s:required
Kind string `json:"kind" protobuf:"bytes,1,opt,name=kind"`
// APIGroup holds the API group of the referenced subject.
// Defaults to "" for ServiceAccount subjects.
@ -88,6 +86,8 @@ type Subject struct {
// +optional
APIGroup string `json:"apiGroup,omitempty" protobuf:"bytes,2,opt,name=apiGroup"`
// Name of the object being referenced.
// +required
// +k8s:required
Name string `json:"name" protobuf:"bytes,3,opt,name=name"`
// Namespace of the referenced object. If the object kind is non-namespace, such as "User" or "Group", and this value is not empty
// the Authorizer should report an error.
@ -101,10 +101,10 @@ type RoleRef struct {
// APIGroup is the group for the resource being referenced
APIGroup string `json:"apiGroup" protobuf:"bytes,1,opt,name=apiGroup"`
// Kind is the type of resource being referenced
// +required
// +k8s:required
Kind string `json:"kind" protobuf:"bytes,2,opt,name=kind"`
// Name is the name of resource being referenced
// +required
// +k8s:required
Name string `json:"name" protobuf:"bytes,3,opt,name=name"`
}

View file

@ -190,6 +190,8 @@ message RoleRef {
optional string kind = 2;
// Name is the name of resource being referenced
// +required
// +k8s:required
optional string name = 3;
}
@ -208,6 +210,8 @@ message Subject {
optional string apiVersion = 2;
// Name of the object being referenced.
// +required
// +k8s:required
optional string name = 3;
// Namespace of the referenced object. If the object kind is non-namespace, such as "User" or "Group", and this value is not empty

View file

@ -78,8 +78,6 @@ type PolicyRule struct {
type Subject struct {
// Kind of object being referenced. Values defined by this API group are "User", "Group", and "ServiceAccount".
// If the Authorizer does not recognized the kind value, the Authorizer should report an error.
// +required
// +k8s:required
Kind string `json:"kind" protobuf:"bytes,1,opt,name=kind"`
// APIVersion holds the API group and version of the referenced subject.
// Defaults to "v1" for ServiceAccount subjects.
@ -88,6 +86,8 @@ type Subject struct {
// +optional
APIVersion string `json:"apiVersion,omitempty" protobuf:"bytes,2,opt,name=apiVersion"`
// Name of the object being referenced.
// +required
// +k8s:required
Name string `json:"name" protobuf:"bytes,3,opt,name=name"`
// Namespace of the referenced object. If the object kind is non-namespace, such as "User" or "Group", and this value is not empty
// the Authorizer should report an error.
@ -100,10 +100,10 @@ type RoleRef struct {
// APIGroup is the group for the resource being referenced
APIGroup string `json:"apiGroup" protobuf:"bytes,1,opt,name=apiGroup"`
// Kind is the type of resource being referenced
// +required
// +k8s:required
Kind string `json:"kind" protobuf:"bytes,2,opt,name=kind"`
// Name is the name of resource being referenced
// +required
// +k8s:required
Name string `json:"name" protobuf:"bytes,3,opt,name=name"`
}

View file

@ -191,6 +191,8 @@ message RoleRef {
optional string kind = 2;
// Name is the name of resource being referenced
// +required
// +k8s:required
optional string name = 3;
}
@ -208,6 +210,8 @@ message Subject {
optional string apiGroup = 2;
// Name of the object being referenced.
// +required
// +k8s:required
optional string name = 3;
// Namespace of the referenced object. If the object kind is non-namespace, such as "User" or "Group", and this value is not empty

View file

@ -79,8 +79,6 @@ type PolicyRule struct {
type Subject struct {
// Kind of object being referenced. Values defined by this API group are "User", "Group", and "ServiceAccount".
// If the Authorizer does not recognized the kind value, the Authorizer should report an error.
// +required
// +k8s:required
Kind string `json:"kind" protobuf:"bytes,1,opt,name=kind"`
// APIGroup holds the API group of the referenced subject.
// Defaults to "" for ServiceAccount subjects.
@ -88,6 +86,8 @@ type Subject struct {
// +optional
APIGroup string `json:"apiGroup,omitempty" protobuf:"bytes,2,opt,name=apiGroup"`
// Name of the object being referenced.
// +required
// +k8s:required
Name string `json:"name" protobuf:"bytes,3,opt,name=name"`
// Namespace of the referenced object. If the object kind is non-namespace, such as "User" or "Group", and this value is not empty
// the Authorizer should report an error.
@ -100,10 +100,10 @@ type RoleRef struct {
// APIGroup is the group for the resource being referenced
APIGroup string `json:"apiGroup" protobuf:"bytes,1,opt,name=apiGroup"`
// Kind is the type of resource being referenced
// +required
// +k8s:required
Kind string `json:"kind" protobuf:"bytes,2,opt,name=kind"`
// Name is the name of resource being referenced
// +required
// +k8s:required
Name string `json:"name" protobuf:"bytes,3,opt,name=name"`
}