diff --git a/deploy/kubernetes/webhook-example/admission-configuration-template b/deploy/kubernetes/webhook-example/admission-configuration-template index 8c412404..5825adc6 100644 --- a/deploy/kubernetes/webhook-example/admission-configuration-template +++ b/deploy/kubernetes/webhook-example/admission-configuration-template @@ -20,3 +20,26 @@ webhooks: sideEffects: None failurePolicy: Ignore # We recommend switching to Fail only after successful installation of the webhook server and webhook. timeoutSeconds: 2 # This will affect the latency and performance. Finetune this value based on your application's tolerance. +--- +apiVersion: admissionregistration.k8s.io/v1 +kind: ValidatingWebhookConfiguration +metadata: + name: "validation-webhook.groupsnapshot.storage.k8s.io" +webhooks: +- name: "validation-webhook.groupsnapshot.storage.k8s.io" + rules: + - apiGroups: ["groupsnapshot.storage.k8s.io"] + apiVersions: ["v1alpha1"] + operations: ["CREATE", "UPDATE"] + resources: ["volumegroupsnapshots", "volumegroupsnapshotcontents", "volumegroupsnapshotclasses"] + scope: "*" + clientConfig: + service: + namespace: "default" + name: "snapshot-validation-service" + path: "/volumegroupsnapshot" + caBundle: ${CA_BUNDLE} + admissionReviewVersions: ["v1"] + sideEffects: None + failurePolicy: Ignore # We recommend switching to Fail only after successful installation of the webhook server and webhook. + timeoutSeconds: 2 # This will affect the latency and performance. Finetune this value based on your application's tolerance. diff --git a/deploy/kubernetes/webhook-example/rbac-snapshot-webhook.yaml b/deploy/kubernetes/webhook-example/rbac-snapshot-webhook.yaml index e2d69843..17057a2f 100644 --- a/deploy/kubernetes/webhook-example/rbac-snapshot-webhook.yaml +++ b/deploy/kubernetes/webhook-example/rbac-snapshot-webhook.yaml @@ -19,6 +19,9 @@ rules: - apiGroups: ["snapshot.storage.k8s.io"] resources: ["volumesnapshotclasses"] verbs: ["get", "list", "watch"] + - apiGroups: ["groupsnapshot.storage.k8s.io"] + resources: ["volumegroupsnapshotclasses"] + verbs: ["get", "list", "watch"] --- kind: ClusterRoleBinding apiVersion: rbac.authorization.k8s.io/v1 diff --git a/pkg/utils/util.go b/pkg/utils/util.go index c4cf84dd..9c89d16c 100644 --- a/pkg/utils/util.go +++ b/pkg/utils/util.go @@ -77,7 +77,8 @@ const ( // Name of finalizer on PVCs that is being used as a source to create VolumeSnapshots PVCFinalizer = "snapshot.storage.kubernetes.io/pvc-as-source-protection" - IsDefaultSnapshotClassAnnotation = "snapshot.storage.kubernetes.io/is-default-class" + IsDefaultSnapshotClassAnnotation = "snapshot.storage.kubernetes.io/is-default-class" + IsDefaultGroupSnapshotClassAnnotation = "groupsnapshot.storage.kubernetes.io/is-default-class" // AnnVolumeSnapshotBeingDeleted annotation applies to VolumeSnapshotContents. // It indicates that the common snapshot controller has verified that volume diff --git a/pkg/validation-webhook/groupsnapshot.go b/pkg/validation-webhook/groupsnapshot.go new file mode 100644 index 00000000..05f3344b --- /dev/null +++ b/pkg/validation-webhook/groupsnapshot.go @@ -0,0 +1,254 @@ +/* +Copyright 2023 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 webhook + +import ( + "fmt" + "reflect" + + volumegroupsnapshotv1alpha1 "github.com/kubernetes-csi/external-snapshotter/client/v6/apis/volumegroupsnapshot/v1alpha1" + groupsnapshotlisters "github.com/kubernetes-csi/external-snapshotter/client/v6/listers/volumegroupsnapshot/v1alpha1" + "github.com/kubernetes-csi/external-snapshotter/v6/pkg/utils" + v1 "k8s.io/api/admission/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/klog/v2" +) + +var ( + // GroupSnapshotV1Alpha1GVR is GroupVersionResource for v1alpha1 VolumeGroupSnapshots + GroupSnapshotV1Alpha1GVR = metav1.GroupVersionResource{Group: volumegroupsnapshotv1alpha1.GroupName, Version: "v1alpha1", Resource: "volumegroupsnapshots"} + // GroupSnapshotContentV1Apha1GVR is GroupVersionResource for v1alpha1 VolumeGroupSnapshotContents + GroupSnapshotContentV1Apha1GVR = metav1.GroupVersionResource{Group: volumegroupsnapshotv1alpha1.GroupName, Version: "v1alpha1", Resource: "volumegroupsnapshotcontents"} + // GroupSnapshotClassV1Apha1GVR is GroupVersionResource for v1alpha1 VolumeGroupSnapshotClasses + GroupSnapshotClassV1Apha1GVR = metav1.GroupVersionResource{Group: volumegroupsnapshotv1alpha1.GroupName, Version: "v1alpha1", Resource: "volumegroupsnapshotclasses"} +) + +type GroupSnapshotAdmitter interface { + Admit(v1.AdmissionReview) *v1.AdmissionResponse +} + +type groupSnapshotAdmitter struct { + lister groupsnapshotlisters.VolumeGroupSnapshotClassLister +} + +func NewGroupSnapshotAdmitter(lister groupsnapshotlisters.VolumeGroupSnapshotClassLister) GroupSnapshotAdmitter { + return &groupSnapshotAdmitter{ + lister: lister, + } +} + +// Add a label {"added-label": "yes"} to the object +func (a groupSnapshotAdmitter) Admit(ar v1.AdmissionReview) *v1.AdmissionResponse { + klog.V(2).Info("admitting volumegroupsnapshots volumegroupsnapshotcontents " + + "or volumegroupsnapshotclasses") + + reviewResponse := &v1.AdmissionResponse{ + Allowed: true, + Result: &metav1.Status{}, + } + + // Admit requests other than Update and Create + if !(ar.Request.Operation == v1.Update || ar.Request.Operation == v1.Create) { + return reviewResponse + } + isUpdate := ar.Request.Operation == v1.Update + + raw := ar.Request.Object.Raw + oldRaw := ar.Request.OldObject.Raw + + deserializer := codecs.UniversalDeserializer() + switch ar.Request.Resource { + case GroupSnapshotV1Alpha1GVR: + groupSnapshot := &volumegroupsnapshotv1alpha1.VolumeGroupSnapshot{} + if _, _, err := deserializer.Decode(raw, nil, groupSnapshot); err != nil { + klog.Error(err) + return toV1AdmissionResponse(err) + } + oldGroupSnapshot := &volumegroupsnapshotv1alpha1.VolumeGroupSnapshot{} + if _, _, err := deserializer.Decode(oldRaw, nil, oldGroupSnapshot); err != nil { + klog.Error(err) + return toV1AdmissionResponse(err) + } + return decideGroupSnapshotV1Alpha1(groupSnapshot, oldGroupSnapshot, isUpdate) + case GroupSnapshotContentV1Apha1GVR: + groupSnapContent := &volumegroupsnapshotv1alpha1.VolumeGroupSnapshotContent{} + if _, _, err := deserializer.Decode(raw, nil, groupSnapContent); err != nil { + klog.Error(err) + return toV1AdmissionResponse(err) + } + oldGroupSnapContent := &volumegroupsnapshotv1alpha1.VolumeGroupSnapshotContent{} + if _, _, err := deserializer.Decode(oldRaw, nil, oldGroupSnapContent); err != nil { + klog.Error(err) + return toV1AdmissionResponse(err) + } + return decideGroupSnapshotContentV1Alpha1(groupSnapContent, oldGroupSnapContent, isUpdate) + case GroupSnapshotClassV1Apha1GVR: + groupSnapClass := &volumegroupsnapshotv1alpha1.VolumeGroupSnapshotClass{} + if _, _, err := deserializer.Decode(raw, nil, groupSnapClass); err != nil { + klog.Error(err) + return toV1AdmissionResponse(err) + } + oldGroupSnapClass := &volumegroupsnapshotv1alpha1.VolumeGroupSnapshotClass{} + if _, _, err := deserializer.Decode(oldRaw, nil, oldGroupSnapClass); err != nil { + klog.Error(err) + return toV1AdmissionResponse(err) + } + return decideGroupSnapshotClassV1Alpha1(groupSnapClass, oldGroupSnapClass, a.lister) + default: + err := fmt.Errorf("expect resource to be %s, %s, or %s, but found %v", + GroupSnapshotV1Alpha1GVR, GroupSnapshotContentV1Apha1GVR, + GroupSnapshotClassV1Apha1GVR, ar.Request.Resource) + klog.Error(err) + return toV1AdmissionResponse(err) + } +} + +func decideGroupSnapshotV1Alpha1(groupSnapshot, oldGroupSnapshot *volumegroupsnapshotv1alpha1.VolumeGroupSnapshot, isUpdate bool) *v1.AdmissionResponse { + reviewResponse := &v1.AdmissionResponse{ + Allowed: true, + Result: &metav1.Status{}, + } + + if isUpdate { + // if it is an UPDATE and oldGroupSnapshot is valid, check immutable fields + if err := checkGroupSnapshotImmutableFieldsV1Alpha1(groupSnapshot, oldGroupSnapshot); err != nil { + reviewResponse.Allowed = false + reviewResponse.Result.Message = err.Error() + return reviewResponse + } + } + // Enforce strict validation for CREATE requests. Immutable checks don't apply for CREATE requests. + // Enforce strict validation for UPDATE requests where old is valid and passes immutability check. + if err := ValidateV1Alpha1GroupSnapshot(groupSnapshot); err != nil { + reviewResponse.Allowed = false + reviewResponse.Result.Message = err.Error() + } + return reviewResponse +} + +func decideGroupSnapshotContentV1Alpha1(groupSnapcontent, oldGroupSnapcontent *volumegroupsnapshotv1alpha1.VolumeGroupSnapshotContent, isUpdate bool) *v1.AdmissionResponse { + reviewResponse := &v1.AdmissionResponse{ + Allowed: true, + Result: &metav1.Status{}, + } + + if isUpdate { + // if it is an UPDATE and oldGroupSnapcontent is valid, check immutable fields + if err := checkGroupSnapshotContentImmutableFieldsV1Alpha1(groupSnapcontent, oldGroupSnapcontent); err != nil { + reviewResponse.Allowed = false + reviewResponse.Result.Message = err.Error() + return reviewResponse + } + } + // Enforce strict validation for all CREATE requests. Immutable checks don't apply for CREATE requests. + // Enforce strict validation for UPDATE requests where old is valid and passes immutability check. + if err := ValidateV1Alpha1GroupSnapshotContent(groupSnapcontent); err != nil { + reviewResponse.Allowed = false + reviewResponse.Result.Message = err.Error() + } + return reviewResponse +} + +func decideGroupSnapshotClassV1Alpha1(groupSnapClass, oldGroupSnapClass *volumegroupsnapshotv1alpha1.VolumeGroupSnapshotClass, lister groupsnapshotlisters.VolumeGroupSnapshotClassLister) *v1.AdmissionResponse { + reviewResponse := &v1.AdmissionResponse{ + Allowed: true, + Result: &metav1.Status{}, + } + + // Only Validate when a new group snapshot class is being set as a default. + if groupSnapClass.Annotations[utils.IsDefaultGroupSnapshotClassAnnotation] != "true" { + return reviewResponse + } + + // If the old group snapshot class has this, then we can assume that it was validated if driver is the same. + if oldGroupSnapClass.Annotations[utils.IsDefaultGroupSnapshotClassAnnotation] == "true" && oldGroupSnapClass.Driver == groupSnapClass.Driver { + return reviewResponse + } + + ret, err := lister.List(labels.Everything()) + if err != nil { + reviewResponse.Allowed = false + reviewResponse.Result.Message = err.Error() + return reviewResponse + } + + for _, groupSnapshotClass := range ret { + if groupSnapshotClass.Annotations[utils.IsDefaultGroupSnapshotClassAnnotation] != "true" { + continue + } + if groupSnapshotClass.Driver == groupSnapClass.Driver { + reviewResponse.Allowed = false + reviewResponse.Result.Message = fmt.Sprintf("default group snapshot class: %v already exists for driver: %v", groupSnapshotClass.Name, groupSnapClass.Driver) + return reviewResponse + } + } + + return reviewResponse +} + +func checkGroupSnapshotImmutableFieldsV1Alpha1(groupSnapshot, oldGroupSnapshot *volumegroupsnapshotv1alpha1.VolumeGroupSnapshot) error { + if groupSnapshot == nil { + return fmt.Errorf("VolumeGroupSnapshot is nil") + } + if oldGroupSnapshot == nil { + return fmt.Errorf("old VolumeGroupSnapshot is nil") + } + + source := groupSnapshot.Spec.Source + oldSource := oldGroupSnapshot.Spec.Source + + if !reflect.DeepEqual(source.Selector, oldSource.Selector) { + return fmt.Errorf("Spec.Source.Selector is immutable but was changed from %s to %s", oldSource.Selector, source.Selector) + } + if !reflect.DeepEqual(source.VolumeGroupSnapshotContentName, oldSource.VolumeGroupSnapshotContentName) { + return fmt.Errorf("Spec.Source.VolumeGroupSnapshotContentName is immutable but was changed from %s to %s", strPtrDereference(oldSource.VolumeGroupSnapshotContentName), strPtrDereference(source.VolumeGroupSnapshotContentName)) + } + + return nil +} + +func checkGroupSnapshotContentImmutableFieldsV1Alpha1(groupSnapcontent, oldGroupSnapcontent *volumegroupsnapshotv1alpha1.VolumeGroupSnapshotContent) error { + if groupSnapcontent == nil { + return fmt.Errorf("VolumeGroupSnapshotContent is nil") + } + if oldGroupSnapcontent == nil { + return fmt.Errorf("old VolumeGroupSnapshotContent is nil") + } + + source := groupSnapcontent.Spec.Source + oldSource := oldGroupSnapcontent.Spec.Source + + if !reflect.DeepEqual(source.VolumeGroupSnapshotHandle, oldSource.VolumeGroupSnapshotHandle) { + return fmt.Errorf("Spec.Source.VolumeGroupSnapshotHandle is immutable but was changed from %s to %s", strPtrDereference(oldSource.VolumeGroupSnapshotHandle), strPtrDereference(source.VolumeGroupSnapshotHandle)) + } + if !reflect.DeepEqual(source.PersistentVolumeNames, oldSource.PersistentVolumeNames) { + return fmt.Errorf("Spec.Source.PersistentVolumeNames is immutable but was changed from %v to %v", oldSource.PersistentVolumeNames, source.PersistentVolumeNames) + } + + ref := groupSnapcontent.Spec.VolumeGroupSnapshotRef + oldRef := oldGroupSnapcontent.Spec.VolumeGroupSnapshotRef + + if ref.Name != oldRef.Name { + return fmt.Errorf("Spec.VolumeGroupSnapshotRef.Name is immutable but was changed from %s to %s", oldRef.Name, ref.Name) + } + if ref.Namespace != oldRef.Namespace { + return fmt.Errorf("Spec.VolumeGroupSnapshotRef.Namespace is immutable but was changed from %s to %s", oldRef.Namespace, ref.Namespace) + } + + return nil +} diff --git a/pkg/validation-webhook/groupsnapshot_test.go b/pkg/validation-webhook/groupsnapshot_test.go new file mode 100644 index 00000000..40701c8d --- /dev/null +++ b/pkg/validation-webhook/groupsnapshot_test.go @@ -0,0 +1,678 @@ +/* +Copyright 2023 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 webhook + +import ( + "encoding/json" + "fmt" + "testing" + + volumegroupsnapshotv1alpha1 "github.com/kubernetes-csi/external-snapshotter/client/v6/apis/volumegroupsnapshot/v1alpha1" + groupsnapshotlisters "github.com/kubernetes-csi/external-snapshotter/client/v6/listers/volumegroupsnapshot/v1alpha1" + "github.com/kubernetes-csi/external-snapshotter/v6/pkg/utils" + v1 "k8s.io/api/admission/v1" + core_v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/runtime" +) + +type fakeGroupSnapshotLister struct { + values []*volumegroupsnapshotv1alpha1.VolumeGroupSnapshotClass +} + +func (f *fakeGroupSnapshotLister) List(selector labels.Selector) (ret []*volumegroupsnapshotv1alpha1.VolumeGroupSnapshotClass, err error) { + return f.values, nil +} + +func (f *fakeGroupSnapshotLister) Get(name string) (*volumegroupsnapshotv1alpha1.VolumeGroupSnapshotClass, error) { + for _, v := range f.values { + if v.Name == name { + return v, nil + } + } + return nil, nil +} + +func TestAdmitVolumeGroupSnapshotV1Alpha1(t *testing.T) { + selector := metav1.LabelSelector{MatchLabels: map[string]string{ + "group": "A", + }} + mutatedField := "changed-immutable-field" + contentname := "groupsnapcontent1" + emptyVolumeGroupSnapshotClassName := "" + + testCases := []struct { + name string + volumeGroupSnapshot *volumegroupsnapshotv1alpha1.VolumeGroupSnapshot + oldVolumeGroupSnapshot *volumegroupsnapshotv1alpha1.VolumeGroupSnapshot + shouldAdmit bool + msg string + operation v1.Operation + }{ + { + name: "Delete: new and old are nil. Should admit", + volumeGroupSnapshot: nil, + oldVolumeGroupSnapshot: nil, + shouldAdmit: true, + operation: v1.Delete, + }, + { + name: "Create: old is nil and new is valid, with contentname", + volumeGroupSnapshot: &volumegroupsnapshotv1alpha1.VolumeGroupSnapshot{ + Spec: volumegroupsnapshotv1alpha1.VolumeGroupSnapshotSpec{ + Source: volumegroupsnapshotv1alpha1.VolumeGroupSnapshotSource{ + VolumeGroupSnapshotContentName: &contentname, + }, + }, + }, + oldVolumeGroupSnapshot: nil, + shouldAdmit: true, + msg: "", + operation: v1.Create, + }, + { + name: "Create: old is nil and new is valid, with selector", + volumeGroupSnapshot: &volumegroupsnapshotv1alpha1.VolumeGroupSnapshot{ + Spec: volumegroupsnapshotv1alpha1.VolumeGroupSnapshotSpec{ + Source: volumegroupsnapshotv1alpha1.VolumeGroupSnapshotSource{ + Selector: selector, + }, + }, + }, + oldVolumeGroupSnapshot: nil, + shouldAdmit: true, + msg: "", + operation: v1.Create, + }, + { + name: "Update: old is valid and new is invalid", + volumeGroupSnapshot: &volumegroupsnapshotv1alpha1.VolumeGroupSnapshot{ + Spec: volumegroupsnapshotv1alpha1.VolumeGroupSnapshotSpec{ + Source: volumegroupsnapshotv1alpha1.VolumeGroupSnapshotSource{ + VolumeGroupSnapshotContentName: &contentname, + }, + VolumeGroupSnapshotClassName: &emptyVolumeGroupSnapshotClassName, + }, + }, + oldVolumeGroupSnapshot: &volumegroupsnapshotv1alpha1.VolumeGroupSnapshot{ + Spec: volumegroupsnapshotv1alpha1.VolumeGroupSnapshotSpec{ + Source: volumegroupsnapshotv1alpha1.VolumeGroupSnapshotSource{ + VolumeGroupSnapshotContentName: &contentname, + }, + }, + }, + shouldAdmit: false, + operation: v1.Update, + msg: "Spec.VolumeGroupSnapshotClassName must not be the empty string", + }, + { + name: "Update: old is valid and new is valid", + volumeGroupSnapshot: &volumegroupsnapshotv1alpha1.VolumeGroupSnapshot{ + Spec: volumegroupsnapshotv1alpha1.VolumeGroupSnapshotSpec{ + Source: volumegroupsnapshotv1alpha1.VolumeGroupSnapshotSource{ + VolumeGroupSnapshotContentName: &contentname, + }, + }, + }, + oldVolumeGroupSnapshot: &volumegroupsnapshotv1alpha1.VolumeGroupSnapshot{ + Spec: volumegroupsnapshotv1alpha1.VolumeGroupSnapshotSpec{ + Source: volumegroupsnapshotv1alpha1.VolumeGroupSnapshotSource{ + VolumeGroupSnapshotContentName: &contentname, + }, + }, + }, + shouldAdmit: true, + operation: v1.Update, + }, + { + name: "Update: old is valid and new is valid but changes immutable field spec.source", + volumeGroupSnapshot: &volumegroupsnapshotv1alpha1.VolumeGroupSnapshot{ + Spec: volumegroupsnapshotv1alpha1.VolumeGroupSnapshotSpec{ + Source: volumegroupsnapshotv1alpha1.VolumeGroupSnapshotSource{ + VolumeGroupSnapshotContentName: &mutatedField, + }, + }, + }, + oldVolumeGroupSnapshot: &volumegroupsnapshotv1alpha1.VolumeGroupSnapshot{ + Spec: volumegroupsnapshotv1alpha1.VolumeGroupSnapshotSpec{ + Source: volumegroupsnapshotv1alpha1.VolumeGroupSnapshotSource{ + VolumeGroupSnapshotContentName: &contentname, + }, + }, + }, + shouldAdmit: false, + operation: v1.Update, + msg: fmt.Sprintf("Spec.Source.VolumeGroupSnapshotContentName is immutable but was changed from %s to %s", contentname, mutatedField), + }, + { + name: "Update: old is invalid and new is valid", + volumeGroupSnapshot: &volumegroupsnapshotv1alpha1.VolumeGroupSnapshot{ + Spec: volumegroupsnapshotv1alpha1.VolumeGroupSnapshotSpec{ + Source: volumegroupsnapshotv1alpha1.VolumeGroupSnapshotSource{ + VolumeGroupSnapshotContentName: &contentname, + }, + }, + }, + oldVolumeGroupSnapshot: &volumegroupsnapshotv1alpha1.VolumeGroupSnapshot{ + Spec: volumegroupsnapshotv1alpha1.VolumeGroupSnapshotSpec{ + Source: volumegroupsnapshotv1alpha1.VolumeGroupSnapshotSource{ + VolumeGroupSnapshotContentName: &contentname, + Selector: selector, + }, + }, + }, + shouldAdmit: false, + operation: v1.Update, + msg: fmt.Sprintf("Spec.Source.Selector is immutable but was changed from %v to %v", selector, metav1.LabelSelector{}), + }, + { + // will be handled by schema validation + name: "Update: old is invalid and new is invalid", + volumeGroupSnapshot: &volumegroupsnapshotv1alpha1.VolumeGroupSnapshot{ + Spec: volumegroupsnapshotv1alpha1.VolumeGroupSnapshotSpec{ + Source: volumegroupsnapshotv1alpha1.VolumeGroupSnapshotSource{ + VolumeGroupSnapshotContentName: &contentname, + Selector: selector, + }, + }, + }, + oldVolumeGroupSnapshot: &volumegroupsnapshotv1alpha1.VolumeGroupSnapshot{ + Spec: volumegroupsnapshotv1alpha1.VolumeGroupSnapshotSpec{ + Source: volumegroupsnapshotv1alpha1.VolumeGroupSnapshotSource{ + VolumeGroupSnapshotContentName: &contentname, + Selector: selector, + }, + }, + }, + shouldAdmit: true, + operation: v1.Update, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + groupSnapshot := tc.volumeGroupSnapshot + raw, err := json.Marshal(groupSnapshot) + if err != nil { + t.Fatal(err) + } + oldGroupSnapshot := tc.oldVolumeGroupSnapshot + oldRaw, err := json.Marshal(oldGroupSnapshot) + if err != nil { + t.Fatal(err) + } + review := v1.AdmissionReview{ + Request: &v1.AdmissionRequest{ + Object: runtime.RawExtension{ + Raw: raw, + }, + OldObject: runtime.RawExtension{ + Raw: oldRaw, + }, + Resource: GroupSnapshotV1Alpha1GVR, + Operation: tc.operation, + }, + } + sa := NewGroupSnapshotAdmitter(nil) + response := sa.Admit(review) + shouldAdmit := response.Allowed + msg := response.Result.Message + + expectedResponse := tc.shouldAdmit + expectedMsg := tc.msg + + if shouldAdmit != expectedResponse { + t.Errorf("expected \"%v\" to equal \"%v\": %v", shouldAdmit, expectedResponse, msg) + } + if msg != expectedMsg { + t.Errorf("expected \"%v\" to equal \"%v\"", msg, expectedMsg) + } + }) + } +} + +func TestAdmitVolumeGroupSnapshotContentV1Alpha1(t *testing.T) { + volumeHandle := "volumeHandle1" + modifiedField := "modified-field" + groupSnapshotHandle := "groupsnapshotHandle1" + volumeGroupSnapshotClassName := "volume-snapshot-class-1" + validContent := &volumegroupsnapshotv1alpha1.VolumeGroupSnapshotContent{ + Spec: volumegroupsnapshotv1alpha1.VolumeGroupSnapshotContentSpec{ + Source: volumegroupsnapshotv1alpha1.VolumeGroupSnapshotContentSource{ + VolumeGroupSnapshotHandle: &groupSnapshotHandle, + }, + VolumeGroupSnapshotRef: core_v1.ObjectReference{ + Name: "group-snapshot-ref", + Namespace: "default-ns", + }, + VolumeGroupSnapshotClassName: &volumeGroupSnapshotClassName, + }, + } + invalidContent := &volumegroupsnapshotv1alpha1.VolumeGroupSnapshotContent{ + Spec: volumegroupsnapshotv1alpha1.VolumeGroupSnapshotContentSpec{ + Source: volumegroupsnapshotv1alpha1.VolumeGroupSnapshotContentSource{ + VolumeGroupSnapshotHandle: &groupSnapshotHandle, + PersistentVolumeNames: []string{volumeHandle}, + }, + VolumeGroupSnapshotRef: core_v1.ObjectReference{ + Name: "", + Namespace: "default-ns", + }}, + } + + testCases := []struct { + name string + groupSnapContent *volumegroupsnapshotv1alpha1.VolumeGroupSnapshotContent + oldGroupSnapContent *volumegroupsnapshotv1alpha1.VolumeGroupSnapshotContent + shouldAdmit bool + msg string + operation v1.Operation + }{ + { + name: "Delete: both new and old are nil", + groupSnapContent: nil, + oldGroupSnapContent: nil, + shouldAdmit: true, + operation: v1.Delete, + }, + { + name: "Create: old is nil and new is valid", + groupSnapContent: validContent, + oldGroupSnapContent: nil, + shouldAdmit: true, + operation: v1.Create, + }, + { + name: "Update: old is valid and new is invalid", + groupSnapContent: invalidContent, + oldGroupSnapContent: validContent, + shouldAdmit: false, + operation: v1.Update, + msg: fmt.Sprintf("Spec.Source.PersistentVolumeNames is immutable but was changed from %s to %s", []string{}, []string{volumeHandle}), + }, + { + name: "Update: old is valid and new is valid", + groupSnapContent: validContent, + oldGroupSnapContent: validContent, + shouldAdmit: true, + operation: v1.Update, + }, + { + name: "Update: old is valid and new is valid but modifies immutable field", + groupSnapContent: &volumegroupsnapshotv1alpha1.VolumeGroupSnapshotContent{ + Spec: volumegroupsnapshotv1alpha1.VolumeGroupSnapshotContentSpec{ + Source: volumegroupsnapshotv1alpha1.VolumeGroupSnapshotContentSource{ + VolumeGroupSnapshotHandle: &modifiedField, + }, + VolumeGroupSnapshotRef: core_v1.ObjectReference{ + Name: "snapshot-ref", + Namespace: "default-ns", + }, + }, + }, + oldGroupSnapContent: validContent, + shouldAdmit: false, + operation: v1.Update, + msg: fmt.Sprintf("Spec.Source.VolumeGroupSnapshotHandle is immutable but was changed from %s to %s", groupSnapshotHandle, modifiedField), + }, + { + name: "Update: old is valid and new is valid but modifies immutable ref", + groupSnapContent: &volumegroupsnapshotv1alpha1.VolumeGroupSnapshotContent{ + Spec: volumegroupsnapshotv1alpha1.VolumeGroupSnapshotContentSpec{ + Source: volumegroupsnapshotv1alpha1.VolumeGroupSnapshotContentSource{ + VolumeGroupSnapshotHandle: &groupSnapshotHandle, + }, + VolumeGroupSnapshotRef: core_v1.ObjectReference{ + Name: modifiedField, + Namespace: "default-ns", + }, + }, + }, + oldGroupSnapContent: validContent, + shouldAdmit: false, + operation: v1.Update, + msg: fmt.Sprintf("Spec.VolumeGroupSnapshotRef.Name is immutable but was changed from %s to %s", + validContent.Spec.VolumeGroupSnapshotRef.Name, modifiedField), + }, + { + name: "Update: old is invalid and new is valid", + groupSnapContent: validContent, + oldGroupSnapContent: invalidContent, + shouldAdmit: false, + operation: v1.Update, + msg: fmt.Sprintf("Spec.Source.PersistentVolumeNames is immutable but was changed from %s to %s", []string{volumeHandle}, []string{}), + }, + { + name: "Update: old is invalid and new is invalid", + groupSnapContent: invalidContent, + oldGroupSnapContent: invalidContent, + shouldAdmit: false, + operation: v1.Update, + msg: "both Spec.VolumeGroupSnapshotRef.Name = and Spec.VolumeGroupSnapshotRef.Namespace = default-ns must be set", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + groupSnapContent := tc.groupSnapContent + raw, err := json.Marshal(groupSnapContent) + if err != nil { + t.Fatal(err) + } + oldGroupSnapContent := tc.oldGroupSnapContent + oldRaw, err := json.Marshal(oldGroupSnapContent) + if err != nil { + t.Fatal(err) + } + review := v1.AdmissionReview{ + Request: &v1.AdmissionRequest{ + Object: runtime.RawExtension{ + Raw: raw, + }, + OldObject: runtime.RawExtension{ + Raw: oldRaw, + }, + Resource: GroupSnapshotContentV1Apha1GVR, + Operation: tc.operation, + }, + } + sa := NewGroupSnapshotAdmitter(nil) + response := sa.Admit(review) + shouldAdmit := response.Allowed + msg := response.Result.Message + + expectedResponse := tc.shouldAdmit + expectedMsg := tc.msg + + if shouldAdmit != expectedResponse { + t.Errorf("expected \"%v\" to equal \"%v\"", shouldAdmit, expectedResponse) + } + if msg != expectedMsg { + t.Errorf("expected \"%v\" to equal \"%v\"", msg, expectedMsg) + } + }) + } +} + +func TestAdmitVolumeGroupSnapshotClassV1Alpha1(t *testing.T) { + testCases := []struct { + name string + groupSnapClass *volumegroupsnapshotv1alpha1.VolumeGroupSnapshotClass + oldGroupSnapClass *volumegroupsnapshotv1alpha1.VolumeGroupSnapshotClass + shouldAdmit bool + msg string + operation v1.Operation + lister groupsnapshotlisters.VolumeGroupSnapshotClassLister + }{ + { + name: "new default for group snapshot class with no existing group snapshot classes", + groupSnapClass: &volumegroupsnapshotv1alpha1.VolumeGroupSnapshotClass{ + TypeMeta: metav1.TypeMeta{}, + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + utils.IsDefaultGroupSnapshotClassAnnotation: "true", + }, + }, + Driver: "test.csi.io", + }, + oldGroupSnapClass: nil, + shouldAdmit: true, + msg: "", + operation: v1.Create, + lister: &fakeGroupSnapshotLister{values: []*volumegroupsnapshotv1alpha1.VolumeGroupSnapshotClass{}}, + }, + { + name: "new default for group snapshot class for with existing default group snapshot class with different drivers", + groupSnapClass: &volumegroupsnapshotv1alpha1.VolumeGroupSnapshotClass{ + TypeMeta: metav1.TypeMeta{}, + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + utils.IsDefaultGroupSnapshotClassAnnotation: "true", + }, + }, + Driver: "test.csi.io", + }, + oldGroupSnapClass: &volumegroupsnapshotv1alpha1.VolumeGroupSnapshotClass{}, + shouldAdmit: true, + msg: "", + operation: v1.Create, + lister: &fakeGroupSnapshotLister{values: []*volumegroupsnapshotv1alpha1.VolumeGroupSnapshotClass{ + { + TypeMeta: metav1.TypeMeta{}, + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + utils.IsDefaultGroupSnapshotClassAnnotation: "true", + }, + }, + Driver: "existing.test.csi.io", + }, + }}, + }, + { + name: "new default for group snapshot class with existing default group snapshot class same driver", + groupSnapClass: &volumegroupsnapshotv1alpha1.VolumeGroupSnapshotClass{ + TypeMeta: metav1.TypeMeta{}, + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + utils.IsDefaultGroupSnapshotClassAnnotation: "true", + }, + }, + Driver: "test.csi.io", + }, + oldGroupSnapClass: &volumegroupsnapshotv1alpha1.VolumeGroupSnapshotClass{}, + shouldAdmit: false, + msg: "default group snapshot class: driver-a already exists for driver: test.csi.io", + operation: v1.Create, + lister: &fakeGroupSnapshotLister{values: []*volumegroupsnapshotv1alpha1.VolumeGroupSnapshotClass{ + { + TypeMeta: metav1.TypeMeta{}, + ObjectMeta: metav1.ObjectMeta{ + Name: "driver-a", + Annotations: map[string]string{ + utils.IsDefaultGroupSnapshotClassAnnotation: "true", + }, + }, + Driver: "test.csi.io", + }, + }}, + }, + { + name: "default for group snapshot class with existing default group snapshot class same driver update", + groupSnapClass: &volumegroupsnapshotv1alpha1.VolumeGroupSnapshotClass{ + TypeMeta: metav1.TypeMeta{}, + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + utils.IsDefaultGroupSnapshotClassAnnotation: "true", + }, + }, + Driver: "test.csi.io", + }, + oldGroupSnapClass: &volumegroupsnapshotv1alpha1.VolumeGroupSnapshotClass{ + TypeMeta: metav1.TypeMeta{}, + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + utils.IsDefaultGroupSnapshotClassAnnotation: "true", + }, + }, + Driver: "test.csi.io", + }, + shouldAdmit: true, + msg: "", + operation: v1.Update, + lister: &fakeGroupSnapshotLister{values: []*volumegroupsnapshotv1alpha1.VolumeGroupSnapshotClass{ + { + TypeMeta: metav1.TypeMeta{}, + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + utils.IsDefaultGroupSnapshotClassAnnotation: "true", + }, + }, + Driver: "test.csi.io", + }, + }}, + }, + { + name: "new group snapshot for group snapshot class with existing default group snapshot class same driver", + groupSnapClass: &volumegroupsnapshotv1alpha1.VolumeGroupSnapshotClass{ + TypeMeta: metav1.TypeMeta{}, + ObjectMeta: metav1.ObjectMeta{}, + Driver: "test.csi.io", + }, + oldGroupSnapClass: &volumegroupsnapshotv1alpha1.VolumeGroupSnapshotClass{}, + shouldAdmit: true, + msg: "", + operation: v1.Create, + lister: &fakeGroupSnapshotLister{values: []*volumegroupsnapshotv1alpha1.VolumeGroupSnapshotClass{ + { + TypeMeta: metav1.TypeMeta{}, + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + utils.IsDefaultGroupSnapshotClassAnnotation: "true", + }, + }, + Driver: "test.csi.io", + }, + }}, + }, + { + name: "new group snapshot for group snapshot class with existing group snapshot default classes", + groupSnapClass: &volumegroupsnapshotv1alpha1.VolumeGroupSnapshotClass{ + TypeMeta: metav1.TypeMeta{}, + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + utils.IsDefaultGroupSnapshotClassAnnotation: "true", + }, + }, + Driver: "test.csi.io", + }, + oldGroupSnapClass: &volumegroupsnapshotv1alpha1.VolumeGroupSnapshotClass{}, + shouldAdmit: false, + msg: "default group snapshot class: driver-is-default already exists for driver: test.csi.io", + operation: v1.Create, + lister: &fakeGroupSnapshotLister{[]*volumegroupsnapshotv1alpha1.VolumeGroupSnapshotClass{ + { + TypeMeta: metav1.TypeMeta{}, + ObjectMeta: metav1.ObjectMeta{ + Name: "driver-is-default", + Annotations: map[string]string{ + utils.IsDefaultGroupSnapshotClassAnnotation: "true", + }, + }, + Driver: "test.csi.io", + }, + { + TypeMeta: metav1.TypeMeta{}, + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + utils.IsDefaultGroupSnapshotClassAnnotation: "true", + }, + }, + Driver: "test.csi.io", + }, + }}, + }, + { + name: "update group snapshot class to new driver with existing default group snapshot classes", + groupSnapClass: &volumegroupsnapshotv1alpha1.VolumeGroupSnapshotClass{ + TypeMeta: metav1.TypeMeta{}, + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + utils.IsDefaultGroupSnapshotClassAnnotation: "true", + }, + }, + Driver: "driver.test.csi.io", + }, + oldGroupSnapClass: &volumegroupsnapshotv1alpha1.VolumeGroupSnapshotClass{ + TypeMeta: metav1.TypeMeta{}, + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + utils.IsDefaultGroupSnapshotClassAnnotation: "true", + }, + }, + Driver: "test.csi.io", + }, + shouldAdmit: false, + msg: "default group snapshot class: driver-test-default already exists for driver: driver.test.csi.io", + operation: v1.Update, + lister: &fakeGroupSnapshotLister{values: []*volumegroupsnapshotv1alpha1.VolumeGroupSnapshotClass{ + { + TypeMeta: metav1.TypeMeta{}, + ObjectMeta: metav1.ObjectMeta{ + Name: "driver-is-default", + Annotations: map[string]string{ + utils.IsDefaultGroupSnapshotClassAnnotation: "true", + }, + }, + Driver: "test.csi.io", + }, + { + TypeMeta: metav1.TypeMeta{}, + ObjectMeta: metav1.ObjectMeta{ + Name: "driver-test-default", + Annotations: map[string]string{ + utils.IsDefaultGroupSnapshotClassAnnotation: "true", + }, + }, + Driver: "driver.test.csi.io", + }, + }}, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + groupSnapContent := tc.groupSnapClass + raw, err := json.Marshal(groupSnapContent) + if err != nil { + t.Fatal(err) + } + oldGroupSnapClass := tc.oldGroupSnapClass + oldRaw, err := json.Marshal(oldGroupSnapClass) + if err != nil { + t.Fatal(err) + } + review := v1.AdmissionReview{ + Request: &v1.AdmissionRequest{ + Object: runtime.RawExtension{ + Raw: raw, + }, + OldObject: runtime.RawExtension{ + Raw: oldRaw, + }, + Resource: GroupSnapshotClassV1Apha1GVR, + Operation: tc.operation, + }, + } + sa := NewGroupSnapshotAdmitter(tc.lister) + response := sa.Admit(review) + + shouldAdmit := response.Allowed + msg := response.Result.Message + + expectedResponse := tc.shouldAdmit + expectedMsg := tc.msg + + if shouldAdmit != expectedResponse { + t.Errorf("expected \"%v\" to equal \"%v\"", shouldAdmit, expectedResponse) + } + if msg != expectedMsg { + t.Errorf("expected \"%v\" to equal \"%v\"", msg, expectedMsg) + } + }) + } +} diff --git a/pkg/validation-webhook/snapshot.go b/pkg/validation-webhook/snapshot.go index 1d6bb93c..0ddff0e9 100644 --- a/pkg/validation-webhook/snapshot.go +++ b/pkg/validation-webhook/snapshot.go @@ -109,7 +109,8 @@ func (a admitter) Admit(ar v1.AdmissionReview) *v1.AdmissionResponse { } return decideSnapshotClassV1(snapClass, oldSnapClass, a.lister) default: - err := fmt.Errorf("expect resource to be %s, %s or %s", SnapshotV1GVR, SnapshotContentV1GVR, SnapshotClassV1GVR) + err := fmt.Errorf("expect resource to be %s, %s, or %s, but found %v", + SnapshotV1GVR, SnapshotContentV1GVR, SnapshotClassV1GVR, ar.Request.Resource) klog.Error(err) return toV1AdmissionResponse(err) } diff --git a/pkg/validation-webhook/snapshot_test.go b/pkg/validation-webhook/snapshot_test.go index a1cec8ea..62f0fae5 100644 --- a/pkg/validation-webhook/snapshot_test.go +++ b/pkg/validation-webhook/snapshot_test.go @@ -314,7 +314,7 @@ func TestAdmitVolumeSnapshotContentV1(t *testing.T) { oldVolumeSnapshotContent: invalidContent, shouldAdmit: false, operation: v1.Update, - msg: fmt.Sprintf("both Spec.VolumeSnapshotRef.Name = and Spec.VolumeSnapshotRef.Namespace = default-ns must be set"), + msg: "both Spec.VolumeSnapshotRef.Name = and Spec.VolumeSnapshotRef.Namespace = default-ns must be set", }, } diff --git a/pkg/validation-webhook/validation.go b/pkg/validation-webhook/validation.go index b097c422..7d9329c1 100644 --- a/pkg/validation-webhook/validation.go +++ b/pkg/validation-webhook/validation.go @@ -19,6 +19,7 @@ package webhook import ( "fmt" + groupsnapshotcrdv1alpha1 "github.com/kubernetes-csi/external-snapshotter/client/v6/apis/volumegroupsnapshot/v1alpha1" crdv1 "github.com/kubernetes-csi/external-snapshotter/client/v6/apis/volumesnapshot/v1" ) @@ -53,3 +54,35 @@ func ValidateV1SnapshotContent(snapcontent *crdv1.VolumeSnapshotContent) error { return nil } + +// ValidateV1Alpha1GroupSnapshotContent performs additional strict validation. +// Do NOT rely on this function to fully validate group snapshot content objects. +// This function will only check the additional rules provided by the webhook. +func ValidateV1Alpha1GroupSnapshotContent(groupSnapcontent *groupsnapshotcrdv1alpha1.VolumeGroupSnapshotContent) error { + if groupSnapcontent == nil { + return fmt.Errorf("VolumeGroupSnapshotContent is nil") + } + + vgsref := groupSnapcontent.Spec.VolumeGroupSnapshotRef + + if vgsref.Name == "" || vgsref.Namespace == "" { + return fmt.Errorf("both Spec.VolumeGroupSnapshotRef.Name = %s and Spec.VolumeGroupSnapshotRef.Namespace = %s must be set", vgsref.Name, vgsref.Namespace) + } + + return nil +} + +// ValidateV1Alpha1GroupSnapshot performs additional strict validation. +// Do NOT rely on this function to fully validate group snapshot objects. +// This function will only check the additional rules provided by the webhook. +func ValidateV1Alpha1GroupSnapshot(snapshot *groupsnapshotcrdv1alpha1.VolumeGroupSnapshot) error { + if snapshot == nil { + return fmt.Errorf("VolumeGroupSnapshot is nil") + } + + vgscname := snapshot.Spec.VolumeGroupSnapshotClassName + if vgscname != nil && *vgscname == "" { + return fmt.Errorf("Spec.VolumeGroupSnapshotClassName must not be the empty string") + } + return nil +} diff --git a/pkg/validation-webhook/webhook.go b/pkg/validation-webhook/webhook.go index 42d87d68..6da0b8a5 100644 --- a/pkg/validation-webhook/webhook.go +++ b/pkg/validation-webhook/webhook.go @@ -26,7 +26,8 @@ import ( "os" clientset "github.com/kubernetes-csi/external-snapshotter/client/v6/clientset/versioned" - storagelisters "github.com/kubernetes-csi/external-snapshotter/client/v6/listers/volumesnapshot/v1" + groupsnapshotlisters "github.com/kubernetes-csi/external-snapshotter/client/v6/listers/volumegroupsnapshot/v1alpha1" + snapshotlisters "github.com/kubernetes-csi/external-snapshotter/client/v6/listers/volumesnapshot/v1" "github.com/spf13/cobra" informers "github.com/kubernetes-csi/external-snapshotter/client/v6/informers/externalversions" @@ -183,15 +184,29 @@ func serve(w http.ResponseWriter, r *http.Request, admit admitHandler) { } } -type serveWebhook struct { - lister storagelisters.VolumeSnapshotClassLister +type serveSnapshotWebhook struct { + lister snapshotlisters.VolumeSnapshotClassLister } -func (s serveWebhook) ServeHTTP(w http.ResponseWriter, r *http.Request) { +func (s serveSnapshotWebhook) ServeHTTP(w http.ResponseWriter, r *http.Request) { serve(w, r, newDelegateToV1AdmitHandler(NewSnapshotAdmitter(s.lister))) } -func startServer(ctx context.Context, tlsConfig *tls.Config, cw *CertWatcher, lister storagelisters.VolumeSnapshotClassLister) error { +type serveGroupSnapshotWebhook struct { + lister groupsnapshotlisters.VolumeGroupSnapshotClassLister +} + +func (s serveGroupSnapshotWebhook) ServeHTTP(w http.ResponseWriter, r *http.Request) { + serve(w, r, newDelegateToV1AdmitHandler(NewGroupSnapshotAdmitter(s.lister))) +} + +func startServer( + ctx context.Context, + tlsConfig *tls.Config, + cw *CertWatcher, + vscLister snapshotlisters.VolumeSnapshotClassLister, + vgscLister groupsnapshotlisters.VolumeGroupSnapshotClassLister, +) error { go func() { klog.Info("Starting certificate watcher") if err := cw.Start(ctx); err != nil { @@ -199,13 +214,17 @@ func startServer(ctx context.Context, tlsConfig *tls.Config, cw *CertWatcher, li } }() // Pipe through the informer at some point here. - s := &serveWebhook{ - lister: lister, + snapshotWebhook := serveSnapshotWebhook{ + lister: vscLister, + } + groupSnapshotWebhook := serveGroupSnapshotWebhook{ + lister: vgscLister, } fmt.Println("Starting webhook server") mux := http.NewServeMux() - mux.Handle("/volumesnapshot", s) + mux.Handle("/volumesnapshot", snapshotWebhook) + mux.Handle("/volumegroupsnapshot", groupSnapshotWebhook) mux.HandleFunc("/readyz", func(w http.ResponseWriter, req *http.Request) { w.Write([]byte("ok")) }) srv := &http.Server{ Handler: mux, @@ -247,14 +266,15 @@ func main(cmd *cobra.Command, args []string) { } factory := informers.NewSharedInformerFactory(snapClient, 0) - lister := factory.Snapshot().V1().VolumeSnapshotClasses().Lister() + snapshotLister := factory.Snapshot().V1().VolumeSnapshotClasses().Lister() + groupSnapshotLister := factory.Groupsnapshot().V1alpha1().VolumeGroupSnapshotClasses().Lister() // Start the informers factory.Start(ctx.Done()) // wait for the caches to sync factory.WaitForCacheSync(ctx.Done()) - if err := startServer(ctx, tlsConfig, cw, lister); err != nil { + if err := startServer(ctx, tlsConfig, cw, snapshotLister, groupSnapshotLister); err != nil { klog.Fatalf("server stopped: %v", err) } } diff --git a/pkg/validation-webhook/webhook_test.go b/pkg/validation-webhook/webhook_test.go index 552fc03c..daba532c 100644 --- a/pkg/validation-webhook/webhook_test.go +++ b/pkg/validation-webhook/webhook_test.go @@ -32,7 +32,10 @@ func TestWebhookCertReload(t *testing.T) { t.Errorf("unexpected error occurred while deleting certs: %v", err) } }() - generateTestCertKeyPair(t, certFile, keyFile) + err = generateTestCertKeyPair(t, certFile, keyFile) + if err != nil { + t.Errorf("unexpected error occurred while generating test certs: %v", err) + } // Start test server ctx, cancel := context.WithCancel(context.Background()) @@ -45,7 +48,12 @@ func TestWebhookCertReload(t *testing.T) { GetCertificate: cw.GetCertificate, } go func() { - if err := startServer(ctx, tlsConfig, cw, &fakeSnapshotLister{}); err != nil { + err := startServer(ctx, + tlsConfig, + cw, + &fakeSnapshotLister{}, + &fakeGroupSnapshotLister{}) + if err != nil { panic(err) } }() @@ -74,7 +82,10 @@ func TestWebhookCertReload(t *testing.T) { // TC: Certificate should consistently change with a file change for i := 0; i < 5; i++ { // Generate new key/cert - generateTestCertKeyPair(t, certFile, keyFile) + err = generateTestCertKeyPair(t, certFile, keyFile) + if err != nil { + t.Errorf("unexpected error occurred while generating test certs: %v", err) + } // Wait for certwatcher to update time.Sleep(250 * time.Millisecond)