Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions agentteams-controller/api/v1beta1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,10 @@ const LabelWorkerEdgeUUID = "agentteams.io/worker-edge-uuid"
// re-issues credentials and updates the annotation to match.
const AnnotationEdgeAppliedUUID = "agentteams.io/edge-applied-uuid"

// AnnotationHumanManagedGroupAllowExtra records Human Matrix IDs added to a
// Worker's groupAllowExtra by the Human controller.
const AnnotationHumanManagedGroupAllowExtra = "agentteams.io/human-group-allow-extra"

// AccessEntry declares one cloud-permission grant under a logical
// service. v1 supported services: "object-storage", "ai-gateway", "ai-registry", "schedulerx3".
//
Expand Down
3 changes: 3 additions & 0 deletions agentteams-controller/internal/controller/human_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,9 @@ func (r *HumanReconciler) reconcileHumanNormal(ctx context.Context, s *humanScop
return reconcile.Result{RequeueAfter: reconcileInterval}, err
}
r.reconcileHumanRooms(ctx, s)
if err := r.reconcileHumanWorkerAllowlists(ctx, s); err != nil {
return reconcile.Result{RequeueAfter: reconcileInterval}, err
}

return reconcile.Result{RequeueAfter: reconcileInterval}, nil
}
Expand Down
164 changes: 164 additions & 0 deletions agentteams-controller/internal/controller/human_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,170 @@ func newReadyTeam(name, roomID string) *v1beta1.Team {
return tm
}

func TestHumanReconciler_SyncsAccessibleWorkerAllowlist(t *testing.T) {
worker := newReadyWorker("w1", "!room-w1:localhost")
human := newHuman("alice", v1beta1.HumanSpec{
AccessibleWorkers: []string{"w1"},
})
human.Status.MatrixUserID = "@alice:localhost"
human.Status.InitialPassword = "stored-pw"
human.Status.Rooms = []string{"!room-w1:localhost"}
human.Status.Phase = "Active"
human.Finalizers = []string{finalizerName}

rig := newHumanRig(t, human, worker)
if _, _, err := rig.reconcile("alice"); err != nil {
t.Fatalf("reconcile: %v", err)
}

var out v1beta1.Worker
if err := rig.client.Get(context.Background(),
types.NamespacedName{Name: "w1", Namespace: "default"}, &out); err != nil {
t.Fatalf("get worker: %v", err)
}
if out.Spec.ChannelPolicy == nil ||
len(out.Spec.ChannelPolicy.GroupAllowExtra) != 1 ||
out.Spec.ChannelPolicy.GroupAllowExtra[0] != "@alice:localhost" {
t.Fatalf("groupAllowExtra=%v, want [@alice:localhost]", out.Spec.ChannelPolicy)
}
if got := out.Annotations["agentteams.io/human-group-allow-extra"]; got != `{"alice":"@alice:localhost"}` {
t.Fatalf("managed allowlist annotation=%q, want alice ownership", got)
}
}

func TestHumanReconciler_PreservesManualWorkerAllowlistEntry(t *testing.T) {
worker := newReadyWorker("w1", "!room-w1:localhost")
worker.Spec.ChannelPolicy = &v1beta1.ChannelPolicySpec{
GroupAllowExtra: []string{"@alice:localhost"},
}
human := activeHumanWithWorker("alice", "w1", "@alice:localhost")
rig := newHumanRig(t, human, worker)

if _, _, err := rig.reconcile("alice"); err != nil {
t.Fatalf("initial reconcile: %v", err)
}
updateHumanWorkers(t, rig, "alice", nil)
if _, _, err := rig.reconcile("alice"); err != nil {
t.Fatalf("revoke reconcile: %v", err)
}

out := getHumanTestWorker(t, rig, "w1")
if got := out.Spec.ChannelPolicy.GroupAllowExtra; len(got) != 1 || got[0] != "@alice:localhost" {
t.Fatalf("groupAllowExtra=%v, want manual entry preserved", got)
}
if got := out.Annotations[v1beta1.AnnotationHumanManagedGroupAllowExtra]; got != "" {
t.Fatalf("manual entry must not be controller-owned, annotation=%q", got)
}
}

func TestHumanReconciler_MultipleHumansShareWorkerAndRevokeOne(t *testing.T) {
worker := newReadyWorker("w1", "!room-w1:localhost")
alice := activeHumanWithWorker("alice", "w1", "@alice:localhost")
bob := activeHumanWithWorker("bob", "w1", "@bob:localhost")
rig := newHumanRig(t, alice, bob, worker)

if _, _, err := rig.reconcile("alice"); err != nil {
t.Fatalf("reconcile alice: %v", err)
}
if _, _, err := rig.reconcile("bob"); err != nil {
t.Fatalf("reconcile bob: %v", err)
}
updateHumanWorkers(t, rig, "alice", nil)
if _, _, err := rig.reconcile("alice"); err != nil {
t.Fatalf("revoke alice: %v", err)
}

out := getHumanTestWorker(t, rig, "w1")
if got := out.Spec.ChannelPolicy.GroupAllowExtra; len(got) != 1 || got[0] != "@bob:localhost" {
t.Fatalf("groupAllowExtra=%v, want only bob", got)
}
if got := out.Annotations[v1beta1.AnnotationHumanManagedGroupAllowExtra]; got != `{"bob":"@bob:localhost"}` {
t.Fatalf("managed allowlist annotation=%q, want only bob ownership", got)
}
}

func TestHumanReconciler_DeleteRemovesOnlyManagedWorkerAllowlistEntry(t *testing.T) {
now := metav1.Now()
human := activeHumanWithWorker("alice", "w1", "@alice:localhost")
human.DeletionTimestamp = &now
worker := newReadyWorker("w1", "!room-w1:localhost")
worker.Annotations = map[string]string{
v1beta1.AnnotationHumanManagedGroupAllowExtra: `{"alice":"@alice:localhost"}`,
}
worker.Spec.ChannelPolicy = &v1beta1.ChannelPolicySpec{
GroupAllowExtra: []string{"@alice:localhost", "@manual:localhost"},
}
rig := newHumanRig(t, human, worker)

if _, _, err := rig.reconcile("alice"); err != nil {
t.Fatalf("delete reconcile: %v", err)
}

out := getHumanTestWorker(t, rig, "w1")
if got := out.Spec.ChannelPolicy.GroupAllowExtra; len(got) != 1 || got[0] != "@manual:localhost" {
t.Fatalf("groupAllowExtra=%v, want only manual entry", got)
}
if got := out.Annotations[v1beta1.AnnotationHumanManagedGroupAllowExtra]; got != "" {
t.Fatalf("managed allowlist annotation=%q, want removed", got)
}
}

func TestHumanReconciler_WorkerAllowlistIsIdempotent(t *testing.T) {
worker := newReadyWorker("w1", "!room-w1:localhost")
human := activeHumanWithWorker("alice", "w1", "@alice:localhost")
rig := newHumanRig(t, human, worker)

if _, _, err := rig.reconcile("alice"); err != nil {
t.Fatalf("first reconcile: %v", err)
}
first := getHumanTestWorker(t, rig, "w1")
if _, _, err := rig.reconcile("alice"); err != nil {
t.Fatalf("second reconcile: %v", err)
}
second := getHumanTestWorker(t, rig, "w1")

if second.ResourceVersion != first.ResourceVersion {
t.Fatalf("Worker resourceVersion changed on idempotent reconcile: %q -> %q",
first.ResourceVersion, second.ResourceVersion)
}
if got := second.Spec.ChannelPolicy.GroupAllowExtra; len(got) != 1 || got[0] != "@alice:localhost" {
t.Fatalf("groupAllowExtra=%v, want one alice entry", got)
}
}

func activeHumanWithWorker(name, workerName, matrixUserID string) *v1beta1.Human {
human := newHuman(name, v1beta1.HumanSpec{AccessibleWorkers: []string{workerName}})
human.Status.MatrixUserID = matrixUserID
human.Status.InitialPassword = "stored-pw"
human.Status.Rooms = []string{"!room-" + workerName + ":localhost"}
human.Status.Phase = "Active"
human.Finalizers = []string{finalizerName}
return human
}

func updateHumanWorkers(t *testing.T, rig *humanTestRig, name string, workers []string) {
t.Helper()
var human v1beta1.Human
key := types.NamespacedName{Name: name, Namespace: "default"}
if err := rig.client.Get(context.Background(), key, &human); err != nil {
t.Fatalf("get Human %s: %v", name, err)
}
human.Spec.AccessibleWorkers = workers
if err := rig.client.Update(context.Background(), &human); err != nil {
t.Fatalf("update Human %s: %v", name, err)
}
}

func getHumanTestWorker(t *testing.T, rig *humanTestRig, name string) *v1beta1.Worker {
t.Helper()
var worker v1beta1.Worker
if err := rig.client.Get(context.Background(),
types.NamespacedName{Name: name, Namespace: "default"}, &worker); err != nil {
t.Fatalf("get Worker %s: %v", name, err)
}
return &worker
}

// TestHumanReconciler_Create_HappyPath covers the first-time provisioning
// path: a brand-new Human CR, one ready Worker, one ready Team. Asserts
// that a **single** reconcile converges — EnsureHumanUser is called
Expand Down
138 changes: 138 additions & 0 deletions agentteams-controller/internal/controller/human_reconcile_allowlist.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
package controller

import (
"context"
"encoding/json"
"fmt"

v1beta1 "github.com/agentscope-ai/AgentTeams/agentteams-controller/api/v1beta1"
kerrors "k8s.io/apimachinery/pkg/util/errors"
"sigs.k8s.io/controller-runtime/pkg/client"
)

func (r *HumanReconciler) reconcileHumanWorkerAllowlists(ctx context.Context, s *humanScope) error {
desired := make(map[string]struct{}, len(s.human.Spec.AccessibleWorkers))
for _, workerName := range s.human.Spec.AccessibleWorkers {
desired[workerName] = struct{}{}
}
return r.syncHumanWorkerAllowlists(ctx, s.human, s.identity.MatrixUserID, desired)
}

func (r *HumanReconciler) cleanupHumanWorkerAllowlists(ctx context.Context, s *humanScope) error {
userID := s.human.Status.MatrixUserID
if userID == "" {
userID = s.identity.MatrixUserID
}
return r.syncHumanWorkerAllowlists(ctx, s.human, userID, nil)
}

func (r *HumanReconciler) syncHumanWorkerAllowlists(
ctx context.Context,
human *v1beta1.Human,
userID string,
desired map[string]struct{},
) error {
var workers v1beta1.WorkerList
if err := r.List(ctx, &workers, client.InNamespace(human.Namespace)); err != nil {
return fmt.Errorf("list workers for human allowlist: %w", err)
}

var errs []error
for i := range workers.Items {
worker := &workers.Items[i]
_, shouldAllow := desired[worker.Name]
if err := r.syncHumanWorkerAllowlist(ctx, worker, human.Name, userID, shouldAllow); err != nil {
errs = append(errs, err)
}
}
return kerrors.NewAggregate(errs)
}

func (r *HumanReconciler) syncHumanWorkerAllowlist(
ctx context.Context,
worker *v1beta1.Worker,
humanName string,
userID string,
shouldAllow bool,
) error {
managed, err := managedHumanAllowlist(worker.Annotations)
if err != nil {
return fmt.Errorf("parse managed Human allowlist for Worker %s: %w", worker.Name, err)
}

managedID, isManaged := managed[humanName]
if !shouldAllow && !isManaged {
return nil
}

base := worker.DeepCopy()
changed := false
if shouldAllow {
if worker.Spec.ChannelPolicy == nil {
worker.Spec.ChannelPolicy = &v1beta1.ChannelPolicySpec{}
}
allowlist := worker.Spec.ChannelPolicy.GroupAllowExtra
if !containsString(allowlist, userID) {
if isManaged && managedID != userID {
worker.Spec.ChannelPolicy.GroupAllowExtra = removeString(allowlist, managedID)
}
worker.Spec.ChannelPolicy.GroupAllowExtra = append(worker.Spec.ChannelPolicy.GroupAllowExtra, userID)
managed[humanName] = userID
changed = true
} else if isManaged && managedID != userID {
worker.Spec.ChannelPolicy.GroupAllowExtra = removeString(allowlist, managedID)
delete(managed, humanName)
changed = true
}
} else {
if worker.Spec.ChannelPolicy != nil {
worker.Spec.ChannelPolicy.GroupAllowExtra = removeString(
worker.Spec.ChannelPolicy.GroupAllowExtra,
managedID,
)
}
delete(managed, humanName)
changed = true
}

if !changed {
return nil
}
if err := setManagedHumanAllowlist(worker, managed); err != nil {
return err
}
if err := r.Patch(ctx, worker, client.MergeFrom(base)); err != nil {
return fmt.Errorf("patch Worker %s Human allowlist: %w", worker.Name, err)
}
return nil
}

func managedHumanAllowlist(annotations map[string]string) (map[string]string, error) {
managed := map[string]string{}
if annotations == nil || annotations[v1beta1.AnnotationHumanManagedGroupAllowExtra] == "" {
return managed, nil
}
if err := json.Unmarshal(
[]byte(annotations[v1beta1.AnnotationHumanManagedGroupAllowExtra]),
&managed,
); err != nil {
return nil, err
}
return managed, nil
}

func setManagedHumanAllowlist(worker *v1beta1.Worker, managed map[string]string) error {
if worker.Annotations == nil {
worker.Annotations = map[string]string{}
}
if len(managed) == 0 {
delete(worker.Annotations, v1beta1.AnnotationHumanManagedGroupAllowExtra)
return nil
}
value, err := json.Marshal(managed)
if err != nil {
return fmt.Errorf("marshal managed Human allowlist: %w", err)
}
worker.Annotations[v1beta1.AnnotationHumanManagedGroupAllowExtra] = string(value)
return nil
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@ import (
// human to /leave (password may be stale), so we rely on the Tuwunel
// admin bot's force-leave-room command instead.
//
// Every external call here is non-fatal: a transient Matrix or OSS
// failure must not wedge finalizer removal, and the homeserver's
// delete_rooms_after_leave / forget_forced_upon_leave flags provide a
// safety net if any force-leave never lands.
// Matrix room cleanup is non-fatal: the homeserver's delete_rooms_after_leave /
// forget_forced_upon_leave flags provide a safety net if a force-leave never
// lands. Worker allowlist cleanup must succeed before finalizer removal,
// otherwise the deleted Human could retain access indefinitely.
func (r *HumanReconciler) reconcileHumanDelete(ctx context.Context, s *humanScope) (reconcile.Result, error) {
logger := log.FromContext(ctx)
h := s.human
Expand All @@ -35,6 +35,10 @@ func (r *HumanReconciler) reconcileHumanDelete(ctx context.Context, s *humanScop
}
}

if err := r.cleanupHumanWorkerAllowlists(ctx, s); err != nil {
return reconcile.Result{RequeueAfter: reconcileInterval}, err
}

if s.identity.Source != nil {
if err := s.identity.Source.EnsureDeactivated(ctx, &h.Spec, &h.Status); err != nil {
return reconcile.Result{RequeueAfter: reconcileInterval}, err
Expand Down
1 change: 1 addition & 0 deletions changelog/current.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ Record image-affecting changes to `manager/`, `worker/`, `copaw/`, `hermes/`, `o

**Bug Fixes**

- **Human Worker access allowlist**: Synchronize each Human's accessible Workers with runtime group allowlists while preserving manually managed entries.
- **Team Worker room boundary convergence**: Remove Manager again after standalone Worker infrastructure reconciliation restores regular Team Worker personal-room membership. ([b5b0add](https://github.com/agentscope-ai/AgentTeams/commit/b5b0add))
- **Team Worker reference enforcement**: Keep referenced Worker CRs protected during direct deletion and reject Team API members whose required role is empty. ([d96f1ed](https://github.com/agentscope-ai/AgentTeams/commit/d96f1ed))
- **Team Worker room membership**: Force Manager out of regular Team Worker personal rooms when equal Matrix power levels prevent a normal kick. ([43545c2](https://github.com/agentscope-ai/AgentTeams/commit/43545c2))
Expand Down