Skip to content

add unit and integration tests for rbac authorizer#26753

Merged
k8s-github-robot merged 1 commit intokubernetes:masterfrom
ericchiang:rbac-authorizer-tests
Jun 20, 2016
Merged

add unit and integration tests for rbac authorizer#26753
k8s-github-robot merged 1 commit intokubernetes:masterfrom
ericchiang:rbac-authorizer-tests

Conversation

@ericchiang
Copy link
Copy Markdown
Contributor

@ericchiang ericchiang commented Jun 2, 2016

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

@ericchiang ericchiang force-pushed the rbac-authorizer-tests branch from ad064d9 to d07ec41 Compare June 2, 2016 23:48
@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 force-pushed the rbac-authorizer-tests branch from d07ec41 to 7104d4a Compare June 7, 2016 17:31
@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
Copy Markdown
Contributor Author

ericchiang commented Jun 7, 2016

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

@ericchiang
Copy link
Copy Markdown
Contributor Author

ericchiang commented Jun 7, 2016

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 force-pushed the rbac-authorizer-tests branch from 7104d4a to cc7611b Compare June 7, 2016 22:28
@ericchiang ericchiang mentioned this pull request Jun 8, 2016
Closed
erictune
erictune reviewed Jun 9, 2016
View reviewed changes
Comment thread test/integration/rbac_test.go Outdated
Copy link
Copy Markdown
Contributor

@erictune erictune Jun 9, 2016

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
Copy Markdown
Contributor

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
Copy Markdown
Contributor

davidopp commented Jun 12, 2016

Added 1.3 milestone since the associated issue is marked 1.3

@erictune
Copy link
Copy Markdown
Contributor

erictune commented Jun 14, 2016

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
Copy Markdown
Contributor

erictune commented Jun 14, 2016

lgtm

@erictune
Copy link
Copy Markdown
Contributor

erictune commented Jun 14, 2016 

--- 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
Copy Markdown
Contributor Author

ericchiang commented Jun 14, 2016

@erictune am looking into this

@ericchiang
Copy link
Copy Markdown
Contributor Author

ericchiang commented Jun 14, 2016

git bisect says #27255 is when this started failing.

@ericchiang ericchiang force-pushed the rbac-authorizer-tests branch from cc7611b to d13e351 Compare June 14, 2016 18:07
@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
Copy Markdown
Contributor Author

ericchiang commented Jun 14, 2016

@erictune this should be good now

@erictune
Copy link
Copy Markdown
Contributor

erictune commented Jun 15, 2016

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
Copy Markdown
Contributor Author

ericchiang commented Jun 17, 2016

@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
Copy Markdown
Contributor

goltermann commented Jun 20, 2016

@k8s-bot e2e test this issue: #IGNORE

@k8s-bot
Copy link
Copy Markdown

k8s-bot commented Jun 20, 2016

GCE e2e build/test passed for commit d13e351.

@k8s-github-robot
Copy link
Copy Markdown

k8s-github-robot commented Jun 20, 2016

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

@k8s-bot
Copy link
Copy Markdown

k8s-bot commented Jun 20, 2016

GCE e2e build/test passed for commit d13e351.

@k8s-github-robot
Copy link
Copy Markdown

k8s-github-robot commented Jun 20, 2016

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.

8 participants

@ericchiang @erictune @davidopp @goltermann @k8s-bot @k8s-github-robot @googlebot @ixdy