Skip to content

Commit beaa3ea

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request kubernetes-sigs#9458 from Ankitasw/move-docker-api-webhooks
🌱 Move docker infrastructure API v1beta1 webhooks to separate package
2 parents d981496 + a0ec023 commit beaa3ea

File tree

10 files changed

+211
-76
lines changed

10 files changed

+211
-76
lines changed

‎Makefile

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -334,13 +334,15 @@ generate-manifests-kubeadm-control-plane: $(CONTROLLER_GEN) ## Generate manifest
334334

335335
.PHONY: generate-manifests-docker-infrastructure
336336
generate-manifests-docker-infrastructure: $(CONTROLLER_GEN) ## Generate manifests e.g. CRD, RBAC etc. for docker infrastructure provider
337-
$(MAKE) clean-generated-yaml SRC_DIRS="$(CAPD_DIR)/config/crd/bases"
337+
$(MAKE) clean-generated-yaml SRC_DIRS="$(CAPD_DIR)/config/crd/bases,$(CAPD_DIR)/config/webhook/manifests.yaml"
338338
cd $(CAPD_DIR); $(CONTROLLER_GEN) \
339339
paths=./ \
340340
paths=./api/... \
341341
paths=./$(EXP_DIR)/api/... \
342342
paths=./$(EXP_DIR)/internal/controllers/... \
343+
paths=./$(EXP_DIR)/internal/webhooks/... \
343344
paths=./internal/controllers/... \
345+
paths=./internal/webhooks/... \
344346
crd:crdVersions=v1 \
345347
rbac:roleName=manager-role \
346348
output:crd:dir=./config/crd/bases \

‎test/infrastructure/docker/internal/webhooks/doc.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
/*
2+
Copyright 2023 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
// Package webhooks implements docker infrastructure webhooks.
18+
package webhooks

‎test/infrastructure/docker/api/v1beta1/dockercluster_webhook.go renamed to ‎test/infrastructure/docker/internal/webhooks/dockercluster_webhook.go

Lines changed: 33 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -14,60 +14,79 @@ See the License for the specific language governing permissions and
1414
limitations under the License.
1515
*/
1616

17-
package v1beta1
17+
package webhooks
1818

1919
import (
20+
"context"
21+
"fmt"
22+
2023
apierrors "k8s.io/apimachinery/pkg/api/errors"
2124
"k8s.io/apimachinery/pkg/runtime"
2225
"k8s.io/apimachinery/pkg/util/validation/field"
2326
ctrl "sigs.k8s.io/controller-runtime"
2427
"sigs.k8s.io/controller-runtime/pkg/webhook"
2528
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
29+
30+
infrav1 "sigs.k8s.io/cluster-api/test/infrastructure/docker/api/v1beta1"
2631
)
2732

28-
func (c *DockerCluster) SetupWebhookWithManager(mgr ctrl.Manager) error {
33+
// DockerCluster implements a validating and defaulting webhook for DockerCluster.
34+
type DockerCluster struct{}
35+
36+
func (webhook *DockerCluster) SetupWebhookWithManager(mgr ctrl.Manager) error {
2937
return ctrl.NewWebhookManagedBy(mgr).
30-
For(c).
38+
For(&infrav1.DockerCluster{}).
39+
WithDefaulter(webhook).
40+
WithValidator(webhook).
3141
Complete()
3242
}
3343

3444
// +kubebuilder:webhook:verbs=create;update,path=/mutate-infrastructure-cluster-x-k8s-io-v1beta1-dockercluster,mutating=true,failurePolicy=fail,matchPolicy=Equivalent,groups=infrastructure.cluster.x-k8s.io,resources=dockerclusters,versions=v1beta1,name=default.dockercluster.infrastructure.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1;v1beta1
3545

36-
var _ webhook.Defaulter = &DockerCluster{}
46+
var _ webhook.CustomDefaulter = &DockerCluster{}
3747

3848
// Default implements webhook.Defaulter so a webhook will be registered for the type.
39-
func (c *DockerCluster) Default() {
40-
defaultDockerClusterSpec(&c.Spec)
49+
func (webhook *DockerCluster) Default(_ context.Context, obj runtime.Object) error {
50+
cluster, ok := obj.(*infrav1.DockerCluster)
51+
if !ok {
52+
return apierrors.NewBadRequest(fmt.Sprintf("expected a DockerCluster but got a %T", obj))
53+
}
54+
defaultDockerClusterSpec(&cluster.Spec)
55+
return nil
4156
}
4257

4358
// +kubebuilder:webhook:verbs=create;update,path=/validate-infrastructure-cluster-x-k8s-io-v1beta1-dockercluster,mutating=false,failurePolicy=fail,matchPolicy=Equivalent,groups=infrastructure.cluster.x-k8s.io,resources=dockerclusters,versions=v1beta1,name=validation.dockercluster.infrastructure.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1;v1beta1
4459

45-
var _ webhook.Validator = &DockerCluster{}
60+
var _ webhook.CustomValidator = &DockerCluster{}
4661

4762
// ValidateCreate implements webhook.Validator so a webhook will be registered for the type.
48-
func (c *DockerCluster) ValidateCreate() (admission.Warnings, error) {
49-
if allErrs := validateDockerClusterSpec(c.Spec); len(allErrs) > 0 {
50-
return nil, apierrors.NewInvalid(GroupVersion.WithKind("DockerCluster").GroupKind(), c.Name, allErrs)
63+
func (webhook *DockerCluster) ValidateCreate(_ context.Context, obj runtime.Object) (admission.Warnings, error) {
64+
cluster, ok := obj.(*infrav1.DockerCluster)
65+
if !ok {
66+
return nil, apierrors.NewBadRequest(fmt.Sprintf("expected a DockerCluster but got a %T", obj))
67+
}
68+
if allErrs := validateDockerClusterSpec(cluster.Spec); len(allErrs) > 0 {
69+
return nil, apierrors.NewInvalid(infrav1.GroupVersion.WithKind("DockerCluster").GroupKind(), cluster.Name, allErrs)
5170
}
5271
return nil, nil
5372
}
5473

5574
// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type.
56-
func (c *DockerCluster) ValidateUpdate(_ runtime.Object) (admission.Warnings, error) {
75+
func (webhook *DockerCluster) ValidateUpdate(_ context.Context, _, _ runtime.Object) (admission.Warnings, error) {
5776
return nil, nil
5877
}
5978

6079
// ValidateDelete implements webhook.Validator so a webhook will be registered for the type.
61-
func (c *DockerCluster) ValidateDelete() (admission.Warnings, error) {
80+
func (webhook *DockerCluster) ValidateDelete(_ context.Context, _ runtime.Object) (admission.Warnings, error) {
6281
return nil, nil
6382
}
6483

65-
func defaultDockerClusterSpec(s *DockerClusterSpec) {
84+
func defaultDockerClusterSpec(s *infrav1.DockerClusterSpec) {
6685
if s.ControlPlaneEndpoint.Port == 0 {
6786
s.ControlPlaneEndpoint.Port = 6443
6887
}
6988
}
7089

71-
func validateDockerClusterSpec(_ DockerClusterSpec) field.ErrorList {
90+
func validateDockerClusterSpec(_ infrav1.DockerClusterSpec) field.ErrorList {
7291
return nil
7392
}

‎test/infrastructure/docker/api/v1beta1/dockerclustertemplate_webhook.go renamed to ‎test/infrastructure/docker/internal/webhooks/dockerclustertemplate_webhook.go

Lines changed: 38 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,10 @@ See the License for the specific language governing permissions and
1414
limitations under the License.
1515
*/
1616

17-
package v1beta1
17+
package webhooks
1818

1919
import (
20+
"context"
2021
"fmt"
2122
"reflect"
2223

@@ -28,31 +29,42 @@ import (
2829
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
2930

3031
"sigs.k8s.io/cluster-api/feature"
32+
infrav1 "sigs.k8s.io/cluster-api/test/infrastructure/docker/api/v1beta1"
3133
)
3234

3335
const dockerClusterTemplateImmutableMsg = "DockerClusterTemplate spec.template.spec field is immutable. Please create a new resource instead."
3436

35-
func (r *DockerClusterTemplate) SetupWebhookWithManager(mgr ctrl.Manager) error {
37+
// DockerClusterTemplate implements a validating and defaulting webhook for DockerClusterTemplate.
38+
type DockerClusterTemplate struct{}
39+
40+
func (webhook *DockerClusterTemplate) SetupWebhookWithManager(mgr ctrl.Manager) error {
3641
return ctrl.NewWebhookManagedBy(mgr).
37-
For(r).
42+
For(&infrav1.DockerClusterTemplate{}).
43+
WithDefaulter(webhook).
44+
WithValidator(webhook).
3845
Complete()
3946
}
4047

4148
// +kubebuilder:webhook:verbs=create;update,path=/mutate-infrastructure-cluster-x-k8s-io-v1beta1-dockerclustertemplate,mutating=true,failurePolicy=fail,matchPolicy=Equivalent,groups=infrastructure.cluster.x-k8s.io,resources=dockerclustertemplates,versions=v1beta1,name=default.dockerclustertemplate.infrastructure.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1;v1beta1
4249

43-
var _ webhook.Defaulter = &DockerClusterTemplate{}
50+
var _ webhook.CustomDefaulter = &DockerClusterTemplate{}
4451

4552
// Default implements webhook.Defaulter so a webhook will be registered for the type.
46-
func (r *DockerClusterTemplate) Default() {
47-
defaultDockerClusterSpec(&r.Spec.Template.Spec)
53+
func (webhook *DockerClusterTemplate) Default(_ context.Context, obj runtime.Object) error {
54+
clusterTemplate, ok := obj.(*infrav1.DockerClusterTemplate)
55+
if !ok {
56+
return apierrors.NewBadRequest(fmt.Sprintf("expected a DockerClusterTemplate but got a %T", obj))
57+
}
58+
defaultDockerClusterSpec(&clusterTemplate.Spec.Template.Spec)
59+
return nil
4860
}
4961

5062
// +kubebuilder:webhook:verbs=create;update,path=/validate-infrastructure-cluster-x-k8s-io-v1beta1-dockerclustertemplate,mutating=false,failurePolicy=fail,matchPolicy=Equivalent,groups=infrastructure.cluster.x-k8s.io,resources=dockerclustertemplates,versions=v1beta1,name=validation.dockerclustertemplate.infrastructure.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1;v1beta1
5163

52-
var _ webhook.Validator = &DockerClusterTemplate{}
64+
var _ webhook.CustomValidator = &DockerClusterTemplate{}
5365

5466
// ValidateCreate implements webhook.Validator so a webhook will be registered for the type.
55-
func (r *DockerClusterTemplate) ValidateCreate() (admission.Warnings, error) {
67+
func (webhook *DockerClusterTemplate) ValidateCreate(_ context.Context, obj runtime.Object) (admission.Warnings, error) {
5668
// NOTE: DockerClusterTemplate is behind ClusterTopology feature gate flag; the web hook
5769
// must prevent creating new objects in case the feature flag is disabled.
5870
if !feature.Gates.Enabled(feature.ClusterTopology) {
@@ -62,34 +74,43 @@ func (r *DockerClusterTemplate) ValidateCreate() (admission.Warnings, error) {
6274
)
6375
}
6476

65-
allErrs := validateDockerClusterSpec(r.Spec.Template.Spec)
77+
clusterTemplate, ok := obj.(*infrav1.DockerClusterTemplate)
78+
if !ok {
79+
return nil, apierrors.NewBadRequest(fmt.Sprintf("expected a DockerClusterTemplate but got a %T", obj))
80+
}
81+
82+
allErrs := validateDockerClusterSpec(clusterTemplate.Spec.Template.Spec)
6683

6784
// Validate the metadata of the template.
68-
allErrs = append(allErrs, r.Spec.Template.ObjectMeta.Validate(field.NewPath("spec", "template", "metadata"))...)
85+
allErrs = append(allErrs, clusterTemplate.Spec.Template.ObjectMeta.Validate(field.NewPath("spec", "template", "metadata"))...)
6986

7087
if len(allErrs) > 0 {
71-
return nil, apierrors.NewInvalid(GroupVersion.WithKind("DockerClusterTemplate").GroupKind(), r.Name, allErrs)
88+
return nil, apierrors.NewInvalid(infrav1.GroupVersion.WithKind("DockerClusterTemplate").GroupKind(), clusterTemplate.Name, allErrs)
7289
}
7390
return nil, nil
7491
}
7592

7693
// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type.
77-
func (r *DockerClusterTemplate) ValidateUpdate(oldRaw runtime.Object) (admission.Warnings, error) {
94+
func (webhook *DockerClusterTemplate) ValidateUpdate(_ context.Context, oldRaw, newRaw runtime.Object) (admission.Warnings, error) {
7895
var allErrs field.ErrorList
79-
old, ok := oldRaw.(*DockerClusterTemplate)
96+
oldTemplate, ok := oldRaw.(*infrav1.DockerClusterTemplate)
8097
if !ok {
8198
return nil, apierrors.NewBadRequest(fmt.Sprintf("expected a DockerClusterTemplate but got a %T", oldRaw))
8299
}
83-
if !reflect.DeepEqual(r.Spec.Template.Spec, old.Spec.Template.Spec) {
84-
allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "template", "spec"), r, dockerClusterTemplateImmutableMsg))
100+
newTemplate, ok := newRaw.(*infrav1.DockerClusterTemplate)
101+
if !ok {
102+
return nil, apierrors.NewBadRequest(fmt.Sprintf("expected a DockerClusterTemplate but got a %T", newRaw))
103+
}
104+
if !reflect.DeepEqual(newTemplate.Spec.Template.Spec, oldTemplate.Spec.Template.Spec) {
105+
allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "template", "spec"), newTemplate, dockerClusterTemplateImmutableMsg))
85106
}
86107
if len(allErrs) == 0 {
87108
return nil, nil
88109
}
89-
return nil, apierrors.NewInvalid(GroupVersion.WithKind("DockerClusterTemplate").GroupKind(), r.Name, allErrs)
110+
return nil, apierrors.NewInvalid(infrav1.GroupVersion.WithKind("DockerClusterTemplate").GroupKind(), newTemplate.Name, allErrs)
90111
}
91112

92113
// ValidateDelete implements webhook.Validator so a webhook will be registered for the type.
93-
func (r *DockerClusterTemplate) ValidateDelete() (admission.Warnings, error) {
114+
func (webhook *DockerClusterTemplate) ValidateDelete(_ context.Context, _ runtime.Object) (admission.Warnings, error) {
94115
return nil, nil
95116
}

‎test/infrastructure/docker/api/v1beta1/dockerclustertemplate_webhook_test.go renamed to ‎test/infrastructure/docker/internal/webhooks/dockerclustertemplate_webhook_test.go

Lines changed: 23 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
1414
limitations under the License.
1515
*/
1616

17-
package v1beta1
17+
package webhooks
1818

1919
import (
2020
"strings"
@@ -23,28 +23,33 @@ import (
2323
. "github.com/onsi/gomega"
2424
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2525
utilfeature "k8s.io/component-base/featuregate/testing"
26+
ctrl "sigs.k8s.io/controller-runtime"
2627

2728
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
2829
"sigs.k8s.io/cluster-api/feature"
30+
infrav1 "sigs.k8s.io/cluster-api/test/infrastructure/docker/api/v1beta1"
2931
)
3032

33+
var ctx = ctrl.SetupSignalHandler()
34+
3135
func TestDockerClusterTemplateValidationFeatureGateEnabled(t *testing.T) {
3236
defer utilfeature.SetFeatureGateDuringTest(t, feature.Gates, feature.ClusterTopology, true)()
3337

3438
t.Run("create dockerclustertemplate should pass if gate enabled and valid dockerclustertemplate", func(t *testing.T) {
3539
g := NewWithT(t)
36-
dct := &DockerClusterTemplate{
40+
dct := &infrav1.DockerClusterTemplate{
3741
ObjectMeta: metav1.ObjectMeta{
3842
Name: "dockerclustertemplate-test",
3943
Namespace: "test-namespace",
4044
},
41-
Spec: DockerClusterTemplateSpec{
42-
Template: DockerClusterTemplateResource{
43-
Spec: DockerClusterSpec{},
45+
Spec: infrav1.DockerClusterTemplateSpec{
46+
Template: infrav1.DockerClusterTemplateResource{
47+
Spec: infrav1.DockerClusterSpec{},
4448
},
4549
},
4650
}
47-
warnings, err := dct.ValidateCreate()
51+
webhook := DockerClusterTemplate{}
52+
warnings, err := webhook.ValidateCreate(ctx, dct)
4853
g.Expect(err).ToNot(HaveOccurred())
4954
g.Expect(warnings).To(BeEmpty())
5055
})
@@ -54,18 +59,19 @@ func TestDockerClusterTemplateValidationFeatureGateDisabled(t *testing.T) {
5459
// NOTE: ClusterTopology feature flag is disabled by default, thus preventing to create DockerClusterTemplate.
5560
t.Run("create dockerclustertemplate should not pass if gate disabled and valid dockerclustertemplate", func(t *testing.T) {
5661
g := NewWithT(t)
57-
dct := &DockerClusterTemplate{
62+
dct := &infrav1.DockerClusterTemplate{
5863
ObjectMeta: metav1.ObjectMeta{
5964
Name: "dockerclustertemplate-test",
6065
Namespace: "test-namespace",
6166
},
62-
Spec: DockerClusterTemplateSpec{
63-
Template: DockerClusterTemplateResource{
64-
Spec: DockerClusterSpec{},
67+
Spec: infrav1.DockerClusterTemplateSpec{
68+
Template: infrav1.DockerClusterTemplateResource{
69+
Spec: infrav1.DockerClusterSpec{},
6570
},
6671
},
6772
}
68-
warnings, err := dct.ValidateCreate()
73+
webhook := DockerClusterTemplate{}
74+
warnings, err := webhook.ValidateCreate(ctx, dct)
6975
g.Expect(err).To(HaveOccurred())
7076
g.Expect(warnings).To(BeEmpty())
7177
})
@@ -97,13 +103,13 @@ func TestDockerClusterTemplateValidationMetadata(t *testing.T) {
97103
for _, tt := range tests {
98104
t.Run(tt.name, func(t *testing.T) {
99105
g := NewWithT(t)
100-
dct := &DockerClusterTemplate{
106+
dct := &infrav1.DockerClusterTemplate{
101107
ObjectMeta: metav1.ObjectMeta{
102108
Name: "dockerclustertemplate-test",
103109
Namespace: "test-namespace",
104110
},
105-
Spec: DockerClusterTemplateSpec{
106-
Template: DockerClusterTemplateResource{
111+
Spec: infrav1.DockerClusterTemplateSpec{
112+
Template: infrav1.DockerClusterTemplateResource{
107113
ObjectMeta: clusterv1.ObjectMeta{
108114
Labels: map[string]string{
109115
"foo": "$invalid-key",
@@ -114,11 +120,12 @@ func TestDockerClusterTemplateValidationMetadata(t *testing.T) {
114120
"/invalid-key": "foo",
115121
},
116122
},
117-
Spec: DockerClusterSpec{},
123+
Spec: infrav1.DockerClusterSpec{},
118124
},
119125
},
120126
}
121-
warnings, err := dct.ValidateCreate()
127+
webhook := DockerClusterTemplate{}
128+
warnings, err := webhook.ValidateCreate(ctx, dct)
122129
if tt.expectErr {
123130
g.Expect(err).To(HaveOccurred())
124131
g.Expect(warnings).To(BeEmpty())

0 commit comments

Comments
 (0)