You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airavata.apache.org by ya...@apache.org on 2026/10/03 21:56:36 UTC
(airavata-custos) 01/01: fix allocation membership roles and slurm association revocation
This is an automated email from the ASF dual-hosted git repository.
yasithdev pushed a commit to branch membership-role
in repository https://gitbox.apache.org/repos/asf/airavata-custos.git
commit 43b0bfeacdd00a31a6a7ebac92895855af750868
Author: Yasith Jayawardana <ya...@icloud.com>
AuthorDate: Sat Oct 3 16:44:03 2026 -0400
fix allocation membership roles and slurm association revocation
---
api/core.openapi.yaml | 23 ++++-
.../internal/operations/ensure_allocation_group.go | 2 +-
.../internal/subscribers/deprovision.go | 53 +++++++++-
.../internal/subscribers/deprovision_test.go | 107 +++++++++++++++------
.../internal/subscribers/members_test.go | 11 ++-
.../internal/subscribers/reconciler.go | 2 +-
.../SLURM/Rest-Client/pkg/client/associations.go | 25 +++++
.../Rest-Client/pkg/client/associations_test.go | 14 +++
internal/server/allocation_membership.go | 29 ++++--
.../allocation_membership_role_integration_test.go | 98 +++++++++++++++++++
pkg/service/compute_allocation_membership.go | 27 ++++++
pkg/service/project_membership.go | 18 ++++
web/src/generated/core/index.ts | 2 +-
web/src/generated/core/types.gen.ts | 17 +++-
web/src/generated/core/zod.gen.ts | 16 ++-
15 files changed, 394 insertions(+), 50 deletions(-)
diff --git a/api/core.openapi.yaml b/api/core.openapi.yaml
index c63146b1c..97906a582 100644
--- a/api/core.openapi.yaml
+++ b/api/core.openapi.yaml
@@ -868,6 +868,25 @@ definitions:
- UserTypeClusterLocal
- UserTypeVirtual
- UserTypeSystem
+ allocationMembershipRequest:
+ properties:
+ compute_allocation_id:
+ type: string
+ end_time:
+ type: string
+ id:
+ type: string
+ membership_status:
+ allOf:
+ - $ref: '#/definitions/AllocationStatus'
+ description: ACTIVE, INACTIVE, etc.
+ role:
+ type: string
+ start_time:
+ type: string
+ user_id:
+ type: string
+ type: object
attachResourceRequest:
properties:
compute_allocation_resource_id:
@@ -1655,7 +1674,7 @@ paths:
name: request
required: true
schema:
- $ref: '#/definitions/ComputeAllocationMembership'
+ $ref: '#/definitions/allocationMembershipRequest'
produces:
- application/json
responses:
@@ -1738,7 +1757,7 @@ paths:
name: request
required: true
schema:
- $ref: '#/definitions/ComputeAllocationMembership'
+ $ref: '#/definitions/allocationMembershipRequest'
produces:
- application/json
responses:
diff --git a/connectors/COmanage/Identity-Provisioner/internal/operations/ensure_allocation_group.go b/connectors/COmanage/Identity-Provisioner/internal/operations/ensure_allocation_group.go
index 8e30d9c63..2e9e320cb 100644
--- a/connectors/COmanage/Identity-Provisioner/internal/operations/ensure_allocation_group.go
+++ b/connectors/COmanage/Identity-Provisioner/internal/operations/ensure_allocation_group.go
@@ -75,7 +75,7 @@ func (o *Orchestrator) EnsureAllocationGroup(ctx context.Context, a *models.Comp
if m.MembershipStatus != models.ACTIVE || m.ProvisionedAt == nil {
continue
}
- _, composite, err := o.findCoPerson(ctx, &models.User{ID: m.UserID})
+ _, composite, err := o.findCoPerson(ctx, &models.User{ID: m.UserID, Email: m.Email})
if err != nil {
return err
}
diff --git a/connectors/SLURM/Association-Mapper/internal/subscribers/deprovision.go b/connectors/SLURM/Association-Mapper/internal/subscribers/deprovision.go
index 23ddaf124..61ec08b14 100644
--- a/connectors/SLURM/Association-Mapper/internal/subscribers/deprovision.go
+++ b/connectors/SLURM/Association-Mapper/internal/subscribers/deprovision.go
@@ -22,6 +22,7 @@ import (
"errors"
"fmt"
"log/slog"
+ "slices"
"github.com/apache/airavata-custos/connectors/SLURM/Rest-Client/pkg/client"
"github.com/apache/airavata-custos/pkg/models"
@@ -34,7 +35,7 @@ import (
//
// A member holds one association per partition on the allocation. The filter
// names the cluster, account, and user but leaves the partition empty, which
-// matches every partition, so one call removes the member's whole set.
+// matches every partition, so revoke removes the member's whole set.
func (a *AssociationSubscriber) removeAssociationsForMembership(ctx context.Context, membership models.ComputeAllocationMembership) error {
allocation, err := a.coreService.GetComputeAllocation(ctx, membership.ComputeAllocationID)
if err != nil {
@@ -54,7 +55,7 @@ func (a *AssociationSubscriber) removeAssociationsForMembership(ctx context.Cont
Account: allocation.Name,
User: csu.LocalUsername,
}
- if err := a.slurmClient.DeleteAssociation(filter); err != nil {
+ if err := a.revoke(filter); err != nil {
return fmt.Errorf("delete associations for %s on %s: %w", csu.LocalUsername, allocation.Name, err)
}
slog.Info("Removed associations for membership",
@@ -73,7 +74,7 @@ func (a *AssociationSubscriber) removeAssociationsForAllocation(ctx context.Cont
Cluster: cluster.Name,
Account: allocation.Name,
}
- if err := a.slurmClient.DeleteAssociation(filter); err != nil {
+ if err := a.revoke(filter); err != nil {
return fmt.Errorf("delete associations for allocation %s: %w", allocation.Name, err)
}
slog.Info("Removed associations for allocation",
@@ -81,6 +82,52 @@ func (a *AssociationSubscriber) removeAssociationsForAllocation(ctx context.Cont
return nil
}
+// revoke removes the member associations f matches, one call each, since
+// slurmrestd removes at most one per call. Slurm refuses to remove a user's
+// default association while they hold another on the cluster, so the default
+// first moves to another account they keep, or, when they keep only other
+// partitions of the same account, those go too and the reconciler adds them back.
+func (a *AssociationSubscriber) revoke(f client.AssocFilter) error {
+ matched, err := a.slurmClient.ListAssociations(f)
+ if err != nil {
+ return err
+ }
+ byUser := map[string][]client.Association{}
+ for _, m := range matched {
+ if m.User != "" {
+ byUser[m.User] = append(byUser[m.User], m)
+ }
+ }
+ for user, drop := range byUser {
+ held, err := a.slurmClient.ListAssociations(client.AssocFilter{Cluster: f.Cluster, User: user})
+ if err != nil {
+ return err
+ }
+ kept := slices.DeleteFunc(held, func(h client.Association) bool {
+ return slices.ContainsFunc(drop, func(d client.Association) bool { return d.Account == h.Account && d.Partition == h.Partition })
+ })
+ if i := slices.IndexFunc(drop, func(d client.Association) bool { return d.IsDefault != nil && *d.IsDefault }); i >= 0 {
+ def := drop[i]
+ drop = append(slices.Delete(drop, i, i+1), def)
+ if j := slices.IndexFunc(kept, func(k client.Association) bool { return k.Account != def.Account }); j >= 0 {
+ next, yes := kept[j], true
+ next.ID, next.IsDefault = 0, &yes
+ if err := a.slurmClient.UpsertAssociation(next); err != nil {
+ return fmt.Errorf("move default of %s to %s: %w", user, next.Account, err)
+ }
+ } else {
+ drop = append(kept, drop...)
+ }
+ }
+ for _, d := range drop {
+ if err := a.slurmClient.DeleteAssociation(client.AssocFilter{Cluster: d.Cluster, Account: d.Account, User: d.User, Partition: d.Partition}); err != nil {
+ return err
+ }
+ }
+ }
+ return nil
+}
+
// activeAllocation reports whether an allocation should still grant access.
func activeAllocation(allocation models.ComputeAllocation) bool {
return allocation.Status == models.ACTIVE
diff --git a/connectors/SLURM/Association-Mapper/internal/subscribers/deprovision_test.go b/connectors/SLURM/Association-Mapper/internal/subscribers/deprovision_test.go
index 6c6a2c54f..871ebec38 100644
--- a/connectors/SLURM/Association-Mapper/internal/subscribers/deprovision_test.go
+++ b/connectors/SLURM/Association-Mapper/internal/subscribers/deprovision_test.go
@@ -19,6 +19,7 @@ package subscribers
import (
"context"
+ "slices"
"testing"
"time"
@@ -32,22 +33,37 @@ func deactivatedMembership() models.ComputeAllocationMembership {
return m
}
+// held is the allocation's own association and its member's two partitions.
+func held() *fakeSlurmClient {
+ return &fakeSlurmClient{existing: []client.Association{
+ {Account: "test-alloc", Cluster: "testcluster"},
+ {Account: "test-alloc", Cluster: "testcluster", User: "testuser", Partition: "compute"},
+ {Account: "test-alloc", Cluster: "testcluster", User: "testuser", Partition: "gpu"},
+ }}
+}
+
+// removedPartitions checks every delete is one of testuser's test-alloc associations.
+func removedPartitions(t *testing.T, got []client.AssocFilter) []string {
+ t.Helper()
+ var partitions []string
+ for _, f := range got {
+ if f.User != "testuser" || f.Account != "test-alloc" || f.Cluster != "testcluster" {
+ t.Errorf("unexpected delete filter: %+v", f)
+ }
+ partitions = append(partitions, f.Partition)
+ }
+ return partitions
+}
+
func TestMembershipDeactivationRemovesAssociations(t *testing.T) {
core := coreMock(mockOpts{provisionedAt: ago(time.Minute)})
- slurm := &fakeSlurmClient{}
+ slurm := held()
NewAssociationSubscriber(slurm, nil, core, 0, 0).
SubscribeToComputeAllocationMembershipUpdate(context.Background(), deactivatedMembership())
- got := slurm.allDeletes()
- if len(got) != 1 {
- t.Fatalf("expected 1 delete, got %d", len(got))
- }
- // Deleting by user+account covers every partition in one call.
- if got[0].User != "testuser" || got[0].Account != "test-alloc" || got[0].Cluster != "testcluster" {
- t.Errorf("unexpected delete filter: %+v", got[0])
- }
- if got[0].Partition != "" {
- t.Errorf("filter should not pin a partition, got %q", got[0].Partition)
+ // slurmrestd removes one association per call, so each partition goes on its own.
+ if got := removedPartitions(t, slurm.allDeletes()); !slices.Equal(got, []string{"compute", "gpu"}) {
+ t.Fatalf("expected both partitions removed, got %v", got)
}
if n := len(slurm.all()); n != 0 {
t.Errorf("a deactivation must not write associations, got %d", n)
@@ -71,32 +87,27 @@ func TestMembershipReactivationRestoresAssociations(t *testing.T) {
func TestMembershipDeletionRemovesAssociations(t *testing.T) {
core := coreMock(mockOpts{provisionedAt: ago(time.Minute)})
- slurm := &fakeSlurmClient{}
+ slurm := held()
NewAssociationSubscriber(slurm, nil, core, 0, 0).
SubscribeToComputeAllocationMembershipDeletion(context.Background(), testMembership())
- got := slurm.allDeletes()
- if len(got) != 1 || got[0].User != "testuser" || got[0].Account != "test-alloc" {
- t.Fatalf("expected the member's associations to be removed, got %+v", got)
+ if got := removedPartitions(t, slurm.allDeletes()); len(got) != 2 {
+ t.Fatalf("expected the member's associations to be removed, got %v", got)
}
}
func TestAllocationDeactivationRemovesAllAssociations(t *testing.T) {
core := coreMock(mockOpts{})
- slurm := &fakeSlurmClient{}
+ slurm := held()
alloc := models.ComputeAllocation{
ID: "alloc-1", Name: "test-alloc", ComputeClusterID: "cluster-1", Status: models.INACTIVE,
}
NewAssociationSubscriber(slurm, nil, core, 0, 0).
SubscribeToComputeAllocationUpdate(context.Background(), alloc)
- got := slurm.allDeletes()
- if len(got) != 1 {
- t.Fatalf("expected 1 delete, got %d", len(got))
- }
- // No user pinned: every member of the allocation loses access.
- if got[0].Account != "test-alloc" || got[0].User != "" {
- t.Errorf("expected an account-wide delete, got %+v", got[0])
+ // Every member loses access; the allocation's own association stays.
+ if got := removedPartitions(t, slurm.allDeletes()); len(got) != 2 {
+ t.Fatalf("expected every member association removed, got %v", got)
}
}
@@ -142,16 +153,15 @@ func TestAllocationReactivationSkipsUnprovisionedMembers(t *testing.T) {
func TestAllocationDeletionRemovesAllAssociations(t *testing.T) {
core := coreMock(mockOpts{})
- slurm := &fakeSlurmClient{}
+ slurm := held()
alloc := models.ComputeAllocation{
ID: "alloc-1", Name: "test-alloc", ComputeClusterID: "cluster-1", Status: models.ACTIVE,
}
NewAssociationSubscriber(slurm, nil, core, 0, 0).
SubscribeToComputeAllocationDeletion(context.Background(), alloc)
- got := slurm.allDeletes()
- if len(got) != 1 || got[0].Account != "test-alloc" || got[0].User != "" {
- t.Fatalf("expected an account-wide delete on allocation deletion, got %+v", got)
+ if got := removedPartitions(t, slurm.allDeletes()); len(got) != 2 {
+ t.Fatalf("expected every member association removed on allocation deletion, got %v", got)
}
}
@@ -179,6 +189,49 @@ func TestReconcilerRemovesStaleAssociation(t *testing.T) {
}
}
+func TestRevokeMovesTheDefaultFirst(t *testing.T) {
+ yes := true
+ assoc := func(account, partition string, def bool) client.Association {
+ a := client.Association{Cluster: "c", Account: account, User: "u", Partition: partition}
+ if def {
+ a.IsDefault = &yes
+ }
+ return a
+ }
+ tests := []struct {
+ name string
+ held []client.Association
+ revoke client.AssocFilter
+ upsert, delete []string
+ }{
+ {name: "default moves to another account", held: []client.Association{assoc("a", "cpu", true), assoc("b", "cpu", false)},
+ revoke: client.AssocFilter{Account: "a"}, upsert: []string{"b/cpu default"}, delete: []string{"a/cpu"}},
+ {name: "default goes last", held: []client.Association{assoc("a", "cpu", true), assoc("a", "gpu", false)},
+ revoke: client.AssocFilter{Account: "a"}, delete: []string{"a/gpu", "a/cpu"}},
+ {name: "same-account partitions go along", held: []client.Association{assoc("a", "cpu", true), assoc("a", "gpu", false)},
+ revoke: client.AssocFilter{Account: "a", Partition: "cpu"}, delete: []string{"a/gpu", "a/cpu"}},
+ }
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ slurm := &fakeSlurmClient{existing: tt.held}
+ tt.revoke.Cluster, tt.revoke.User = "c", "u"
+ if err := NewAssociationSubscriber(slurm, nil, nil, 0, 0).revoke(tt.revoke); err != nil {
+ t.Fatal(err)
+ }
+ var upserts, deletes []string
+ for _, a := range slurm.upserts {
+ upserts = append(upserts, a.Account+"/"+a.Partition+map[bool]string{true: " default"}[a.IsDefault != nil])
+ }
+ for _, f := range slurm.allDeletes() {
+ deletes = append(deletes, f.Account+"/"+f.Partition)
+ }
+ if !slices.Equal(upserts, tt.upsert) || !slices.Equal(deletes, tt.delete) {
+ t.Fatalf("upserts %v, deletes %v; want %v, %v", upserts, deletes, tt.upsert, tt.delete)
+ }
+ })
+ }
+}
+
// Guard: an empty desired set almost always means a failed lookup, so the
// sweep must not read it as "revoke everyone".
func TestReconcilerDoesNotPruneWhenNothingIsDesired(t *testing.T) {
diff --git a/connectors/SLURM/Association-Mapper/internal/subscribers/members_test.go b/connectors/SLURM/Association-Mapper/internal/subscribers/members_test.go
index d3be7f56d..9d21a1991 100644
--- a/connectors/SLURM/Association-Mapper/internal/subscribers/members_test.go
+++ b/connectors/SLURM/Association-Mapper/internal/subscribers/members_test.go
@@ -50,8 +50,15 @@ func (f *fakeSlurmClient) allDeletes() []client.AssocFilter {
return append([]client.AssocFilter(nil), f.deletes...)
}
-func (f *fakeSlurmClient) ListAssociations(client.AssocFilter) ([]client.Association, error) {
- return f.existing, nil
+func (f *fakeSlurmClient) ListAssociations(q client.AssocFilter) ([]client.Association, error) {
+ var out []client.Association
+ for _, a := range f.existing {
+ if (q.Account == "" || q.Account == a.Account) && (q.User == "" || q.User == a.User) &&
+ (q.Cluster == "" || q.Cluster == a.Cluster) && (q.Partition == "" || q.Partition == a.Partition) {
+ out = append(out, a)
+ }
+ }
+ return out, nil
}
func (f *fakeSlurmClient) CreateAccount(client.Account, string) error { return nil }
diff --git a/connectors/SLURM/Association-Mapper/internal/subscribers/reconciler.go b/connectors/SLURM/Association-Mapper/internal/subscribers/reconciler.go
index c761a4bac..e5e955c95 100644
--- a/connectors/SLURM/Association-Mapper/internal/subscribers/reconciler.go
+++ b/connectors/SLURM/Association-Mapper/internal/subscribers/reconciler.go
@@ -196,7 +196,7 @@ func (a *AssociationSubscriber) pruneStaleAssociations(ctx context.Context, clus
User: assoc.User,
Partition: assoc.Partition,
}
- if err := a.slurmClient.DeleteAssociation(filter); err != nil {
+ if err := a.revoke(filter); err != nil {
slog.Error("Association reconciler: failed to remove stale association",
"account", assoc.Account, "user", assoc.User, "partition", assoc.Partition, "error", err)
continue
diff --git a/connectors/SLURM/Rest-Client/pkg/client/associations.go b/connectors/SLURM/Rest-Client/pkg/client/associations.go
index b1eca11be..254eaf579 100644
--- a/connectors/SLURM/Rest-Client/pkg/client/associations.go
+++ b/connectors/SLURM/Rest-Client/pkg/client/associations.go
@@ -20,6 +20,7 @@ package client
import (
"errors"
+ "fmt"
"log"
"log/slog"
"net/url"
@@ -76,6 +77,11 @@ func (c *Client) UpsertAssociation(a Association) error {
log.Printf("Failed to upsert association: %v", err)
return err
}
+ if a.User != "" {
+ if err := c.ensureUser(a.User, a.Account); err != nil {
+ return fmt.Errorf("ensure user %s: %w", a.User, err)
+ }
+ }
filter := AssocFilter{
Account: a.Account,
@@ -137,6 +143,25 @@ func tresSliceEqualUnordered(a, b []TRES) bool {
return true
}
+// ensureUser creates a missing user record defaulting to account, which writing
+// an association does not, and without which Slurm cannot later remove it.
+func (c *Client) ensureUser(name, account string) error {
+ var out struct {
+ Users []struct {
+ Name string `json:"name"`
+ } `json:"users"`
+ }
+ if _, err := c.do("GET", "/slurmdb/v0.0."+c.apiVersion+"/user/"+url.PathEscape(name), nil, &out); err != nil {
+ return err
+ }
+ if len(out.Users) > 0 {
+ return nil
+ }
+ body := map[string]any{"users": []map[string]any{{"name": name, "default": map[string]any{"account": account}}}}
+ _, err := c.do("POST", "/slurmdb/v0.0."+c.apiVersion+"/users", body, nil)
+ return err
+}
+
func (c *Client) DeleteAssociation(f AssocFilter) error {
path := "/slurmdb/v0.0." + c.apiVersion + "/association"
if q := f.query(); q != "" {
diff --git a/connectors/SLURM/Rest-Client/pkg/client/associations_test.go b/connectors/SLURM/Rest-Client/pkg/client/associations_test.go
index bd9bd3b49..9bc57302d 100644
--- a/connectors/SLURM/Rest-Client/pkg/client/associations_test.go
+++ b/connectors/SLURM/Rest-Client/pkg/client/associations_test.go
@@ -49,6 +49,7 @@ func TestListAssociationsByAccount(t *testing.T) {
}
func TestCreateAssociation(t *testing.T) {
+ var createdUser string
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
// Support to list associations so we can verify the association was created successfully
@@ -63,6 +64,16 @@ func TestCreateAssociation(t *testing.T) {
return
}
+ if r.Method == "GET" && r.URL.Path == "/slurmdb/v0.0.41/user/alice" {
+ _, _ = w.Write([]byte(`{"users":[]}`))
+ return
+ }
+ if r.Method == "POST" && r.URL.Path == "/slurmdb/v0.0.41/users" {
+ b, _ := io.ReadAll(r.Body)
+ createdUser = string(b)
+ _, _ = w.Write([]byte(`{}`))
+ return
+ }
if r.Method != "POST" || r.URL.Path != "/slurmdb/v0.0.41/associations" {
t.Fatalf("unexpected %s %s", r.Method, r.URL.Path)
}
@@ -83,6 +94,9 @@ func TestCreateAssociation(t *testing.T) {
if err != nil {
t.Fatal(err)
}
+ if createdUser != `{"users":[{"default":{"account":"eng"},"name":"alice"}]}` {
+ t.Errorf("a user with no record must be created defaulting to the account, got %s", createdUser)
+ }
}
func TestDeleteAssociation(t *testing.T) {
diff --git a/internal/server/allocation_membership.go b/internal/server/allocation_membership.go
index 994dd4bf4..07e62b9d9 100644
--- a/internal/server/allocation_membership.go
+++ b/internal/server/allocation_membership.go
@@ -24,22 +24,30 @@ import (
"github.com/apache/airavata-custos/pkg/models"
)
+type allocationMembershipRequest struct {
+ models.ComputeAllocationMembership
+ Role string `json:"role,omitempty"`
+}
+
// @Summary Create a compute allocation membership
// @Tags Compute Allocation Memberships
// @Security BearerAuth
// @Accept json
// @Produce json
-// @Param request body models.ComputeAllocationMembership true "Membership payload"
+// @Param request body allocationMembershipRequest true "Membership payload"
// @Success 201 {object} models.ComputeAllocationMembership
// @Failure 400 {object} object{error=string}
// @Router /compute-allocation-memberships [post]
func (s *Server) createComputeAllocationMembership(w http.ResponseWriter, r *http.Request) {
- var m models.ComputeAllocationMembership
- if err := common.DecodeJSON(r, &m); err != nil {
+ var req allocationMembershipRequest
+ if err := common.DecodeJSON(r, &req); err != nil {
common.WriteError(w, http.StatusBadRequest, err)
return
}
- created, err := s.svc.CreateComputeAllocationMembership(r.Context(), &m)
+ created, err := s.svc.CreateComputeAllocationMembership(r.Context(), &req.ComputeAllocationMembership)
+ if err == nil && req.Role != "" {
+ err = s.svc.SetAllocationMemberRole(r.Context(), created.ComputeAllocationID, created.UserID, req.Role)
+ }
if err != nil {
common.WriteServiceError(w, err)
return
@@ -70,19 +78,22 @@ func (s *Server) getComputeAllocationMembership(w http.ResponseWriter, r *http.R
// @Accept json
// @Produce json
// @Param id path string true "Membership ID"
-// @Param request body models.ComputeAllocationMembership true "Membership payload"
+// @Param request body allocationMembershipRequest true "Membership payload"
// @Success 200 {object} models.ComputeAllocationMembership
// @Failure 400 {object} object{error=string}
// @Failure 404 {object} object{error=string}
// @Router /compute-allocation-memberships/{id} [put]
func (s *Server) updateComputeAllocationMembership(w http.ResponseWriter, r *http.Request) {
- var m models.ComputeAllocationMembership
- if err := common.DecodeJSON(r, &m); err != nil {
+ var req allocationMembershipRequest
+ if err := common.DecodeJSON(r, &req); err != nil {
common.WriteError(w, http.StatusBadRequest, err)
return
}
- m.ID = r.PathValue("id")
- updated, err := s.svc.UpdateComputeAllocationMembership(r.Context(), &m)
+ req.ID = r.PathValue("id")
+ updated, err := s.svc.UpdateComputeAllocationMembership(r.Context(), &req.ComputeAllocationMembership)
+ if err == nil && req.Role != "" {
+ err = s.svc.SetAllocationMemberRole(r.Context(), updated.ComputeAllocationID, updated.UserID, req.Role)
+ }
if err != nil {
common.WriteServiceError(w, err)
return
diff --git a/internal/server/allocation_membership_role_integration_test.go b/internal/server/allocation_membership_role_integration_test.go
new file mode 100644
index 000000000..7db064d00
--- /dev/null
+++ b/internal/server/allocation_membership_role_integration_test.go
@@ -0,0 +1,98 @@
+//go:build integration
+
+// 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 server
+
+import (
+ "bytes"
+ "encoding/json"
+ "fmt"
+ "net/http"
+ "net/http/httptest"
+ "testing"
+ "time"
+
+ "github.com/apache/airavata-custos/pkg/models"
+)
+
+func TestAllocationMembershipRole_SetsProjectRoleAndKeepsUpstreamRoles(t *testing.T) {
+ database, svc, srv := setupTestStack(t)
+ fx := seedScopedFixtures(t, database, svc)
+ userID := seedUser(t, database, fmt.Sprintf("role.member+%d@example.edu", time.Now().UnixNano()))
+
+ send := func(method, path string, body any) *httptest.ResponseRecorder {
+ b, _ := json.Marshal(body)
+ rr := httptest.NewRecorder()
+ req := withTestCaller(httptest.NewRequest(method, path, bytes.NewReader(b)), fx.piID, models.AllocationsWrite, models.AllocationsRead)
+ srv.ServeHTTP(rr, req)
+ return rr
+ }
+ roleOf := func() string {
+ var rows []AllocationMembershipResponse
+ _ = json.NewDecoder(doGet(t, srv, "/compute-allocations/"+fx.allocA.ID+"/memberships", fx.piID, models.AllocationsRead).Body).Decode(&rows)
+ for _, r := range rows {
+ if r.UserID == userID {
+ return r.Role
+ }
+ }
+ return ""
+ }
+
+ now := time.Now().UTC()
+ rr := send(http.MethodPost, "/compute-allocation-memberships", map[string]any{
+ "compute_allocation_id": fx.allocA.ID, "user_id": userID,
+ "start_time": now, "end_time": now.AddDate(0, 1, 0), "role": "ALLOCATION_MANAGER",
+ })
+ if rr.Code != http.StatusCreated {
+ t.Fatalf("create: got %d (%s)", rr.Code, rr.Body.String())
+ }
+ var created models.ComputeAllocationMembership
+ _ = json.NewDecoder(rr.Body).Decode(&created)
+ if got := roleOf(); got != "ALLOCATION_MANAGER" {
+ t.Fatalf("role after create: got %q", got)
+ }
+
+ if rr := send(http.MethodPut, "/compute-allocation-memberships/"+created.ID, map[string]any{"role": "MEMBER"}); rr.Code != http.StatusOK {
+ t.Fatalf("update: got %d (%s)", rr.Code, rr.Body.String())
+ }
+ if got := roleOf(); got != "MEMBER" {
+ t.Fatalf("role after update: got %q", got)
+ }
+
+ if rr := send(http.MethodPut, "/compute-allocation-memberships/"+created.ID, map[string]any{"role": "PI"}); rr.Code != http.StatusBadRequest {
+ t.Fatalf("PI must not be assignable: got %d (%s)", rr.Code, rr.Body.String())
+ }
+
+ if err := svc.EnsureProjectMembership(t.Context(), fx.allocA.ProjectID, userID, "CO_PI"); err != nil {
+ t.Fatal(err)
+ }
+ if rr := send(http.MethodPut, "/compute-allocation-memberships/"+created.ID, map[string]any{"role": "MEMBER"}); rr.Code != http.StatusOK || roleOf() != "CO_PI" {
+ t.Fatalf("a CO_PI must keep the role: got %d, role %q", rr.Code, roleOf())
+ }
+
+ if err := svc.EnsureProjectMembership(t.Context(), fx.allocA.ProjectID, userID, "ALLOCATION_MANAGER"); err != nil {
+ t.Fatal(err)
+ }
+ if rr := send(http.MethodDelete, "/compute-allocation-memberships/"+created.ID, nil); rr.Code != http.StatusNoContent {
+ t.Fatalf("delete: got %d (%s)", rr.Code, rr.Body.String())
+ }
+ if role, err := svc.ProjectRoleForUser(t.Context(), fx.allocA.ProjectID, userID); err != nil || role != "" {
+ t.Fatalf("a manager with no membership left must lose the role: got %q (%v)", role, err)
+ }
+}
diff --git a/pkg/service/compute_allocation_membership.go b/pkg/service/compute_allocation_membership.go
index 33cf4d274..a617c7b41 100644
--- a/pkg/service/compute_allocation_membership.go
+++ b/pkg/service/compute_allocation_membership.go
@@ -21,6 +21,7 @@ import (
"context"
"database/sql"
"fmt"
+ "slices"
"github.com/apache/airavata-custos/internal/store"
"github.com/apache/airavata-custos/pkg/events"
@@ -222,13 +223,39 @@ func (s *Service) DeleteComputeAllocationMembership(ctx context.Context, id stri
if existing == nil {
return ErrNotFound
}
+ projectID, drop, err := s.orphanedManagerRole(ctx, existing)
+ if err != nil {
+ return err
+ }
if err := s.inTx(ctx, func(tx *sql.Tx) error {
if err := s.memberships.Delete(ctx, tx, id); err != nil {
return err
}
+ if drop {
+ if err := s.projMemberships.Delete(ctx, tx, projectID, existing.UserID); err != nil {
+ return err
+ }
+ }
return s.eventBus.Publish(ctx, tx, events.ComputeAllocationMembershipDeleteEvent, existing)
}); err != nil {
return fmt.Errorf("delete compute allocation membership: %w", err)
}
return nil
}
+
+// orphanedManagerRole reports whether deleting m leaves its holder an
+// ALLOCATION_MANAGER, a project-wide role, with no membership in the project.
+func (s *Service) orphanedManagerRole(ctx context.Context, m *models.ComputeAllocationMembership) (string, bool, error) {
+ alloc, err := s.GetComputeAllocation(ctx, m.ComputeAllocationID)
+ if err != nil {
+ return "", false, err
+ }
+ role, err := s.ProjectRoleForUser(ctx, alloc.ProjectID, m.UserID)
+ if err != nil || role != models.ProjectRoleAllocationManager {
+ return "", false, err
+ }
+ rest, err := s.ListMembersForProject(ctx, alloc.ProjectID)
+ return alloc.ProjectID, err == nil && !slices.ContainsFunc(rest, func(r store.MembershipWithUser) bool {
+ return r.UserID == m.UserID && r.ID != m.ID
+ }), err
+}
diff --git a/pkg/service/project_membership.go b/pkg/service/project_membership.go
index b7abf593c..e983c3ccf 100644
--- a/pkg/service/project_membership.go
+++ b/pkg/service/project_membership.go
@@ -102,6 +102,24 @@ func (s *Service) EnsureProjectMembership(ctx context.Context, projectID, userID
})
}
+// SetAllocationMemberRole sets a member's role on the allocation's project,
+// where roles live. PI and CO_PI arrive from upstream: they are not assignable,
+// and a member holding one keeps it.
+func (s *Service) SetAllocationMemberRole(ctx context.Context, allocationID, userID, role string) error {
+ if role != "MEMBER" && role != string(models.ProjectRoleAllocationManager) {
+ return fmt.Errorf("%w: role %q is not assignable", ErrInvalidInput, role)
+ }
+ alloc, err := s.GetComputeAllocation(ctx, allocationID)
+ if err != nil {
+ return err
+ }
+ current, err := s.ProjectRoleForUser(ctx, alloc.ProjectID, userID)
+ if err != nil || current == models.ProjectRolePI || current == models.ProjectRoleCoPI {
+ return err
+ }
+ return s.EnsureProjectMembership(ctx, alloc.ProjectID, userID, role)
+}
+
// ProjectRoleForUser returns the user's governance role on a project, or an
// empty role when they hold none.
func (s *Service) ProjectRoleForUser(ctx context.Context, projectID, userID string) (models.ProjectRole, error) {
diff --git a/web/src/generated/core/index.ts b/web/src/generated/core/index.ts
index ed2a49a39..675a22b17 100644
--- a/web/src/generated/core/index.ts
+++ b/web/src/generated/core/index.ts
@@ -1,4 +1,4 @@
// This file is auto-generated by @hey-api/openapi-ts
export { deleteComputeAllocationChangeRequestEventsById, deleteComputeAllocationChangeRequestsById, deleteComputeAllocationDiffsById, deleteComputeAllocationMembershipResourceOverridesById, deleteComputeAllocationMembershipsById, deleteComputeAllocationsByIdResourcesByResourceId, deleteComputeAllocationUsagesById, deleteComputeClusterUsersById, deleteRolesById, deleteRolesByIdPrivilegesByKey, deleteUserIdentitiesById, deleteUsersByIdPrivilegesByKey, deleteUsersByIdRolesByRoleId, getAudit [...]
-export type { AllocationMembershipResponse, AllocationStatus, AllocationSuTotalResponse, AttachResourceRequest, AuditEvent, CallerProfileResponse, CallerRoleGrant, ClientOptions, ClusterAccessLevel, ClusterAccountApproval, ClusterAccountListResponse, ClusterAccountResponse, ComputeAllocation, ComputeAllocationChangeRequest, ComputeAllocationChangeRequestEvent, ComputeAllocationDiff, ComputeAllocationListResponse, ComputeAllocationMembership, ComputeAllocationMembershipResourceOverride, C [...]
+export type { AllocationMembershipRequest, AllocationMembershipResponse, AllocationStatus, AllocationSuTotalResponse, AttachResourceRequest, AuditEvent, CallerProfileResponse, CallerRoleGrant, ClientOptions, ClusterAccessLevel, ClusterAccountApproval, ClusterAccountListResponse, ClusterAccountResponse, ComputeAllocation, ComputeAllocationChangeRequest, ComputeAllocationChangeRequestEvent, ComputeAllocationDiff, ComputeAllocationListResponse, ComputeAllocationMembership, ComputeAllocation [...]
diff --git a/web/src/generated/core/types.gen.ts b/web/src/generated/core/types.gen.ts
index a8694fc3a..0152a4deb 100644
--- a/web/src/generated/core/types.gen.ts
+++ b/web/src/generated/core/types.gen.ts
@@ -601,6 +601,19 @@ export type UserStatus = 'PENDING' | 'ACTIVE' | 'INACTIVE' | 'SUSPENDED' | 'MERG
export type UserType = 'CLUSTER_LOCAL' | 'VIRTUAL' | 'SYSTEM';
+export type AllocationMembershipRequest = {
+ compute_allocation_id?: string;
+ end_time?: string;
+ id?: string;
+ /**
+ * ACTIVE, INACTIVE, etc.
+ */
+ membership_status?: AllocationStatus;
+ role?: string;
+ start_time?: string;
+ user_id?: string;
+};
+
export type AttachResourceRequest = {
compute_allocation_resource_id?: string;
resource_amount?: number;
@@ -1389,7 +1402,7 @@ export type PostComputeAllocationMembershipsData = {
/**
* Membership payload
*/
- body: ComputeAllocationMembership;
+ body: AllocationMembershipRequest;
path?: never;
query?: never;
url: '/compute-allocation-memberships';
@@ -1481,7 +1494,7 @@ export type PutComputeAllocationMembershipsByIdData = {
/**
* Membership payload
*/
- body: ComputeAllocationMembership;
+ body: AllocationMembershipRequest;
path: {
/**
* Membership ID
diff --git a/web/src/generated/core/zod.gen.ts b/web/src/generated/core/zod.gen.ts
index d4838221e..ac2f6a938 100644
--- a/web/src/generated/core/zod.gen.ts
+++ b/web/src/generated/core/zod.gen.ts
@@ -543,6 +543,18 @@ export const zUserListResponse = z.object({
export type userListResponseZodType = z.infer<typeof zUserListResponse>;
+export const zAllocationMembershipRequest = z.object({
+ compute_allocation_id: z.string().optional(),
+ end_time: z.string().optional(),
+ id: z.string().optional(),
+ membership_status: zAllocationStatus.optional(),
+ role: z.string().optional(),
+ start_time: z.string().optional(),
+ user_id: z.string().optional()
+});
+
+export type allocationMembershipRequestZodType = z.infer<typeof zAllocationMembershipRequest>;
+
export const zAttachResourceRequest = z.object({
compute_allocation_resource_id: z.string().optional(),
resource_amount: z.int().optional(),
@@ -840,7 +852,7 @@ export const zPutComputeAllocationMembershipResourceOverridesByIdResponse = zCom
/**
* Membership payload
*/
-export const zPostComputeAllocationMembershipsBody = zComputeAllocationMembership;
+export const zPostComputeAllocationMembershipsBody = zAllocationMembershipRequest;
/**
* Created
@@ -863,7 +875,7 @@ export const zGetComputeAllocationMembershipsByIdResponse = zComputeAllocationMe
/**
* Membership payload
*/
-export const zPutComputeAllocationMembershipsByIdBody = zComputeAllocationMembership;
+export const zPutComputeAllocationMembershipsByIdBody = zAllocationMembershipRequest;
export const zPutComputeAllocationMembershipsByIdPath = z.object({
id: z.string()