-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix exception handling in AsyncDefaultAdmissionRequestMutator #284
Merged
csviri
merged 5 commits into
operator-framework:main
from
Donnerbart:bugfix/fix-async-mutator-exception-handling
Nov 20, 2024
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
1f82202
refactor: unify usage of var keyword and formatting
Donnerbart 2ad960e
refactor: fix suppressions
Donnerbart 6b9a484
refactor: pull out allowedAdmissionResponse method
Donnerbart ef65577
refactor: make EndToEndTestBase a proper abstract class
Donnerbart 6a6492f
fix: fix exception handling in AsyncDefaultAdmissionRequestMutator an…
Donnerbart File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,5 +6,4 @@ | |
public interface AdmissionRequestHandler { | ||
|
||
AdmissionResponse handle(AdmissionRequest admissionRequest); | ||
|
||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,12 +1,12 @@ | ||
package io.javaoperatorsdk.webhook.admission.mutation; | ||
|
||
import java.util.concurrent.CompletableFuture; | ||
import java.util.concurrent.CompletionException; | ||
import java.util.concurrent.CompletionStage; | ||
|
||
import io.fabric8.kubernetes.api.model.KubernetesResource; | ||
import io.fabric8.kubernetes.api.model.admission.v1.AdmissionRequest; | ||
import io.fabric8.kubernetes.api.model.admission.v1.AdmissionResponse; | ||
import io.javaoperatorsdk.webhook.admission.AdmissionUtils; | ||
import io.javaoperatorsdk.webhook.admission.AsyncAdmissionRequestHandler; | ||
import io.javaoperatorsdk.webhook.admission.NotAllowedException; | ||
import io.javaoperatorsdk.webhook.admission.Operation; | ||
|
@@ -15,37 +15,48 @@ | |
|
||
import static io.javaoperatorsdk.webhook.admission.AdmissionUtils.admissionResponseFromMutation; | ||
import static io.javaoperatorsdk.webhook.admission.AdmissionUtils.getTargetResource; | ||
import static io.javaoperatorsdk.webhook.admission.AdmissionUtils.notAllowedExceptionToAdmissionResponse; | ||
|
||
public class AsyncDefaultAdmissionRequestMutator<T extends KubernetesResource> | ||
implements AsyncAdmissionRequestHandler { | ||
|
||
private final AsyncMutator<T> mutator; | ||
private final AsyncMutator<T> asyncMutator; | ||
private final Cloner<T> cloner; | ||
|
||
public AsyncDefaultAdmissionRequestMutator(AsyncMutator<T> mutator) { | ||
public AsyncDefaultAdmissionRequestMutator(Mutator<T> mutator) { | ||
this(mutator, new ObjectMapperCloner<>()); | ||
} | ||
|
||
public AsyncDefaultAdmissionRequestMutator(AsyncMutator<T> mutator, Cloner<T> cloner) { | ||
this.mutator = mutator; | ||
public AsyncDefaultAdmissionRequestMutator(Mutator<T> mutator, Cloner<T> cloner) { | ||
this((AsyncMutator<T>) (resource, operation) -> CompletableFuture.supplyAsync( | ||
() -> mutator.mutate(resource, operation)), cloner); | ||
} | ||
|
||
public AsyncDefaultAdmissionRequestMutator(AsyncMutator<T> asyncMutator) { | ||
this(asyncMutator, new ObjectMapperCloner<>()); | ||
} | ||
|
||
public AsyncDefaultAdmissionRequestMutator(AsyncMutator<T> asyncMutator, Cloner<T> cloner) { | ||
this.asyncMutator = asyncMutator; | ||
this.cloner = cloner; | ||
} | ||
|
||
@Override | ||
@SuppressWarnings("unchecked") | ||
public CompletionStage<AdmissionResponse> handle(AdmissionRequest admissionRequest) { | ||
Operation operation = Operation.valueOf(admissionRequest.getOperation()); | ||
var operation = Operation.valueOf(admissionRequest.getOperation()); | ||
var originalResource = (T) getTargetResource(admissionRequest, operation); | ||
var clonedResource = cloner.clone(originalResource); | ||
CompletionStage<AdmissionResponse> admissionResponse; | ||
try { | ||
var mutatedResource = mutator.mutate(clonedResource, operation); | ||
admissionResponse = | ||
mutatedResource.thenApply(mr -> admissionResponseFromMutation(originalResource, mr)); | ||
} catch (NotAllowedException e) { | ||
admissionResponse = CompletableFuture | ||
.supplyAsync(() -> AdmissionUtils.notAllowedExceptionToAdmissionResponse(e)); | ||
} | ||
return admissionResponse; | ||
return asyncMutator.mutate(clonedResource, operation) | ||
.thenApply(resource -> admissionResponseFromMutation(originalResource, resource)) | ||
.exceptionally(e -> { | ||
if (e instanceof CompletionException) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the actual fix. |
||
if (e.getCause() instanceof NotAllowedException) { | ||
return notAllowedExceptionToAdmissionResponse((NotAllowedException) e.getCause()); | ||
} | ||
throw new IllegalStateException(e.getCause()); | ||
} | ||
throw new IllegalStateException(e); | ||
}); | ||
} | ||
|
||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the internal convenience constructor to enable the use of a
Mutator
for an async endpoint.