Skip to content

Commit f0cd053

Browse files
committed
verifyUpdatePrevented function updated -
Takes the machine object directly (no need to fetch by name) Uses komega.Update for consistent async behavior Accepts variable error substrings for flexible validation
1 parent fef0721 commit f0cd053

File tree

1 file changed

+25
-32
lines changed

1 file changed

+25
-32
lines changed

e2e/machine_migration_test.go

Lines changed: 25 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -272,12 +272,11 @@ var _ = Describe("[sig-cluster-lifecycle][OCPFeatureGate:MachineAPIMigration] Ma
272272
Eventually(komega.Update(testMachine, func() {
273273
testMachine.Spec.AuthoritativeAPI = machinev1beta1.MachineAuthorityClusterAPI
274274
}), capiframework.WaitMedium, capiframework.RetryMedium).Should(Succeed(), "Should allow modification of authoritativeAPI")
275-
// Clean up the test machine
276-
// Clean up the test machine
277275

276+
// Clean up the test machine
278277
DeferCleanup(func() {
279278
By("Cleaning up test machine")
280-
// Try to delete the MAPI machine first
279+
// Delete the MAPI machine
281280
if testMachine != nil {
282281
mapiframework.DeleteMachines(ctx, cl, testMachine)
283282
}
@@ -312,33 +311,33 @@ var _ = Describe("[sig-cluster-lifecycle][OCPFeatureGate:MachineAPIMigration] Ma
312311
})
313312

314313
It("should prevent modification of InstanceType in non-authoritative MAPI Machine", func() {
315-
verifyFieldModificationPrevented(cl, testMapiMachine.Name, "InstanceType", func(machine *machinev1beta1.Machine) {
314+
verifyFieldModificationPrevented(testMapiMachine, "InstanceType", func(machine *machinev1beta1.Machine) {
316315
Expect(updateAWSMachineProviderSpec(machine, func(providerSpec *machinev1beta1.AWSMachineProviderConfig) {
317316
providerSpec.InstanceType = testInstanceType
318317
})).To(Succeed(), "Expected failure attempting to update InstanceType in provider spec")
319318
})
320319
})
321320

322321
It("should prevent removal of labels in non-authoritative MAPI Machine", func() {
323-
verifyFieldModificationPrevented(cl, testMapiMachine.Name, "labels", func(machine *machinev1beta1.Machine) {
322+
verifyFieldModificationPrevented(testMapiMachine, "labels", func(machine *machinev1beta1.Machine) {
324323
machine.Labels = nil
325324
})
326325
})
327326

328327
It("should prevent addition of annotations machine.openshift.io. in non-authoritative MAPI Machine", func() {
329-
verifyAnnotationModificationPrevented(cl, testMapiMachine.Name)
328+
verifyAnnotationModificationPrevented(testMapiMachine)
330329
})
331330

332331
It("should prevent modification of AMI ID in non-authoritative MAPI Machine", func() {
333-
verifyFieldModificationPrevented(cl, testMapiMachine.Name, "AMI ID", func(machine *machinev1beta1.Machine) {
332+
verifyFieldModificationPrevented(testMapiMachine, "AMI ID", func(machine *machinev1beta1.Machine) {
334333
Expect(updateAWSMachineProviderSpec(machine, func(providerSpec *machinev1beta1.AWSMachineProviderConfig) {
335334
providerSpec.AMI.ID = ptr.To(testAMIID)
336335
})).To(Succeed(), "Expected failure attempting to update AMI ID in provider spec")
337336
})
338337
})
339338

340339
It("should prevent modification of encryption for block devices in non-authoritative MAPI Machine", func() {
341-
verifyFieldModificationPrevented(cl, testMapiMachine.Name, "block device encryption", func(machine *machinev1beta1.Machine) {
340+
verifyFieldModificationPrevented(testMapiMachine, "block device encryption", func(machine *machinev1beta1.Machine) {
342341
Expect(updateAWSMachineProviderSpec(machine, func(providerSpec *machinev1beta1.AWSMachineProviderConfig) {
343342
if len(providerSpec.BlockDevices) > 0 && providerSpec.BlockDevices[0].EBS != nil {
344343
providerSpec.BlockDevices[0].EBS.Encrypted = ptr.To(false)
@@ -348,7 +347,7 @@ var _ = Describe("[sig-cluster-lifecycle][OCPFeatureGate:MachineAPIMigration] Ma
348347
})
349348

350349
It("should prevent modification of VolumeSize in non-authoritative MAPI Machine", func() {
351-
verifyFieldModificationPrevented(cl, testMapiMachine.Name, "VolumeSize", func(machine *machinev1beta1.Machine) {
350+
verifyFieldModificationPrevented(testMapiMachine, "VolumeSize", func(machine *machinev1beta1.Machine) {
352351
Expect(updateAWSMachineProviderSpec(machine, func(providerSpec *machinev1beta1.AWSMachineProviderConfig) {
353352
if len(providerSpec.BlockDevices) > 0 && providerSpec.BlockDevices[0].EBS != nil {
354353
providerSpec.BlockDevices[0].EBS.VolumeSize = ptr.To(testVolumeSize)
@@ -358,7 +357,7 @@ var _ = Describe("[sig-cluster-lifecycle][OCPFeatureGate:MachineAPIMigration] Ma
358357
})
359358

360359
It("should prevent modification of VolumeType in non-authoritative MAPI Machine", func() {
361-
verifyFieldModificationPrevented(cl, testMapiMachine.Name, "VolumeType", func(machine *machinev1beta1.Machine) {
360+
verifyFieldModificationPrevented(testMapiMachine, "VolumeType", func(machine *machinev1beta1.Machine) {
362361
Expect(updateAWSMachineProviderSpec(machine, func(providerSpec *machinev1beta1.AWSMachineProviderConfig) {
363362
if len(providerSpec.BlockDevices) > 0 && providerSpec.BlockDevices[0].EBS != nil {
364363
providerSpec.BlockDevices[0].EBS.VolumeType = ptr.To(testVolumeType)
@@ -368,23 +367,23 @@ var _ = Describe("[sig-cluster-lifecycle][OCPFeatureGate:MachineAPIMigration] Ma
368367
})
369368

370369
It("should prevent modification of AvailabilityZone in non-authoritative MAPI Machine", func() {
371-
verifyFieldModificationPrevented(cl, testMapiMachine.Name, "AvailabilityZone", func(machine *machinev1beta1.Machine) {
370+
verifyFieldModificationPrevented(testMapiMachine, "AvailabilityZone", func(machine *machinev1beta1.Machine) {
372371
Expect(updateAWSMachineProviderSpec(machine, func(providerSpec *machinev1beta1.AWSMachineProviderConfig) {
373372
providerSpec.Placement.AvailabilityZone = testAvailabilityZone
374373
})).To(Succeed(), "Expected failure attempting to update AvailabilityZone in provider spec")
375374
})
376375
})
377376

378377
It("should prevent modification of Subnet in non-authoritative MAPI Machine", func() {
379-
verifyFieldModificationPrevented(cl, testMapiMachine.Name, "Subnet", func(machine *machinev1beta1.Machine) {
378+
verifyFieldModificationPrevented(testMapiMachine, "Subnet", func(machine *machinev1beta1.Machine) {
380379
Expect(updateAWSMachineProviderSpec(machine, func(providerSpec *machinev1beta1.AWSMachineProviderConfig) {
381380
providerSpec.Subnet.ID = ptr.To(testSubnetID)
382381
})).To(Succeed(), "Expected failure attempting to update Subnet in provider spec")
383382
})
384383
})
385384

386385
It("should prevent modification of SecurityGroups in non-authoritative MAPI Machine", func() {
387-
verifyFieldModificationPrevented(cl, testMapiMachine.Name, "SecurityGroups", func(machine *machinev1beta1.Machine) {
386+
verifyFieldModificationPrevented(testMapiMachine, "SecurityGroups", func(machine *machinev1beta1.Machine) {
388387
Expect(updateAWSMachineProviderSpec(machine, func(providerSpec *machinev1beta1.AWSMachineProviderConfig) {
389388
providerSpec.SecurityGroups = append(providerSpec.SecurityGroups, machinev1beta1.AWSResourceReference{
390389
ID: ptr.To(testSecurityGroupID),
@@ -394,7 +393,7 @@ var _ = Describe("[sig-cluster-lifecycle][OCPFeatureGate:MachineAPIMigration] Ma
394393
})
395394

396395
It("should prevent modification of Tags in non-authoritative MAPI Machine", func() {
397-
verifyFieldModificationPrevented(cl, testMapiMachine.Name, "Tags", func(machine *machinev1beta1.Machine) {
396+
verifyFieldModificationPrevented(testMapiMachine, "Tags", func(machine *machinev1beta1.Machine) {
398397
Expect(updateAWSMachineProviderSpec(machine, func(providerSpec *machinev1beta1.AWSMachineProviderConfig) {
399398
providerSpec.Tags = append(providerSpec.Tags, machinev1beta1.TagSpecification{
400399
Name: testTagName,
@@ -405,7 +404,7 @@ var _ = Describe("[sig-cluster-lifecycle][OCPFeatureGate:MachineAPIMigration] Ma
405404
})
406405

407406
It("should prevent modification of capacityReservationId in non-authoritative MAPI Machine", func() {
408-
verifyFieldModificationPrevented(cl, testMapiMachine.Name, "capacityReservationId", func(machine *machinev1beta1.Machine) {
407+
verifyFieldModificationPrevented(testMapiMachine, "capacityReservationId", func(machine *machinev1beta1.Machine) {
409408
Expect(updateAWSMachineProviderSpec(machine, func(providerSpec *machinev1beta1.AWSMachineProviderConfig) {
410409
providerSpec.CapacityReservationID = testCapacityReservationID
411410
})).To(Succeed(), "Expected failure attempting to update capacityReservationId in provider spec")
@@ -699,33 +698,27 @@ func updateAWSMachineProviderSpec(machine *machinev1beta1.Machine, updateFunc fu
699698
machine.Spec.ProviderSpec.Value.Raw = rawProviderSpec
700699
return nil
701700
}
702-
func verifyUpdatePrevented(cl client.Client, machineName string, description string, updateFunc func(*machinev1beta1.Machine), expectedErrorSubstrings ...string) {
701+
func verifyUpdatePrevented(machine *machinev1beta1.Machine, description string, updateFunc func(*machinev1beta1.Machine), expectedErrorSubstrings ...string) {
703702
By(fmt.Sprintf("Attempting to %s in MAPI Machine", description))
704703

705-
currentMachine, err := mapiframework.GetMachine(cl, machineName)
706-
Expect(err).NotTo(HaveOccurred(), "Failed to get current machine")
707-
updatedMachine := currentMachine.DeepCopy()
708-
updateFunc(updatedMachine)
709-
710-
err = cl.Update(ctx, updatedMachine)
711-
Expect(err).To(HaveOccurred())
712-
713-
// Check for the default VAP error if no specific errors provided
704+
// Set default VAP error if no specific errors provided
714705
if len(expectedErrorSubstrings) == 0 {
715706
expectedErrorSubstrings = []string{"ValidatingAdmissionPolicy 'machine-api-machine-vap' with binding 'machine-api-machine-vap' denied request"}
716707
}
717708

718-
for _, expectedSubstring := range expectedErrorSubstrings {
719-
Expect(err.Error()).To(ContainSubstring(expectedSubstring))
720-
}
709+
Eventually(komega.Update(machine, func() {
710+
updateFunc(machine)
711+
}), capiframework.WaitMedium, capiframework.RetryMedium).Should(
712+
MatchError(ContainSubstring(expectedErrorSubstrings[0])),
713+
)
721714
}
722715

723-
func verifyFieldModificationPrevented(cl client.Client, machineName string, fieldName string, modifyFunc func(*machinev1beta1.Machine)) {
724-
verifyUpdatePrevented(cl, machineName, fmt.Sprintf("modify %s", fieldName), modifyFunc)
716+
func verifyFieldModificationPrevented(machine *machinev1beta1.Machine, fieldName string, modifyFunc func(*machinev1beta1.Machine)) {
717+
verifyUpdatePrevented(machine, fmt.Sprintf("modify %s", fieldName), modifyFunc)
725718
}
726719

727-
func verifyAnnotationModificationPrevented(cl client.Client, machineName string) {
728-
verifyUpdatePrevented(cl, machineName, "add annotations machine.openshift.io.", func(machine *machinev1beta1.Machine) {
720+
func verifyAnnotationModificationPrevented(machine *machinev1beta1.Machine) {
721+
verifyUpdatePrevented(machine, "add annotations machine.openshift.io.", func(machine *machinev1beta1.Machine) {
729722
if machine.Annotations == nil {
730723
machine.Annotations = make(map[string]string)
731724
}

0 commit comments

Comments
 (0)