Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

add unit and integration tests for rbac authorizer #26753

Merged
merged 1 commit into from
Jun 20, 2016
Merged

add unit and integration tests for rbac authorizer #26753

merged 1 commit into from
Jun 20, 2016

Conversation

ericchiang
Copy link
Contributor

This PR adds lots of tests for the RBAC authorizer.

The plan over the next couple days is to add a lot more test cases.

Updates #23396

cc @erictune

@k8s-github-robot k8s-github-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. release-note-label-needed labels Jun 2, 2016
@ixdy ixdy assigned erictune and unassigned ixdy Jun 3, 2016
@ericchiang ericchiang changed the title add unit and integration tests for rbac authorizer [wip] add unit and integration tests for rbac authorizer Jun 7, 2016
@ericchiang
Copy link
Contributor Author

@erictune this is a bit more fleshed out. Any comments?

@ericchiang
Copy link
Contributor Author

e2e testing woes

• Failure [301.928 seconds]
[k8s.io] Kubelet
/var/lib/jenkins/workspace/node-pull-build-e2e-test@2/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/test/e2e/framework/framework.go:639
  metrics api
  /var/lib/jenkins/workspace/node-pull-build-e2e-test@2/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/test/e2e_node/kubelet_test.go:165
    when querying /stats/summary
    /var/lib/jenkins/workspace/node-pull-build-e2e-test@2/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/test/e2e_node/kubelet_test.go:164
      it should report resource usage through the stats api [It]
      /var/lib/jenkins/workspace/node-pull-build-e2e-test@2/go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/test/e2e_node/kubelet_test.go:163

      Timed out after 300.000s.
      Expected
          <*errors.errorString | 0xc8209f5310>: {
              s: "expected metrics for kubelet",
          }
      to be nil

@ericchiang ericchiang mentioned this pull request Jun 8, 2016
Closed
erictune
erictune reviewed Jun 9, 2016
View reviewed changes
test/integration/rbac_test.go
// Create the namespace used later in the test
{superUser, "POST", "", "namespaces", "", "", jobNamespace, http.StatusCreated},

{"user-with-no-permissions", "POST", "batch", "jobs", "job-namespace", "", aJob, http.StatusForbidden},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are testing denial when the object does not exist. You should also test denial when the object does exist.

@erictune
Copy link
Member

erictune commented Jun 9, 2016

Suggest: for each type of object, have a test case where that object is modified, and then authz outcomes change.

@davidopp davidopp added this to the v1.3 milestone Jun 12, 2016
@davidopp
Copy link
Member

Added 1.3 milestone since the associated issue is marked 1.3

@erictune
Copy link
Member

LGTM

@erictune erictune added lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesn't merit a release note. and removed release-note-label-needed labels Jun 14, 2016
@erictune
Copy link
Member

lgtm

@erictune
Copy link
Member 

--- FAIL: TestRBAC (0.15s)
    rbac_test.go:438: case 0, req 6: pod-reader GET pods expected "200 OK" got "403 Forbidden"
    rbac_test.go:438: case 1, req 7: job-writer GET jobs expected "200 OK" got "403 Forbidden"
    rbac_test.go:438: case 1, req 8: job-writer GET jobs expected "404 Not Found" got "403 Forbidden"
    rbac_test.go:438: case 1, req 9: job-writer POST jobs expected "201 Created" got "403 Forbidden"
    rbac_test.go:438: case 1, req 10: job-writer GET jobs expected "200 OK" got "403 Forbidden"
    rbac_test.go:438: case 1, req 11: job-writer-namespace GET jobs expected "200 OK" got "403 Forbidden"
    rbac_test.go:438: case 1, req 12: job-writer-namespace GET jobs expected "404 Not Found" got "403 Forbidden"
    rbac_test.go:438: case 1, req 13: job-writer-namespace POST jobs expected "201 Created" got "403 Forbidden"
    rbac_test.go:438: case 1, req 14: job-writer-namespace GET jobs expected "200 OK" got "403 Forbidden"

@ericchiang
Copy link
Contributor Author

@erictune am looking into this

@ericchiang
Copy link
Contributor Author

git bisect says #27255 is when this started failing.

@k8s-github-robot k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 14, 2016
@ericchiang
Copy link
Contributor Author

@erictune this should be good now

@erictune
Copy link
Member

LGTM

@erictune erictune added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 15, 2016
@erictune erictune mentioned this pull request Jun 17, 2016
Closed
@erictune erictune added the priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. label Jun 17, 2016
@ericchiang
Copy link
Contributor Author

@erictune This test failure doesn't look related to the PR

# k8s.io/kubernetes/cmd/kubelet
/usr/local/go/pkg/tool/linux_amd64/link: running aarch64-linux-gnu-gcc failed: fork/exec /usr/bin/aarch64-linux-gnu-gcc: cannot allocate memory

@goltermann
Copy link
Contributor

@k8s-bot e2e test this issue: #IGNORE

@k8s-bot
Copy link

k8s-bot commented Jun 20, 2016

GCE e2e build/test passed for commit d13e351.

@k8s-github-robot
Copy link

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

@k8s-bot
Copy link

k8s-bot commented Jun 20, 2016

GCE e2e build/test passed for commit d13e351.

@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit 6fbf99b into kubernetes:master Jun 20, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@erictune erictune

Labels
lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. release-note-none Denotes a PR that doesn't merit a release note. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Milestone
v1.3
Development

Successfully merging this pull request may close these issues.

None yet
8 participants
@ericchiang @erictune @davidopp @goltermann @k8s-bot @k8s-github-robot @googlebot @ixdy