You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@texera.apache.org by "aicam (via GitHub)" <gi...@apache.org> on 2026/07/24 21:10:45 UTC
[PR] feat(computing-unit): out-of-pod dataset mount infrastructure [texera]
aicam opened a new pull request, #6866:
URL: https://github.com/apache/texera/pull/6866
### What changes were proposed in this PR?
Perform the FUSE mount for dataset repositories **outside** the (unprivileged) computing-unit pod — the infrastructure foundation of the dataset-mounting feature (#6606).
- **`texera-mounter` DaemonSet** — a per-node privileged agent (`bin/mounter/mounter.py` + tests, dockerfile, helm daemonset/rbac/values) that runs GeeseFS on a pod's behalf. The read-only mount is exposed back into the CU pod via Kubernetes **mount propagation**, scoped to that computing unit, so the pod that runs untrusted user code stays **unprivileged**.
- **Unprivileged CU pod wiring** (`KubernetesClient`) — the propagation volume + node-IP / mounter env.
- **File-service JWT S3 proxy** (`S3ProxyServlet`) — fronts the LakeFS S3 gateway: verifies the pod's JWT, checks the user's read access, and re-signs to LakeFS with credentials held only server-side. No global credential ever enters the pod.
Foundation only — nothing triggers a mount yet; the platform integration (engine client + per-CU mount API + UI + UDF bindings) comes in the follow-up PR.
### Any related issues, documentation, discussions?
Closes #6862 · part of #6606.
### How was this PR tested?
`sbt FileService/compile ComputingUnitManagingService/compile` green. The mounter has its own pytest suite (`bin/mounter/tests`). The whole feature (this PR + the integration follow-up) was validated end-to-end on minikube: a Python UDF read a ~2 GB sharded PyTorch model from a propagated mount via `torch.load`, bit-exact; the mounter's reaper/unmount and the proxy's JWT authorization (positive + negative) were exercised.
### Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Opus 4.8
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(computing-unit): out-of-pod dataset mount infrastructure [texera]
Posted by "kunwp1 (via GitHub)" <gi...@apache.org>.
kunwp1 commented on code in PR #6866:
URL: https://github.com/apache/texera/pull/6866#discussion_r3780198054
##########
bin/mounter/mounter.py:
##########
Review Comment:
I still think we need authentication on `/mount` and `/mounts`. Attacker can use CU A to mount his dataset into CU B's directory and also use GET /mounts to enumerate all the repos/commits/paths. I also don't think it's a good idea to solve security issues in later PRs. Better to just include authentication here to be safe.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(computing-unit): out-of-pod dataset mount infrastructure [texera]
Posted by "aicam (via GitHub)" <gi...@apache.org>.
aicam commented on code in PR #6866:
URL: https://github.com/apache/texera/pull/6866#discussion_r3779409150
##########
bin/mounter/mounter.py:
##########
Review Comment:
I agree, the problem is that mounter is a light weight microservice which only perform mounting. It is notable that the security leak is only in `unmount` endpoint, although mounting endpoint does not check token and CU access but it just mount it and user can not use it because S3 Servlet proxy in `file-service` check user access in every request, so we have two issues:
1- cuid=B can unmount a LakeFS repo from user A
2- cuid=B can mount a LakeFS repo from user A but can not access it
To address them, I changed the design slightly:
1- Removed unmount endpoint. It is not needed because mounter service receives termination signal from K8s so no need to have an endpoint. It is now removed in this PR.
2- We put `access-control-service` in between CU and mounter. The diagram is updated but this PR does not include that. Later in #6896 we will add new endpoint to access control service and also add network policy to Envoy Gateway to make sure only access control service can talk to mounter and we do access control in the service.
This way we keep mounter a simple microservice that only performs mounting.
##########
bin/mounter/mounter.py:
##########
Review Comment:
I agree, the problem is that mounter is a light weight microservice which only perform mounting. It is notable that the security leak is only in `unmount` endpoint, although mounting endpoint does not check token and CU access but it just mount it and user can not use it because S3 Servlet proxy in `file-service` check user access in every request, so we have two issues:
1- cuid=B can unmount a LakeFS repo from user A
2- cuid=B can mount a LakeFS repo from user A but can not access it
To address them, I changed the design slightly:
1- Removed unmount endpoint. It is not needed because mounter service receives termination signal from K8s so no need to have an endpoint. It is now removed in this PR.
2- We put `access-control-service` in between CU and mounter. The diagram is updated but this PR does not include that. Later in #6896 we will add new endpoint to access control service and also add network policy to Envoy Gateway to make sure only access control service can talk to mounter and we do access control in the service.
This way we keep mounter a simple microservice that only performs mounting.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(computing-unit): out-of-pod dataset mount infrastructure [texera]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #6866:
URL: https://github.com/apache/texera/pull/6866#issuecomment-5074547800
## [Codecov](https://app.codecov.io/gh/apache/texera/pull/6866?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `11.76471%` with `90 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 82.77%. Comparing base ([`f02dd2f`](https://app.codecov.io/gh/apache/texera/commit/f02dd2f1cd6fa2a7372971dba3f0b03fdd87a310?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`ddf160e`](https://app.codecov.io/gh/apache/texera/commit/ddf160ec29c98181b75e994ea2c0e0648693284a?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 1 commits behind head on main.
| [Files with missing lines](https://app.codecov.io/gh/apache/texera/pull/6866?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...rg/apache/texera/service/util/S3ProxyServlet.scala](https://app.codecov.io/gh/apache/texera/pull/6866?src=pr&el=tree&filepath=file-service%2Fsrc%2Fmain%2Fscala%2Forg%2Fapache%2Ftexera%2Fservice%2Futil%2FS3ProxyServlet.scala&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZmlsZS1zZXJ2aWNlL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvdGV4ZXJhL3NlcnZpY2UvdXRpbC9TM1Byb3h5U2VydmxldC5zY2FsYQ==) | 5.79% | [65 Missing :warning: ](https://app.codecov.io/gh/apache/texera/pull/6866?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [.../apache/texera/service/util/KubernetesClient.scala](https://app.codecov.io/gh/apache/texera/pull/6866?src=pr&el=tree&filepath=computing-unit-managing-service%2Fsrc%2Fmain%2Fscala%2Forg%2Fapache%2Ftexera%2Fservice%2Futil%2FKubernetesClient.scala&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29tcHV0aW5nLXVuaXQtbWFuYWdpbmctc2VydmljZS9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL3RleGVyYS9zZXJ2aWNlL3V0aWwvS3ViZXJuZXRlc0NsaWVudC5zY2FsYQ==) | 4.00% | [24 Missing :warning: ](https://app.codecov.io/gh/apache/texera/pull/6866?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [.../scala/org/apache/texera/service/FileService.scala](https://app.codecov.io/gh/apache/texera/pull/6866?src=pr&el=tree&filepath=file-service%2Fsrc%2Fmain%2Fscala%2Forg%2Fapache%2Ftexera%2Fservice%2FFileService.scala&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZmlsZS1zZXJ2aWNlL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvdGV4ZXJhL3NlcnZpY2UvRmlsZVNlcnZpY2Uuc2NhbGE=) | 0.00% | [1 Missing :warning: ](https://app.codecov.io/gh/apache/texera/pull/6866?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
:x: Your patch status has failed because the patch coverage (11.76%) is below the target coverage (60.00%). You can increase the patch coverage or adjust the [target](https://docs.codecov.com/docs/commit-status#target) coverage.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## main #6866 +/- ##
============================================
+ Coverage 77.30% 82.77% +5.46%
+ Complexity 3524 2354 -1170
============================================
Files 1161 893 -268
Lines 45922 37691 -8231
Branches 5101 3968 -1133
============================================
- Hits 35501 31199 -4302
+ Misses 8840 5357 -3483
+ Partials 1581 1135 -446
```
| [Flag](https://app.codecov.io/gh/apache/texera/pull/6866/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | | *Carryforward flag |
|---|---|---|---|
| [access-control-service](https://app.codecov.io/gh/apache/texera/pull/6866/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `70.00% <ø> (ø)` | | |
| [agent-service](https://app.codecov.io/gh/apache/texera/pull/6866/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `76.76% <ø> (ø)` | | Carriedforward from [f02dd2f](https://app.codecov.io/gh/apache/texera/commit/f02dd2f1cd6fa2a7372971dba3f0b03fdd87a310?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [amber](https://app.codecov.io/gh/apache/texera/pull/6866/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `85.90% <100.00%> (+16.80%)` | :arrow_up: | |
| [computing-unit-managing-service](https://app.codecov.io/gh/apache/texera/pull/6866/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `19.37% <4.00%> (-1.13%)` | :arrow_down: | |
| [config-service](https://app.codecov.io/gh/apache/texera/pull/6866/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `66.66% <ø> (ø)` | | |
| [file-service](https://app.codecov.io/gh/apache/texera/pull/6866/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `59.49% <5.71%> (-7.72%)` | :arrow_down: | |
| [frontend](https://app.codecov.io/gh/apache/texera/pull/6866/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `82.58% <ø> (ø)` | | Carriedforward from [f02dd2f](https://app.codecov.io/gh/apache/texera/commit/f02dd2f1cd6fa2a7372971dba3f0b03fdd87a310?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [notebook-migration-service](https://app.codecov.io/gh/apache/texera/pull/6866/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `78.94% <ø> (ø)` | | |
| [pyamber](https://app.codecov.io/gh/apache/texera/pull/6866/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `92.15% <ø> (ø)` | | Carriedforward from [f02dd2f](https://app.codecov.io/gh/apache/texera/commit/f02dd2f1cd6fa2a7372971dba3f0b03fdd87a310?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [workflow-compiling-service](https://app.codecov.io/gh/apache/texera/pull/6866/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `55.14% <ø> (ø)` | | |
*This pull request uses carry forward flags. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/texera/pull/6866?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(computing-unit): out-of-pod dataset mount infrastructure [texera]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #6866:
URL: https://github.com/apache/texera/pull/6866#issuecomment-5074441842
<!-- texera-reviewer-suggestion -->
### Automated Reviewer Suggestions
Based on the `git blame` history of the changed files, we recommend the following reviewers:
- **Contributors with relevant context:** `@Ma77Ball`, `@eugenegujing`
You can notify them by mentioning `@Ma77Ball`, `@eugenegujing` in a comment.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(computing-unit): out-of-pod dataset mount infrastructure [texera]
Posted by "kunwp1 (via GitHub)" <gi...@apache.org>.
kunwp1 commented on code in PR #6866:
URL: https://github.com/apache/texera/pull/6866#discussion_r3780198054
##########
bin/mounter/mounter.py:
##########
Review Comment:
I still think we need authentication on `/mount` and `/mounts`. Attacker can use CU A to mount his dataset into CU B's directory and also use GET /mounts to enumerate all the repos/commits/paths. I also don't think it's a good idea to solve security issues in later PRs. Better to just include authentication here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(computing-unit): out-of-pod dataset mount infrastructure [texera]
Posted by "kunwp1 (via GitHub)" <gi...@apache.org>.
kunwp1 commented on code in PR #6866:
URL: https://github.com/apache/texera/pull/6866#discussion_r3736549551
##########
bin/mounter/mounter.py:
##########
Review Comment:
The mounter seems to not have an authentication (no auth on mount/unmount). For example, CU pod A can send cuid=B and unmount CU pod B's dataset.
Also, I do see that the cuid, repo, and commit go straight into os.path.join(MOUNT_ROOT, cuid, repo, commit). Can you confirm and reject any cuid, repo, commit containing path separators or `..`?
##########
bin/k8s/templates/base/workflow-computing-unit-manager/workflow-computing-unit-manager-deployment.yaml:
##########
@@ -64,6 +64,8 @@ spec:
value: {{ .Values.workflowComputingUnitPool.namespace }}
- name: KUBERNETES_COMPUTE_UNIT_SERVICE_NAME
value: {{ .Values.workflowComputingUnitPool.name }}-svc
+ - name: KUBERNETES_COMPUTE_UNIT_POD_NAME_PREFIX
Review Comment:
Shouldn't you also inject the KUBERNETES_MOUNTER_PORT and KUBERNETES_MOUNTER_HOST_ROOT from the helm values?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(computing-unit): out-of-pod LakeFS repository mount infrastructure [texera]
Posted by "aicam (via GitHub)" <gi...@apache.org>.
aicam commented on code in PR #6866:
URL: https://github.com/apache/texera/pull/6866#discussion_r3944836317
##########
file-service/src/main/scala/org/apache/texera/service/util/S3ProxyServlet.scala:
##########
@@ -0,0 +1,273 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.
+ */
+
+package org.apache.texera.service.util
+
+import com.typesafe.scalalogging.LazyLogging
+import jakarta.servlet.http.{HttpServlet, HttpServletRequest, HttpServletResponse}
+import org.apache.texera.auth.JwtParser
+import org.apache.texera.common.config.StorageConfig
+import org.apache.texera.dao.SqlServer
+import org.apache.texera.dao.SqlServer.withTransaction
+import org.apache.texera.dao.jooq.generated.tables.daos.{DatasetDao, ModelDao}
+import org.apache.texera.service.resource.{DatasetAccessResource, ModelAccessResource}
+import software.amazon.awssdk.auth.credentials.AwsBasicCredentials
+import software.amazon.awssdk.auth.signer.AwsS3V4Signer
+import software.amazon.awssdk.auth.signer.params.AwsS3V4SignerParams
+import software.amazon.awssdk.http.{SdkHttpFullRequest, SdkHttpMethod}
+import software.amazon.awssdk.regions.Region
+
+import java.net.{URI, URLDecoder}
+import java.net.http.{HttpClient, HttpRequest, HttpResponse}
+import java.time.Duration
+import scala.jdk.CollectionConverters._
+import scala.jdk.OptionConverters._
+
+/**
+ * Read-only, JWT-authenticated, re-signing reverse proxy in front of the LakeFS S3
+ * gateway. A computing-unit pod's GeeseFS mount talks to this servlet using the pod's
+ * own per-user JWT as the S3 credential: the JWT is passed to GeeseFS as
+ * `AWS_ACCESS_KEY_ID`, so it rides in the request's SigV4/SigV2 `Authorization` header.
+ * Reusing the JWT that is already present in the pod means no separate mount credential
+ * is ever issued, stored, or made multi-replica-consistent. The servlet:
+ *
+ * 1. reads the JWT back out of the incoming `Authorization` header (the JWT is the
+ * bearer capability; the pod-side S3 signature is not re-validated, and no LakeFS
+ * credentials ever leave this service),
+ * 2. verifies the JWT and checks that its user has read access to the requested
+ * repository (the S3 bucket), using the same `userHasReadAccess` gate as the
+ * dataset REST endpoints, and
+ * 3. re-signs the request with the global LakeFS credentials and forwards it to the
+ * LakeFS S3 gateway, streaming the response back.
+ *
+ * Because requests are forwarded verbatim, the proxy behaves identically to a direct
+ * GeeseFS -> LakeFS-gateway mount, just re-authenticated. GeeseFS mounts read-only, so
+ * only GET and HEAD are handled.
+ */
+class S3ProxyServlet extends HttpServlet with LazyLogging {
+
+ // The LakeFS S3 gateway shares the LakeFS server address: the configured API endpoint
+ // with the trailing /api/v1 suffix removed.
+ private val gatewayEndpoint: URI =
+ URI.create(StorageConfig.lakefsEndpoint.stripSuffix("/").stripSuffix("/api/v1"))
+
+ private val lakefsCredentials =
+ AwsBasicCredentials.create(StorageConfig.lakefsUsername, StorageConfig.lakefsPassword)
+
+ // The S3-specific SigV4 signer adds and signs the x-amz-content-sha256 header (which
+ // S3 / the LakeFS gateway require) and disables path double-encoding — both essential
+ // for the signature to validate. The generic Aws4Signer omits x-amz-content-sha256.
+ private val signer = AwsS3V4Signer.create()
+
+ private val httpClient: HttpClient = HttpClient
+ .newBuilder()
+ .followRedirects(HttpClient.Redirect.NEVER)
+ .connectTimeout(Duration.ofSeconds(10))
+ .build()
+
+ private val forwardedResponseHeaderPrefixes =
+ Seq("content-", "etag", "last-modified", "accept-ranges", "x-amz-")
+
+ override def doGet(req: HttpServletRequest, resp: HttpServletResponse): Unit =
+ proxy(req, resp, SdkHttpMethod.GET, streamBody = true)
+
+ override def doHead(req: HttpServletRequest, resp: HttpServletResponse): Unit =
+ proxy(req, resp, SdkHttpMethod.HEAD, streamBody = false)
+
+ private def proxy(
+ req: HttpServletRequest,
+ resp: HttpServletResponse,
+ method: SdkHttpMethod,
+ streamBody: Boolean
+ ): Unit = {
+ val user = S3ProxyServlet
+ .extractCredentialToken(req.getHeader("Authorization"))
+ .flatMap(token => JwtParser.parseToken(token).toScala)
+ if (user.isEmpty) {
+ // GeeseFS probes the bucket unauthenticated on mount, so this is expected noise.
+ resp.sendError(HttpServletResponse.SC_FORBIDDEN, "missing or invalid user token")
+ return
+ }
+
+ val uid = user.get.getUid
+ val repositoryName = S3ProxyServlet.bucketFromUri(req.getRequestURI)
+ if (repositoryName.isEmpty || !authorizedToRead(uid, repositoryName)) {
+ logger.warn(
+ s"user $uid denied mount access to repository '$repositoryName' for ${req.getRequestURI}"
+ )
+ resp.sendError(HttpServletResponse.SC_FORBIDDEN, "no read access to the requested repository")
+ return
+ }
+
+ try {
+ writeResponse(forward(req, method), resp, streamBody)
+ } catch {
+ case e: Exception =>
+ logger.error(s"error proxying ${req.getRequestURI} to LakeFS gateway", e)
+ resp.sendError(HttpServletResponse.SC_BAD_GATEWAY, "upstream error")
+ }
+ }
+
+ /**
+ * True iff `uid` has read access to the versioned resource backing `repositoryName`.
+ *
+ * Read access to a repository grants read to all of its commits, so no per-commit check
+ * is needed: a session addresses a single repository's data and any version the user may
+ * already read.
+ *
+ * Both resource types are searched, because a repository backs either a dataset or a
+ * model, and the name is matched rather than parsed: rows created today are named
+ * `<type>-<id>`, but `sql/updates/15.sql` backfilled the column from the dataset's plain
+ * `name`, so an upgraded deployment still has repositories called e.g. `my-data`.
+ *
+ * Exactly one row must match, or the request is denied. A repository name is unique in
+ * practice -- LakeFS will not create two repositories with one name, and dataset creation
+ * checked the name globally besides -- but `repository_name` carries no unique constraint
+ * (the only UNIQUE on either table is (owner_uid, name)), so nothing in the schema says
+ * so. Rather than let `.head` pick a row, and decide access arbitrarily in front of a
+ * proxy that re-signs with the global LakeFS credentials, an ambiguous name fails closed.
+ */
+ private def authorizedToRead(uid: Integer, repositoryName: String): Boolean =
+ withTransaction(SqlServer.getInstance().createDSLContext()) { ctx =>
+ val datasets = new DatasetDao(ctx.configuration())
+ .fetchByRepositoryName(repositoryName)
+ .asScala
+ .toList
+ val models = new ModelDao(ctx.configuration())
Review Comment:
Since we read whole file, the authentication is done once, means we query both tables once and no need to query them every time for every part.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(computing-unit): out-of-pod dataset mount infrastructure [texera]
Posted by "chenlica (via GitHub)" <gi...@apache.org>.
chenlica commented on PR #6866:
URL: https://github.com/apache/texera/pull/6866#issuecomment-5085592818
@aicam Please revise the diagram to use a layout consistent with other diagrams.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(computing-unit): out-of-pod dataset mount infrastructure [texera]
Posted by "aicam (via GitHub)" <gi...@apache.org>.
aicam commented on code in PR #6866:
URL: https://github.com/apache/texera/pull/6866#discussion_r3897449409
##########
bin/mounter/mounter.py:
##########
@@ -0,0 +1,402 @@
+#!/usr/bin/env python3
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you 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.
+
+"""
+texera-mounter: a per-node privileged service that performs GeeseFS FUSE mounts on behalf
+of (unprivileged) computing-unit pods.
+
+A CU pod POSTs /mount with {cuid, repositoryName, commitHash, jwt, fileServiceBase}. The
+mounter runs GeeseFS against file-service's JWT-authenticated S3 proxy (passing the pod's
+JWT as the S3 access key) and mounts read-only under MOUNT_ROOT/<cuid>/<repo>/<commit>.
+That host directory is bind-mounted (mountPropagation: Bidirectional) into the mounter and
+propagates back into the CU pod (mountPropagation: HostToContainer), so the CU pod sees
+the mount without any privilege of its own.
+
+The mounter holds no LakeFS credentials; authorization stays entirely in file-service.
+A background watcher unmounts a CU's directories as soon as its pod is deleted.
+"""
+
+import json
+import os
+import shutil
+import ssl
+import subprocess
+import threading
+import time
+import urllib.request
+from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer
+from urllib.parse import parse_qs, urlparse
+
+MOUNT_ROOT = os.environ.get("MOUNT_ROOT", "/var/lib/texera-mounts")
+MOUNTER_PORT = int(os.environ.get("MOUNTER_PORT", "8100"))
+POOL_NAMESPACE = os.environ.get("POOL_NAMESPACE", "texera-workflow-computing-unit-pool")
+MOUNT_SECRET_PLACEHOLDER = "texera-jwt-mount"
+MOUNT_TIMEOUT_S = 30
+# How long the API server keeps a watch open. Each expiry re-runs the reconcile, which is
+# the safety net for events missed while disconnected and for orphans still busy earlier.
+WATCH_TIMEOUT_S = 300
+WATCH_RETRY_S = 10
+
+SA_DIR = "/var/run/secrets/kubernetes.io/serviceaccount"
+# Computing-unit pods are named "<prefix>-<cuid>"; the helm chart gives this and the
+# computing-unit manager's kubernetes.compute-unit-pod-name-prefix the same value.
+CU_POD_NAME_PREFIX = os.environ.get("CU_POD_NAME_PREFIX", "computing-unit")
+PROC_MOUNTS = "/proc/mounts"
+
+
+def log(msg):
+ print(f"[mounter] {msg}", flush=True)
+
+
+def _unescape(field):
+ """Decode the four characters /proc/mounts escapes octally."""
+ for code, char in (("\\040", " "), ("\\011", "\t"), ("\\012", "\n"), ("\\134", "\\")):
+ field = field.replace(code, char)
+ return field
+
+
+def mount_targets_under(path):
+ """Mount targets at or under `path`, deepest first.
+
+ Read from /proc/mounts rather than using os.path.ismount(): when a FUSE server dies
+ (the mounter being restarted kills every GeeseFS it started) its mount entry survives
+ here, but stat()ing the mount point fails with ENOTCONN, which os.path.ismount()
+ reports as "not a mount point". Such a dead mount would then never be unmounted, and
+ its directory could never be removed.
+ """
+ path = os.path.normpath(path)
+ prefix = path + "/"
+ targets = []
+ try:
+ with open(PROC_MOUNTS) as mounts:
+ for line in mounts:
+ fields = line.split()
+ if len(fields) < 2:
+ continue
+ target = _unescape(fields[1])
+ if target == path or target.startswith(prefix):
+ targets.append(target)
+ except OSError as e:
+ log(f"reading {PROC_MOUNTS} failed: {e}")
+ # Deepest first, so nested mounts are detached before their parents.
+ return sorted(targets, key=len, reverse=True)
+
+
+def is_mounted(path):
+ """True if `path` itself is a mount point, alive or dead."""
+ return os.path.normpath(path) in mount_targets_under(path)
+
+
+def _responds(path):
+ """True if `path` can be stat()ed — false for a mount whose FUSE server is gone."""
+ try:
+ os.stat(path)
+ return True
+ except OSError:
+ return False
+
+
+def _remove_empty_dirs(path, cuid):
+ """Remove `path` and any parents left empty, stopping at the CU's own directory."""
+ stop_at = os.path.normpath(os.path.join(MOUNT_ROOT, cuid))
+ path = os.path.normpath(path)
+ while path.startswith(stop_at):
Review Comment:
Thats correct, and removing the directory happen when a pod is deleted. In mounter, when a K8s action happen, it receives an event, if that event says a CU is now deleted, mounter will remove corresponding mounted paths.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(computing-unit): out-of-pod dataset mount infrastructure [texera]
Posted by "kunwp1 (via GitHub)" <gi...@apache.org>.
kunwp1 commented on code in PR #6866:
URL: https://github.com/apache/texera/pull/6866#discussion_r3810195859
##########
bin/mounter/mounter.py:
##########
@@ -0,0 +1,402 @@
+#!/usr/bin/env python3
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you 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.
+
+"""
+texera-mounter: a per-node privileged service that performs GeeseFS FUSE mounts on behalf
+of (unprivileged) computing-unit pods.
+
+A CU pod POSTs /mount with {cuid, repositoryName, commitHash, jwt, fileServiceBase}. The
+mounter runs GeeseFS against file-service's JWT-authenticated S3 proxy (passing the pod's
+JWT as the S3 access key) and mounts read-only under MOUNT_ROOT/<cuid>/<repo>/<commit>.
+That host directory is bind-mounted (mountPropagation: Bidirectional) into the mounter and
+propagates back into the CU pod (mountPropagation: HostToContainer), so the CU pod sees
+the mount without any privilege of its own.
+
+The mounter holds no LakeFS credentials; authorization stays entirely in file-service.
+A background watcher unmounts a CU's directories as soon as its pod is deleted.
+"""
+
+import json
+import os
+import shutil
+import ssl
+import subprocess
+import threading
+import time
+import urllib.request
+from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer
+from urllib.parse import parse_qs, urlparse
+
+MOUNT_ROOT = os.environ.get("MOUNT_ROOT", "/var/lib/texera-mounts")
+MOUNTER_PORT = int(os.environ.get("MOUNTER_PORT", "8100"))
+POOL_NAMESPACE = os.environ.get("POOL_NAMESPACE", "texera-workflow-computing-unit-pool")
+MOUNT_SECRET_PLACEHOLDER = "texera-jwt-mount"
+MOUNT_TIMEOUT_S = 30
+# How long the API server keeps a watch open. Each expiry re-runs the reconcile, which is
+# the safety net for events missed while disconnected and for orphans still busy earlier.
+WATCH_TIMEOUT_S = 300
+WATCH_RETRY_S = 10
+
+SA_DIR = "/var/run/secrets/kubernetes.io/serviceaccount"
+# Computing-unit pods are named "<prefix>-<cuid>"; the helm chart gives this and the
+# computing-unit manager's kubernetes.compute-unit-pod-name-prefix the same value.
+CU_POD_NAME_PREFIX = os.environ.get("CU_POD_NAME_PREFIX", "computing-unit")
+PROC_MOUNTS = "/proc/mounts"
+
+
+def log(msg):
+ print(f"[mounter] {msg}", flush=True)
+
+
+def _unescape(field):
+ """Decode the four characters /proc/mounts escapes octally."""
+ for code, char in (("\\040", " "), ("\\011", "\t"), ("\\012", "\n"), ("\\134", "\\")):
+ field = field.replace(code, char)
+ return field
+
+
+def mount_targets_under(path):
+ """Mount targets at or under `path`, deepest first.
+
+ Read from /proc/mounts rather than using os.path.ismount(): when a FUSE server dies
+ (the mounter being restarted kills every GeeseFS it started) its mount entry survives
+ here, but stat()ing the mount point fails with ENOTCONN, which os.path.ismount()
+ reports as "not a mount point". Such a dead mount would then never be unmounted, and
+ its directory could never be removed.
+ """
+ path = os.path.normpath(path)
+ prefix = path + "/"
+ targets = []
+ try:
+ with open(PROC_MOUNTS) as mounts:
+ for line in mounts:
+ fields = line.split()
+ if len(fields) < 2:
+ continue
+ target = _unescape(fields[1])
+ if target == path or target.startswith(prefix):
+ targets.append(target)
+ except OSError as e:
+ log(f"reading {PROC_MOUNTS} failed: {e}")
+ # Deepest first, so nested mounts are detached before their parents.
+ return sorted(targets, key=len, reverse=True)
+
+
+def is_mounted(path):
+ """True if `path` itself is a mount point, alive or dead."""
+ return os.path.normpath(path) in mount_targets_under(path)
+
+
+def _responds(path):
+ """True if `path` can be stat()ed — false for a mount whose FUSE server is gone."""
+ try:
+ os.stat(path)
+ return True
+ except OSError:
+ return False
+
+
+def _remove_empty_dirs(path, cuid):
+ """Remove `path` and any parents left empty, stopping at the CU's own directory."""
+ stop_at = os.path.normpath(os.path.join(MOUNT_ROOT, cuid))
+ path = os.path.normpath(path)
+ while path.startswith(stop_at):
+ try:
+ os.rmdir(path)
+ except OSError:
+ return # not empty (another commit is mounted here) or already gone
+ path = os.path.dirname(path)
+
+
+def ensure_shared_root():
+ """Make MOUNT_ROOT a shared mount so mounts created under it propagate to peers."""
+ os.makedirs(MOUNT_ROOT, exist_ok=True)
+ if not os.path.ismount(MOUNT_ROOT):
+ subprocess.run(["mount", "--bind", MOUNT_ROOT, MOUNT_ROOT], check=False)
+ subprocess.run(["mount", "--make-rshared", MOUNT_ROOT], check=False)
+
+
+def do_mount(cuid, repo, commit, jwt, file_service_base):
+ """Idempotently mount repo:commit for cuid. Returns the mount target path."""
+ if not cuid or not repo or not commit or not jwt or not file_service_base:
+ raise ValueError("cuid, repositoryName, commitHash, jwt and fileServiceBase are required")
+
+ target = os.path.join(MOUNT_ROOT, cuid, repo, commit)
+ if is_mounted(target):
Review Comment:
We had a discussion that `repo` and `commit` are validated by LakeFS. But I see nobody validates `cuid`. So a request can still name someone else's `cuid`.
For example, a legitimate user running CU `5` can `POST /mount` with `cuid=8` and a repo their own JWT can read -> LakeFS authorizes it -> the mount lands in `MOUNT_ROOT/8/...`, and mount propagation pushes it into CU `8`'s pod -> `GET
/mounts?cuid=8` similarly enumerates another CU's mounts.
Also, a `cuid` like `../..` can flow straight into `os.path.join`/`os.makedirs`.
I suggest deriving `cuid` from the authenticated caller rather than the request body and also reject `cuid` values that aren't a single path segment.
##########
bin/mounter/mounter.py:
##########
@@ -0,0 +1,402 @@
+#!/usr/bin/env python3
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you 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.
+
+"""
+texera-mounter: a per-node privileged service that performs GeeseFS FUSE mounts on behalf
+of (unprivileged) computing-unit pods.
+
+A CU pod POSTs /mount with {cuid, repositoryName, commitHash, jwt, fileServiceBase}. The
+mounter runs GeeseFS against file-service's JWT-authenticated S3 proxy (passing the pod's
+JWT as the S3 access key) and mounts read-only under MOUNT_ROOT/<cuid>/<repo>/<commit>.
+That host directory is bind-mounted (mountPropagation: Bidirectional) into the mounter and
+propagates back into the CU pod (mountPropagation: HostToContainer), so the CU pod sees
+the mount without any privilege of its own.
+
+The mounter holds no LakeFS credentials; authorization stays entirely in file-service.
+A background watcher unmounts a CU's directories as soon as its pod is deleted.
+"""
+
+import json
+import os
+import shutil
+import ssl
+import subprocess
+import threading
+import time
+import urllib.request
+from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer
+from urllib.parse import parse_qs, urlparse
+
+MOUNT_ROOT = os.environ.get("MOUNT_ROOT", "/var/lib/texera-mounts")
+MOUNTER_PORT = int(os.environ.get("MOUNTER_PORT", "8100"))
+POOL_NAMESPACE = os.environ.get("POOL_NAMESPACE", "texera-workflow-computing-unit-pool")
+MOUNT_SECRET_PLACEHOLDER = "texera-jwt-mount"
+MOUNT_TIMEOUT_S = 30
+# How long the API server keeps a watch open. Each expiry re-runs the reconcile, which is
+# the safety net for events missed while disconnected and for orphans still busy earlier.
+WATCH_TIMEOUT_S = 300
+WATCH_RETRY_S = 10
+
+SA_DIR = "/var/run/secrets/kubernetes.io/serviceaccount"
+# Computing-unit pods are named "<prefix>-<cuid>"; the helm chart gives this and the
+# computing-unit manager's kubernetes.compute-unit-pod-name-prefix the same value.
+CU_POD_NAME_PREFIX = os.environ.get("CU_POD_NAME_PREFIX", "computing-unit")
+PROC_MOUNTS = "/proc/mounts"
+
+
+def log(msg):
+ print(f"[mounter] {msg}", flush=True)
+
+
+def _unescape(field):
+ """Decode the four characters /proc/mounts escapes octally."""
+ for code, char in (("\\040", " "), ("\\011", "\t"), ("\\012", "\n"), ("\\134", "\\")):
+ field = field.replace(code, char)
+ return field
+
+
+def mount_targets_under(path):
+ """Mount targets at or under `path`, deepest first.
+
+ Read from /proc/mounts rather than using os.path.ismount(): when a FUSE server dies
+ (the mounter being restarted kills every GeeseFS it started) its mount entry survives
+ here, but stat()ing the mount point fails with ENOTCONN, which os.path.ismount()
+ reports as "not a mount point". Such a dead mount would then never be unmounted, and
+ its directory could never be removed.
+ """
+ path = os.path.normpath(path)
+ prefix = path + "/"
+ targets = []
+ try:
+ with open(PROC_MOUNTS) as mounts:
+ for line in mounts:
+ fields = line.split()
+ if len(fields) < 2:
+ continue
+ target = _unescape(fields[1])
+ if target == path or target.startswith(prefix):
+ targets.append(target)
+ except OSError as e:
+ log(f"reading {PROC_MOUNTS} failed: {e}")
+ # Deepest first, so nested mounts are detached before their parents.
+ return sorted(targets, key=len, reverse=True)
+
+
+def is_mounted(path):
+ """True if `path` itself is a mount point, alive or dead."""
+ return os.path.normpath(path) in mount_targets_under(path)
+
+
+def _responds(path):
+ """True if `path` can be stat()ed — false for a mount whose FUSE server is gone."""
+ try:
+ os.stat(path)
+ return True
+ except OSError:
+ return False
+
+
+def _remove_empty_dirs(path, cuid):
+ """Remove `path` and any parents left empty, stopping at the CU's own directory."""
+ stop_at = os.path.normpath(os.path.join(MOUNT_ROOT, cuid))
+ path = os.path.normpath(path)
+ while path.startswith(stop_at):
Review Comment:
I am confused with the word `stop_at`. The logic seems like it's removing `MOUNT_ROOT/<cuid>` as well. Does deleting it break mount propagation into the already-running pod? If not, the docstring and `stop_at` name are confusing since it says it stops there.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(computing-unit): out-of-pod dataset mount infrastructure [texera]
Posted by "chenlica (via GitHub)" <gi...@apache.org>.
chenlica commented on PR #6866:
URL: https://github.com/apache/texera/pull/6866#issuecomment-5233799137
@aicam Please revise the diagram as requested. Thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(computing-unit): out-of-pod dataset mount infrastructure [texera]
Posted by "aicam (via GitHub)" <gi...@apache.org>.
aicam commented on code in PR #6866:
URL: https://github.com/apache/texera/pull/6866#discussion_r3779431765
##########
bin/k8s/templates/base/workflow-computing-unit-manager/workflow-computing-unit-manager-deployment.yaml:
##########
@@ -64,6 +64,8 @@ spec:
value: {{ .Values.workflowComputingUnitPool.namespace }}
- name: KUBERNETES_COMPUTE_UNIT_SERVICE_NAME
value: {{ .Values.workflowComputingUnitPool.name }}-svc
+ - name: KUBERNETES_COMPUTE_UNIT_POD_NAME_PREFIX
Review Comment:
I don't think so, for configurations that typically use default value we don't add them to values.yaml because they always have default value. For example, we don't put `KUBERNETES_COMPUTE_UNIT_POOL_NAMESPACE` in values.yaml because we always use the default `texera-workflow-computing-unit-pool` value
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(computing-unit): out-of-pod dataset mount infrastructure [texera]
Posted by "aicam (via GitHub)" <gi...@apache.org>.
aicam commented on code in PR #6866:
URL: https://github.com/apache/texera/pull/6866#discussion_r3798612149
##########
bin/mounter/mounter.py:
##########
Review Comment:
Since we limit access to `access-control` service, Python UDF can not mount
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(computing-unit): out-of-pod LakeFS repository mount infrastructure [texera]
Posted by "mengw15 (via GitHub)" <gi...@apache.org>.
mengw15 commented on code in PR #6866:
URL: https://github.com/apache/texera/pull/6866#discussion_r3928979050
##########
file-service/src/main/scala/org/apache/texera/service/util/S3ProxyServlet.scala:
##########
@@ -0,0 +1,260 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.
+ */
+
+package org.apache.texera.service.util
+
+import com.typesafe.scalalogging.LazyLogging
+import jakarta.servlet.http.{HttpServlet, HttpServletRequest, HttpServletResponse}
+import org.apache.texera.auth.JwtParser
+import org.apache.texera.common.config.StorageConfig
+import org.apache.texera.dao.SqlServer
+import org.apache.texera.dao.SqlServer.withTransaction
+import org.apache.texera.dao.jooq.generated.tables.daos.DatasetDao
+import org.apache.texera.service.resource.DatasetAccessResource.userHasReadAccess
+import software.amazon.awssdk.auth.credentials.AwsBasicCredentials
+import software.amazon.awssdk.auth.signer.AwsS3V4Signer
+import software.amazon.awssdk.auth.signer.params.AwsS3V4SignerParams
+import software.amazon.awssdk.http.{SdkHttpFullRequest, SdkHttpMethod}
+import software.amazon.awssdk.regions.Region
+
+import java.net.{URI, URLDecoder}
+import java.net.http.{HttpClient, HttpRequest, HttpResponse}
+import java.time.Duration
+import java.util.concurrent.ConcurrentHashMap
+import scala.jdk.CollectionConverters._
+import scala.jdk.OptionConverters._
+
+/**
+ * Read-only, JWT-authenticated, re-signing reverse proxy in front of the LakeFS S3
+ * gateway. A computing-unit pod's GeeseFS mount talks to this servlet using the pod's
+ * own per-user JWT as the S3 credential: the JWT is passed to GeeseFS as
+ * `AWS_ACCESS_KEY_ID`, so it rides in the request's SigV4/SigV2 `Authorization` header.
+ * Reusing the JWT that is already present in the pod means no separate mount credential
+ * is ever issued, stored, or made multi-replica-consistent. The servlet:
+ *
+ * 1. reads the JWT back out of the incoming `Authorization` header (the JWT is the
+ * bearer capability; the pod-side S3 signature is not re-validated, and no LakeFS
+ * credentials ever leave this service),
+ * 2. verifies the JWT and checks that its user has read access to the requested
+ * repository (the S3 bucket), using the same `userHasReadAccess` gate as the
+ * dataset REST endpoints, and
+ * 3. re-signs the request with the global LakeFS credentials and forwards it to the
+ * LakeFS S3 gateway, streaming the response back.
+ *
+ * Because requests are forwarded verbatim, the proxy behaves identically to a direct
+ * GeeseFS -> LakeFS-gateway mount, just re-authenticated. GeeseFS mounts read-only, so
+ * only GET and HEAD are handled.
+ */
+class S3ProxyServlet extends HttpServlet with LazyLogging {
+
+ // The LakeFS S3 gateway shares the LakeFS server address: the configured API endpoint
+ // with the trailing /api/v1 suffix removed.
+ private val gatewayEndpoint: URI =
+ URI.create(StorageConfig.lakefsEndpoint.stripSuffix("/").stripSuffix("/api/v1"))
+
+ private val lakefsCredentials =
+ AwsBasicCredentials.create(StorageConfig.lakefsUsername, StorageConfig.lakefsPassword)
+
+ // The S3-specific SigV4 signer adds and signs the x-amz-content-sha256 header (which
+ // S3 / the LakeFS gateway require) and disables path double-encoding — both essential
+ // for the signature to validate. The generic Aws4Signer omits x-amz-content-sha256.
+ private val signer = AwsS3V4Signer.create()
+
+ private val httpClient: HttpClient = HttpClient
+ .newBuilder()
+ .followRedirects(HttpClient.Redirect.NEVER)
+ .connectTimeout(Duration.ofSeconds(10))
+ .build()
+
+ private val forwardedResponseHeaderPrefixes =
+ Seq("content-", "etag", "last-modified", "accept-ranges", "x-amz-")
+
+ // Short-lived cache of (uid, repository) -> read-access decision. A mount issues many
+ // range reads for the same repository, so this avoids a DB round-trip per request. It
+ // is a pure optimization: each replica caches independently and a miss just re-queries,
+ // so unlike a shared session store it needs no cross-replica consistency.
+ private val AuthCacheTtlMs = 60000L
+ private case class CachedDecision(allowed: Boolean, expiresAtMs: Long)
+ private val authCache = new ConcurrentHashMap[String, CachedDecision]()
+
+ override def doGet(req: HttpServletRequest, resp: HttpServletResponse): Unit =
+ proxy(req, resp, SdkHttpMethod.GET, streamBody = true)
+
+ override def doHead(req: HttpServletRequest, resp: HttpServletResponse): Unit =
+ proxy(req, resp, SdkHttpMethod.HEAD, streamBody = false)
+
+ private def proxy(
+ req: HttpServletRequest,
+ resp: HttpServletResponse,
+ method: SdkHttpMethod,
+ streamBody: Boolean
+ ): Unit = {
+ val user = S3ProxyServlet
+ .extractCredentialToken(req.getHeader("Authorization"))
+ .flatMap(token => JwtParser.parseToken(token).toScala)
+ if (user.isEmpty) {
+ // GeeseFS probes the bucket unauthenticated on mount, so this is expected noise.
+ resp.sendError(HttpServletResponse.SC_FORBIDDEN, "missing or invalid user token")
+ return
+ }
+
+ val uid = user.get.getUid
+ val repositoryName = S3ProxyServlet.bucketFromUri(req.getRequestURI)
+ if (repositoryName.isEmpty || !authorizedToRead(uid, repositoryName)) {
+ logger.warn(
+ s"user $uid denied mount access to repository '$repositoryName' for ${req.getRequestURI}"
+ )
+ resp.sendError(HttpServletResponse.SC_FORBIDDEN, "no read access to the requested repository")
+ return
+ }
+
+ try {
+ writeResponse(forward(req, method), resp, streamBody)
+ } catch {
+ case e: Exception =>
+ logger.error(s"error proxying ${req.getRequestURI} to LakeFS gateway", e)
+ resp.sendError(HttpServletResponse.SC_BAD_GATEWAY, "upstream error")
+ }
+ }
+
+ /**
+ * True iff `uid` has read access to the dataset backing `repositoryName`, cached for a
+ * short window. Read access to a repository grants read to all of its commits, so no
+ * per-commit check is needed: a session addresses a single repository's data and any
+ * version the user may already read.
+ */
+ private def authorizedToRead(uid: Integer, repositoryName: String): Boolean = {
+ val now = System.currentTimeMillis()
+ val cacheKey = s"$uid:$repositoryName"
+ val cached = authCache.get(cacheKey)
+ if (cached != null && cached.expiresAtMs > now) {
+ return cached.allowed
+ }
+
+ val allowed = withTransaction(SqlServer.getInstance().createDSLContext()) { ctx =>
+ val datasets = new DatasetDao(ctx.configuration())
+ .fetchByRepositoryName(repositoryName)
+ .asScala
+ .toList
+ datasets.nonEmpty && userHasReadAccess(ctx, datasets.head.getDid, uid)
Review Comment:
`datasets.head` decides the access question against an arbitrary row. `dataset.repository_name` has no unique constraint — `sql/texera_ddl.sql` declares it `repository_name VARCHAR(128),` (nullable), and the table's only `UNIQUE` is `(owner_uid, name)` — so `fetchByRepositoryName` can legitimately return more than one row, and which one lands in `.head` is jOOQ's choice.
It's unique in practice today because `repository_name` is written as `dataset-<did>`, so this isn't exploitable as it stands. But this is the gate in front of a proxy that re-signs with the global LakeFS credentials, and a uniqueness assumption that only the calling convention enforces is a thin thing to rest that on: if two rows ever share a name, the decision is arbitrary rather than fail-closed.
Cheap either way — require exactly one match and deny otherwise, or parse the did out of the repository name and look that up directly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(computing-unit): out-of-pod dataset mount infrastructure [texera]
Posted by "chenlica (via GitHub)" <gi...@apache.org>.
chenlica commented on PR #6866:
URL: https://github.com/apache/texera/pull/6866#issuecomment-5106238727
@aicam The layout IS different, especially the orientation. Please revise it to be consistent.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(computing-unit): out-of-pod dataset mount infrastructure [texera]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #6866:
URL: https://github.com/apache/texera/pull/6866#issuecomment-5074481484
<!-- texera-benchmarks-comment -->
## ⚠️ Benchmark changes need a look
🟢 2 better · 🔴 2 worse · ⚪ 11 noise (<±5%) · 0 without baseline
> Compared against main `f02dd2f` benchmarked on **this same runner**, so the delta is largely free of cross-runner hardware noise. The "7d avg" column still reflects the gh-pages dashboard. Treat <±5% as noise unless repeated.
[Dashboard](https://apache.github.io/texera/dev/bench/) · [Run](https://github.com/apache/texera/actions/runs/30126810964)
| | config | throughput | MB/s | latency | max Δ latest / 7d |
|---|---|---:|---:|---:|---:|
| 🔴 | bs=10 sw=10 sl=64 | 453 | 0.276 | 20,926/30,741/30,741 us | 🔴 +5.4% / 🔴 +85.2% |
| 🟢 | bs=100 sw=10 sl=64 | 963 | 0.588 | 103,144/116,161/116,161 us | 🟢 -11.8% / 🔴 +6.5% |
| ⚪ | bs=1000 sw=10 sl=64 | 1,083 | 0.661 | 923,020/973,037/973,037 us | ⚪ within ±5% / 🟢 -9.4% |
<details><summary>Baseline details</summary>
Latest main `f02dd2f` from same runner
| config | metric | PR | latest main | 7d avg | Δ latest | Δ 7d |
|---|---|---:|---:|---:|---:|---:|
| bs=10 sw=10 sl=64 | throughput | 453 tuples/sec | 473 tuples/sec | 754.55 tuples/sec | -4.2% | -40.0% |
| bs=10 sw=10 sl=64 | MB/s | 0.276 MB/s | 0.289 MB/s | 0.461 MB/s | -4.5% | -40.1% |
| bs=10 sw=10 sl=64 | p50 | 20,926 us | 21,252 us | 12,816 us | -1.5% | +63.3% |
| bs=10 sw=10 sl=64 | p95 | 30,741 us | 29,165 us | 16,594 us | +5.4% | +85.2% |
| bs=10 sw=10 sl=64 | p99 | 30,741 us | 29,165 us | 19,806 us | +5.4% | +55.2% |
| bs=100 sw=10 sl=64 | throughput | 963 tuples/sec | 964 tuples/sec | 969.38 tuples/sec | -0.1% | -0.7% |
| bs=100 sw=10 sl=64 | MB/s | 0.588 MB/s | 0.588 MB/s | 0.592 MB/s | 0.0% | -0.6% |
| bs=100 sw=10 sl=64 | p50 | 103,144 us | 102,846 us | 103,584 us | +0.3% | -0.4% |
| bs=100 sw=10 sl=64 | p95 | 116,161 us | 131,641 us | 109,097 us | -11.8% | +6.5% |
| bs=100 sw=10 sl=64 | p99 | 116,161 us | 131,641 us | 117,304 us | -11.8% | -1.0% |
| bs=1000 sw=10 sl=64 | throughput | 1,083 tuples/sec | 1,085 tuples/sec | 1,004 tuples/sec | -0.2% | +7.9% |
| bs=1000 sw=10 sl=64 | MB/s | 0.661 MB/s | 0.662 MB/s | 0.613 MB/s | -0.2% | +7.9% |
| bs=1000 sw=10 sl=64 | p50 | 923,020 us | 921,358 us | 1,002,357 us | +0.2% | -7.9% |
| bs=1000 sw=10 sl=64 | p95 | 973,037 us | 983,661 us | 1,046,463 us | -1.1% | -7.0% |
| bs=1000 sw=10 sl=64 | p99 | 973,037 us | 983,661 us | 1,073,661 us | -1.1% | -9.4% |
</details>
<details><summary>Raw CSV</summary>
```csv
config_idx,batch_size,schema_width,string_len,num_batches,total_ms,total_tuples,total_bytes,tuples_per_sec,mb_per_sec,lat_p50_us,lat_p95_us,lat_p99_us
0,10,10,64,20,441.71,200,128000,453,0.276,20926.45,30740.64,30740.64
1,100,10,64,20,2075.83,2000,1280000,963,0.588,103143.72,116160.56,116160.56
2,1000,10,64,20,18462.71,20000,12800000,1083,0.661,923020.33,973036.86,973036.86
```
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(computing-unit): out-of-pod dataset mount infrastructure [texera]
Posted by "kunwp1 (via GitHub)" <gi...@apache.org>.
kunwp1 commented on code in PR #6866:
URL: https://github.com/apache/texera/pull/6866#discussion_r3780247728
##########
bin/mounter/mounter.py:
##########
Review Comment:
> Regarding path, computing unit name is "computing-unit-" which has no `..` and version is hex digest which also don't have `..` so path will be always correct.
Your claim assumes if it's only passed by trusted caller. Authenticated user can still for example write a python UDF with POST /mount code and pass weird values for repo name and commit hash. For security reasons, better to sanitize what the user sends.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(computing-unit): out-of-pod dataset mount infrastructure [texera]
Posted by "kunwp1 (via GitHub)" <gi...@apache.org>.
kunwp1 commented on code in PR #6866:
URL: https://github.com/apache/texera/pull/6866#discussion_r3780198054
##########
bin/mounter/mounter.py:
##########
Review Comment:
I still think we need authentication on `/mount` and `/mounts`. Attacker can use CU A to mount his dataset into CU B's directory and also use GET /mounts to enumerate all the repos/commits/paths. I also don't think it's a good idea to solve security issues in later PRs. Better to just include it here if possible.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(computing-unit): out-of-pod LakeFS repository mount infrastructure [texera]
Posted by "mengw15 (via GitHub)" <gi...@apache.org>.
mengw15 commented on code in PR #6866:
URL: https://github.com/apache/texera/pull/6866#discussion_r3929282602
##########
file-service/src/main/scala/org/apache/texera/service/util/S3ProxyServlet.scala:
##########
@@ -0,0 +1,273 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.
+ */
+
+package org.apache.texera.service.util
+
+import com.typesafe.scalalogging.LazyLogging
+import jakarta.servlet.http.{HttpServlet, HttpServletRequest, HttpServletResponse}
+import org.apache.texera.auth.JwtParser
+import org.apache.texera.common.config.StorageConfig
+import org.apache.texera.dao.SqlServer
+import org.apache.texera.dao.SqlServer.withTransaction
+import org.apache.texera.dao.jooq.generated.tables.daos.{DatasetDao, ModelDao}
+import org.apache.texera.service.resource.{DatasetAccessResource, ModelAccessResource}
+import software.amazon.awssdk.auth.credentials.AwsBasicCredentials
+import software.amazon.awssdk.auth.signer.AwsS3V4Signer
+import software.amazon.awssdk.auth.signer.params.AwsS3V4SignerParams
+import software.amazon.awssdk.http.{SdkHttpFullRequest, SdkHttpMethod}
+import software.amazon.awssdk.regions.Region
+
+import java.net.{URI, URLDecoder}
+import java.net.http.{HttpClient, HttpRequest, HttpResponse}
+import java.time.Duration
+import scala.jdk.CollectionConverters._
+import scala.jdk.OptionConverters._
+
+/**
+ * Read-only, JWT-authenticated, re-signing reverse proxy in front of the LakeFS S3
+ * gateway. A computing-unit pod's GeeseFS mount talks to this servlet using the pod's
+ * own per-user JWT as the S3 credential: the JWT is passed to GeeseFS as
+ * `AWS_ACCESS_KEY_ID`, so it rides in the request's SigV4/SigV2 `Authorization` header.
+ * Reusing the JWT that is already present in the pod means no separate mount credential
+ * is ever issued, stored, or made multi-replica-consistent. The servlet:
+ *
+ * 1. reads the JWT back out of the incoming `Authorization` header (the JWT is the
+ * bearer capability; the pod-side S3 signature is not re-validated, and no LakeFS
+ * credentials ever leave this service),
+ * 2. verifies the JWT and checks that its user has read access to the requested
+ * repository (the S3 bucket), using the same `userHasReadAccess` gate as the
+ * dataset REST endpoints, and
+ * 3. re-signs the request with the global LakeFS credentials and forwards it to the
+ * LakeFS S3 gateway, streaming the response back.
+ *
+ * Because requests are forwarded verbatim, the proxy behaves identically to a direct
+ * GeeseFS -> LakeFS-gateway mount, just re-authenticated. GeeseFS mounts read-only, so
+ * only GET and HEAD are handled.
+ */
+class S3ProxyServlet extends HttpServlet with LazyLogging {
+
+ // The LakeFS S3 gateway shares the LakeFS server address: the configured API endpoint
+ // with the trailing /api/v1 suffix removed.
+ private val gatewayEndpoint: URI =
+ URI.create(StorageConfig.lakefsEndpoint.stripSuffix("/").stripSuffix("/api/v1"))
+
+ private val lakefsCredentials =
+ AwsBasicCredentials.create(StorageConfig.lakefsUsername, StorageConfig.lakefsPassword)
+
+ // The S3-specific SigV4 signer adds and signs the x-amz-content-sha256 header (which
+ // S3 / the LakeFS gateway require) and disables path double-encoding — both essential
+ // for the signature to validate. The generic Aws4Signer omits x-amz-content-sha256.
+ private val signer = AwsS3V4Signer.create()
+
+ private val httpClient: HttpClient = HttpClient
+ .newBuilder()
+ .followRedirects(HttpClient.Redirect.NEVER)
+ .connectTimeout(Duration.ofSeconds(10))
+ .build()
+
+ private val forwardedResponseHeaderPrefixes =
+ Seq("content-", "etag", "last-modified", "accept-ranges", "x-amz-")
+
+ override def doGet(req: HttpServletRequest, resp: HttpServletResponse): Unit =
+ proxy(req, resp, SdkHttpMethod.GET, streamBody = true)
+
+ override def doHead(req: HttpServletRequest, resp: HttpServletResponse): Unit =
+ proxy(req, resp, SdkHttpMethod.HEAD, streamBody = false)
+
+ private def proxy(
+ req: HttpServletRequest,
+ resp: HttpServletResponse,
+ method: SdkHttpMethod,
+ streamBody: Boolean
+ ): Unit = {
+ val user = S3ProxyServlet
+ .extractCredentialToken(req.getHeader("Authorization"))
+ .flatMap(token => JwtParser.parseToken(token).toScala)
+ if (user.isEmpty) {
+ // GeeseFS probes the bucket unauthenticated on mount, so this is expected noise.
+ resp.sendError(HttpServletResponse.SC_FORBIDDEN, "missing or invalid user token")
+ return
+ }
+
+ val uid = user.get.getUid
+ val repositoryName = S3ProxyServlet.bucketFromUri(req.getRequestURI)
+ if (repositoryName.isEmpty || !authorizedToRead(uid, repositoryName)) {
+ logger.warn(
+ s"user $uid denied mount access to repository '$repositoryName' for ${req.getRequestURI}"
+ )
+ resp.sendError(HttpServletResponse.SC_FORBIDDEN, "no read access to the requested repository")
+ return
+ }
+
+ try {
+ writeResponse(forward(req, method), resp, streamBody)
+ } catch {
+ case e: Exception =>
+ logger.error(s"error proxying ${req.getRequestURI} to LakeFS gateway", e)
+ resp.sendError(HttpServletResponse.SC_BAD_GATEWAY, "upstream error")
+ }
+ }
+
+ /**
+ * True iff `uid` has read access to the versioned resource backing `repositoryName`.
+ *
+ * Read access to a repository grants read to all of its commits, so no per-commit check
+ * is needed: a session addresses a single repository's data and any version the user may
+ * already read.
+ *
+ * Both resource types are searched, because a repository backs either a dataset or a
+ * model, and the name is matched rather than parsed: rows created today are named
+ * `<type>-<id>`, but `sql/updates/15.sql` backfilled the column from the dataset's plain
+ * `name`, so an upgraded deployment still has repositories called e.g. `my-data`.
+ *
+ * Exactly one row must match, or the request is denied. A repository name is unique in
+ * practice -- LakeFS will not create two repositories with one name, and dataset creation
+ * checked the name globally besides -- but `repository_name` carries no unique constraint
+ * (the only UNIQUE on either table is (owner_uid, name)), so nothing in the schema says
+ * so. Rather than let `.head` pick a row, and decide access arbitrarily in front of a
+ * proxy that re-signs with the global LakeFS credentials, an ambiguous name fails closed.
+ */
+ private def authorizedToRead(uid: Integer, repositoryName: String): Boolean =
+ withTransaction(SqlServer.getInstance().createDSLContext()) { ctx =>
+ val datasets = new DatasetDao(ctx.configuration())
+ .fetchByRepositoryName(repositoryName)
+ .asScala
+ .toList
+ val models = new ModelDao(ctx.configuration())
Review Comment:
The fail-closed rewrite is right, and better than what I suggested — thanks for chasing `15.sql` down. You're correct that parsing the id would have denied every upgraded deployment's datasets, since `UPDATE dataset SET repository_name = name` leaves them named `my-data` rather than `dataset-<did>`. That also cuts against my own framing: legacy rows carry the dataset's plain `name`, and `name` is only unique per owner, so collisions were more plausible than my comment implied — which makes denying on ambiguity better motivated, not less.
The cache removal looks like it overshot, though. Copilot's objection was unboundedness — an attacker-controlled key growing the heap — and the remedy it named was a cache with expiry *and* a maximum size. Deleting it instead trades a memory bound for database load, and the comment that went with it stated the cost up front: a mount issues many range reads for the same repository.
What each of those reads now does, inside a transaction: `SELECT * FROM dataset WHERE repository_name = ?`, then the same on `model`, then the access check. Neither table has an index on `repository_name` — `texera_ddl.sql` and `sql/updates/*` define only `idx_dataset_contributor_did_email` for this family — so that's two sequential scans per range GET. The model query also runs even when the dataset already matched, since both lists are built before the `match`. Your own end-to-end run read a ~2 GB sharded model through this path.
Any one of three would do: short-circuit the model lookup when a dataset matched, index `repository_name` on both tables, or bring the cache back with the bound Copilot asked for.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(computing-unit): out-of-pod LakeFS repository mount infrastructure [texera]
Posted by "aicam (via GitHub)" <gi...@apache.org>.
aicam commented on code in PR #6866:
URL: https://github.com/apache/texera/pull/6866#discussion_r3929226028
##########
file-service/src/main/scala/org/apache/texera/service/util/S3ProxyServlet.scala:
##########
@@ -0,0 +1,260 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.
+ */
+
+package org.apache.texera.service.util
+
+import com.typesafe.scalalogging.LazyLogging
+import jakarta.servlet.http.{HttpServlet, HttpServletRequest, HttpServletResponse}
+import org.apache.texera.auth.JwtParser
+import org.apache.texera.common.config.StorageConfig
+import org.apache.texera.dao.SqlServer
+import org.apache.texera.dao.SqlServer.withTransaction
+import org.apache.texera.dao.jooq.generated.tables.daos.DatasetDao
+import org.apache.texera.service.resource.DatasetAccessResource.userHasReadAccess
+import software.amazon.awssdk.auth.credentials.AwsBasicCredentials
+import software.amazon.awssdk.auth.signer.AwsS3V4Signer
+import software.amazon.awssdk.auth.signer.params.AwsS3V4SignerParams
+import software.amazon.awssdk.http.{SdkHttpFullRequest, SdkHttpMethod}
+import software.amazon.awssdk.regions.Region
+
+import java.net.{URI, URLDecoder}
+import java.net.http.{HttpClient, HttpRequest, HttpResponse}
+import java.time.Duration
+import java.util.concurrent.ConcurrentHashMap
+import scala.jdk.CollectionConverters._
+import scala.jdk.OptionConverters._
+
+/**
+ * Read-only, JWT-authenticated, re-signing reverse proxy in front of the LakeFS S3
+ * gateway. A computing-unit pod's GeeseFS mount talks to this servlet using the pod's
+ * own per-user JWT as the S3 credential: the JWT is passed to GeeseFS as
+ * `AWS_ACCESS_KEY_ID`, so it rides in the request's SigV4/SigV2 `Authorization` header.
+ * Reusing the JWT that is already present in the pod means no separate mount credential
+ * is ever issued, stored, or made multi-replica-consistent. The servlet:
+ *
+ * 1. reads the JWT back out of the incoming `Authorization` header (the JWT is the
+ * bearer capability; the pod-side S3 signature is not re-validated, and no LakeFS
+ * credentials ever leave this service),
+ * 2. verifies the JWT and checks that its user has read access to the requested
+ * repository (the S3 bucket), using the same `userHasReadAccess` gate as the
+ * dataset REST endpoints, and
+ * 3. re-signs the request with the global LakeFS credentials and forwards it to the
+ * LakeFS S3 gateway, streaming the response back.
+ *
+ * Because requests are forwarded verbatim, the proxy behaves identically to a direct
+ * GeeseFS -> LakeFS-gateway mount, just re-authenticated. GeeseFS mounts read-only, so
+ * only GET and HEAD are handled.
+ */
+class S3ProxyServlet extends HttpServlet with LazyLogging {
+
+ // The LakeFS S3 gateway shares the LakeFS server address: the configured API endpoint
+ // with the trailing /api/v1 suffix removed.
+ private val gatewayEndpoint: URI =
+ URI.create(StorageConfig.lakefsEndpoint.stripSuffix("/").stripSuffix("/api/v1"))
+
+ private val lakefsCredentials =
+ AwsBasicCredentials.create(StorageConfig.lakefsUsername, StorageConfig.lakefsPassword)
+
+ // The S3-specific SigV4 signer adds and signs the x-amz-content-sha256 header (which
+ // S3 / the LakeFS gateway require) and disables path double-encoding — both essential
+ // for the signature to validate. The generic Aws4Signer omits x-amz-content-sha256.
+ private val signer = AwsS3V4Signer.create()
+
+ private val httpClient: HttpClient = HttpClient
+ .newBuilder()
+ .followRedirects(HttpClient.Redirect.NEVER)
+ .connectTimeout(Duration.ofSeconds(10))
+ .build()
+
+ private val forwardedResponseHeaderPrefixes =
+ Seq("content-", "etag", "last-modified", "accept-ranges", "x-amz-")
+
+ // Short-lived cache of (uid, repository) -> read-access decision. A mount issues many
+ // range reads for the same repository, so this avoids a DB round-trip per request. It
+ // is a pure optimization: each replica caches independently and a miss just re-queries,
+ // so unlike a shared session store it needs no cross-replica consistency.
+ private val AuthCacheTtlMs = 60000L
+ private case class CachedDecision(allowed: Boolean, expiresAtMs: Long)
+ private val authCache = new ConcurrentHashMap[String, CachedDecision]()
+
+ override def doGet(req: HttpServletRequest, resp: HttpServletResponse): Unit =
+ proxy(req, resp, SdkHttpMethod.GET, streamBody = true)
+
+ override def doHead(req: HttpServletRequest, resp: HttpServletResponse): Unit =
+ proxy(req, resp, SdkHttpMethod.HEAD, streamBody = false)
+
+ private def proxy(
+ req: HttpServletRequest,
+ resp: HttpServletResponse,
+ method: SdkHttpMethod,
+ streamBody: Boolean
+ ): Unit = {
+ val user = S3ProxyServlet
+ .extractCredentialToken(req.getHeader("Authorization"))
+ .flatMap(token => JwtParser.parseToken(token).toScala)
+ if (user.isEmpty) {
+ // GeeseFS probes the bucket unauthenticated on mount, so this is expected noise.
+ resp.sendError(HttpServletResponse.SC_FORBIDDEN, "missing or invalid user token")
+ return
+ }
+
+ val uid = user.get.getUid
+ val repositoryName = S3ProxyServlet.bucketFromUri(req.getRequestURI)
+ if (repositoryName.isEmpty || !authorizedToRead(uid, repositoryName)) {
+ logger.warn(
+ s"user $uid denied mount access to repository '$repositoryName' for ${req.getRequestURI}"
+ )
+ resp.sendError(HttpServletResponse.SC_FORBIDDEN, "no read access to the requested repository")
+ return
+ }
+
+ try {
+ writeResponse(forward(req, method), resp, streamBody)
+ } catch {
+ case e: Exception =>
+ logger.error(s"error proxying ${req.getRequestURI} to LakeFS gateway", e)
+ resp.sendError(HttpServletResponse.SC_BAD_GATEWAY, "upstream error")
+ }
+ }
+
+ /**
+ * True iff `uid` has read access to the dataset backing `repositoryName`, cached for a
+ * short window. Read access to a repository grants read to all of its commits, so no
+ * per-commit check is needed: a session addresses a single repository's data and any
+ * version the user may already read.
+ */
+ private def authorizedToRead(uid: Integer, repositoryName: String): Boolean = {
+ val now = System.currentTimeMillis()
+ val cacheKey = s"$uid:$repositoryName"
+ val cached = authCache.get(cacheKey)
+ if (cached != null && cached.expiresAtMs > now) {
+ return cached.allowed
+ }
+
+ val allowed = withTransaction(SqlServer.getInstance().createDSLContext()) { ctx =>
+ val datasets = new DatasetDao(ctx.configuration())
+ .fetchByRepositoryName(repositoryName)
+ .asScala
+ .toList
+ datasets.nonEmpty && userHasReadAccess(ctx, datasets.head.getDid, uid)
Review Comment:
Fixed, and you were right that it is not exploitable — worth recording why, since it is stronger than the naming convention: LakeFS refuses a second repository with the same name, and dataset creation checked the name globally (`fetchByName`, not per-owner) before that. So one repository name is one row.
The schema still does not say so, and the fix is free, so the lookup now requires exactly one match across `dataset` and `model` and denies (with a log) otherwise, rather than letting `.head` pick.
One thing your comment led me to that does matter: I first tried parsing the `<type>-<id>` out of the name and looking it up by primary key. That is wrong — `sql/updates/15.sql` backfilled `repository_name` from the dataset's plain `name`, so upgraded deployments still have repositories called e.g. `my-data`, and parsing would have denied every one of them. Matching on the column handles both. (34c8a12d3)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(computing-unit): out-of-pod dataset mount infrastructure [texera]
Posted by "aicam (via GitHub)" <gi...@apache.org>.
aicam commented on PR #6866:
URL: https://github.com/apache/texera/pull/6866#issuecomment-5094976497
> @aicam Please revise the diagram to use a layout consistent with other diagrams.
Its the same layout, its just horizontal to save space and non-related microservices are removed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(computing-unit): out-of-pod dataset mount infrastructure [texera]
Posted by "aicam (via GitHub)" <gi...@apache.org>.
aicam commented on PR #6866:
URL: https://github.com/apache/texera/pull/6866#issuecomment-5247022112
> @aicam Please revise the diagram as requested. Thanks.
Done, I adopted our base diagram
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(computing-unit): out-of-pod dataset mount infrastructure [texera]
Posted by "kunwp1 (via GitHub)" <gi...@apache.org>.
kunwp1 commented on code in PR #6866:
URL: https://github.com/apache/texera/pull/6866#discussion_r3780121455
##########
bin/k8s/templates/base/workflow-computing-unit-manager/workflow-computing-unit-manager-deployment.yaml:
##########
@@ -64,6 +64,8 @@ spec:
value: {{ .Values.workflowComputingUnitPool.namespace }}
- name: KUBERNETES_COMPUTE_UNIT_SERVICE_NAME
value: {{ .Values.workflowComputingUnitPool.name }}-svc
+ - name: KUBERNETES_COMPUTE_UNIT_POD_NAME_PREFIX
Review Comment:
The issue is different from `KUBERNETES_COMPUTE_UNIT_POOL_NAMESPACE`. In your comment, you mentioned that the Helm values must match the manager's `kubernetes.mounter-port` and `kubernetes.mounter-host-root`. But currently, the parameters are exposed to `Values.yaml` and Helm updates only the DaemonSet and the manager always falls back to `kubernetes.conf` defaults.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(computing-unit): out-of-pod LakeFS repository mount infrastructure [texera]
Posted by "aicam (via GitHub)" <gi...@apache.org>.
aicam commented on code in PR #6866:
URL: https://github.com/apache/texera/pull/6866#discussion_r3929224728
##########
file-service/src/main/scala/org/apache/texera/service/util/S3ProxyServlet.scala:
##########
@@ -0,0 +1,260 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.
+ */
+
+package org.apache.texera.service.util
+
+import com.typesafe.scalalogging.LazyLogging
+import jakarta.servlet.http.{HttpServlet, HttpServletRequest, HttpServletResponse}
+import org.apache.texera.auth.JwtParser
+import org.apache.texera.common.config.StorageConfig
+import org.apache.texera.dao.SqlServer
+import org.apache.texera.dao.SqlServer.withTransaction
+import org.apache.texera.dao.jooq.generated.tables.daos.DatasetDao
+import org.apache.texera.service.resource.DatasetAccessResource.userHasReadAccess
+import software.amazon.awssdk.auth.credentials.AwsBasicCredentials
+import software.amazon.awssdk.auth.signer.AwsS3V4Signer
+import software.amazon.awssdk.auth.signer.params.AwsS3V4SignerParams
+import software.amazon.awssdk.http.{SdkHttpFullRequest, SdkHttpMethod}
+import software.amazon.awssdk.regions.Region
+
+import java.net.{URI, URLDecoder}
+import java.net.http.{HttpClient, HttpRequest, HttpResponse}
+import java.time.Duration
+import java.util.concurrent.ConcurrentHashMap
+import scala.jdk.CollectionConverters._
+import scala.jdk.OptionConverters._
+
+/**
+ * Read-only, JWT-authenticated, re-signing reverse proxy in front of the LakeFS S3
+ * gateway. A computing-unit pod's GeeseFS mount talks to this servlet using the pod's
+ * own per-user JWT as the S3 credential: the JWT is passed to GeeseFS as
+ * `AWS_ACCESS_KEY_ID`, so it rides in the request's SigV4/SigV2 `Authorization` header.
+ * Reusing the JWT that is already present in the pod means no separate mount credential
+ * is ever issued, stored, or made multi-replica-consistent. The servlet:
+ *
+ * 1. reads the JWT back out of the incoming `Authorization` header (the JWT is the
+ * bearer capability; the pod-side S3 signature is not re-validated, and no LakeFS
+ * credentials ever leave this service),
+ * 2. verifies the JWT and checks that its user has read access to the requested
+ * repository (the S3 bucket), using the same `userHasReadAccess` gate as the
+ * dataset REST endpoints, and
+ * 3. re-signs the request with the global LakeFS credentials and forwards it to the
+ * LakeFS S3 gateway, streaming the response back.
+ *
+ * Because requests are forwarded verbatim, the proxy behaves identically to a direct
+ * GeeseFS -> LakeFS-gateway mount, just re-authenticated. GeeseFS mounts read-only, so
+ * only GET and HEAD are handled.
+ */
+class S3ProxyServlet extends HttpServlet with LazyLogging {
+
+ // The LakeFS S3 gateway shares the LakeFS server address: the configured API endpoint
+ // with the trailing /api/v1 suffix removed.
+ private val gatewayEndpoint: URI =
+ URI.create(StorageConfig.lakefsEndpoint.stripSuffix("/").stripSuffix("/api/v1"))
+
+ private val lakefsCredentials =
+ AwsBasicCredentials.create(StorageConfig.lakefsUsername, StorageConfig.lakefsPassword)
+
+ // The S3-specific SigV4 signer adds and signs the x-amz-content-sha256 header (which
+ // S3 / the LakeFS gateway require) and disables path double-encoding — both essential
+ // for the signature to validate. The generic Aws4Signer omits x-amz-content-sha256.
+ private val signer = AwsS3V4Signer.create()
+
+ private val httpClient: HttpClient = HttpClient
+ .newBuilder()
+ .followRedirects(HttpClient.Redirect.NEVER)
+ .connectTimeout(Duration.ofSeconds(10))
+ .build()
+
+ private val forwardedResponseHeaderPrefixes =
+ Seq("content-", "etag", "last-modified", "accept-ranges", "x-amz-")
+
+ // Short-lived cache of (uid, repository) -> read-access decision. A mount issues many
+ // range reads for the same repository, so this avoids a DB round-trip per request. It
+ // is a pure optimization: each replica caches independently and a miss just re-queries,
+ // so unlike a shared session store it needs no cross-replica consistency.
+ private val AuthCacheTtlMs = 60000L
+ private case class CachedDecision(allowed: Boolean, expiresAtMs: Long)
+ private val authCache = new ConcurrentHashMap[String, CachedDecision]()
Review Comment:
Removed the cache entirely — you are right that it only evicted on read and inserted on every request, denials included. Each request now queries directly; the round-trip saved was not worth a bounded-cache dependency here. (34c8a12d3)
##########
file-service/src/main/scala/org/apache/texera/service/util/S3ProxyServlet.scala:
##########
@@ -0,0 +1,260 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.
+ */
+
+package org.apache.texera.service.util
+
+import com.typesafe.scalalogging.LazyLogging
+import jakarta.servlet.http.{HttpServlet, HttpServletRequest, HttpServletResponse}
+import org.apache.texera.auth.JwtParser
+import org.apache.texera.common.config.StorageConfig
+import org.apache.texera.dao.SqlServer
+import org.apache.texera.dao.SqlServer.withTransaction
+import org.apache.texera.dao.jooq.generated.tables.daos.DatasetDao
+import org.apache.texera.service.resource.DatasetAccessResource.userHasReadAccess
+import software.amazon.awssdk.auth.credentials.AwsBasicCredentials
+import software.amazon.awssdk.auth.signer.AwsS3V4Signer
+import software.amazon.awssdk.auth.signer.params.AwsS3V4SignerParams
+import software.amazon.awssdk.http.{SdkHttpFullRequest, SdkHttpMethod}
+import software.amazon.awssdk.regions.Region
+
+import java.net.{URI, URLDecoder}
+import java.net.http.{HttpClient, HttpRequest, HttpResponse}
+import java.time.Duration
+import java.util.concurrent.ConcurrentHashMap
+import scala.jdk.CollectionConverters._
+import scala.jdk.OptionConverters._
+
+/**
+ * Read-only, JWT-authenticated, re-signing reverse proxy in front of the LakeFS S3
+ * gateway. A computing-unit pod's GeeseFS mount talks to this servlet using the pod's
+ * own per-user JWT as the S3 credential: the JWT is passed to GeeseFS as
+ * `AWS_ACCESS_KEY_ID`, so it rides in the request's SigV4/SigV2 `Authorization` header.
+ * Reusing the JWT that is already present in the pod means no separate mount credential
+ * is ever issued, stored, or made multi-replica-consistent. The servlet:
+ *
+ * 1. reads the JWT back out of the incoming `Authorization` header (the JWT is the
+ * bearer capability; the pod-side S3 signature is not re-validated, and no LakeFS
+ * credentials ever leave this service),
+ * 2. verifies the JWT and checks that its user has read access to the requested
+ * repository (the S3 bucket), using the same `userHasReadAccess` gate as the
+ * dataset REST endpoints, and
+ * 3. re-signs the request with the global LakeFS credentials and forwards it to the
+ * LakeFS S3 gateway, streaming the response back.
+ *
+ * Because requests are forwarded verbatim, the proxy behaves identically to a direct
+ * GeeseFS -> LakeFS-gateway mount, just re-authenticated. GeeseFS mounts read-only, so
+ * only GET and HEAD are handled.
+ */
+class S3ProxyServlet extends HttpServlet with LazyLogging {
+
+ // The LakeFS S3 gateway shares the LakeFS server address: the configured API endpoint
+ // with the trailing /api/v1 suffix removed.
+ private val gatewayEndpoint: URI =
+ URI.create(StorageConfig.lakefsEndpoint.stripSuffix("/").stripSuffix("/api/v1"))
+
+ private val lakefsCredentials =
+ AwsBasicCredentials.create(StorageConfig.lakefsUsername, StorageConfig.lakefsPassword)
+
+ // The S3-specific SigV4 signer adds and signs the x-amz-content-sha256 header (which
+ // S3 / the LakeFS gateway require) and disables path double-encoding — both essential
+ // for the signature to validate. The generic Aws4Signer omits x-amz-content-sha256.
+ private val signer = AwsS3V4Signer.create()
+
+ private val httpClient: HttpClient = HttpClient
+ .newBuilder()
+ .followRedirects(HttpClient.Redirect.NEVER)
+ .connectTimeout(Duration.ofSeconds(10))
+ .build()
+
+ private val forwardedResponseHeaderPrefixes =
+ Seq("content-", "etag", "last-modified", "accept-ranges", "x-amz-")
+
+ // Short-lived cache of (uid, repository) -> read-access decision. A mount issues many
+ // range reads for the same repository, so this avoids a DB round-trip per request. It
+ // is a pure optimization: each replica caches independently and a miss just re-queries,
+ // so unlike a shared session store it needs no cross-replica consistency.
+ private val AuthCacheTtlMs = 60000L
+ private case class CachedDecision(allowed: Boolean, expiresAtMs: Long)
+ private val authCache = new ConcurrentHashMap[String, CachedDecision]()
+
+ override def doGet(req: HttpServletRequest, resp: HttpServletResponse): Unit =
+ proxy(req, resp, SdkHttpMethod.GET, streamBody = true)
+
+ override def doHead(req: HttpServletRequest, resp: HttpServletResponse): Unit =
+ proxy(req, resp, SdkHttpMethod.HEAD, streamBody = false)
+
+ private def proxy(
+ req: HttpServletRequest,
+ resp: HttpServletResponse,
+ method: SdkHttpMethod,
+ streamBody: Boolean
+ ): Unit = {
+ val user = S3ProxyServlet
+ .extractCredentialToken(req.getHeader("Authorization"))
+ .flatMap(token => JwtParser.parseToken(token).toScala)
+ if (user.isEmpty) {
+ // GeeseFS probes the bucket unauthenticated on mount, so this is expected noise.
+ resp.sendError(HttpServletResponse.SC_FORBIDDEN, "missing or invalid user token")
+ return
+ }
+
+ val uid = user.get.getUid
+ val repositoryName = S3ProxyServlet.bucketFromUri(req.getRequestURI)
+ if (repositoryName.isEmpty || !authorizedToRead(uid, repositoryName)) {
+ logger.warn(
+ s"user $uid denied mount access to repository '$repositoryName' for ${req.getRequestURI}"
+ )
+ resp.sendError(HttpServletResponse.SC_FORBIDDEN, "no read access to the requested repository")
+ return
+ }
+
+ try {
+ writeResponse(forward(req, method), resp, streamBody)
+ } catch {
+ case e: Exception =>
+ logger.error(s"error proxying ${req.getRequestURI} to LakeFS gateway", e)
+ resp.sendError(HttpServletResponse.SC_BAD_GATEWAY, "upstream error")
+ }
+ }
+
+ /**
+ * True iff `uid` has read access to the dataset backing `repositoryName`, cached for a
+ * short window. Read access to a repository grants read to all of its commits, so no
+ * per-commit check is needed: a session addresses a single repository's data and any
+ * version the user may already read.
+ */
+ private def authorizedToRead(uid: Integer, repositoryName: String): Boolean = {
+ val now = System.currentTimeMillis()
+ val cacheKey = s"$uid:$repositoryName"
+ val cached = authCache.get(cacheKey)
+ if (cached != null && cached.expiresAtMs > now) {
+ return cached.allowed
+ }
+
+ val allowed = withTransaction(SqlServer.getInstance().createDSLContext()) { ctx =>
+ val datasets = new DatasetDao(ctx.configuration())
+ .fetchByRepositoryName(repositoryName)
+ .asScala
+ .toList
+ datasets.nonEmpty && userHasReadAccess(ctx, datasets.head.getDid, uid)
Review Comment:
Fixed — both `dataset` and `model` are now searched, with the matching access check (`DatasetAccessResource` / `ModelAccessResource`). (34c8a12d3)
##########
computing-unit-managing-service/src/main/scala/org/apache/texera/service/util/KubernetesClient.scala:
##########
@@ -179,6 +194,18 @@ class KubernetesClient(client: io.fabric8.kubernetes.client.KubernetesClient) {
.withEnv(envList)
.withResources(resourceBuilder.build())
+ // The FUSE mount is performed by the per-node texera-mounter (privileged), not here,
+ // so this pod stays UNPRIVILEGED. It only *receives* the mount via HostToContainer
+ // propagation from a host directory scoped to this CU id (see the hostPath volume below).
+ if (KubernetesConfig.mounterEnabled) {
+ containerBuilder
+ .addNewVolumeMount()
+ .withName("texera-mounts")
+ .withMountPath(inPodMountRoot)
+ .withMountPropagation("HostToContainer")
Review Comment:
Added: a pod-capture test with mounting enabled asserting the per-cuid host path, `DirectoryOrCreate`, `HostToContainer` propagation, both env vars, and that the container stays unprivileged. The default-path test keeps its assertions and now also asserts no `texera-mounts` volume or mount, guarding the PodSecurity-safe default. (d2e62f808)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(computing-unit): out-of-pod LakeFS repository mount infrastructure [texera]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #6866:
URL: https://github.com/apache/texera/pull/6866#discussion_r3928955040
##########
file-service/src/main/scala/org/apache/texera/service/util/S3ProxyServlet.scala:
##########
@@ -0,0 +1,260 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.
+ */
+
+package org.apache.texera.service.util
+
+import com.typesafe.scalalogging.LazyLogging
+import jakarta.servlet.http.{HttpServlet, HttpServletRequest, HttpServletResponse}
+import org.apache.texera.auth.JwtParser
+import org.apache.texera.common.config.StorageConfig
+import org.apache.texera.dao.SqlServer
+import org.apache.texera.dao.SqlServer.withTransaction
+import org.apache.texera.dao.jooq.generated.tables.daos.DatasetDao
+import org.apache.texera.service.resource.DatasetAccessResource.userHasReadAccess
+import software.amazon.awssdk.auth.credentials.AwsBasicCredentials
+import software.amazon.awssdk.auth.signer.AwsS3V4Signer
+import software.amazon.awssdk.auth.signer.params.AwsS3V4SignerParams
+import software.amazon.awssdk.http.{SdkHttpFullRequest, SdkHttpMethod}
+import software.amazon.awssdk.regions.Region
+
+import java.net.{URI, URLDecoder}
+import java.net.http.{HttpClient, HttpRequest, HttpResponse}
+import java.time.Duration
+import java.util.concurrent.ConcurrentHashMap
+import scala.jdk.CollectionConverters._
+import scala.jdk.OptionConverters._
+
+/**
+ * Read-only, JWT-authenticated, re-signing reverse proxy in front of the LakeFS S3
+ * gateway. A computing-unit pod's GeeseFS mount talks to this servlet using the pod's
+ * own per-user JWT as the S3 credential: the JWT is passed to GeeseFS as
+ * `AWS_ACCESS_KEY_ID`, so it rides in the request's SigV4/SigV2 `Authorization` header.
+ * Reusing the JWT that is already present in the pod means no separate mount credential
+ * is ever issued, stored, or made multi-replica-consistent. The servlet:
+ *
+ * 1. reads the JWT back out of the incoming `Authorization` header (the JWT is the
+ * bearer capability; the pod-side S3 signature is not re-validated, and no LakeFS
+ * credentials ever leave this service),
+ * 2. verifies the JWT and checks that its user has read access to the requested
+ * repository (the S3 bucket), using the same `userHasReadAccess` gate as the
+ * dataset REST endpoints, and
+ * 3. re-signs the request with the global LakeFS credentials and forwards it to the
+ * LakeFS S3 gateway, streaming the response back.
+ *
+ * Because requests are forwarded verbatim, the proxy behaves identically to a direct
+ * GeeseFS -> LakeFS-gateway mount, just re-authenticated. GeeseFS mounts read-only, so
+ * only GET and HEAD are handled.
+ */
+class S3ProxyServlet extends HttpServlet with LazyLogging {
+
+ // The LakeFS S3 gateway shares the LakeFS server address: the configured API endpoint
+ // with the trailing /api/v1 suffix removed.
+ private val gatewayEndpoint: URI =
+ URI.create(StorageConfig.lakefsEndpoint.stripSuffix("/").stripSuffix("/api/v1"))
+
+ private val lakefsCredentials =
+ AwsBasicCredentials.create(StorageConfig.lakefsUsername, StorageConfig.lakefsPassword)
+
+ // The S3-specific SigV4 signer adds and signs the x-amz-content-sha256 header (which
+ // S3 / the LakeFS gateway require) and disables path double-encoding — both essential
+ // for the signature to validate. The generic Aws4Signer omits x-amz-content-sha256.
+ private val signer = AwsS3V4Signer.create()
+
+ private val httpClient: HttpClient = HttpClient
+ .newBuilder()
+ .followRedirects(HttpClient.Redirect.NEVER)
+ .connectTimeout(Duration.ofSeconds(10))
+ .build()
+
+ private val forwardedResponseHeaderPrefixes =
+ Seq("content-", "etag", "last-modified", "accept-ranges", "x-amz-")
+
+ // Short-lived cache of (uid, repository) -> read-access decision. A mount issues many
+ // range reads for the same repository, so this avoids a DB round-trip per request. It
+ // is a pure optimization: each replica caches independently and a miss just re-queries,
+ // so unlike a shared session store it needs no cross-replica consistency.
+ private val AuthCacheTtlMs = 60000L
+ private case class CachedDecision(allowed: Boolean, expiresAtMs: Long)
+ private val authCache = new ConcurrentHashMap[String, CachedDecision]()
+
+ override def doGet(req: HttpServletRequest, resp: HttpServletResponse): Unit =
+ proxy(req, resp, SdkHttpMethod.GET, streamBody = true)
+
+ override def doHead(req: HttpServletRequest, resp: HttpServletResponse): Unit =
+ proxy(req, resp, SdkHttpMethod.HEAD, streamBody = false)
+
+ private def proxy(
+ req: HttpServletRequest,
+ resp: HttpServletResponse,
+ method: SdkHttpMethod,
+ streamBody: Boolean
+ ): Unit = {
+ val user = S3ProxyServlet
+ .extractCredentialToken(req.getHeader("Authorization"))
+ .flatMap(token => JwtParser.parseToken(token).toScala)
+ if (user.isEmpty) {
+ // GeeseFS probes the bucket unauthenticated on mount, so this is expected noise.
+ resp.sendError(HttpServletResponse.SC_FORBIDDEN, "missing or invalid user token")
+ return
+ }
+
+ val uid = user.get.getUid
+ val repositoryName = S3ProxyServlet.bucketFromUri(req.getRequestURI)
+ if (repositoryName.isEmpty || !authorizedToRead(uid, repositoryName)) {
+ logger.warn(
+ s"user $uid denied mount access to repository '$repositoryName' for ${req.getRequestURI}"
+ )
+ resp.sendError(HttpServletResponse.SC_FORBIDDEN, "no read access to the requested repository")
+ return
+ }
+
+ try {
+ writeResponse(forward(req, method), resp, streamBody)
+ } catch {
+ case e: Exception =>
+ logger.error(s"error proxying ${req.getRequestURI} to LakeFS gateway", e)
+ resp.sendError(HttpServletResponse.SC_BAD_GATEWAY, "upstream error")
+ }
+ }
+
+ /**
+ * True iff `uid` has read access to the dataset backing `repositoryName`, cached for a
+ * short window. Read access to a repository grants read to all of its commits, so no
+ * per-commit check is needed: a session addresses a single repository's data and any
+ * version the user may already read.
+ */
+ private def authorizedToRead(uid: Integer, repositoryName: String): Boolean = {
+ val now = System.currentTimeMillis()
+ val cacheKey = s"$uid:$repositoryName"
+ val cached = authCache.get(cacheKey)
+ if (cached != null && cached.expiresAtMs > now) {
+ return cached.allowed
+ }
+
+ val allowed = withTransaction(SqlServer.getInstance().createDSLContext()) { ctx =>
+ val datasets = new DatasetDao(ctx.configuration())
+ .fetchByRepositoryName(repositoryName)
+ .asScala
+ .toList
+ datasets.nonEmpty && userHasReadAccess(ctx, datasets.head.getDid, uid)
Review Comment:
The proxy only resolves `Dataset` rows, but this PR and #6606 describe the mount infrastructure as supporting LakeFS repositories for both datasets and models. A valid `model-<mid>` bucket therefore always produces an empty result and is denied. Resolve the repository against both resource types and apply the corresponding access check, or explicitly narrow the advertised scope.
##########
file-service/src/main/scala/org/apache/texera/service/util/S3ProxyServlet.scala:
##########
@@ -0,0 +1,260 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.
+ */
+
+package org.apache.texera.service.util
+
+import com.typesafe.scalalogging.LazyLogging
+import jakarta.servlet.http.{HttpServlet, HttpServletRequest, HttpServletResponse}
+import org.apache.texera.auth.JwtParser
+import org.apache.texera.common.config.StorageConfig
+import org.apache.texera.dao.SqlServer
+import org.apache.texera.dao.SqlServer.withTransaction
+import org.apache.texera.dao.jooq.generated.tables.daos.DatasetDao
+import org.apache.texera.service.resource.DatasetAccessResource.userHasReadAccess
+import software.amazon.awssdk.auth.credentials.AwsBasicCredentials
+import software.amazon.awssdk.auth.signer.AwsS3V4Signer
+import software.amazon.awssdk.auth.signer.params.AwsS3V4SignerParams
+import software.amazon.awssdk.http.{SdkHttpFullRequest, SdkHttpMethod}
+import software.amazon.awssdk.regions.Region
+
+import java.net.{URI, URLDecoder}
+import java.net.http.{HttpClient, HttpRequest, HttpResponse}
+import java.time.Duration
+import java.util.concurrent.ConcurrentHashMap
+import scala.jdk.CollectionConverters._
+import scala.jdk.OptionConverters._
+
+/**
+ * Read-only, JWT-authenticated, re-signing reverse proxy in front of the LakeFS S3
+ * gateway. A computing-unit pod's GeeseFS mount talks to this servlet using the pod's
+ * own per-user JWT as the S3 credential: the JWT is passed to GeeseFS as
+ * `AWS_ACCESS_KEY_ID`, so it rides in the request's SigV4/SigV2 `Authorization` header.
+ * Reusing the JWT that is already present in the pod means no separate mount credential
+ * is ever issued, stored, or made multi-replica-consistent. The servlet:
+ *
+ * 1. reads the JWT back out of the incoming `Authorization` header (the JWT is the
+ * bearer capability; the pod-side S3 signature is not re-validated, and no LakeFS
+ * credentials ever leave this service),
+ * 2. verifies the JWT and checks that its user has read access to the requested
+ * repository (the S3 bucket), using the same `userHasReadAccess` gate as the
+ * dataset REST endpoints, and
+ * 3. re-signs the request with the global LakeFS credentials and forwards it to the
+ * LakeFS S3 gateway, streaming the response back.
+ *
+ * Because requests are forwarded verbatim, the proxy behaves identically to a direct
+ * GeeseFS -> LakeFS-gateway mount, just re-authenticated. GeeseFS mounts read-only, so
+ * only GET and HEAD are handled.
+ */
+class S3ProxyServlet extends HttpServlet with LazyLogging {
+
+ // The LakeFS S3 gateway shares the LakeFS server address: the configured API endpoint
+ // with the trailing /api/v1 suffix removed.
+ private val gatewayEndpoint: URI =
+ URI.create(StorageConfig.lakefsEndpoint.stripSuffix("/").stripSuffix("/api/v1"))
+
+ private val lakefsCredentials =
+ AwsBasicCredentials.create(StorageConfig.lakefsUsername, StorageConfig.lakefsPassword)
+
+ // The S3-specific SigV4 signer adds and signs the x-amz-content-sha256 header (which
+ // S3 / the LakeFS gateway require) and disables path double-encoding — both essential
+ // for the signature to validate. The generic Aws4Signer omits x-amz-content-sha256.
+ private val signer = AwsS3V4Signer.create()
+
+ private val httpClient: HttpClient = HttpClient
+ .newBuilder()
+ .followRedirects(HttpClient.Redirect.NEVER)
+ .connectTimeout(Duration.ofSeconds(10))
+ .build()
+
+ private val forwardedResponseHeaderPrefixes =
+ Seq("content-", "etag", "last-modified", "accept-ranges", "x-amz-")
+
+ // Short-lived cache of (uid, repository) -> read-access decision. A mount issues many
+ // range reads for the same repository, so this avoids a DB round-trip per request. It
+ // is a pure optimization: each replica caches independently and a miss just re-queries,
+ // so unlike a shared session store it needs no cross-replica consistency.
+ private val AuthCacheTtlMs = 60000L
+ private case class CachedDecision(allowed: Boolean, expiresAtMs: Long)
+ private val authCache = new ConcurrentHashMap[String, CachedDecision]()
Review Comment:
This cache never evicts entries: the TTL is checked only when a key is read, and every request (including a denial) is inserted into the `ConcurrentHashMap`. Because an authenticated caller controls the bucket segment, it can generate unlimited unique repository names and grow the file-service heap indefinitely. Use a cache with both expiry-based eviction and a maximum size.
##########
computing-unit-managing-service/src/main/scala/org/apache/texera/service/util/KubernetesClient.scala:
##########
@@ -179,6 +194,18 @@ class KubernetesClient(client: io.fabric8.kubernetes.client.KubernetesClient) {
.withEnv(envList)
.withResources(resourceBuilder.build())
+ // The FUSE mount is performed by the per-node texera-mounter (privileged), not here,
+ // so this pod stays UNPRIVILEGED. It only *receives* the mount via HostToContainer
+ // propagation from a host directory scoped to this CU id (see the hostPath volume below).
+ if (KubernetesConfig.mounterEnabled) {
+ containerBuilder
+ .addNewVolumeMount()
+ .withName("texera-mounts")
+ .withMountPath(inPodMountRoot)
+ .withMountPropagation("HostToContainer")
Review Comment:
The enabled branch adds the security- and scheduling-sensitive hostPath/mount-propagation contract, but `KubernetesClientSpec` only constructs pods with the default disabled configuration. Add a pod-capture test with mounting enabled that asserts the per-CUID host path, `HostToContainer` propagation, and both injected env vars; retain the disabled assertions to guard the PodSecurity-safe default.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(computing-unit): out-of-pod dataset mount infrastructure [texera]
Posted by "kunwp1 (via GitHub)" <gi...@apache.org>.
kunwp1 commented on code in PR #6866:
URL: https://github.com/apache/texera/pull/6866#discussion_r3780198054
##########
bin/mounter/mounter.py:
##########
Review Comment:
I still think we need authentication on `/mount` and `/mounts`. Attacker can use CU A to mount his dataset into CU B's directory and also use GET /mounts to enumerate all the repos/commits/paths. I also don't think it's a good idea to solve security issues in later PRs. Better to just include it here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(computing-unit): out-of-pod dataset mount infrastructure [texera]
Posted by "kunwp1 (via GitHub)" <gi...@apache.org>.
kunwp1 commented on code in PR #6866:
URL: https://github.com/apache/texera/pull/6866#discussion_r3780121455
##########
bin/k8s/templates/base/workflow-computing-unit-manager/workflow-computing-unit-manager-deployment.yaml:
##########
@@ -64,6 +64,8 @@ spec:
value: {{ .Values.workflowComputingUnitPool.namespace }}
- name: KUBERNETES_COMPUTE_UNIT_SERVICE_NAME
value: {{ .Values.workflowComputingUnitPool.name }}-svc
+ - name: KUBERNETES_COMPUTE_UNIT_POD_NAME_PREFIX
Review Comment:
The issue is different from `KUBERNETES_COMPUTE_UNIT_POOL_NAMESPACE`. In your comment, you mentioned that the Helm values must match the manager's `kubernetes.mounter-port` and `kubernetes.mounter-host-root`. But currently, Helm updates only the DaemonSet and the manager always falls back to `kubernetes.conf` defaults.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(computing-unit): out-of-pod dataset mount infrastructure [texera]
Posted by "aicam (via GitHub)" <gi...@apache.org>.
aicam commented on code in PR #6866:
URL: https://github.com/apache/texera/pull/6866#discussion_r3779409865
##########
bin/mounter/mounter.py:
##########
Review Comment:
Regarding path, computing unit name is "computing-unit-<cuid>" which has no `..` and version is hex digest which also don't have `..` so path will be always correct.
I agree, the problem is that mounter is a light weight microservice which only perform mounting. It is notable that the security leak is only in `unmount` endpoint, although mounting endpoint does not check token and CU access but it just mount it and user can not use it because S3 Servlet proxy in `file-service` check user access in every request, so we have two issues:
1- cuid=B can unmount a LakeFS repo from user A
2- cuid=B can mount a LakeFS repo from user A but can not access it
To address them, I changed the design slightly:
1- Removed unmount endpoint. It is not needed because mounter service receives termination signal from K8s so no need to have an endpoint. It is now removed in this PR.
2- We put `access-control-service` in between CU and mounter. The diagram is updated but this PR does not include that. Later in #6896 we will add new endpoint to access control service and also add network policy to Envoy Gateway to make sure only access control service can talk to mounter and we do access control in the service.
This way we keep mounter a simple microservice that only performs mounting.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(computing-unit): out-of-pod LakeFS repository mount infrastructure [texera]
Posted by "aicam (via GitHub)" <gi...@apache.org>.
aicam merged PR #6866:
URL: https://github.com/apache/texera/pull/6866
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(computing-unit): out-of-pod dataset mount infrastructure [texera]
Posted by "aicam (via GitHub)" <gi...@apache.org>.
aicam commented on code in PR #6866:
URL: https://github.com/apache/texera/pull/6866#discussion_r3897442460
##########
bin/mounter/mounter.py:
##########
@@ -0,0 +1,402 @@
+#!/usr/bin/env python3
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you 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.
+
+"""
+texera-mounter: a per-node privileged service that performs GeeseFS FUSE mounts on behalf
+of (unprivileged) computing-unit pods.
+
+A CU pod POSTs /mount with {cuid, repositoryName, commitHash, jwt, fileServiceBase}. The
+mounter runs GeeseFS against file-service's JWT-authenticated S3 proxy (passing the pod's
+JWT as the S3 access key) and mounts read-only under MOUNT_ROOT/<cuid>/<repo>/<commit>.
+That host directory is bind-mounted (mountPropagation: Bidirectional) into the mounter and
+propagates back into the CU pod (mountPropagation: HostToContainer), so the CU pod sees
+the mount without any privilege of its own.
+
+The mounter holds no LakeFS credentials; authorization stays entirely in file-service.
+A background watcher unmounts a CU's directories as soon as its pod is deleted.
+"""
+
+import json
+import os
+import shutil
+import ssl
+import subprocess
+import threading
+import time
+import urllib.request
+from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer
+from urllib.parse import parse_qs, urlparse
+
+MOUNT_ROOT = os.environ.get("MOUNT_ROOT", "/var/lib/texera-mounts")
+MOUNTER_PORT = int(os.environ.get("MOUNTER_PORT", "8100"))
+POOL_NAMESPACE = os.environ.get("POOL_NAMESPACE", "texera-workflow-computing-unit-pool")
+MOUNT_SECRET_PLACEHOLDER = "texera-jwt-mount"
+MOUNT_TIMEOUT_S = 30
+# How long the API server keeps a watch open. Each expiry re-runs the reconcile, which is
+# the safety net for events missed while disconnected and for orphans still busy earlier.
+WATCH_TIMEOUT_S = 300
+WATCH_RETRY_S = 10
+
+SA_DIR = "/var/run/secrets/kubernetes.io/serviceaccount"
+# Computing-unit pods are named "<prefix>-<cuid>"; the helm chart gives this and the
+# computing-unit manager's kubernetes.compute-unit-pod-name-prefix the same value.
+CU_POD_NAME_PREFIX = os.environ.get("CU_POD_NAME_PREFIX", "computing-unit")
+PROC_MOUNTS = "/proc/mounts"
+
+
+def log(msg):
+ print(f"[mounter] {msg}", flush=True)
+
+
+def _unescape(field):
+ """Decode the four characters /proc/mounts escapes octally."""
+ for code, char in (("\\040", " "), ("\\011", "\t"), ("\\012", "\n"), ("\\134", "\\")):
+ field = field.replace(code, char)
+ return field
+
+
+def mount_targets_under(path):
+ """Mount targets at or under `path`, deepest first.
+
+ Read from /proc/mounts rather than using os.path.ismount(): when a FUSE server dies
+ (the mounter being restarted kills every GeeseFS it started) its mount entry survives
+ here, but stat()ing the mount point fails with ENOTCONN, which os.path.ismount()
+ reports as "not a mount point". Such a dead mount would then never be unmounted, and
+ its directory could never be removed.
+ """
+ path = os.path.normpath(path)
+ prefix = path + "/"
+ targets = []
+ try:
+ with open(PROC_MOUNTS) as mounts:
+ for line in mounts:
+ fields = line.split()
+ if len(fields) < 2:
+ continue
+ target = _unescape(fields[1])
+ if target == path or target.startswith(prefix):
+ targets.append(target)
+ except OSError as e:
+ log(f"reading {PROC_MOUNTS} failed: {e}")
+ # Deepest first, so nested mounts are detached before their parents.
+ return sorted(targets, key=len, reverse=True)
+
+
+def is_mounted(path):
+ """True if `path` itself is a mount point, alive or dead."""
+ return os.path.normpath(path) in mount_targets_under(path)
+
+
+def _responds(path):
+ """True if `path` can be stat()ed — false for a mount whose FUSE server is gone."""
+ try:
+ os.stat(path)
+ return True
+ except OSError:
+ return False
+
+
+def _remove_empty_dirs(path, cuid):
+ """Remove `path` and any parents left empty, stopping at the CU's own directory."""
+ stop_at = os.path.normpath(os.path.join(MOUNT_ROOT, cuid))
+ path = os.path.normpath(path)
+ while path.startswith(stop_at):
+ try:
+ os.rmdir(path)
+ except OSError:
+ return # not empty (another commit is mounted here) or already gone
+ path = os.path.dirname(path)
+
+
+def ensure_shared_root():
+ """Make MOUNT_ROOT a shared mount so mounts created under it propagate to peers."""
+ os.makedirs(MOUNT_ROOT, exist_ok=True)
+ if not os.path.ismount(MOUNT_ROOT):
+ subprocess.run(["mount", "--bind", MOUNT_ROOT, MOUNT_ROOT], check=False)
+ subprocess.run(["mount", "--make-rshared", MOUNT_ROOT], check=False)
+
+
+def do_mount(cuid, repo, commit, jwt, file_service_base):
+ """Idempotently mount repo:commit for cuid. Returns the mount target path."""
+ if not cuid or not repo or not commit or not jwt or not file_service_base:
+ raise ValueError("cuid, repositoryName, commitHash, jwt and fileServiceBase are required")
+
+ target = os.path.join(MOUNT_ROOT, cuid, repo, commit)
+ if is_mounted(target):
Review Comment:
You are right, I did an experiment and surprisingly, GeeseFS recognize `..` as a valid path and can mount mistakenly. However, this validation should be done in `access-control-service` (ACS), as the diagram in PR description shows, ACS is the only service allowed to talk to mounter. We want to keep mounter a light-weight service to just do the mounting. In later PRs when mounter APIs are used, we will add this validation to ACS.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org