Skip to content

Commit bd62503

Browse files
authored
Merge pull request kubernetes#76337 from johnSchnake/frameworkAuthUtilRefactor
[e2e] Refactor of e2e/framework/authorizer_util.go
2 parents 0772f85 + 028df04 commit bd62503

File tree

18 files changed

+156
-54
lines changed

18 files changed

+156
-54
lines changed

test/e2e/BUILD

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ go_library(
6464
"//staging/src/k8s.io/component-base/logs:go_default_library",
6565
"//test/e2e/common:go_default_library",
6666
"//test/e2e/framework:go_default_library",
67+
"//test/e2e/framework/auth:go_default_library",
6768
"//test/e2e/framework/ginkgowrapper:go_default_library",
6869
"//test/e2e/framework/metrics:go_default_library",
6970
"//test/e2e/framework/providers/aws:go_default_library",

test/e2e/auth/BUILD

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ go_library(
5353
"//staging/src/k8s.io/client-go/util/cert:go_default_library",
5454
"//test/e2e/common:go_default_library",
5555
"//test/e2e/framework:go_default_library",
56+
"//test/e2e/framework/auth:go_default_library",
5657
"//test/e2e/framework/job:go_default_library",
5758
"//test/utils:go_default_library",
5859
"//test/utils/image:go_default_library",

test/e2e/auth/audit.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,14 +31,15 @@ import (
3131
"k8s.io/apimachinery/pkg/types"
3232
"k8s.io/apimachinery/pkg/util/wait"
3333
auditinternal "k8s.io/apiserver/pkg/apis/audit"
34-
"k8s.io/apiserver/pkg/apis/audit/v1"
34+
auditv1 "k8s.io/apiserver/pkg/apis/audit/v1"
3535
clientset "k8s.io/client-go/kubernetes"
3636
restclient "k8s.io/client-go/rest"
3737
"k8s.io/kubernetes/test/e2e/framework"
38+
"k8s.io/kubernetes/test/e2e/framework/auth"
3839
"k8s.io/kubernetes/test/utils"
3940
imageutils "k8s.io/kubernetes/test/utils/image"
4041

41-
"github.com/evanphx/json-patch"
42+
jsonpatch "github.com/evanphx/json-patch"
4243
. "github.com/onsi/ginkgo"
4344
)
4445

@@ -652,7 +653,7 @@ var _ = SIGDescribe("Advanced Audit [DisabledForLargeClusters][Flaky]", func() {
652653

653654
// test authorizer annotations, RBAC is required.
654655
It("should audit API calls to get a pod with unauthorized user.", func() {
655-
if !framework.IsRBACEnabled(f) {
656+
if !auth.IsRBACEnabled(f.ClientSet.RbacV1beta1()) {
656657
framework.Skipf("RBAC not enabled.")
657658
}
658659

@@ -735,7 +736,7 @@ func expectEvents(f *framework.Framework, expectedEvents []utils.AuditEvent) {
735736
return false, err
736737
}
737738
defer stream.Close()
738-
missingReport, err := utils.CheckAuditLines(stream, expectedEvents, v1.SchemeGroupVersion)
739+
missingReport, err := utils.CheckAuditLines(stream, expectedEvents, auditv1.SchemeGroupVersion)
739740
if err != nil {
740741
framework.Logf("Failed to observe audit events: %v", err)
741742
} else if len(missingReport.MissingEvents) > 0 {

test/e2e/auth/audit_dynamic.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ import (
3535
clientset "k8s.io/client-go/kubernetes"
3636
restclient "k8s.io/client-go/rest"
3737
"k8s.io/kubernetes/test/e2e/framework"
38+
"k8s.io/kubernetes/test/e2e/framework/auth"
3839
"k8s.io/kubernetes/test/utils"
3940
imageutils "k8s.io/kubernetes/test/utils/image"
4041
)
@@ -346,7 +347,7 @@ var _ = SIGDescribe("[Feature:DynamicAudit]", func() {
346347
},
347348
}
348349

349-
if framework.IsRBACEnabled(f) {
350+
if auth.IsRBACEnabled(f.ClientSet.RbacV1beta1()) {
350351
testCases = append(testCases, annotationTestCases...)
351352
}
352353
expectedEvents := []utils.AuditEvent{}

test/e2e/auth/pod_security_policy.go

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ package auth
1919
import (
2020
"fmt"
2121

22-
"k8s.io/api/core/v1"
22+
v1 "k8s.io/api/core/v1"
2323
policy "k8s.io/api/policy/v1beta1"
2424
rbacv1beta1 "k8s.io/api/rbac/v1beta1"
2525
apierrs "k8s.io/apimachinery/pkg/api/errors"
@@ -33,6 +33,7 @@ import (
3333
psputil "k8s.io/kubernetes/pkg/security/podsecuritypolicy/util"
3434
"k8s.io/kubernetes/test/e2e/common"
3535
"k8s.io/kubernetes/test/e2e/framework"
36+
"k8s.io/kubernetes/test/e2e/framework/auth"
3637
imageutils "k8s.io/kubernetes/test/utils/image"
3738
utilpointer "k8s.io/utils/pointer"
3839

@@ -54,7 +55,7 @@ var _ = SIGDescribe("PodSecurityPolicy", func() {
5455
if !framework.IsPodSecurityPolicyEnabled(f) {
5556
framework.Skipf("PodSecurityPolicy not enabled")
5657
}
57-
if !framework.IsRBACEnabled(f) {
58+
if !auth.IsRBACEnabled(f.ClientSet.RbacV1beta1()) {
5859
framework.Skipf("RBAC not enabled")
5960
}
6061
ns = f.Namespace.Name
@@ -70,8 +71,9 @@ var _ = SIGDescribe("PodSecurityPolicy", func() {
7071
framework.ExpectNoError(err)
7172

7273
By("Binding the edit role to the default SA")
73-
framework.BindClusterRole(f.ClientSet.RbacV1beta1(), "edit", ns,
74+
err = auth.BindClusterRole(f.ClientSet.RbacV1beta1(), "edit", ns,
7475
rbacv1beta1.Subject{Kind: rbacv1beta1.ServiceAccountKind, Namespace: ns, Name: "default"})
76+
framework.ExpectNoError(err)
7577
})
7678

7779
It("should forbid pod creation when no PSP is available", func() {
@@ -202,7 +204,6 @@ func testPrivilegedPods(tester func(pod *v1.Pod)) {
202204
sysadmin.Spec.Containers[0].SecurityContext.RunAsUser = &uid
203205
tester(sysadmin)
204206
})
205-
206207
}
207208

208209
// createAndBindPSP creates a PSP in the policy API group.
@@ -231,12 +232,14 @@ func createAndBindPSP(f *framework.Framework, pspTemplate *policy.PodSecurityPol
231232
framework.ExpectNoError(err, "Failed to create PSP role")
232233

233234
// Bind the role to the namespace.
234-
framework.BindRoleInNamespace(f.ClientSet.RbacV1beta1(), name, ns, rbacv1beta1.Subject{
235+
err = auth.BindRoleInNamespace(f.ClientSet.RbacV1beta1(), name, ns, rbacv1beta1.Subject{
235236
Kind: rbacv1beta1.ServiceAccountKind,
236237
Namespace: ns,
237238
Name: "default",
238239
})
239-
framework.ExpectNoError(framework.WaitForNamedAuthorizationUpdate(f.ClientSet.AuthorizationV1beta1(),
240+
framework.ExpectNoError(err)
241+
242+
framework.ExpectNoError(auth.WaitForNamedAuthorizationUpdate(f.ClientSet.AuthorizationV1beta1(),
240243
serviceaccount.MakeUsername(ns, "default"), ns, "use", name,
241244
schema.GroupResource{Group: "policy", Resource: "podsecuritypolicies"}, true))
242245

test/e2e/examples.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import (
3030
podutil "k8s.io/kubernetes/pkg/api/v1/pod"
3131
commonutils "k8s.io/kubernetes/test/e2e/common"
3232
"k8s.io/kubernetes/test/e2e/framework"
33+
"k8s.io/kubernetes/test/e2e/framework/auth"
3334
"k8s.io/kubernetes/test/e2e/framework/testfiles"
3435

3536
. "github.com/onsi/ginkgo"
@@ -51,10 +52,11 @@ var _ = framework.KubeDescribe("[Feature:Example]", func() {
5152

5253
// this test wants powerful permissions. Since the namespace names are unique, we can leave this
5354
// lying around so we don't have to race any caches
54-
framework.BindClusterRoleInNamespace(c.RbacV1beta1(), "edit", f.Namespace.Name,
55+
err := auth.BindClusterRoleInNamespace(c.RbacV1beta1(), "edit", f.Namespace.Name,
5556
rbacv1beta1.Subject{Kind: rbacv1beta1.ServiceAccountKind, Namespace: f.Namespace.Name, Name: "default"})
57+
framework.ExpectNoError(err)
5658

57-
err := framework.WaitForAuthorizationUpdate(c.AuthorizationV1beta1(),
59+
err = auth.WaitForAuthorizationUpdate(c.AuthorizationV1beta1(),
5860
serviceaccount.MakeUsername(f.Namespace.Name, "default"),
5961
f.Namespace.Name, "create", schema.GroupResource{Resource: "pods"}, true)
6062
framework.ExpectNoError(err)

test/e2e/framework/BUILD

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library")
55
go_library(
66
name = "go_default_library",
77
srcs = [
8-
"authorizer_util.go",
98
"cleanup.go",
109
"create.go",
1110
"deployment_util.go",
@@ -68,7 +67,6 @@ go_library(
6867
"//pkg/volume/util:go_default_library",
6968
"//staging/src/k8s.io/api/apps/v1:go_default_library",
7069
"//staging/src/k8s.io/api/apps/v1beta2:go_default_library",
71-
"//staging/src/k8s.io/api/authorization/v1beta1:go_default_library",
7270
"//staging/src/k8s.io/api/batch/v1:go_default_library",
7371
"//staging/src/k8s.io/api/core/v1:go_default_library",
7472
"//staging/src/k8s.io/api/extensions/v1beta1:go_default_library",
@@ -103,9 +101,7 @@ go_library(
103101
"//staging/src/k8s.io/client-go/dynamic:go_default_library",
104102
"//staging/src/k8s.io/client-go/kubernetes:go_default_library",
105103
"//staging/src/k8s.io/client-go/kubernetes/scheme:go_default_library",
106-
"//staging/src/k8s.io/client-go/kubernetes/typed/authorization/v1beta1:go_default_library",
107104
"//staging/src/k8s.io/client-go/kubernetes/typed/core/v1:go_default_library",
108-
"//staging/src/k8s.io/client-go/kubernetes/typed/rbac/v1beta1:go_default_library",
109105
"//staging/src/k8s.io/client-go/rest:go_default_library",
110106
"//staging/src/k8s.io/client-go/restmapper:go_default_library",
111107
"//staging/src/k8s.io/client-go/scale:go_default_library",
@@ -116,6 +112,7 @@ go_library(
116112
"//staging/src/k8s.io/client-go/tools/watch:go_default_library",
117113
"//staging/src/k8s.io/client-go/util/retry:go_default_library",
118114
"//staging/src/k8s.io/component-base/cli/flag:go_default_library",
115+
"//test/e2e/framework/auth:go_default_library",
119116
"//test/e2e/framework/ginkgowrapper:go_default_library",
120117
"//test/e2e/framework/metrics:go_default_library",
121118
"//test/e2e/framework/testfiles:go_default_library",
@@ -148,6 +145,7 @@ filegroup(
148145
name = "all-srcs",
149146
srcs = [
150147
":package-srcs",
148+
"//test/e2e/framework/auth:all-srcs",
151149
"//test/e2e/framework/config:all-srcs",
152150
"//test/e2e/framework/ginkgowrapper:all-srcs",
153151
"//test/e2e/framework/gpu:all-srcs",

test/e2e/framework/auth/BUILD

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
load("@io_bazel_rules_go//go:def.bzl", "go_library")
2+
3+
go_library(
4+
name = "go_default_library",
5+
srcs = ["helpers.go"],
6+
importpath = "k8s.io/kubernetes/test/e2e/framework/auth",
7+
visibility = ["//visibility:public"],
8+
deps = [
9+
"//staging/src/k8s.io/api/authorization/v1beta1:go_default_library",
10+
"//staging/src/k8s.io/api/rbac/v1beta1:go_default_library",
11+
"//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library",
12+
"//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
13+
"//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
14+
"//staging/src/k8s.io/apimachinery/pkg/util/wait:go_default_library",
15+
"//staging/src/k8s.io/client-go/kubernetes/typed/authorization/v1beta1:go_default_library",
16+
"//staging/src/k8s.io/client-go/kubernetes/typed/rbac/v1beta1:go_default_library",
17+
"//vendor/github.com/onsi/ginkgo:go_default_library",
18+
"//vendor/github.com/pkg/errors:go_default_library",
19+
],
20+
)
21+
22+
filegroup(
23+
name = "package-srcs",
24+
srcs = glob(["**"]),
25+
tags = ["automanaged"],
26+
visibility = ["//visibility:private"],
27+
)
28+
29+
filegroup(
30+
name = "all-srcs",
31+
srcs = [":package-srcs"],
32+
tags = ["automanaged"],
33+
visibility = ["//visibility:public"],
34+
)

test/e2e/framework/authorizer_util.go renamed to test/e2e/framework/auth/helpers.go

Lines changed: 67 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,15 @@ See the License for the specific language governing permissions and
1414
limitations under the License.
1515
*/
1616

17-
package framework
17+
package auth
1818

1919
import (
20-
"k8s.io/klog"
20+
"fmt"
2121
"sync"
2222
"time"
2323

24+
"github.com/onsi/ginkgo"
25+
"github.com/pkg/errors"
2426
authorizationv1beta1 "k8s.io/api/authorization/v1beta1"
2527
rbacv1beta1 "k8s.io/api/rbac/v1beta1"
2628
apierrors "k8s.io/apimachinery/pkg/api/errors"
@@ -36,6 +38,12 @@ const (
3638
policyCachePollTimeout = 5 * time.Second
3739
)
3840

41+
type bindingsGetter interface {
42+
v1beta1rbac.RoleBindingsGetter
43+
v1beta1rbac.ClusterRoleBindingsGetter
44+
v1beta1rbac.ClusterRolesGetter
45+
}
46+
3947
// WaitForAuthorizationUpdate checks if the given user can perform the named verb and action.
4048
// If policyCachePollTimeout is reached without the expected condition matching, an error is returned
4149
func WaitForAuthorizationUpdate(c v1beta1authorization.SubjectAccessReviewsGetter, user, namespace, verb string, resource schema.GroupResource, allowed bool) error {
@@ -57,12 +65,15 @@ func WaitForNamedAuthorizationUpdate(c v1beta1authorization.SubjectAccessReviews
5765
User: user,
5866
},
5967
}
68+
6069
err := wait.Poll(policyCachePollInterval, policyCachePollTimeout, func() (bool, error) {
6170
response, err := c.SubjectAccessReviews().Create(review)
6271
// GKE doesn't enable the SAR endpoint. Without this endpoint, we cannot determine if the policy engine
6372
// has adjusted as expected. In this case, simply wait one second and hope it's up to date
73+
// TODO: Should have a check for the provider here but that introduces too tight of
74+
// coupling with the `framework` package. See: https://github.com/kubernetes/kubernetes/issues/76726
6475
if apierrors.IsNotFound(err) {
65-
klog.Info("SubjectAccessReview endpoint is missing")
76+
logf("SubjectAccessReview endpoint is missing")
6677
time.Sleep(1 * time.Second)
6778
return true, nil
6879
}
@@ -77,8 +88,13 @@ func WaitForNamedAuthorizationUpdate(c v1beta1authorization.SubjectAccessReviews
7788
return err
7889
}
7990

80-
// BindClusterRole binds the cluster role at the cluster scope
81-
func BindClusterRole(c v1beta1rbac.ClusterRoleBindingsGetter, clusterRole, ns string, subjects ...rbacv1beta1.Subject) {
91+
// BindClusterRole binds the cluster role at the cluster scope. If RBAC is not enabled, nil
92+
// is returned with no action.
93+
func BindClusterRole(c bindingsGetter, clusterRole, ns string, subjects ...rbacv1beta1.Subject) error {
94+
if !IsRBACEnabled(c) {
95+
return nil
96+
}
97+
8298
// Since the namespace names are unique, we can leave this lying around so we don't have to race any caches
8399
_, err := c.ClusterRoleBindings().Create(&rbacv1beta1.ClusterRoleBinding{
84100
ObjectMeta: metav1.ObjectMeta{
@@ -92,23 +108,30 @@ func BindClusterRole(c v1beta1rbac.ClusterRoleBindingsGetter, clusterRole, ns st
92108
Subjects: subjects,
93109
})
94110

95-
// if we failed, don't fail the entire test because it may still work. RBAC may simply be disabled.
96111
if err != nil {
97-
klog.Errorf("Error binding clusterrole/%s for %q for %v\n", clusterRole, ns, subjects)
112+
return errors.Wrapf(err, "binding clusterrole/%s for %q for %v", clusterRole, ns, subjects)
98113
}
114+
115+
return nil
99116
}
100117

101-
// BindClusterRoleInNamespace binds the cluster role at the namespace scope
102-
func BindClusterRoleInNamespace(c v1beta1rbac.RoleBindingsGetter, clusterRole, ns string, subjects ...rbacv1beta1.Subject) {
103-
bindInNamespace(c, "ClusterRole", clusterRole, ns, subjects...)
118+
// BindClusterRoleInNamespace binds the cluster role at the namespace scope. If RBAC is not enabled, nil
119+
// is returned with no action.
120+
func BindClusterRoleInNamespace(c bindingsGetter, clusterRole, ns string, subjects ...rbacv1beta1.Subject) error {
121+
return bindInNamespace(c, "ClusterRole", clusterRole, ns, subjects...)
104122
}
105123

106-
// BindRoleInNamespace binds the role at the namespace scope
107-
func BindRoleInNamespace(c v1beta1rbac.RoleBindingsGetter, role, ns string, subjects ...rbacv1beta1.Subject) {
108-
bindInNamespace(c, "Role", role, ns, subjects...)
124+
// BindRoleInNamespace binds the role at the namespace scope. If RBAC is not enabled, nil
125+
// is returned with no action.
126+
func BindRoleInNamespace(c bindingsGetter, role, ns string, subjects ...rbacv1beta1.Subject) error {
127+
return bindInNamespace(c, "Role", role, ns, subjects...)
109128
}
110129

111-
func bindInNamespace(c v1beta1rbac.RoleBindingsGetter, roleType, role, ns string, subjects ...rbacv1beta1.Subject) {
130+
func bindInNamespace(c bindingsGetter, roleType, role, ns string, subjects ...rbacv1beta1.Subject) error {
131+
if !IsRBACEnabled(c) {
132+
return nil
133+
}
134+
112135
// Since the namespace names are unique, we can leave this lying around so we don't have to race any caches
113136
_, err := c.RoleBindings(ns).Create(&rbacv1beta1.RoleBinding{
114137
ObjectMeta: metav1.ObjectMeta{
@@ -122,10 +145,11 @@ func bindInNamespace(c v1beta1rbac.RoleBindingsGetter, roleType, role, ns string
122145
Subjects: subjects,
123146
})
124147

125-
// if we failed, don't fail the entire test because it may still work. RBAC may simply be disabled.
126148
if err != nil {
127-
klog.Errorf("Error binding %s/%s into %q for %v\n", roleType, role, ns, subjects)
149+
return errors.Wrapf(err, "binding %s/%s into %q for %v", roleType, role, ns, subjects)
128150
}
151+
152+
return nil
129153
}
130154

131155
var (
@@ -134,19 +158,41 @@ var (
134158
)
135159

136160
// IsRBACEnabled returns true if RBAC is enabled. Otherwise false.
137-
func IsRBACEnabled(f *Framework) bool {
161+
func IsRBACEnabled(crGetter v1beta1rbac.ClusterRolesGetter) bool {
138162
isRBACEnabledOnce.Do(func() {
139-
crs, err := f.ClientSet.RbacV1().ClusterRoles().List(metav1.ListOptions{})
163+
crs, err := crGetter.ClusterRoles().List(metav1.ListOptions{})
140164
if err != nil {
141-
Logf("Error listing ClusterRoles; assuming RBAC is disabled: %v", err)
165+
logf("Error listing ClusterRoles; assuming RBAC is disabled: %v", err)
142166
isRBACEnabled = false
143167
} else if crs == nil || len(crs.Items) == 0 {
144-
Logf("No ClusterRoles found; assuming RBAC is disabled.")
168+
logf("No ClusterRoles found; assuming RBAC is disabled.")
145169
isRBACEnabled = false
146170
} else {
147-
Logf("Found ClusterRoles; assuming RBAC is enabled.")
171+
logf("Found ClusterRoles; assuming RBAC is enabled.")
148172
isRBACEnabled = true
149173
}
150174
})
175+
151176
return isRBACEnabled
152177
}
178+
179+
// logf logs INFO lines to the GinkgoWriter.
180+
// TODO: Log functions like these should be put into their own package,
181+
// see: https://github.com/kubernetes/kubernetes/issues/76728
182+
func logf(format string, args ...interface{}) {
183+
log("INFO", format, args...)
184+
}
185+
186+
// log prints formatted log messages to the global GinkgoWriter.
187+
// TODO: Log functions like these should be put into their own package,
188+
// see: https://github.com/kubernetes/kubernetes/issues/76728
189+
func log(level string, format string, args ...interface{}) {
190+
fmt.Fprintf(ginkgo.GinkgoWriter, nowStamp()+": "+level+": "+format+"\n", args...)
191+
}
192+
193+
// nowStamp returns the current time formatted for placement in the logs (time.StampMilli).
194+
// TODO: If only used for logging, this should be put into a logging package,
195+
// see: https://github.com/kubernetes/kubernetes/issues/76728
196+
func nowStamp() string {
197+
return time.Now().Format(time.StampMilli)
198+
}

0 commit comments

Comments
 (0)