Check name mismatch when ID and Name both provided for template and offering

Author: wanyufe

pkg/cloud/instance.go

@@ -82,14 +82,19 @@ func (c *client) ResolveVMInstanceDetails(csMachine *infrav1.CloudStackMachine)
 
 func (c *client) ResolveServiceOffering(csMachine *infrav1.CloudStackMachine) (offeringID string, retErr error) {
 	if len(csMachine.Spec.Offering.ID) > 0 {
-		_, count, err := c.cs.ServiceOffering.GetServiceOfferingByID(csMachine.Spec.Offering.ID)
+		csOffering, count, err := c.cs.ServiceOffering.GetServiceOfferingByID(csMachine.Spec.Offering.ID)
 		if err != nil {
 			return "", multierror.Append(retErr, errors.Wrapf(
 				err, "could not get Service Offering by ID %s", csMachine.Spec.Offering.ID))
 		} else if count != 1 {
 			return "", multierror.Append(retErr, errors.Errorf(
 				"expected 1 Service Offering with UUID %s, but got %d", csMachine.Spec.Offering.ID, count))
 		}
+
+		if len(csMachine.Spec.Offering.Name) > 0 && csMachine.Spec.Offering.Name != csOffering.Name {
+			return "", multierror.Append(retErr, errors.Errorf(
+				"offering name %s does not match name %s returned using UUID %s", csMachine.Spec.Offering.Name, csOffering.Name, csMachine.Spec.Offering.ID))
+		}
 		return csMachine.Spec.Offering.ID, nil
 	}
 	offeringID, count, err := c.cs.ServiceOffering.GetServiceOfferingID(csMachine.Spec.Offering.Name)
@@ -109,14 +114,19 @@ func (c *client) ResolveTemplate(
 	zoneID string,
 ) (templateID string, retErr error) {
 	if len(csMachine.Spec.Template.ID) > 0 {
-		_, count, err := c.cs.Template.GetTemplateByID(csMachine.Spec.Template.ID, "all")
+		csTemplate, count, err := c.cs.Template.GetTemplateByID(csMachine.Spec.Template.ID, "all")
 		if err != nil {
 			return "", multierror.Append(retErr, errors.Wrapf(
 				err, "could not get Template by ID %s", csMachine.Spec.Template.ID))
 		} else if count != 1 {
 			return "", multierror.Append(retErr, errors.Errorf(
 				"expected 1 Template with UUID %s, but got %d", csMachine.Spec.Template.ID, count))
 		}
+
+		if len(csMachine.Spec.Template.Name) > 0 && csMachine.Spec.Template.Name != csTemplate.Name {
+			return "", multierror.Append(retErr, errors.Errorf(
+				"template name %s does not match name %s returned using UUID %s", csMachine.Spec.Template.Name, csTemplate.Name, csMachine.Spec.Template.ID))
+		}
 		return csMachine.Spec.Template.ID, nil
 	}
 	templateID, count, err := c.cs.Template.GetTemplateID(csMachine.Spec.Template.Name, "all", zoneID)

pkg/cloud/instance_test.go

@@ -248,7 +248,7 @@ var _ = Describe("Instance", func() {
 				dummies.CSMachine1.Spec.Offering.Name = ""
 				dummies.CSMachine1.Spec.Template.Name = "template"
 
-				sos.EXPECT().GetServiceOfferingByID(dummies.CSMachine1.Spec.Offering.ID).Return(&cloudstack.ServiceOffering{}, 1, nil)
+				sos.EXPECT().GetServiceOfferingByID(dummies.CSMachine1.Spec.Offering.ID).Return(&cloudstack.ServiceOffering{Name: ""}, 1, nil)
 				ts.EXPECT().GetTemplateID(dummies.CSMachine1.Spec.Template.Name, allFilter, dummies.Zone1.ID).
 					Return(templateFakeID, 1, nil)
 
@@ -262,7 +262,7 @@ var _ = Describe("Instance", func() {
 				dummies.CSMachine1.Spec.Template.Name = ""
 
 				sos.EXPECT().GetServiceOfferingID(dummies.CSMachine1.Spec.Offering.Name).Return(offeringFakeID, 1, nil)
-				ts.EXPECT().GetTemplateByID(dummies.CSMachine1.Spec.Template.ID, allFilter).Return(&cloudstack.Template{}, 1, nil)
+				ts.EXPECT().GetTemplateByID(dummies.CSMachine1.Spec.Template.ID, allFilter).Return(&cloudstack.Template{Name: ""}, 1, nil)
 
 				ActionAndAssert()
 			})
@@ -273,8 +273,8 @@ var _ = Describe("Instance", func() {
 				dummies.CSMachine1.Spec.Offering.Name = ""
 				dummies.CSMachine1.Spec.Template.Name = ""
 
-				sos.EXPECT().GetServiceOfferingByID(dummies.CSMachine1.Spec.Offering.ID).Return(&cloudstack.ServiceOffering{}, 1, nil)
-				ts.EXPECT().GetTemplateByID(dummies.CSMachine1.Spec.Template.ID, allFilter).Return(&cloudstack.Template{}, 1, nil)
+				sos.EXPECT().GetServiceOfferingByID(dummies.CSMachine1.Spec.Offering.ID).Return(&cloudstack.ServiceOffering{Name: "offering"}, 1, nil)
+				ts.EXPECT().GetTemplateByID(dummies.CSMachine1.Spec.Template.ID, allFilter).Return(&cloudstack.Template{Name: "template"}, 1, nil)
 
 				ActionAndAssert()
 			})
@@ -285,11 +285,43 @@ var _ = Describe("Instance", func() {
 				dummies.CSMachine1.Spec.Offering.Name = "offering"
 				dummies.CSMachine1.Spec.Template.Name = "template"
 
-				sos.EXPECT().GetServiceOfferingByID(dummies.CSMachine1.Spec.Offering.ID).Return(&cloudstack.ServiceOffering{}, 1, nil)
-				ts.EXPECT().GetTemplateByID(dummies.CSMachine1.Spec.Template.ID, allFilter).Return(&cloudstack.Template{}, 1, nil)
+				sos.EXPECT().GetServiceOfferingByID(dummies.CSMachine1.Spec.Offering.ID).Return(&cloudstack.ServiceOffering{Name: "offering"}, 1, nil)
+				ts.EXPECT().GetTemplateByID(dummies.CSMachine1.Spec.Template.ID, allFilter).Return(&cloudstack.Template{Name: "template"}, 1, nil)
 
 				ActionAndAssert()
 			})
 		})
+
+		Context("when using both UUIDs and names to locate service offerings and templates", func() {
+			BeforeEach(func() {
+				vms.EXPECT().GetVirtualMachinesMetricByID(*dummies.CSMachine1.Spec.InstanceID).
+					Return(nil, -1, notFoundError)
+				vms.EXPECT().GetVirtualMachinesMetricByName(dummies.CSMachine1.Name).Return(nil, -1, notFoundError)
+			})
+
+			It("works with Id and name both provided, offering name mismatch", func() {
+				dummies.CSMachine1.Spec.Offering.ID = offeringFakeID
+				dummies.CSMachine1.Spec.Template.ID = templateFakeID
+				dummies.CSMachine1.Spec.Offering.Name = "offering"
+				dummies.CSMachine1.Spec.Template.Name = "template"
+
+				sos.EXPECT().GetServiceOfferingByID(dummies.CSMachine1.Spec.Offering.ID).Return(&cloudstack.ServiceOffering{Name: "offering-not-match"}, 1, nil)
+				requiredRegexp := "offering name %s does not match name %s returned using UUID %s"
+				Ω(client.GetOrCreateVMInstance(dummies.CSMachine1, dummies.CAPIMachine, dummies.CSCluster, "")).Should(MatchError(MatchRegexp(requiredRegexp, dummies.CSMachine1.Spec.Offering.Name, "offering-not-match", offeringFakeID)))
+			})
+
+			It("works with Id and name both provided, template name mismatch", func() {
+				dummies.CSMachine1.Spec.Offering.ID = offeringFakeID
+				dummies.CSMachine1.Spec.Template.ID = templateFakeID
+				dummies.CSMachine1.Spec.Offering.Name = "offering"
+				dummies.CSMachine1.Spec.Template.Name = "template"
+
+				sos.EXPECT().GetServiceOfferingByID(dummies.CSMachine1.Spec.Offering.ID).Return(&cloudstack.ServiceOffering{Name: "offering"}, 1, nil)
+				ts.EXPECT().GetTemplateByID(dummies.CSMachine1.Spec.Template.ID, allFilter).Return(&cloudstack.Template{Name: "template-not-match"}, 1, nil)
+				requiredRegexp := "template name %s does not match name %s returned using UUID %s"
+				Ω(client.GetOrCreateVMInstance(dummies.CSMachine1, dummies.CAPIMachine, dummies.CSCluster, "")).Should(MatchError(MatchRegexp(requiredRegexp, dummies.CSMachine1.Spec.Template.Name, "template-not-match", templateFakeID)))
+
+			})
+		})
 	})
 })