Skip to content

Commit d03e9df

Browse files
fmountfmount
authored
Merge pull request #671 from stuggi/reconcile_anno
Add generic EnsureWebhookTrigger function to webhook package
2 parents e2c25ec + bfd638c commit d03e9df

2 files changed

Lines changed: 251 additions & 0 deletions

File tree

‎modules/common/webhook/trigger.go‎

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
/*
2+
Copyright 2024 Red Hat
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
// Package webhook provides validation utilities for admission webhooks and RFC compliance
18+
package webhook
19+
20+
import (
21+
"context"
22+
"errors"
23+
"fmt"
24+
"time"
25+
26+
"github.com/go-logr/logr"
27+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
28+
ctrl "sigs.k8s.io/controller-runtime"
29+
"sigs.k8s.io/controller-runtime/pkg/client"
30+
)
31+
32+
// ErrWebhookTriggerTimeout is returned when the webhook trigger annotation
33+
// remains on the object longer than the configured timeout, indicating the
34+
// webhook failed to process or remove it
35+
var ErrWebhookTriggerTimeout = errors.New("webhook failed to process trigger annotation within timeout")
36+
37+
// EnsureWebhookTrigger ensures the reconcile trigger annotation is set to trigger UPDATE webhook
38+
// The annotation value is set to the current timestamp to ensure it triggers updates
39+
// Returns ctrl.Result{Requeue: true}, nil to requeue and let the defer block save the annotation
40+
// reason is logged to help identify why the trigger was added (e.g., "Service name caching")
41+
// timeout specifies how long to wait before considering the annotation stale (defaults to 5 minutes if 0)
42+
// If the annotation exists for more than the timeout, it's considered stale and removed with an error
43+
//
44+
// example usage:
45+
//
46+
// result, err := webhook.EnsureWebhookTrigger(
47+
// ctx,
48+
// instance,
49+
// "openstack.org/reconcile-trigger",
50+
// "Cinder service name caching",
51+
// log,
52+
// 0, // use default 5 minute timeout
53+
// )
54+
func EnsureWebhookTrigger(
55+
ctx context.Context,
56+
obj client.Object,
57+
annotationKey string,
58+
reason string,
59+
log logr.Logger,
60+
timeout time.Duration,
61+
) (ctrl.Result, error) {
62+
// Default to 5 minutes if timeout not provided
63+
if timeout == 0 {
64+
timeout = 5 * time.Minute
65+
}
66+
67+
annotations := obj.GetAnnotations()
68+
if annotations == nil {
69+
annotations = make(map[string]string)
70+
}
71+
72+
if val, exists := annotations[annotationKey]; exists {
73+
// Annotation exists - check if it's stale (webhook may have failed)
74+
if timestamp, err := time.Parse(time.RFC3339, val); err == nil {
75+
if time.Since(timestamp) > timeout {
76+
// Annotation is stale - webhook failed to process or remove it
77+
log.Error(nil, "Stale reconcile-trigger annotation detected, removing", "reason", reason, "age", time.Since(timestamp))
78+
delete(annotations, annotationKey)
79+
obj.SetAnnotations(annotations)
80+
return ctrl.Result{}, fmt.Errorf("%w: %s after %v", ErrWebhookTriggerTimeout, reason, timeout)
81+
}
82+
}
83+
// Annotation exists but not stale yet - webhook is still processing
84+
log.Info(fmt.Sprintf("Waiting for webhook to process: %s", reason))
85+
return ctrl.Result{Requeue: true}, nil
86+
}
87+
88+
// Add annotation with timestamp to trigger webhook
89+
// The defer block will save this change, which triggers the UPDATE webhook
90+
annotations[annotationKey] = metav1.Now().Format(time.RFC3339)
91+
obj.SetAnnotations(annotations)
92+
log.Info(fmt.Sprintf("Adding reconcile-trigger annotation: %s", reason))
93+
94+
// Requeue - defer will save annotation, which triggers webhook
95+
return ctrl.Result{Requeue: true}, nil
96+
}

‎modules/common/webhook/trigger_test.go‎

Lines changed: 155 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,155 @@
1+
/*
2+
Copyright 2024 Red Hat
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package webhook
18+
19+
import (
20+
"context"
21+
"testing"
22+
"time"
23+
24+
"github.com/go-logr/logr"
25+
. "github.com/onsi/gomega"
26+
corev1 "k8s.io/api/core/v1"
27+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
28+
ctrl "sigs.k8s.io/controller-runtime"
29+
)
30+
31+
func TestEnsureWebhookTrigger(t *testing.T) {
32+
const annotationKey = "test.org/reconcile-trigger"
33+
34+
tests := []struct {
35+
name string
36+
existingAnnotations map[string]string
37+
reason string
38+
timeout time.Duration
39+
wantRequeue bool
40+
wantError bool
41+
}{
42+
{
43+
name: "no existing annotation - should add and requeue",
44+
existingAnnotations: nil,
45+
reason: "test reason",
46+
timeout: 0, // default
47+
wantRequeue: true,
48+
wantError: false,
49+
},
50+
{
51+
name: "existing fresh annotation - should requeue",
52+
existingAnnotations: map[string]string{
53+
annotationKey: metav1.Now().Format(time.RFC3339),
54+
},
55+
reason: "test reason",
56+
timeout: 0, // default
57+
wantRequeue: true,
58+
wantError: false,
59+
},
60+
{
61+
name: "existing stale annotation with default timeout - should error",
62+
existingAnnotations: map[string]string{
63+
annotationKey: metav1.Time{Time: time.Now().Add(-10 * time.Minute)}.Format(time.RFC3339),
64+
},
65+
reason: "test reason",
66+
timeout: 0, // default 5 minutes
67+
wantRequeue: false,
68+
wantError: true,
69+
},
70+
{
71+
name: "existing annotation within custom timeout - should requeue",
72+
existingAnnotations: map[string]string{
73+
annotationKey: metav1.Time{Time: time.Now().Add(-2 * time.Minute)}.Format(time.RFC3339),
74+
},
75+
reason: "test reason",
76+
timeout: 10 * time.Minute, // custom long timeout
77+
wantRequeue: true,
78+
wantError: false,
79+
},
80+
{
81+
name: "existing annotation exceeds custom timeout - should error",
82+
existingAnnotations: map[string]string{
83+
annotationKey: metav1.Time{Time: time.Now().Add(-2 * time.Minute)}.Format(time.RFC3339),
84+
},
85+
reason: "test reason",
86+
timeout: 1 * time.Minute, // custom short timeout
87+
wantRequeue: false,
88+
wantError: true,
89+
},
90+
}
91+
92+
for _, tt := range tests {
93+
t.Run(tt.name, func(t *testing.T) {
94+
g := NewWithT(t)
95+
ctx := context.Background()
96+
97+
// Create a test object (using ConfigMap as a simple client.Object)
98+
obj := &corev1.ConfigMap{
99+
ObjectMeta: metav1.ObjectMeta{
100+
Name: "test-obj",
101+
Namespace: "test-ns",
102+
Annotations: tt.existingAnnotations,
103+
},
104+
}
105+
106+
// Create a no-op logger for testing
107+
log := logr.Discard()
108+
109+
result, err := EnsureWebhookTrigger(ctx, obj, annotationKey, tt.reason, log, tt.timeout)
110+
111+
if tt.wantError {
112+
g.Expect(err).To(HaveOccurred())
113+
g.Expect(obj.GetAnnotations()).ToNot(HaveKey(annotationKey))
114+
} else {
115+
g.Expect(err).ToNot(HaveOccurred())
116+
g.Expect(obj.GetAnnotations()).To(HaveKey(annotationKey))
117+
// Verify the annotation value is a valid RFC3339 timestamp
118+
timestamp := obj.GetAnnotations()[annotationKey]
119+
_, parseErr := time.Parse(time.RFC3339, timestamp)
120+
g.Expect(parseErr).ToNot(HaveOccurred())
121+
}
122+
123+
if tt.wantRequeue {
124+
g.Expect(result).To(Equal(ctrl.Result{Requeue: true}))
125+
} else {
126+
g.Expect(result).To(Equal(ctrl.Result{}))
127+
}
128+
})
129+
}
130+
}
131+
132+
func TestEnsureWebhookTrigger_AnnotationFormat(t *testing.T) {
133+
g := NewWithT(t)
134+
ctx := context.Background()
135+
const annotationKey = "test.org/reconcile-trigger"
136+
137+
obj := &corev1.ConfigMap{
138+
ObjectMeta: metav1.ObjectMeta{
139+
Name: "test-obj",
140+
Namespace: "test-ns",
141+
},
142+
}
143+
144+
log := logr.Discard()
145+
_, err := EnsureWebhookTrigger(ctx, obj, annotationKey, "test", log, 0)
146+
147+
g.Expect(err).ToNot(HaveOccurred())
148+
g.Expect(obj.GetAnnotations()).To(HaveKey(annotationKey))
149+
150+
// Verify the timestamp format
151+
timestampStr := obj.GetAnnotations()[annotationKey]
152+
timestamp, err := time.Parse(time.RFC3339, timestampStr)
153+
g.Expect(err).ToNot(HaveOccurred())
154+
g.Expect(timestamp).To(BeTemporally("~", time.Now(), time.Second))
155+
}

0 commit comments

Comments
 (0)