Skip to content

Commit 4c051dd

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 3178594 commit 4c051dd

File tree

6 files changed

+244
-82
lines changed

6 files changed

+244
-82
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: 29 additions & 20 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,39 +15,48 @@
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

3444
@Override
3545
@SuppressWarnings("unchecked")
3646
public CompletionStage<AdmissionResponse> handle(AdmissionRequest admissionRequest) {
37-
Operation operation = Operation.valueOf(admissionRequest.getOperation());
38-
var originalResource = (T) getTargetResource(admissionRequest, operation);
39-
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-
}
51-
5247
final var operation = Operation.valueOf(admissionRequest.getOperation());
48+
final var originalResource = (T) getTargetResource(admissionRequest, operation);
49+
final var clonedResource = cloner.clone(originalResource);
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+
});
61+
}
5362
}

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

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

3-
import org.junit.jupiter.api.Test;
4-
53
import io.fabric8.kubernetes.api.model.HasMetadata;
4+
import io.javaoperatorsdk.webhook.admission.mutation.Mutator;
5+
import io.javaoperatorsdk.webhook.admission.validation.Validator;
6+
import org.junit.jupiter.api.Test;
67

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

911
class AdmissionControllerTest {
1012

1113
AdmissionTestSupport admissionTestSupport = new AdmissionTestSupport();
1214

1315
@Test
1416
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-
});
17+
var admissionController = new AdmissionController<HasMetadata>((resource, operation) -> {
18+
});
2119
admissionTestSupport.validatesResource(admissionController::handle);
2220
}
2321

22+
@Test
23+
void validatesResource_whenNotAllowedException() {
24+
var admissionController =
25+
new AdmissionController<>((Validator<HasMetadata>) (resource, operation) -> {
26+
throw new NotAllowedException(MISSING_REQUIRED_LABEL);
27+
});
28+
admissionTestSupport.notAllowedException(admissionController::handle);
29+
}
30+
31+
@Test
32+
void validatesResource_whenOtherException() {
33+
var admissionController =
34+
new AdmissionController<>((Validator<HasMetadata>) (resource, operation) -> {
35+
throw new IllegalArgumentException("Invalid resource");
36+
});
37+
38+
admissionTestSupport.assertThatThrownBy(admissionController::handle)
39+
.isInstanceOf(IllegalArgumentException.class)
40+
.hasMessage("Invalid resource");
41+
}
42+
2443
@Test
2544
void mutatesResource() {
26-
AdmissionController<HasMetadata> admissionController =
27-
new AdmissionController<>((resource, operation) -> {
45+
var admissionController =
46+
new AdmissionController<HasMetadata>((resource, operation) -> {
2847
resource.getMetadata().getLabels().putIfAbsent(AdmissionTestSupport.LABEL_KEY,
2948
LABEL_TEST_VALUE);
3049
return resource;
3150
});
3251
admissionTestSupport.mutatesResource(admissionController::handle);
3352
}
53+
54+
@Test
55+
void mutatesResource_whenNotAllowedException() {
56+
var admissionController =
57+
new AdmissionController<>((Mutator<HasMetadata>) (resource, operation) -> {
58+
throw new NotAllowedException(MISSING_REQUIRED_LABEL);
59+
});
60+
61+
admissionTestSupport.notAllowedException(admissionController::handle);
62+
}
63+
64+
@Test
65+
void mutatesResource_whenOtherException() {
66+
var admissionController =
67+
new AdmissionController<>((Mutator<HasMetadata>) (resource, operation) -> {
68+
throw new IllegalArgumentException("Invalid resource");
69+
});
70+
71+
admissionTestSupport.assertThatThrownBy(admissionController::handle)
72+
.isInstanceOf(IllegalArgumentException.class)
73+
.hasMessage("Invalid resource");
74+
}
3475
}

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

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,20 +41,37 @@ void validatesResource(Function<AdmissionReview, AdmissionReview> function) {
4141

4242
assertThat(admissionReview.getResponse().getUid())
4343
.isEqualTo(inputAdmissionReview.getRequest().getUid());
44-
assertThat(admissionReview.getResponse().getStatus().getCode()).isEqualTo(403);
45-
assertThat(admissionReview.getResponse().getStatus().getMessage())
46-
.isEqualTo(MISSING_REQUIRED_LABEL);
47-
assertThat(admissionReview.getResponse().getAllowed()).isFalse();
44+
assertThat(admissionReview.getResponse().getAllowed()).isTrue();
45+
assertThat(admissionReview.getResponse().getStatus()).isNull();
4846
}
4947

5048
void mutatesResource(Function<AdmissionReview, AdmissionReview> function) {
5149
var inputAdmissionReview = createTestAdmissionReview();
5250
var admissionReview = function.apply(inputAdmissionReview);
5351

52+
assertThat(admissionReview.getResponse().getUid())
53+
.isEqualTo(inputAdmissionReview.getRequest().getUid());
5454
assertThat(admissionReview.getResponse().getAllowed()).isTrue();
5555
var patch = new String(Base64.getDecoder().decode(admissionReview.getResponse().getPatch()));
5656
assertThat(patch).isEqualTo(
5757
"[{\"op\":\"add\",\"path\":\"/metadata/labels/app.kubernetes.io~1name\",\"value\":\"mutation-test\"}]");
5858
}
5959

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

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

Lines changed: 110 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,46 +1,139 @@
11
package io.javaoperatorsdk.webhook.admission;
22

33
import java.util.concurrent.CompletableFuture;
4-
import java.util.concurrent.ExecutionException;
5-
6-
import org.junit.jupiter.api.Test;
4+
import java.util.concurrent.CompletionException;
75

86
import io.fabric8.kubernetes.api.model.HasMetadata;
97
import io.javaoperatorsdk.webhook.admission.mutation.AsyncMutator;
8+
import io.javaoperatorsdk.webhook.admission.mutation.Mutator;
9+
import io.javaoperatorsdk.webhook.admission.validation.Validator;
10+
import org.junit.jupiter.api.Test;
1011

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

1316
class AsyncAdmissionControllerTest {
1417

1518
AdmissionTestSupport admissionTestSupport = new AdmissionTestSupport();
1619

1720
@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-
21+
void validatesResource() {
22+
var admissionController = new AsyncAdmissionController<HasMetadata>((resource, operation) -> {
23+
});
2624

2725
admissionTestSupport
2826
.validatesResource(res -> admissionController.handle(res).toCompletableFuture().join());
27+
}
2928

29+
@Test
30+
void validatesResource_whenNotAllowedException() {
31+
var admissionController =
32+
new AsyncAdmissionController<>((Validator<HasMetadata>) (resource, operation) -> {
33+
throw new NotAllowedException(MISSING_REQUIRED_LABEL);
34+
});
35+
36+
admissionTestSupport
37+
.notAllowedException(res -> admissionController.handle(res).toCompletableFuture().join());
38+
}
3039

40+
@Test
41+
void validatesResource_whenOtherException() {
42+
var admissionController =
43+
new AsyncAdmissionController<>((Validator<HasMetadata>) (resource, operation) -> {
44+
throw new IllegalArgumentException("Invalid resource");
45+
});
46+
47+
admissionTestSupport.assertThatThrownBy(
48+
res -> admissionController.handle(res).toCompletableFuture()
49+
.join())
50+
.isInstanceOf(CompletionException.class)
51+
.hasCauseInstanceOf(IllegalStateException.class)
52+
.hasRootCauseInstanceOf(IllegalArgumentException.class)
53+
.hasRootCauseMessage("Invalid resource");
54+
}
55+
56+
@Test
57+
void mutatesResource_withMutator() {
58+
var admissionController =
59+
new AsyncAdmissionController<>((Mutator<HasMetadata>) (resource,
60+
operation) -> {
61+
resource.getMetadata().getLabels().putIfAbsent(LABEL_KEY, LABEL_TEST_VALUE);
62+
return resource;
63+
});
64+
65+
admissionTestSupport
66+
.mutatesResource(res -> admissionController.handle(res).toCompletableFuture().join());
3167
}
3268

3369
@Test
34-
void mutatesResource() throws ExecutionException, InterruptedException {
35-
AsyncAdmissionController<HasMetadata> admissionController =
70+
void mutatesResource_withAsyncMutator() {
71+
var admissionController =
3672
new AsyncAdmissionController<>((AsyncMutator<HasMetadata>) (resource,
3773
operation) -> CompletableFuture.supplyAsync(() -> {
38-
resource.getMetadata().getLabels().putIfAbsent(LABEL_KEY, LABEL_TEST_VALUE);
39-
return resource;
40-
}));
74+
resource.getMetadata().getLabels().putIfAbsent(LABEL_KEY, LABEL_TEST_VALUE);
75+
return resource;
76+
}));
4177

4278
admissionTestSupport
4379
.mutatesResource(res -> admissionController.handle(res).toCompletableFuture().join());
4480
}
4581

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

0 commit comments

Comments
 (0)