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

Revert "api: Allow changes to CloudstackMachineTemplate"

Author: rejoshed

api/v1beta2/cloudstackmachinetemplate_webhook.go

@@ -17,8 +17,11 @@ 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"
@@ -76,7 +79,30 @@ 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 {
-	return r.ValidateCreate()
+	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)
 }
 
 // ValidateDelete implements webhook.Validator so a webhook will be registered for the type

api/v1beta2/cloudstackmachinetemplate_webhook_test.go

@@ -28,6 +28,7 @@ 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.
@@ -61,4 +62,39 @@ 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())
+		})
+	})
 })