Skip to content

Commit 5fffead

Browse files
ary1992ary1992
committed
Adapt webhook/admission custom Validator and CustomValidator interfaces to return warnings
kubernetes-sigs/controller-runtime#2014
1 parent a166665 commit 5fffead

File tree

7 files changed

+197
-174
lines changed

7 files changed

+197
-174
lines changed

‎pkg/admissioncontroller/webhook/admission/internaldomainsecret/handler.go

+25-24
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import (
2525
"k8s.io/apimachinery/pkg/runtime"
2626
"k8s.io/apimachinery/pkg/runtime/schema"
2727
"sigs.k8s.io/controller-runtime/pkg/client"
28+
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
2829

2930
gardencore "github.com/gardener/gardener/pkg/apis/core"
3031
gardencorev1beta1 "github.com/gardener/gardener/pkg/apis/core/v1beta1"
@@ -40,53 +41,53 @@ type Handler struct {
4041
}
4142

4243
// ValidateCreate performs the check.
43-
func (h *Handler) ValidateCreate(ctx context.Context, obj runtime.Object) error {
44+
func (h *Handler) ValidateCreate(ctx context.Context, obj runtime.Object) (admission.Warnings, error) {
4445
ctx, cancel := context.WithTimeout(ctx, 10*time.Second)
4546
defer cancel()
4647

4748
secret, ok := obj.(*corev1.Secret)
4849
if !ok {
49-
return apierrors.NewBadRequest(fmt.Sprintf("expected *corev1.Secret but got %T", obj))
50+
return nil, apierrors.NewBadRequest(fmt.Sprintf("expected *corev1.Secret but got %T", obj))
5051
}
5152

5253
seedName := gardenerutils.ComputeSeedName(secret.Namespace)
5354
if secret.Namespace != v1beta1constants.GardenNamespace && seedName == "" {
54-
return nil
55+
return nil, nil
5556
}
5657

5758
exists, err := h.internalDomainSecretExists(ctx, secret.Namespace)
5859
if err != nil {
59-
return apierrors.NewInternalError(err)
60+
return nil, apierrors.NewInternalError(err)
6061
}
6162
if exists {
62-
return apierrors.NewConflict(schema.GroupResource{Group: corev1.GroupName, Resource: "Secret"}, secret.Name, fmt.Errorf("cannot create internal domain secret because there can be only one secret with the 'internal-domain' secret role per namespace"))
63+
return nil, apierrors.NewConflict(schema.GroupResource{Group: corev1.GroupName, Resource: "Secret"}, secret.Name, fmt.Errorf("cannot create internal domain secret because there can be only one secret with the 'internal-domain' secret role per namespace"))
6364
}
6465

6566
if _, _, _, _, _, err := gardenerutils.GetDomainInfoFromAnnotations(secret.Annotations); err != nil {
66-
return apierrors.NewBadRequest(err.Error())
67+
return nil, apierrors.NewBadRequest(err.Error())
6768
}
6869

69-
return nil
70+
return nil, nil
7071
}
7172

7273
// ValidateUpdate performs the check.
73-
func (h *Handler) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) error {
74+
func (h *Handler) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) {
7475
ctx, cancel := context.WithTimeout(ctx, 10*time.Second)
7576
defer cancel()
7677

7778
secret, ok := newObj.(*corev1.Secret)
7879
if !ok {
79-
return apierrors.NewBadRequest(fmt.Sprintf("expected *corev1.Secret but got %T", newObj))
80+
return nil, apierrors.NewBadRequest(fmt.Sprintf("expected *corev1.Secret but got %T", newObj))
8081
}
8182

8283
oldSecret, ok := oldObj.(*corev1.Secret)
8384
if !ok {
84-
return apierrors.NewBadRequest(fmt.Sprintf("expected *corev1.Secret but got %T", oldObj))
85+
return nil, apierrors.NewBadRequest(fmt.Sprintf("expected *corev1.Secret but got %T", oldObj))
8586
}
8687

8788
seedName := gardenerutils.ComputeSeedName(secret.Namespace)
8889
if secret.Namespace != v1beta1constants.GardenNamespace && seedName == "" {
89-
return nil
90+
return nil, nil
9091
}
9192

9293
// If secret was newly labeled with gardener.cloud/role=internal-domain then check whether another internal domain
@@ -95,59 +96,59 @@ func (h *Handler) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Obj
9596
secret.Labels[v1beta1constants.GardenRole] == v1beta1constants.GardenRoleInternalDomain {
9697
exists, err := h.internalDomainSecretExists(ctx, secret.Namespace)
9798
if err != nil {
98-
return apierrors.NewInternalError(err)
99+
return nil, apierrors.NewInternalError(err)
99100
}
100101
if exists {
101-
return apierrors.NewConflict(schema.GroupResource{Group: corev1.GroupName, Resource: "Secret"}, secret.Name, fmt.Errorf("cannot update secret because there can be only one secret with the 'internal-domain' secret role per namespace"))
102+
return nil, apierrors.NewConflict(schema.GroupResource{Group: corev1.GroupName, Resource: "Secret"}, secret.Name, fmt.Errorf("cannot update secret because there can be only one secret with the 'internal-domain' secret role per namespace"))
102103
}
103104
}
104105

105106
_, oldDomain, _, _, _, err := gardenerutils.GetDomainInfoFromAnnotations(oldSecret.Annotations)
106107
if err != nil {
107-
return apierrors.NewInternalError(err)
108+
return nil, apierrors.NewInternalError(err)
108109
}
109110
_, newDomain, _, _, _, err := gardenerutils.GetDomainInfoFromAnnotations(secret.Annotations)
110111
if err != nil {
111-
return apierrors.NewInternalError(err)
112+
return nil, apierrors.NewInternalError(err)
112113
}
113114

114115
if oldDomain != newDomain {
115116
atLeastOneShoot, err := h.atLeastOneShootExists(ctx, seedName)
116117
if err != nil {
117-
return apierrors.NewInternalError(err)
118+
return nil, apierrors.NewInternalError(err)
118119
}
119120
if atLeastOneShoot {
120-
return apierrors.NewForbidden(schema.GroupResource{Group: corev1.GroupName, Resource: "Secret"}, secret.Name, fmt.Errorf("cannot change domain because there are still shoots left in the system"))
121+
return nil, apierrors.NewForbidden(schema.GroupResource{Group: corev1.GroupName, Resource: "Secret"}, secret.Name, fmt.Errorf("cannot change domain because there are still shoots left in the system"))
121122
}
122123
}
123124

124-
return nil
125+
return nil, nil
125126
}
126127

127128
// ValidateDelete performs the check.
128-
func (h *Handler) ValidateDelete(ctx context.Context, obj runtime.Object) error {
129+
func (h *Handler) ValidateDelete(ctx context.Context, obj runtime.Object) (admission.Warnings, error) {
129130
ctx, cancel := context.WithTimeout(ctx, 10*time.Second)
130131
defer cancel()
131132

132133
secret, ok := obj.(*corev1.Secret)
133134
if !ok {
134-
return apierrors.NewBadRequest(fmt.Sprintf("expected *corev1.Secret but got %T", obj))
135+
return nil, apierrors.NewBadRequest(fmt.Sprintf("expected *corev1.Secret but got %T", obj))
135136
}
136137

137138
seedName := gardenerutils.ComputeSeedName(secret.Namespace)
138139
if secret.Namespace != v1beta1constants.GardenNamespace && seedName == "" {
139-
return nil
140+
return nil, nil
140141
}
141142

142143
atLeastOneShoot, err := h.atLeastOneShootExists(ctx, seedName)
143144
if err != nil {
144-
return apierrors.NewInternalError(err)
145+
return nil, apierrors.NewInternalError(err)
145146
}
146147
if atLeastOneShoot {
147-
return apierrors.NewForbidden(schema.GroupResource{Group: corev1.GroupName, Resource: "Secret"}, secret.Name, fmt.Errorf("cannot delete internal domain secret because there are still shoots left in the system"))
148+
return nil, apierrors.NewForbidden(schema.GroupResource{Group: corev1.GroupName, Resource: "Secret"}, secret.Name, fmt.Errorf("cannot delete internal domain secret because there are still shoots left in the system"))
148149
}
149150

150-
return nil
151+
return nil, nil
151152
}
152153

153154
func (h *Handler) atLeastOneShootExists(ctx context.Context, seedName string) (bool, error) {

‎pkg/admissioncontroller/webhook/admission/internaldomainsecret/handler_test.go

+15-5
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,9 @@ var _ = Describe("handler", func() {
111111
client.Limit(1),
112112
).Return(fakeErr)
113113

114-
err := handler.ValidateCreate(ctx, secret)
114+
warnings, err := handler.ValidateCreate(ctx, secret)
115+
Expect(warnings).To(BeNil())
116+
115117
statusError, ok := err.(*apierrors.StatusError)
116118
Expect(ok).To(BeTrue())
117119
Expect(statusError.Status().Code).To(Equal(int32(http.StatusInternalServerError)))
@@ -130,7 +132,9 @@ var _ = Describe("handler", func() {
130132
return nil
131133
})
132134

133-
Expect(handler.ValidateCreate(ctx, secret)).To(MatchError(ContainSubstring("there can be only one secret with the 'internal-domain' secret role")))
135+
warnings, err := handler.ValidateCreate(ctx, secret)
136+
Expect(warnings).To(BeNil())
137+
Expect(err).To(MatchError(ContainSubstring("there can be only one secret with the 'internal-domain' secret role")))
134138
})
135139

136140
It("should fail because another internal domain secret exists in the same seed namespace", func() {
@@ -193,7 +197,9 @@ var _ = Describe("handler", func() {
193197
client.Limit(1),
194198
).Return(fakeErr)
195199

196-
err := handler.ValidateUpdate(ctx, oldSecret, secret)
200+
warnings, err := handler.ValidateUpdate(ctx, oldSecret, secret)
201+
Expect(warnings).To(BeNil())
202+
197203
statusError, ok := err.(*apierrors.StatusError)
198204
Expect(ok).To(BeTrue())
199205
Expect(statusError.Status().Code).To(Equal(int32(http.StatusInternalServerError)))
@@ -238,7 +244,9 @@ var _ = Describe("handler", func() {
238244
oldSecret := secret.DeepCopy()
239245
secret.Annotations["dns.gardener.cloud/domain"] = "foobar"
240246

241-
err := handler.ValidateUpdate(ctx, oldSecret, secret)
247+
warnings, err := handler.ValidateUpdate(ctx, oldSecret, secret)
248+
Expect(warnings).To(BeNil())
249+
242250
statusError, ok := err.(*apierrors.StatusError)
243251
Expect(ok).To(BeTrue())
244252
Expect(statusError.Status().Code).To(Equal(int32(http.StatusInternalServerError)))
@@ -304,7 +312,9 @@ var _ = Describe("handler", func() {
304312
client.Limit(1),
305313
).Return(fakeErr)
306314

307-
err := handler.ValidateDelete(ctx, secret)
315+
warnings, err := handler.ValidateDelete(ctx, secret)
316+
Expect(warnings).To(BeNil())
317+
308318
statusError, ok := err.(*apierrors.StatusError)
309319
Expect(ok).To(BeTrue())
310320
Expect(statusError.Status().Code).To(Equal(int32(http.StatusInternalServerError)))

‎pkg/admissioncontroller/webhook/admission/kubeconfigsecret/handler.go

+12-12
Original file line numberDiff line numberDiff line change
@@ -41,46 +41,46 @@ type Handler struct {
4141
}
4242

4343
// ValidateCreate performs the check.
44-
func (h *Handler) ValidateCreate(ctx context.Context, obj runtime.Object) error {
44+
func (h *Handler) ValidateCreate(ctx context.Context, obj runtime.Object) (admission.Warnings, error) {
4545
return h.handle(ctx, obj)
4646
}
4747

4848
// ValidateUpdate performs the check.
49-
func (h *Handler) ValidateUpdate(ctx context.Context, _, newObj runtime.Object) error {
49+
func (h *Handler) ValidateUpdate(ctx context.Context, _, newObj runtime.Object) (admission.Warnings, error) {
5050
return h.handle(ctx, newObj)
5151
}
5252

5353
// ValidateDelete returns nil (not implemented by this handler).
54-
func (h *Handler) ValidateDelete(_ context.Context, _ runtime.Object) error {
55-
return nil
54+
func (h *Handler) ValidateDelete(_ context.Context, _ runtime.Object) (admission.Warnings, error) {
55+
return nil, nil
5656
}
5757

58-
func (h *Handler) handle(ctx context.Context, obj runtime.Object) error {
58+
func (h *Handler) handle(ctx context.Context, obj runtime.Object) (admission.Warnings, error) {
5959
secret, ok := obj.(*corev1.Secret)
6060
if !ok {
61-
return apierrors.NewBadRequest(fmt.Sprintf("expected *corev1.Secret but got %T", obj))
61+
return nil, apierrors.NewBadRequest(fmt.Sprintf("expected *corev1.Secret but got %T", obj))
6262
}
6363

6464
req, err := admission.RequestFromContext(ctx)
6565
if err != nil {
66-
return apierrors.NewInternalError(err)
66+
return nil, apierrors.NewInternalError(err)
6767
}
6868

6969
kubeconfig, ok := secret.Data[kubernetes.KubeConfig]
7070
if !ok {
71-
return nil
71+
return nil, nil
7272
}
7373

7474
clientConfig, err := clientcmd.NewClientConfigFromBytes(kubeconfig)
7575
if err != nil {
76-
return apierrors.NewBadRequest(err.Error())
76+
return nil, apierrors.NewBadRequest(err.Error())
7777
}
7878

7979
// Validate that the given kubeconfig doesn't have fields in its auth-info that are
8080
// not acceptable.
8181
rawConfig, err := clientConfig.RawConfig()
8282
if err != nil {
83-
return apierrors.NewBadRequest(err.Error())
83+
return nil, apierrors.NewBadRequest(err.Error())
8484
}
8585

8686
if err := kubernetes.ValidateConfig(rawConfig); err != nil {
@@ -98,8 +98,8 @@ func (h *Handler) handle(ctx context.Context, obj runtime.Object) error {
9898
metricReasonRejectedKubeconfig,
9999
).Inc()
100100

101-
return apierrors.NewInvalid(schema.GroupKind{Group: corev1.GroupName, Kind: "Secret"}, secret.Name, field.ErrorList{field.Invalid(field.NewPath("data", "kubeconfig"), kubeconfig, fmt.Sprintf("secret contains invalid kubeconfig: %s", err))})
101+
return nil, apierrors.NewInvalid(schema.GroupKind{Group: corev1.GroupName, Kind: "Secret"}, secret.Name, field.ErrorList{field.Invalid(field.NewPath("data", "kubeconfig"), kubeconfig, fmt.Sprintf("secret contains invalid kubeconfig: %s", err))})
102102
}
103103

104-
return nil
104+
return nil, nil
105105
}

‎pkg/admissioncontroller/webhook/admission/namespacedeletion/handler.go

+16-16
Original file line numberDiff line numberDiff line change
@@ -40,70 +40,70 @@ type Handler struct {
4040
}
4141

4242
// ValidateCreate returns nil (not implemented by this handler).
43-
func (h *Handler) ValidateCreate(_ context.Context, _ runtime.Object) error {
44-
return nil
43+
func (h *Handler) ValidateCreate(_ context.Context, _ runtime.Object) (admission.Warnings, error) {
44+
return nil, nil
4545
}
4646

4747
// ValidateUpdate returns nil (not implemented by this handler).
48-
func (h *Handler) ValidateUpdate(_ context.Context, _, _ runtime.Object) error {
49-
return nil
48+
func (h *Handler) ValidateUpdate(_ context.Context, _, _ runtime.Object) (admission.Warnings, error) {
49+
return nil, nil
5050
}
5151

5252
// ValidateDelete validates the namespace deletion.
53-
func (h *Handler) ValidateDelete(ctx context.Context, _ runtime.Object) error {
53+
func (h *Handler) ValidateDelete(ctx context.Context, _ runtime.Object) (admission.Warnings, error) {
5454
ctx, cancel := context.WithTimeout(ctx, 10*time.Second)
5555
defer cancel()
5656

5757
req, err := admission.RequestFromContext(ctx)
5858
if err != nil {
59-
return apierrors.NewInternalError(err)
59+
return nil, apierrors.NewInternalError(err)
6060
}
6161

6262
if err := h.admitNamespace(ctx, req.Name); err != nil {
6363
h.Logger.Info("Rejected namespace deletion", "user", req.UserInfo.Username, "reason", err.Error())
6464
return err
6565
}
6666

67-
return nil
67+
return nil, nil
6868
}
6969

7070
// admitNamespace does only allow the request if no Shoots exist in this specific namespace anymore.
71-
func (h *Handler) admitNamespace(ctx context.Context, namespaceName string) error {
71+
func (h *Handler) admitNamespace(ctx context.Context, namespaceName string) (admission.Warnings, error) {
7272
// Determine project for given namespace.
7373
// TODO: we should use a direct lookup here, as we might falsely allow the request, if our cache is
7474
// out of sync and doesn't know about the project. We should use a field selector for looking up the project
7575
// belonging to a given namespace.
7676
project, namespace, err := gardenerutils.ProjectAndNamespaceFromReader(ctx, h.Client, namespaceName)
7777
if err != nil {
7878
if apierrors.IsNotFound(err) {
79-
return nil
79+
return nil, nil
8080
}
81-
return apierrors.NewInternalError(err)
81+
return nil, apierrors.NewInternalError(err)
8282
}
8383

8484
if project == nil {
85-
return nil
85+
return nil, nil
8686
}
8787

8888
switch {
8989
case namespace.DeletionTimestamp != nil:
90-
return nil
90+
return nil, nil
9191

9292
case project.DeletionTimestamp != nil:
9393
// if project is marked for deletion we need to wait until all shoots in the namespace are gone
9494
namespaceInUse, err := kubernetesutils.ResourcesExist(ctx, h.APIReader, gardencorev1beta1.SchemeGroupVersion.WithKind("ShootList"), client.InNamespace(namespace.Name))
9595
if err != nil {
96-
return apierrors.NewInternalError(err)
96+
return nil, apierrors.NewInternalError(err)
9797
}
9898

9999
if !namespaceInUse {
100-
return nil
100+
return nil, nil
101101
}
102102

103-
return apierrors.NewForbidden(schema.GroupResource{Group: corev1.GroupName, Resource: "Namespace"}, namespace.Name, fmt.Errorf("deletion of namespace %q is not permitted (it still contains Shoots)", namespace.Name))
103+
return nil, apierrors.NewForbidden(schema.GroupResource{Group: corev1.GroupName, Resource: "Namespace"}, namespace.Name, fmt.Errorf("deletion of namespace %q is not permitted (it still contains Shoots)", namespace.Name))
104104
}
105105

106106
// Namespace is not yet marked for deletion and project is not marked as well. We do not admit and respond that
107107
// namespace deletion is only allowed via project deletion.
108-
return apierrors.NewForbidden(schema.GroupResource{Group: corev1.GroupName, Resource: "Namespace"}, namespace.Name, fmt.Errorf("direct deletion of namespace %q is not permitted (you must delete the corresponding project %q)", namespace.Name, project.Name))
108+
return nil, apierrors.NewForbidden(schema.GroupResource{Group: corev1.GroupName, Resource: "Namespace"}, namespace.Name, fmt.Errorf("direct deletion of namespace %q is not permitted (you must delete the corresponding project %q)", namespace.Name, project.Name))
109109
}

0 commit comments

Comments
 (0)