Skip to content

Commit 05d2e14

Browse files
openshift-merge-robotopenshift-merge-robot
authored
Merge pull request #17009 from jim-minter/issue16654
Automatic merge from submit-queue (batch tested with PRs 17007, 17003, 17001, 17009). wait for group cache in templateinstance tests fixes #16654 (builds on #16979) @gabemontero fyi
2 parents 01f2147 + 9d8b6be commit 05d2e14

File tree

2 files changed

+102
-65
lines changed

2 files changed

+102
-65
lines changed

‎test/extended/templates/templateinstance_impersonation.go

Lines changed: 81 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,16 @@
11
package templates
22

33
import (
4+
"time"
5+
46
g "github.com/onsi/ginkgo"
57
o "github.com/onsi/gomega"
68

79
kerrors "k8s.io/apimachinery/pkg/api/errors"
810
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
911
"k8s.io/apimachinery/pkg/runtime"
1012
"k8s.io/apimachinery/pkg/util/sets"
13+
"k8s.io/apimachinery/pkg/util/wait"
1114
kapi "k8s.io/kubernetes/pkg/api"
1215
"k8s.io/kubernetes/pkg/client/retry"
1316

@@ -53,17 +56,91 @@ var _ = g.Describe("[Conformance][templates] templateinstance impersonation test
5356
)
5457

5558
g.BeforeEach(func() {
56-
var err error
57-
5859
adminuser = createUser(cli, "adminuser", bootstrappolicy.AdminRoleName)
5960
impersonateuser = createUser(cli, "impersonateuser", bootstrappolicy.EditRoleName)
6061
impersonatebygroupuser = createUser(cli, "impersonatebygroupuser", bootstrappolicy.EditRoleName)
61-
impersonategroup = createGroup(cli, "impersonategroup", bootstrappolicy.EditRoleName)
62-
addUserToGroup(cli, impersonatebygroupuser.Name, impersonategroup.Name)
6362
edituser1 = createUser(cli, "edituser1", bootstrappolicy.EditRoleName)
6463
edituser2 = createUser(cli, "edituser2", bootstrappolicy.EditRoleName)
6564
viewuser = createUser(cli, "viewuser", bootstrappolicy.ViewRoleName)
6665

66+
impersonategroup = createGroup(cli, "impersonategroup", bootstrappolicy.EditRoleName)
67+
addUserToGroup(cli, impersonatebygroupuser.Name, impersonategroup.Name)
68+
69+
// additional plumbing to enable impersonateuser to impersonate edituser1
70+
role, err := cli.AdminAuthorizationClient().Authorization().Roles(cli.Namespace()).Create(&authorizationapi.Role{
71+
ObjectMeta: metav1.ObjectMeta{
72+
Name: "impersonater",
73+
},
74+
Rules: []authorizationapi.PolicyRule{
75+
{
76+
Verbs: sets.NewString("assign"),
77+
APIGroups: []string{templateapi.GroupName},
78+
Resources: sets.NewString("templateinstances"),
79+
},
80+
},
81+
})
82+
o.Expect(err).NotTo(o.HaveOccurred())
83+
84+
_, err = cli.AdminAuthorizationClient().Authorization().RoleBindings(cli.Namespace()).Create(&authorizationapi.RoleBinding{
85+
ObjectMeta: metav1.ObjectMeta{
86+
Name: "impersonater-binding",
87+
},
88+
RoleRef: kapi.ObjectReference{
89+
Name: role.Name,
90+
Namespace: cli.Namespace(),
91+
},
92+
Subjects: []kapi.ObjectReference{
93+
{
94+
Kind: authorizationapi.UserKind,
95+
Name: impersonateuser.Name,
96+
},
97+
{
98+
Kind: authorizationapi.GroupKind,
99+
Name: impersonategroup.Name,
100+
},
101+
},
102+
})
103+
o.Expect(err).NotTo(o.HaveOccurred())
104+
105+
// I think we get flakes when the group cache hasn't yet noticed the
106+
// new group membership made above. Wait until all it looks like
107+
// all the users above have access to the namespace as expected.
108+
err = wait.PollImmediate(time.Second, 30*time.Second, func() (done bool, err error) {
109+
for _, user := range []*userapi.User{adminuser, impersonateuser, impersonatebygroupuser, edituser1, edituser2, viewuser} {
110+
cli.ChangeUser(user.Name)
111+
sar, err := cli.AuthorizationClient().Authorization().LocalSubjectAccessReviews(cli.Namespace()).Create(&authorizationapi.LocalSubjectAccessReview{
112+
Action: authorizationapi.Action{
113+
Verb: "get",
114+
Resource: "pods",
115+
},
116+
})
117+
if err != nil {
118+
return false, err
119+
}
120+
if !sar.Allowed {
121+
return false, nil
122+
}
123+
}
124+
125+
cli.ChangeUser(impersonatebygroupuser.Name)
126+
sar, err := cli.AuthorizationClient().Authorization().LocalSubjectAccessReviews(cli.Namespace()).Create(&authorizationapi.LocalSubjectAccessReview{
127+
Action: authorizationapi.Action{
128+
Verb: "assign",
129+
Group: templateapi.GroupName,
130+
Resource: "templateinstances",
131+
},
132+
})
133+
if err != nil {
134+
return false, err
135+
}
136+
if !sar.Allowed {
137+
return false, nil
138+
}
139+
140+
return true, nil
141+
})
142+
o.Expect(err).NotTo(o.HaveOccurred())
143+
67144
dummytemplateinstance = &templateapi.TemplateInstance{
68145
ObjectMeta: metav1.ObjectMeta{
69146
Name: "test",
@@ -141,42 +218,6 @@ var _ = g.Describe("[Conformance][templates] templateinstance impersonation test
141218
hasUpdateStatusPermission: false,
142219
},
143220
}
144-
145-
// additional plumbing to enable impersonateuser to impersonate edituser1
146-
role, err := cli.AdminAuthorizationClient().Authorization().Roles(cli.Namespace()).Create(&authorizationapi.Role{
147-
ObjectMeta: metav1.ObjectMeta{
148-
Name: "impersonater",
149-
},
150-
Rules: []authorizationapi.PolicyRule{
151-
{
152-
Verbs: sets.NewString("assign"),
153-
APIGroups: []string{templateapi.GroupName},
154-
Resources: sets.NewString("templateinstances"),
155-
},
156-
},
157-
})
158-
o.Expect(err).NotTo(o.HaveOccurred())
159-
160-
_, err = cli.AdminAuthorizationClient().Authorization().RoleBindings(cli.Namespace()).Create(&authorizationapi.RoleBinding{
161-
ObjectMeta: metav1.ObjectMeta{
162-
Name: "impersonater-binding",
163-
},
164-
RoleRef: kapi.ObjectReference{
165-
Name: role.Name,
166-
Namespace: cli.Namespace(),
167-
},
168-
Subjects: []kapi.ObjectReference{
169-
{
170-
Kind: authorizationapi.UserKind,
171-
Name: impersonateuser.Name,
172-
},
173-
{
174-
Kind: authorizationapi.GroupKind,
175-
Name: impersonategroup.Name,
176-
},
177-
},
178-
})
179-
o.Expect(err).NotTo(o.HaveOccurred())
180221
})
181222

182223
g.AfterEach(func() {

‎test/extended/templates/templateinstance_security.go

Lines changed: 21 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -77,32 +77,28 @@ var _ = g.Describe("[Conformance][templates] templateinstance security tests", f
7777
editgroup = createGroup(cli, "editgroup", bootstrappolicy.EditRoleName)
7878
addUserToGroup(cli, editbygroupuser.Name, editgroup.Name)
7979

80-
/*
81-
// jminter: commenting this out for now in case it turns out to be superstition
82-
83-
// I think we get flakes when the group cache hasn't yet noticed the
84-
// new group membership made above. Wait until all it looks like
85-
// all the users above have access to the namespace as expected.
86-
err := wait.PollImmediate(time.Second, 30*time.Second, func() (done bool, err error) {
87-
for _, user := range []*userapi.User{adminuser, edituser, editbygroupuser} {
88-
cli.ChangeUser(user.Name)
89-
sar, err := cli.AuthorizationClient().Authorization().LocalSubjectAccessReviews(cli.Namespace()).Create(&authorizationapi.LocalSubjectAccessReview{
90-
Action: authorizationapi.Action{
91-
Verb: "get",
92-
Resource: "pods",
93-
},
94-
})
95-
if err != nil {
96-
return false, err
97-
}
98-
if !sar.Allowed {
99-
return false, nil
100-
}
80+
// I think we get flakes when the group cache hasn't yet noticed the
81+
// new group membership made above. Wait until all it looks like
82+
// all the users above have access to the namespace as expected.
83+
err := wait.PollImmediate(time.Second, 30*time.Second, func() (done bool, err error) {
84+
for _, user := range []*userapi.User{adminuser, edituser, editbygroupuser} {
85+
cli.ChangeUser(user.Name)
86+
sar, err := cli.AuthorizationClient().Authorization().LocalSubjectAccessReviews(cli.Namespace()).Create(&authorizationapi.LocalSubjectAccessReview{
87+
Action: authorizationapi.Action{
88+
Verb: "get",
89+
Resource: "pods",
90+
},
91+
})
92+
if err != nil {
93+
return false, err
10194
}
102-
return true, nil
103-
})
104-
o.Expect(err).NotTo(o.HaveOccurred())
105-
*/
95+
if !sar.Allowed {
96+
return false, nil
97+
}
98+
}
99+
return true, nil
100+
})
101+
o.Expect(err).NotTo(o.HaveOccurred())
106102
})
107103

108104
g.AfterEach(func() {

0 commit comments

Comments
 (0)