Skip to content

Commit

Permalink
Replace webhooks with CEL validation rules for LocalQueue (kubernetes…
Browse files Browse the repository at this point in the history
…-sigs#1938)

* Replace webhooks with CEL for LocalQueue

* Rename error matchers file

* Add generic error matcher

* Rename api error functions

* Reimplement existing error matchers with the generic one
  • Loading branch information
IrvingMg authored and vsoch committed Apr 18, 2024
1 parent 92a177e commit b172595
Show file tree
Hide file tree
Showing 13 changed files with 101 additions and 282 deletions.
3 changes: 3 additions & 0 deletions apis/kueue/v1beta1/localqueue_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,13 @@ import (
// LocalQueueSpec defines the desired state of LocalQueue
type LocalQueueSpec struct {
// clusterQueue is a reference to a clusterQueue that backs this localQueue.
// +kubebuilder:validation:XValidation:rule="self == oldSelf", message="field is immutable"
ClusterQueue ClusterQueueReference `json:"clusterQueue,omitempty"`
}

// ClusterQueueReference is the name of the ClusterQueue.
// +kubebuilder:validation:MaxLength=253
// +kubebuilder:validation:Pattern="^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$"
type ClusterQueueReference string

// LocalQueueStatus defines the observed state of LocalQueue
Expand Down
5 changes: 5 additions & 0 deletions charts/kueue/templates/crd/kueue.x-k8s.io_localqueues.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,12 @@ spec:
clusterQueue:
description: clusterQueue is a reference to a clusterQueue that backs
this localQueue.
maxLength: 253
pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$
type: string
x-kubernetes-validations:
- message: field is immutable
rule: self == oldSelf
type: object
status:
description: LocalQueueStatus defines the observed state of LocalQueue
Expand Down
2 changes: 2 additions & 0 deletions charts/kueue/templates/crd/kueue.x-k8s.io_workloads.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -7822,6 +7822,8 @@ spec:
clusterQueue:
description: clusterQueue is the name of the ClusterQueue that
admitted this workload.
maxLength: 253
pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$
type: string
podSetAssignments:
description: PodSetAssignments hold the admission results for
Expand Down
20 changes: 0 additions & 20 deletions charts/kueue/templates/webhook/webhook.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -580,26 +580,6 @@ webhooks:
resources:
- clusterqueues
sideEffects: None
- admissionReviewVersions:
- v1
clientConfig:
service:
name: '{{ include "kueue.fullname" . }}-webhook-service'
namespace: '{{ .Release.Namespace }}'
path: /validate-kueue-x-k8s-io-v1beta1-localqueue
failurePolicy: Fail
name: vlocalqueue.kb.io
rules:
- apiGroups:
- kueue.x-k8s.io
apiVersions:
- v1beta1
operations:
- CREATE
- UPDATE
resources:
- localqueues
sideEffects: None
- admissionReviewVersions:
- v1
clientConfig:
Expand Down
5 changes: 5 additions & 0 deletions config/components/crd/bases/kueue.x-k8s.io_localqueues.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,12 @@ spec:
clusterQueue:
description: clusterQueue is a reference to a clusterQueue that backs
this localQueue.
maxLength: 253
pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$
type: string
x-kubernetes-validations:
- message: field is immutable
rule: self == oldSelf
type: object
status:
description: LocalQueueStatus defines the observed state of LocalQueue
Expand Down
2 changes: 2 additions & 0 deletions config/components/crd/bases/kueue.x-k8s.io_workloads.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -7807,6 +7807,8 @@ spec:
clusterQueue:
description: clusterQueue is the name of the ClusterQueue that
admitted this workload.
maxLength: 253
pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$
type: string
podSetAssignments:
description: PodSetAssignments hold the admission results for
Expand Down
20 changes: 0 additions & 20 deletions config/components/webhook/manifests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -536,26 +536,6 @@ webhooks:
resources:
- clusterqueues
sideEffects: None
- admissionReviewVersions:
- v1
clientConfig:
service:
name: webhook-service
namespace: system
path: /validate-kueue-x-k8s-io-v1beta1-localqueue
failurePolicy: Fail
name: vlocalqueue.kb.io
rules:
- apiGroups:
- kueue.x-k8s.io
apiVersions:
- v1beta1
operations:
- CREATE
- UPDATE
resources:
- localqueues
sideEffects: None
- admissionReviewVersions:
- v1
clientConfig:
Expand Down
78 changes: 78 additions & 0 deletions pkg/util/testing/error_matchers.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
/*
Copyright 2022 The Kubernetes Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package testing

import (
"fmt"

"github.com/onsi/gomega/format"
"github.com/onsi/gomega/types"
apierrors "k8s.io/apimachinery/pkg/api/errors"
)

func BeNotFoundError() types.GomegaMatcher {
return BeAPIError(NotFoundError)
}

func BeForbiddenError() types.GomegaMatcher {
return BeAPIError(ForbiddenError)
}

type errorMatcher int

const (
NotFoundError errorMatcher = iota
ForbiddenError
InvalidError
)

func (em errorMatcher) String() string {
return []string{"NotFoundError", "ForbiddenError", "InvalidError"}[em]
}

type apiError func(error) bool

func (em errorMatcher) isAPIError(err error) bool {
return []apiError{apierrors.IsNotFound, apierrors.IsForbidden, apierrors.IsInvalid}[em](err)
}

type isErrorMatch struct {
name errorMatcher
}

func BeAPIError(name errorMatcher) types.GomegaMatcher {
return &isErrorMatch{
name: name,
}
}

func (matcher *isErrorMatch) Match(actual interface{}) (success bool, err error) {
err, ok := actual.(error)
if !ok {
return false, fmt.Errorf("%s expects an error", matcher.name.String())
}

return err != nil && matcher.name.isAPIError(err), nil
}

func (matcher *isErrorMatch) FailureMessage(actual interface{}) (message string) {
return format.Message(actual, "to be a %s", matcher.name.String())
}

func (matcher *isErrorMatch) NegatedFailureMessage(actual interface{}) (message string) {
return format.Message(actual, "not to be %s", matcher.name.String())
}
77 changes: 0 additions & 77 deletions pkg/util/testing/not_found_error_cmp.go

This file was deleted.

77 changes: 0 additions & 77 deletions pkg/webhooks/localqueue_webhook.go

This file was deleted.

0 comments on commit b172595

Please sign in to comment.