Skip to content

allow review endpoints on missing namespaces #11321

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

Merged
merged 2 commits into from
Oct 18, 2016
Merged

allow review endpoints on missing namespaces #11321

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8a1e1ea
UPSTREAM: <carry>: update namespace lifecycle to allow review APIs
deads2k Oct 12, 2016
e4c061f
allow review endpoints on missing namespaces
deads2k Oct 12, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 14 additions & 17 deletions pkg/project/admission/lifecycle/admission.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,11 @@ import (

"k8s.io/kubernetes/pkg/admission"
"k8s.io/kubernetes/pkg/api/meta"
"k8s.io/kubernetes/pkg/api/unversioned"
"k8s.io/kubernetes/pkg/apimachinery/registered"
clientset "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset"
"k8s.io/kubernetes/pkg/util/sets"

"github.com/openshift/origin/pkg/api"
authorizationapi "github.com/openshift/origin/pkg/authorization/api"
oadmission "github.com/openshift/origin/pkg/cmd/server/admission"
"github.com/openshift/origin/pkg/project/cache"
projectutil "github.com/openshift/origin/pkg/project/util"
Expand All @@ -33,10 +33,17 @@ type lifecycle struct {
cache *cache.ProjectCache

// creatableResources is a set of resources that can be created even if the namespace is terminating
creatableResources sets.String
creatableResources map[unversioned.GroupResource]bool
}

var recommendedCreatableResources = sets.NewString("resourceaccessreviews", "localresourceaccessreviews")
var recommendedCreatableResources = map[unversioned.GroupResource]bool{
authorizationapi.Resource("resourceaccessreviews"): true,
authorizationapi.Resource("localresourceaccessreviews"): true,
authorizationapi.Resource("subjectaccessreviews"): true,
authorizationapi.Resource("localsubjectaccessreviews"): true,
authorizationapi.Resource("selfsubjectrulesreviews"): true,
authorizationapi.Resource("subjectrulesreviews"): true,
Copy link
Contributor

Choose a reason for hiding this comment

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

subjectrulesreviews isn't in the upstream set of accessReviewResources, need a carry commit to add it there

}
var _ = oadmission.WantsProjectCache(&lifecycle{})
var _ = oadmission.Validator(&lifecycle{})

Expand All @@ -46,9 +53,8 @@ func (e *lifecycle) Admit(a admission.Attributes) (err error) {
if len(a.GetNamespace()) == 0 {
return nil
}
// always allow a SAR request through, the SAR will return information about
// the ability to take action on the object, no need to verify it here.
if isSubjectAccessReview(a) {
// always allow creatable resources through. These requests should always be allowed.
if e.creatableResources[a.GetResource().GroupResource()] {
return nil
}

Expand Down Expand Up @@ -117,18 +123,9 @@ func (e *lifecycle) Validate() error {
return nil
}

func NewLifecycle(client clientset.Interface, creatableResources sets.String) (admission.Interface, error) {
func NewLifecycle(client clientset.Interface, creatableResources map[unversioned.GroupResource]bool) (admission.Interface, error) {
return &lifecycle{
client: client,
creatableResources: creatableResources,
}, nil
}

var (
sar = api.Kind("SubjectAccessReview")
lsar = api.Kind("LocalSubjectAccessReview")
)

func isSubjectAccessReview(a admission.Attributes) bool {
return a.GetKind().GroupKind() == sar || a.GetKind().GroupKind() == lsar
}
2 changes: 1 addition & 1 deletion pkg/project/admission/lifecycle/admission_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ func TestSAR(t *testing.T) {
cache := projectcache.NewFake(mockClient.Namespaces(), store, "")

mockClientset := clientsetfake.NewSimpleClientset()
handler := &lifecycle{client: mockClientset}
handler := &lifecycle{client: mockClientset, creatableResources: recommendedCreatableResources}
handler.SetProjectCache(cache)

tests := map[string]struct {
Expand Down
17 changes: 17 additions & 0 deletions test/cmd/projects.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,21 @@ source "$(dirname "${BASH_SOURCE}")/../../hack/lib/init.sh"
trap os::test::junit::reconcile_output EXIT

os::test::junit::declare_suite_start "cmd/projects"

os::test::junit::declare_suite_start "cmd/projects/lifecycle"
# resourceaccessreview
os::cmd::expect_success 'oc policy who-can get pods -n missing-ns'
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a weak test

# selfsubjectaccessreview
os::cmd::expect_success 'oc policy can-i get pods -n missing-ns'
# selfsubjectrulesreivew
os::cmd::expect_success 'oc policy can-i --list -n missing-ns'
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a weak test

# subjectaccessreview
os::cmd::expect_success 'oc policy can-i get pods --user=bob -n missing-ns'
# subjectrulesreview
os::cmd::expect_success 'oc policy can-i --list --user=bob -n missing-ns'
echo 'project lifecycle ok'
os::test::junit::declare_suite_end

os::cmd::expect_failure_and_text 'oc projects test_arg' 'no arguments'
# log in as a test user and expect no projects
os::cmd::expect_success 'oc login -u test -p test'
Expand All @@ -21,4 +36,6 @@ os::cmd::try_until_text 'oc projects' 'test6'
os::cmd::expect_success_and_text 'oc project test6' 'Now using project "test6"'
os::cmd::expect_success_and_text 'oc config view -o jsonpath="{.contexts[*].context.namespace}"' '\btest6\b'
echo 'projects command ok'


os::test::junit::declare_suite_end
1 change: 1 addition & 0 deletions vendor/k8s.io/kubernetes/plugin/pkg/admission/namespace/lifecycle/admission.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.