Skip to content

Commit e260205

Browse files
ryotaraiclaude
andcommitted
Replace HierarchicalResourceQuota webhook with WebhookManagedBy.
- Convert HRQ to use ValidateCreate/Update/Delete methods instead of Handle - Add NewHRQ constructor with client parameter for dry-run client setup - Update validate method to return errors instead of admission responses - Convert validation to use apierrors.NewInvalid with field validation - Update tests to use validate method directly with error checking - Remove old injection methods (InjectClient, InjectDecoder) - Add missing imports for field validation and runtime types 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
1 parent 2e4277f commit e260205

File tree

2 files changed

+66
-32
lines changed

2 files changed

+66
-32
lines changed

internal/hrq/hrqvalidator.go

Lines changed: 59 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,10 @@ import (
88

99
"github.com/go-logr/logr"
1010
v1 "k8s.io/api/core/v1"
11-
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
11+
apierrors "k8s.io/apimachinery/pkg/api/errors"
12+
"k8s.io/apimachinery/pkg/runtime"
13+
"k8s.io/apimachinery/pkg/util/validation/field"
14+
ctrl "sigs.k8s.io/controller-runtime"
1215
"sigs.k8s.io/controller-runtime/pkg/client"
1316
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
1417

@@ -35,9 +38,15 @@ const (
3538
// +<removewhenready>kubebuilder:webhook:admissionReviewVersions=v1;v1beta1,path=/validate-hnc-x-k8s-io-v1alpha2-hrq,mutating=false,failurePolicy=fail,groups="hnc.x-k8s.io",resources=hierarchicalresourcequotas,sideEffects=None,verbs=create;update,versions=v1alpha2,name=hrq.hnc.x-k8s.io
3639

3740
type HRQ struct {
38-
server serverClient
39-
Log logr.Logger
40-
decoder admission.Decoder
41+
server serverClient
42+
Log logr.Logger
43+
}
44+
45+
func NewHRQ(c client.Client) *HRQ {
46+
return &HRQ{
47+
server: &realClient{client: client.NewDryRunClient(c)},
48+
Log: ctrl.Log.WithName("validators").WithName("HierarchicalResourceQuota"),
49+
}
4150
}
4251

4352
// serverClient represents the checks that should typically be performed against
@@ -47,39 +56,71 @@ type serverClient interface {
4756
validate(ctx context.Context, inst *v1.ResourceQuota) error
4857
}
4958

50-
func (v *HRQ) Handle(ctx context.Context, req admission.Request) admission.Response {
59+
func (v *HRQ) ValidateCreate(ctx context.Context, obj runtime.Object) (admission.Warnings, error) {
60+
req, err := admission.RequestFromContext(ctx)
61+
if err != nil {
62+
return nil, apierrors.NewInternalError(err)
63+
}
64+
5165
log := logutils.WithRID(v.Log).WithValues("nm", req.Name, "nnm", req.Namespace)
5266

53-
inst := &api.HierarchicalResourceQuota{}
54-
if err := v.decoder.Decode(req, inst); err != nil {
55-
log.Error(err, "Couldn't decode request")
56-
return deny(metav1.StatusReasonBadRequest, err.Error())
67+
inst, ok := obj.(*api.HierarchicalResourceQuota)
68+
if !ok {
69+
return nil, apierrors.NewInternalError(fmt.Errorf("expected a HierarchicalResourceQuota but got a %T", obj))
5770
}
5871

59-
resp := v.handle(ctx, log, inst)
60-
if resp.Allowed {
61-
log.V(1).Info("Allowed", "message", resp.Result.Message)
62-
} else {
63-
log.Info("Denied", "code", resp.Result.Code, "reason", resp.Result.Reason, "message", resp.Result.Message)
72+
if err := v.validate(ctx, log, inst); err != nil {
73+
log.Info("Denied", "reason", err)
74+
return nil, err
6475
}
65-
return resp
76+
77+
log.V(1).Info("Allowed")
78+
return nil, nil
79+
}
80+
81+
func (v *HRQ) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) {
82+
req, err := admission.RequestFromContext(ctx)
83+
if err != nil {
84+
return nil, apierrors.NewInternalError(err)
85+
}
86+
87+
log := logutils.WithRID(v.Log).WithValues("nm", req.Name, "nnm", req.Namespace)
88+
89+
inst, ok := newObj.(*api.HierarchicalResourceQuota)
90+
if !ok {
91+
return nil, apierrors.NewInternalError(fmt.Errorf("expected a HierarchicalResourceQuota but got a %T", newObj))
92+
}
93+
94+
if err := v.validate(ctx, log, inst); err != nil {
95+
log.Info("Denied", "reason", err)
96+
return nil, err
97+
}
98+
99+
log.V(1).Info("Allowed")
100+
return nil, nil
101+
}
102+
103+
func (v *HRQ) ValidateDelete(ctx context.Context, obj runtime.Object) (admission.Warnings, error) {
104+
// HierarchicalResourceQuota deletion doesn't need validation for our purposes
105+
return nil, nil
66106
}
67107

68108
// Validate if the resources in the spec are valid resource types for quota. We
69109
// will dry-run writing the HRQ resources to an RQ to see if we get an error
70110
// from the apiserver or not. If yes, we will deny the HRQ request.
71-
func (v *HRQ) handle(ctx context.Context, log logr.Logger, inst *api.HierarchicalResourceQuota) admission.Response {
111+
func (v *HRQ) validate(ctx context.Context, log logr.Logger, inst *api.HierarchicalResourceQuota) error {
72112
rq := &v1.ResourceQuota{}
73113
rq.Namespace = inst.Namespace
74114
rq.Name = createRQName()
75115
rq.Spec.Hard = inst.Spec.Hard
76116
rq.Spec.ScopeSelector = inst.Spec.ScopeSelector
77117
log.V(1).Info("Validating resource types in the HRQ spec by writing them to a resource quota on apiserver", "limits", inst.Spec.Hard)
78118
if err := v.server.validate(ctx, rq); err != nil {
79-
return denyInvalidField(fieldInfo, ignoreRQErr(err.Error()))
119+
return apierrors.NewInvalid(api.GroupVersion.WithKind("HierarchicalResourceQuota").GroupKind(), inst.Name,
120+
field.ErrorList{field.Invalid(field.NewPath("spec").Child("hard"), inst.Spec.Hard, ignoreRQErr(err.Error()))})
80121
}
81122

82-
return allow("")
123+
return nil
83124
}
84125

85126
func createRQName() string {
@@ -116,14 +157,3 @@ func (c *realClient) validate(ctx context.Context, inst *v1.ResourceQuota) error
116157
}
117158
return nil
118159
}
119-
120-
func (v *HRQ) InjectClient(c client.Client) error {
121-
// Create a dry-run client.
122-
v.server = &realClient{client: client.NewDryRunClient(c)}
123-
return nil
124-
}
125-
126-
func (r *HRQ) InjectDecoder(d admission.Decoder) error {
127-
r.decoder = d
128-
return nil
129-
}

internal/hrq/hrqvalidator_test.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package hrq
33
import (
44
"context"
55
"errors"
6+
"strings"
67
"testing"
78

89
v1 "k8s.io/api/core/v1"
@@ -42,9 +43,12 @@ func TestHRQSpec(t *testing.T) {
4243
hrq := &api.HierarchicalResourceQuota{}
4344
hrq.Spec.Hard = argsToResourceList(tc.hrqLimit...)
4445
v := HRQ{server: fakeServer("")}
45-
got := v.handle(context.Background(), l, hrq)
46-
if got.AdmissionResponse.Allowed == tc.fail || got.Result.Message != tc.msg {
47-
t.Errorf("unexpected admission response. Expected: %t, %s; Got: %t, %s", !tc.fail, tc.msg, got.AdmissionResponse.Allowed, got.Result.Message)
46+
err := v.validate(context.Background(), l, hrq)
47+
if (err != nil) != tc.fail {
48+
t.Errorf("unexpected validation result: got error=%v, expected failure=%v", err, tc.fail)
49+
}
50+
if err != nil && tc.msg != "" && !strings.Contains(err.Error(), tc.msg) {
51+
t.Errorf("unexpected error message. Expected to contain: %s; Got: %s", tc.msg, err.Error())
4852
}
4953
})
5054
}

0 commit comments

Comments
 (0)