-
Notifications
You must be signed in to change notification settings - Fork 80
SREP-1220: Add E2E test for revoking and deleting CertificateRequests with certman label #377
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
f77305f
9f1aade
82df677
0364f62
7cabfdd
4fd5fa2
0714b76
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -694,4 +694,98 @@ var _ = ginkgo.Describe("Certman Operator", ginkgo.Ordered, ginkgo.ContinueOnFai | |
| logger.Info("Cleanup: AfterAll cleanup completed") | ||
| }) | ||
|
|
||
| ginkgo.It("Delete a labeled CertificateRequest and ensures it is recreated", func(ctx context.Context) { | ||
| crGVR := schema.GroupVersionResource{ | ||
| Group: "certman.managed.openshift.io", | ||
| Version: "v1alpha1", | ||
| Resource: "certificaterequests", | ||
| } | ||
|
|
||
| log.Log.Info("STEP 1: Fetching existing CertificateRequest with owned=true label") | ||
| crList, err := dynamicClient.Resource(crGVR).Namespace(namespace).List(ctx, metav1.ListOptions{ | ||
| LabelSelector: "certificaterequests.certman.managed.openshift.io", | ||
| }) | ||
|
|
||
| if len(crList.Items) == 0 { | ||
| log.Log.Info("No labeled CertificateRequest found, skipping test") | ||
| ginkgo.Skip("SKIPPED: No labeled CertificateRequest found. This test only runs if a CR with 'owned=true' label is present.") | ||
| } | ||
|
|
||
| originalCR := crList.Items[0] | ||
| originalCRName := originalCR.GetName() | ||
| originalCRUID := originalCR.GetUID() | ||
| initialIssuedCertCount := len(crList.Items) | ||
|
|
||
| // Step 2: Delete the CertificateRequest | ||
| log.Log.Info("STEP 2: Deleting the original CertificateRequest") | ||
| err = dynamicClient.Resource(crGVR).Namespace(namespace).Delete(ctx, originalCRName, metav1.DeleteOptions{}) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Which certificaterequests are we deleting here? As far as I can tell, nothing was created in the In general, I would caution against us blindly deleting objects that we retrieved with a list operation unless we're using a specific |
||
| gomega.Expect(err).ToNot(gomega.HaveOccurred(), "Failed to delete CertificateRequest") | ||
|
|
||
| // Step 3: Handle deletion blocked by finalizer | ||
| gomega.Eventually(func() bool { | ||
| cr, err := dynamicClient.Resource(crGVR).Namespace(namespace).Get(ctx, originalCRName, metav1.GetOptions{}) | ||
| if err != nil { | ||
| log.Log.Info("CR appears to be deleted already", "name", originalCRName) | ||
| return true | ||
| } | ||
| if cr.GetDeletionTimestamp() == nil { | ||
| log.Log.Info("CR not marked for deletion yet", "name", cr.GetName()) | ||
| return false | ||
| } | ||
|
|
||
| finalizers, found, err := unstructured.NestedStringSlice(cr.Object, "metadata", "finalizers") | ||
| if err != nil { | ||
| log.Log.Error(err, "Error retrieving finalizers") | ||
| return false | ||
| } | ||
| if !found || len(finalizers) == 0 { | ||
| log.Log.Info("No finalizers present", "name", cr.GetName()) | ||
| return false | ||
| } | ||
|
|
||
| crCopy := cr.DeepCopy() | ||
| _ = unstructured.SetNestedStringSlice(crCopy.Object, []string{}, "metadata", "finalizers") | ||
|
|
||
| _, err = dynamicClient.Resource(crGVR).Namespace(namespace).Update(ctx, crCopy, metav1.UpdateOptions{}) | ||
| if err != nil { | ||
| log.Log.Error(err, "Failed to remove finalizer") | ||
| return false | ||
| } | ||
| return true | ||
| }, 1*time.Minute, 5*time.Second).Should(gomega.BeTrue(), "Finalizer should be removed") | ||
|
Comment on lines
+725
to
+755
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why remove the finalizer? Shouldn't this test include coverage for certman-operator's own finalizing logic? |
||
|
|
||
| // Step 4: Wait for new CertificateRequest with new UID | ||
| var newCRName string | ||
| gomega.Eventually(func() bool { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unless we're creating a |
||
| newList, err := dynamicClient.Resource(crGVR).Namespace(namespace).List(ctx, metav1.ListOptions{}) | ||
| if err != nil { | ||
| log.Log.Error(err, "Failed to list new CertificateRequests") | ||
| return false | ||
| } | ||
| if len(newList.Items) == 0 { | ||
| log.Log.Info("Still waiting for new CertificateRequest (none found)") | ||
| return false | ||
| } | ||
|
|
||
| newCount := len(newList.Items) | ||
| logger.Info("CertificateRequest count after reconciliation", "count", newCount) | ||
| if newCount != initialIssuedCertCount { | ||
| logger.Info("CertificateRequest count mismatch", "expected", initialIssuedCertCount, "got", newCount) | ||
| return false | ||
| } | ||
|
|
||
| for _, cr := range newList.Items { | ||
| log.Log.Info("Found CR candidate", "name", cr.GetName(), "uid", cr.GetUID()) | ||
| if cr.GetUID() != originalCRUID { | ||
| newCRName = cr.GetName() | ||
| log.Log.Info("New CertificateRequest detected", "name", newCRName, "uid", cr.GetUID()) | ||
| return true | ||
| } | ||
| } | ||
| return false | ||
| }, 4*time.Minute, 10*time.Second).Should(gomega.BeTrue(), "New CertificateRequest should appear") | ||
|
|
||
| log.Log.Info("✅ Test completed: Secret successfully recreated with new CertificateRequest", "secret", secretName) | ||
| }) | ||
|
|
||
| }) | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This label does not exist on any certificaterequest that I've seen: which component do we expect adds this?