Skip to content

Commit 66be336

Browse files
DirectXMan12DirectXMan12
committed
Clean up tests to follow standards
This cleans up tests to more closely follow CR style, favoring behavior-style tests as opposed to pure unit tests.
1 parent c7969dc commit 66be336

File tree

3 files changed

+118
-110
lines changed

3 files changed

+118
-110
lines changed

‎pkg/webhook/admission/decode_test.go

Lines changed: 35 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -22,33 +22,25 @@ import (
2222

2323
admissionv1beta1 "k8s.io/api/admission/v1beta1"
2424
corev1 "k8s.io/api/core/v1"
25+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2526
"k8s.io/apimachinery/pkg/runtime"
2627
"k8s.io/client-go/kubernetes/scheme"
2728
)
2829

29-
var _ = Describe("admission webhook decoder", func() {
30+
var _ = Describe("Admission Webhook Decoder", func() {
3031
var decoder *Decoder
31-
BeforeEach(func(done Done) {
32+
BeforeEach(func() {
33+
By("creating a new decoder for a scheme")
3234
var err error
3335
decoder, err = NewDecoder(scheme.Scheme)
3436
Expect(err).NotTo(HaveOccurred())
3537
Expect(decoder).NotTo(BeNil())
36-
close(done)
3738
})
3839

39-
Describe("NewDecoder", func() {
40-
It("should return a decoder without an error", func() {
41-
decoder, err := NewDecoder(scheme.Scheme)
42-
Expect(err).NotTo(HaveOccurred())
43-
Expect(decoder).NotTo(BeNil())
44-
})
45-
})
46-
47-
Describe("Decode", func() {
48-
req := Request{
49-
AdmissionRequest: admissionv1beta1.AdmissionRequest{
50-
Object: runtime.RawExtension{
51-
Raw: []byte(`{
40+
req := Request{
41+
AdmissionRequest: admissionv1beta1.AdmissionRequest{
42+
Object: runtime.RawExtension{
43+
Raw: []byte(`{
5244
"apiVersion": "v1",
5345
"kind": "Pod",
5446
"metadata": {
@@ -64,19 +56,35 @@ var _ = Describe("admission webhook decoder", func() {
6456
]
6557
}
6658
}`),
67-
},
6859
},
69-
}
60+
},
61+
}
7062

71-
It("should be able to decode", func() {
72-
err := decoder.Decode(req, &corev1.Pod{})
73-
Expect(err).NotTo(HaveOccurred())
74-
})
63+
It("should decode a valid admission request", func() {
64+
By("extracting the object from the request")
65+
var actualObj corev1.Pod
66+
Expect(decoder.Decode(req, &actualObj)).To(Succeed())
67+
68+
By("verifying that all data is present in the object")
69+
Expect(actualObj).To(Equal(corev1.Pod{
70+
TypeMeta: metav1.TypeMeta{
71+
APIVersion: "v1",
72+
Kind: "Pod",
73+
},
74+
ObjectMeta: metav1.ObjectMeta{
75+
Name: "foo",
76+
Namespace: "default",
77+
},
78+
Spec: corev1.PodSpec{
79+
Containers: []corev1.Container{
80+
{Image: "bar", Name: "bar"},
81+
},
82+
},
83+
}))
84+
})
7585

76-
It("should return an error if the GVK mismatch", func() {
77-
err := decoder.Decode(req, &corev1.Node{})
78-
Expect(err).To(HaveOccurred())
79-
Expect(err.Error()).Should(ContainSubstring("unable to decode"))
80-
})
86+
It("should fail to decode if the object in the request doesn't match the passed-in type", func() {
87+
By("trying to extract a pod into a node")
88+
Expect(decoder.Decode(req, &corev1.Node{})).NotTo(Succeed())
8189
})
8290
})

‎pkg/webhook/admission/multi_test.go

Lines changed: 82 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -26,107 +26,107 @@ import (
2626
admissionv1beta1 "k8s.io/api/admission/v1beta1"
2727
)
2828

29-
var _ = Describe("Admission Webhooks", func() {
30-
Describe("Multi-handler Webhooks", func() {
31-
alwaysAllow := &fakeHandler{
29+
var _ = Describe("Multi-Handler Admission Webhooks", func() {
30+
alwaysAllow := &fakeHandler{
31+
fn: func(ctx context.Context, req Request) Response {
32+
return Response{
33+
AdmissionResponse: admissionv1beta1.AdmissionResponse{
34+
Allowed: true,
35+
},
36+
}
37+
},
38+
}
39+
alwaysDeny := &fakeHandler{
40+
fn: func(ctx context.Context, req Request) Response {
41+
return Response{
42+
AdmissionResponse: admissionv1beta1.AdmissionResponse{
43+
Allowed: false,
44+
},
45+
}
46+
},
47+
}
48+
49+
Context("with validating handlers", func() {
50+
It("should deny the request if any handler denies the request", func() {
51+
By("setting up a handler with accept and deny")
52+
handler := MultiValidatingHandler(alwaysAllow, alwaysDeny)
53+
54+
By("checking that the handler denies the request")
55+
resp := handler.Handle(context.Background(), Request{})
56+
Expect(resp.Allowed).To(BeFalse())
57+
})
58+
59+
It("should allow the request if all handlers allow the request", func() {
60+
By("setting up a handler with only accept")
61+
handler := MultiValidatingHandler(alwaysAllow, alwaysAllow)
62+
63+
By("checking that the handler allows the request")
64+
resp := handler.Handle(context.Background(), Request{})
65+
Expect(resp.Allowed).To(BeTrue())
66+
})
67+
})
68+
69+
Context("with mutating handlers", func() {
70+
patcher1 := &fakeHandler{
3271
fn: func(ctx context.Context, req Request) Response {
3372
return Response{
73+
Patches: []jsonpatch.JsonPatchOperation{
74+
{
75+
Operation: "add",
76+
Path: "/metadata/annotation/new-key",
77+
Value: "new-value",
78+
},
79+
{
80+
Operation: "replace",
81+
Path: "/spec/replicas",
82+
Value: "2",
83+
},
84+
},
3485
AdmissionResponse: admissionv1beta1.AdmissionResponse{
35-
Allowed: true,
86+
Allowed: true,
87+
PatchType: func() *admissionv1beta1.PatchType { pt := admissionv1beta1.PatchTypeJSONPatch; return &pt }(),
3688
},
3789
}
3890
},
3991
}
40-
alwaysDeny := &fakeHandler{
92+
patcher2 := &fakeHandler{
4193
fn: func(ctx context.Context, req Request) Response {
4294
return Response{
95+
Patches: []jsonpatch.JsonPatchOperation{
96+
{
97+
Operation: "add",
98+
Path: "/metadata/annotation/hello",
99+
Value: "world",
100+
},
101+
},
43102
AdmissionResponse: admissionv1beta1.AdmissionResponse{
44-
Allowed: false,
103+
Allowed: true,
104+
PatchType: func() *admissionv1beta1.PatchType { pt := admissionv1beta1.PatchTypeJSONPatch; return &pt }(),
45105
},
46106
}
47107
},
48108
}
49109

50-
Context("with validating handlers", func() {
51-
It("should deny the request if any handler denies the request", func() {
52-
By("setting up a handler with accept and deny")
53-
handler := MultiValidatingHandler(alwaysAllow, alwaysDeny)
54-
55-
By("checking that the handler denies the request")
56-
resp := handler.Handle(context.Background(), Request{})
57-
Expect(resp.Allowed).To(BeFalse())
58-
})
110+
It("should not return any patches if the request is denied", func() {
111+
By("setting up a webhook with some patches and a deny")
112+
handler := MultiMutatingHandler(patcher1, patcher2, alwaysDeny)
59113

60-
It("should allow the request if all handlers allow the request", func() {
61-
By("setting up a handler with only accept")
62-
handler := MultiValidatingHandler(alwaysAllow, alwaysAllow)
63-
64-
By("checking that the handler allows the request")
65-
resp := handler.Handle(context.Background(), Request{})
66-
Expect(resp.Allowed).To(BeTrue())
67-
})
114+
By("checking that the handler denies the request and produces no patches")
115+
resp := handler.Handle(context.Background(), Request{})
116+
Expect(resp.Allowed).To(BeFalse())
117+
Expect(resp.Patches).To(BeEmpty())
68118
})
69119

70-
Context("with mutating handlers", func() {
71-
patcher1 := &fakeHandler{
72-
fn: func(ctx context.Context, req Request) Response {
73-
return Response{
74-
Patches: []jsonpatch.JsonPatchOperation{
75-
{
76-
Operation: "add",
77-
Path: "/metadata/annotation/new-key",
78-
Value: "new-value",
79-
},
80-
{
81-
Operation: "replace",
82-
Path: "/spec/replicas",
83-
Value: "2",
84-
},
85-
},
86-
AdmissionResponse: admissionv1beta1.AdmissionResponse{
87-
Allowed: true,
88-
PatchType: func() *admissionv1beta1.PatchType { pt := admissionv1beta1.PatchTypeJSONPatch; return &pt }(),
89-
},
90-
}
91-
},
92-
}
93-
patcher2 := &fakeHandler{
94-
fn: func(ctx context.Context, req Request) Response {
95-
return Response{
96-
Patches: []jsonpatch.JsonPatchOperation{
97-
{
98-
Operation: "add",
99-
Path: "/metadata/annotation/hello",
100-
Value: "world",
101-
},
102-
},
103-
AdmissionResponse: admissionv1beta1.AdmissionResponse{
104-
Allowed: true,
105-
PatchType: func() *admissionv1beta1.PatchType { pt := admissionv1beta1.PatchTypeJSONPatch; return &pt }(),
106-
},
107-
}
108-
},
109-
}
120+
It("should produce all patches if the requests are all allowed", func() {
121+
By("setting up a webhook with some patches")
122+
handler := MultiMutatingHandler(patcher1, patcher2, alwaysAllow)
110123

111-
It("should not return any patches if the request is denied", func() {
112-
By("setting up a webhook with some patches and a deny")
113-
handler := MultiMutatingHandler(patcher1, patcher2, alwaysDeny)
114-
115-
By("checking that the handler denies the request and produces no patches")
116-
resp := handler.Handle(context.Background(), Request{})
117-
Expect(resp.Allowed).To(BeFalse())
118-
Expect(resp.Patches).To(BeEmpty())
119-
})
120-
121-
It("should produce all patches if the requests are all allowed", func() {
122-
By("setting up a webhook with some patches")
123-
handler := MultiMutatingHandler(patcher1, patcher2, alwaysAllow)
124-
125-
By("checking that the handler accepts the request and returns all patches")
126-
resp := handler.Handle(context.Background(), Request{})
127-
Expect(resp.Allowed).To(BeTrue())
128-
Expect(resp.Patch).To(Equal([]byte(`[{"op":"add","path":"/metadata/annotation/new-key","value":"new-value"},{"op":"replace","path":"/spec/replicas","value":"2"},{"op":"add","path":"/metadata/annotation/hello","value":"world"}]`)))
129-
})
124+
By("checking that the handler accepts the request and returns all patches")
125+
resp := handler.Handle(context.Background(), Request{})
126+
Expect(resp.Allowed).To(BeTrue())
127+
Expect(resp.Patch).To(Equal([]byte(
128+
`[{"op":"add","path":"/metadata/annotation/new-key","value":"new-value"},` +
129+
`{"op":"replace","path":"/spec/replicas","value":"2"},{"op":"add","path":"/metadata/annotation/hello","value":"world"}]`)))
130130
})
131131
})
132132
})

‎pkg/webhook/admission/response_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ import (
2828
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2929
)
3030

31-
var _ = Describe("admission webhook response", func() {
31+
var _ = Describe("Admission Webhook Response Helpers", func() {
3232
Describe("ErrorResponse", func() {
3333
It("should return the response with an error", func() {
3434
err := errors.New("this is an error")

0 commit comments

Comments
 (0)