Skip to content

Commit cdc0ee7

Browse files
demonCoder95demonCoder95
committed
Refactor how the testCase output is evaluated
We now match each SubjectAccessReview object with the expected result. We then populate a list of all those that didn't meet expectation. If a test fails, we use the String() function on the testCaseOutput to print all these violating SARs. If there are not such SARs, we consider the test passed.
1 parent 0566c2a commit cdc0ee7

File tree

2 files changed

+94
-94
lines changed

2 files changed

+94
-94
lines changed

‎test/e2e/authorization.go

Lines changed: 36 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -52,17 +52,14 @@ var _ = g.Describe("Authorization [RBAC] [Zalando]", func() {
5252
// will clear these values for other specs.
5353
// https://onsi.github.io/ginkgo/#organizing-specs-with-container-nodes
5454
tc.data.resources = []string{"users", "groups"}
55-
tc.run(context.TODO(), cs)
56-
output := tc.output
57-
gomega.Expect(output.denied).To(gomega.BeTrue())
58-
55+
tc.run(context.TODO(), cs, false)
56+
gomega.Expect(tc.output.passed).To(gomega.BeTrue(), tc.output.String())
5957
})
6058
g.It("should deny access for service accounts", func() {
6159
tc.data.resources = []string{"serviceaccounts"}
6260
tc.data.namespaces = []string{"", "teapot", "kube-system"}
63-
tc.run(context.TODO(), cs)
64-
output := tc.output
65-
gomega.Expect(output.denied).To(gomega.BeTrue())
61+
tc.run(context.TODO(), cs, false)
62+
gomega.Expect(tc.output.passed).To(gomega.BeTrue(), tc.output.String())
6663
})
6764
})
6865
g.When("the verb is escalate", func() {
@@ -72,16 +69,14 @@ var _ = g.Describe("Authorization [RBAC] [Zalando]", func() {
7269

7370
g.It("should deny access for cluster roles", func() {
7471
tc.data.resources = []string{"rbac.authorization.k8s.io/clusterrole"}
75-
tc.run(context.TODO(), cs)
76-
output := tc.output
77-
gomega.Expect(output.denied).To(gomega.BeTrue())
72+
tc.run(context.TODO(), cs, false)
73+
gomega.Expect(tc.output.passed).To(gomega.BeTrue(), tc.output.String())
7874
})
7975
g.It("should deny access for roles in all namespaces", func() {
8076
tc.data.resources = []string{"rbac.authorization.k8s.io/role"}
8177
tc.data.namespaces = []string{"", "teapot", "kube-system"}
82-
tc.run(context.TODO(), cs)
83-
output := tc.output
84-
gomega.Expect(output.denied).To(gomega.BeTrue())
78+
tc.run(context.TODO(), cs, false)
79+
gomega.Expect(tc.output.passed).To(gomega.BeTrue(), tc.output.String())
8580
})
8681
})
8782
})
@@ -99,9 +94,8 @@ var _ = g.Describe("Authorization [RBAC] [Zalando]", func() {
9994
g.It("should deny access in all namespaces", func() {
10095
tc.data.verbs = []string{"get", "list", "watch", "create", "update", "delete", "patch"}
10196
tc.data.namespaces = []string{"", "teapot", "kube-system"}
102-
tc.run(context.TODO(), cs)
103-
output := tc.output
104-
gomega.Expect(output.denied).To(gomega.BeTrue())
97+
tc.run(context.TODO(), cs, false)
98+
gomega.Expect(tc.output.passed).To(gomega.BeTrue(), tc.output.String())
10599
})
106100
})
107101
g.When("the resource is not a Secret resource", func() {
@@ -122,16 +116,13 @@ var _ = g.Describe("Authorization [RBAC] [Zalando]", func() {
122116
})
123117
g.It("should allow read access in all namespaces", func() {
124118
tc.data.verbs = []string{"get", "list", "watch"}
125-
tc.run(context.TODO(), cs)
126-
output := tc.output
127-
gomega.Expect(output.allowed).To(gomega.BeTrue(),
128-
"Reason: %v", output.reason)
119+
tc.run(context.TODO(), cs, true)
120+
gomega.Expect(tc.output.passed).To(gomega.BeTrue(), tc.output.String())
129121
})
130122
g.It("should deny write access in all namespaces", func() {
131123
tc.data.verbs = []string{"create", "update", "delete", "patch"}
132-
tc.run(context.TODO(), cs)
133-
output := tc.output
134-
gomega.Expect(output.denied).To(gomega.BeTrue())
124+
tc.run(context.TODO(), cs, false)
125+
gomega.Expect(tc.output.passed).To(gomega.BeTrue(), tc.output.String())
135126
})
136127
})
137128
g.When("the resource is a global resource", func() {
@@ -145,16 +136,13 @@ var _ = g.Describe("Authorization [RBAC] [Zalando]", func() {
145136
}
146137
g.It("should allow read access", func() {
147138
tc.data.verbs = []string{"get", "list", "watch"}
148-
tc.run(context.TODO(), cs)
149-
output := tc.output
150-
gomega.Expect(output.allowed).To(gomega.BeTrue(),
151-
"Reason: %v", output.reason)
139+
tc.run(context.TODO(), cs, true)
140+
gomega.Expect(tc.output.passed).To(gomega.BeTrue(), tc.output.String())
152141
})
153142
g.It("should deny write access", func() {
154143
tc.data.verbs = []string{"create", "update", "delete", "patch"}
155-
tc.run(context.TODO(), cs)
156-
output := tc.output
157-
gomega.Expect(output.denied).To(gomega.BeTrue())
144+
tc.run(context.TODO(), cs, false)
145+
gomega.Expect(tc.output.passed).To(gomega.BeTrue(), tc.output.String())
158146
})
159147
})
160148
})
@@ -175,43 +163,37 @@ var _ = g.Describe("Authorization [RBAC] [Zalando]", func() {
175163
tc.data.resources = []string{"secrets"}
176164
tc.data.namespaces = []string{"kube-system", "visibility"}
177165
tc.data.verbs = []string{"get", "list", "watch"}
178-
tc.run(context.TODO(), cs)
179-
output := tc.output
180-
gomega.Expect(output.denied).To(gomega.BeTrue())
166+
tc.run(context.TODO(), cs, false)
167+
gomega.Expect(tc.output.passed).To(gomega.BeTrue(), tc.output.String())
181168
})
182169

183170
g.It("should deny write access to Nodes", func() {
184171
tc.data.resources = []string{"nodes"}
185172
tc.data.verbs = []string{"create", "update", "delete", "patch"}
186-
tc.run(context.TODO(), cs)
187-
output := tc.output
188-
gomega.Expect(output.denied).To(gomega.BeTrue())
173+
tc.run(context.TODO(), cs, false)
174+
gomega.Expect(tc.output.passed).To(gomega.BeTrue(), tc.output.String())
189175
})
190176

191177
g.It("should deny write access to DaemonSets", func() {
192178
tc.data.resources = []string{"apps/daemonsets"}
193179
tc.data.verbs = []string{"create", "update", "delete", "patch"}
194-
tc.run(context.TODO(), cs)
195-
output := tc.output
196-
gomega.Expect(output.denied).To(gomega.BeTrue())
180+
tc.run(context.TODO(), cs, false)
181+
gomega.Expect(tc.output.passed).To(gomega.BeTrue(), tc.output.String())
197182
})
198183

199-
// TODO: Double check if the original test case is correct
200184
g.It("should allow deleting CRDs", func() {
201185
tc.data.resources = []string{"apiextensions.k8s.io/customresourcedefinitions"}
202186
tc.data.verbs = []string{"delete"}
203-
tc.run(context.TODO(), cs)
204-
output := tc.output
205-
gomega.Expect(output.allowed).To(gomega.BeTrue())
187+
tc.run(context.TODO(), cs, true)
188+
gomega.Expect(tc.output.passed).To(gomega.BeTrue(), tc.output.String())
206189
})
207190

208191
g.It("should deny deleting kube-system or visibility namespaces", func() {
209192
tc.data.resources = []string{"namespaces"}
210193
tc.data.namespaces = []string{"kube-system", "visibility"}
211194
tc.data.verbs = []string{"delete"}
212-
tc.run(context.TODO(), cs)
213-
output := tc.output
214-
gomega.Expect(output.denied).To(gomega.BeTrue())
195+
tc.run(context.TODO(), cs, false)
196+
gomega.Expect(tc.output.passed).To(gomega.BeTrue(), tc.output.String())
215197
})
216198

217199
g.When("the resource is a namespaced resource", func() {
@@ -231,16 +213,13 @@ var _ = g.Describe("Authorization [RBAC] [Zalando]", func() {
231213
})
232214
g.It("should deny write access in kube-system and visibility namespaces", func() {
233215
tc.data.namespaces = []string{"kube-system", "visibility"}
234-
tc.run(context.TODO(), cs)
235-
output := tc.output
236-
gomega.Expect(output.denied).To(gomega.BeTrue())
216+
tc.run(context.TODO(), cs, false)
217+
gomega.Expect(tc.output.passed).To(gomega.BeTrue(), tc.output.String())
237218
})
238219
g.It("should allow write access in namespaces other than kube-system and visibility", func() {
239220
tc.data.namespaces = []string{"", "teapot"}
240-
tc.run(context.TODO(), cs)
241-
output := tc.output
242-
gomega.Expect(output.allowed).To(gomega.BeTrue(),
243-
"Reason: %v", output.reason)
221+
tc.run(context.TODO(), cs, true)
222+
gomega.Expect(tc.output.passed).To(gomega.BeTrue(), tc.output.String())
244223
})
245224
})
246225
g.When("the resource is a global resource", func() {
@@ -249,20 +228,17 @@ var _ = g.Describe("Authorization [RBAC] [Zalando]", func() {
249228
})
250229
g.It("should deny write access to Nodes", func() {
251230
tc.data.resources = []string{"nodes"}
252-
tc.run(context.TODO(), cs)
253-
output := tc.output
254-
gomega.Expect(output.denied).To(gomega.BeTrue())
231+
tc.run(context.TODO(), cs, false)
232+
gomega.Expect(tc.output.passed).To(gomega.BeTrue(), tc.output.String())
255233
})
256234
g.It("should allow write access to resources other than Nodes", func() {
257235
tc.data.resources = []string{
258236
"namespaces",
259237
"storage.k8s.io/storageclasses",
260238
"apiextensions.k8s.io/customresourcedefinitions",
261239
}
262-
tc.run(context.TODO(), cs)
263-
output := tc.output
264-
gomega.Expect(output.allowed).To(gomega.BeTrue(),
265-
"Reason: %v", output.reason)
240+
tc.run(context.TODO(), cs, true)
241+
gomega.Expect(tc.output.passed).To(gomega.BeTrue(), tc.output.String())
266242
})
267243
})
268244
})

‎test/e2e/authorization_utils.go

Lines changed: 58 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package e2e
22

33
import (
44
"context"
5+
"strconv"
56
"strings"
67

78
authv1 "k8s.io/api/authorization/v1"
@@ -41,12 +42,24 @@ type testcaseData struct {
4142
// and we need to determine the "final" result and expose that as the testcase output.
4243
type testcaseOutput struct {
4344
// the final result based on results of individual SubjectAccessReview objects
44-
allowed, denied bool
45-
// the set of reasons from individual SubjectAccessReview objects
46-
reason []string
45+
passed bool
46+
// the set of SARs that failed expectation in the test. This is an empty slice if the test passed.
47+
// It contains pretty printed SAR objects for debugging, when a test fails. It will
48+
// contain all the SAR objects that didn't match expectation.
49+
failingSARs []string
4750
}
4851

49-
func (t *testCase) run(ctx context.Context, cs kubernetes.Interface) error {
52+
// String returns a pretty printed string of the testcase output. This is used
53+
// to help debug the RBAC test cases.
54+
func (o *testcaseOutput) String() string {
55+
outputStr := ""
56+
for _, sar := range o.failingSARs {
57+
outputStr += sar
58+
}
59+
return outputStr
60+
}
61+
62+
func (t *testCase) run(ctx context.Context, cs kubernetes.Interface, allowExpected bool) error {
5063
// Generate the list of SubjectAccessReview objects based on the testcase data
5164
sars := t.generateSubjectAccessReviews()
5265

@@ -58,7 +71,7 @@ func (t *testCase) run(ctx context.Context, cs kubernetes.Interface) error {
5871

5972
// Evaluate the output based on the created SubjectAccessReview objects
6073
// and set the final result in the testcase output
61-
t.evaluateOutput(createdSars)
74+
t.evaluateOutput(createdSars, allowExpected)
6275

6376
return nil
6477
}
@@ -309,40 +322,51 @@ func createSubjectAccessReview(ctx context.Context, cs kubernetes.Interface, sar
309322
}
310323

311324
// evaluateOutput evaluates the output based on the created SubjectAccessReview objects
312-
func (t *testCase) evaluateOutput(createdSars []authv1.SubjectAccessReview) {
313-
tcOutput := testcaseOutput{}
314-
// TODO: Test should only pass if all SubjectAccessReviews have expected
315-
// value. Need to rethink this composition logic.
316-
// For example if we have 3 SubjectAccessReviews and the expecataion is 'deny',
317-
// then ALL 3 of them should have a 'denied: true' in response. In this implementation
318-
// even if 1 of them was denied, the test would pass even if the other 2 were allowed.
319-
320-
// Iterate over all the SubjectAccessReviews created and determine the final result
321-
// We don't break the loop if we have denied access from one SubjectAccessReview,
322-
// we continue to check all of them and collect all reasons.
325+
// allowExpected is a boolean that determines if the expected result is 'allow' or 'deny'.
326+
func (t *testCase) evaluateOutput(createdSars []authv1.SubjectAccessReview, allowExpected bool) {
327+
328+
//TODO: check if it's safe to override the output object of the testcase like this
329+
330+
// Iterate over all the SubjectAccessReviews created and check for expecated result.
331+
// We don't break the loop if a result doesn't match expectation since we want to
332+
// capture all the failing SubjectAccessReviews for debugging.
323333
for _, sar := range createdSars {
324-
// We skip the SubjectAccessReviews that are allowed
325-
if sar.Status.Allowed {
326-
continue
327-
}
328-
// If any of the SubjectAccessReviews have denied access, the final result is denied
329-
if sar.Status.Denied {
330-
tcOutput.denied = true
331-
tcOutput.reason = append(tcOutput.reason, sar.Status.Reason)
332-
continue
334+
// if the expected result is 'allow' and the SAR is denied, we add it to the failingSARs
335+
if allowExpected && !sar.Status.Allowed {
336+
t.output.failingSARs = append(t.output.failingSARs, prettyPrintSAR(sar))
333337
}
334-
// Undecided access is also considered as denied
335-
if !sar.Status.Allowed && !sar.Status.Denied {
336-
tcOutput.denied = true
337-
tcOutput.reason = append(tcOutput.reason, sar.Status.Reason)
338-
continue
338+
// if the expected result is 'deny' and the SAR is allowed, we add it to the failingSARs
339+
if !allowExpected && sar.Status.Allowed {
340+
t.output.failingSARs = append(t.output.failingSARs, prettyPrintSAR(sar))
339341
}
340342
}
341343

342-
// If none of the SubjectAccessReviews have denied access, the final result is allowed
343-
if !tcOutput.denied {
344-
tcOutput.allowed = true
344+
// if the failingSARs is empty, it means the test passed
345+
if len(t.output.failingSARs) == 0 {
346+
t.output.passed = true
345347
}
348+
}
346349

347-
t.output = tcOutput
350+
// prettyPrintSAR pretty prints the SubjectAccessReview object. This is used
351+
// to help debug the RBAC test cases.
352+
func prettyPrintSAR(sar authv1.SubjectAccessReview) string {
353+
str := "SubjectAccessReviewSpec:"
354+
str += "\n Namespace: " + sar.Spec.ResourceAttributes.Namespace
355+
str += "\n Verb: " + sar.Spec.ResourceAttributes.Verb
356+
str += "\n APIGroup: " + sar.Spec.ResourceAttributes.Group
357+
str += "\n Resource: " + sar.Spec.ResourceAttributes.Resource
358+
str += "\n Subresource: " + sar.Spec.ResourceAttributes.Subresource
359+
str += "\n Name: " + sar.Spec.ResourceAttributes.Name
360+
if sar.Spec.NonResourceAttributes != nil {
361+
str += "\n NonResourcePath: " + sar.Spec.NonResourceAttributes.Path
362+
str += "\n NonResourceVerb: " + sar.Spec.NonResourceAttributes.Verb
363+
}
364+
str += "\n User: " + sar.Spec.User
365+
str += "\n Groups: " + strings.Join(sar.Spec.Groups, ",")
366+
str += "\nSubjectAccessReviewStatus:"
367+
str += "\n Allowed: " + strconv.FormatBool(sar.Status.Allowed)
368+
str += "\n Denied: " + strconv.FormatBool(sar.Status.Denied)
369+
str += "\n Reason: " + sar.Status.Reason
370+
str += "\n"
371+
return str
348372
}

0 commit comments

Comments
 (0)