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.

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

updates admission attributes to eliminate ambiguity #17972

Merged
merged 1 commit into from Dec 7, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 6 additions & 5 deletions pkg/admission/attributes.go
Expand Up @@ -17,22 +17,23 @@ limitations under the License.
package admission

import (
"k8s.io/kubernetes/pkg/api/unversioned"
"k8s.io/kubernetes/pkg/auth/user"
"k8s.io/kubernetes/pkg/runtime"
)

type attributesRecord struct {
kind string
kind unversioned.GroupKind
namespace string
name string
resource string
resource unversioned.GroupResource
subresource string
operation Operation
object runtime.Object
userInfo user.Info
}

func NewAttributesRecord(object runtime.Object, kind, namespace, name, resource, subresource string, operation Operation, userInfo user.Info) Attributes {
func NewAttributesRecord(object runtime.Object, kind unversioned.GroupKind, namespace, name string, resource unversioned.GroupResource, subresource string, operation Operation, userInfo user.Info) Attributes {
return &attributesRecord{
kind: kind,
namespace: namespace,
Expand All @@ -45,7 +46,7 @@ func NewAttributesRecord(object runtime.Object, kind, namespace, name, resource,
}
}

func (record *attributesRecord) GetKind() string {
func (record *attributesRecord) GetKind() unversioned.GroupKind {
return record.kind
}

Expand All @@ -57,7 +58,7 @@ func (record *attributesRecord) GetName() string {
return record.name
}

func (record *attributesRecord) GetResource() string {
func (record *attributesRecord) GetResource() unversioned.GroupResource {
return record.resource
}

Expand Down
4 changes: 3 additions & 1 deletion pkg/admission/chain_test.go
Expand Up @@ -19,6 +19,8 @@ package admission
import (
"fmt"
"testing"

"k8s.io/kubernetes/pkg/api/unversioned"
)

type FakeHandler struct {
Expand Down Expand Up @@ -98,7 +100,7 @@ func TestAdmit(t *testing.T) {
},
}
for _, test := range tests {
err := test.chain.Admit(NewAttributesRecord(nil, "", "", "", "", "", test.operation, nil))
err := test.chain.Admit(NewAttributesRecord(nil, unversioned.GroupKind{}, "", "", unversioned.GroupResource{}, "", test.operation, nil))
accepted := (err == nil)
if accepted != test.accept {
t.Errorf("%s: unexpected result of admit call: %v\n", test.name, accepted)
Expand Down
9 changes: 5 additions & 4 deletions pkg/admission/errors.go
Expand Up @@ -19,17 +19,18 @@ package admission
import (
"k8s.io/kubernetes/pkg/api"
apierrors "k8s.io/kubernetes/pkg/api/errors"
"k8s.io/kubernetes/pkg/api/unversioned"
utilerrors "k8s.io/kubernetes/pkg/util/errors"
)

func extractKindName(a Attributes) (name, kind string, err error) {
func extractKindName(a Attributes) (name string, kind unversioned.GroupKind, err error) {
name = "Unknown"
kind = a.GetKind()
obj := a.GetObject()
if obj != nil {
objectMeta, err := api.ObjectMetaFor(obj)
if err != nil {
return "", "", err
return "", unversioned.GroupKind{}, err
}

// this is necessary because name object name generation has not occurred yet
Expand All @@ -52,7 +53,7 @@ func NewForbidden(a Attributes, internalError error) error {
if err != nil {
return apierrors.NewInternalError(utilerrors.NewAggregate([]error{internalError, err}))
}
return apierrors.NewForbidden(kind, name, internalError)
return apierrors.NewForbidden(kind.Kind, name, internalError)
}

// NewNotFound is a utility function to return a well-formatted admission control error response
Expand All @@ -61,5 +62,5 @@ func NewNotFound(a Attributes) error {
if err != nil {
return apierrors.NewInternalError(err)
}
return apierrors.NewNotFound(kind, name)
return apierrors.NewNotFound(kind.Kind, name)
}
5 changes: 3 additions & 2 deletions pkg/admission/interfaces.go
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package admission

import (
"k8s.io/kubernetes/pkg/api/unversioned"
"k8s.io/kubernetes/pkg/auth/user"
"k8s.io/kubernetes/pkg/runtime"
)
Expand All @@ -31,7 +32,7 @@ type Attributes interface {
// GetNamespace is the namespace associated with the request (if any)
GetNamespace() string
// GetResource is the name of the resource being requested. This is not the kind. For example: pods
GetResource() string
GetResource() unversioned.GroupResource
// GetSubresource is the name of the subresource being requested. This is a different resource, scoped to the parent resource, but it may have a different kind.
// For instance, /pods has the resource "pods" and the kind "Pod", while /pods/foo/status has the resource "pods", the sub resource "status", and the kind "Pod"
// (because status operates on pods). The binding resource for a pod though may be /pods/foo/binding, which has resource "pods", subresource "binding", and kind "Binding".
Expand All @@ -41,7 +42,7 @@ type Attributes interface {
// GetObject is the object from the incoming request prior to default values being applied
GetObject() runtime.Object
// GetKind is the type of object being manipulated. For example: Pod
GetKind() string
GetKind() unversioned.GroupKind
// GetUserInfo is information about the requesting user
GetUserInfo() user.Info
}
Expand Down
5 changes: 5 additions & 0 deletions pkg/api/register.go
Expand Up @@ -32,6 +32,11 @@ func Kind(kind string) unversioned.GroupKind {
return SchemeGroupVersion.WithKind(kind).GroupKind()
}

// Resource takes an unqualified resource and returns back a Group qualified GroupResource
func Resource(resource string) unversioned.GroupResource {
return SchemeGroupVersion.WithResource(resource).GroupResource()
}

func init() {
Scheme.AddKnownTypes(SchemeGroupVersion,
&Pod{},
Expand Down
17 changes: 16 additions & 1 deletion pkg/api/unversioned/group_version.go
Expand Up @@ -22,6 +22,17 @@ import (
"strings"
)

// GroupResource specifies a Group and a Resource, but does not force a version. This is useful for identifying
// concepts during lookup stages without having partially valid types
type GroupResource struct {
Group string
Resource string
}

func (gr *GroupResource) String() string {
return strings.Join([]string{gr.Group, ", Resource=", gr.Resource}, "")
}

// GroupVersionResource unambiguously identifies a resource. It doesn't anonymously include GroupVersion
// to avoid automatic coersion. It doesn't use a GroupVersion to avoid custom marshalling
type GroupVersionResource struct {
Expand All @@ -30,12 +41,16 @@ type GroupVersionResource struct {
Resource string
}

func (gvr GroupVersionResource) GroupResource() GroupResource {
return GroupResource{Group: gvr.Group, Resource: gvr.Resource}
}

func (gvr GroupVersionResource) GroupVersion() GroupVersion {
return GroupVersion{Group: gvr.Group, Version: gvr.Version}
}

func (gvr *GroupVersionResource) String() string {
return gvr.Group + "/" + gvr.Version + ", Resource=" + gvr.Resource
return strings.Join([]string{gvr.Group, "/", gvr.Version, ", Resource=", gvr.Resource}, "")
}

// GroupKind specifies a Group and a Kind, but does not force a version. This is useful for identifying
Expand Down
5 changes: 5 additions & 0 deletions pkg/apis/componentconfig/register.go
Expand Up @@ -33,6 +33,11 @@ func Kind(kind string) unversioned.GroupKind {
return SchemeGroupVersion.WithKind(kind).GroupKind()
}

// Resource takes an unqualified resource and returns back a Group qualified GroupResource
func Resource(resource string) unversioned.GroupResource {
return SchemeGroupVersion.WithResource(resource).GroupResource()
}

func addKnownTypes() {
// TODO this will get cleaned up with the scheme types are fixed
api.Scheme.AddKnownTypes(SchemeGroupVersion,
Expand Down
5 changes: 5 additions & 0 deletions pkg/apis/extensions/register.go
Expand Up @@ -29,6 +29,11 @@ func Kind(kind string) unversioned.GroupKind {
return SchemeGroupVersion.WithKind(kind).GroupKind()
}

// Resource takes an unqualified resource and returns back a Group qualified GroupResource
func Resource(resource string) unversioned.GroupResource {
return SchemeGroupVersion.WithResource(resource).GroupResource()
}

func init() {
// Register the API.
addKnownTypes()
Expand Down
5 changes: 5 additions & 0 deletions pkg/apis/metrics/register.go
Expand Up @@ -34,6 +34,11 @@ func Kind(kind string) unversioned.GroupKind {
return SchemeGroupVersion.WithKind(kind).GroupKind()
}

// Resource takes an unqualified resource and returns back a Group qualified GroupResource
func Resource(resource string) unversioned.GroupResource {
return SchemeGroupVersion.WithResource(resource).GroupResource()
}

// Adds the list of known types to api.Scheme.
func addKnownTypes() {
// TODO this will get cleaned up with the scheme types are fixed
Expand Down
10 changes: 5 additions & 5 deletions pkg/apiserver/resthandler.go
Expand Up @@ -186,7 +186,7 @@ func ConnectResource(connecter rest.Connecter, scope RequestScope, admit admissi
}
userInfo, _ := api.UserFrom(ctx)

err = admit.Admit(admission.NewAttributesRecord(connectRequest, scope.Kind.Kind, namespace, name, scope.Resource.Resource, scope.Subresource, admission.Connect, userInfo))
err = admit.Admit(admission.NewAttributesRecord(connectRequest, scope.Kind.GroupKind(), namespace, name, scope.Resource.GroupResource(), scope.Subresource, admission.Connect, userInfo))
if err != nil {
errorJSON(err, scope.Codec, w)
return
Expand Down Expand Up @@ -361,7 +361,7 @@ func createHandler(r rest.NamedCreater, scope RequestScope, typer runtime.Object
if admit != nil && admit.Handles(admission.Create) {
userInfo, _ := api.UserFrom(ctx)

err = admit.Admit(admission.NewAttributesRecord(obj, scope.Kind.Kind, namespace, name, scope.Resource.Resource, scope.Subresource, admission.Create, userInfo))
err = admit.Admit(admission.NewAttributesRecord(obj, scope.Kind.GroupKind(), namespace, name, scope.Resource.GroupResource(), scope.Subresource, admission.Create, userInfo))
if err != nil {
errorJSON(err, scope.Codec, w)
return
Expand Down Expand Up @@ -432,7 +432,7 @@ func PatchResource(r rest.Patcher, scope RequestScope, typer runtime.ObjectTyper
if admit.Handles(admission.Update) {
userInfo, _ := api.UserFrom(ctx)

err = admit.Admit(admission.NewAttributesRecord(obj, scope.Kind.Kind, namespace, name, scope.Resource.Resource, scope.Subresource, admission.Update, userInfo))
err = admit.Admit(admission.NewAttributesRecord(obj, scope.Kind.GroupKind(), namespace, name, scope.Resource.GroupResource(), scope.Subresource, admission.Update, userInfo))
if err != nil {
errorJSON(err, scope.Codec, w)
return
Expand Down Expand Up @@ -599,7 +599,7 @@ func UpdateResource(r rest.Updater, scope RequestScope, typer runtime.ObjectType
if admit != nil && admit.Handles(admission.Update) {
userInfo, _ := api.UserFrom(ctx)

err = admit.Admit(admission.NewAttributesRecord(obj, scope.Kind.Kind, namespace, name, scope.Resource.Resource, scope.Subresource, admission.Update, userInfo))
err = admit.Admit(admission.NewAttributesRecord(obj, scope.Kind.GroupKind(), namespace, name, scope.Resource.GroupResource(), scope.Subresource, admission.Update, userInfo))
if err != nil {
errorJSON(err, scope.Codec, w)
return
Expand Down Expand Up @@ -664,7 +664,7 @@ func DeleteResource(r rest.GracefulDeleter, checkBody bool, scope RequestScope,
if admit != nil && admit.Handles(admission.Delete) {
userInfo, _ := api.UserFrom(ctx)

err = admit.Admit(admission.NewAttributesRecord(nil, scope.Kind.Kind, namespace, name, scope.Resource.Resource, scope.Subresource, admission.Delete, userInfo))
err = admit.Admit(admission.NewAttributesRecord(nil, scope.Kind.GroupKind(), namespace, name, scope.Resource.GroupResource(), scope.Subresource, admission.Delete, userInfo))
if err != nil {
errorJSON(err, scope.Codec, w)
return
Expand Down
3 changes: 2 additions & 1 deletion plugin/pkg/admission/deny/admission_test.go
Expand Up @@ -20,11 +20,12 @@ import (
"testing"

"k8s.io/kubernetes/pkg/admission"
"k8s.io/kubernetes/pkg/api"
)

func TestAdmission(t *testing.T) {
handler := NewAlwaysDeny()
err := handler.Admit(admission.NewAttributesRecord(nil, "kind", "namespace", "name", "resource", "subresource", admission.Create, nil))
err := handler.Admit(admission.NewAttributesRecord(nil, api.Kind("kind"), "namespace", "name", api.Resource("resource"), "subresource", admission.Create, nil))
if err == nil {
t.Errorf("Expected error returned from admission handler")
}
Expand Down
4 changes: 2 additions & 2 deletions plugin/pkg/admission/exec/admission_test.go
Expand Up @@ -101,7 +101,7 @@ func testAdmission(t *testing.T, pod *api.Pod, handler *denyExec, shouldAccept b
// pods/exec
{
req := &rest.ConnectRequest{Name: pod.Name, ResourcePath: "pods/exec"}
err := handler.Admit(admission.NewAttributesRecord(req, "Pod", "test", "name", "pods", "exec", admission.Connect, nil))
err := handler.Admit(admission.NewAttributesRecord(req, api.Kind("Pod"), "test", "name", api.Resource("pods"), "exec", admission.Connect, nil))
if shouldAccept && err != nil {
t.Errorf("Unexpected error returned from admission handler: %v", err)
}
Expand All @@ -113,7 +113,7 @@ func testAdmission(t *testing.T, pod *api.Pod, handler *denyExec, shouldAccept b
// pods/attach
{
req := &rest.ConnectRequest{Name: pod.Name, ResourcePath: "pods/attach"}
err := handler.Admit(admission.NewAttributesRecord(req, "Pod", "test", "name", "pods", "attach", admission.Connect, nil))
err := handler.Admit(admission.NewAttributesRecord(req, api.Kind("Pod"), "test", "name", api.Resource("pods"), "attach", admission.Connect, nil))
if shouldAccept && err != nil {
t.Errorf("Unexpected error returned from admission handler: %v", err)
}
Expand Down
2 changes: 1 addition & 1 deletion plugin/pkg/admission/initialresources/admission.go
Expand Up @@ -73,7 +73,7 @@ func newInitialResources(source dataSource, percentile int64, nsOnly bool) admis

func (ir initialResources) Admit(a admission.Attributes) (err error) {
// Ignore all calls to subresources or resources other than pods.
if a.GetSubresource() != "" || a.GetResource() != string(api.ResourcePods) {
if a.GetSubresource() != "" || a.GetResource() != api.Resource("pods") {
return nil
}
pod, ok := a.GetObject().(*api.Pod)
Expand Down
2 changes: 1 addition & 1 deletion plugin/pkg/admission/initialresources/admission_test.go
Expand Up @@ -107,7 +107,7 @@ func expectNoAnnotation(t *testing.T, pod *api.Pod) {
func admit(t *testing.T, ir admission.Interface, pods []*api.Pod) {
for i := range pods {
p := pods[i]
if err := ir.Admit(admission.NewAttributesRecord(p, "Pod", "test", p.ObjectMeta.Name, "pods", "", admission.Create, nil)); err != nil {
if err := ir.Admit(admission.NewAttributesRecord(p, api.Kind("Pod"), "test", p.ObjectMeta.Name, api.Resource("pods"), "", admission.Create, nil)); err != nil {
t.Error(err)
}
}
Expand Down
5 changes: 2 additions & 3 deletions plugin/pkg/admission/limitranger/admission.go
Expand Up @@ -61,7 +61,6 @@ func (l *limitRanger) Admit(a admission.Attributes) (err error) {
}

obj := a.GetObject()
resource := a.GetResource()
name := "Unknown"
if obj != nil {
name, _ = meta.NewAccessor().Name(obj)
Expand All @@ -78,7 +77,7 @@ func (l *limitRanger) Admit(a admission.Attributes) (err error) {
}
items, err := l.indexer.Index("namespace", key)
if err != nil {
return admission.NewForbidden(a, fmt.Errorf("Unable to %s %s at this time because there was an error enforcing limit ranges", a.GetOperation(), resource))
return admission.NewForbidden(a, fmt.Errorf("Unable to %s %v at this time because there was an error enforcing limit ranges", a.GetOperation(), a.GetResource()))
}
if len(items) == 0 {
return nil
Expand All @@ -87,7 +86,7 @@ func (l *limitRanger) Admit(a admission.Attributes) (err error) {
// ensure it meets each prescribed min/max
for i := range items {
limitRange := items[i].(*api.LimitRange)
err = l.limitFunc(limitRange, a.GetResource(), a.GetObject())
err = l.limitFunc(limitRange, a.GetResource().Resource, a.GetObject())
if err != nil {
return admission.NewForbidden(a, err)
}
Expand Down
4 changes: 2 additions & 2 deletions plugin/pkg/admission/limitranger/admission_test.go
Expand Up @@ -442,12 +442,12 @@ func TestLimitRangerIgnoresSubresource(t *testing.T) {
testPod := validPod("testPod", 1, api.ResourceRequirements{})

indexer.Add(&limitRange)
err := handler.Admit(admission.NewAttributesRecord(&testPod, "Pod", limitRange.Namespace, "testPod", "pods", "", admission.Update, nil))
err := handler.Admit(admission.NewAttributesRecord(&testPod, api.Kind("Pod"), limitRange.Namespace, "testPod", api.Resource("pods"), "", admission.Update, nil))
if err == nil {
t.Errorf("Expected an error since the pod did not specify resource limits in its update call")
}

err = handler.Admit(admission.NewAttributesRecord(&testPod, "Pod", limitRange.Namespace, "testPod", "pods", "status", admission.Update, nil))
err = handler.Admit(admission.NewAttributesRecord(&testPod, api.Kind("Pod"), limitRange.Namespace, "testPod", api.Resource("pods"), "status", admission.Update, nil))
if err != nil {
t.Errorf("Should have ignored calls to any subresource of pod %v", err)
}
Expand Down
2 changes: 1 addition & 1 deletion plugin/pkg/admission/namespace/autoprovision/admission.go
Expand Up @@ -46,7 +46,7 @@ type provision struct {
}

func (p *provision) Admit(a admission.Attributes) (err error) {
gvk, err := api.RESTMapper.KindFor(a.GetResource())
gvk, err := api.RESTMapper.KindFor(a.GetResource().Resource)
if err != nil {
return admission.NewForbidden(a, err)
}
Expand Down
Expand Up @@ -42,7 +42,7 @@ func TestAdmission(t *testing.T) {
Containers: []api.Container{{Name: "ctr", Image: "image"}},
},
}
err := handler.Admit(admission.NewAttributesRecord(&pod, "Pod", pod.Namespace, pod.Name, "pods", "", admission.Create, nil))
err := handler.Admit(admission.NewAttributesRecord(&pod, api.Kind("Pod"), pod.Namespace, pod.Name, api.Resource("pods"), "", admission.Create, nil))
if err != nil {
t.Errorf("Unexpected error returned from admission handler")
}
Expand Down Expand Up @@ -74,7 +74,7 @@ func TestAdmissionNamespaceExists(t *testing.T) {
Containers: []api.Container{{Name: "ctr", Image: "image"}},
},
}
err := handler.Admit(admission.NewAttributesRecord(&pod, "Pod", pod.Namespace, pod.Name, "pods", "", admission.Create, nil))
err := handler.Admit(admission.NewAttributesRecord(&pod, api.Kind("Pod"), pod.Namespace, pod.Name, api.Resource("pods"), "", admission.Create, nil))
if err != nil {
t.Errorf("Unexpected error returned from admission handler")
}
Expand All @@ -95,7 +95,7 @@ func TestIgnoreAdmission(t *testing.T) {
Containers: []api.Container{{Name: "ctr", Image: "image"}},
},
}
err := handler.Admit(admission.NewAttributesRecord(&pod, "Pod", pod.Namespace, pod.Name, "pods", "", admission.Update, nil))
err := handler.Admit(admission.NewAttributesRecord(&pod, api.Kind("Pod"), pod.Namespace, pod.Name, api.Resource("pods"), "", admission.Update, nil))
if err != nil {
t.Errorf("Unexpected error returned from admission handler")
}
Expand Down Expand Up @@ -124,7 +124,7 @@ func TestAdmissionNamespaceExistsUnknownToHandler(t *testing.T) {
Containers: []api.Container{{Name: "ctr", Image: "image"}},
},
}
err := handler.Admit(admission.NewAttributesRecord(&pod, "Pod", pod.Namespace, pod.Name, "pods", "", admission.Create, nil))
err := handler.Admit(admission.NewAttributesRecord(&pod, api.Kind("Pod"), pod.Namespace, pod.Name, api.Resource("pods"), "", admission.Create, nil))
if err != nil {
t.Errorf("Unexpected error returned from admission handler")
}
Expand Down
2 changes: 1 addition & 1 deletion plugin/pkg/admission/namespace/exists/admission.go
Expand Up @@ -47,7 +47,7 @@ type exists struct {
}

func (e *exists) Admit(a admission.Attributes) (err error) {
gvk, err := api.RESTMapper.KindFor(a.GetResource())
gvk, err := api.RESTMapper.KindFor(a.GetResource().Resource)
if err != nil {
return errors.NewInternalError(err)
}
Expand Down