Skip to content

Commit 02450fd

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #479 from prateekpandey14/refact-validation
Move snapshot validations to validation-webhook directory
2 parents de6c35a + 533ed88 commit 02450fd

File tree

4 files changed

+114
-91
lines changed

4 files changed

+114
-91
lines changed

‎pkg/common-controller/snapshot_controller.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ import (
3434
crdv1 "github.com/kubernetes-csi/external-snapshotter/client/v4/apis/volumesnapshot/v1"
3535
"github.com/kubernetes-csi/external-snapshotter/v4/pkg/metrics"
3636
"github.com/kubernetes-csi/external-snapshotter/v4/pkg/utils"
37+
webhook "github.com/kubernetes-csi/external-snapshotter/v4/pkg/validation-webhook"
3738
)
3839

3940
// ==================================================================
@@ -1505,7 +1506,7 @@ func (ctrl *csiSnapshotCommonController) setAnnVolumeSnapshotBeingDeleted(conten
15051506
// checkAndSetInvalidContentLabel adds a label to unlabeled invalid content objects and removes the label from valid ones.
15061507
func (ctrl *csiSnapshotCommonController) checkAndSetInvalidContentLabel(content *crdv1.VolumeSnapshotContent) (*crdv1.VolumeSnapshotContent, error) {
15071508
hasLabel := utils.MapContainsKey(content.ObjectMeta.Labels, utils.VolumeSnapshotContentInvalidLabel)
1508-
err := utils.ValidateV1SnapshotContent(content)
1509+
err := webhook.ValidateV1SnapshotContent(content)
15091510
if err != nil {
15101511
klog.Errorf("syncContent[%s]: Invalid content detected, %s", content.Name, err.Error())
15111512
}
@@ -1546,7 +1547,7 @@ func (ctrl *csiSnapshotCommonController) checkAndSetInvalidContentLabel(content
15461547
// checkAndSetInvalidSnapshotLabel adds a label to unlabeled invalid snapshot objects and removes the label from valid ones.
15471548
func (ctrl *csiSnapshotCommonController) checkAndSetInvalidSnapshotLabel(snapshot *crdv1.VolumeSnapshot) (*crdv1.VolumeSnapshot, error) {
15481549
hasLabel := utils.MapContainsKey(snapshot.ObjectMeta.Labels, utils.VolumeSnapshotInvalidLabel)
1549-
err := utils.ValidateV1Snapshot(snapshot)
1550+
err := webhook.ValidateV1Snapshot(snapshot)
15501551
if err != nil {
15511552
klog.Errorf("syncSnapshot[%s]: Invalid snapshot detected, %s", utils.SnapshotKey(snapshot), err.Error())
15521553
}

‎pkg/utils/util.go

Lines changed: 0 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ import (
2525
"time"
2626

2727
crdv1 "github.com/kubernetes-csi/external-snapshotter/client/v4/apis/volumesnapshot/v1"
28-
crdv1beta1 "github.com/kubernetes-csi/external-snapshotter/client/v4/apis/volumesnapshot/v1beta1"
2928
v1 "k8s.io/api/core/v1"
3029
"k8s.io/apimachinery/pkg/api/meta"
3130
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -122,87 +121,6 @@ var SnapshotterListSecretParams = secretParamsMap{
122121
secretNamespaceKey: PrefixedSnapshotterListSecretNamespaceKey,
123122
}
124123

125-
// ValidateV1Snapshot performs additional strict validation.
126-
// Do NOT rely on this function to fully validate snapshot objects.
127-
// This function will only check the additional rules provided by the webhook.
128-
func ValidateV1Snapshot(snapshot *crdv1.VolumeSnapshot) error {
129-
if snapshot == nil {
130-
return fmt.Errorf("VolumeSnapshot is nil")
131-
}
132-
133-
vscname := snapshot.Spec.VolumeSnapshotClassName
134-
if vscname != nil && *vscname == "" {
135-
return fmt.Errorf("Spec.VolumeSnapshotClassName must not be the empty string")
136-
}
137-
return nil
138-
}
139-
140-
// ValidateV1Beta1Snapshot performs additional strict validation.
141-
// Do NOT rely on this function to fully validate snapshot objects.
142-
// This function will only check the additional rules provided by the webhook.
143-
func ValidateV1Beta1Snapshot(snapshot *crdv1beta1.VolumeSnapshot) error {
144-
if snapshot == nil {
145-
return fmt.Errorf("VolumeSnapshot is nil")
146-
}
147-
148-
source := snapshot.Spec.Source
149-
150-
if source.PersistentVolumeClaimName != nil && source.VolumeSnapshotContentName != nil {
151-
return fmt.Errorf("only one of Spec.Source.PersistentVolumeClaimName = %s and Spec.Source.VolumeSnapshotContentName = %s should be set", *source.PersistentVolumeClaimName, *source.VolumeSnapshotContentName)
152-
}
153-
if source.PersistentVolumeClaimName == nil && source.VolumeSnapshotContentName == nil {
154-
return fmt.Errorf("one of Spec.Source.PersistentVolumeClaimName and Spec.Source.VolumeSnapshotContentName should be set")
155-
}
156-
vscname := snapshot.Spec.VolumeSnapshotClassName
157-
if vscname != nil && *vscname == "" {
158-
return fmt.Errorf("Spec.VolumeSnapshotClassName must not be the empty string")
159-
}
160-
return nil
161-
}
162-
163-
// ValidateV1SnapshotContent performs additional strict validation.
164-
// Do NOT rely on this function to fully validate snapshot content objects.
165-
// This function will only check the additional rules provided by the webhook.
166-
func ValidateV1SnapshotContent(snapcontent *crdv1.VolumeSnapshotContent) error {
167-
if snapcontent == nil {
168-
return fmt.Errorf("VolumeSnapshotContent is nil")
169-
}
170-
171-
vsref := snapcontent.Spec.VolumeSnapshotRef
172-
173-
if vsref.Name == "" || vsref.Namespace == "" {
174-
return fmt.Errorf("both Spec.VolumeSnapshotRef.Name = %s and Spec.VolumeSnapshotRef.Namespace = %s must be set", vsref.Name, vsref.Namespace)
175-
}
176-
177-
return nil
178-
}
179-
180-
// ValidateV1Beta1SnapshotContent performs additional strict validation.
181-
// Do NOT rely on this function to fully validate snapshot content objects.
182-
// This function will only check the additional rules provided by the webhook.
183-
func ValidateV1Beta1SnapshotContent(snapcontent *crdv1beta1.VolumeSnapshotContent) error {
184-
if snapcontent == nil {
185-
return fmt.Errorf("VolumeSnapshotContent is nil")
186-
}
187-
188-
source := snapcontent.Spec.Source
189-
190-
if source.VolumeHandle != nil && source.SnapshotHandle != nil {
191-
return fmt.Errorf("only one of Spec.Source.VolumeHandle = %s and Spec.Source.SnapshotHandle = %s should be set", *source.VolumeHandle, *source.SnapshotHandle)
192-
}
193-
if source.VolumeHandle == nil && source.SnapshotHandle == nil {
194-
return fmt.Errorf("one of Spec.Source.VolumeHandle and Spec.Source.SnapshotHandle should be set")
195-
}
196-
197-
vsref := snapcontent.Spec.VolumeSnapshotRef
198-
199-
if vsref.Name == "" || vsref.Namespace == "" {
200-
return fmt.Errorf("both Spec.VolumeSnapshotRef.Name = %s and Spec.VolumeSnapshotRef.Namespace = %s must be set", vsref.Name, vsref.Namespace)
201-
}
202-
203-
return nil
204-
}
205-
206124
// MapContainsKey checks if a given map of string to string contains the provided string.
207125
func MapContainsKey(m map[string]string, s string) bool {
208126
_, r := m[s]

‎pkg/validation-webhook/snapshot.go

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ import (
2222

2323
volumesnapshotv1 "github.com/kubernetes-csi/external-snapshotter/client/v4/apis/volumesnapshot/v1"
2424
volumesnapshotv1beta1 "github.com/kubernetes-csi/external-snapshotter/client/v4/apis/volumesnapshot/v1beta1"
25-
"github.com/kubernetes-csi/external-snapshotter/v4/pkg/utils"
2625
v1 "k8s.io/api/admission/v1"
2726
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2827
"k8s.io/klog/v2"
@@ -126,7 +125,7 @@ func decideSnapshotV1beta1(snapshot, oldSnapshot *volumesnapshotv1beta1.VolumeSn
126125
// Which allows the remover of finalizers and therefore deletion of this object
127126
// Don't rely on the pointers to be nil, because the deserialization method will convert it to
128127
// The empty struct value. Instead check the operation type.
129-
if err := utils.ValidateV1Beta1Snapshot(oldSnapshot); err != nil {
128+
if err := ValidateV1Beta1Snapshot(oldSnapshot); err != nil {
130129
return reviewResponse
131130
}
132131

@@ -139,7 +138,7 @@ func decideSnapshotV1beta1(snapshot, oldSnapshot *volumesnapshotv1beta1.VolumeSn
139138
}
140139
// Enforce strict validation for CREATE requests. Immutable checks don't apply for CREATE requests.
141140
// Enforce strict validation for UPDATE requests where old is valid and passes immutability check.
142-
if err := utils.ValidateV1Beta1Snapshot(snapshot); err != nil {
141+
if err := ValidateV1Beta1Snapshot(snapshot); err != nil {
143142
reviewResponse.Allowed = false
144143
reviewResponse.Result.Message = err.Error()
145144
}
@@ -162,7 +161,7 @@ func decideSnapshotV1(snapshot, oldSnapshot *volumesnapshotv1.VolumeSnapshot, is
162161
}
163162
// Enforce strict validation for CREATE requests. Immutable checks don't apply for CREATE requests.
164163
// Enforce strict validation for UPDATE requests where old is valid and passes immutability check.
165-
if err := utils.ValidateV1Snapshot(snapshot); err != nil {
164+
if err := ValidateV1Snapshot(snapshot); err != nil {
166165
reviewResponse.Allowed = false
167166
reviewResponse.Result.Message = err.Error()
168167
}
@@ -181,7 +180,7 @@ func decideSnapshotContentV1beta1(snapcontent, oldSnapcontent *volumesnapshotv1b
181180
// Which allows the remover of finalizers and therefore deletion of this object
182181
// Don't rely on the pointers to be nil, because the deserialization method will convert it to
183182
// The empty struct value. Instead check the operation type.
184-
if err := utils.ValidateV1Beta1SnapshotContent(oldSnapcontent); err != nil {
183+
if err := ValidateV1Beta1SnapshotContent(oldSnapcontent); err != nil {
185184
return reviewResponse
186185
}
187186

@@ -194,7 +193,7 @@ func decideSnapshotContentV1beta1(snapcontent, oldSnapcontent *volumesnapshotv1b
194193
}
195194
// Enforce strict validation for all CREATE requests. Immutable checks don't apply for CREATE requests.
196195
// Enforce strict validation for UPDATE requests where old is valid and passes immutability check.
197-
if err := utils.ValidateV1Beta1SnapshotContent(snapcontent); err != nil {
196+
if err := ValidateV1Beta1SnapshotContent(snapcontent); err != nil {
198197
reviewResponse.Allowed = false
199198
reviewResponse.Result.Message = err.Error()
200199
}
@@ -217,7 +216,7 @@ func decideSnapshotContentV1(snapcontent, oldSnapcontent *volumesnapshotv1.Volum
217216
}
218217
// Enforce strict validation for all CREATE requests. Immutable checks don't apply for CREATE requests.
219218
// Enforce strict validation for UPDATE requests where old is valid and passes immutability check.
220-
if err := utils.ValidateV1SnapshotContent(snapcontent); err != nil {
219+
if err := ValidateV1SnapshotContent(snapcontent); err != nil {
221220
reviewResponse.Allowed = false
222221
reviewResponse.Result.Message = err.Error()
223222
}

‎pkg/validation-webhook/validation.go

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
/*
2+
Copyright 2021 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 webhook
18+
19+
import (
20+
"fmt"
21+
22+
crdv1 "github.com/kubernetes-csi/external-snapshotter/client/v4/apis/volumesnapshot/v1"
23+
crdv1beta1 "github.com/kubernetes-csi/external-snapshotter/client/v4/apis/volumesnapshot/v1beta1"
24+
)
25+
26+
// ValidateV1Snapshot performs additional strict validation.
27+
// Do NOT rely on this function to fully validate snapshot objects.
28+
// This function will only check the additional rules provided by the webhook.
29+
func ValidateV1Snapshot(snapshot *crdv1.VolumeSnapshot) error {
30+
if snapshot == nil {
31+
return fmt.Errorf("VolumeSnapshot is nil")
32+
}
33+
34+
vscname := snapshot.Spec.VolumeSnapshotClassName
35+
if vscname != nil && *vscname == "" {
36+
return fmt.Errorf("Spec.VolumeSnapshotClassName must not be the empty string")
37+
}
38+
return nil
39+
}
40+
41+
// ValidateV1Beta1Snapshot performs additional strict validation.
42+
// Do NOT rely on this function to fully validate snapshot objects.
43+
// This function will only check the additional rules provided by the webhook.
44+
func ValidateV1Beta1Snapshot(snapshot *crdv1beta1.VolumeSnapshot) error {
45+
if snapshot == nil {
46+
return fmt.Errorf("VolumeSnapshot is nil")
47+
}
48+
49+
source := snapshot.Spec.Source
50+
51+
if source.PersistentVolumeClaimName != nil && source.VolumeSnapshotContentName != nil {
52+
return fmt.Errorf("only one of Spec.Source.PersistentVolumeClaimName = %s and Spec.Source.VolumeSnapshotContentName = %s should be set", *source.PersistentVolumeClaimName, *source.VolumeSnapshotContentName)
53+
}
54+
if source.PersistentVolumeClaimName == nil && source.VolumeSnapshotContentName == nil {
55+
return fmt.Errorf("one of Spec.Source.PersistentVolumeClaimName and Spec.Source.VolumeSnapshotContentName should be set")
56+
}
57+
vscname := snapshot.Spec.VolumeSnapshotClassName
58+
if vscname != nil && *vscname == "" {
59+
return fmt.Errorf("Spec.VolumeSnapshotClassName must not be the empty string")
60+
}
61+
return nil
62+
}
63+
64+
// ValidateV1SnapshotContent performs additional strict validation.
65+
// Do NOT rely on this function to fully validate snapshot content objects.
66+
// This function will only check the additional rules provided by the webhook.
67+
func ValidateV1SnapshotContent(snapcontent *crdv1.VolumeSnapshotContent) error {
68+
if snapcontent == nil {
69+
return fmt.Errorf("VolumeSnapshotContent is nil")
70+
}
71+
72+
vsref := snapcontent.Spec.VolumeSnapshotRef
73+
74+
if vsref.Name == "" || vsref.Namespace == "" {
75+
return fmt.Errorf("both Spec.VolumeSnapshotRef.Name = %s and Spec.VolumeSnapshotRef.Namespace = %s must be set", vsref.Name, vsref.Namespace)
76+
}
77+
78+
return nil
79+
}
80+
81+
// ValidateV1Beta1SnapshotContent performs additional strict validation.
82+
// Do NOT rely on this function to fully validate snapshot content objects.
83+
// This function will only check the additional rules provided by the webhook.
84+
func ValidateV1Beta1SnapshotContent(snapcontent *crdv1beta1.VolumeSnapshotContent) error {
85+
if snapcontent == nil {
86+
return fmt.Errorf("VolumeSnapshotContent is nil")
87+
}
88+
89+
source := snapcontent.Spec.Source
90+
91+
if source.VolumeHandle != nil && source.SnapshotHandle != nil {
92+
return fmt.Errorf("only one of Spec.Source.VolumeHandle = %s and Spec.Source.SnapshotHandle = %s should be set", *source.VolumeHandle, *source.SnapshotHandle)
93+
}
94+
if source.VolumeHandle == nil && source.SnapshotHandle == nil {
95+
return fmt.Errorf("one of Spec.Source.VolumeHandle and Spec.Source.SnapshotHandle should be set")
96+
}
97+
98+
vsref := snapcontent.Spec.VolumeSnapshotRef
99+
100+
if vsref.Name == "" || vsref.Namespace == "" {
101+
return fmt.Errorf("both Spec.VolumeSnapshotRef.Name = %s and Spec.VolumeSnapshotRef.Namespace = %s must be set", vsref.Name, vsref.Namespace)
102+
}
103+
104+
return nil
105+
}

0 commit comments

Comments
 (0)