Merge pull request #142 from kubernetes-sigs/allow-upgrades

api: Allow changes to CloudstackMachineTemplate

Author: rejoshed

api/v1beta2/cloudstackmachinetemplate_webhook.go

@@ -17,11 +17,8 @@ limitations under the License.
 package v1beta2
 
 import (
-	"fmt"
-	"reflect"
 	"strings"
 
-	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	"sigs.k8s.io/cluster-api-provider-cloudstack/pkg/webhookutil"
@@ -79,30 +76,7 @@ func (r *CloudStackMachineTemplate) ValidateCreate() error {
 
 // ValidateUpdate implements webhook.Validator so a webhook will be registered for the type
 func (r *CloudStackMachineTemplate) ValidateUpdate(old runtime.Object) error {
-	cloudstackmachinetemplatelog.V(1).Info("entered validate update webhook", "api resource name", r.Name)
-
-	oldMachineTemplate, ok := old.(*CloudStackMachineTemplate)
-	if !ok {
-		return errors.NewBadRequest(fmt.Sprintf("expected a CloudStackMachineTemplate but got a %T", old))
-	}
-
-	// CloudStackMachineTemplateSpec.CloudStackMachineTemplateResource.CloudStackMachineSpec
-	spec := r.Spec.Spec.Spec
-	oldSpec := oldMachineTemplate.Spec.Spec.Spec
-
-	errorList := field.ErrorList(nil)
-	errorList = webhookutil.EnsureBothFieldsAreEqual(spec.Offering.ID, spec.Offering.Name, oldSpec.Offering.ID, oldSpec.Offering.Name, "offering", errorList)
-	errorList = webhookutil.EnsureBothFieldsAreEqual(spec.DiskOffering.ID, spec.DiskOffering.Name, oldSpec.DiskOffering.ID, oldSpec.DiskOffering.Name, "diskOffering", errorList)
-	errorList = webhookutil.EnsureStringFieldsAreEqual(spec.SSHKey, oldSpec.SSHKey, "sshkey", errorList)
-	errorList = webhookutil.EnsureBothFieldsAreEqual(spec.Template.ID, spec.Template.Name, oldSpec.Template.ID, oldSpec.Template.Name, "template", errorList)
-	errorList = webhookutil.EnsureStringStringMapFieldsAreEqual(&spec.Details, &oldSpec.Details, "details", errorList)
-	errorList = webhookutil.EnsureStringFieldsAreEqual(spec.Affinity, oldSpec.Affinity, "affinity", errorList)
-
-	if !reflect.DeepEqual(spec.AffinityGroupIDs, oldSpec.AffinityGroupIDs) { // Equivalent to other Ensure funcs.
-		errorList = append(errorList, field.Forbidden(field.NewPath("spec", "AffinityGroupIDs"), "AffinityGroupIDs"))
-	}
-
-	return webhookutil.AggregateObjErrors(r.GroupVersionKind().GroupKind(), r.Name, errorList)
+	return r.ValidateCreate()
 }
 
 // ValidateDelete implements webhook.Validator so a webhook will be registered for the type

api/v1beta2/cloudstackmachinetemplate_webhook_test.go

@@ -28,7 +28,6 @@ import (
 
 var _ = Describe("CloudStackMachineTemplate webhook", func() {
 	var ctx context.Context
-	forbiddenRegex := "admission webhook.*denied the request.*Forbidden\\: %s"
 	requiredRegex := "admission webhook.*denied the request.*Required value\\: %s"
 
 	BeforeEach(func() { // Reset test vars to initial state.
@@ -62,39 +61,4 @@ var _ = Describe("CloudStackMachineTemplate webhook", func() {
 		})
 	})
 
-	Context("When updating a CloudStackMachineTemplate", func() {
-		BeforeEach(func() { // Reset test vars to initial state.
-			Ω(k8sClient.Create(ctx, dummies.CSMachineTemplate1)).Should(Succeed())
-		})
-
-		It("should reject VM template updates to the CloudStackMachineTemplate", func() {
-			dummies.CSMachineTemplate1.Spec.Spec.Spec.Template = infrav1.CloudStackResourceIdentifier{Name: "ArbitraryUpdateTemplate"}
-			Ω(k8sClient.Update(ctx, dummies.CSMachineTemplate1)).
-				Should(MatchError(MatchRegexp(forbiddenRegex, "template")))
-		})
-
-		It("should reject VM disk offering updates to the CloudStackMachineTemplate", func() {
-			dummies.CSMachineTemplate1.Spec.Spec.Spec.DiskOffering = infrav1.CloudStackResourceDiskOffering{
-				CloudStackResourceIdentifier: infrav1.CloudStackResourceIdentifier{Name: "DiskOffering2"}}
-			Ω(k8sClient.Update(ctx, dummies.CSMachineTemplate1)).
-				Should(MatchError(MatchRegexp(forbiddenRegex, "diskOffering")))
-		})
-
-		It("should reject VM offering updates to the CloudStackMachineTemplate", func() {
-			dummies.CSMachineTemplate1.Spec.Spec.Spec.Offering = infrav1.CloudStackResourceIdentifier{Name: "Offering2"}
-			Ω(k8sClient.Update(ctx, dummies.CSMachineTemplate1)).
-				Should(MatchError(MatchRegexp(forbiddenRegex, "offering")))
-		})
-
-		It("should reject updates to VM details of the CloudStackMachineTemplate", func() {
-			dummies.CSMachineTemplate1.Spec.Spec.Spec.Details = map[string]string{"memoryOvercommitRatio": "1.5"}
-			Ω(k8sClient.Update(ctx, dummies.CSMachineTemplate1)).
-				Should(MatchError(MatchRegexp(forbiddenRegex, "details")))
-		})
-
-		It("should reject updates to the list of AffinityGroupIDs of the CloudStackMachineTemplate", func() {
-			dummies.CSMachineTemplate1.Spec.Spec.Spec.AffinityGroupIDs = []string{"28b907b8-75a7-4214-bd3d-6c61961fc2ag"}
-			Ω(k8sClient.Update(ctx, dummies.CSMachineTemplate1)).ShouldNot(Succeed())
-		})
-	})
 })