Skip to content

Validate AdmissionWebhookAdapter#597

Merged
scothis merged 1 commit intoreconcilerio:mainfrom
scothis:validate-webhook-reconciler
Feb 19, 2025
Merged

Validate AdmissionWebhookAdapter#597
scothis merged 1 commit intoreconcilerio:mainfrom
scothis:validate-webhook-reconciler

Conversation

@scothis
Copy link
Member

@scothis scothis commented Feb 19, 2025

Defines and calls Validate() on AdmissionWebhookAdapter when building the webhook. Calls validate with nested validation inside AdmissionWebhookTestCase.

AdmissionWebhookAdapter#Build is now deprecated in favor of AdmissionWebhookAdapter#BuildWithContext which accepts a context and returns an error when validation fails. The previous Build will use a TODO context and panic on validation errors.

AdmissionWebhookTestSuite#Run, AdmissionWebhookTests#Run, and AdmissionWebhookTestCase#Run are deprecated in favor of RunWithContext. The existing Run method delegates to RunWithContext, but requires users to manually configure nested validation (if desired).

Follow up to #583

Defines and calls Validate() on AdmissionWebhookAdapter when building
the webhook. Calls validate with nested validation inside
AdmissionWebhookTestCase.

AdmissionWebhookAdapter#Build is now deprecated in favor of
AdmissionWebhookAdapter#BuildWithContext which accepts a context and
returns an error when validation fails. The previous Build will use a
TODO context and panic on validation errors.

AdmissionWebhookTestSuite#Run, AdmissionWebhookTests#Run, and
AdmissionWebhookTestCase#Run are deprecated in favor of RunWithContext.
The existing Run method delegates to RunWithContext, but requires users
to manually configure nested validation (if desired).

Signed-off-by: Scott Andrews <scott@andrews.me>
@scothis scothis requested a review from mamachanko February 19, 2025 16:33
@codecov
Copy link

codecov bot commented Feb 19, 2025

Codecov Report

Attention: Patch coverage is 50.00000% with 31 lines in your changes missing coverage. Please review.

Project coverage is 59.25%. Comparing base (b2bb2eb) to head (2ffe211).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
testing/webhook.go 0.00% 24 Missing ⚠️
reconcilers/webhook.go 81.57% 6 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #597      +/-   ##
==========================================
- Coverage   59.35%   59.25%   -0.11%     
==========================================
  Files          35       35              
  Lines        3981     4032      +51     
==========================================
+ Hits         2363     2389      +26     
- Misses       1522     1547      +25     
  Partials       96       96              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@mamachanko mamachanko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had just started to work on this 😆 main...mamachanko:reconcilerio-runtime:topic/mamachanko/main/validate-webhooks, but I ran into the problems you solved with BuildWithContext.

This is excellent! ty

@scothis scothis merged commit d4697d5 into reconcilerio:main Feb 19, 2025
2 of 4 checks passed
@scothis scothis deleted the validate-webhook-reconciler branch February 19, 2025 16:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants