feat: add oauth2-proxy sidecar support #77
Merged
Conversation
This file contains hidden or 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
EnricoMi
reviewed
Oct 21, 2025
|
|
||
| val finalPodSpecWithOAuth = currentPodSpec | ||
| .withRestartPolicy("Never") | ||
| .withTerminationGracePeriodSeconds(0) |
Collaborator
There was a problem hiding this comment.
We might want to use the value of spark.kubernetes.appKillPodDeletionGracePeriod here.
Member
Author
There was a problem hiding this comment.
I'll add it, but in the long run, I don't like the pattern where we reuse kubernetes configs as we do a mix-and-match between spark.armada and spark.kubernetes
src/main/scala/org/apache/spark/deploy/armada/submit/ArmadaClientApplication.scala
Outdated
Show resolved
Hide resolved
src/main/scala/org/apache/spark/deploy/armada/submit/ArmadaClientApplication.scala
Outdated
Show resolved
Hide resolved
src/main/scala/org/apache/spark/deploy/armada/submit/ArmadaClientApplication.scala
Outdated
Show resolved
Hide resolved
src/main/scala/org/apache/spark/deploy/armada/submit/OAuthSidecarBuilder.scala
Outdated
Show resolved
Hide resolved
src/main/scala/org/apache/spark/deploy/armada/submit/OAuthSidecarBuilder.scala
Outdated
Show resolved
Hide resolved
src/main/scala/org/apache/spark/deploy/armada/submit/PodMerger.scala
Outdated
Show resolved
Hide resolved
src/main/scala/org/apache/spark/deploy/armada/submit/PodMerger.scala
Outdated
Show resolved
Hide resolved
Comment on lines
33
to
42
| val resources = new ResourceRequirements() | ||
| val limits = new HashMap[String, Quantity]() | ||
| limits.put("cpu", new Quantity("64m")) | ||
| limits.put("memory", new Quantity("64Mi")) | ||
| resources.setLimits(limits) | ||
|
|
||
| val requests = new HashMap[String, Quantity]() | ||
| requests.put("cpu", new Quantity("64m")) | ||
| requests.put("memory", new Quantity("64Mi")) | ||
| resources.setRequests(requests) |
Collaborator
There was a problem hiding this comment.
The 64m looks unusual for CPU resource:
Suggested change
| val resources = new ResourceRequirements() | |
| val limits = new HashMap[String, Quantity]() | |
| limits.put("cpu", new Quantity("64m")) | |
| limits.put("memory", new Quantity("64Mi")) | |
| resources.setLimits(limits) | |
| val requests = new HashMap[String, Quantity]() | |
| requests.put("cpu", new Quantity("64m")) | |
| requests.put("memory", new Quantity("64Mi")) | |
| resources.setRequests(requests) | |
| val resources = new ResourceRequirements() | |
| val limits = new HashMap[String, Quantity]() | |
| limits.put("cpu", new Quantity("100m")) | |
| limits.put("memory", new Quantity("64Mi")) | |
| resources.setLimits(limits) | |
| val requests = new HashMap[String, Quantity]() | |
| requests.put("cpu", new Quantity("100m")) | |
| requests.put("memory", new Quantity("64Mi")) | |
| resources.setRequests(requests) |
Member
Author
There was a problem hiding this comment.
We don't need a lot of CPU for the oauth proxy, it works just fine with 64m, doesn't get any benefit with the additional 36 millicores, nor it helps accounting in Armada.
Only benefit would be cosmetic due to a round number.
src/test/scala/org/apache/spark/deploy/armada/e2e/featurestep/ExecutorFeatureStep.scala
Outdated
Show resolved
Hide resolved
dejanzele
force-pushed
the
feat/oauth-sidecar-support
branch
from
b2b084f to
7e12f10
Compare
dejanzele
force-pushed
the
feat/oauth-sidecar-support
branch
4 times, most recently
from
60163cb to
87da29f
Compare
dejanzele
force-pushed
the
feat/oauth-sidecar-support
branch
5 times, most recently
from
4552e2d to
cb8faad
Compare
Signed-off-by: Dejan Zele Pejchev <pejcev.dejan@gmail.com>
dejanzele
force-pushed
the
feat/oauth-sidecar-support
branch
2 times, most recently
from
a30b0ff to
cf86a08
Compare
Signed-off-by: Dejan Zele Pejchev <pejcev.dejan@gmail.com>
dejanzele
force-pushed
the
feat/oauth-sidecar-support
branch
from
cf86a08 to
2b7f3b1
Compare
GeorgeJahad
reviewed
Jan 15, 2026
| val allInitContainers = currentPodSpec.initContainers | ||
| val sidecars = extractSidecarContainers(Some(currentPodSpec)) | ||
| val oauthSidecar = OAuthSidecarBuilder.buildOAuthSidecar(conf) | ||
| val allInitContainers = currentPodSpec.initContainers ++ oauthSidecar.toSeq |
Collaborator
There was a problem hiding this comment.
oauth is a sidecar container, but it is being added as an init container? why?
Member
Author
There was a problem hiding this comment.
Native sidecars are configured as init containers with restartPolicy: Always which starts them during init phase but their lifecycle continues along the main container - https://kubernetes.io/blog/2023/08/25/native-sidecar-containers/#what-are-sidecar-containers-in-1-28
GeorgeJahad
reviewed
Jan 15, 2026
Signed-off-by: Dejan Zele Pejchev <pejcev.dejan@gmail.com>
GeorgeJahad
approved these changes
Jan 19, 2026
Collaborator
GeorgeJahad
left a comment
GeorgeJahad
left a comment
There was a problem hiding this comment.
lgtm, thanks @dejanzele
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What type of PR is this?
Enhancement
What this PR does / why we need it:
Adds comprehensive oauth2-proxy sidecar support for Spark Driver UI authentication.
OAuth2-proxy Integration: Implements native OAuth2-proxy sidecar containers with 35+ configuration options, supporting OIDC discovery, custom providers, and secure cookie management. Enables authenticated access to Spark Driver UI through configurable authentication providers.
Ingress Support: Adds TLS-enabled ingress configuration with intelligent port routing - OAuth proxy serves on port 4180, Spark UI on port 4040, with proper precedence resolution (CLI > Template > Default).
Does this PR introduce a user-facing change?
Check the
docs/architecture.mdanddocs/ui.mdfor changes.spark.armada.oauthcontains configuration settings for the oauth proxy.