Skip to content

Commit dd07a01

Browse files
onutconutc
committed
fix(provisioning): scope reservations to control namespace
1 parent 9c4900a commit dd07a01

4 files changed

Lines changed: 121 additions & 10 deletions

File tree

‎api/main.go‎

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ type server struct {
3434
restConfig *rest.Config
3535
scheme *runtime.Scheme
3636
namespace string
37+
controlNamespace string
3738
auth authConfig
3839
internalAuth internalAuthConfig
3940
ingressDefaults ingressDefaults
@@ -68,6 +69,16 @@ func main() {
6869
os.Exit(1)
6970
}
7071
ns := os.Getenv("SPRITZ_NAMESPACE")
72+
controlNamespace := strings.TrimSpace(os.Getenv("SPRITZ_CONTROL_NAMESPACE"))
73+
if controlNamespace == "" {
74+
controlNamespace = strings.TrimSpace(os.Getenv("POD_NAMESPACE"))
75+
}
76+
if controlNamespace == "" {
77+
controlNamespace = strings.TrimSpace(ns)
78+
}
79+
if controlNamespace == "" {
80+
controlNamespace = "default"
81+
}
7182
{
7283
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
7384
defer cancel()
@@ -129,6 +140,7 @@ func main() {
129140
restConfig: cfg,
130141
scheme: scheme,
131142
namespace: ns,
143+
controlNamespace: controlNamespace,
132144
auth: auth,
133145
internalAuth: internalAuth,
134146
ingressDefaults: ingressDefaults,
@@ -265,6 +277,17 @@ func (s *server) resolveSpritzNamespace(requested string) (string, error) {
265277
return namespace, nil
266278
}
267279

280+
func (s *server) namespaceOverrideRequested(requested, resolved string) bool {
281+
requested = strings.TrimSpace(requested)
282+
if requested == "" {
283+
return false
284+
}
285+
if strings.TrimSpace(s.namespace) == "" {
286+
return true
287+
}
288+
return requested != strings.TrimSpace(resolved)
289+
}
290+
268291
func (s *server) listPresets(c echo.Context) error {
269292
principal, ok := principalFromContext(c)
270293
if s.auth.enabled() && (!ok || principal.ID == "") {
@@ -388,8 +411,7 @@ func (s *server) createSpritz(c echo.Context) error {
388411
if err != nil {
389412
return writeError(c, http.StatusForbidden, err.Error())
390413
}
391-
requestedNamespaceValue := strings.TrimSpace(body.Namespace)
392-
requestedNamespace := requestedNamespaceValue != "" && requestedNamespaceValue != namespace
414+
requestedNamespace := s.namespaceOverrideRequested(body.Namespace, namespace)
393415

394416
owner, err := normalizeCreateOwner(&body, principal, s.auth.enabled())
395417
if err != nil {
@@ -570,7 +592,7 @@ func (s *server) createSpritz(c echo.Context) error {
570592
return writeError(c, http.StatusInternalServerError, err.Error())
571593
}
572594
if principal.isService() {
573-
if err := s.completeIdempotencyReservation(c.Request().Context(), namespace, principal.ID, body.IdempotencyKey, spritz); err != nil {
595+
if err := s.completeIdempotencyReservation(c.Request().Context(), principal.ID, body.IdempotencyKey, spritz); err != nil {
574596
return writeError(c, http.StatusInternalServerError, err.Error())
575597
}
576598
}

‎api/main_create_owner_test.go‎

Lines changed: 76 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,10 @@ func newCreateSpritzTestServer(t *testing.T) *server {
3838
t.Helper()
3939
scheme := newTestSpritzScheme(t)
4040
return &server{
41-
client: fake.NewClientBuilder().WithScheme(scheme).Build(),
42-
scheme: scheme,
43-
namespace: "spritz-test",
41+
client: fake.NewClientBuilder().WithScheme(scheme).Build(),
42+
scheme: scheme,
43+
namespace: "spritz-test",
44+
controlNamespace: "spritz-test",
4445
auth: authConfig{
4546
mode: authModeHeader,
4647
headerID: "X-Spritz-User-Id",
@@ -758,6 +759,78 @@ func TestCreateSpritzAllowsProvisionerCurrentNamespaceWithoutOverride(t *testing
758759
}
759760
}
760761

762+
func TestCreateSpritzRejectsExplicitNamespaceForProvisionerWhenOverrideDisabled(t *testing.T) {
763+
s := newCreateSpritzTestServer(t)
764+
configureProvisionerTestServer(s)
765+
s.namespace = ""
766+
s.controlNamespace = "spritz-system"
767+
s.provisioners.allowedNamespaces = map[string]struct{}{"team-a": {}}
768+
769+
e := echo.New()
770+
secured := e.Group("", s.authMiddleware())
771+
secured.POST("/api/spritzes", s.createSpritz)
772+
773+
body := []byte(`{"presetId":"openclaw","ownerId":"user-123","idempotencyKey":"discord-ns-override","namespace":"team-a"}`)
774+
req := httptest.NewRequest(http.MethodPost, "/api/spritzes", bytes.NewReader(body))
775+
req.Header.Set(echo.HeaderContentType, echo.MIMEApplicationJSON)
776+
req.Header.Set("X-Spritz-User-Id", "zenobot")
777+
req.Header.Set("X-Spritz-Principal-Type", "service")
778+
req.Header.Set("X-Spritz-Principal-Scopes", "spritz.instances.create,spritz.instances.assign_owner")
779+
rec := httptest.NewRecorder()
780+
781+
e.ServeHTTP(rec, req)
782+
783+
if rec.Code != http.StatusBadRequest {
784+
t.Fatalf("expected status 400, got %d: %s", rec.Code, rec.Body.String())
785+
}
786+
if !strings.Contains(rec.Body.String(), "namespace override is not allowed") {
787+
t.Fatalf("expected namespace override error, got %s", rec.Body.String())
788+
}
789+
}
790+
791+
func TestCreateSpritzRejectsProvisionerIdempotencyReuseAcrossNamespaces(t *testing.T) {
792+
s := newCreateSpritzTestServer(t)
793+
configureProvisionerTestServer(s)
794+
s.namespace = ""
795+
s.controlNamespace = "spritz-system"
796+
s.provisioners.allowNamespaceOverride = true
797+
s.provisioners.allowedNamespaces = map[string]struct{}{"team-a": {}, "team-b": {}}
798+
799+
e := echo.New()
800+
secured := e.Group("", s.authMiddleware())
801+
secured.POST("/api/spritzes", s.createSpritz)
802+
803+
first := []byte(`{"presetId":"openclaw","ownerId":"user-123","idempotencyKey":"discord-cross-ns","namespace":"team-a"}`)
804+
second := []byte(`{"presetId":"openclaw","ownerId":"user-123","idempotencyKey":"discord-cross-ns","namespace":"team-b"}`)
805+
806+
req1 := httptest.NewRequest(http.MethodPost, "/api/spritzes", bytes.NewReader(first))
807+
req1.Header.Set(echo.HeaderContentType, echo.MIMEApplicationJSON)
808+
req1.Header.Set("X-Spritz-User-Id", "zenobot")
809+
req1.Header.Set("X-Spritz-Principal-Type", "service")
810+
req1.Header.Set("X-Spritz-Principal-Scopes", "spritz.instances.create,spritz.instances.assign_owner")
811+
rec1 := httptest.NewRecorder()
812+
e.ServeHTTP(rec1, req1)
813+
814+
if rec1.Code != http.StatusCreated {
815+
t.Fatalf("expected first create to succeed, got %d: %s", rec1.Code, rec1.Body.String())
816+
}
817+
818+
req2 := httptest.NewRequest(http.MethodPost, "/api/spritzes", bytes.NewReader(second))
819+
req2.Header.Set(echo.HeaderContentType, echo.MIMEApplicationJSON)
820+
req2.Header.Set("X-Spritz-User-Id", "zenobot")
821+
req2.Header.Set("X-Spritz-Principal-Type", "service")
822+
req2.Header.Set("X-Spritz-Principal-Scopes", "spritz.instances.create,spritz.instances.assign_owner")
823+
rec2 := httptest.NewRecorder()
824+
e.ServeHTTP(rec2, req2)
825+
826+
if rec2.Code != http.StatusConflict {
827+
t.Fatalf("expected status 409, got %d: %s", rec2.Code, rec2.Body.String())
828+
}
829+
if !strings.Contains(rec2.Body.String(), "idempotencyKey already used with a different request") {
830+
t.Fatalf("expected idempotency conflict, got %s", rec2.Body.String())
831+
}
832+
}
833+
761834
func TestCreateSpritzRetriesPendingIdempotencyReservationWithConflictingOccupant(t *testing.T) {
762835
s := newCreateSpritzTestServer(t)
763836
configureProvisionerTestServer(s)

‎api/provisioning.go‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -577,15 +577,26 @@ func idempotencyReservationName(actorID, key string) string {
577577
return fmt.Sprintf("%s%x", idempotencyReservationPrefix, sum[:16])
578578
}
579579

580+
func (s *server) idempotencyReservationNamespace() string {
581+
if namespace := strings.TrimSpace(s.controlNamespace); namespace != "" {
582+
return namespace
583+
}
584+
if namespace := strings.TrimSpace(s.namespace); namespace != "" {
585+
return namespace
586+
}
587+
return "default"
588+
}
589+
580590
func (s *server) reserveIdempotentCreateName(ctx context.Context, namespace string, principal principal, key, fingerprint, desiredName string) (string, bool, error) {
581591
if strings.TrimSpace(key) == "" {
582592
return desiredName, false, nil
583593
}
584594
reservationName := idempotencyReservationName(principal.ID, key)
595+
reservationNamespace := s.idempotencyReservationNamespace()
585596
record := &corev1.ConfigMap{
586597
ObjectMeta: metav1.ObjectMeta{
587598
Name: reservationName,
588-
Namespace: namespace,
599+
Namespace: reservationNamespace,
589600
Labels: map[string]string{
590601
actorLabelKey: actorLabelValue(principal.ID),
591602
idempotencyLabelKey: idempotencyLabelValue(key),
@@ -602,7 +613,7 @@ func (s *server) reserveIdempotentCreateName(ctx context.Context, namespace stri
602613
return "", false, err
603614
}
604615
existing := &corev1.ConfigMap{}
605-
if getErr := s.client.Get(ctx, clientKey(namespace, reservationName), existing); getErr != nil {
616+
if getErr := s.client.Get(ctx, clientKey(reservationNamespace, reservationName), existing); getErr != nil {
606617
return "", false, getErr
607618
}
608619
if strings.TrimSpace(existing.Data[idempotencyReservationHashKey]) != fingerprint {
@@ -646,13 +657,14 @@ func (s *server) reserveIdempotentCreateName(ctx context.Context, namespace stri
646657
return desiredName, false, nil
647658
}
648659

649-
func (s *server) completeIdempotencyReservation(ctx context.Context, namespace, actorID, key string, spritz *spritzv1.Spritz) error {
660+
func (s *server) completeIdempotencyReservation(ctx context.Context, actorID, key string, spritz *spritzv1.Spritz) error {
650661
if strings.TrimSpace(actorID) == "" || strings.TrimSpace(key) == "" || spritz == nil {
651662
return nil
652663
}
653664
reservationName := idempotencyReservationName(actorID, key)
665+
reservationNamespace := s.idempotencyReservationNamespace()
654666
current := &corev1.ConfigMap{}
655-
if err := s.client.Get(ctx, clientKey(namespace, reservationName), current); err != nil {
667+
if err := s.client.Get(ctx, clientKey(reservationNamespace, reservationName), current); err != nil {
656668
if apierrors.IsNotFound(err) {
657669
return nil
658670
}

‎helm/spritz/templates/api-deployment.yaml‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,10 @@ spec:
4040
{{- toYaml .Values.api.resources | nindent 12 }}
4141
{{- end }}
4242
env:
43+
- name: POD_NAMESPACE
44+
valueFrom:
45+
fieldRef:
46+
fieldPath: metadata.namespace
4347
- name: SPRITZ_NAMESPACE
4448
value: {{ .Values.spritz.namespace | quote }}
4549
{{- if .Values.api.defaultAnnotations }}

0 commit comments

Comments
 (0)