Skip to content

feat(webhook): Add webhook validation for SparkConnect CRD#2862

Merged
google-oss-prow[bot] merged 1 commit into
kubeflow:masterfrom
tariq-hasan:feat/sparkconnect-webhook-validation
Mar 13, 2026
Merged

feat(webhook): Add webhook validation for SparkConnect CRD#2862
google-oss-prow[bot] merged 1 commit into
kubeflow:masterfrom
tariq-hasan:feat/sparkconnect-webhook-validation

Conversation

@tariq-hasan

@tariq-hasan tariq-hasan commented Mar 2, 2026

Copy link
Copy Markdown
Member

Purpose of this PR

This PR introduces admission webhook validation for the SparkConnect CRD, aligning it with the existing validation patterns used for SparkApplication.

Change Category

  • Bugfix (non-breaking change which fixes an issue)
  • Feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that could affect existing functionality)
  • Documentation update

Rationale

The Spark Operator currently includes webhook validation for SparkApplication, but similar validation was not implemented for SparkConnect. This creates inconsistency and requires SDK-side validation that is better suited for controller-side enforcement.

This change:

  • Aligns SparkConnect behavior with existing Spark CRDs
  • Prevents invalid resource specifications at admission time
  • Allows SDK-side validation logic to be simplified or removed

Validation rules implemented:

  • Name must be DNS-1035 compliant (required for Service creation)
  • SparkVersion is required
  • Pod templates require Spark version >= 3.0.0
  • DynamicAllocation: minExecutors <= maxExecutors
  • DynamicAllocation: initialExecutors within min/max range
  • Memory format validation (e.g., 1g, 512m, 1024k)

Helm chart has been updated and make manifests has been run to regenerate webhook configs.

Checklist

  • I have conducted a self-review of my own code.
  • I have updated documentation accordingly.
  • I have added tests that prove my changes are effective or that my feature works.
  • Existing unit tests pass locally with my changes.

Additional Notes

Closes #2848.

Copilot AI review requested due to automatic review settings March 2, 2026 20:44
@google-oss-prow google-oss-prow Bot requested review from ImpSy and nabuskey March 2, 2026 20:44
Copilot AI reviewed Mar 2, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds admission webhooks for the SparkConnect (v1alpha1) CRD to enforce server-side validation/defaulting similar to existing Spark CRDs, and wires the new webhooks into generated manifests and the Helm chart.

Changes:

  • Registers SparkConnect in the webhook scheme and starts mutating/validating webhooks for the CRD.
  • Implements SparkConnect admission validation rules (name, sparkVersion, image presence, dynamic allocation bounds, memory string format) plus a defaulter.
  • Adds unit tests for the validator and updates webhook manifests/Helm templates to include SparkConnect webhooks.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
pkg/scheme/scheme.go Registers v1alpha1 types into the webhook scheme so defaults/webhooks can operate on SparkConnect.
internal/webhook/sparkconnect_validator.go New SparkConnect validating webhook implementation.
internal/webhook/sparkconnect_defaulter.go New SparkConnect mutating webhook (defaulting) implementation.
internal/webhook/sparkconnect_validator_test.go New unit tests covering SparkConnect validation behaviors.
cmd/operator/webhook/start.go Registers SparkConnect defaulter/validator with the webhook manager.
config/webhook/manifests.yaml Adds generated webhook configuration entries for SparkConnect.
charts/spark-operator-chart/templates/webhook/validatingwebhookconfiguration.yaml Helm: adds validating webhook entry for SparkConnect.
charts/spark-operator-chart/templates/webhook/mutatingwebhookconfiguration.yaml Helm: adds mutating webhook entry for SparkConnect.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread internal/webhook/sparkconnect_validator.go Outdated
Comment thread internal/webhook/sparkconnect_validator.go Outdated
Comment thread internal/webhook/sparkconnect_validator.go Outdated
Comment thread internal/webhook/sparkconnect_validator_test.go
@google-oss-prow google-oss-prow Bot added the lgtm label Mar 3, 2026
@tariq-hasan tariq-hasan mentioned this pull request Mar 4, 2026
Merged
4 tasks
@google-oss-prow google-oss-prow Bot removed the lgtm label Mar 5, 2026
@tariq-hasan tariq-hasan force-pushed the feat/sparkconnect-webhook-validation branch from 8f8c630 to fa6aba5 Compare March 5, 2026 06:02
@tariq-hasan tariq-hasan force-pushed the feat/sparkconnect-webhook-validation branch from fa6aba5 to 31cec01 Compare March 5, 2026 06:08
andreyvelich
andreyvelich reviewed Mar 13, 2026
View reviewed changes

@andreyvelich andreyvelich left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

/lgtm
/approve
/hold for final approval from @vara-bonthu and @nabuskey
cc @Shekharrajak

@google-oss-prow google-oss-prow Bot added the lgtm label Mar 13, 2026
@google-oss-prow

google-oss-prow Bot commented Mar 13, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: andreyvelich

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

nabuskey
nabuskey approved these changes Mar 13, 2026
View reviewed changes

@nabuskey nabuskey left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@andreyvelich

andreyvelich commented Mar 13, 2026

Copy link
Copy Markdown
Member

/hold cancel

@google-oss-prow google-oss-prow Bot merged commit 13593c5 into kubeflow:master Mar 13, 2026
18 checks passed
@andreyvelich

andreyvelich commented Mar 13, 2026

Copy link
Copy Markdown
Member

/cherry-pick release-2.5

@google-oss-robot

google-oss-robot commented Mar 13, 2026

Copy link
Copy Markdown

@andreyvelich: new pull request created: #2877

Details

In response to this:

/cherry-pick release-2.5

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@google-oss-robot google-oss-robot mentioned this pull request Mar 13, 2026
Merged
@tariq-hasan tariq-hasan mentioned this pull request Apr 16, 2026
Merged
@tariq-hasan tariq-hasan deleted the feat/sparkconnect-webhook-validation branch May 25, 2026 07:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@andreyvelich andreyvelich andreyvelich left review comments
Copilot code review Copilot Copilot left review comments
@nabuskey nabuskey nabuskey approved these changes
@ChenYi015 ChenYi015 ChenYi015 approved these changes
@ImpSy ImpSy Awaiting requested review from ImpSy

Assignees

@nabuskey nabuskey

@andreyvelich andreyvelich

@ChenYi015 ChenYi015

Labels

approved lgtm size/XL

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add webhook validation for SparkConnect CRD

6 participants

@tariq-hasan @andreyvelich @google-oss-robot @nabuskey @ChenYi015