Skip to content

Commit 096f1fa

Browse files
simo5simo5
committed
Update RoleBindingRestriction admission to handle RBAC
Signed-off-by: Simo Sorce <[email protected]>
1 parent 047c984 commit 096f1fa

File tree

3 files changed

+102
-26
lines changed

3 files changed

+102
-26
lines changed

‎pkg/authorization/admission/restrictusers/restrictusers.go

+40-1
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
kerrors "k8s.io/apimachinery/pkg/util/errors"
1313
"k8s.io/apiserver/pkg/admission"
1414
kapi "k8s.io/kubernetes/pkg/api"
15+
"k8s.io/kubernetes/pkg/apis/rbac"
1516
kadmission "k8s.io/kubernetes/pkg/kubeapiserver/admission"
1617

1718
authorizationapi "github.com/openshift/origin/pkg/authorization/apis/authorization"
@@ -98,7 +99,12 @@ func (q *restrictUsersAdmission) Admit(a admission.Attributes) (err error) {
9899
// We only care about rolebindings and policybindings; ignore anything else.
99100
gr := a.GetResource().GroupResource()
100101
switch {
101-
case authorizationapi.IsResourceOrLegacy("policybindings", gr), authorizationapi.IsResourceOrLegacy("rolebindings", gr):
102+
case authorizationapi.IsResourceOrLegacy("policybindings", gr):
103+
// legacy policy bindings
104+
case authorizationapi.IsResourceOrLegacy("rolebindings", gr):
105+
// legacy origin policy based rolebindings
106+
case gr == rbac.Resource("rolebindings"):
107+
// RBAC based role binding
102108
default:
103109
return nil
104110
}
@@ -118,6 +124,39 @@ func (q *restrictUsersAdmission) Admit(a admission.Attributes) (err error) {
118124

119125
obj, oldObj := a.GetObject(), a.GetOldObject()
120126
switch {
127+
case gr == rbac.Resource("rolebindings"):
128+
rolebinding, ok := obj.(*rbac.RoleBinding)
129+
if !ok {
130+
return admission.NewForbidden(a,
131+
fmt.Errorf("wrong object type for new rolebinding: %T", obj))
132+
}
133+
134+
if len(rolebinding.Subjects) == 0 {
135+
return nil
136+
}
137+
subjects, err = authorizationapi.Convert_rbac_Subjects_To_authorization_Subjects(rolebinding.Subjects)
138+
if err != nil {
139+
return admission.NewForbidden(a,
140+
fmt.Errorf("Failed to convert subjects for rolebinding: %v", err))
141+
}
142+
143+
if oldObj != nil {
144+
oldrolebinding, ok := oldObj.(*rbac.RoleBinding)
145+
if !ok {
146+
return admission.NewForbidden(a,
147+
fmt.Errorf("wrong object type for old rolebinding: %T", oldObj))
148+
}
149+
150+
oldSubjects, err = authorizationapi.Convert_rbac_Subjects_To_authorization_Subjects(oldrolebinding.Subjects)
151+
if err != nil {
152+
return admission.NewForbidden(a,
153+
fmt.Errorf("Failed to convert subjects for rolebinding: %v", err))
154+
}
155+
}
156+
157+
glog.V(4).Infof("Handling rolebinding %s/%s",
158+
rolebinding.Namespace, rolebinding.Name)
159+
121160
case authorizationapi.IsResourceOrLegacy("rolebindings", gr):
122161
rolebinding, ok := obj.(*authorizationapi.RoleBinding)
123162
if !ok {

‎pkg/authorization/admission/restrictusers/restrictusers_test.go

+59-22
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"k8s.io/apiserver/pkg/admission"
1111
"k8s.io/apiserver/pkg/authentication/user"
1212
kapi "k8s.io/kubernetes/pkg/api"
13+
"k8s.io/kubernetes/pkg/apis/rbac"
1314
kadmission "k8s.io/kubernetes/pkg/kubeapiserver/admission"
1415
//kcache "k8s.io/client-go/tools/cache"
1516
"k8s.io/apimachinery/pkg/runtime"
@@ -22,6 +23,19 @@ import (
2223
userapi "github.com/openshift/origin/pkg/user/apis/user"
2324
)
2425

26+
type testCase struct {
27+
name string
28+
expectedErr string
29+
30+
object runtime.Object
31+
oldObject runtime.Object
32+
kind schema.GroupVersionKind
33+
resource schema.GroupVersionResource
34+
namespace string
35+
subresource string
36+
objects []runtime.Object
37+
}
38+
2539
func TestAdmission(t *testing.T) {
2640
var (
2741
userAlice = userapi.User{
@@ -79,18 +93,7 @@ func TestAdmission(t *testing.T) {
7993
}
8094
)
8195

82-
testCases := []struct {
83-
name string
84-
expectedErr string
85-
86-
object runtime.Object
87-
oldObject runtime.Object
88-
kind schema.GroupVersionKind
89-
resource schema.GroupVersionResource
90-
namespace string
91-
subresource string
92-
objects []runtime.Object
93-
}{
96+
testCases := []testCase{
9497
{
9598
name: "ignore (allow) if subresource is nonempty",
9699
object: &authorizationapi.RoleBinding{
@@ -127,7 +130,7 @@ func TestAdmission(t *testing.T) {
127130
Name: "rolebinding",
128131
},
129132
Subjects: []kapi.ObjectReference{userAliceRef},
130-
RoleRef: kapi.ObjectReference{Namespace: authorizationapi.PolicyName},
133+
RoleRef: kapi.ObjectReference{Namespace: "namespace"},
131134
},
132135
oldObject: &authorizationapi.RoleBinding{
133136
ObjectMeta: metav1.ObjectMeta{
@@ -163,15 +166,15 @@ func TestAdmission(t *testing.T) {
163166
systemgroupRef,
164167
systemuserRef,
165168
},
166-
RoleRef: kapi.ObjectReference{Namespace: authorizationapi.PolicyName},
169+
RoleRef: kapi.ObjectReference{Namespace: "namespace"},
167170
},
168171
oldObject: &authorizationapi.RoleBinding{
169172
ObjectMeta: metav1.ObjectMeta{
170173
Namespace: "namespace",
171174
Name: "rolebinding",
172175
},
173176
Subjects: []kapi.ObjectReference{},
174-
RoleRef: kapi.ObjectReference{Namespace: authorizationapi.PolicyName},
177+
RoleRef: kapi.ObjectReference{Namespace: "namespace"},
175178
},
176179
kind: authorizationapi.Kind("RoleBinding").WithVersion("version"),
177180
resource: authorizationapi.Resource("rolebindings").WithVersion("version"),
@@ -197,7 +200,7 @@ func TestAdmission(t *testing.T) {
197200
groupRef,
198201
serviceaccountRef,
199202
},
200-
RoleRef: kapi.ObjectReference{Namespace: authorizationapi.PolicyName},
203+
RoleRef: kapi.ObjectReference{Namespace: "namespace"},
201204
},
202205
oldObject: &authorizationapi.RoleBinding{
203206
ObjectMeta: metav1.ObjectMeta{
@@ -209,7 +212,7 @@ func TestAdmission(t *testing.T) {
209212
groupRef,
210213
serviceaccountRef,
211214
},
212-
RoleRef: kapi.ObjectReference{Namespace: authorizationapi.PolicyName},
215+
RoleRef: kapi.ObjectReference{Namespace: "namespace"},
213216
},
214217
kind: authorizationapi.Kind("RoleBinding").WithVersion("version"),
215218
resource: authorizationapi.Resource("rolebindings").WithVersion("version"),
@@ -246,15 +249,15 @@ func TestAdmission(t *testing.T) {
246249
serviceaccountRef,
247250
groupRef,
248251
},
249-
RoleRef: kapi.ObjectReference{Namespace: authorizationapi.PolicyName},
252+
RoleRef: kapi.ObjectReference{Namespace: "namespace"},
250253
},
251254
oldObject: &authorizationapi.RoleBinding{
252255
ObjectMeta: metav1.ObjectMeta{
253256
Namespace: "namespace",
254257
Name: "rolebinding",
255258
},
256259
Subjects: []kapi.ObjectReference{},
257-
RoleRef: kapi.ObjectReference{Namespace: authorizationapi.PolicyName},
260+
RoleRef: kapi.ObjectReference{Namespace: "namespace"},
258261
},
259262
kind: authorizationapi.Kind("RoleBinding").WithVersion("version"),
260263
resource: authorizationapi.Resource("rolebindings").WithVersion("version"),
@@ -340,15 +343,15 @@ func TestAdmission(t *testing.T) {
340343
Subjects: []kapi.ObjectReference{
341344
userAliceRef,
342345
},
343-
RoleRef: kapi.ObjectReference{Namespace: authorizationapi.PolicyName},
346+
RoleRef: kapi.ObjectReference{Namespace: "namespace"},
344347
},
345348
oldObject: &authorizationapi.RoleBinding{
346349
ObjectMeta: metav1.ObjectMeta{
347350
Namespace: "namespace",
348351
Name: "rolebinding",
349352
},
350353
Subjects: []kapi.ObjectReference{},
351-
RoleRef: kapi.ObjectReference{Namespace: authorizationapi.PolicyName},
354+
RoleRef: kapi.ObjectReference{Namespace: "namespace"},
352355
},
353356
kind: authorizationapi.Kind("RoleBinding").WithVersion("version"),
354357
resource: authorizationapi.Resource("rolebindings").WithVersion("version"),
@@ -569,6 +572,40 @@ func TestAdmission(t *testing.T) {
569572
stopCh := make(chan struct{})
570573
defer close(stopCh)
571574

575+
// for each testCase that involves an authorizationapi rolebinding also
576+
// automatically create a rbac rolebinding test case by converting the
577+
// object
578+
convTestCases := []testCase{}
579+
for _, tc := range testCases {
580+
switch tc.object.(type) {
581+
case *authorizationapi.RoleBinding:
582+
newObj := rbac.RoleBinding{}
583+
oldObj := rbac.RoleBinding{}
584+
if err := authorizationapi.Convert_authorization_RoleBinding_To_rbac_RoleBinding(tc.object.(*authorizationapi.RoleBinding), &newObj, nil); err != nil {
585+
t.Errorf("unexpected error converting role binding: %v", err)
586+
}
587+
if err := authorizationapi.Convert_authorization_RoleBinding_To_rbac_RoleBinding(tc.oldObject.(*authorizationapi.RoleBinding), &oldObj, nil); err != nil {
588+
t.Errorf("unexpected error converting old role binding: %v", err)
589+
}
590+
591+
convtc := testCase{
592+
name: "rbac: " + tc.name,
593+
expectedErr: tc.expectedErr,
594+
object: &newObj,
595+
oldObject: &oldObj,
596+
kind: rbac.Kind("RoleBinding").WithVersion("version"),
597+
resource: rbac.Resource("rolebindings").WithVersion("version"),
598+
namespace: tc.namespace,
599+
subresource: tc.subresource,
600+
objects: tc.objects,
601+
}
602+
convTestCases = append(convTestCases, convtc)
603+
default:
604+
// ignore
605+
}
606+
}
607+
testCases = append(testCases, convTestCases...)
608+
572609
for _, tc := range testCases {
573610
kclientset := fake.NewSimpleClientset(otestclient.UpstreamObjects(tc.objects)...)
574611
oclient := otestclient.NewSimpleFake(otestclient.OriginObjects(tc.objects)...)
@@ -592,7 +629,7 @@ func TestAdmission(t *testing.T) {
592629
tc.oldObject,
593630
tc.kind,
594631
tc.namespace,
595-
"name",
632+
tc.name,
596633
tc.resource,
597634
tc.subresource,
598635
admission.Create,

‎pkg/authorization/apis/authorization/conversion.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,7 @@ func Convert_rbac_Role_To_authorization_Role(in *rbac.Role, out *Role, _ convers
172172

173173
func Convert_rbac_ClusterRoleBinding_To_authorization_ClusterRoleBinding(in *rbac.ClusterRoleBinding, out *ClusterRoleBinding, _ conversion.Scope) error {
174174
var err error
175-
if out.Subjects, err = convert_rbac_Subjects_To_authorization_Subjects(in.Subjects); err != nil {
175+
if out.Subjects, err = Convert_rbac_Subjects_To_authorization_Subjects(in.Subjects); err != nil {
176176
return err
177177
}
178178
if out.RoleRef, err = convert_rbac_RoleRef_To_authorization_RoleRef(&in.RoleRef, ""); err != nil {
@@ -184,7 +184,7 @@ func Convert_rbac_ClusterRoleBinding_To_authorization_ClusterRoleBinding(in *rba
184184

185185
func Convert_rbac_RoleBinding_To_authorization_RoleBinding(in *rbac.RoleBinding, out *RoleBinding, _ conversion.Scope) error {
186186
var err error
187-
if out.Subjects, err = convert_rbac_Subjects_To_authorization_Subjects(in.Subjects); err != nil {
187+
if out.Subjects, err = Convert_rbac_Subjects_To_authorization_Subjects(in.Subjects); err != nil {
188188
return err
189189
}
190190
if out.RoleRef, err = convert_rbac_RoleRef_To_authorization_RoleRef(&in.RoleRef, in.Namespace); err != nil {
@@ -194,7 +194,7 @@ func Convert_rbac_RoleBinding_To_authorization_RoleBinding(in *rbac.RoleBinding,
194194
return nil
195195
}
196196

197-
func convert_rbac_Subjects_To_authorization_Subjects(in []rbac.Subject) ([]api.ObjectReference, error) {
197+
func Convert_rbac_Subjects_To_authorization_Subjects(in []rbac.Subject) ([]api.ObjectReference, error) {
198198
subjects := make([]api.ObjectReference, 0, len(in))
199199
for _, subject := range in {
200200
s := api.ObjectReference{

0 commit comments

Comments
 (0)