Skip to content

Commit 6a6492f

Browse files
DonnerbartDonnerbart
committed
fix: fix exception handling in AsyncDefaultAdmissionRequestMutator and add constructor for Mutator interface
Signed-off-by: David Sondermann <[email protected]>
1 parent ef65577 commit 6a6492f

File tree

6 files changed

+239
-70
lines changed

6 files changed

+239
-70
lines changed

‎core/src/main/java/io/javaoperatorsdk/webhook/admission/AsyncAdmissionController.java‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,17 +6,22 @@
66
import io.fabric8.kubernetes.api.model.admission.v1.AdmissionReview;
77
import io.javaoperatorsdk.webhook.admission.mutation.AsyncDefaultAdmissionRequestMutator;
88
import io.javaoperatorsdk.webhook.admission.mutation.AsyncMutator;
9+
import io.javaoperatorsdk.webhook.admission.mutation.Mutator;
910
import io.javaoperatorsdk.webhook.admission.validation.AsyncDefaultAdmissionRequestValidator;
1011
import io.javaoperatorsdk.webhook.admission.validation.Validator;
1112

1213
public class AsyncAdmissionController<T extends KubernetesResource> {
1314

1415
private final AsyncAdmissionRequestHandler requestHandler;
1516

16-
public AsyncAdmissionController(AsyncMutator<T> mutator) {
17+
public AsyncAdmissionController(Mutator<T> mutator) {
1718
this(new AsyncDefaultAdmissionRequestMutator<>(mutator));
1819
}
1920

21+
public AsyncAdmissionController(AsyncMutator<T> asyncMutator) {
22+
this(new AsyncDefaultAdmissionRequestMutator<>(asyncMutator));
23+
}
24+
2025
public AsyncAdmissionController(Validator<T> validator) {
2126
this(new AsyncDefaultAdmissionRequestValidator<>(validator));
2227
}

‎core/src/main/java/io/javaoperatorsdk/webhook/admission/mutation/AsyncDefaultAdmissionRequestMutator.java‎

Lines changed: 26 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
package io.javaoperatorsdk.webhook.admission.mutation;
22

33
import java.util.concurrent.CompletableFuture;
4+
import java.util.concurrent.CompletionException;
45
import java.util.concurrent.CompletionStage;
56

67
import io.fabric8.kubernetes.api.model.KubernetesResource;
78
import io.fabric8.kubernetes.api.model.admission.v1.AdmissionRequest;
89
import io.fabric8.kubernetes.api.model.admission.v1.AdmissionResponse;
9-
import io.javaoperatorsdk.webhook.admission.AdmissionUtils;
1010
import io.javaoperatorsdk.webhook.admission.AsyncAdmissionRequestHandler;
1111
import io.javaoperatorsdk.webhook.admission.NotAllowedException;
1212
import io.javaoperatorsdk.webhook.admission.Operation;
@@ -15,19 +15,29 @@
1515

1616
import static io.javaoperatorsdk.webhook.admission.AdmissionUtils.admissionResponseFromMutation;
1717
import static io.javaoperatorsdk.webhook.admission.AdmissionUtils.getTargetResource;
18+
import static io.javaoperatorsdk.webhook.admission.AdmissionUtils.notAllowedExceptionToAdmissionResponse;
1819

1920
public class AsyncDefaultAdmissionRequestMutator<T extends KubernetesResource>
2021
implements AsyncAdmissionRequestHandler {
2122

22-
private final AsyncMutator<T> mutator;
23+
private final AsyncMutator<T> asyncMutator;
2324
private final Cloner<T> cloner;
2425

25-
public AsyncDefaultAdmissionRequestMutator(AsyncMutator<T> mutator) {
26+
public AsyncDefaultAdmissionRequestMutator(Mutator<T> mutator) {
2627
this(mutator, new ObjectMapperCloner<>());
2728
}
2829

29-
public AsyncDefaultAdmissionRequestMutator(AsyncMutator<T> mutator, Cloner<T> cloner) {
30-
this.mutator = mutator;
30+
public AsyncDefaultAdmissionRequestMutator(Mutator<T> mutator, Cloner<T> cloner) {
31+
this((AsyncMutator<T>) (resource, operation) -> CompletableFuture.supplyAsync(
32+
() -> mutator.mutate(resource, operation)), cloner);
33+
}
34+
35+
public AsyncDefaultAdmissionRequestMutator(AsyncMutator<T> asyncMutator) {
36+
this(asyncMutator, new ObjectMapperCloner<>());
37+
}
38+
39+
public AsyncDefaultAdmissionRequestMutator(AsyncMutator<T> asyncMutator, Cloner<T> cloner) {
40+
this.asyncMutator = asyncMutator;
3141
this.cloner = cloner;
3242
}
3343

@@ -37,15 +47,16 @@ public CompletionStage<AdmissionResponse> handle(AdmissionRequest admissionReque
3747
var operation = Operation.valueOf(admissionRequest.getOperation());
3848
var originalResource = (T) getTargetResource(admissionRequest, operation);
3949
var clonedResource = cloner.clone(originalResource);
40-
CompletionStage<AdmissionResponse> admissionResponse;
41-
try {
42-
var mutatedResource = mutator.mutate(clonedResource, operation);
43-
admissionResponse =
44-
mutatedResource.thenApply(mr -> admissionResponseFromMutation(originalResource, mr));
45-
} catch (NotAllowedException e) {
46-
admissionResponse = CompletableFuture
47-
.supplyAsync(() -> AdmissionUtils.notAllowedExceptionToAdmissionResponse(e));
48-
}
49-
return admissionResponse;
50+
return asyncMutator.mutate(clonedResource, operation)
51+
.thenApply(resource -> admissionResponseFromMutation(originalResource, resource))
52+
.exceptionally(e -> {
53+
if (e instanceof CompletionException) {
54+
if (e.getCause() instanceof NotAllowedException) {
55+
return notAllowedExceptionToAdmissionResponse((NotAllowedException) e.getCause());
56+
}
57+
throw new IllegalStateException(e.getCause());
58+
}
59+
throw new IllegalStateException(e);
60+
});
5061
}
5162
}

‎core/src/test/java/io/javaoperatorsdk/webhook/admission/AdmissionControllerTest.java‎

Lines changed: 51 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,32 +3,74 @@
33
import org.junit.jupiter.api.Test;
44

55
import io.fabric8.kubernetes.api.model.HasMetadata;
6+
import io.javaoperatorsdk.webhook.admission.mutation.Mutator;
7+
import io.javaoperatorsdk.webhook.admission.validation.Validator;
68

7-
import static io.javaoperatorsdk.webhook.admission.AdmissionTestSupport.*;
9+
import static io.javaoperatorsdk.webhook.admission.AdmissionTestSupport.LABEL_TEST_VALUE;
10+
import static io.javaoperatorsdk.webhook.admission.AdmissionTestSupport.MISSING_REQUIRED_LABEL;
811

912
class AdmissionControllerTest {
1013

1114
AdmissionTestSupport admissionTestSupport = new AdmissionTestSupport();
1215

1316
@Test
1417
void validatesResource() {
15-
AdmissionController<HasMetadata> admissionController =
16-
new AdmissionController<>((resource, operation) -> {
17-
if (resource.getMetadata().getLabels().get(AdmissionTestSupport.LABEL_KEY) == null) {
18-
throw new NotAllowedException(MISSING_REQUIRED_LABEL);
19-
}
20-
});
18+
var admissionController = new AdmissionController<HasMetadata>((resource, operation) -> {
19+
});
2120
admissionTestSupport.validatesResource(admissionController::handle);
2221
}
2322

23+
@Test
24+
void validatesResource_whenNotAllowedException() {
25+
var admissionController =
26+
new AdmissionController<>((Validator<HasMetadata>) (resource, operation) -> {
27+
throw new NotAllowedException(MISSING_REQUIRED_LABEL);
28+
});
29+
admissionTestSupport.notAllowedException(admissionController::handle);
30+
}
31+
32+
@Test
33+
void validatesResource_whenOtherException() {
34+
var admissionController =
35+
new AdmissionController<>((Validator<HasMetadata>) (resource, operation) -> {
36+
throw new IllegalArgumentException("Invalid resource");
37+
});
38+
39+
admissionTestSupport.assertThatThrownBy(admissionController::handle)
40+
.isInstanceOf(IllegalArgumentException.class)
41+
.hasMessage("Invalid resource");
42+
}
43+
2444
@Test
2545
void mutatesResource() {
26-
AdmissionController<HasMetadata> admissionController =
27-
new AdmissionController<>((resource, operation) -> {
46+
var admissionController =
47+
new AdmissionController<HasMetadata>((resource, operation) -> {
2848
resource.getMetadata().getLabels().putIfAbsent(AdmissionTestSupport.LABEL_KEY,
2949
LABEL_TEST_VALUE);
3050
return resource;
3151
});
3252
admissionTestSupport.mutatesResource(admissionController::handle);
3353
}
54+
55+
@Test
56+
void mutatesResource_whenNotAllowedException() {
57+
var admissionController =
58+
new AdmissionController<>((Mutator<HasMetadata>) (resource, operation) -> {
59+
throw new NotAllowedException(MISSING_REQUIRED_LABEL);
60+
});
61+
62+
admissionTestSupport.notAllowedException(admissionController::handle);
63+
}
64+
65+
@Test
66+
void mutatesResource_whenOtherException() {
67+
var admissionController =
68+
new AdmissionController<>((Mutator<HasMetadata>) (resource, operation) -> {
69+
throw new IllegalArgumentException("Invalid resource");
70+
});
71+
72+
admissionTestSupport.assertThatThrownBy(admissionController::handle)
73+
.isInstanceOf(IllegalArgumentException.class)
74+
.hasMessage("Invalid resource");
75+
}
3476
}

‎core/src/test/java/io/javaoperatorsdk/webhook/admission/AdmissionTestSupport.java‎

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,9 @@
55
import java.util.UUID;
66
import java.util.function.Function;
77

8+
import org.assertj.core.api.AbstractThrowableAssert;
9+
import org.assertj.core.api.Assertions;
10+
811
import io.fabric8.kubernetes.api.model.admission.v1.AdmissionRequest;
912
import io.fabric8.kubernetes.api.model.admission.v1.AdmissionReview;
1013
import io.fabric8.kubernetes.api.model.apps.Deployment;
@@ -39,20 +42,37 @@ void validatesResource(Function<AdmissionReview, AdmissionReview> function) {
3942

4043
assertThat(admissionReview.getResponse().getUid())
4144
.isEqualTo(inputAdmissionReview.getRequest().getUid());
42-
assertThat(admissionReview.getResponse().getStatus().getCode()).isEqualTo(403);
43-
assertThat(admissionReview.getResponse().getStatus().getMessage())
44-
.isEqualTo(MISSING_REQUIRED_LABEL);
45-
assertThat(admissionReview.getResponse().getAllowed()).isFalse();
45+
assertThat(admissionReview.getResponse().getAllowed()).isTrue();
46+
assertThat(admissionReview.getResponse().getStatus()).isNull();
4647
}
4748

4849
void mutatesResource(Function<AdmissionReview, AdmissionReview> function) {
4950
var inputAdmissionReview = createTestAdmissionReview();
5051
var admissionReview = function.apply(inputAdmissionReview);
5152

53+
assertThat(admissionReview.getResponse().getUid())
54+
.isEqualTo(inputAdmissionReview.getRequest().getUid());
5255
assertThat(admissionReview.getResponse().getAllowed()).isTrue();
5356
var patch = new String(Base64.getDecoder().decode(admissionReview.getResponse().getPatch()));
5457
assertThat(patch).isEqualTo(
5558
"[{\"op\":\"add\",\"path\":\"/metadata/labels/app.kubernetes.io~1name\",\"value\":\"mutation-test\"}]");
5659
}
5760

61+
void notAllowedException(Function<AdmissionReview, AdmissionReview> function) {
62+
var inputAdmissionReview = createTestAdmissionReview();
63+
var admissionReview = function.apply(inputAdmissionReview);
64+
65+
assertThat(admissionReview.getResponse().getUid())
66+
.isEqualTo(inputAdmissionReview.getRequest().getUid());
67+
assertThat(admissionReview.getResponse().getAllowed()).isFalse();
68+
assertThat(admissionReview.getResponse().getStatus().getCode()).isEqualTo(403);
69+
assertThat(admissionReview.getResponse().getStatus().getMessage()).isEqualTo(
70+
MISSING_REQUIRED_LABEL);
71+
}
72+
73+
AbstractThrowableAssert<?, ? extends Throwable> assertThatThrownBy(
74+
Function<AdmissionReview, AdmissionReview> function) {
75+
var inputAdmissionReview = createTestAdmissionReview();
76+
return Assertions.assertThatThrownBy(() -> function.apply(inputAdmissionReview));
77+
}
5878
}

‎core/src/test/java/io/javaoperatorsdk/webhook/admission/AsyncAdmissionControllerTest.java‎

Lines changed: 106 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,38 +1,75 @@
11
package io.javaoperatorsdk.webhook.admission;
22

33
import java.util.concurrent.CompletableFuture;
4-
import java.util.concurrent.ExecutionException;
4+
import java.util.concurrent.CompletionException;
55

66
import org.junit.jupiter.api.Test;
77

88
import io.fabric8.kubernetes.api.model.HasMetadata;
99
import io.javaoperatorsdk.webhook.admission.mutation.AsyncMutator;
10+
import io.javaoperatorsdk.webhook.admission.mutation.Mutator;
11+
import io.javaoperatorsdk.webhook.admission.validation.Validator;
1012

11-
import static io.javaoperatorsdk.webhook.admission.AdmissionTestSupport.*;
13+
import static io.javaoperatorsdk.webhook.admission.AdmissionTestSupport.LABEL_KEY;
14+
import static io.javaoperatorsdk.webhook.admission.AdmissionTestSupport.LABEL_TEST_VALUE;
15+
import static io.javaoperatorsdk.webhook.admission.AdmissionTestSupport.MISSING_REQUIRED_LABEL;
1216

1317
class AsyncAdmissionControllerTest {
1418

1519
AdmissionTestSupport admissionTestSupport = new AdmissionTestSupport();
1620

1721
@Test
18-
void validatesResource() throws ExecutionException, InterruptedException {
19-
AsyncAdmissionController<HasMetadata> admissionController =
20-
new AsyncAdmissionController<>((resource, operation) -> {
21-
if (resource.getMetadata().getLabels().get(LABEL_KEY) == null) {
22-
throw new NotAllowedException(MISSING_REQUIRED_LABEL);
23-
}
24-
});
25-
22+
void validatesResource() {
23+
var admissionController = new AsyncAdmissionController<HasMetadata>((resource, operation) -> {
24+
});
2625

2726
admissionTestSupport
2827
.validatesResource(res -> admissionController.handle(res).toCompletableFuture().join());
28+
}
29+
30+
@Test
31+
void validatesResource_whenNotAllowedException() {
32+
var admissionController =
33+
new AsyncAdmissionController<>((Validator<HasMetadata>) (resource, operation) -> {
34+
throw new NotAllowedException(MISSING_REQUIRED_LABEL);
35+
});
2936

37+
admissionTestSupport
38+
.notAllowedException(res -> admissionController.handle(res).toCompletableFuture().join());
39+
}
40+
41+
@Test
42+
void validatesResource_whenOtherException() {
43+
var admissionController =
44+
new AsyncAdmissionController<>((Validator<HasMetadata>) (resource, operation) -> {
45+
throw new IllegalArgumentException("Invalid resource");
46+
});
3047

48+
admissionTestSupport.assertThatThrownBy(
49+
res -> admissionController.handle(res).toCompletableFuture()
50+
.join())
51+
.isInstanceOf(CompletionException.class)
52+
.hasCauseInstanceOf(IllegalStateException.class)
53+
.hasRootCauseInstanceOf(IllegalArgumentException.class)
54+
.hasRootCauseMessage("Invalid resource");
3155
}
3256

3357
@Test
34-
void mutatesResource() throws ExecutionException, InterruptedException {
35-
AsyncAdmissionController<HasMetadata> admissionController =
58+
void mutatesResource_withMutator() {
59+
var admissionController =
60+
new AsyncAdmissionController<>((Mutator<HasMetadata>) (resource,
61+
operation) -> {
62+
resource.getMetadata().getLabels().putIfAbsent(LABEL_KEY, LABEL_TEST_VALUE);
63+
return resource;
64+
});
65+
66+
admissionTestSupport
67+
.mutatesResource(res -> admissionController.handle(res).toCompletableFuture().join());
68+
}
69+
70+
@Test
71+
void mutatesResource_withAsyncMutator() {
72+
var admissionController =
3673
new AsyncAdmissionController<>((AsyncMutator<HasMetadata>) (resource,
3774
operation) -> CompletableFuture.supplyAsync(() -> {
3875
resource.getMetadata().getLabels().putIfAbsent(LABEL_KEY, LABEL_TEST_VALUE);
@@ -43,4 +80,61 @@ void mutatesResource() throws ExecutionException, InterruptedException {
4380
.mutatesResource(res -> admissionController.handle(res).toCompletableFuture().join());
4481
}
4582

83+
@Test
84+
void mutatesResource_withMutator_whenNotAllowedException() {
85+
var admissionController =
86+
new AsyncAdmissionController<>((Mutator<HasMetadata>) (resource,
87+
operation) -> {
88+
throw new NotAllowedException(MISSING_REQUIRED_LABEL);
89+
});
90+
91+
admissionTestSupport.notAllowedException(
92+
res -> admissionController.handle(res).toCompletableFuture().join());
93+
}
94+
95+
@Test
96+
void mutatesResource_withAsyncMutator_whenNotAllowedException() {
97+
var admissionController =
98+
new AsyncAdmissionController<>((AsyncMutator<HasMetadata>) (resource,
99+
operation) -> CompletableFuture.supplyAsync(() -> {
100+
throw new NotAllowedException(MISSING_REQUIRED_LABEL);
101+
}));
102+
103+
admissionTestSupport.notAllowedException(
104+
res -> admissionController.handle(res).toCompletableFuture().join());
105+
}
106+
107+
@Test
108+
void mutatesResource_withMutator_whenOtherException() {
109+
var admissionController =
110+
new AsyncAdmissionController<>((Mutator<HasMetadata>) (resource,
111+
operation) -> {
112+
throw new IllegalArgumentException("Invalid resource");
113+
});
114+
115+
admissionTestSupport.assertThatThrownBy(
116+
res -> admissionController.handle(res).toCompletableFuture()
117+
.join())
118+
.isInstanceOf(CompletionException.class)
119+
.hasCauseInstanceOf(IllegalStateException.class)
120+
.hasRootCauseInstanceOf(IllegalArgumentException.class)
121+
.hasRootCauseMessage("Invalid resource");
122+
}
123+
124+
@Test
125+
void mutatesResource_withAsyncMutator_whenOtherException() {
126+
var admissionController =
127+
new AsyncAdmissionController<>((AsyncMutator<HasMetadata>) (resource,
128+
operation) -> CompletableFuture.supplyAsync(() -> {
129+
throw new IllegalArgumentException("Invalid resource");
130+
}));
131+
132+
admissionTestSupport.assertThatThrownBy(
133+
res -> admissionController.handle(res).toCompletableFuture()
134+
.join())
135+
.isInstanceOf(CompletionException.class)
136+
.hasCauseInstanceOf(IllegalStateException.class)
137+
.hasRootCauseInstanceOf(IllegalArgumentException.class)
138+
.hasRootCauseMessage("Invalid resource");
139+
}
46140
}

0 commit comments

Comments
 (0)