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

split limitranger admission #55487

Merged
merged 2 commits into from
Nov 13, 2017
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
2 changes: 2 additions & 0 deletions plugin/pkg/admission/defaulttolerationseconds/admission.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ type Plugin struct {
*admission.Handler
}

var _ admission.MutationInterface = &Plugin{}

// NewDefaultTolerationSeconds creates a new instance of the DefaultTolerationSeconds admission controller
func NewDefaultTolerationSeconds() *Plugin {
return &Plugin{
Expand Down
2 changes: 2 additions & 0 deletions plugin/pkg/admission/initialresources/admission.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,8 @@ type InitialResources struct {
nsOnly bool
}

var _ admission.MutationInterface = &InitialResources{}

func newInitialResources(source dataSource, percentile int64, nsOnly bool) *InitialResources {
return &InitialResources{
Handler: admission.NewHandler(admission.Create),
Expand Down
88 changes: 63 additions & 25 deletions plugin/pkg/admission/limitranger/admission.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,10 @@ type LimitRanger struct {
liveTTL time.Duration
}

var _ admission.MutationInterface = &LimitRanger{}
var _ admission.ValidationInterface = &LimitRanger{}
var _ kubeapiserveradmission.WantsInternalKubeInformerFactory = &LimitRanger{}

type liveLookupEntry struct {
expiry time.Time
items []*api.LimitRange
Expand All @@ -87,6 +91,15 @@ func (l *LimitRanger) ValidateInitialization() error {

// Admit admits resources into cluster that do not violate any defined LimitRange in the namespace
func (l *LimitRanger) Admit(a admission.Attributes) (err error) {
return l.runLimitFunc(a, l.actions.MutateLimit)
}

// Validate admits resources into cluster that do not violate any defined LimitRange in the namespace
func (l *LimitRanger) Validate(a admission.Attributes) (err error) {
return l.runLimitFunc(a, l.actions.ValidateLimit)
}

func (l *LimitRanger) runLimitFunc(a admission.Attributes, limitFn func(limitRange *api.LimitRange, kind string, obj runtime.Object) error) (err error) {
if !l.actions.SupportsAttributes(a) {
return nil
}
Expand All @@ -100,9 +113,31 @@ func (l *LimitRanger) Admit(a admission.Attributes) (err error) {
}
}

items, err := l.GetLimitRanges(a)
if err != nil {
return err
}

// ensure it meets each prescribed min/max
for i := range items {
limitRange := items[i]

if !l.actions.SupportsLimit(limitRange) {
continue
}

err = limitFn(limitRange, a.GetResource().Resource, a.GetObject())
if err != nil {
return admission.NewForbidden(a, err)
}
}
return nil
Copy link
Contributor

Choose a reason for hiding this comment

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

They are really the same with the exception l.actions.ValidateLimit vs. l.actions.MutateLimit? Maybe make that explicit?

Copy link
Contributor

Choose a reason for hiding this comment

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

Much better now.

}

func (l *LimitRanger) GetLimitRanges(a admission.Attributes) ([]*api.LimitRange, error) {
items, err := l.lister.LimitRanges(a.GetNamespace()).List(labels.Everything())
if err != nil {
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()))
return nil, 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 there are no items held in our indexer, check our live-lookup LRU, if that misses, do the live lookup to prime it.
Expand All @@ -116,7 +151,7 @@ func (l *LimitRanger) Admit(a admission.Attributes) (err error) {
// throttling - see #22422 for details.
liveList, err := l.client.Core().LimitRanges(a.GetNamespace()).List(metav1.ListOptions{})
if err != nil {
return admission.NewForbidden(a, err)
return nil, admission.NewForbidden(a, err)
}
newEntry := liveLookupEntry{expiry: time.Now().Add(l.liveTTL)}
for i := range liveList.Items {
Expand All @@ -133,20 +168,7 @@ func (l *LimitRanger) Admit(a admission.Attributes) (err error) {

}

// ensure it meets each prescribed min/max
for i := range items {
limitRange := items[i]

if !l.actions.SupportsLimit(limitRange) {
continue
}

err = l.actions.Limit(limitRange, a.GetResource().Resource, a.GetObject())
if err != nil {
return admission.NewForbidden(a, err)
}
}
return nil
return items, nil
}

// NewLimitRanger returns an object that enforces limits based on the supplied limit function
Expand Down Expand Up @@ -399,12 +421,23 @@ var _ LimitRangerActions = &DefaultLimitRangerActions{}
// Limit enforces resource requirements of incoming resources against enumerated constraints
// on the LimitRange. It may modify the incoming object to apply default resource requirements
// if not specified, and enumerated on the LimitRange
func (d *DefaultLimitRangerActions) Limit(limitRange *api.LimitRange, resourceName string, obj runtime.Object) error {
func (d *DefaultLimitRangerActions) MutateLimit(limitRange *api.LimitRange, resourceName string, obj runtime.Object) error {
switch resourceName {
case "pods":
return PodLimitFunc(limitRange, obj.(*api.Pod))
return PodMutateLimitFunc(limitRange, obj.(*api.Pod))
}
return nil
}

// Limit enforces resource requirements of incoming resources against enumerated constraints
// on the LimitRange. It may modify the incoming object to apply default resource requirements
// if not specified, and enumerated on the LimitRange
func (d *DefaultLimitRangerActions) ValidateLimit(limitRange *api.LimitRange, resourceName string, obj runtime.Object) error {
switch resourceName {
case "pods":
return PodValidateLimitFunc(limitRange, obj.(*api.Pod))
case "persistentvolumeclaims":
return PersistentVolumeClaimLimitFunc(limitRange, obj.(*api.PersistentVolumeClaim))
return PersistentVolumeClaimValidateLimitFunc(limitRange, obj.(*api.PersistentVolumeClaim))
}
return nil
}
Expand All @@ -424,11 +457,11 @@ func (d *DefaultLimitRangerActions) SupportsLimit(limitRange *api.LimitRange) bo
return true
}

// PersistentVolumeClaimLimitFunc enforces storage limits for PVCs.
// PersistentVolumeClaimValidateLimitFunc enforces storage limits for PVCs.
Copy link
Contributor

Choose a reason for hiding this comment

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

validates storage limits ....

// Users request storage via pvc.Spec.Resources.Requests. Min/Max is enforced by an admin with LimitRange.
// Claims will not be modified with default values because storage is a required part of pvc.Spec.
// All storage enforced values *only* apply to pvc.Spec.Resources.Requests.
func PersistentVolumeClaimLimitFunc(limitRange *api.LimitRange, pvc *api.PersistentVolumeClaim) error {
func PersistentVolumeClaimValidateLimitFunc(limitRange *api.LimitRange, pvc *api.PersistentVolumeClaim) error {
var errs []error
for i := range limitRange.Spec.Limits {
limit := limitRange.Spec.Limits[i]
Expand All @@ -452,14 +485,19 @@ func PersistentVolumeClaimLimitFunc(limitRange *api.LimitRange, pvc *api.Persist
return utilerrors.NewAggregate(errs)
}

// PodLimitFunc enforces resource requirements enumerated by the pod against
// PodMutateLimitFunc sets resource requirements enumerated by the pod against
// the specified LimitRange. The pod may be modified to apply default resource
// requirements if not specified, and enumerated on the LimitRange
func PodLimitFunc(limitRange *api.LimitRange, pod *api.Pod) error {
var errs []error

func PodMutateLimitFunc(limitRange *api.LimitRange, pod *api.Pod) error {
defaultResources := defaultContainerResourceRequirements(limitRange)
mergePodResourceRequirements(pod, &defaultResources)
return nil
}

// PodValidateLimitFunc enforces resource requirements enumerated by the pod against
// the specified LimitRange.
func PodValidateLimitFunc(limitRange *api.LimitRange, pod *api.Pod) error {
var errs []error

for i := range limitRange.Spec.Limits {
limit := limitRange.Spec.Limits[i]
Expand Down
30 changes: 23 additions & 7 deletions plugin/pkg/admission/limitranger/admission_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -430,7 +430,11 @@ func TestPodLimitFunc(t *testing.T) {
}
for i := range successCases {
test := successCases[i]
err := PodLimitFunc(&test.limitRange, &test.pod)
err := PodMutateLimitFunc(&test.limitRange, &test.pod)
if err != nil {
t.Errorf("Unexpected error for pod: %s, %v", test.pod.Name, err)
}
err = PodValidateLimitFunc(&test.limitRange, &test.pod)
if err != nil {
t.Errorf("Unexpected error for pod: %s, %v", test.pod.Name, err)
}
Expand Down Expand Up @@ -610,7 +614,11 @@ func TestPodLimitFunc(t *testing.T) {
}
for i := range errorCases {
test := errorCases[i]
err := PodLimitFunc(&test.limitRange, &test.pod)
err := PodMutateLimitFunc(&test.limitRange, &test.pod)
if err != nil {
t.Errorf("Unexpected error for pod: %s, %v", test.pod.Name, err)
}
err = PodValidateLimitFunc(&test.limitRange, &test.pod)
if err == nil {
t.Errorf("Expected error for pod: %s", test.pod.Name)
}
Expand All @@ -628,7 +636,7 @@ func getLocalStorageResourceList(ephemeralStorage string) api.ResourceList {
func TestPodLimitFuncApplyDefault(t *testing.T) {
limitRange := validLimitRange()
testPod := validPodInit(validPod("foo", 1, getResourceRequirements(api.ResourceList{}, api.ResourceList{})), getResourceRequirements(api.ResourceList{}, api.ResourceList{}))
err := PodLimitFunc(&limitRange, &testPod)
err := PodMutateLimitFunc(&limitRange, &testPod)
if err != nil {
t.Errorf("Unexpected error for valid pod: %s, %v", testPod.Name, err)
}
Expand Down Expand Up @@ -687,11 +695,15 @@ func TestLimitRangerIgnoresSubresource(t *testing.T) {

testPod := validPod("testPod", 1, api.ResourceRequirements{})
err = handler.Admit(admission.NewAttributesRecord(&testPod, nil, api.Kind("Pod").WithVersion("version"), limitRange.Namespace, "testPod", api.Resource("pods").WithVersion("version"), "", admission.Update, nil))
if err != nil {
t.Fatal(err)
}
err = handler.Validate(admission.NewAttributesRecord(&testPod, nil, api.Kind("Pod").WithVersion("version"), limitRange.Namespace, "testPod", api.Resource("pods").WithVersion("version"), "", 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, nil, api.Kind("Pod").WithVersion("version"), limitRange.Namespace, "testPod", api.Resource("pods").WithVersion("version"), "status", admission.Update, nil))
err = handler.Validate(admission.NewAttributesRecord(&testPod, nil, api.Kind("Pod").WithVersion("version"), limitRange.Namespace, "testPod", api.Resource("pods").WithVersion("version"), "status", admission.Update, nil))
if err != nil {
t.Errorf("Should have ignored calls to any subresource of pod %v", err)
}
Expand All @@ -709,11 +721,15 @@ func TestLimitRangerAdmitPod(t *testing.T) {

testPod := validPod("testPod", 1, api.ResourceRequirements{})
err = handler.Admit(admission.NewAttributesRecord(&testPod, nil, api.Kind("Pod").WithVersion("version"), limitRange.Namespace, "testPod", api.Resource("pods").WithVersion("version"), "", admission.Update, nil))
if err != nil {
t.Fatal(err)
}
err = handler.Validate(admission.NewAttributesRecord(&testPod, nil, api.Kind("Pod").WithVersion("version"), limitRange.Namespace, "testPod", api.Resource("pods").WithVersion("version"), "", 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, nil, api.Kind("Pod").WithVersion("version"), limitRange.Namespace, "testPod", api.Resource("pods").WithVersion("version"), "status", admission.Update, nil))
err = handler.Validate(admission.NewAttributesRecord(&testPod, nil, api.Kind("Pod").WithVersion("version"), limitRange.Namespace, "testPod", api.Resource("pods").WithVersion("version"), "status", admission.Update, nil))
if err != nil {
t.Errorf("Should have ignored calls to any subresource of pod %v", err)
}
Expand Down Expand Up @@ -786,7 +802,7 @@ func TestPersistentVolumeClaimLimitFunc(t *testing.T) {
}
for i := range successCases {
test := successCases[i]
err := PersistentVolumeClaimLimitFunc(&test.limitRange, &test.pvc)
err := PersistentVolumeClaimValidateLimitFunc(&test.limitRange, &test.pvc)
if err != nil {
t.Errorf("Unexpected error for pvc: %s, %v", test.pvc.Name, err)
}
Expand All @@ -804,7 +820,7 @@ func TestPersistentVolumeClaimLimitFunc(t *testing.T) {
}
for i := range errorCases {
test := errorCases[i]
err := PersistentVolumeClaimLimitFunc(&test.limitRange, &test.pvc)
err := PersistentVolumeClaimValidateLimitFunc(&test.limitRange, &test.pvc)
if err == nil {
t.Errorf("Expected error for pvc: %s", test.pvc.Name)
}
Expand Down
6 changes: 4 additions & 2 deletions plugin/pkg/admission/limitranger/interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,10 @@ import (
)

type LimitRangerActions interface {
// Limit is a pluggable function to enforce limits on the object.
Limit(limitRange *api.LimitRange, kind string, obj runtime.Object) error
// MutateLimit is a pluggable function to set limits on the object.
MutateLimit(limitRange *api.LimitRange, kind string, obj runtime.Object) error
// ValidateLimits is a pluggable function to enforce limits on the object.
Copy link
Contributor

Choose a reason for hiding this comment

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

ic, enforce = validate

ValidateLimit(limitRange *api.LimitRange, kind string, obj runtime.Object) error
// SupportsAttributes is a pluggable function to allow overridding what resources the limitranger
// supports.
SupportsAttributes(attr admission.Attributes) bool
Expand Down
1 change: 1 addition & 0 deletions plugin/pkg/admission/namespace/autoprovision/admission.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ type Provision struct {
namespaceLister corelisters.NamespaceLister
}

var _ admission.MutationInterface = &Provision{}
var _ = kubeapiserveradmission.WantsInternalKubeInformerFactory(&Provision{})
var _ = kubeapiserveradmission.WantsInternalKubeClientSet(&Provision{})

Expand Down
1 change: 1 addition & 0 deletions plugin/pkg/admission/persistentvolume/label/admission.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ type persistentVolumeLabel struct {
gceCloudProvider *gce.GCECloud
}

var _ admission.MutationInterface = &persistentVolumeLabel{}
var _ kubeapiserveradmission.WantsCloudConfig = &persistentVolumeLabel{}

// NewPersistentVolumeLabel returns an admission.Interface implementation which adds labels to PersistentVolume CREATE requests,
Expand Down
1 change: 1 addition & 0 deletions plugin/pkg/admission/podpreset/admission.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ type podPresetPlugin struct {
lister settingslisters.PodPresetLister
}

var _ admission.MutationInterface = &podPresetPlugin{}
var _ = kubeapiserveradmission.WantsInternalKubeInformerFactory(&podPresetPlugin{})
var _ = kubeapiserveradmission.WantsInternalKubeClientSet(&podPresetPlugin{})

Expand Down
2 changes: 2 additions & 0 deletions plugin/pkg/admission/podtolerationrestriction/admission.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ const (
NSWLTolerations string = "scheduler.alpha.kubernetes.io/tolerationsWhitelist"
)

var _ admission.MutationInterface = &podTolerationsPlugin{}
var _ admission.ValidationInterface = &podTolerationsPlugin{}
var _ = kubeapiserveradmission.WantsInternalKubeInformerFactory(&podTolerationsPlugin{})

type podTolerationsPlugin struct {
Expand Down
1 change: 1 addition & 0 deletions plugin/pkg/admission/storageclass/setdefault/admission.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ type claimDefaulterPlugin struct {
}

var _ admission.Interface = &claimDefaulterPlugin{}
var _ admission.MutationInterface = &claimDefaulterPlugin{}
var _ = kubeapiserveradmission.WantsInternalKubeInformerFactory(&claimDefaulterPlugin{})

// newPlugin creates a new admission plugin.
Expand Down