Skip to content

Commit

Permalink
Merge pull request kubevirt#12666 from orelmisan/refactor-migration-u…
Browse files Browse the repository at this point in the history 
…pdate-admitter

Migration update admitter: Refactor tests
  • Loading branch information
@kubevirt-bot
kubevirt-bot authored Sep 6, 2024
2 parents d5d61a3 + b1c688a commit 4571d0b
Show file tree
Hide file tree
Showing 2 changed files with 111 additions and 86 deletions.
5 changes: 2 additions & 3 deletions pkg/virt-api/webhooks/validating-webhook/admitters/migration-update-admitter.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (
v1 "kubevirt.io/api/core/v1"

webhookutils "kubevirt.io/kubevirt/pkg/util/webhooks"
validating_webhooks "kubevirt.io/kubevirt/pkg/util/webhooks/validating-webhooks"
)

type MigrationUpdateAdmitter struct {
Expand Down Expand Up @@ -89,7 +90,5 @@ func (admitter *MigrationUpdateAdmitter) Admit(_ context.Context, ar *admissionv
return webhookutils.ToAdmissionResponse(causes)
}

reviewResponse := admissionv1.AdmissionResponse{}
reviewResponse.Allowed = true
return &reviewResponse
return validating_webhooks.NewPassingAdmissionResponse()
}
192 changes: 109 additions & 83 deletions pkg/virt-api/webhooks/validating-webhook/admitters/migration-update-admitter_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,30 +17,32 @@
*
*/

package admitters
package admitters_test

import (
"context"
"encoding/json"
"net/http"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"

admissionv1 "k8s.io/api/admission/v1"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"

"kubevirt.io/client-go/api"

v1 "kubevirt.io/api/core/v1"

"kubevirt.io/client-go/api"

"kubevirt.io/kubevirt/pkg/virt-api/webhooks"
"kubevirt.io/kubevirt/pkg/virt-api/webhooks/validating-webhook/admitters"
)

var _ = Describe("Validating MigrationUpdate Admitter", func() {
migrationUpdateAdmitter := &MigrationUpdateAdmitter{}

It("should reject Migration on update if spec changes", func() {
migration := v1.VirtualMachineInstanceMigration{
migration := &v1.VirtualMachineInstanceMigration{
ObjectMeta: metav1.ObjectMeta{
Name: "somemigrationthatchanged",
Namespace: "default",
Expand All @@ -50,31 +52,38 @@ var _ = Describe("Validating MigrationUpdate Admitter", func() {
VMIName: "testmigratevmiupdate",
},
}
oldMigrationBytes, _ := json.Marshal(&migration)

newMigration := migration.DeepCopy()
newMigration.Spec.VMIName = "somethingelse"
newMigrationBytes, _ := json.Marshal(&newMigration)

ar := &admissionv1.AdmissionReview{
Request: &admissionv1.AdmissionRequest{
Resource: webhooks.MigrationGroupVersionResource,
Object: runtime.RawExtension{
Raw: newMigrationBytes,
ar, err := newAdmissionReviewForVMIMUpdate(migration, newMigration)
Expect(err).ToNot(HaveOccurred())

admitter := &admitters.MigrationUpdateAdmitter{}
resp := admitter.Admit(context.Background(), ar)

expectedResponse := &admissionv1.AdmissionResponse{
Allowed: false,
Result: &metav1.Status{
Code: http.StatusUnprocessableEntity,
Message: "update of Migration object's spec is restricted",
Reason: metav1.StatusReasonInvalid,
Details: &metav1.StatusDetails{
Causes: []metav1.StatusCause{
{
Type: metav1.CauseTypeFieldValueNotSupported,
Message: "update of Migration object's spec is restricted",
},
},
},
OldObject: runtime.RawExtension{
Raw: oldMigrationBytes,
},
Operation: admissionv1.Update,
},
}

resp := migrationUpdateAdmitter.Admit(context.Background(), ar)
Expect(resp.Allowed).To(BeFalse())
Expect(resp).To(Equal(expectedResponse))
})

It("should accept Migration on update if spec doesn't change", func() {
migration := v1.VirtualMachineInstanceMigration{
migration := &v1.VirtualMachineInstanceMigration{
ObjectMeta: metav1.ObjectMeta{
Name: "somemigration",
Namespace: "default",
Expand All @@ -85,29 +94,20 @@ var _ = Describe("Validating MigrationUpdate Admitter", func() {
},
}

migrationBytes, _ := json.Marshal(&migration)
newMigration := migration.DeepCopy()

ar := &admissionv1.AdmissionReview{
Request: &admissionv1.AdmissionRequest{
Resource: webhooks.MigrationGroupVersionResource,
Object: runtime.RawExtension{
Raw: migrationBytes,
},
OldObject: runtime.RawExtension{
Raw: migrationBytes,
},
Operation: admissionv1.Update,
},
}
ar, err := newAdmissionReviewForVMIMUpdate(migration, newMigration)
Expect(err).ToNot(HaveOccurred())

resp := migrationUpdateAdmitter.Admit(context.Background(), ar)
Expect(resp.Allowed).To(BeTrue())
admitter := &admitters.MigrationUpdateAdmitter{}
resp := admitter.Admit(context.Background(), ar)
Expect(resp).To(Equal(allowedAdmissionResponse()))
})

It("should reject Migration on update if labels include our selector and are removed", func() {
vmi := api.NewMinimalVMI("testmigratevmiupdate-labelsremoved")

migration := v1.VirtualMachineInstanceMigration{
migration := &v1.VirtualMachineInstanceMigration{
ObjectMeta: metav1.ObjectMeta{
Name: "somemigration",
Namespace: "default",
Expand All @@ -122,33 +122,39 @@ var _ = Describe("Validating MigrationUpdate Admitter", func() {
},
}

oldMigrationBytes, _ := json.Marshal(&migration)

newMigration := migration.DeepCopy()
newMigration.Labels = nil
newMigrationBytes, _ := json.Marshal(&newMigration)

ar := &admissionv1.AdmissionReview{
Request: &admissionv1.AdmissionRequest{
Resource: webhooks.MigrationGroupVersionResource,
Object: runtime.RawExtension{
Raw: newMigrationBytes,
},
OldObject: runtime.RawExtension{
Raw: oldMigrationBytes,
ar, err := newAdmissionReviewForVMIMUpdate(migration, newMigration)
Expect(err).ToNot(HaveOccurred())

admitter := &admitters.MigrationUpdateAdmitter{}
resp := admitter.Admit(context.Background(), ar)

expectedResponse := &admissionv1.AdmissionResponse{
Allowed: false,
Result: &metav1.Status{
Code: http.StatusUnprocessableEntity,
Message: "selector label can't be removed from an in-flight migration",
Reason: metav1.StatusReasonInvalid,
Details: &metav1.StatusDetails{
Causes: []metav1.StatusCause{
{
Type: metav1.CauseTypeFieldValueNotSupported,
Message: "selector label can't be removed from an in-flight migration",
},
},
},
Operation: admissionv1.Update,
},
}

resp := migrationUpdateAdmitter.Admit(context.Background(), ar)
Expect(resp.Allowed).To(BeFalse())
Expect(resp).To(Equal(expectedResponse))
})

It("should reject Migration on update if our selector label is removed", func() {
vmi := api.NewMinimalVMI("testmigratevmiupdate-selectorremoved")

migration := v1.VirtualMachineInstanceMigration{
migration := &v1.VirtualMachineInstanceMigration{
ObjectMeta: metav1.ObjectMeta{
Name: "somemigration",
Namespace: "default",
Expand All @@ -163,33 +169,39 @@ var _ = Describe("Validating MigrationUpdate Admitter", func() {
},
}

oldMigrationBytes, _ := json.Marshal(&migration)

newMigration := migration.DeepCopy()
delete(newMigration.Labels, v1.MigrationSelectorLabel)
newMigrationBytes, _ := json.Marshal(&newMigration)

ar := &admissionv1.AdmissionReview{
Request: &admissionv1.AdmissionRequest{
Resource: webhooks.MigrationGroupVersionResource,
Object: runtime.RawExtension{
Raw: newMigrationBytes,
},
OldObject: runtime.RawExtension{
Raw: oldMigrationBytes,
ar, err := newAdmissionReviewForVMIMUpdate(migration, newMigration)
Expect(err).ToNot(HaveOccurred())

admitter := &admitters.MigrationUpdateAdmitter{}
resp := admitter.Admit(context.Background(), ar)

expectedResponse := &admissionv1.AdmissionResponse{
Allowed: false,
Result: &metav1.Status{
Code: http.StatusUnprocessableEntity,
Message: "selector label can't be modified on an in-flight migration",
Reason: metav1.StatusReasonInvalid,
Details: &metav1.StatusDetails{
Causes: []metav1.StatusCause{
{
Type: metav1.CauseTypeFieldValueNotSupported,
Message: "selector label can't be modified on an in-flight migration",
},
},
},
Operation: admissionv1.Update,
},
}

resp := migrationUpdateAdmitter.Admit(context.Background(), ar)
Expect(resp.Allowed).To(BeFalse())
Expect(resp).To(Equal(expectedResponse))
})

It("should accept Migration on update if non-selector label is removed", func() {
vmi := api.NewMinimalVMI("testmigratevmiupdate-otherremoved")

migration := v1.VirtualMachineInstanceMigration{
migration := &v1.VirtualMachineInstanceMigration{
ObjectMeta: metav1.ObjectMeta{
Name: "somemigration",
Namespace: "default",
Expand All @@ -204,26 +216,40 @@ var _ = Describe("Validating MigrationUpdate Admitter", func() {
},
}

oldMigrationBytes, _ := json.Marshal(&migration)

newMigration := migration.DeepCopy()
delete(newMigration.Labels, "someOtherLabel")
newMigrationBytes, _ := json.Marshal(&newMigration)

ar := &admissionv1.AdmissionReview{
Request: &admissionv1.AdmissionRequest{
Resource: webhooks.MigrationGroupVersionResource,
Object: runtime.RawExtension{
Raw: newMigrationBytes,
},
OldObject: runtime.RawExtension{
Raw: oldMigrationBytes,
},
Operation: admissionv1.Update,
},
}
ar, err := newAdmissionReviewForVMIMUpdate(migration, newMigration)
Expect(err).ToNot(HaveOccurred())

admitter := &admitters.MigrationUpdateAdmitter{}
resp := admitter.Admit(context.Background(), ar)

resp := migrationUpdateAdmitter.Admit(context.Background(), ar)
Expect(resp.Allowed).To(BeTrue())
Expect(resp).To(Equal(allowedAdmissionResponse()))
})
})

func newAdmissionReviewForVMIMUpdate(oldMigration, newMigration *v1.VirtualMachineInstanceMigration) (*admissionv1.AdmissionReview, error) {
oldMigrationBytes, err := json.Marshal(oldMigration)
if err != nil {
return nil, err
}

newMigrationBytes, err := json.Marshal(newMigration)
if err != nil {
return nil, err
}

return &admissionv1.AdmissionReview{
Request: &admissionv1.AdmissionRequest{
Resource: webhooks.MigrationGroupVersionResource,
Object: runtime.RawExtension{
Raw: newMigrationBytes,
},
OldObject: runtime.RawExtension{
Raw: oldMigrationBytes,
},
Operation: admissionv1.Update,
},
}, nil
}

0 comments on commit 4571d0b

Please sign in to comment.