Skip to content

Commit 10f132a

Browse files
dlipovetskydlipovetsky
committed
fixup! feat: Add preflight checks framework
Remove Name from CheckResult, because the name is given by check.Name()
1 parent a4078ad commit 10f132a

File tree

2 files changed

+47
-47
lines changed

2 files changed

+47
-47
lines changed

‎pkg/webhook/preflight/preflight.go‎

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,6 @@ type (
4444
// list of causes for the failure. It also contains a list of warnings that were
4545
// generated during the check.
4646
CheckResult struct {
47-
Name string
48-
4947
Allowed bool
5048
Error bool
5149

@@ -77,6 +75,11 @@ func New(client ctrlclient.Client, decoder admission.Decoder, factories ...Check
7775
return h
7876
}
7977

78+
type namedResult struct {
79+
Name string
80+
CheckResult
81+
}
82+
8083
func (h *WebhookHandler) Handle(ctx context.Context, req admission.Request) admission.Response {
8184
if req.Operation == admissionv1.Delete {
8285
return admission.Allowed("")
@@ -146,8 +149,8 @@ func run(ctx context.Context,
146149
client ctrlclient.Client,
147150
cluster *clusterv1.Cluster,
148151
factories []CheckerFactory,
149-
) [][]CheckResult {
150-
resultsOrderedByCheckerAndCheck := make([][]CheckResult, len(factories))
152+
) [][]namedResult {
153+
resultsOrderedByCheckerAndCheck := make([][]namedResult, len(factories))
151154

152155
checkersWG := sync.WaitGroup{}
153156
for i, factory := range factories {
@@ -157,7 +160,7 @@ func run(ctx context.Context,
157160
checker := factory(client, cluster)
158161

159162
checks := checker.Init(ctx)
160-
resultsOrderedByCheck := make([]CheckResult, len(checks))
163+
resultsOrderedByCheck := make([]namedResult, len(checks))
161164

162165
checksWG := sync.WaitGroup{}
163166
for j, check := range checks {
@@ -166,13 +169,15 @@ func run(ctx context.Context,
166169
defer checksWG.Done()
167170
defer func() {
168171
if r := recover(); r != nil {
169-
resultsOrderedByCheck[j] = CheckResult{
170-
Name: check.Name(),
171-
Error: true,
172-
Causes: []Cause{
173-
{
174-
Message: fmt.Sprintf("internal error (panic): %s", r),
175-
Field: "",
172+
resultsOrderedByCheck[j] = namedResult{
173+
Name: check.Name(),
174+
CheckResult: CheckResult{
175+
Error: true,
176+
Causes: []Cause{
177+
{
178+
Message: fmt.Sprintf("internal error (panic): %s", r),
179+
Field: "",
180+
},
176181
},
177182
},
178183
}
@@ -187,7 +192,10 @@ func run(ctx context.Context,
187192
}
188193
}()
189194
result := check.Run(ctx)
190-
resultsOrderedByCheck[j] = result
195+
resultsOrderedByCheck[j] = namedResult{
196+
Name: check.Name(),
197+
CheckResult: result,
198+
}
191199
}(ctx, check, j)
192200
}
193201
checksWG.Wait()

‎pkg/webhook/preflight/preflight_test.go‎

Lines changed: 26 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,6 @@ func TestHandle(t *testing.T) {
215215
&mockCheck{
216216
name: "Test1",
217217
result: CheckResult{
218-
Name: "Test1",
219218
Allowed: false,
220219
Causes: []Cause{
221220
{
@@ -305,7 +304,6 @@ func TestHandle(t *testing.T) {
305304
&mockCheck{
306305
name: "Test1",
307306
result: CheckResult{
308-
Name: "Test1",
309307
Allowed: false,
310308
Error: true,
311309
Causes: []Cause{
@@ -322,7 +320,6 @@ func TestHandle(t *testing.T) {
322320
&mockCheck{
323321
name: "Test2",
324322
result: CheckResult{
325-
Name: "Test2",
326323
Allowed: false,
327324
Causes: []Cause{
328325
{
@@ -338,7 +335,6 @@ func TestHandle(t *testing.T) {
338335
&mockCheck{
339336
name: "Test3",
340337
result: CheckResult{
341-
Name: "Test3",
342338
Allowed: true,
343339
},
344340
},
@@ -442,7 +438,6 @@ func (c *cancellableCheck) Run(ctx context.Context) CheckResult {
442438
case <-ctx.Done():
443439
c.cancelled = true
444440
return CheckResult{
445-
Name: c.name,
446441
Allowed: false,
447442
Error: true,
448443
Causes: []Cause{
@@ -453,7 +448,6 @@ func (c *cancellableCheck) Run(ctx context.Context) CheckResult {
453448
}
454449
case <-time.After(c.delay):
455450
return CheckResult{
456-
Name: c.name,
457451
Allowed: true,
458452
}
459453
}
@@ -573,7 +567,6 @@ func TestRun_SingleCheckerSingleCheck(t *testing.T) {
573567
&mockCheck{
574568
name: "check1",
575569
result: CheckResult{
576-
Name: "check1",
577570
Allowed: true,
578571
},
579572
},
@@ -597,14 +590,12 @@ func TestRun_MultipleCheckersMultipleChecks(t *testing.T) {
597590
&mockCheck{
598591
name: "c1-check1",
599592
result: CheckResult{
600-
Name: "c1-check1",
601593
Allowed: true,
602594
},
603595
},
604596
&mockCheck{
605597
name: "c1-check2",
606598
result: CheckResult{
607-
Name: "c1-check2",
608599
Allowed: false,
609600
},
610601
},
@@ -615,7 +606,6 @@ func TestRun_MultipleCheckersMultipleChecks(t *testing.T) {
615606
&mockCheck{
616607
name: "c2-check1",
617608
result: CheckResult{
618-
Name: "c2-check1",
619609
Error: true,
620610
},
621611
},
@@ -663,7 +653,7 @@ func (c *delayedCheck) Run(ctx context.Context) CheckResult {
663653
c.mu.Unlock()
664654
}
665655

666-
return CheckResult{Name: c.name, Allowed: true}
656+
return CheckResult{Allowed: true}
667657
}
668658

669659
func TestRun_ChecksRunInParallel(t *testing.T) {
@@ -766,11 +756,11 @@ func TestRun_ContextCancellation(t *testing.T) {
766756
check := &contextAwareCheck{
767757
name: "cancellable-check",
768758
onCancel: func() CheckResult {
769-
return CheckResult{Name: "cancelled", Error: true}
759+
return CheckResult{Error: true}
770760
},
771761
onTimeout: func() CheckResult {
772762
close(completed)
773-
return CheckResult{Name: "completed", Allowed: true}
763+
return CheckResult{Allowed: true}
774764
},
775765
}
776766

@@ -870,7 +860,6 @@ func TestRun_LargeNumberOfCheckersAndChecks(t *testing.T) {
870860
checks[j] = &mockCheck{
871861
name: fmt.Sprintf("checker%d-check%d", checkerIndex, checkIndex),
872862
result: CheckResult{
873-
Name: fmt.Sprintf("checker%d-check%d", checkerIndex, checkIndex),
874863
Allowed: true,
875864
},
876865
}
@@ -906,7 +895,6 @@ func TestRun_ErrorHandlingInChecks(t *testing.T) {
906895
errorCheck := &mockCheck{
907896
name: "error-check",
908897
result: CheckResult{
909-
Name: "error-check",
910898
Error: true,
911899
Causes: []Cause{
912900
{
@@ -926,13 +914,15 @@ func TestRun_ErrorHandlingInChecks(t *testing.T) {
926914
assert.Len(t, resultsOrderedByCheckerAndCheck, 1, "expected results for 1 checker")
927915
assert.Len(t, resultsOrderedByCheckerAndCheck[0], 1, "expected 1 result from the checker")
928916

929-
wantErrorCheckResult := CheckResult{
930-
Name: "error-check",
931-
Error: true,
932-
Causes: []Cause{
933-
{
934-
Message: "simulated error in check",
935-
Field: "spec.errorField",
917+
wantErrorCheckResult := namedResult{
918+
Name: "error-check",
919+
CheckResult: CheckResult{
920+
Error: true,
921+
Causes: []Cause{
922+
{
923+
Message: "simulated error in check",
924+
Field: "spec.errorField",
925+
},
936926
},
937927
},
938928
}
@@ -971,7 +961,6 @@ func TestRun_PanicHandlingInChecks(t *testing.T) {
971961
normalCheck := &mockCheck{
972962
name: "normal-check",
973963
result: CheckResult{
974-
Name: "normal-check",
975964
Allowed: true,
976965
},
977966
}
@@ -989,17 +978,21 @@ func TestRun_PanicHandlingInChecks(t *testing.T) {
989978
assert.Len(t, resultsOrderedByCheckerAndCheck, 1, "expected results for 1 checker")
990979
assert.Len(t, resultsOrderedByCheckerAndCheck[0], 2, "expected 2 results from the checker")
991980

992-
wantNormalCheckResult := CheckResult{
993-
Name: "normal-check",
994-
Allowed: true,
981+
wantNormalCheckResult := namedResult{
982+
Name: "normal-check",
983+
CheckResult: CheckResult{
984+
Allowed: true,
985+
},
995986
}
996-
wantPanicCheckResult := CheckResult{
997-
Name: "panicking-check",
998-
Error: true,
999-
Causes: []Cause{
1000-
{
1001-
Message: "internal error (panic): simulated panic in check",
1002-
Field: "",
987+
wantPanicCheckResult := namedResult{
988+
Name: "panicking-check",
989+
CheckResult: CheckResult{
990+
Error: true,
991+
Causes: []Cause{
992+
{
993+
Message: "internal error (panic): simulated panic in check",
994+
Field: "",
995+
},
1003996
},
1004997
},
1005998
}
@@ -1021,7 +1014,6 @@ func TestRun_ZeroChecksFromChecker(t *testing.T) {
10211014
&mockCheck{
10221015
name: "check1",
10231016
result: CheckResult{
1024-
Name: "check1",
10251017
Allowed: true,
10261018
},
10271019
},

0 commit comments

Comments
 (0)