Merge pull request #26984 from ericchiang/rbac-api-group-make-subject-apiversion-optional

Automatic merge from submit-queue

Rbac api group make subject apiversion optional

This fixes the verification for the "apiVerion" field in the RBAC subject and makes it optional. This field isn't used and currently won't pass validation if it's filled.

```yml
kind: ClusterRoleBinding
apiVersion: rbac.authorization.k8s.io/v1alpha1
metadata:
  name: admins
subject:
  - kind: User
    name: admin-user
    # apiVersion: "entering anything here will fail validation"
roleRef:
  kind: ClusterRole
  name: admin
  apiVersion: rbac.authorization.k8s.io/v1alpha1
```

api/swagger-spec/rbac.authorization.k8s.io_v1alpha1.json

@@ -2821,7 +2821,6 @@
     "description": "Subject contains a reference to the object or user identities a role binding applies to.  This can either hold a direct API object reference, or a value for non-objects such as user and group names.",
     "required": [
      "kind",
-     "apiVersion",
      "name"
     ],
     "properties": {
@@ -2831,7 +2830,7 @@
      },
      "apiVersion": {
       "type": "string",
-      "description": "APIVersion holds the API group and version of the referenced object. For non-object references such as \"Group\" and \"User\" this is expected to be API version of this API group. For example \"rbac/v1alpha1\"."
+      "description": "APIVersion holds the API group and version of the referenced object."
      },
      "name": {
       "type": "string",

pkg/apis/rbac/v1alpha1/generated.proto

@@ -146,8 +146,7 @@ message Subject {
   // If the Authorizer does not recognized the kind value, the Authorizer should report an error.
   optional string kind = 1;
 
-  // APIVersion holds the API group and version of the referenced object. For non-object references such as "Group" and "User" this is
-  // expected to be API version of this API group. For example "rbac/v1alpha1".
+  // APIVersion holds the API group and version of the referenced object.
   optional string apiVersion = 2;
 
   // Name of the object being referenced.

pkg/apis/rbac/v1alpha1/types.generated.go

@@ -604,12 +604,13 @@ func (x *Subject) CodecEncodeSelf(e *codec1978.Encoder) {
 			var yyq2 [4]bool
 			_, _, _ = yysep2, yyq2, yy2arr2
 			const yyr2 bool = false
+			yyq2[1] = x.APIVersion != ""
 			yyq2[3] = x.Namespace != ""
 			var yynn2 int
 			if yyr2 || yy2arr2 {
 				r.EncodeArrayStart(4)
 			} else {
-				yynn2 = 3
+				yynn2 = 2
 				for _, b := range yyq2 {
 					if b {
 						yynn2++
@@ -639,21 +640,27 @@ func (x *Subject) CodecEncodeSelf(e *codec1978.Encoder) {
 			}
 			if yyr2 || yy2arr2 {
 				z.EncSendContainerState(codecSelfer_containerArrayElem1234)
-				yym7 := z.EncBinary()
-				_ = yym7
-				if false {
+				if yyq2[1] {
+					yym7 := z.EncBinary()
+					_ = yym7
+					if false {
+					} else {
+						r.EncodeString(codecSelferC_UTF81234, string(x.APIVersion))
+					}
 				} else {
-					r.EncodeString(codecSelferC_UTF81234, string(x.APIVersion))
+					r.EncodeString(codecSelferC_UTF81234, "")
 				}
 			} else {
-				z.EncSendContainerState(codecSelfer_containerMapKey1234)
-				r.EncodeString(codecSelferC_UTF81234, string("apiVersion"))
-				z.EncSendContainerState(codecSelfer_containerMapValue1234)
-				yym8 := z.EncBinary()
-				_ = yym8
-				if false {
-				} else {
-					r.EncodeString(codecSelferC_UTF81234, string(x.APIVersion))
+				if yyq2[1] {
+					z.EncSendContainerState(codecSelfer_containerMapKey1234)
+					r.EncodeString(codecSelferC_UTF81234, string("apiVersion"))
+					z.EncSendContainerState(codecSelfer_containerMapValue1234)
+					yym8 := z.EncBinary()
+					_ = yym8
+					if false {
+					} else {
+						r.EncodeString(codecSelferC_UTF81234, string(x.APIVersion))
+					}
 				}
 			}
 			if yyr2 || yy2arr2 {

pkg/apis/rbac/v1alpha1/types.go

@@ -54,9 +54,8 @@ 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.
 	Kind string `json:"kind" protobuf:"bytes,1,opt,name=kind"`
-	// APIVersion holds the API group and version of the referenced object. For non-object references such as "Group" and "User" this is
-	// expected to be API version of this API group. For example "rbac/v1alpha1".
-	APIVersion string `json:"apiVersion" protobuf:"bytes,2,opt.name=apiVersion"`
+	// APIVersion holds the API group and version of the referenced object.
+	APIVersion string `json:"apiVersion,omitempty" protobuf:"bytes,2,opt.name=apiVersion"`
 	// Name of the object being referenced.
 	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

pkg/apis/rbac/v1alpha1/types_swagger_doc_generated.go

@@ -126,7 +126,7 @@ func (RoleList) SwaggerDoc() map[string]string {
 var map_Subject = map[string]string{
 	"":           "Subject contains a reference to the object or user identities a role binding applies to.  This can either hold a direct API object reference, or a value for non-objects such as user and group names.",
 	"kind":       "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.",
-	"apiVersion": "APIVersion holds the API group and version of the referenced object. For non-object references such as \"Group\" and \"User\" this is expected to be API version of this API group. For example \"rbac/v1alpha1\".",
+	"apiVersion": "APIVersion holds the API group and version of the referenced object.",
 	"name":       "Name of the object being referenced.",
 	"namespace":  "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.",
 }

pkg/apis/rbac/validation/validation.go

@@ -105,9 +105,6 @@ func validateRoleBindingSubject(subject rbac.Subject, isNamespaced bool, fldPath
 	if len(subject.Name) == 0 {
 		allErrs = append(allErrs, field.Required(fldPath.Child("name"), ""))
 	}
-	if len(subject.APIVersion) != 0 {
-		allErrs = append(allErrs, field.Forbidden(fldPath.Child("apiVersion"), subject.APIVersion))
-	}
 
 	switch subject.Kind {
 	case rbac.ServiceAccountKind:

pkg/apis/rbac/validation/validation_test.go

@@ -96,15 +96,6 @@ func TestValidateRoleBinding(t *testing.T) {
 			T: field.ErrorTypeInvalid,
 			F: "subjects[0].name",
 		},
-		"forbidden fields": {
-			A: rbac.RoleBinding{
-				ObjectMeta: api.ObjectMeta{Namespace: api.NamespaceDefault, Name: "master"},
-				RoleRef:    api.ObjectReference{Namespace: "master", Name: "valid"},
-				Subjects:   []rbac.Subject{{Name: "subject", Kind: rbac.ServiceAccountKind, APIVersion: "foo"}},
-			},
-			T: field.ErrorTypeForbidden,
-			F: "subjects[0].apiVersion",
-		},
 		"missing subject name": {
 			A: rbac.RoleBinding{
 				ObjectMeta: api.ObjectMeta{Namespace: api.NamespaceDefault, Name: "master"},