Skip to content

Commit 1fdaac6

Browse files
akosveresakosveres
committedNov 18, 2024·
fix(annotations): handle the resource disable state
* if the resource annotation is no longer present * if the enabled annotation is changed to false
1 parent dfad553 commit 1fdaac6

File tree

2 files changed

+93
-37
lines changed

2 files changed

+93
-37
lines changed
 

‎internal/controller/networking/ingress_controller.go

+62-37
Original file line numberDiff line numberDiff line change
@@ -73,21 +73,25 @@ func (r *IngressReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
7373
}
7474
logger.Info("Ingress Object found")
7575

76-
// Do we want to do anything with the ingress?
77-
// TODO: What if the annotation was deleted?
78-
_, checklyAnnotationExists := ingress.Annotations[annotationEnabled]
79-
if (!checklyAnnotationExists) || (ingress.Annotations[annotationEnabled] == "false") {
80-
logger.Info("Ingress does not need to be handled by the operator, skipping.", "Ingress Name", ingress.Name, "Ingress namespace", ingress.Namespace)
81-
return ctrl.Result{}, nil
82-
}
83-
8476
// Gather data for the checkly check
8577
logger.Info("Gathering data for the check")
8678
apiCheckResources, err := r.gatherApiCheckData(&ingress)
8779
if err != nil {
8880
logger.Info("unable to gather data for the apiCheck resource", "Ingress Name", ingress.Name, "Ingress namespace", ingress.Namespace)
8981
return ctrl.Result{}, err
9082
}
83+
84+
// Do we want to do anything with the ingress?
85+
_, checklyAnnotationExists := ingress.Annotations[annotationEnabled]
86+
if (!checklyAnnotationExists) || (ingress.Annotations[annotationEnabled] == "false") {
87+
logger.Info("Checking to see if we need to delete any resources as we're not handling this ingress.", "Ingress Name", ingress.Name, "Ingress namespace", ingress.Namespace)
88+
89+
r.deleteIngressApiChecks(ctx, req, apiCheckResources, ingress)
90+
91+
if ingress.GetDeletionTimestamp() == nil {
92+
return ctrl.Result{}, nil
93+
}
94+
}
9195
// ////////////////////////////////
9296
// Delete Logic
9397
// ///////////////////////////////
@@ -96,24 +100,7 @@ func (r *IngressReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
96100
if controllerutil.ContainsFinalizer(&ingress, checklyFinalizer) {
97101
logger.Info("Finalizer present, need to delete ApiCheck first.", "Ingress Name", ingress.Name, "Ingress namespace", ingress.Namespace)
98102

99-
for _, apiCheckResource := range apiCheckResources {
100-
101-
logger.Info("Checking if ApiCheck was created", "Ingress Name", ingress.Name, "Ingress namespace", ingress.Namespace)
102-
err = r.Get(ctx, req.NamespacedName, apiCheckResource)
103-
if err != nil {
104-
logger.Info("ApiCheck resource is not present, we don't need to do anything.", "Ingress Name", ingress.Name, "Ingress namespace", ingress.Namespace)
105-
continue
106-
}
107-
108-
logger.Info("ApiCheck resource is present, we need to delete it..", "Ingress Name", ingress.Name, "Ingress namespace", ingress.Namespace)
109-
err = r.Delete(ctx, apiCheckResource)
110-
if err != nil {
111-
logger.Error(err, "Failed to delete ApiCheck", "Name:", apiCheckResource.Name, "Namespace:", apiCheckResource.Namespace)
112-
continue
113-
}
114-
115-
logger.Info("ApiCheck resource deleted successfully.", apiCheckResource.Name, "Namespace:", apiCheckResource.Namespace)
116-
}
103+
r.deleteIngressApiChecks(ctx, req, apiCheckResources, ingress)
117104

118105
// Delete finalizer logic
119106
logger.Info("Deleting finalizer", "Ingress Name", ingress.Name, "Ingress namespace", ingress.Namespace)
@@ -236,6 +223,11 @@ func (r *IngressReconciler) gatherApiCheckData(ingress *networkingv1.Ingress) (a
236223
labels["ingress-controller"] = ingress.Name
237224

238225
// Get the host(s) and path(s) from the ingress object
226+
// No Rules specified, nothing to do
227+
if len(ingress.Spec.Rules) == 0 {
228+
return
229+
}
230+
239231
for _, rule := range ingress.Spec.Rules {
240232

241233
// Get the host
@@ -247,18 +239,27 @@ func (r *IngressReconciler) gatherApiCheckData(ingress *networkingv1.Ingress) (a
247239
}
248240

249241
// Get the path(s)
250-
var path string
251-
for _, rulePath := range rule.HTTP.Paths {
252-
if ingress.Annotations[annotationPath] == "" {
253-
if rulePath.Path == "" {
254-
path = "/"
242+
var paths []string
243+
244+
if rule.HTTP == nil { // HTTP may not exist
245+
paths = append(paths, "/")
246+
} else if rule.HTTP.Paths == nil { // Paths may not exist
247+
paths = append(paths, "/")
248+
} else {
249+
for _, rulePath := range rule.HTTP.Paths {
250+
if ingress.Annotations[annotationPath] == "" {
251+
if rulePath.Path == "" {
252+
paths = append(paths, "/")
253+
} else {
254+
paths = append(paths, rulePath.Path)
255+
}
255256
} else {
256-
path = rulePath.Path
257+
paths = append(paths, ingress.Annotations[annotationPath])
257258
}
258-
} else {
259-
path = ingress.Annotations[annotationPath]
260259
}
260+
}
261261

262+
for _, path := range paths {
262263
// Replace path /
263264
path = strings.TrimPrefix(path, "/")
264265

@@ -271,7 +272,8 @@ func (r *IngressReconciler) gatherApiCheckData(ingress *networkingv1.Ingress) (a
271272
// Set endpoint
272273
endpoint := fmt.Sprintf("https://%s/%s", host, path)
273274

274-
apiCheckSpec := checklyv1alpha1.ApiCheckSpec{
275+
// Construct ApiCheck Spec
276+
apiCheckSpec := &checklyv1alpha1.ApiCheckSpec{
275277
Endpoint: endpoint,
276278
Group: group,
277279
Success: success,
@@ -287,12 +289,11 @@ func (r *IngressReconciler) gatherApiCheckData(ingress *networkingv1.Ingress) (a
287289
},
288290
Labels: labels,
289291
},
290-
Spec: apiCheckSpec,
292+
Spec: *apiCheckSpec,
291293
}
292294

293295
apiChecks = append(apiChecks, newApiCheck)
294296
}
295-
296297
}
297298

298299
// Last return
@@ -347,3 +348,27 @@ func (r *IngressReconciler) compareApiChecks(ctx context.Context, ingress *netwo
347348

348349
return
349350
}
351+
352+
func (r *IngressReconciler) deleteIngressApiChecks(ctx context.Context, req ctrl.Request, apiCheckResources []*checklyv1alpha1.ApiCheck, ingress networkingv1.Ingress) {
353+
354+
logger := log.FromContext(ctx)
355+
356+
for _, apiCheckResource := range apiCheckResources {
357+
358+
logger.Info("Checking if ApiCheck was created", "Ingress Name", ingress.Name, "Ingress namespace", ingress.Namespace)
359+
err := r.Get(ctx, req.NamespacedName, apiCheckResource)
360+
if err != nil {
361+
logger.Info("ApiCheck resource is not present, we don't need to do anything.", "Ingress Name", ingress.Name, "Ingress namespace", ingress.Namespace)
362+
continue
363+
}
364+
365+
logger.Info("ApiCheck resource is present, we need to delete it..", "Ingress Name", ingress.Name, "Ingress namespace", ingress.Namespace)
366+
err = r.Delete(ctx, apiCheckResource)
367+
if err != nil {
368+
logger.Error(err, "Failed to delete ApiCheck", "Name:", apiCheckResource.Name, "Namespace:", apiCheckResource.Namespace)
369+
continue
370+
}
371+
372+
logger.Info("ApiCheck resource deleted successfully.", apiCheckResource.Name, "Namespace:", apiCheckResource.Namespace)
373+
}
374+
}

‎internal/controller/networking/ingress_controller_test.go

+31
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,37 @@ var _ = Describe("Ingress Controller", func() {
138138
return true
139139
}, timeout, interval).Should(BeTrue(), "Timed out waiting for success")
140140

141+
// Set enabled false
142+
By("Expecting enabled false to remove ApiCheck")
143+
Eventually(func() error {
144+
// Get existing ingress object
145+
f := &networkingv1.Ingress{}
146+
err := k8sClient.Get(context.Background(), ingressKey, f)
147+
if err != nil {
148+
return err
149+
}
150+
151+
// Update annotations with `enabled` set to false
152+
f.Annotations["testing.domain.tld/enabled"] = "false"
153+
err = k8sClient.Update(context.Background(), f)
154+
if err != nil {
155+
return err
156+
}
157+
158+
u := &networkingv1.Ingress{}
159+
err = k8sClient.Get(context.Background(), ingressKey, u)
160+
if err != nil {
161+
return err
162+
}
163+
164+
Expect(u.Annotations["testing.domain.tld/enabled"]).To(Equal("false"), "Enabled annotation should be false")
165+
166+
// Expect API Check to not exist anymore
167+
Expect(k8sClient.Get(context.Background(), apiCheckKey, f)).ShouldNot(Succeed())
168+
169+
return nil
170+
}, timeout, interval).Should(Succeed(), "Timeout waiting for update")
171+
141172
// Delete
142173
By("Expecting to delete successfully")
143174
Eventually(func() error {

0 commit comments

Comments
 (0)
Please sign in to comment.