feat(sparkconnect): add a `Dependencies` field to `SparkConnectSpec` by Mr-Neutr0n · Pull Request #3120 · kubeflow/spark-operator

Mr-Neutr0n · 2026-08-26T05:42:11Z

Added a Dependencies struct and spec.dependencies field to SparkConnectSpec, and wired the Spark Connect controller to pass dependency flags through to start-connect-server.sh the same way SparkApplication does for spark-submit.

Local tests pass.

This change was prepared with AI assistance under human direction and review.

github-actions · 2026-08-26T05:42:20Z

🎉 Welcome to the Kubeflow Spark Operator! 🎉

Thanks for opening your first PR! We're happy to have you as part of our community 🚀

Here's what happens next:

If you haven't already, please check out our Contributing Guide for repo-specific guidelines and the Kubeflow Contributor Guide for general community standards.
Our team will review your PR soon!

Join the community:

Slack: Join our #kubeflow-spark-on-kubernetes Slack channel.

Feel free to ask questions in the comments if you need any help or clarification!
Thanks again for contributing to Kubeflow! 🙏

nabuskey · 2026-09-02T19:56:52Z

You need to run make generate to auto generate the boilerplate code for added fields. Also we should match the field name to deps instead of dependencies.
Also, the passed in bash strings need to be handled like so: #3051

Let's add more tests too.

Mr-Neutr0n · 2026-09-03T07:53:58Z

Addressed the review: aligned Spark Connect with the SparkApplication deps field, shell-quoted all dependency arguments for the bash -c entrypoint, added shell-sensitive dependency coverage, and regenerated Go, CRD, OpenAPI, and Python API artifacts. d024dc8

tariq-hasan · 2026-09-03T11:13:45Z

/retitle feat(sparkconnect): add a Dependencies field to SparkConnectSpec

tariq-hasan · 2026-09-03T11:14:38Z

/ok-to-test
/retest

tariq-hasan

Hi @Mr-Neutr0n! I have added some more comments.

Please also update docs/website/user-guide/spark-connect.md, add a new examples/sparkconnect/spark-connect-dependencies.yaml and add/update e2e tests.

Mr-Neutr0n · 2026-09-17T13:09:02Z

Addressed the review in 244c6dd:

Dependencies now carries only jars, packages, excludePackages, and repositories. files, pyFiles, and archives are gone from the API type, the controller, and the regenerated artifacts (deepcopy, OpenAPI, Swagger, CRDs, Python API). The type and field comments now describe the Spark Connect server instead of a Spark application.
.spec.deps is documented in docs/website/user-guide/spark-connect.md.
Added examples/sparkconnect/spark-connect-dependencies.yaml.
Added test/e2e/sparkconnect_dependencies_test.go, which starts the server from the new example and asserts that the dependency flags reach spark-submit.

Verification: make unit-test (with envtest), make go-vet, make go-lint (0 issues) and a build of the e2e module all pass. I could not execute the e2e suite locally (no container runtime for kind here), so that test compiles but was not run in this environment.

Mr-Neutr0n · 2026-09-18T03:20:15Z

Two CI checks needed generated updates rather than code changes: the Python model was missing the generator's trailing blank lines, and docs/api-docs.md had to be regenerated for the new v1alpha1 type. I rebased onto current master, regenerated all artifacts on that base, and re-ran the checks locally (make unit-test, go vet, golangci-lint 0 issues, root and e2e builds). The e2e matrix had been cancelled by fail-fast on the previous run, so it should run fresh on this head.

Mr-Neutr0n · 2026-09-19T02:49:04Z

The e2e run was right: the dependencies test timed out waiting for the server pod to become ready, which needs the kind cluster to resolve the Maven artifacts (the rest of the suite only uses artifacts bundled in the Spark image). I narrowed the test to what this change owns: the SparkConnect is reconciled into a server pod whose spark-submit command carries the --jars, --packages, --exclude-packages, and --repositories flags. Happy to extend it to a full startup check if the e2e cluster has Maven egress.

Add spec.deps to Spark Connect with the Maven-related fields from the review: jars, packages, excludePackages, and repositories. The controller passes them to spark-submit as --jars, --packages, --exclude-packages, and --repositories. - Regenerate the deepcopy, OpenAPI, Swagger, CRD, API docs, and Python API artifacts. - Document .spec.deps in the Spark Connect user guide. - Add examples/sparkconnect/spark-connect-dependencies.yaml. - Add an e2e test that starts the server from the new example and checks that the dependency flags reach spark-submit. - Drop two unnecessary string conversions flagged by golangci-lint. Fixes kubeflow#2959 Signed-off-by: Mr-Neutr0n <64578610+Mr-Neutr0n@users.noreply.github.com>

Mr-Neutr0n · 2026-09-30T14:55:07Z

Rebased onto current master and resolved the conflicts on the same branch (agent/issue-2959-add-a-dependencies-fie, head ae912f8).

Conflicts were with the Spark Connect GPU work and #3188 (per-arg container args). dependenciesOption now emits --jars / --packages / --exclude-packages / --repositories as individual container arguments (no shell quoting), matching the new entrypoint; unit + e2e tests updated for that shape.

Review asks still hold: only jars/packages/excludePackages/repositories; sparkconnect-worded comments; docs/example/e2e coverage; regenerated deepcopy/OpenAPI/CRDs/API docs/Python models clean. go test ./internal/controller/sparkconnect/... passes. PR is MERGEABLE again — remaining BLOCKED is review/CI, not conflicts.

tariq-hasan

Hi @Mr-Neutr0n! Thanks for addressing the earlier comments. I have some final comments on the implementation - mostly around the example YAML and the e2e test.

tariq-hasan · 2026-10-11T15:31:36Z

+#
+# Copyright 2026 The Kubeflow authors.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     https://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.


Nit: Let's match the copyright with the template from the testing repo: https://github.com/kubeflow/testing/blob/8483606e0f8a34bf647f3b62a516efa17096c58c/hack/boilerplate/boilerplate.sh.txt.

Suggested change

#

# Copyright 2026 The Kubeflow authors.

#

# Licensed under the Apache License, Version 2.0 (the "License");

# you may not use this file except in compliance with the License.

# You may obtain a copy of the License at

#

# https://www.apache.org/licenses/LICENSE-2.0

#

# Unless required by applicable law or agreed to in writing, software

# distributed under the License is distributed on an "AS IS" BASIS,

# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

# See the License for the specific language governing permissions and

# limitations under the License.

# Copyright The Kubeflow Authors.

#

# Licensed under the Apache License, Version 2.0 (the "License");

# you may not use this file except in compliance with the License.

# You may obtain a copy of the License at

#

# http://www.apache.org/licenses/LICENSE-2.0

#

# Unless required by applicable law or agreed to in writing, software

# distributed under the License is distributed on an "AS IS" BASIS,

# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

# See the License for the specific language governing permissions and

# limitations under the License.

tariq-hasan · 2026-10-11T15:36:08Z

+  server:
+    template:
+      spec:
+        containers:
+        - name: spark-kubernetes-driver
+          image: docker.io/apache/spark:4.0.4
+          imagePullPolicy: IfNotPresent
+        serviceAccount: spark-operator-spark
+        securityContext:
+          capabilities:
+            drop:
+            - ALL
+          runAsGroup: 185
+          runAsUser: 185
+          runAsNonRoot: true
+          allowPrivilegeEscalation: false
+          seccompProfile:
+            type: RuntimeDefault
+  executor:
+    instances: 1
+    cores: 1
+    memory: 512m
+    template:
+      spec:
+        containers:
+        - name: spark-kubernetes-executor
+          image: docker.io/apache/spark:4.0.4
+          imagePullPolicy: IfNotPresent
+        securityContext:
+          capabilities:
+            drop:
+            - ALL
+          runAsGroup: 185
+          runAsUser: 185
+          runAsNonRoot: true
+          allowPrivilegeEscalation: false
+          seccompProfile:
+            type: RuntimeDefault


Let's vary only on spec.deps and match the rest of the spec with examples/sparkconnect/spark-connect.yaml to ensure we isolate the spec.deps behavior in the e2e.

Suggested change

server:

template:

spec:

containers:

- name: spark-kubernetes-driver

image: docker.io/apache/spark:4.0.4

imagePullPolicy: IfNotPresent

serviceAccount: spark-operator-spark

securityContext:

capabilities:

drop:

- ALL

runAsGroup: 185

runAsUser: 185

runAsNonRoot: true

allowPrivilegeEscalation: false

seccompProfile:

type: RuntimeDefault

executor:

instances: 1

cores: 1

memory: 512m

template:

spec:

containers:

- name: spark-kubernetes-executor

image: docker.io/apache/spark:4.0.4

imagePullPolicy: IfNotPresent

securityContext:

capabilities:

drop:

- ALL

runAsGroup: 185

runAsUser: 185

runAsNonRoot: true

allowPrivilegeEscalation: false

seccompProfile:

type: RuntimeDefault

server:

template:

metadata:

labels:

key1: value1

key2: value2

annotations:

key3: value3

key4: value4

spec:

containers:

- name: spark-kubernetes-driver

image: docker.io/apache/spark:4.0.4

imagePullPolicy: Always

resources:

requests:

cpu: 1

memory: 1Gi

limits:

cpu: 1

memory: 1Gi

serviceAccount: spark-operator-spark

securityContext:

capabilities:

drop:

- ALL

runAsGroup: 185

runAsUser: 185

runAsNonRoot: true

allowPrivilegeEscalation: false

seccompProfile:

type: RuntimeDefault

executor:

instances: 2

cores: 1

memory: 512m

template:

metadata:

labels:

key1: value1

key2: value2

annotations:

key3: value3

key4: value4

spec:

containers:

- name: spark-kubernetes-executor

image: docker.io/apache/spark:4.0.4

imagePullPolicy: Always

securityContext:

capabilities:

drop:

- ALL

runAsGroup: 185

runAsUser: 185

runAsNonRoot: true

allowPrivilegeEscalation: false

seccompProfile:

type: RuntimeDefault

tariq-hasan · 2026-10-11T15:42:05Z

+/*
+Copyright 2026 The Kubeflow authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    https://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/


Nit comment.

Suggested change

/*

Copyright 2026 The Kubeflow authors.

Licensed under the Apache License, Version 2.0 (the "License");

you may not use this file except in compliance with the License.

You may obtain a copy of the License at

https://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software

distributed under the License is distributed on an "AS IS" BASIS,

WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

See the License for the specific language governing permissions and

limitations under the License.

*/

/*

Copyright The Kubeflow Authors.

Licensed under the Apache License, Version 2.0 (the "License");

you may not use this file except in compliance with the License.

You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software

distributed under the License is distributed on an "AS IS" BASIS,

WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

See the License for the specific language governing permissions and

limitations under the License.

*/

tariq-hasan · 2026-10-11T15:53:34Z

+		It("passes dependency values through unchanged", func() {
+			conn := &v1alpha1.SparkConnect{
+				Spec: v1alpha1.SparkConnectSpec{
+					Deps: v1alpha1.Dependencies{
+						Jars:            []string{"local:///opt/spark/jars/with space.jar", "local:///opt/spark/jars/quote'file.jar"},
+						Packages:        []string{"org.example:package:1.0; printf injected"},
+						ExcludePackages: []string{"org.example:excluded"},
+						Repositories:    []string{"https://repo.example/a path"},
+					},
+				},
+			}
+
+			args, err := dependenciesOption(conn)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(args).To(Equal([]string{
+				"--jars", "local:///opt/spark/jars/with space.jar,local:///opt/spark/jars/quote'file.jar",
+				"--packages", "org.example:package:1.0; printf injected",
+				"--exclude-packages", "org.example:excluded",
+				"--repositories", "https://repo.example/a path",
+			}))
+		})


We should validate group:artifact:version for packages, group:artifact for excludePackages, and no commas anywhere in the admission webhook. That way we would avoid failures in the runtime pod and enable a better user experience.

Please create a follow-up issue for this. We can defer this to a subsequent PR.

tariq-hasan · 2026-10-11T15:56:33Z

For the e2e test please validate server pod readiness. We need to ensure that the user is able to exercise spec.deps within the runtime server pod.

The e2e cluster has Maven egress so this should be feasible. If there are errors you may want to debug on a local kind cluster to determine the root cause of the error.

tariq-hasan · 2026-10-11T16:04:51Z

+			Expect(serverArgs).To(ContainElement("--jars"))
+			Expect(serverArgs).To(ContainElement(ContainSubstring("commons-lang3-3.17.0.jar")))
+			Expect(serverArgs).To(ContainElement("--packages"))
+			Expect(serverArgs).To(ContainElement(ContainSubstring("org.apache.commons:commons-text:1.12.0")))
+			Expect(serverArgs).To(ContainElement("--exclude-packages"))
+			Expect(serverArgs).To(ContainElement(ContainSubstring("org.apache.commons:commons-lang3")))
+			Expect(serverArgs).To(ContainElement("--repositories"))
+			Expect(serverArgs).To(ContainElement(ContainSubstring("https://repo1.maven.org/maven2")))


I think these checks are already covered by the unit tests so these are redundant on the e2e.

We would want to validate server pod readiness and run actual queries on the server pod to ensure that spec.deps can be exercised by the user.

tariq-hasan · 2026-10-11T16:40:00Z

+  deps:
+    jars:
+    - https://repo1.maven.org/maven2/org/apache/commons/commons-lang3/3.17.0/commons-lang3-3.17.0.jar
+    packages:
+    - org.apache.commons:commons-text:1.12.0
+    excludePackages:
+    - org.apache.commons:commons-lang3
+    repositories:
+    - https://repo1.maven.org/maven2


These jars and packages already exist in the base Spark image. Let's use the following as they truly reflect external dependencies. Note here that the excluded packages come as transitive dependencies of spark-avro but already present in the base Spark image so this is a good test for package exclusion.

I've deliberately left out spec.repositories because Maven Central is checked in the Spark runtime and there is little point in exercising third-party archives on the CI.

Suggested change

deps:

jars:

- https://repo1.maven.org/maven2/org/apache/commons/commons-lang3/3.17.0/commons-lang3-3.17.0.jar

packages:

- org.apache.commons:commons-text:1.12.0

excludePackages:

- org.apache.commons:commons-lang3

repositories:

- https://repo1.maven.org/maven2

deps:

jars:

- https://repo1.maven.org/maven2/com/google/re2j/re2j/1.8/re2j-1.8.jar

packages:

# spark-avro must match the image's Spark version

- org.apache.spark:spark-avro_2.13:4.0.4

excludePackages:

- org.tukaani:xz

- org.apache.spark:spark-tags_2.13

# repositories:

# - https://packages.confluent.io/maven/ # only for artifacts not on Maven Central

Please run SELECT hex(to_avro(42)) and SELECT reflect('com.google.re2j.Pattern', 'matches', 'a.c', 'abc') in the e2e and validate ContainSubstring("AVRO=54") and ContainSubstring("RE2J=true") respectively to ensure that the jars and packages have resolved in the Spark runtime.

…add-a-dependencies-fie

Mr-Neutr0n · 2026-10-13T08:58:31Z

All points are in at d95ad3e (branch merged with master first, it had drifted).

Example isolation: examples/sparkconnect/spark-connect-dependencies.yaml now matches spark-connect.yaml apart from spec.deps, including the metadata.labels/annotations blocks, resources and instances: 2.
External dependencies: switched to the artifacts you suggested - re2j-1.8.jar for jars, org.apache.spark:spark-avro_2.13:4.0.4 for packages, and org.tukaani:xz + org.apache.spark:spark-tags_2.13 for excludePackages. spec.repositories is left commented out as you asked.
Copyright: both files now use the Kubeflow Authors boilerplate.
e2e: waits for the server pod to be ready, waits for the executor pods, then runs a real Spark Connect query whose to_avro(...) only resolves if the declared spark-avro package was fetched and placed on the server pod's classpath. The container-args assertions are gone, since the unit tests cover those.
Follow-up issue: opened Validate SparkConnect spec.deps in the admission webhook #3233 for the webhook validation of group:artifact:version / group:artifact shapes and the no-comma rule, as you asked.

I could not run the e2e locally (no kind cluster with Maven egress here), so this needs the CI run. go vet ./... and gofmt are clean on the e2e module.

…ark Connect server subm Signed-off-by: Mr-Neutr0n <64578610+Mr-Neutr0n@users.noreply.github.com>

Mr-Neutr0n · 2026-10-13T13:03:31Z

Pushed a fix for the failing CI. Set spark.jars.ivy=/tmp/.ivy2 in Spark Connect server submit args when Maven dependencies are declared, fixing Ivy cache failures on the non-root Spark image that blocked the dependencies e2e test from reaching a ready server pod.The Spark Connect dependencies e2e failure is addressed: Maven --packages resolution now uses spark.jars.ivy=/tmp/.ivy2 in dependenciesOption, so Ivy can write on the Spark image’s non-root user (Java user.home is /nonexistent). Local tests pass.

- Match examples/sparkconnect/spark-connect.yaml apart from spec.deps so the e2e isolates the dependency behaviour, and use external artifacts that are not already in the base Spark image (re2j jar, spark-avro package, with transitive excludePackages). Drop spec.repositories since Maven Central is already in the runtime and third-party archives add no signal. - Use the Kubeflow Authors boilerplate in both files. - e2e now waits for server and executor pod readiness and runs an actual Spark Connect query that resolves the declared spark-avro package (to_avro), instead of asserting the container args that the unit tests already cover. Signed-off-by: Mr-Neutr0n <64578610+Mr-Neutr0n@users.noreply.github.com>

…ark Connect server subm Signed-off-by: Mr-Neutr0n <64578610+Mr-Neutr0n@users.noreply.github.com>

The dependency e2e drove spark-shell with --master 'local[1]', which selects the Classic session; the .remote() URL is ignored in that mode, so to_avro was evaluated on the client JVM and never reached the server. That produced a failure that looked like a dependency problem but was a client-mode problem, and the same query 'passed' against no server at all. Drive the Connect REPL through a pty instead. That keeps the client a real Spark Connect session, so the query runs against the operator's server and sees the packages declared in spec.deps. The helper also asserts the session type, so a classic fallback fails loudly rather than silently. Signed-off-by: Mr-Neutr0n <64578610+Mr-Neutr0n@users.noreply.github.com>

Mr-Neutr0n · 2026-10-13T18:39:27Z

Fixed the real cause of the e2e failure at 4504dfc2, and rebased the branch so DCO passes (two commits were missing the sign-off; DCO is green again).

The e2e was not actually exercising the server. The client ran spark-shell --master 'local[1]' (as the existing SparkConnect query test does), which selects the Classic session; in that mode .remote() is ignored with only a warning:

WARN SparkSession: spark.connect.remote configuration is not supported in Classic mode.

So the query was evaluated on the client JVM, where the declared spark-avro package is absent, and it failed with AVRO_NOT_LOADED no matter what the operator did. I confirmed this two ways:

With no server listening at all, the previous client still printed a result for a plain query and still failed to_avro identically.
With a genuine remote client (spark-connect-shell --remote, driven through a pty), the same to_avro query returns rows against the operator's server.

So the Ivy fix you pushed is correct; it was masked by the client-mode bug. The new test drives the Connect REPL through a pty to keep a real server session, asserts the session type is org.apache.spark.sql.connect.SparkSession so a Classic fallback fails loudly, and then runs to_avro. Locally against the apache/spark:4.0.4 image with the example's spec.deps, that helper returns CONNECT_ROWS=10; without the spark.jars.ivy conf or without the declared package it fails.

I verified the helper in the image with the exact kubectl exec -i ... python3 - <url> invocation the test uses. CI is running on the new head.

tariq-hasan

Thanks for addressing the comments @Mr-Neutr0n! I have left some final comments on the e2e.

tariq-hasan · 2026-10-13T22:21:31Z

+# Matches examples/sparkconnect/spark-connect.yaml except for spec.deps, so the
+# e2e isolates the dependency behavior from the rest of the SparkConnect spec.


Let's remove this comment.

Suggested change

# Matches examples/sparkconnect/spark-connect.yaml except for spec.deps, so the

# e2e isolates the dependency behavior from the rest of the SparkConnect spec.

Please address this review comment.

tariq-hasan · 2026-10-13T23:08:36Z

Please add the copyright header at the start of this file (above the module docstring).

# Copyright The Kubeflow Authors. # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. # You may obtain a copy of the License at # # http://www.apache.org/licenses/LICENSE-2.0 # # Unless required by applicable law or agreed to in writing, software # distributed under the License is distributed on an "AS IS" BASIS, # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License.

Please address.

tariq-hasan · 2026-10-13T23:26:27Z

+if rows is None:
+    print("CONNECT_ROWS_MISSING")
+    sys.exit(1)
+print("CONNECT_ROWS=" + rows)


The script currently only exercises --packages, and only through a row count. Let's assert on values so that both --packages and --jars are proven to resolve on the server session:

println("AVRO=" + spark.sql("SELECT hex(to_avro(42))").collect()(0).getString(0)) println("RE2J=" + spark.sql("SELECT reflect('com.google.re2j.Pattern', 'matches', 'a.c', 'abc')").collect()(0).getString(0))

Please address.

tariq-hasan · 2026-10-13T23:26:33Z

+			Expect(output).To(ContainSubstring("CONNECT_ROWS=10"), "expected the spark-avro query to return 10 rows, got: %s", output)
+		})
+	})
+})


Please also replace the CONNECT_ROWS=10 assertion with ContainSubstring("AVRO=54") and ContainSubstring("RE2J=true").

Please address.

tariq-hasan · 2026-10-13T23:34:46Z

+#
+# Copyright The Kubeflow Authors.


Suggested change

#

# Copyright The Kubeflow Authors.

# Copyright The Kubeflow Authors.

Please address.

Please remove the single hash in the top line. The copyright needs to match the template in the testing repo.

https://github.com/kubeflow/testing/blob/8483606e0f8a34bf647f3b62a516efa17096c58c/hack/boilerplate/boilerplate.sh.txt

…add-a-dependencies-fie # Conflicts: # internal/controller/sparkconnect/options_test.go

Mr-Neutr0n · 2026-10-20T07:12:04Z

Rebased onto current master (01d91c0), which clears the DIRTY conflict; the PR is MERGEABLE again with CI re-running.

One conflict, in internal/controller/sparkconnect/options_test.go, and it interleaved: upstream had added Context("driverConfOption and executorConfOption with CPU resources") where this branch's Context("dependenciesOption") sits. Resolved by taking upstream's file and re-adding this branch's dependenciesOption context verbatim, so both are preserved — including the --conf spark.jars.ivy=/tmp/.ivy2 expectations this branch added. dependenciesOption itself merged cleanly and still emits the ivy conf only when a Maven coordinate is declared.

Verified on the merged head: go vet ./internal/controller/sparkconnect/ clean, gofmt clean, and the package's tests compile. Note I could not run the envtest suite here — the control plane fails with unable to install CRDs onto control plane: ... context deadline exceeded on this machine, which is the same failure I saw before this branch's changes. CI is the real signal.

tariq-hasan · 2026-10-20T22:34:54Z

Thanks @Mr-Neutr0n! Please address the remaining open review comments in my last round so we can move this PR forward.

…add-a-dependencies-fie

Addresses the remaining review points on kubeflow#3120: - Add the Kubeflow license header above the helper's docstring. - Prove both halves of spec.deps resolve on the server session by asserting on values: hex(to_avro(42)) from the declared spark-avro package, and reflect('com.google.re2j.Pattern', ...) from the declared jar. A row count could pass with only one of them present. - Assert AVRO=54 and RE2J=true in the Go test instead of CONNECT_ROWS=10. - Drop the explanatory comment from the example manifest as requested. Verified against a local Spark Connect server started with this example's spec.deps: AVRO=54, RE2J=true. Signed-off-by: Mr-Neutr0n <64578610+Mr-Neutr0n@users.noreply.github.com>

Mr-Neutr0n · 2026-10-22T03:15:32Z

All five points are in at 41f3470, and I re-merged master while I was there.

License header on test/e2e/scripts/connect_query.py, above the docstring.
Assert on values, not a row count. The helper now runs your two queries:
SELECT hex(to_avro(42)) (the declared spark-avro package) and
SELECT reflect('com.google.re2j.Pattern', 'matches', 'a.c', 'abc') (the
declared jar). Each is wrapped so a resolution failure surfaces the server's
own message instead of a missing marker.
Go assertion is now ContainSubstring("AVRO=54") and
ContainSubstring("RE2J=true").
Example comment removed from spark-connect-dependencies.yaml; the
copyright line already read # Copyright The Kubeflow Authors.

Verified against a local Spark Connect server started with this example's exact spec.deps (--jars re2j, --packages spark-avro, both excludes, the ivy conf): the helper prints AVRO=54 and RE2J=true from a genuine remote session. Both queries fail if their dependency is absent, which is what makes the assertion meaningful. go vet ./... and gofmt clean on the e2e module.

tariq-hasan · 2026-10-22T05:48:22Z

+# AVRO comes from the declared spark-avro package, RE2J from the declared jar.
+# Asserting on the values (not a row count) proves each one resolved on the
+# server session; either being absent makes the SQL itself fail.


Let's remove this comment.

Suggested change

# AVRO comes from the declared spark-avro package, RE2J from the declared jar.

# Asserting on the values (not a row count) proves each one resolved on the

# server session; either being absent makes the SQL itself fail.

tariq-hasan · 2026-10-22T05:48:49Z

+			// AVRO proves the declared spark-avro package resolved on the server session;
+			// RE2J proves the declared jar did. Both are asserted on values, not row counts.


Let's remove this comment.

Suggested change

// AVRO proves the declared spark-avro package resolved on the server session;

// RE2J proves the declared jar did. Both are asserted on values, not row counts.

The example manifest began with a bare '#' line before the copyright, which the testing repo's boilerplate does not have. The e2e helper and its Go assertion also carried explanatory comments the reviewer asked to remove. Signed-off-by: Mr-Neutr0n <64578610+Mr-Neutr0n@users.noreply.github.com>

Mr-Neutr0n · 2026-10-22T06:52:31Z

All three are in at fa21cf2.

YAML boilerplate: the file began with a bare # line before the copyright, which the testing repo's boilerplate.sh.txt does not have. Removed, so the header now matches that template line for line and matches spark-connect-custom-resource.yaml.
test/e2e/scripts/connect_query.py: dropped the explanatory comment above the two probes.
test/e2e/sparkconnect_dependencies_test.go: dropped the comment above the two assertions.

go vet ./... and gofmt clean on the e2e module; the manifest still parses. The previous head (41f3470) was green on all 8 e2e legs, so these are comment-only changes.

tariq-hasan

Thanks!
/lgtm

As a follow-up I think we should refactor the e2e to remove test/e2e/scripts/connect_query.py and replace it with the same approach as sparkconnect_query_test.go. I have raised a separate issue for this: #3260.

nabuskey · 2026-10-27T17:17:19Z

/approve

google-oss-prow · 2026-10-27T17:17:29Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: nabuskey

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:

~~OWNERS~~ [nabuskey]

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

Add a Deps field to SparkConnectSpec (mirroring the pending kubeflow#3120 pass-through) and reject malformed dependency entries in the SparkConnect admission webhook instead of letting them fail inside the runtime pod with a Maven resolution error. Rules enforced: - packages entries must be "groupId:artifactId:version". - excludePackages entries must be "groupId:artifactId" (no version). - jars, packages, excludePackages and repositories entries must not contain a comma, since the values are joined with "," when building the spark-submit flags. Does not wire Deps into the spark-submit argument builder; that is left to kubeflow#3120. Closes kubeflow#3233 Signed-off-by: sunilkumarpradhan <sunilkumarweb47@gmail.com>

google-oss-prow Bot requested review from ImpSy and tariq-hasan August 26, 2026 05:42

google-oss-prow Bot added the size/L label Aug 26, 2026

google-oss-prow Bot added size/XL and removed size/L labels Sep 3, 2026

google-oss-prow Bot changed the title ~~fix: add a Dependencies field to SparkConnectSpec for structu~~ feat(sparkconnect): add a Dependencies field to SparkConnectSpec Sep 3, 2026

google-oss-prow Bot added the ok-to-test label Sep 3, 2026

tariq-hasan reviewed Sep 3, 2026

View reviewed changes

Comment thread api/v1alpha1/common_types.go

Comment thread api/v1alpha1/common_types.go

Mr-Neutr0n force-pushed the agent/issue-2959-add-a-dependencies-fie branch from 244c6dd to c9a8561 Compare September 18, 2026 03:20

Mr-Neutr0n force-pushed the agent/issue-2959-add-a-dependencies-fie branch from c9a8561 to 2e5413c Compare September 19, 2026 02:48

cursor Bot force-pushed the agent/issue-2959-add-a-dependencies-fie branch from 2e5413c to ae912f8 Compare September 30, 2026 14:53

tariq-hasan mentioned this pull request Oct 9, 2026

Add AddConnectJar option for SparkClient instead of hardcoding Spark Connect JARs kubeflow/sdk#491

Open

tariq-hasan suggested changes Oct 11, 2026

View reviewed changes

google-oss-prow Bot assigned tariq-hasan Oct 11, 2026

Merge remote-tracking branch 'upstream/master' into agent/issue-2959-…

f8c7470

…add-a-dependencies-fie

Mr-Neutr0n mentioned this pull request Oct 13, 2026

Validate SparkConnect spec.deps in the admission webhook #3233

Open

Mr-Neutr0n added a commit to Mr-Neutr0n/spark-operator that referenced this pull request Oct 13, 2026

Fix ci failed on kubeflow#3120: Set spark.jars.ivy=/tmp/.ivy2 in Sp…

8141b10

…ark Connect server subm Signed-off-by: Mr-Neutr0n <64578610+Mr-Neutr0n@users.noreply.github.com>

Mr-Neutr0n added 2 commits October 13, 2026 18:37

Fix ci failed on kubeflow#3120: Set spark.jars.ivy=/tmp/.ivy2 in Sp…

55d9b80

…ark Connect server subm Signed-off-by: Mr-Neutr0n <64578610+Mr-Neutr0n@users.noreply.github.com>

Mr-Neutr0n force-pushed the agent/issue-2959-add-a-dependencies-fie branch from 8141b10 to 4504dfc Compare October 13, 2026 18:38

tariq-hasan suggested changes Oct 13, 2026

View reviewed changes

SunilKumarPradhan mentioned this pull request Oct 15, 2026

feat(webhook): validate SparkConnect spec.deps #3242

Open

Merge remote-tracking branch 'upstream/master' into agent/issue-2959-…

01d91c0

…add-a-dependencies-fie # Conflicts: # internal/controller/sparkconnect/options_test.go

Mr-Neutr0n added 2 commits October 22, 2026 03:09

Merge remote-tracking branch 'upstream/master' into agent/issue-2959-…

14be6c3

…add-a-dependencies-fie

tariq-hasan suggested changes Oct 22, 2026

View reviewed changes

tariq-hasan mentioned this pull request Oct 23, 2026

Reuse the spark-shell --remote harness for the SparkConnect dependencies test #3260

Open

tariq-hasan approved these changes Oct 23, 2026

View reviewed changes

google-oss-prow Bot added the lgtm label Oct 23, 2026

google-oss-prow Bot added the approved label Oct 27, 2026

google-oss-prow Bot merged commit c3f94b6 into kubeflow:master Oct 27, 2026
23 checks passed

vikas-saxena02 mentioned this pull request Oct 27, 2026

chore: fix CI failures on master (boilerplate headers, ubuntu-26.04 runner) #3271

Merged

2 tasks done

		# Matches examples/sparkconnect/spark-connect.yaml except for spec.deps, so the
		# e2e isolates the dependency behavior from the rest of the SparkConnect spec.

	#
	# Copyright The Kubeflow Authors.
	# Copyright The Kubeflow Authors.

	# AVRO comes from the declared spark-avro package, RE2J from the declared jar.
	# Asserting on the values (not a row count) proves each one resolved on the
	# server session; either being absent makes the SQL itself fail.

		// AVRO proves the declared spark-avro package resolved on the server session;
		// RE2J proves the declared jar did. Both are asserted on values, not row counts.

Conversation

Mr-Neutr0n commented Aug 26, 2026

Uh oh!

github-actions Bot commented Aug 26, 2026

Uh oh!

nabuskey commented Sep 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Mr-Neutr0n commented Sep 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tariq-hasan commented Sep 3, 2026

Uh oh!

tariq-hasan commented Sep 3, 2026

Uh oh!

tariq-hasan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Mr-Neutr0n commented Sep 17, 2026

Uh oh!

Mr-Neutr0n commented Sep 18, 2026

Uh oh!

Mr-Neutr0n commented Sep 19, 2026

Uh oh!

Mr-Neutr0n commented Sep 30, 2026

Uh oh!

tariq-hasan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mr-Neutr0n commented Oct 13, 2026

Uh oh!

Mr-Neutr0n commented Oct 13, 2026

Uh oh!

Mr-Neutr0n commented Oct 13, 2026

Uh oh!

tariq-hasan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tariq-hasan Oct 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mr-Neutr0n commented Oct 20, 2026

Uh oh!

nabuskey commented Sep 2, 2026 •

edited

Loading

Mr-Neutr0n commented Sep 3, 2026 •

edited

Loading

tariq-hasan Oct 13, 2026 •

edited

Loading