Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fixed AdmissionResponse.PatchType to an optional value #537

Merged
merged 1 commit into from Feb 27, 2023
Merged

fixed AdmissionResponse.PatchType to an optional value #537

merged 1 commit into from Feb 27, 2023

Conversation

tomasfabian
Copy link
Contributor

@tomasfabian tomasfabian commented Feb 23, 2023 •
edited by buehler

This PR changes back the AdmissionResponse.PatchType to an optional value #536 since validation web hooks cannot contain a PatchType.

Fixes #536

@buehler buehler merged commit 0d9e504 into buehler:master Feb 27, 2023
@tomasfabian
Copy link
Contributor Author

tomasfabian commented Feb 27, 2023 •
edited

Hi Christhoph @buehler,
something came to my mind regarding this fix.
Since we disabled the default JSONPatch value for validating web hooks, the mutating web hooks now don't have it filled from the AdmissionResponse.JsonPatch const.

Before v7.x.x it was done in this way.

Do you agree that we should add back the bellow assignment intoIAdmissionWebhook<TEntity, MutationResult>.TransformResult function in IMutationWebhook<TEntity>, please?

if (result.ModifiedObject != null)
{
    //...
    response.PatchType = AdmissionResponse.JsonPatch;
}

See the follow up PR #540 .

@tomasfabian tomasfabian mentioned this pull request Feb 27, 2023
Merged
buehler added a commit that referenced this pull request Mar 1, 2023
@tomasfabian @buehler
fix: AdmissionResponse.PatchType set to AdmissionResponse.JsonPatch i…
dcf1f49 
…n mutating webhooks (#540)

…in mutating webhooks for modifications. This is a follow up fix for
#536 #537.

Co-authored-by: Christoph Bühler <buehler@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[bug]: Validating webhook may not return response.patchType
2 participants
@tomasfabian @buehler