test/e2e: Authz tests in combination #377
Conversation
The "resource" test case is not related to static.
stlaz
left a comment
stlaz
left a comment
There was a problem hiding this comment.
reviewed first two commits
| resources: | ||
| - tokenreviews |
There was a problem hiding this comment.
nit: the inconsistency in how string slices are constructed in the yaml is weird
| }, | ||
| { | ||
| name: "non-resource/get/metrics/forbidden", | ||
| name: "static/forbidden", |
There was a problem hiding this comment.
should be staticAuthz non-resource/get/metrics/forbidden
| }{ | ||
| { | ||
| name: "resource/namespace/metrics/query rewrite/granted", | ||
| name: "static/granted", |
There was a problem hiding this comment.
Needs better name. What exactly is granted?
test/e2e/combined.go
Outdated
| check kubetest.Action | ||
| }{ | ||
| { | ||
| name: "templated-query-rewrite-static/granted", |
There was a problem hiding this comment.
the original names were more descriptive, see how to incorporate them here
| @@ -0,0 +1,14 @@ | |||
| apiVersion: rbac.authorization.k8s.io/v1 | |||
There was a problem hiding this comment.
revert all of the file moves, like test/e2e/static-auth/serviceAccount.yaml → test/e2e/authz-static/serviceAccount.yaml
This is really hard to review otherwise and the benefit of the move is minimal
test/e2e/main_test.go
Outdated
| // "Basics": testBasics(client), | ||
| // "H2CUpstream": testH2CUpstream(client), |
| ), | ||
| }, | ||
| { | ||
| name: "static/forbidden -> rbac/granted", |
There was a problem hiding this comment.
We need good test names given the test framework makes the tests super cryptic on their own. I am having a hard time understanding what this might mean.
|
Any move or modification of an e2e test is currently incredibly painful to do and also to review. I created #380 to address these kinds of issues. I would like for it to merge before any other bigger changes to the e2e test suite, including this PR. |
|
I can't wait to merge it into this PR 😭 |
2 participants
What
Add more authorization tests to test the authorization chain, if different authorization methods are set.
Why
Those tests are missing and make it hard to guarantee the same behavior in the sig-auth-acceptance branch.