You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@texera.apache.org by gi...@apache.org on 2026/08/13 03:58:25 UTC
(texera) 01/01: test(computing-unit): extend ComputingUnitManagingResourceSpec to cover the create, rename and configuration endpoints (#7580)
This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-7580-dd7d813e92e9194e96d74b6926e48065462901ca
in repository https://gitbox.apache.org/repos/asf/texera.git
commit 374c8d883e59b8313ca41a58319d134310b1bd66
Author: Eugene Gu <eu...@outlook.com>
AuthorDate: Thu Aug 13 03:58:01 2026 +0000
test(computing-unit): extend ComputingUnitManagingResourceSpec to cover the create, rename and configuration endpoints (#7580)
### What changes were proposed in this PR?
This PR extends `ComputingUnitManagingResourceSpec` (added in #6853,
extended in #7337) to the endpoints of `ComputingUnitManagingResource`
that had no coverage. The existing spec only covered
`getComputingUnitInfo`, `getComputingUnitMetricsEndpoint`,
`listComputingUnits` and `terminateComputingUnit`; the create, rename
and configuration endpoints were untested.
All new tests keep the existing spec's approach: local-type units driven
against the embedded Postgres (`MockTexeraDB`), so no Kubernetes calls
are made. New coverage, by endpoint:
- **createWorkflowComputingUnit** — local happy path (persisted with a
generated `cuid`, the user URI landing in both the `uri` column and the
resource JSON's `nodeAddresses`, response reporting owner/WRITE/Running
with NaN metrics); whitespace-only name rejected with
`ForbiddenException` and nothing stored; unknown type (`quantum`)
rejected; `kubernetes` type rejected while disabled; missing and blank
URI rejected; the per-user running-unit quota not applying to local
units.
- **renameComputingUnit** — owner success; non-owner without access 403
(name kept); READ-only grantee 403; WRITE grantee success; blank name
400; nonexistent unit `NotFoundException`; database failure (name
overflowing the VARCHAR(128) column) rolling back and keeping the name;
and an admin who neither owns nor was granted access getting 403
(rename, unlike terminate, has no ADMIN bypass).
- **terminateComputingUnit** — a WRITE grantee rejected with 400 and the
unit not terminated (terminate requires strict ownership or the ADMIN
role).
- **getComputingUnitInfo** — a READ grantee sees the unit with `isOwner
= false` and `accessPrivilege = READ`; a nonexistent unit yields
`NotFoundException`; the owner response also reports the non-empty owner
avatar.
- **getComputingUnitTypes** — lists exactly `local` while Kubernetes is
disabled.
- **getComputingUnitLimitOptions** — returns the configured
cpu/memory/gpu option lists.
- **getComputingUnitResourceLimit** — the local branch returns NaN
limits for the owner; a non-owner gets `BadRequestException`; a
nonexistent unit yields `NotFoundException`.
- **getComputingUnitMetricsEndpoint** — adds the missing negative
direction: a non-owner gets `BadRequestException`.
Note: the Kubernetes-only validation in `createWorkflowComputingUnit`
sits behind the supported-type gate and is unreachable while
`kubernetes.enabled` is false; that flag is a load-time val the test JVM
does not override, so those branches cannot be exercised in this suite.
A spec comment records this.
No production code is changed.
### Any related issues, documentation, discussions?
Closes #7576
### How was this PR tested?
This PR is itself test-only. The new specs were run with:
```
sbt "ComputingUnitManagingService/testOnly org.apache.texera.service.resource.ComputingUnitManagingResourceSpec"
```
All 31 tests pass (7 pre-existing + 24 new) against the embedded
database; no external services are needed. The suite was
mutation-checked: targeted mutations of the resource (removing the
blank-name and missing-URI checks, inverting the rename ownership gate,
skipping the rename blank-name 400, making `getComputingUnitTypes` also
return `kubernetes`, and removing the non-owner check in
`getComputingUnitResourceLimit`) each caused at least one new test to
fail, and the source was restored afterwards.
`ComputingUnitManagingService/Test/scalafmtCheck` passes.
### Was this PR authored or co-authored using generative AI tooling?
Co-authored by: Claude Code (Claude Fable 5)
---
.../ComputingUnitManagingResourceSpec.scala | 303 ++++++++++++++++++++-
1 file changed, 299 insertions(+), 4 deletions(-)
diff --git a/computing-unit-managing-service/src/test/scala/org/apache/texera/service/resource/ComputingUnitManagingResourceSpec.scala b/computing-unit-managing-service/src/test/scala/org/apache/texera/service/resource/ComputingUnitManagingResourceSpec.scala
index e1142a6346..5b7c64104a 100644
--- a/computing-unit-managing-service/src/test/scala/org/apache/texera/service/resource/ComputingUnitManagingResourceSpec.scala
+++ b/computing-unit-managing-service/src/test/scala/org/apache/texera/service/resource/ComputingUnitManagingResourceSpec.scala
@@ -19,21 +19,36 @@
package org.apache.texera.service.resource
-import jakarta.ws.rs.NotFoundException
+import jakarta.ws.rs.{BadRequestException, ForbiddenException, NotFoundException}
import org.apache.texera.auth.SessionUser
+import org.apache.texera.common.config.KubernetesConfig.maxNumOfRunningComputingUnitsPerUser
import org.apache.texera.dao.MockTexeraDB
import org.apache.texera.dao.jooq.generated.enums.{
PrivilegeEnum,
UserRoleEnum,
WorkflowComputingUnitTypeEnum
}
-import org.apache.texera.dao.jooq.generated.tables.daos.{UserDao, WorkflowComputingUnitDao}
-import org.apache.texera.dao.jooq.generated.tables.pojos.{User, WorkflowComputingUnit}
-import org.apache.texera.service.resource.ComputingUnitManagingResource.WorkflowComputingUnitMetrics
+import org.apache.texera.dao.jooq.generated.tables.daos.{
+ ComputingUnitUserAccessDao,
+ UserDao,
+ WorkflowComputingUnitDao
+}
+import org.apache.texera.dao.jooq.generated.tables.pojos.{
+ ComputingUnitUserAccess,
+ User,
+ WorkflowComputingUnit
+}
+import org.apache.texera.service.resource.ComputingUnitManagingResource.{
+ WorkflowComputingUnitCreationParams,
+ WorkflowComputingUnitMetrics,
+ WorkflowComputingUnitResourceLimit
+}
import org.scalatest.BeforeAndAfterAll
import org.scalatest.flatspec.AnyFlatSpec
import org.scalatest.matchers.should.Matchers
+import scala.jdk.CollectionConverters.CollectionHasAsScala
+
// Drives the per-user computing-unit endpoints against the embedded database using
// local units (so no Kubernetes calls are made).
class ComputingUnitManagingResourceSpec
@@ -69,6 +84,10 @@ class ComputingUnitManagingResourceSpec
new SessionUser(makeUser(900, "admin", UserRoleEnum.ADMIN))
private lazy val strangerUser: SessionUser =
new SessionUser(makeUser(902, "stranger", UserRoleEnum.REGULAR))
+ // Dedicated owner for the create tests, so the units they create never show up in
+ // listComputingUnits(user)'s exact-set assertion.
+ private lazy val creatorUser: SessionUser =
+ new SessionUser(makeUser(903, "creator", UserRoleEnum.REGULAR))
private def localUnit(cuid: Int, name: String): WorkflowComputingUnit =
localUnitOwnedBy(cuid, uid, name)
@@ -91,6 +110,43 @@ class ComputingUnitManagingResourceSpec
.fetchOneByCuid(cuid)
.getTerminateTime != null
+ private def unitName(cuid: Int): String =
+ new WorkflowComputingUnitDao(getDSLContext.configuration())
+ .fetchOneByCuid(cuid)
+ .getName
+
+ private def runningUnitCountOwnedBy(ownerUid: Int): Int =
+ new WorkflowComputingUnitDao(getDSLContext.configuration())
+ .fetchByUid(ownerUid)
+ .asScala
+ .count(_.getTerminateTime == null)
+
+ /** Inserts an access row directly, bypassing the grant endpoint, to set up the rename tests. */
+ private def grantAccess(cuid: Int, granteeUid: Int, privilege: PrivilegeEnum): Unit = {
+ val access = new ComputingUnitUserAccess
+ access.setCuid(cuid)
+ access.setUid(granteeUid)
+ access.setPrivilege(privilege)
+ new ComputingUnitUserAccessDao(getDSLContext.configuration()).insert(access)
+ }
+
+ // The cpu/memory/gpu/jvm/shm quantities are ignored for local units, so the create tests only
+ // vary the fields the local branch actually reads: name, unitType and uri.
+ private def localCreationParams(
+ name: String,
+ uri: Option[String]
+ ): WorkflowComputingUnitCreationParams =
+ WorkflowComputingUnitCreationParams(
+ name = name,
+ unitType = "local",
+ cpuLimit = "NaN",
+ memoryLimit = "NaN",
+ gpuLimit = "NaN",
+ jvmMemorySize = "NaN",
+ shmSize = "NaN",
+ uri = uri
+ )
+
override protected def beforeAll(): Unit = {
super.beforeAll()
initializeDBAndReplaceDSLContext()
@@ -99,6 +155,7 @@ class ComputingUnitManagingResourceSpec
userDao.insert(adminUser.getUser)
userDao.insert(makeUser(901, "victim", UserRoleEnum.REGULAR))
userDao.insert(strangerUser.getUser)
+ userDao.insert(creatorUser.getUser)
val unitDao = new WorkflowComputingUnitDao(getDSLContext.configuration())
unitDao.insert(localUnit(800, "cu-a"))
unitDao.insert(localUnit(801, "cu-b"))
@@ -119,6 +176,23 @@ class ComputingUnitManagingResourceSpec
info.isOwner shouldBe true
info.accessPrivilege shouldBe PrivilegeEnum.WRITE
info.ownerName shouldBe "owner"
+ info.ownerGoogleAvatar shouldBe "owner-avatar"
+ }
+
+ it should "report READ access for a grantee holding READ access" in {
+ insertLocalUnit(cuid = 950, ownerUid = 901, name = "shared-info-cu")
+ grantAccess(950, granteeUid = 902, PrivilegeEnum.READ)
+
+ val info = resource.getComputingUnitInfo(950, strangerUser)
+
+ info.computingUnit.getCuid.intValue() shouldBe 950
+ info.isOwner shouldBe false
+ info.accessPrivilege shouldBe PrivilegeEnum.READ
+ info.ownerName shouldBe "victim"
+ }
+
+ it should "throw NotFoundException for a nonexistent unit" in {
+ a[NotFoundException] should be thrownBy resource.getComputingUnitInfo(99999, user)
}
"getComputingUnitMetricsEndpoint" should "return NaN metrics for an owned local unit" in {
@@ -126,6 +200,11 @@ class ComputingUnitManagingResourceSpec
WorkflowComputingUnitMetrics("NaN", "NaN")
}
+ it should "reject a non-owner with BadRequestException" in {
+ a[BadRequestException] should be thrownBy
+ resource.getComputingUnitMetricsEndpoint("800", strangerUser)
+ }
+
"listComputingUnits" should "return the caller's owned, non-terminated units" in {
val result = resource.listComputingUnits(user)
@@ -162,7 +241,223 @@ class ComputingUnitManagingResourceSpec
isTerminated(912) shouldBe true
}
+ it should "reject a WRITE grantee with 400 and not terminate the unit" in {
+ // Unlike rename, terminate requires strict ownership (or the ADMIN role), so
+ // even a WRITE grantee is rejected.
+ insertLocalUnit(cuid = 913, ownerUid = 901, name = "grantee-cannot-terminate")
+ grantAccess(913, granteeUid = 902, PrivilegeEnum.WRITE)
+
+ val response = resource.terminateComputingUnit(913, strangerUser)
+
+ response.getStatus shouldBe 400
+ isTerminated(913) shouldBe false
+ }
+
it should "return 404 when an admin terminates a nonexistent unit" in {
a[NotFoundException] should be thrownBy resource.terminateComputingUnit(99999, adminUser)
}
+
+ // The kubernetes type is disabled in the test JVM (kubernetes.conf's enabled flag defaults to
+ // false and is a load-time val), so createWorkflowComputingUnit rejects it at the supported-type
+ // gate. The kubernetes-only validation behind that gate (cpu/memory/gpu limit options, shm
+ // parsing and bounds, JVM-heap bound, and the running-unit quota throw) is unreachable here.
+
+ "createWorkflowComputingUnit" should "create a local unit and report it as owned, Running and writable" in {
+ val uri = "http://localhost:8085"
+
+ val created =
+ resource.createWorkflowComputingUnit(localCreationParams("local-cu", Some(uri)), creatorUser)
+
+ val cuid = created.computingUnit.getCuid
+ cuid.intValue() should be > 0
+ created.computingUnit.getUid.intValue() shouldBe 903
+ created.computingUnit.getName shouldBe "local-cu"
+ created.computingUnit.getType shouldBe WorkflowComputingUnitTypeEnum.local
+ created.computingUnit.getUri shouldBe uri
+ created.computingUnit.getResource should include(s""""nodeAddresses":["$uri"]""")
+ created.computingUnit.getResource should include(""""cpuLimit":"NaN"""")
+ created.status shouldBe "Running"
+ created.metrics shouldBe WorkflowComputingUnitMetrics("NaN", "NaN")
+ created.isOwner shouldBe true
+ created.accessPrivilege shouldBe PrivilegeEnum.WRITE
+ created.ownerName shouldBe "creator"
+ // makeUser sets no avatar, so the owner-avatar lookup resolves to null
+ created.ownerGoogleAvatar shouldBe null
+
+ // The unit is persisted, not just echoed back
+ unitName(cuid) shouldBe "local-cu"
+ }
+
+ it should "reject a whitespace-only name with ForbiddenException and store nothing" in {
+ val before = runningUnitCountOwnedBy(903)
+
+ val ex = the[ForbiddenException] thrownBy
+ resource.createWorkflowComputingUnit(
+ localCreationParams(" ", Some("http://localhost:8085")),
+ creatorUser
+ )
+
+ ex.getMessage should include("name cannot be empty")
+ runningUnitCountOwnedBy(903) shouldBe before
+ }
+
+ it should "reject an unknown unit type with ForbiddenException" in {
+ val params = localCreationParams("unknown-type-cu", Some("http://localhost:8085"))
+ .copy(unitType = "quantum")
+
+ val ex = the[ForbiddenException] thrownBy
+ resource.createWorkflowComputingUnit(params, creatorUser)
+
+ ex.getMessage should include("Unit type 'quantum' is not allowed")
+ }
+
+ it should "reject the kubernetes type while it is disabled in the configuration" in {
+ val params = localCreationParams("k8s-cu", None).copy(unitType = "kubernetes")
+
+ val ex = the[ForbiddenException] thrownBy
+ resource.createWorkflowComputingUnit(params, creatorUser)
+
+ ex.getMessage should include("Unit type 'kubernetes' is not allowed")
+ }
+
+ it should "reject a local unit without a URI" in {
+ val ex = the[ForbiddenException] thrownBy
+ resource.createWorkflowComputingUnit(localCreationParams("no-uri-cu", None), creatorUser)
+
+ ex.getMessage should include("URI is required")
+ }
+
+ it should "reject a local unit whose URI is blank" in {
+ val ex = the[ForbiddenException] thrownBy
+ resource.createWorkflowComputingUnit(
+ localCreationParams("blank-uri-cu", Some(" ")),
+ creatorUser
+ )
+
+ ex.getMessage should include("URI is required")
+ }
+
+ it should "not apply the kubernetes running-unit quota to local units" in {
+ // Fill the creator's quota with directly inserted local units (cuids in the 930s)
+ val base = runningUnitCountOwnedBy(903)
+ ((base + 1) to maxNumOfRunningComputingUnitsPerUser).foreach { i =>
+ insertLocalUnit(cuid = 930 + i, ownerUid = 903, name = s"quota-filler-$i")
+ }
+ runningUnitCountOwnedBy(903) should be >= maxNumOfRunningComputingUnitsPerUser
+
+ val created = resource.createWorkflowComputingUnit(
+ localCreationParams("over-quota-local", Some("http://localhost:8085")),
+ creatorUser
+ )
+
+ created.computingUnit.getName shouldBe "over-quota-local"
+ unitName(created.computingUnit.getCuid) shouldBe "over-quota-local"
+ }
+
+ // Fixtures for the rename tests live in the 920s and are owned by "victim" (901) or
+ // "stranger" (902), so they never show up in listComputingUnits(user)'s exact-set assertion.
+
+ "renameComputingUnit" should "let the owner rename its unit" in {
+ insertLocalUnit(cuid = 920, ownerUid = 902, name = "before-rename")
+
+ val response = resource.renameComputingUnit(920, "after-rename", strangerUser)
+
+ response.getStatus shouldBe 200
+ unitName(920) shouldBe "after-rename"
+ }
+
+ it should "reject a non-owner without granted access with 403 and keep the name" in {
+ insertLocalUnit(cuid = 921, ownerUid = 901, name = "victim-name")
+
+ val response = resource.renameComputingUnit(921, "hijacked", strangerUser)
+
+ response.getStatus shouldBe 403
+ unitName(921) shouldBe "victim-name"
+ }
+
+ it should "reject a grantee holding only READ access with 403 and keep the name" in {
+ insertLocalUnit(cuid = 922, ownerUid = 901, name = "read-only-name")
+ grantAccess(922, granteeUid = 902, PrivilegeEnum.READ)
+
+ val response = resource.renameComputingUnit(922, "hijacked", strangerUser)
+
+ response.getStatus shouldBe 403
+ unitName(922) shouldBe "read-only-name"
+ }
+
+ it should "let a grantee holding WRITE access rename the unit" in {
+ insertLocalUnit(cuid = 923, ownerUid = 901, name = "shared-name")
+ grantAccess(923, granteeUid = 902, PrivilegeEnum.WRITE)
+
+ val response = resource.renameComputingUnit(923, "renamed-by-grantee", strangerUser)
+
+ response.getStatus shouldBe 200
+ unitName(923) shouldBe "renamed-by-grantee"
+ }
+
+ it should "reject a blank name with 400 and keep the name" in {
+ insertLocalUnit(cuid = 924, ownerUid = 902, name = "keep-me")
+
+ val response = resource.renameComputingUnit(924, " ", strangerUser)
+
+ response.getStatus shouldBe 400
+ unitName(924) shouldBe "keep-me"
+ }
+
+ it should "reject an admin who neither owns nor was granted access with 403" in {
+ // Unlike terminate, rename has no ADMIN bypass: the role is never consulted.
+ insertLocalUnit(cuid = 951, ownerUid = 901, name = "admin-cannot-rename")
+
+ val response = resource.renameComputingUnit(951, "hijacked", adminUser)
+
+ response.getStatus shouldBe 403
+ unitName(951) shouldBe "admin-cannot-rename"
+ }
+
+ it should "return 404 for a nonexistent unit" in {
+ a[NotFoundException] should be thrownBy
+ resource.renameComputingUnit(99999, "new-name", strangerUser)
+ }
+
+ it should "propagate a database failure and keep the name" in {
+ insertLocalUnit(cuid = 925, ownerUid = 902, name = "short-name")
+ val oversized = "x" * 200 // the name column is VARCHAR(128)
+
+ // The endpoint's `return Response(500)` inside the withTransaction lambda is a Scala
+ // non-local return (a ControlThrowable, not a RuntimeException), so jOOQ's transaction
+ // wraps it in DataAccessException("Rollback caused") and the intended 500 response is
+ // never produced; this pins the actual behavior.
+ a[org.jooq.exception.DataAccessException] should be thrownBy
+ resource.renameComputingUnit(925, oversized, strangerUser)
+
+ unitName(925) shouldBe "short-name"
+ }
+
+ "getComputingUnitTypes" should "list exactly the local type while kubernetes is disabled" in {
+ resource.getComputingUnitTypes(user).typeOptions shouldBe List("local")
+ }
+
+ "getComputingUnitLimitOptions" should "return the configured cpu, memory and gpu options" in {
+ // The values are kubernetes.conf's defaults; the test JVM does not override them.
+ val options = resource.getComputingUnitLimitOptions(user)
+
+ options.cpuLimitOptions shouldBe List("1", "2", "4")
+ options.memoryLimitOptions shouldBe List("1Gi", "2Gi", "4Gi")
+ options.gpuLimitOptions shouldBe List("0", "1", "2")
+ }
+
+ "getComputingUnitResourceLimit" should "return NaN limits for an owned local unit" in {
+ resource.getComputingUnitResourceLimit("800", user) shouldBe
+ WorkflowComputingUnitResourceLimit("NaN", "NaN", "NaN")
+ }
+
+ it should "reject a non-owner with BadRequestException" in {
+ a[BadRequestException] should be thrownBy
+ resource.getComputingUnitResourceLimit("800", strangerUser)
+ }
+
+ it should "throw NotFoundException for a nonexistent unit" in {
+ a[NotFoundException] should be thrownBy
+ resource.getComputingUnitResourceLimit("99999", user)
+ }
}