Skip to content

Commit

Permalink
Fix for #139 (#141)
Browse files Browse the repository at this point in the history
* Fix application not found error by detecting the error and clearing the app status

* Simplify how we handle missing applications

* Initial stab at an integration test

* fix integration-test name

* fix test domain and name

* Try fixing integration test that was not covering the code changes

* fix integration test

---------

Co-authored-by: Bojan Zelic <[email protected]>
  • Loading branch information
kadaan and BojanZelic authored Jan 4, 2025
1 parent 143e2e1 commit 53779e3
Show file tree
Hide file tree
Showing 2 changed files with 74 additions and 2 deletions.
11 changes: 9 additions & 2 deletions internal/controller/cloudflareaccessapplication_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,9 +127,16 @@ func (r *CloudflareAccessApplicationReconciler) Reconcile(ctx context.Context, r
}
} else {
accessApp, err := api.AccessApplication(ctx, app.Status.AccessApplicationID)
existingaccessApp = &accessApp
if err != nil {
return ctrl.Result{}, errors.Wrap(err, "unable to get access application")
var apiErr *cloudflare.NotFoundError
if errors.As(err, &apiErr) {
log.Info("access application not found - recreating...", "accessApplicationID", app.Status.AccessApplicationID)
app.Status.AccessApplicationID = ""
} else {
return ctrl.Result{}, errors.Wrap(err, "unable to get access application")
}
} else {
existingaccessApp = &accessApp
}
}

Expand Down
65 changes: 65 additions & 0 deletions internal/controller/cloudflareaccessapplication_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -438,5 +438,70 @@ var _ = Describe("CloudflareAccessApplication controller", Ordered, func() {
Expect(err).To(Not(HaveOccurred()))
Expect(cfResource.Name).To(Equal(found.Spec.Name))
})

It("should successfully reconcile CloudflareAccessApplication whose AccessApplicationID references a missing Application", func() {
By("Recreating the custom resource for the Kind CloudflareAccessApplication")
typeNamespaceName := types.NamespacedName{Name: "cloudflare-app-seven", Namespace: cloudflareName}

previousCreatedAndUpdatedDate := metav1.NewTime(time.Now().Add(-time.Hour * 24))
apps := &v1alpha1.CloudflareAccessApplication{
ObjectMeta: metav1.ObjectMeta{
Name: typeNamespaceName.Name,
Namespace: namespace.Name,
},
Spec: v1alpha1.CloudflareAccessApplicationSpec{
Name: "missing application",
Domain: "recreate-application.cf-operator-tests.uk",
},
}

err := k8sClient.Create(ctx, apps)
Expect(err).To(Not(HaveOccurred()))

By("Checking if the custom resource was successfully created")
Eventually(func() error {
found := &v1alpha1.CloudflareAccessApplication{}
return k8sClient.Get(ctx, typeNamespaceName, found)
}, time.Minute, time.Second).Should(Succeed())

found := &v1alpha1.CloudflareAccessApplication{}
By("Checking the latest Status should have the ID of the resource")

oldAccessApplicationID := ""

Eventually(func(g Gomega) {
found = &v1alpha1.CloudflareAccessApplication{}
g.Expect(k8sClient.Get(ctx, typeNamespaceName, found)).To(Not(HaveOccurred()))
g.Expect(found.Status.AccessApplicationID).ToNot(BeEmpty())
oldAccessApplicationID = found.Status.AccessApplicationID
g.Expect(found.Status.CreatedAt.Time).To(Equal(found.Status.UpdatedAt.Time))
g.Expect(found.Status.CreatedAt.Time.After(previousCreatedAndUpdatedDate.Time)).To(BeTrue())
g.Expect(found.Status.UpdatedAt.Time.After(previousCreatedAndUpdatedDate.Time)).To(BeTrue())
}, time.Second*10, time.Second).Should(Succeed())

Expect(api.DeleteAccessApplication(ctx, found.Status.AccessApplicationID)).To(Not(HaveOccurred()))

By("re-trigger reconcile by updating access application")
Eventually(func(g Gomega) {
found = &v1alpha1.CloudflareAccessApplication{}
g.Expect(k8sClient.Get(ctx, typeNamespaceName, found)).To(Not(HaveOccurred()))
found.Spec.Name = "updated name"
Expect(k8sClient.Update(ctx, found)).To(Not(HaveOccurred()))
}, time.Second*10, time.Second).Should(Succeed())

By("Checking the latest Status should have the ID of the resource")
Eventually(func(g Gomega) {
found = &v1alpha1.CloudflareAccessApplication{}
g.Expect(k8sClient.Get(ctx, typeNamespaceName, found)).To(Not(HaveOccurred()))
g.Expect(found.Status.AccessApplicationID).ToNot(Equal(oldAccessApplicationID))
}, time.Second*10, time.Second).Should(Succeed())

By("Cloudflare resource should equal the updated spec")
Eventually(func(g Gomega) {
cfResource, err := api.AccessApplication(ctx, found.Status.AccessApplicationID)
g.Expect(err).To(Not(HaveOccurred()))
g.Expect(cfResource.Name).To(Equal(found.Spec.Name))
}, time.Second*45, time.Second).Should(Succeed(), logOutput.GetOutput()) //sometimes this is cached
})
})
})

0 comments on commit 53779e3

Please sign in to comment.