Fix conversion for role binding to cluster role

This change makes it so that the conversion for authorization
policy correctly handles when a RBAC role binding references a
cluster role.  The code incorrectly assumed that the role binding's
namespace was the referenced role's namespace as well.  Now we use
the role's kind to determine its namespace.  This was missed by the
fuzzer tests because they only made role bindings to roles.  The
tests have been updated to create bindings to both types of roles.

Signed-off-by: Monis Khan <[email protected]>

Author: enj

pkg/authorization/apis/authorization/conversion.go

@@ -41,6 +41,9 @@ func Convert_authorization_Role_To_rbac_Role(in *Role, out *rbac.Role, _ convers
 }
 
 func Convert_authorization_ClusterRoleBinding_To_rbac_ClusterRoleBinding(in *ClusterRoleBinding, out *rbac.ClusterRoleBinding, _ conversion.Scope) error {
+	if len(in.RoleRef.Namespace) != 0 {
+		return fmt.Errorf("invalid origin cluster role binding %s: attempts to reference role in namespace %q instead of cluster scope", in.Name, in.RoleRef.Namespace)
+	}
 	var err error
 	if out.Subjects, err = convert_api_Subjects_To_rbac_Subjects(in.Subjects); err != nil {
 		return err
@@ -166,7 +169,9 @@ func Convert_rbac_ClusterRoleBinding_To_authorization_ClusterRoleBinding(in *rba
 	if out.Subjects, err = convert_rbac_Subjects_To_authorization_Subjects(in.Subjects); err != nil {
 		return err
 	}
-	out.RoleRef = convert_rbac_RoleRef_To_authorization_RoleRef(&in.RoleRef, "")
+	if out.RoleRef, err = convert_rbac_RoleRef_To_authorization_RoleRef(&in.RoleRef, ""); err != nil {
+		return err
+	}
 	out.ObjectMeta = in.ObjectMeta
 	return nil
 }
@@ -176,7 +181,9 @@ func Convert_rbac_RoleBinding_To_authorization_RoleBinding(in *rbac.RoleBinding,
 	if out.Subjects, err = convert_rbac_Subjects_To_authorization_Subjects(in.Subjects); err != nil {
 		return err
 	}
-	out.RoleRef = convert_rbac_RoleRef_To_authorization_RoleRef(&in.RoleRef, in.Namespace)
+	if out.RoleRef, err = convert_rbac_RoleRef_To_authorization_RoleRef(&in.RoleRef, in.Namespace); err != nil {
+		return err
+	}
 	out.ObjectMeta = in.ObjectMeta
 	return nil
 }
@@ -205,13 +212,18 @@ func convert_rbac_Subjects_To_authorization_Subjects(in []rbac.Subject) ([]api.O
 	return subjects, nil
 }
 
-// rbac.RoleRef has no namespace field since that can be inferred.
-// The Origin role ref (api.ObjectReference) requires its namespace value to match the binding's namespace.
-// Thus we have to explicitly provide that value as a parameter.
-func convert_rbac_RoleRef_To_authorization_RoleRef(in *rbac.RoleRef, namespace string) api.ObjectReference {
-	return api.ObjectReference{
-		Name:      in.Name,
-		Namespace: namespace,
+// rbac.RoleRef has no namespace field since that can be inferred from the kind of referenced role.
+// The Origin role ref (api.ObjectReference) requires its namespace value to match the binding's namespace
+// for a binding to a role.  For a binding to a cluster role, the namespace value must be the empty string.
+// Thus we have to explicitly provide the namespace value as a parameter and use it based on the role's kind.
+func convert_rbac_RoleRef_To_authorization_RoleRef(in *rbac.RoleRef, namespace string) (api.ObjectReference, error) {
+	switch in.Kind {
+	case "ClusterRole":
+		return api.ObjectReference{Name: in.Name}, nil
+	case "Role":
+		return api.ObjectReference{Name: in.Name, Namespace: namespace}, nil
+	default:
+		return api.ObjectReference{}, fmt.Errorf("invalid kind %q for rbac role ref %q", in.Kind, in.Name)
 	}
 }
 

pkg/authorization/apis/authorization/conversion_test.go

@@ -193,6 +193,18 @@ func TestConversionErrors(t *testing.T) {
 				}, &rbac.ClusterRoleBinding{}, nil)
 			},
 		},
+		{
+			name:     "invalid origin rol ref namespace",
+			expected: `invalid origin cluster role binding clusterrolebindingname: attempts to reference role in namespace "fancyns" instead of cluster scope`,
+			f: func() error {
+				return authorizationapi.Convert_authorization_ClusterRoleBinding_To_rbac_ClusterRoleBinding(&authorizationapi.ClusterRoleBinding{
+					ObjectMeta: metav1.ObjectMeta{Name: "clusterrolebindingname"},
+					RoleRef: api.ObjectReference{
+						Namespace: "fancyns",
+					},
+				}, &rbac.ClusterRoleBinding{}, nil)
+			},
+		},
 		{
 			name:     "invalid RBAC subject kind",
 			expected: `invalid kind for rbac subject: "evenfancieruser"`,
@@ -204,6 +216,18 @@ func TestConversionErrors(t *testing.T) {
 				}, &authorizationapi.ClusterRoleBinding{}, nil)
 			},
 		},
+		{
+			name:     "invalid RBAC rol ref kind",
+			expected: `invalid kind "anewfancykind" for rbac role ref "fancyrolref"`,
+			f: func() error {
+				return authorizationapi.Convert_rbac_ClusterRoleBinding_To_authorization_ClusterRoleBinding(&rbac.ClusterRoleBinding{
+					RoleRef: rbac.RoleRef{
+						Name: "fancyrolref",
+						Kind: "anewfancykind",
+					},
+				}, &authorizationapi.ClusterRoleBinding{}, nil)
+			},
+		},
 	} {
 		if err := test.f(); err == nil || test.expected != err.Error() {
 			t.Errorf("%s failed: expected %q got %v", test.name, test.expected, err)
@@ -349,7 +373,7 @@ func setRandomRBACRoleBindingData(subjects []rbac.Subject, roleRef *rbac.RoleRef
 		setValidRBACKindAndNamespace(subject, i, c)
 	}
 	roleRef.APIGroup = rbac.GroupName
-	roleRef.Kind = getRBACRoleRefKind(namespace)
+	roleRef.Kind = getRBACRoleRefKind(getRandomScope(namespace, c))
 }
 
 func setValidRBACKindAndNamespace(subject *rbac.Subject, i int, c fuzz.Continue) {
@@ -375,7 +399,15 @@ func setRandomOriginRoleBindingData(subjects []api.ObjectReference, roleRef *api
 	}
 	unsetUnusedOriginFields(roleRef)
 	roleRef.Kind = ""
-	roleRef.Namespace = namespace
+	roleRef.Namespace = getRandomScope(namespace, c)
+}
+
+// we want bindings to both cluster and local roles
+func getRandomScope(namespace string, c fuzz.Continue) string {
+	if c.RandBool() {
+		return ""
+	}
+	return namespace
 }
 
 func setValidOriginKindAndNamespace(subject *api.ObjectReference, i int, c fuzz.Continue) {