Skip to content

Commit 27a38f0

Browse files
davidmogardavidmogar
committed
fix: use loader to avoid using the client
With this change, the loader is used in the webhook, which allow us to use the MockLoader to avoid the flakiness of calling the actual client. Signed-off-by: David Moreno García <[email protected]>
1 parent 3d33348 commit 27a38f0

File tree

3 files changed

+95
-74
lines changed

3 files changed

+95
-74
lines changed

‎api/v1alpha1/webhooks/release/suite_test.go‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ import (
2929
"github.com/redhat-appstudio/release-service/api/v1alpha1"
3030
"github.com/redhat-appstudio/release-service/api/v1alpha1/webhooks/author"
3131

32-
releaseplanwebhook "github.com/redhat-appstudio/release-service/api/v1alpha1/webhooks/releaseplan"
3332
"sigs.k8s.io/controller-runtime/pkg/metrics/server"
3433
crwebhook "sigs.k8s.io/controller-runtime/pkg/webhook"
3534

@@ -105,7 +104,7 @@ var _ = BeforeSuite(func() {
105104
})
106105
Expect(err).NotTo(HaveOccurred())
107106

108-
err = toolkit.SetupWebhooks(mgr, &Webhook{}, &author.Webhook{}, &releaseplanwebhook.Webhook{})
107+
err = toolkit.SetupWebhooks(mgr, &Webhook{}, &author.Webhook{})
109108
Expect(err).NotTo(HaveOccurred())
110109

111110
//+kubebuilder:scaffold:webhook

‎api/v1alpha1/webhooks/release/webhook.go‎

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,13 @@ package release
1919
import (
2020
"context"
2121
"fmt"
22+
"github.com/redhat-appstudio/release-service/loader"
2223
"reflect"
2324

2425
"github.com/go-logr/logr"
2526
"github.com/redhat-appstudio/release-service/api/v1alpha1"
2627
"k8s.io/apimachinery/pkg/api/errors"
2728
"k8s.io/apimachinery/pkg/runtime"
28-
"k8s.io/apimachinery/pkg/types"
2929
ctrl "sigs.k8s.io/controller-runtime"
3030
"sigs.k8s.io/controller-runtime/pkg/client"
3131
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
@@ -34,28 +34,30 @@ import (
3434
// Webhook describes the data structure for the release webhook
3535
type Webhook struct {
3636
client client.Client
37+
loader loader.ObjectLoader
3738
log logr.Logger
3839
}
3940

41+
// Default implements webhook.Defaulter so a webhook will be registered for the type.
4042
func (w *Webhook) Default(ctx context.Context, obj runtime.Object) error {
4143
release := obj.(*v1alpha1.Release)
4244

43-
if release.Spec.GracePeriodDays == 0 {
44-
releasePlan := &v1alpha1.ReleasePlan{}
45-
46-
err := w.client.Get(ctx, types.NamespacedName{
47-
Name: release.Spec.ReleasePlan,
48-
Namespace: release.Namespace,
49-
}, releasePlan)
45+
if release.Spec.GracePeriodDays != 0 {
46+
return nil
47+
}
5048

51-
if err != nil && errors.IsNotFound(err) {
49+
releasePlan, err := w.loader.GetReleasePlan(ctx, w.client, release)
50+
if err != nil {
51+
if errors.IsNotFound(err) {
5252
w.log.Info("releasePlan not found. Not setting ReleaseGracePeriodDays")
53-
5453
return nil
54+
} else {
55+
return err
5556
}
56-
release.Spec.GracePeriodDays = releasePlan.Spec.ReleaseGracePeriodDays
5757
}
5858

59+
release.Spec.GracePeriodDays = releasePlan.Spec.ReleaseGracePeriodDays
60+
5961
return nil
6062
}
6163

@@ -64,6 +66,7 @@ func (w *Webhook) Default(ctx context.Context, obj runtime.Object) error {
6466

6567
func (w *Webhook) Register(mgr ctrl.Manager, log *logr.Logger) error {
6668
w.client = mgr.GetClient()
69+
w.loader = loader.NewLoader()
6770
w.log = log.WithName("release")
6871

6972
return ctrl.NewWebhookManagedBy(mgr).

‎api/v1alpha1/webhooks/release/webhook_test.go‎

Lines changed: 80 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,11 @@ package release
1717

1818
import (
1919
"context"
20-
20+
toolkit "github.com/redhat-appstudio/operator-toolkit/loader"
2121
"github.com/redhat-appstudio/release-service/api/v1alpha1"
22+
"github.com/redhat-appstudio/release-service/loader"
23+
"k8s.io/apimachinery/pkg/api/errors"
24+
"k8s.io/apimachinery/pkg/runtime/schema"
2225

2326
. "github.com/onsi/ginkgo/v2"
2427
. "github.com/onsi/gomega"
@@ -28,10 +31,81 @@ import (
2831
)
2932

3033
var _ = Describe("Release validation webhook", func() {
31-
var release *v1alpha1.Release
32-
var releasePlan *v1alpha1.ReleasePlan
34+
var (
35+
createResources func()
36+
37+
release *v1alpha1.Release
38+
releasePlan *v1alpha1.ReleasePlan
39+
)
40+
41+
When("Default method is called", func() {
42+
var mockedWebhook *Webhook
43+
44+
BeforeEach(func() {
45+
createResources()
46+
47+
mockedWebhook = &Webhook{
48+
client: k8sClient,
49+
loader: loader.NewMockLoader(),
50+
}
51+
})
52+
53+
It("should set GracePeriodDays to ReleasePlan's value and return nil", func() {
54+
mockedCtx := toolkit.GetMockedContext(ctx, []toolkit.MockData{
55+
{
56+
ContextKey: loader.ReleasePlanContextKey,
57+
Resource: releasePlan,
58+
},
59+
})
60+
61+
Expect(mockedWebhook.Default(mockedCtx, release)).To(BeNil())
62+
Expect(release.Spec.GracePeriodDays).To(Equal(releasePlan.Spec.ReleaseGracePeriodDays))
63+
})
64+
65+
It("should return nil and keep the default value of a go `int` for GracePeriodDays when the specified ReleasePlan does not exist", func() {
66+
mockedCtx := toolkit.GetMockedContext(ctx, []toolkit.MockData{
67+
{
68+
ContextKey: loader.ReleasePlanContextKey,
69+
Err: errors.NewNotFound(schema.GroupResource{}, ""),
70+
},
71+
})
72+
73+
Expect(mockedWebhook.Default(mockedCtx, release)).To(BeNil())
74+
Expect(release.Spec.GracePeriodDays).To(Equal(0))
75+
})
76+
})
77+
78+
When("When ValidateUpdate is called", func() {
79+
It("should error out when updating the resource", func() {
80+
updatedRelease := release.DeepCopy()
81+
updatedRelease.Spec.Snapshot = "another-snapshot"
82+
83+
_, err := webhook.ValidateUpdate(ctx, release, updatedRelease)
84+
Expect(err).Should(HaveOccurred())
85+
Expect(err.Error()).Should(ContainSubstring("release resources spec cannot be updated"))
86+
})
87+
88+
It("should not error out when updating the resource metadata", func() {
89+
ctx := context.Background()
90+
91+
updatedRelease := release.DeepCopy()
92+
updatedRelease.ObjectMeta.Annotations = map[string]string{
93+
"foo": "bar",
94+
}
95+
96+
_, err := webhook.ValidateUpdate(ctx, release, updatedRelease)
97+
Expect(err).NotTo(HaveOccurred())
98+
})
99+
})
33100

34-
BeforeEach(func() {
101+
When("ValidateDelete method is called", func() {
102+
It("should return nil", func() {
103+
_, err := webhook.ValidateDelete(ctx, &v1alpha1.Release{})
104+
Expect(err).NotTo(HaveOccurred())
105+
})
106+
})
107+
108+
createResources = func() {
35109
release = &v1alpha1.Release{
36110
TypeMeta: metav1.TypeMeta{
37111
APIVersion: "appstudio.redhat.com/v1alpha1",
@@ -46,6 +120,7 @@ var _ = Describe("Release validation webhook", func() {
46120
ReleasePlan: "test-releaseplan",
47121
},
48122
}
123+
49124
releasePlan = &v1alpha1.ReleasePlan{
50125
TypeMeta: metav1.TypeMeta{
51126
APIVersion: "appstudio.redhat.com/v1alpha1",
@@ -61,61 +136,5 @@ var _ = Describe("Release validation webhook", func() {
61136
ReleaseGracePeriodDays: 7,
62137
},
63138
}
64-
})
65-
66-
AfterEach(func() {
67-
_ = k8sClient.Delete(ctx, release)
68-
})
69-
70-
When("release CR fields are updated", func() {
71-
It("Should error out when updating the resource", func() {
72-
ctx := context.Background()
73-
74-
Expect(k8sClient.Create(ctx, release)).Should(Succeed())
75-
76-
// Try to update the Release snapshot
77-
release.Spec.Snapshot = "another-snapshot"
78-
79-
err := k8sClient.Update(ctx, release)
80-
Expect(err).Should(HaveOccurred())
81-
Expect(err.Error()).Should(ContainSubstring("release resources spec cannot be updated"))
82-
})
83-
84-
It("Should not error out when updating the resource metadata", func() {
85-
ctx := context.Background()
86-
87-
Expect(k8sClient.Create(ctx, release)).Should(Succeed())
88-
89-
// Try to update the Release annotations
90-
release.ObjectMeta.Annotations = map[string]string{
91-
"foo": "bar",
92-
}
93-
94-
Expect(k8sClient.Update(ctx, release)).ShouldNot(HaveOccurred())
95-
})
96-
})
97-
98-
When("ValidateDelete method is called", func() {
99-
It("should return nil", func() {
100-
release := &v1alpha1.Release{}
101-
Expect(webhook.ValidateDelete(ctx, release)).To(BeNil())
102-
})
103-
})
104-
105-
When("a new Release is created", func() {
106-
It("should set GracePeriodDays to ReleasePlan's value and return nil", func() {
107-
ctx := context.Background()
108-
Expect(k8sClient.Create(ctx, releasePlan)).Should(Succeed())
109-
Expect(k8sClient.Create(ctx, release)).Should(Succeed())
110-
Expect(release.Spec.GracePeriodDays).To(Equal(releasePlan.Spec.ReleaseGracePeriodDays))
111-
112-
Expect(k8sClient.Delete(ctx, releasePlan)).Should(Succeed())
113-
})
114-
115-
It("should return nil and keep the default value of a go `int` for GracePeriodDays when the specified ReleasePlan does not exist", func() {
116-
ctx := context.Background()
117-
Expect(k8sClient.Create(ctx, release)).Should(Succeed())
118-
Expect(release.Spec.GracePeriodDays).To(Equal(0))
119-
})
120-
})
139+
}
121140
})

0 commit comments

Comments
 (0)