Skip to content

Commit

Permalink
Allow clone when source pvc doesnt exist if target is populated
Browse files Browse the repository at this point in the history 
Signed-off-by: Shelly Kagan <skagan@redhat.com>
  • Loading branch information
@ShellyKa13
ShellyKa13 committed Aug 22, 2022
1 parent fc88ca2 commit 9fb22e0
Show file tree
Hide file tree
Showing 3 changed files with 70 additions and 8 deletions.
2 changes: 1 addition & 1 deletion pkg/apiserver/webhooks/dataimportcron-validate.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ func (wh *dataImportCronValidatingWebhook) validateDataImportCronSpec(request *a
return causes
}

causes = wh.validateDataVolumeSpec(request, k8sfield.NewPath("Template"), &spec.Template.Spec, nil)
causes = wh.validateDataVolumeSpec(request, k8sfield.NewPath("Template"), &spec.Template.Spec, nil, "")
if len(causes) > 0 {
return causes
}
Expand Down
30 changes: 23 additions & 7 deletions pkg/apiserver/webhooks/datavolume-validate.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ func validateContentTypes(sourcePVC *v1.PersistentVolumeClaim, spec *cdiv1.DataV
return sourceContentType == targetContentType, sourceContentType, targetContentType
}

func (wh *dataVolumeValidatingWebhook) validateDataVolumeSpec(request *admissionv1.AdmissionRequest, field *k8sfield.Path, spec *cdiv1.DataVolumeSpec, namespace *string) []metav1.StatusCause {
func (wh *dataVolumeValidatingWebhook) validateDataVolumeSpec(request *admissionv1.AdmissionRequest, field *k8sfield.Path, spec *cdiv1.DataVolumeSpec, namespace *string, name string) []metav1.StatusCause {
var causes []metav1.StatusCause
var url string
var sourceType string
Expand Down Expand Up @@ -163,7 +163,7 @@ func (wh *dataVolumeValidatingWebhook) validateDataVolumeSpec(request *admission
return causes
}
if spec.SourceRef != nil {
cause := wh.validateSourceRef(request, spec, field, namespace)
cause := wh.validateSourceRef(request, spec, field, namespace, name)
if cause != nil {
causes = append(causes, *cause)
}
Expand Down Expand Up @@ -290,7 +290,7 @@ func (wh *dataVolumeValidatingWebhook) validateDataVolumeSpec(request *admission
return causes
}
if request.Operation == admissionv1.Create {
cause := wh.validateDataVolumeSourcePVC(spec.Source.PVC, field.Child("source", "PVC"), spec)
cause := wh.validateDataVolumeSourcePVC(spec.Source.PVC, field.Child("source", "PVC"), spec, namespace, name)
if cause != nil {
causes = append(causes, *cause)
}
Expand Down Expand Up @@ -363,7 +363,7 @@ func validateDataVolumeSourceRegistry(sourceRegistry *cdiv1.DataVolumeSourceRegi
return causes
}

func (wh *dataVolumeValidatingWebhook) validateSourceRef(request *admissionv1.AdmissionRequest, spec *cdiv1.DataVolumeSpec, field *k8sfield.Path, namespace *string) *metav1.StatusCause {
func (wh *dataVolumeValidatingWebhook) validateSourceRef(request *admissionv1.AdmissionRequest, spec *cdiv1.DataVolumeSpec, field *k8sfield.Path, namespace *string, name string) *metav1.StatusCause {
if spec.SourceRef.Kind == "" {
return &metav1.StatusCause{
Type: metav1.CauseTypeFieldValueInvalid,
Expand Down Expand Up @@ -406,13 +406,29 @@ func (wh *dataVolumeValidatingWebhook) validateSourceRef(request *admissionv1.Ad
Field: field.Child("sourceRef").String(),
}
}
return wh.validateDataVolumeSourcePVC(dataSource.Spec.Source.PVC, field.Child("sourceRef"), spec)
return wh.validateDataVolumeSourcePVC(dataSource.Spec.Source.PVC, field.Child("sourceRef"), spec, namespace, name)
}

func (wh *dataVolumeValidatingWebhook) validateDataVolumeSourcePVC(PVC *cdiv1.DataVolumeSourcePVC, field *k8sfield.Path, spec *cdiv1.DataVolumeSpec) *metav1.StatusCause {
func (wh *dataVolumeValidatingWebhook) validateDataVolumeSourcePVC(PVC *cdiv1.DataVolumeSourcePVC, field *k8sfield.Path, spec *cdiv1.DataVolumeSpec, namespace *string, name string) *metav1.StatusCause {
sourcePVC, err := wh.k8sClient.CoreV1().PersistentVolumeClaims(PVC.Namespace).Get(context.TODO(), PVC.Name, metav1.GetOptions{})
if err != nil {
if k8serrors.IsNotFound(err) {
if namespace != nil {
// If the target PVC is already populated we don't need the source PVC to exists
targetPVC, err := wh.k8sClient.CoreV1().PersistentVolumeClaims(*namespace).Get(context.TODO(), name, metav1.GetOptions{})
if err == nil {
populatedFor, ok := targetPVC.Annotations[controller.AnnPopulatedFor]
if ok && populatedFor == name {
return nil
}
} else if !k8serrors.IsNotFound(err) {
return &metav1.StatusCause{
Message: err.Error(),
Field: field.String(),
}
}
}

return &metav1.StatusCause{
Type: metav1.CauseTypeFieldValueNotFound,
Message: fmt.Sprintf("Source PVC %s/%s not found", PVC.Namespace, PVC.Name),
Expand Down Expand Up @@ -541,7 +557,7 @@ func (wh *dataVolumeValidatingWebhook) Admit(ar admissionv1.AdmissionReview) *ad
}
}

causes = wh.validateDataVolumeSpec(ar.Request, k8sfield.NewPath("spec"), &dv.Spec, &dv.Namespace)
causes = wh.validateDataVolumeSpec(ar.Request, k8sfield.NewPath("spec"), &dv.Spec, &dv.Namespace, dv.Name)
if len(causes) > 0 {
klog.Infof("rejected DataVolume admission %s", causes)
return toRejectedAdmissionResponse(causes)
Expand Down
46 changes: 46 additions & 0 deletions pkg/apiserver/webhooks/datavolume-validate_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,22 @@ var _ = Describe("Validating Webhook", func() {
Expect(resp.Allowed).To(Equal(false))
})

It("should accept DataVolume with PVC source on create if source PVC does not exist, but target pvc exists and populated", func() {
dataVolume := newPVCDataVolume("testDV", "testNamespace", "test")
targetPVC := &corev1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
Name: dataVolume.Name,
Namespace: dataVolume.Namespace,
Annotations: map[string]string{
"cdi.kubevirt.io/storage.populatedFor": dataVolume.Name,
},
},
Spec: *dataVolume.Spec.PVC,
}
resp := validateDataVolumeCreate(dataVolume, targetPVC)
Expect(resp.Allowed).To(Equal(true))
})

It("should reject invalid DataVolume source PVC namespace on create", func() {
dataVolume := newPVCDataVolume("testDV", "", "test")
resp := validateDataVolumeCreate(dataVolume)
Expand Down Expand Up @@ -505,6 +521,36 @@ var _ = Describe("Validating Webhook", func() {
Expect(resp.Allowed).To(Equal(false))
})

It("should accept DataVolume with SourceRef on create if DataSource exists, source PVC does not exist, but target pvc exists and populated", func() {
dataVolume := newDataSourceDataVolume("testDV", &testNamespace, "test")
dataSource := &cdiv1.DataSource{
ObjectMeta: metav1.ObjectMeta{
Name: dataVolume.Spec.SourceRef.Name,
Namespace: testNamespace,
},
Spec: cdiv1.DataSourceSpec{
Source: cdiv1.DataSourceSource{
PVC: &cdiv1.DataVolumeSourcePVC{
Name: "testPVC",
Namespace: testNamespace,
},
},
},
}
targetPVC := &corev1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
Name: dataVolume.Name,
Namespace: dataVolume.Namespace,
Annotations: map[string]string{
"cdi.kubevirt.io/storage.populatedFor": dataVolume.Name,
},
},
Spec: *dataVolume.Spec.PVC,
}
resp := validateDataVolumeCreateEx(dataVolume, []runtime.Object{targetPVC}, []runtime.Object{dataSource})
Expect(resp.Allowed).To(Equal(true))
})

It("should reject DataVolume with empty SourceRef name on create", func() {
dataVolume := newDataSourceDataVolume("testDV", &testNamespace, "")
resp := validateDataVolumeCreate(dataVolume)
Expand Down

0 comments on commit 9fb22e0

Please sign in to comment.