From 3aab748c8644a39cca351bc7a0bc5da73873897a Mon Sep 17 00:00:00 2001 From: Preston Holmes Date: Fri, 5 Jun 2026 13:54:32 -0700 Subject: [PATCH 1/4] fix: prevent stale disconnect from marking reconnected broker offline (#303) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: atomic session-guarded broker disconnect to prevent reconnect race (#131) The onDisconnect callback previously used separate ReleaseRuntimeBrokerConnection and UpdateRuntimeBrokerHeartbeat calls. When a broker disconnects and reconnects rapidly, the stale disconnect's offline stamp can clobber the new connection's online status because UpdateRuntimeBrokerHeartbeat has no session guard — it unconditionally overwrites status. Provider statuses are also clobbered and never restored by heartbeats, leaving the broker permanently invisible until hub restart. Add ReleaseAndMarkBrokerOffline which atomically clears affinity AND stamps status=offline in a single CAS write. If a concurrent reconnect has already claimed the broker with a new session, the compare fails and the callback is a no-op. Also add a re-check guard before updating provider statuses. * docs: add project log for broker disconnect race fix unification --- .../2026-06-05-broker-disconnect-race-fix.md | 48 +++++++++++ pkg/hub/server.go | 25 ++++-- pkg/store/entadapter/broker_affinity_test.go | 84 +++++++++++++++++++ pkg/store/entadapter/project_store.go | 42 ++++++++++ pkg/store/store.go | 9 ++ 5 files changed, 199 insertions(+), 9 deletions(-) create mode 100644 .design/project-log/2026-06-05-broker-disconnect-race-fix.md diff --git a/.design/project-log/2026-06-05-broker-disconnect-race-fix.md b/.design/project-log/2026-06-05-broker-disconnect-race-fix.md new file mode 100644 index 000000000..ec8ea43ad --- /dev/null +++ b/.design/project-log/2026-06-05-broker-disconnect-race-fix.md @@ -0,0 +1,48 @@ +# Project Log: Broker Disconnect Reconnect Race Fix (Issue #131) + +**Date:** 2026-06-05 +**Task:** Unify broker disconnect race fix from two branches into PR #303 + +## Problem + +When a broker disconnects and reconnects rapidly, the stale disconnect callback's +offline stamp can clobber the new connection's online status. The root cause is a +TOCTOU race: `ReleaseRuntimeBrokerConnection` and `UpdateRuntimeBrokerHeartbeat` +were separate calls — the heartbeat update has no session guard and unconditionally +overwrites status. Provider statuses are also clobbered and never restored by +heartbeats, leaving the broker permanently invisible until hub restart. + +## Solution + +Added `ReleaseAndMarkBrokerOffline` to the store interface — a single CAS write +that atomically clears affinity AND stamps status=offline, only if the session +still matches. If a concurrent reconnect has already claimed the broker with a +new session, the compare fails and the callback is a no-op. + +Also added a re-check guard in `server.go` before updating provider statuses: +after the atomic release, re-read the broker to confirm no concurrent +`markBrokerOnline` has re-claimed it before stamping providers offline. + +## Branch Unification + +Two branches addressed this issue: +- `scion/dev-issue-131` (PR #303): had only a docs/project-log commit, no code fix +- `origin/fix/session-guarded-broker-disconnect` (fork PR #144): had the complete + code fix with tests + +The fork branch's fix was the more complete solution. Rebased PR #303 onto +upstream main and cherry-picked the fork's fix commit to produce a single +unified branch. + +## Files Changed + +- `pkg/store/store.go` — added `ReleaseAndMarkBrokerOffline` to `RuntimeBrokerStore` interface +- `pkg/store/entadapter/project_store.go` — implemented `ReleaseAndMarkBrokerOffline` with CAS retry loop +- `pkg/hub/server.go` — rewired `SetOnDisconnect` callback to use the atomic method + provider re-check guard +- `pkg/store/entadapter/broker_affinity_test.go` — 4 new tests covering the atomic method + +## Verification + +- All 10 broker affinity tests pass (4 new + 6 existing) +- Hub package compiles cleanly +- Pre-existing test failures in `pkg/hub` (unrelated to this change) confirmed on upstream main diff --git a/pkg/hub/server.go b/pkg/hub/server.go index e3890f6ca..d77f8434d 100644 --- a/pkg/hub/server.go +++ b/pkg/hub/server.go @@ -724,29 +724,36 @@ func New(cfg ServerConfig, s store.Store) (*Server, error) { Debug: cfg.Debug, }, logging.Subsystem("hub.control-channel")) // Set disconnect callback to mark broker offline when WebSocket drops. - // Compare-and-clear affinity first: only stamp offline if this hub instance + - // session still owns the broker. If affinity has moved (broker flapped to - // another replica or re-dialed with a newer session), this is a stale - // disconnect and we must NOT clobber the live owner's online status. + // ReleaseAndMarkBrokerOffline atomically clears affinity AND stamps + // status=offline in a single CAS write — if a concurrent reconnect has + // already claimed the broker with a new session, the compare fails and the + // callback is a no-op. This eliminates the TOCTOU race where a separate + // ReleaseRuntimeBrokerConnection + UpdateRuntimeBrokerHeartbeat allowed + // the offline stamp to clobber a concurrent markBrokerOnline (issue #131). srv.controlChannel.SetOnDisconnect(func(brokerID, sessionID string) { ctx := context.Background() - cleared, err := s.ReleaseRuntimeBrokerConnection(ctx, brokerID, srv.instanceID, sessionID) + cleared, err := s.ReleaseAndMarkBrokerOffline(ctx, brokerID, srv.instanceID, sessionID) if err != nil { slog.Error("Failed to release broker affinity on disconnect", "brokerID", brokerID, "sessionID", sessionID, "error", err) return } if !cleared { - // Another replica (or a newer session on this replica) already owns - // the socket. Skip the offline stamp to avoid clobbering it. slog.Info("broker reconnected elsewhere; skipping offline stamp", "brokerID", brokerID, "staleSession", sessionID) return } slog.Info("Broker disconnected, marking offline", "brokerID", brokerID, "sessionID", sessionID) - if err := s.UpdateRuntimeBrokerHeartbeat(ctx, brokerID, store.BrokerStatusOffline); err != nil { - slog.Error("Failed to mark broker offline", "brokerID", brokerID, "error", err) + // Guard: re-read the broker before updating provider statuses. A + // concurrent markBrokerOnline may have already re-claimed the broker + // between our atomic release+offline and now. If so, skip provider + // updates to avoid clobbering the new session's online providers. + broker, rerr := s.GetRuntimeBroker(ctx, brokerID) + if rerr == nil && broker.ConnectedSessionID != nil && *broker.ConnectedSessionID != "" { + slog.Info("broker re-claimed by new session after release; skipping provider offline stamp", + "brokerID", brokerID, "staleSession", sessionID, "newSession", *broker.ConnectedSessionID) + return } // Update all project provider records for this broker diff --git a/pkg/store/entadapter/broker_affinity_test.go b/pkg/store/entadapter/broker_affinity_test.go index 6044ee395..c48fc54f9 100644 --- a/pkg/store/entadapter/broker_affinity_test.go +++ b/pkg/store/entadapter/broker_affinity_test.go @@ -106,6 +106,90 @@ func TestReleaseRuntimeBrokerConnection_NoOpWhenUnclaimed(t *testing.T) { assert.False(t, cleared) } +// --------------------------------------------------------------------------- +// ReleaseAndMarkBrokerOffline — atomic release + offline stamp +// --------------------------------------------------------------------------- + +func TestReleaseAndMarkBrokerOffline_StampsOffline(t *testing.T) { + ps := newTestProjectStore(t) + ctx := context.Background() + b := newOfflineBroker(t, ps) + + require.NoError(t, ps.ClaimRuntimeBrokerConnection(ctx, b.ID, "hub-1", "sess-1")) + + cleared, err := ps.ReleaseAndMarkBrokerOffline(ctx, b.ID, "hub-1", "sess-1") + require.NoError(t, err) + assert.True(t, cleared) + + got, err := ps.GetRuntimeBroker(ctx, b.ID) + require.NoError(t, err) + assert.Nil(t, got.ConnectedHubID) + assert.Nil(t, got.ConnectedSessionID) + assert.Nil(t, got.ConnectedAt) + assert.Equal(t, store.BrokerStatusOffline, got.Status) + assert.False(t, got.LastHeartbeat.IsZero()) +} + +func TestReleaseAndMarkBrokerOffline_NoopOnSessionMismatch(t *testing.T) { + ps := newTestProjectStore(t) + ctx := context.Background() + b := newOfflineBroker(t, ps) + + require.NoError(t, ps.ClaimRuntimeBrokerConnection(ctx, b.ID, "hub-1", "sess-NEW")) + + // Stale session tries to release+offline: must be a no-op. + cleared, err := ps.ReleaseAndMarkBrokerOffline(ctx, b.ID, "hub-1", "sess-OLD") + require.NoError(t, err) + assert.False(t, cleared) + + got, err := ps.GetRuntimeBroker(ctx, b.ID) + require.NoError(t, err) + require.NotNil(t, got.ConnectedHubID) + assert.Equal(t, "hub-1", *got.ConnectedHubID) + require.NotNil(t, got.ConnectedSessionID) + assert.Equal(t, "sess-NEW", *got.ConnectedSessionID) + assert.Equal(t, store.BrokerStatusOnline, got.Status, "status must remain online") +} + +// TestReleaseAndMarkBrokerOffline_NoopAfterReclaim reproduces the exact race +// from issue #131: old session releases + stamps offline, but a new session +// has already re-claimed the broker. The stale release must be a no-op. +func TestReleaseAndMarkBrokerOffline_NoopAfterReclaim(t *testing.T) { + ps := newTestProjectStore(t) + ctx := context.Background() + b := newOfflineBroker(t, ps) + + // t0: session A claims. + require.NoError(t, ps.ClaimRuntimeBrokerConnection(ctx, b.ID, "hub-1", "sess-A")) + // t1: session A disconnects, but before the callback runs, session B re-claims. + require.NoError(t, ps.ClaimRuntimeBrokerConnection(ctx, b.ID, "hub-1", "sess-B")) + + // t2: stale callback tries to release+offline for session A. + cleared, err := ps.ReleaseAndMarkBrokerOffline(ctx, b.ID, "hub-1", "sess-A") + require.NoError(t, err) + assert.False(t, cleared, "stale session must not stamp offline") + + got, err := ps.GetRuntimeBroker(ctx, b.ID) + require.NoError(t, err) + require.NotNil(t, got.ConnectedSessionID) + assert.Equal(t, "sess-B", *got.ConnectedSessionID, "new session must still own the broker") + assert.Equal(t, store.BrokerStatusOnline, got.Status, "status must remain online") +} + +func TestReleaseAndMarkBrokerOffline_NoopWhenUnclaimed(t *testing.T) { + ps := newTestProjectStore(t) + ctx := context.Background() + b := newOfflineBroker(t, ps) + + cleared, err := ps.ReleaseAndMarkBrokerOffline(ctx, b.ID, "hub-1", "sess-1") + require.NoError(t, err) + assert.False(t, cleared) +} + +// --------------------------------------------------------------------------- +// Flap / cross-hub scenarios +// --------------------------------------------------------------------------- + // TestBrokerAffinity_FlapAtoB reproduces the design §9.4 disconnect race: a // broker flaps from hub A to hub B; A's delayed onDisconnect must NOT clobber // B's live ownership. diff --git a/pkg/store/entadapter/project_store.go b/pkg/store/entadapter/project_store.go index 056d64089..5b2275a1d 100644 --- a/pkg/store/entadapter/project_store.go +++ b/pkg/store/entadapter/project_store.go @@ -890,6 +890,48 @@ func (s *ProjectStore) ReleaseRuntimeBrokerConnection(ctx context.Context, broke return false, store.ErrVersionConflict } +// ReleaseAndMarkBrokerOffline atomically clears broker affinity AND stamps +// status=offline in a single CAS write, ONLY IF affinity still names +// (hubInstanceID, sessionID). This eliminates the TOCTOU race between a +// separate release and a separate offline stamp: if a concurrent reconnect +// has already claimed the broker with a new session, the compare fails and +// this is a no-op — the new connection's online status is not clobbered. +func (s *ProjectStore) ReleaseAndMarkBrokerOffline(ctx context.Context, brokerID, hubInstanceID, sessionID string) (bool, error) { + uid, err := parseUUID(brokerID) + if err != nil { + return false, err + } + + now := time.Now() + for attempt := 0; attempt < maxCASRetries; attempt++ { + cur, err := s.client.RuntimeBroker.Get(ctx, uid) + if err != nil { + return false, mapError(err) + } + if cur.ConnectedHubID == nil || *cur.ConnectedHubID != hubInstanceID || + cur.ConnectedSessionID == nil || *cur.ConnectedSessionID != sessionID { + return false, nil + } + affected, err := s.client.RuntimeBroker.Update(). + Where(runtimebroker.IDEQ(uid), runtimebroker.LockVersionEQ(cur.LockVersion)). + ClearConnectedHubID(). + ClearConnectedSessionID(). + ClearConnectedAt(). + SetStatus(store.BrokerStatusOffline). + SetLastHeartbeat(now). + SetUpdated(now). + AddLockVersion(1). + Save(ctx) + if err != nil { + return false, mapError(err) + } + if affected == 1 { + return true, nil + } + } + return false, store.ErrVersionConflict +} + // ReapStaleBrokerAffinity clears affinity (connected_hub_id/connected_session_id/ // connected_at) for brokers that still claim affinity but whose last_heartbeat // is older than staleBefore. Does not change broker status. diff --git a/pkg/store/store.go b/pkg/store/store.go index 6f357f624..b148c488b 100644 --- a/pkg/store/store.go +++ b/pkg/store/store.go @@ -321,6 +321,15 @@ type RuntimeBrokerStore interface { // It does not change status (the caller decides offline based on cleared). ReleaseRuntimeBrokerConnection(ctx context.Context, brokerID, hubInstanceID, sessionID string) (cleared bool, err error) + // ReleaseAndMarkBrokerOffline atomically clears broker affinity AND stamps + // status=offline, ONLY IF affinity still names (hubInstanceID, sessionID). + // This prevents a stale disconnect callback from clobbering a concurrent + // reconnect's online status — the session check and the offline stamp happen + // in the same CAS write with no TOCTOU window. + // Returns cleared=true when affinity matched and the broker was stamped offline. + // Returns cleared=false (no-op) when affinity has already moved. + ReleaseAndMarkBrokerOffline(ctx context.Context, brokerID, hubInstanceID, sessionID string) (cleared bool, err error) + // ReapStaleBrokerAffinity clears connected_hub_id/connected_session_id/ // connected_at for brokers whose last_heartbeat is older than staleBefore // and whose connected_hub_id is not NULL (i.e. they still claim affinity). From e04d4313445a6a2eea3ee730ae8a84735949567c Mon Sep 17 00:00:00 2001 From: Preston Holmes Date: Fri, 5 Jun 2026 14:04:40 -0700 Subject: [PATCH 2/4] feat: resource clone & delete with authz hardening (#301) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * docs(design): reduced resource clone/delete design (resolved review) * refactor: remove dead Locked field from Template and HarnessConfig models Remove the Locked bool field, all 16 enforcement sites across 6 handler files, the force query parameter from delete endpoints, 3 locked-template tests, and add a DB migration to drop the column. No production code ever set Locked=true — this simplifies the handlers for the upcoming clone/delete feature. * feat: add harness-config clone endpoint, authz hardening, and slug uniqueness - Add handleHarnessConfigClone mirroring template clone - Add CheckAccess authz to deleteTemplateV2, handleTemplateClone, deleteHarnessConfig, handleHarnessConfigClone - Add DB migration V55: UNIQUE constraint on (slug, scope, scope_id) - Return 409 Conflict on slug collision during clone - Add clone failure cleanup - Add tests for clone, authz, and slug collision * feat(web): add Clone/Delete row actions and clone-from-global to resource list - Add Clone and Delete action menu to shared resource-list component - Add delete confirmation dialog with deleteFiles checkbox (default on) - Add clone dialog with name input and 409 collision handling - Add clone-from-global picker in project settings view - Unify on resource-changed event (migrate resource-imported) - Gate actions on capabilities (canClone, canDelete properties) * fix: address PR review — cleanup orphaned files on DB create failure, remove redundant clone method - Add stor.DeletePrefix cleanup when CreateTemplate/CreateHarnessConfig fails after files were already copied (prevents orphaned storage files) - Remove redundant confirmCloneFromGlobal method — confirmClone already handles cross-scope clone via the component's scope/scopeId properties * fix: adapt Locked removal and slug constraint to Ent-based schema Remove Locked references from entadapter, remove stale sqlite.go (replaced by Ent ORM upstream), add UNIQUE(slug, scope, scope_id) to Ent schema indexes, and regenerate Ent code. * fix: adapt tests and entadapter for Ent-based store (UUID IDs, no Locked) - Use api.NewUUID() for all test entity IDs (Ent enforces UUID format) - Remove Locked field from entadapter create/update calls - Remove stale sqlite.go (replaced by Ent ORM upstream) - Add UNIQUE(slug, scope, scope_id) to Ent schema indexes --- .design/resource-clone-delete.md | 338 ++++++++++++ pkg/ent/harnessconfig.go | 13 - pkg/ent/harnessconfig/harnessconfig.go | 10 - pkg/ent/harnessconfig/where.go | 15 - pkg/ent/harnessconfig_create.go | 65 --- pkg/ent/harnessconfig_update.go | 34 -- pkg/ent/migrate/schema.go | 18 +- pkg/ent/mutation.go | 112 +--- pkg/ent/runtime.go | 20 +- pkg/ent/schema/harnessconfig.go | 4 +- pkg/ent/schema/template.go | 4 +- pkg/ent/template.go | 13 - pkg/ent/template/template.go | 10 - pkg/ent/template/where.go | 15 - pkg/ent/template_create.go | 65 --- pkg/ent/template_update.go | 34 -- pkg/hub/clone_delete_handler_test.go | 206 ++++++++ pkg/hub/harness_config_file_handlers.go | 15 - pkg/hub/harness_config_handlers.go | 184 ++++++- pkg/hub/template_file_handlers.go | 15 - pkg/hub/template_file_handlers_test.go | 74 --- pkg/hub/template_handlers.go | 113 +++- pkg/store/entadapter/template_store.go | 6 - pkg/store/models.go | 8 +- web/src/components/pages/project-settings.ts | 15 +- web/src/components/pages/settings.ts | 10 +- web/src/components/shared/resource-import.ts | 4 +- web/src/components/shared/resource-list.ts | 513 ++++++++++++++++++- 28 files changed, 1329 insertions(+), 604 deletions(-) create mode 100644 .design/resource-clone-delete.md create mode 100644 pkg/hub/clone_delete_handler_test.go diff --git a/.design/resource-clone-delete.md b/.design/resource-clone-delete.md new file mode 100644 index 000000000..744283293 --- /dev/null +++ b/.design/resource-clone-delete.md @@ -0,0 +1,338 @@ +# Resource Clone & Delete (Reduced Hub Resource Management) + +**Status:** Reviewed — all open questions resolved (see §3.1); ready for implementation +**Created:** 2026-06-02 +**Author:** Agent (template-harness-refactor) +**Related:** [hub-template-admin.md](./hub-template-admin.md) (parent — full admin view), [resource-import-refactor.md](./resource-import-refactor.md) (import slice, now built), [grove-level-templates.md](./grove-level-templates.md), [agnostic-template-design.md](./agnostic-template-design.md) + +--- + +## 1. Overview + +`hub-template-admin.md` proposed a full hub admin template-management page: a +dedicated `/admin/templates` list with filters/sorting/pagination and a row action +menu offering View/Edit, **Clone**, Lock/Unlock, Archive, and **Delete**, plus +hub-level import. Since that draft, two things changed the ground under it: + +1. **The import slice is fully built** (`resource-import-refactor.md`, Phases 0–4): + a unified `POST /api/v1/resources/import` endpoint, a shared + `` web component mounted in both Project Settings → + Resources and the Hub Resources page, streaming per-resource progress, and a + parallelized import path. Import is **done** and is no longer part of this work. +2. **A shared, kind-generic resource UI already exists.** `` + (`web/src/components/shared/resource-list.ts`) renders templates *and* + harness-configs identically in both the project and hub surfaces — but it is + **read-only** today (lists + links to the detail/editor page; explicitly "does + not handle import/creation"). + +This document is the **reduced adaptation** of `hub-template-admin.md` against that +updated current state. It drops the standalone admin page, filters, sorting, +pagination, lock/unlock, and archive, and keeps only the two operations the user +asked for — **Clone** and **Delete** — wired into the resource list that already +ships in both surfaces. It also folds in a requested **verification that +re-importing from the same URL pulls fresh content** (§5), which turns out to +already hold and just needs a regression test to lock it in. + +### Why "reduced" + +The full admin page in `hub-template-admin.md` assumed no shared resource UI and no +import. Both now exist. Building a separate `/admin/templates` page would duplicate +the list that `` already renders in two places. Adding Clone and +Delete *to that shared list* gives the two highest-value management actions in both +the project and hub Resources views with no new page, no new list, and a small, +well-contained backend delta. + +--- + +## 2. Current State (as built) + +### 2.1 Backend — what already exists + +| Operation | Endpoint | Handler | Notes | +|-----------|----------|---------|-------| +| Delete template | `DELETE /api/v1/templates/{id}?deleteFiles=true&force=true` | `deleteTemplateV2` (`template_handlers.go:495`) | Deletes DB record; `deleteFiles=true` also `DeletePrefix`es storage; `force=true` required to delete a `Locked` template (see §2.4). | +| Clone template | `POST /api/v1/templates/{id}/clone` | `handleTemplateClone` (`template_handlers.go:693`) | Copies files via `stor.Copy`, sets `BaseTemplate = source.ID`, **destination scope/scopeId/name come from the request body** (`CloneTemplateRequest`) — so it can already clone across scopes. | +| Delete harness-config | `DELETE /api/v1/harness-configs/{id}?deleteFiles=true` | `deleteHarnessConfig` (`harness_config_handlers.go:~370`) | Deletes record; `deleteFiles=true` removes storage. Routed via `handleHarnessConfigByID` → CRUD switch (`harness_config_handlers.go:260`). | + +**Two backend deltas this work introduces (details in §4):** +- **No harness-config clone endpoint.** The `action` switch in `handleHarnessConfigByID` + (`harness_config_handlers.go:230`) handles `upload`, `finalize`, `download`, + `files/…` — but no `clone`. Templates get clone; harness-configs don't. +- **No resource-level authz on delete/clone.** See §2.5. + +### 2.2 Frontend — what already exists + +- `` (`resource-list.ts`, 251 lines) — shared, read-only list + used by **both** Project Settings → Resources and the Hub Resources page + (`settings.ts`). No row actions today. +- `` (`resource-import.ts`) — shared import form, mounted in + both surfaces. Emits `resource-imported` so the host refreshes the list. +- Resource detail/editor page — file browser + inline editor, reachable from each + list row via `detailBasePath`. + +So the surfaces, the shared list, and the import affordance are all in place. What's +missing is **row-level Clone/Delete actions** (and, for cross-scope clone, a +"clone from global" affordance in the project view — §4.2). + +### 2.3 Re-import freshness — current behavior (verified) + +Traced end-to-end for the "re-import the same URL" case: + +1. **Fetch layer** — `FetchRemoteTemplate` (`pkg/config/remote_templates.go:160`) + computes its cache key from the **URL only** (`generateCacheKey(uri)`), then + `os.RemoveAll(templateCachePath)` (`remote_templates.go:180`) **before** + re-downloading. Re-importing the same URL wipes the prior cached copy and pulls a + fresh tarball/checkout every time — there is no stale-cache reuse. +2. **Sync layer** — the import loop calls `ResourceStore.Bootstrap(..., force=true)` + (`pkg/hub/resource_import.go:390`). With `force=true`, Bootstrap **skips the + unchanged-hash short-circuit** (`resource_store.go:179`), re-uploads all files, + and calls `reconcileResourceStorage` to **drop objects no longer in the manifest** + so files removed upstream don't linger. It recomputes the content hash and flips + the record back to `active`. + +**Conclusion: re-importing from the same URL already pulls fresh content** — fresh +bytes at the fetch layer, forced re-upload + stale-file pruning at the sync layer. +This is a behavior to *protect with a test*, not a bug to fix (§5). + +### 2.4 The `Locked` flag is latent (never set today) + +`Locked` (`pkg/store/models.go:422` "Prevent modifications (global templates)") is +**read and enforced** in many places — template/harness-config update, PATCH, file +edit, and delete all reject when `Locked` is set +(`template_handlers.go:409,449,510`; `harness_config_handlers.go:292,330,388`; +`template_file_handlers.go:308,480,610`) — and it is persisted in SQLite. **But no +non-test code path ever sets `Locked = true`.** The CRUD/PATCH handlers only +*preserve* it (`template.Locked = existing.Locked`, `template_handlers.go:424`); the +bootstrap/import path doesn't set it; there is no lock/unlock endpoint. The only +assignments to `true` are in tests (`template_file_handlers_test.go:331,413,563`). + +**Implication for this work:** in practice a template/harness-config is never locked, +so the delete path's locked branch is currently unreachable. We keep delete honoring +the flag (and offer a force fallback, §4.2) as cheap, correct insurance for if/when a +lock-setter is added — but we add **no** lock/unlock UI, and the locked-state UX is +not a focus. (This is the answer to the reviewer's "how do templates get locked?" — +today, they don't.) + +### 2.5 Authz gap on delete/clone (to be closed) + +`/api/v1/*` is wrapped by the global auth **middleware** (`server.go:1880` +`applyMiddleware`), which establishes *authentication* (valid session/bearer). But +the existing `deleteTemplateV2`, `handleTemplateClone`, and `deleteHarnessConfig` +handlers contain **no resource-level `CheckAccess`** — so any authenticated user can +call them directly, regardless of scope or role. By contrast the newer import +endpoint (`handleResourcesImport`) *does* enforce authz (hub-admin for global scope, +project capability for project scope; covered by +`resource_import_handler_test.go`). This work closes that gap (§4.3). + +--- + +## 3. Goals & Non-Goals + +### Goals +- Add **Clone** and **Delete** row actions to the shared ``, so + both Project Settings → Resources and Hub Resources get them with one change. +- Add a **harness-config clone** endpoint mirroring the template clone, so the list's + Clone action works for both kinds. +- Support **cloning a global resource into a project** from the project Resources view + (cross-scope clone, global → project). The clone endpoints already accept a + destination scope/scopeId, so this is mostly UI plus the new harness-config clone. +- **Harden authz**: add `CheckAccess` to the delete and clone handlers, matching the + import endpoint's policy (hub-admin for global-scoped; project capability for + project-scoped). +- A **destructive-action confirmation** for Delete (with a "delete stored files" + checkbox **defaulting on**) and a small **name/destination dialog** for Clone. +- **Lock in re-import freshness** with regression tests (§5). + +### Non-Goals +- The standalone `/admin/templates` page, filters, sorting, pagination — dropped; the + shared list already covers listing. +- **Lock/Unlock** and **Archive** actions/UI. Delete still *honors* the `Locked` + flag, but per §2.4 nothing sets it today, so there is no UI to toggle it. +- A referenced-by / in-use guard on delete — **hard delete** is retained (§3.1 Q5). +- Bulk actions and template usage indicators. +- Any change to the import pipeline or the `ResourceStore` core. + +### 3.1 Resolved decisions (review with project owner) + +- **Q1 → Add authz.** Fold resource-level `CheckAccess` into delete + clone (both + kinds), matching the import endpoint policy (§4.3). Closes the gap in §2.5. +- **Q2 → `deleteFiles` default ON.** The delete dialog's "Also delete stored files" + checkbox defaults checked (clean delete that reclaims storage). +- **Q3 → Locked is latent; no lock UI.** Investigated: no non-test code sets `Locked` + (§2.4). Keep delete honoring the flag with a force-delete confirm fallback; add no + lock/unlock UI. +- **Q4 → Clone from global in the project view.** The project Resources view offers + cloning a **global** resource down into the current project (§4.2). Same-scope clone + (project→project, global→global) is also supported. +- **Q5 → Hard delete.** No referenced-by/in-use guard; delete is immediate with only + the generic "cannot be undone" warning. + +--- + +## 4. Design + +### 4.1 Backend: harness-config clone + shared clone request + +Add a `clone` action to `handleHarnessConfigByID` (`harness_config_handlers.go:230`): + +```go +case "clone": + s.handleHarnessConfigClone(w, r, hcID) +``` + +`handleHarnessConfigClone` mirrors `handleTemplateClone` (`template_handlers.go:693`): + +1. `POST` only; load the source via `GetHarnessConfig`. +2. Read `{ name, scope, scopeId, visibility }` (reuse the `CloneTemplateRequest` + shape; harness-configs also carry `Harness`, copied from the source). +3. Build a new record with a fresh UUID, `Slug = Slugify(name)`, copying + `Harness`/`Description`/`Config`, **destination scope/scopeId from the request** + (default to source scope when omitted), status `pending`. +4. Generate the clone's storage path and `stor.Copy` each source file to it. +5. Persist, set status `active`, return the new record. + +Endpoint: `POST /api/v1/harness-configs/{id}/clone` — dispatches through the existing +`action` switch, no route-table change. + +> **Delete needs no new endpoint** — `deleteTemplateV2` and `deleteHarnessConfig` +> already accept `deleteFiles` (templates also `force`). Only authz is added (§4.3). + +> **Cross-scope clone needs no new endpoint** — `handleTemplateClone` already takes +> the destination scope/scopeId from the body; the new harness-config clone does the +> same. The "clone from global into project" feature is the UI in §4.2 calling these +> with `scope=project, scopeId={projectId}` against a global-scoped source. + +### 4.2 Frontend: Clone + Delete on the shared list, and clone-from-global + +Add an actions affordance to each row in `` — an `sl-dropdown` +(or trailing icon buttons) with **Clone** and **Delete**. Both surfaces inherit it. + +New component state: `cloneTarget`, `deleteTarget`, `cloneFromGlobalOpen`, +`actionInProgress`, `actionError`. + +**Delete flow:** +- Click Delete → confirmation dialog (`sl-dialog`, `danger` confirm button). +- Shows name + kind + scope, an **"Also delete stored files" checkbox (checked by + default**, Q2), and an irreversible-action warning. No referenced-by check (Q5). +- Confirm → `DELETE /api/v1/{templates|harness-configs}/{id}?deleteFiles={checked}`. + On `204`, remove the row locally (or re-fetch) and emit `resource-changed`. +- **Locked fallback (latent, §2.4):** if the response is the locked-template + validation error, surface it and offer a "Force delete" confirm that retries with + `&force=true`. In practice this branch is unreachable today, but it's cheap and + correct. (Harness-configs have no lock concept.) + +**Clone flow (same-scope):** +- Click Clone → dialog prompting for the **new name** (prefilled `"{name}-copy"`); + destination defaults to the current list's scope/scopeId. +- Confirm → `POST /api/v1/{templates|harness-configs}/{id}/clone` with + `{ name, scope, scopeId }`. On success, re-fetch and emit `resource-changed`. + +**Clone-from-global (project view only, Q4):** +- The project Resources view gains a **"Clone from global"** affordance (a button + near the list / import form). It opens a picker listing **global** resources of the + current `kind` (via the existing list API with `scope=global`). +- Selecting one opens the same clone dialog with the **destination fixed to the + current project** (`scope=project, scopeId={projectId}`) and a prefilled name. +- Confirm → clone endpoint with the global source id and the project destination. The + cloned copy then appears in the project list (`BaseTemplate` tracks the global + source for templates). +- This is the cross-scope direction the owner asked for (pull a shared global resource + down into a project to customize it). The hub (global) view keeps same-scope clone + only. + +Endpoint/path selection keys off the existing `kind` property +(`'template' | 'harness-config'`). + +### 4.3 Authorization (Q1) + +Add `authzService.CheckAccess` to the delete and clone handlers, matching +`handleResourcesImport`: + +- **Global-scoped resource** (delete/clone of a global template or harness-config): + require **hub-admin** (the admin bypass / explicit hub-wide policy), the same check + the global import path uses. +- **Project-scoped resource:** require the caller's **project capability** for the + mutating action (mirror the per-project import authz via the shared + `authorizeProjectImport`-style helper) — `ActionDelete` for delete, `ActionCreate` + for clone. +- **Clone specifically** touches two scopes: it **reads** the source and **creates** + at the destination. Enforce read on the source's scope **and** create on the + destination scope. For clone-from-global → project: the source is a global resource + (world-readable to authenticated users for global resources, consistent with the + Hub Resources view) and the destination requires project create capability. + +This makes delete/clone consistent with import and removes the §2.5 gap. UI gating +(admin-only Hub Resources route) stays as defense-in-depth, but the backend is now +authoritative. + +--- + +## 5. Re-import freshness: verify & lock in + +Per §2.3 this **already works**. Add regression tests rather than a fix: + +- **`pkg/hub` integration test** (alongside `resource_import_handler_test.go`): + 1. Import a single-resource workspace dir (file `home/a.txt` = `"v1"`). + 2. Mutate the source: change `a.txt` → `"v2"`, add `b.txt`, delete an existing file. + 3. Re-import the **same source**. + 4. Assert the stored manifest reflects `v2`, includes `b.txt`, and **no longer + includes** the deleted file (exercises `reconcileResourceStorage`), and that the + record's `ContentHash` changed and status is `active`. +- **Remote-cache test** (`pkg/config`): assert `FetchRemoteTemplate` re-fetches for + the same URL — the cache dir is wiped/rewritten on the second call (guards the + `RemoveAll` + URL-keyed cache behavior at `remote_templates.go:175–180`). + +Both lock down the guarantee: "re-importing the same URL pulls fresh content, +including removals." + +--- + +## 6. Phases + +### Phase 1 — Backend: harness-config clone + authz hardening +- Add `handleHarnessConfigClone` + the `clone` action case; mirror template clone, + honoring destination scope/scopeId from the body. +- Add `CheckAccess` to `deleteTemplateV2`, `handleTemplateClone`, + `deleteHarnessConfig`, and the new harness-config clone (§4.3). +- Tests: clone copies files / sets new slug+scope / independent record; authz allows + admin + project-capable callers and 403s others (mirror the import handler tests). + +### Phase 2 — Frontend: Clone & Delete on the shared list + clone-from-global +- Add row actions, delete-confirm dialog (`deleteFiles` checked by default + force + retry for the latent locked case), and the same-scope clone dialog to + ``; wire both kinds; emit `resource-changed`. +- Add the project-view "Clone from global" picker → clone into the current project. +- Verify in both project and hub surfaces. + +### Phase 3 — Re-import freshness regression tests +- Add the `pkg/hub` re-import-mutation test and the `pkg/config` cache-refetch test + (§5). No production code change expected. + +--- + +## 7. Key Files + +| Area | File | +|------|------| +| Template delete / clone (reuse + add authz) | `pkg/hub/template_handlers.go` (`deleteTemplateV2:495`, `handleTemplateClone:693`) | +| Harness-config delete (add authz) + **new clone** | `pkg/hub/harness_config_handlers.go` (action switch `:230`, CRUD `:251`) | +| Authz reference (import) | `pkg/hub/handlers.go` (`handleResourcesImport`), `pkg/hub/resource_import_handler_test.go` | +| Re-import force-sync (verify) | `pkg/hub/resource_import.go` (`:390`), `pkg/hub/resource_store.go` (`Bootstrap:121`, reconcile) | +| Remote fetch cache (verify) | `pkg/config/remote_templates.go` (`FetchRemoteTemplate:160`, `RemoveAll:180`) | +| `Locked` model + enforcement (latent) | `pkg/store/models.go:422,517`; enforcement sites in `template_handlers.go`, `harness_config_handlers.go`, `template_file_handlers.go` | +| Shared list — **add actions** | `web/src/components/shared/resource-list.ts` | +| Host surfaces (inherit actions; project view adds clone-from-global) | `web/src/components/pages/project-settings.ts`, `web/src/components/pages/settings.ts` | +| Tests | `pkg/hub/resource_import_handler_test.go`, `pkg/config/*_test.go` | + +--- + +## 8. Relationship to `hub-template-admin.md` + +This doc implements the **Clone** and **Delete** actions from +`hub-template-admin.md` §2.4–2.5, scoped down to the shared resource list rather than +a new admin page, and reflecting that import (§2.6 there) is already shipped per +`resource-import-refactor.md`. It additionally **hardens delete/clone authz** (a gap +the parent doc assumed away) and confirms the `Locked` flag is currently latent. +Still deferred from the parent doc: the standalone admin page, filtering/sorting/ +pagination, lock/unlock toggle (and a mechanism to *set* `Locked`), archive, bulk +actions, and usage indicators. diff --git a/pkg/ent/harnessconfig.go b/pkg/ent/harnessconfig.go index 4e85897ce..05ab07115 100644 --- a/pkg/ent/harnessconfig.go +++ b/pkg/ent/harnessconfig.go @@ -44,8 +44,6 @@ type HarnessConfig struct { StoragePath string `json:"storage_path,omitempty"` // Files holds the value of the "files" field. Files string `json:"files,omitempty"` - // Locked holds the value of the "locked" field. - Locked bool `json:"locked,omitempty"` // Status holds the value of the "status" field. Status harnessconfig.Status `json:"status,omitempty"` // OwnerID holds the value of the "owner_id" field. @@ -68,8 +66,6 @@ func (*HarnessConfig) scanValues(columns []string) ([]any, error) { values := make([]any, len(columns)) for i := range columns { switch columns[i] { - case harnessconfig.FieldLocked: - values[i] = new(sql.NullBool) case harnessconfig.FieldName, harnessconfig.FieldSlug, harnessconfig.FieldDisplayName, harnessconfig.FieldDescription, harnessconfig.FieldHarness, harnessconfig.FieldConfig, harnessconfig.FieldContentHash, harnessconfig.FieldScope, harnessconfig.FieldScopeID, harnessconfig.FieldStorageURI, harnessconfig.FieldStorageBucket, harnessconfig.FieldStoragePath, harnessconfig.FieldFiles, harnessconfig.FieldStatus, harnessconfig.FieldOwnerID, harnessconfig.FieldCreatedBy, harnessconfig.FieldUpdatedBy, harnessconfig.FieldVisibility: values[i] = new(sql.NullString) case harnessconfig.FieldCreated, harnessconfig.FieldUpdated: @@ -175,12 +171,6 @@ func (_m *HarnessConfig) assignValues(columns []string, values []any) error { } else if value.Valid { _m.Files = value.String } - case harnessconfig.FieldLocked: - if value, ok := values[i].(*sql.NullBool); !ok { - return fmt.Errorf("unexpected type %T for field locked", values[i]) - } else if value.Valid { - _m.Locked = value.Bool - } case harnessconfig.FieldStatus: if value, ok := values[i].(*sql.NullString); !ok { return fmt.Errorf("unexpected type %T for field status", values[i]) @@ -298,9 +288,6 @@ func (_m *HarnessConfig) String() string { builder.WriteString("files=") builder.WriteString(_m.Files) builder.WriteString(", ") - builder.WriteString("locked=") - builder.WriteString(fmt.Sprintf("%v", _m.Locked)) - builder.WriteString(", ") builder.WriteString("status=") builder.WriteString(fmt.Sprintf("%v", _m.Status)) builder.WriteString(", ") diff --git a/pkg/ent/harnessconfig/harnessconfig.go b/pkg/ent/harnessconfig/harnessconfig.go index 9205933d7..988bd2e04 100644 --- a/pkg/ent/harnessconfig/harnessconfig.go +++ b/pkg/ent/harnessconfig/harnessconfig.go @@ -41,8 +41,6 @@ const ( FieldStoragePath = "storage_path" // FieldFiles holds the string denoting the files field in the database. FieldFiles = "files" - // FieldLocked holds the string denoting the locked field in the database. - FieldLocked = "locked" // FieldStatus holds the string denoting the status field in the database. FieldStatus = "status" // FieldOwnerID holds the string denoting the owner_id field in the database. @@ -77,7 +75,6 @@ var Columns = []string{ FieldStorageBucket, FieldStoragePath, FieldFiles, - FieldLocked, FieldStatus, FieldOwnerID, FieldCreatedBy, @@ -106,8 +103,6 @@ var ( HarnessValidator func(string) error // DefaultScope holds the default value on creation for the "scope" field. DefaultScope string - // DefaultLocked holds the default value on creation for the "locked" field. - DefaultLocked bool // DefaultVisibility holds the default value on creation for the "visibility" field. DefaultVisibility string // DefaultCreated holds the default value on creation for the "created" field. @@ -220,11 +215,6 @@ func ByFiles(opts ...sql.OrderTermOption) OrderOption { return sql.OrderByField(FieldFiles, opts...).ToFunc() } -// ByLocked orders the results by the locked field. -func ByLocked(opts ...sql.OrderTermOption) OrderOption { - return sql.OrderByField(FieldLocked, opts...).ToFunc() -} - // ByStatus orders the results by the status field. func ByStatus(opts ...sql.OrderTermOption) OrderOption { return sql.OrderByField(FieldStatus, opts...).ToFunc() diff --git a/pkg/ent/harnessconfig/where.go b/pkg/ent/harnessconfig/where.go index 24974e074..96312c474 100644 --- a/pkg/ent/harnessconfig/where.go +++ b/pkg/ent/harnessconfig/where.go @@ -120,11 +120,6 @@ func Files(v string) predicate.HarnessConfig { return predicate.HarnessConfig(sql.FieldEQ(FieldFiles, v)) } -// Locked applies equality check predicate on the "locked" field. It's identical to LockedEQ. -func Locked(v bool) predicate.HarnessConfig { - return predicate.HarnessConfig(sql.FieldEQ(FieldLocked, v)) -} - // OwnerID applies equality check predicate on the "owner_id" field. It's identical to OwnerIDEQ. func OwnerID(v string) predicate.HarnessConfig { return predicate.HarnessConfig(sql.FieldEQ(FieldOwnerID, v)) @@ -1090,16 +1085,6 @@ func FilesContainsFold(v string) predicate.HarnessConfig { return predicate.HarnessConfig(sql.FieldContainsFold(FieldFiles, v)) } -// LockedEQ applies the EQ predicate on the "locked" field. -func LockedEQ(v bool) predicate.HarnessConfig { - return predicate.HarnessConfig(sql.FieldEQ(FieldLocked, v)) -} - -// LockedNEQ applies the NEQ predicate on the "locked" field. -func LockedNEQ(v bool) predicate.HarnessConfig { - return predicate.HarnessConfig(sql.FieldNEQ(FieldLocked, v)) -} - // StatusEQ applies the EQ predicate on the "status" field. func StatusEQ(v Status) predicate.HarnessConfig { return predicate.HarnessConfig(sql.FieldEQ(FieldStatus, v)) diff --git a/pkg/ent/harnessconfig_create.go b/pkg/ent/harnessconfig_create.go index 7111759b8..fb2a3c5fa 100644 --- a/pkg/ent/harnessconfig_create.go +++ b/pkg/ent/harnessconfig_create.go @@ -182,20 +182,6 @@ func (_c *HarnessConfigCreate) SetNillableFiles(v *string) *HarnessConfigCreate return _c } -// SetLocked sets the "locked" field. -func (_c *HarnessConfigCreate) SetLocked(v bool) *HarnessConfigCreate { - _c.mutation.SetLocked(v) - return _c -} - -// SetNillableLocked sets the "locked" field if the given value is not nil. -func (_c *HarnessConfigCreate) SetNillableLocked(v *bool) *HarnessConfigCreate { - if v != nil { - _c.SetLocked(*v) - } - return _c -} - // SetStatus sets the "status" field. func (_c *HarnessConfigCreate) SetStatus(v harnessconfig.Status) *HarnessConfigCreate { _c.mutation.SetStatus(v) @@ -347,10 +333,6 @@ func (_c *HarnessConfigCreate) defaults() { v := harnessconfig.DefaultScope _c.mutation.SetScope(v) } - if _, ok := _c.mutation.Locked(); !ok { - v := harnessconfig.DefaultLocked - _c.mutation.SetLocked(v) - } if _, ok := _c.mutation.Status(); !ok { v := harnessconfig.DefaultStatus _c.mutation.SetStatus(v) @@ -402,9 +384,6 @@ func (_c *HarnessConfigCreate) check() error { if _, ok := _c.mutation.Scope(); !ok { return &ValidationError{Name: "scope", err: errors.New(`ent: missing required field "HarnessConfig.scope"`)} } - if _, ok := _c.mutation.Locked(); !ok { - return &ValidationError{Name: "locked", err: errors.New(`ent: missing required field "HarnessConfig.locked"`)} - } if _, ok := _c.mutation.Status(); !ok { return &ValidationError{Name: "status", err: errors.New(`ent: missing required field "HarnessConfig.status"`)} } @@ -510,10 +489,6 @@ func (_c *HarnessConfigCreate) createSpec() (*HarnessConfig, *sqlgraph.CreateSpe _spec.SetField(harnessconfig.FieldFiles, field.TypeString, value) _node.Files = value } - if value, ok := _c.mutation.Locked(); ok { - _spec.SetField(harnessconfig.FieldLocked, field.TypeBool, value) - _node.Locked = value - } if value, ok := _c.mutation.Status(); ok { _spec.SetField(harnessconfig.FieldStatus, field.TypeEnum, value) _node.Status = value @@ -804,18 +779,6 @@ func (u *HarnessConfigUpsert) ClearFiles() *HarnessConfigUpsert { return u } -// SetLocked sets the "locked" field. -func (u *HarnessConfigUpsert) SetLocked(v bool) *HarnessConfigUpsert { - u.Set(harnessconfig.FieldLocked, v) - return u -} - -// UpdateLocked sets the "locked" field to the value that was provided on create. -func (u *HarnessConfigUpsert) UpdateLocked() *HarnessConfigUpsert { - u.SetExcluded(harnessconfig.FieldLocked) - return u -} - // SetStatus sets the "status" field. func (u *HarnessConfigUpsert) SetStatus(v harnessconfig.Status) *HarnessConfigUpsert { u.Set(harnessconfig.FieldStatus, v) @@ -1202,20 +1165,6 @@ func (u *HarnessConfigUpsertOne) ClearFiles() *HarnessConfigUpsertOne { }) } -// SetLocked sets the "locked" field. -func (u *HarnessConfigUpsertOne) SetLocked(v bool) *HarnessConfigUpsertOne { - return u.Update(func(s *HarnessConfigUpsert) { - s.SetLocked(v) - }) -} - -// UpdateLocked sets the "locked" field to the value that was provided on create. -func (u *HarnessConfigUpsertOne) UpdateLocked() *HarnessConfigUpsertOne { - return u.Update(func(s *HarnessConfigUpsert) { - s.UpdateLocked() - }) -} - // SetStatus sets the "status" field. func (u *HarnessConfigUpsertOne) SetStatus(v harnessconfig.Status) *HarnessConfigUpsertOne { return u.Update(func(s *HarnessConfigUpsert) { @@ -1784,20 +1733,6 @@ func (u *HarnessConfigUpsertBulk) ClearFiles() *HarnessConfigUpsertBulk { }) } -// SetLocked sets the "locked" field. -func (u *HarnessConfigUpsertBulk) SetLocked(v bool) *HarnessConfigUpsertBulk { - return u.Update(func(s *HarnessConfigUpsert) { - s.SetLocked(v) - }) -} - -// UpdateLocked sets the "locked" field to the value that was provided on create. -func (u *HarnessConfigUpsertBulk) UpdateLocked() *HarnessConfigUpsertBulk { - return u.Update(func(s *HarnessConfigUpsert) { - s.UpdateLocked() - }) -} - // SetStatus sets the "status" field. func (u *HarnessConfigUpsertBulk) SetStatus(v harnessconfig.Status) *HarnessConfigUpsertBulk { return u.Update(func(s *HarnessConfigUpsert) { diff --git a/pkg/ent/harnessconfig_update.go b/pkg/ent/harnessconfig_update.go index 3e2319187..ac8379119 100644 --- a/pkg/ent/harnessconfig_update.go +++ b/pkg/ent/harnessconfig_update.go @@ -264,20 +264,6 @@ func (_u *HarnessConfigUpdate) ClearFiles() *HarnessConfigUpdate { return _u } -// SetLocked sets the "locked" field. -func (_u *HarnessConfigUpdate) SetLocked(v bool) *HarnessConfigUpdate { - _u.mutation.SetLocked(v) - return _u -} - -// SetNillableLocked sets the "locked" field if the given value is not nil. -func (_u *HarnessConfigUpdate) SetNillableLocked(v *bool) *HarnessConfigUpdate { - if v != nil { - _u.SetLocked(*v) - } - return _u -} - // SetStatus sets the "status" field. func (_u *HarnessConfigUpdate) SetStatus(v harnessconfig.Status) *HarnessConfigUpdate { _u.mutation.SetStatus(v) @@ -516,9 +502,6 @@ func (_u *HarnessConfigUpdate) sqlSave(ctx context.Context) (_node int, err erro if _u.mutation.FilesCleared() { _spec.ClearField(harnessconfig.FieldFiles, field.TypeString) } - if value, ok := _u.mutation.Locked(); ok { - _spec.SetField(harnessconfig.FieldLocked, field.TypeBool, value) - } if value, ok := _u.mutation.Status(); ok { _spec.SetField(harnessconfig.FieldStatus, field.TypeEnum, value) } @@ -802,20 +785,6 @@ func (_u *HarnessConfigUpdateOne) ClearFiles() *HarnessConfigUpdateOne { return _u } -// SetLocked sets the "locked" field. -func (_u *HarnessConfigUpdateOne) SetLocked(v bool) *HarnessConfigUpdateOne { - _u.mutation.SetLocked(v) - return _u -} - -// SetNillableLocked sets the "locked" field if the given value is not nil. -func (_u *HarnessConfigUpdateOne) SetNillableLocked(v *bool) *HarnessConfigUpdateOne { - if v != nil { - _u.SetLocked(*v) - } - return _u -} - // SetStatus sets the "status" field. func (_u *HarnessConfigUpdateOne) SetStatus(v harnessconfig.Status) *HarnessConfigUpdateOne { _u.mutation.SetStatus(v) @@ -1084,9 +1053,6 @@ func (_u *HarnessConfigUpdateOne) sqlSave(ctx context.Context) (_node *HarnessCo if _u.mutation.FilesCleared() { _spec.ClearField(harnessconfig.FieldFiles, field.TypeString) } - if value, ok := _u.mutation.Locked(); ok { - _spec.SetField(harnessconfig.FieldLocked, field.TypeBool, value) - } if value, ok := _u.mutation.Status(); ok { _spec.SetField(harnessconfig.FieldStatus, field.TypeEnum, value) } diff --git a/pkg/ent/migrate/schema.go b/pkg/ent/migrate/schema.go index a68e381ac..48fadfe08 100644 --- a/pkg/ent/migrate/schema.go +++ b/pkg/ent/migrate/schema.go @@ -400,7 +400,6 @@ var ( {Name: "storage_bucket", Type: field.TypeString, Nullable: true}, {Name: "storage_path", Type: field.TypeString, Nullable: true}, {Name: "files", Type: field.TypeString, Nullable: true}, - {Name: "locked", Type: field.TypeBool, Default: false}, {Name: "status", Type: field.TypeEnum, Enums: []string{"pending", "active", "archived"}, Default: "active"}, {Name: "owner_id", Type: field.TypeString, Nullable: true}, {Name: "created_by", Type: field.TypeString, Nullable: true}, @@ -416,9 +415,9 @@ var ( PrimaryKey: []*schema.Column{HarnessConfigsColumns[0]}, Indexes: []*schema.Index{ { - Name: "harnessconfig_slug_scope", - Unique: false, - Columns: []*schema.Column{HarnessConfigsColumns[2], HarnessConfigsColumns[8]}, + Name: "harnessconfig_slug_scope_scope_id", + Unique: true, + Columns: []*schema.Column{HarnessConfigsColumns[2], HarnessConfigsColumns[8], HarnessConfigsColumns[9]}, }, { Name: "harnessconfig_harness", @@ -428,7 +427,7 @@ var ( { Name: "harnessconfig_status", Unique: false, - Columns: []*schema.Column{HarnessConfigsColumns[15]}, + Columns: []*schema.Column{HarnessConfigsColumns[14]}, }, { Name: "harnessconfig_content_hash", @@ -934,7 +933,6 @@ var ( {Name: "storage_path", Type: field.TypeString, Nullable: true}, {Name: "files", Type: field.TypeString, Nullable: true}, {Name: "base_template", Type: field.TypeString, Nullable: true}, - {Name: "locked", Type: field.TypeBool, Default: false}, {Name: "status", Type: field.TypeEnum, Enums: []string{"pending", "active", "archived"}, Default: "active"}, {Name: "owner_id", Type: field.TypeString, Nullable: true}, {Name: "created_by", Type: field.TypeString, Nullable: true}, @@ -950,9 +948,9 @@ var ( PrimaryKey: []*schema.Column{TemplatesColumns[0]}, Indexes: []*schema.Index{ { - Name: "template_slug_scope", - Unique: false, - Columns: []*schema.Column{TemplatesColumns[2], TemplatesColumns[10]}, + Name: "template_slug_scope_scope_id", + Unique: true, + Columns: []*schema.Column{TemplatesColumns[2], TemplatesColumns[10], TemplatesColumns[11]}, }, { Name: "template_harness", @@ -962,7 +960,7 @@ var ( { Name: "template_status", Unique: false, - Columns: []*schema.Column{TemplatesColumns[19]}, + Columns: []*schema.Column{TemplatesColumns[18]}, }, { Name: "template_content_hash", diff --git a/pkg/ent/mutation.go b/pkg/ent/mutation.go index 514f2a0e8..da9f49f61 100644 --- a/pkg/ent/mutation.go +++ b/pkg/ent/mutation.go @@ -13137,7 +13137,6 @@ type HarnessConfigMutation struct { storage_bucket *string storage_path *string files *string - locked *bool status *harnessconfig.Status owner_id *string created_by *string @@ -13840,42 +13839,6 @@ func (m *HarnessConfigMutation) ResetFiles() { delete(m.clearedFields, harnessconfig.FieldFiles) } -// SetLocked sets the "locked" field. -func (m *HarnessConfigMutation) SetLocked(b bool) { - m.locked = &b -} - -// Locked returns the value of the "locked" field in the mutation. -func (m *HarnessConfigMutation) Locked() (r bool, exists bool) { - v := m.locked - if v == nil { - return - } - return *v, true -} - -// OldLocked returns the old "locked" field's value of the HarnessConfig entity. -// If the HarnessConfig object wasn't provided to the builder, the object is fetched from the database. -// An error is returned if the mutation operation is not UpdateOne, or the database query fails. -func (m *HarnessConfigMutation) OldLocked(ctx context.Context) (v bool, err error) { - if !m.op.Is(OpUpdateOne) { - return v, errors.New("OldLocked is only allowed on UpdateOne operations") - } - if m.id == nil || m.oldValue == nil { - return v, errors.New("OldLocked requires an ID field in the mutation") - } - oldValue, err := m.oldValue(ctx) - if err != nil { - return v, fmt.Errorf("querying old value for OldLocked: %w", err) - } - return oldValue.Locked, nil -} - -// ResetLocked resets all changes to the "locked" field. -func (m *HarnessConfigMutation) ResetLocked() { - m.locked = nil -} - // SetStatus sets the "status" field. func (m *HarnessConfigMutation) SetStatus(h harnessconfig.Status) { m.status = &h @@ -14201,7 +14164,7 @@ func (m *HarnessConfigMutation) Type() string { // order to get all numeric fields that were incremented/decremented, call // AddedFields(). func (m *HarnessConfigMutation) Fields() []string { - fields := make([]string, 0, 21) + fields := make([]string, 0, 20) if m.name != nil { fields = append(fields, harnessconfig.FieldName) } @@ -14241,9 +14204,6 @@ func (m *HarnessConfigMutation) Fields() []string { if m.files != nil { fields = append(fields, harnessconfig.FieldFiles) } - if m.locked != nil { - fields = append(fields, harnessconfig.FieldLocked) - } if m.status != nil { fields = append(fields, harnessconfig.FieldStatus) } @@ -14299,8 +14259,6 @@ func (m *HarnessConfigMutation) Field(name string) (ent.Value, bool) { return m.StoragePath() case harnessconfig.FieldFiles: return m.Files() - case harnessconfig.FieldLocked: - return m.Locked() case harnessconfig.FieldStatus: return m.Status() case harnessconfig.FieldOwnerID: @@ -14350,8 +14308,6 @@ func (m *HarnessConfigMutation) OldField(ctx context.Context, name string) (ent. return m.OldStoragePath(ctx) case harnessconfig.FieldFiles: return m.OldFiles(ctx) - case harnessconfig.FieldLocked: - return m.OldLocked(ctx) case harnessconfig.FieldStatus: return m.OldStatus(ctx) case harnessconfig.FieldOwnerID: @@ -14466,13 +14422,6 @@ func (m *HarnessConfigMutation) SetField(name string, value ent.Value) error { } m.SetFiles(v) return nil - case harnessconfig.FieldLocked: - v, ok := value.(bool) - if !ok { - return fmt.Errorf("unexpected type %T for field %s", value, name) - } - m.SetLocked(v) - return nil case harnessconfig.FieldStatus: v, ok := value.(harnessconfig.Status) if !ok { @@ -14685,9 +14634,6 @@ func (m *HarnessConfigMutation) ResetField(name string) error { case harnessconfig.FieldFiles: m.ResetFiles() return nil - case harnessconfig.FieldLocked: - m.ResetLocked() - return nil case harnessconfig.FieldStatus: m.ResetStatus() return nil @@ -29847,7 +29793,6 @@ type TemplateMutation struct { storage_path *string files *string base_template *string - locked *bool status *template.Status owner_id *string created_by *string @@ -30746,42 +30691,6 @@ func (m *TemplateMutation) ResetBaseTemplate() { delete(m.clearedFields, template.FieldBaseTemplate) } -// SetLocked sets the "locked" field. -func (m *TemplateMutation) SetLocked(b bool) { - m.locked = &b -} - -// Locked returns the value of the "locked" field in the mutation. -func (m *TemplateMutation) Locked() (r bool, exists bool) { - v := m.locked - if v == nil { - return - } - return *v, true -} - -// OldLocked returns the old "locked" field's value of the Template entity. -// If the Template object wasn't provided to the builder, the object is fetched from the database. -// An error is returned if the mutation operation is not UpdateOne, or the database query fails. -func (m *TemplateMutation) OldLocked(ctx context.Context) (v bool, err error) { - if !m.op.Is(OpUpdateOne) { - return v, errors.New("OldLocked is only allowed on UpdateOne operations") - } - if m.id == nil || m.oldValue == nil { - return v, errors.New("OldLocked requires an ID field in the mutation") - } - oldValue, err := m.oldValue(ctx) - if err != nil { - return v, fmt.Errorf("querying old value for OldLocked: %w", err) - } - return oldValue.Locked, nil -} - -// ResetLocked resets all changes to the "locked" field. -func (m *TemplateMutation) ResetLocked() { - m.locked = nil -} - // SetStatus sets the "status" field. func (m *TemplateMutation) SetStatus(t template.Status) { m.status = &t @@ -31107,7 +31016,7 @@ func (m *TemplateMutation) Type() string { // order to get all numeric fields that were incremented/decremented, call // AddedFields(). func (m *TemplateMutation) Fields() []string { - fields := make([]string, 0, 25) + fields := make([]string, 0, 24) if m.name != nil { fields = append(fields, template.FieldName) } @@ -31159,9 +31068,6 @@ func (m *TemplateMutation) Fields() []string { if m.base_template != nil { fields = append(fields, template.FieldBaseTemplate) } - if m.locked != nil { - fields = append(fields, template.FieldLocked) - } if m.status != nil { fields = append(fields, template.FieldStatus) } @@ -31225,8 +31131,6 @@ func (m *TemplateMutation) Field(name string) (ent.Value, bool) { return m.Files() case template.FieldBaseTemplate: return m.BaseTemplate() - case template.FieldLocked: - return m.Locked() case template.FieldStatus: return m.Status() case template.FieldOwnerID: @@ -31284,8 +31188,6 @@ func (m *TemplateMutation) OldField(ctx context.Context, name string) (ent.Value return m.OldFiles(ctx) case template.FieldBaseTemplate: return m.OldBaseTemplate(ctx) - case template.FieldLocked: - return m.OldLocked(ctx) case template.FieldStatus: return m.OldStatus(ctx) case template.FieldOwnerID: @@ -31428,13 +31330,6 @@ func (m *TemplateMutation) SetField(name string, value ent.Value) error { } m.SetBaseTemplate(v) return nil - case template.FieldLocked: - v, ok := value.(bool) - if !ok { - return fmt.Errorf("unexpected type %T for field %s", value, name) - } - m.SetLocked(v) - return nil case template.FieldStatus: v, ok := value.(template.Status) if !ok { @@ -31683,9 +31578,6 @@ func (m *TemplateMutation) ResetField(name string) error { case template.FieldBaseTemplate: m.ResetBaseTemplate() return nil - case template.FieldLocked: - m.ResetLocked() - return nil case template.FieldStatus: m.ResetStatus() return nil diff --git a/pkg/ent/runtime.go b/pkg/ent/runtime.go index 060050f6d..5f4320d11 100644 --- a/pkg/ent/runtime.go +++ b/pkg/ent/runtime.go @@ -386,20 +386,16 @@ func init() { harnessconfigDescScope := harnessconfigFields[8].Descriptor() // harnessconfig.DefaultScope holds the default value on creation for the scope field. harnessconfig.DefaultScope = harnessconfigDescScope.Default.(string) - // harnessconfigDescLocked is the schema descriptor for locked field. - harnessconfigDescLocked := harnessconfigFields[14].Descriptor() - // harnessconfig.DefaultLocked holds the default value on creation for the locked field. - harnessconfig.DefaultLocked = harnessconfigDescLocked.Default.(bool) // harnessconfigDescVisibility is the schema descriptor for visibility field. - harnessconfigDescVisibility := harnessconfigFields[19].Descriptor() + harnessconfigDescVisibility := harnessconfigFields[18].Descriptor() // harnessconfig.DefaultVisibility holds the default value on creation for the visibility field. harnessconfig.DefaultVisibility = harnessconfigDescVisibility.Default.(string) // harnessconfigDescCreated is the schema descriptor for created field. - harnessconfigDescCreated := harnessconfigFields[20].Descriptor() + harnessconfigDescCreated := harnessconfigFields[19].Descriptor() // harnessconfig.DefaultCreated holds the default value on creation for the created field. harnessconfig.DefaultCreated = harnessconfigDescCreated.Default.(func() time.Time) // harnessconfigDescUpdated is the schema descriptor for updated field. - harnessconfigDescUpdated := harnessconfigFields[21].Descriptor() + harnessconfigDescUpdated := harnessconfigFields[20].Descriptor() // harnessconfig.DefaultUpdated holds the default value on creation for the updated field. harnessconfig.DefaultUpdated = harnessconfigDescUpdated.Default.(func() time.Time) // harnessconfig.UpdateDefaultUpdated holds the default value on update for the updated field. @@ -860,20 +856,16 @@ func init() { templateDescScope := templateFields[10].Descriptor() // template.DefaultScope holds the default value on creation for the scope field. template.DefaultScope = templateDescScope.Default.(string) - // templateDescLocked is the schema descriptor for locked field. - templateDescLocked := templateFields[18].Descriptor() - // template.DefaultLocked holds the default value on creation for the locked field. - template.DefaultLocked = templateDescLocked.Default.(bool) // templateDescVisibility is the schema descriptor for visibility field. - templateDescVisibility := templateFields[23].Descriptor() + templateDescVisibility := templateFields[22].Descriptor() // template.DefaultVisibility holds the default value on creation for the visibility field. template.DefaultVisibility = templateDescVisibility.Default.(string) // templateDescCreated is the schema descriptor for created field. - templateDescCreated := templateFields[24].Descriptor() + templateDescCreated := templateFields[23].Descriptor() // template.DefaultCreated holds the default value on creation for the created field. template.DefaultCreated = templateDescCreated.Default.(func() time.Time) // templateDescUpdated is the schema descriptor for updated field. - templateDescUpdated := templateFields[25].Descriptor() + templateDescUpdated := templateFields[24].Descriptor() // template.DefaultUpdated holds the default value on creation for the updated field. template.DefaultUpdated = templateDescUpdated.Default.(func() time.Time) // template.UpdateDefaultUpdated holds the default value on update for the updated field. diff --git a/pkg/ent/schema/harnessconfig.go b/pkg/ent/schema/harnessconfig.go index 7856e992f..1814ae6ce 100644 --- a/pkg/ent/schema/harnessconfig.go +++ b/pkg/ent/schema/harnessconfig.go @@ -65,8 +65,6 @@ func (HarnessConfig) Fields() []ent.Field { Optional(), field.String("files"). Optional(), - field.Bool("locked"). - Default(false), field.Enum("status"). Values("pending", "active", "archived"). Default("active"), @@ -90,7 +88,7 @@ func (HarnessConfig) Fields() []ent.Field { // Indexes of the HarnessConfig. func (HarnessConfig) Indexes() []ent.Index { return []ent.Index{ - index.Fields("slug", "scope"), + index.Fields("slug", "scope", "scope_id").Unique(), index.Fields("harness"), index.Fields("status"), index.Fields("content_hash"), diff --git a/pkg/ent/schema/template.go b/pkg/ent/schema/template.go index 473c4469b..791e56d61 100644 --- a/pkg/ent/schema/template.go +++ b/pkg/ent/schema/template.go @@ -79,8 +79,6 @@ func (Template) Fields() []ent.Field { Optional(), field.String("base_template"). Optional(), - field.Bool("locked"). - Default(false), field.Enum("status"). Values("pending", "active", "archived"). Default("active"), @@ -104,7 +102,7 @@ func (Template) Fields() []ent.Field { // Indexes of the Template. func (Template) Indexes() []ent.Index { return []ent.Index{ - index.Fields("slug", "scope"), + index.Fields("slug", "scope", "scope_id").Unique(), index.Fields("harness"), index.Fields("status"), index.Fields("content_hash"), diff --git a/pkg/ent/template.go b/pkg/ent/template.go index eed77d8d0..e54da9747 100644 --- a/pkg/ent/template.go +++ b/pkg/ent/template.go @@ -52,8 +52,6 @@ type Template struct { Files string `json:"files,omitempty"` // BaseTemplate holds the value of the "base_template" field. BaseTemplate string `json:"base_template,omitempty"` - // Locked holds the value of the "locked" field. - Locked bool `json:"locked,omitempty"` // Status holds the value of the "status" field. Status template.Status `json:"status,omitempty"` // OwnerID holds the value of the "owner_id" field. @@ -76,8 +74,6 @@ func (*Template) scanValues(columns []string) ([]any, error) { values := make([]any, len(columns)) for i := range columns { switch columns[i] { - case template.FieldLocked: - values[i] = new(sql.NullBool) case template.FieldName, template.FieldSlug, template.FieldDisplayName, template.FieldDescription, template.FieldHarness, template.FieldDefaultHarnessConfig, template.FieldImage, template.FieldConfig, template.FieldContentHash, template.FieldScope, template.FieldScopeID, template.FieldProjectID, template.FieldStorageURI, template.FieldStorageBucket, template.FieldStoragePath, template.FieldFiles, template.FieldBaseTemplate, template.FieldStatus, template.FieldOwnerID, template.FieldCreatedBy, template.FieldUpdatedBy, template.FieldVisibility: values[i] = new(sql.NullString) case template.FieldCreated, template.FieldUpdated: @@ -207,12 +203,6 @@ func (_m *Template) assignValues(columns []string, values []any) error { } else if value.Valid { _m.BaseTemplate = value.String } - case template.FieldLocked: - if value, ok := values[i].(*sql.NullBool); !ok { - return fmt.Errorf("unexpected type %T for field locked", values[i]) - } else if value.Valid { - _m.Locked = value.Bool - } case template.FieldStatus: if value, ok := values[i].(*sql.NullString); !ok { return fmt.Errorf("unexpected type %T for field status", values[i]) @@ -342,9 +332,6 @@ func (_m *Template) String() string { builder.WriteString("base_template=") builder.WriteString(_m.BaseTemplate) builder.WriteString(", ") - builder.WriteString("locked=") - builder.WriteString(fmt.Sprintf("%v", _m.Locked)) - builder.WriteString(", ") builder.WriteString("status=") builder.WriteString(fmt.Sprintf("%v", _m.Status)) builder.WriteString(", ") diff --git a/pkg/ent/template/template.go b/pkg/ent/template/template.go index 13052caa1..1da8b5ce9 100644 --- a/pkg/ent/template/template.go +++ b/pkg/ent/template/template.go @@ -49,8 +49,6 @@ const ( FieldFiles = "files" // FieldBaseTemplate holds the string denoting the base_template field in the database. FieldBaseTemplate = "base_template" - // FieldLocked holds the string denoting the locked field in the database. - FieldLocked = "locked" // FieldStatus holds the string denoting the status field in the database. FieldStatus = "status" // FieldOwnerID holds the string denoting the owner_id field in the database. @@ -89,7 +87,6 @@ var Columns = []string{ FieldStoragePath, FieldFiles, FieldBaseTemplate, - FieldLocked, FieldStatus, FieldOwnerID, FieldCreatedBy, @@ -116,8 +113,6 @@ var ( SlugValidator func(string) error // DefaultScope holds the default value on creation for the "scope" field. DefaultScope string - // DefaultLocked holds the default value on creation for the "locked" field. - DefaultLocked bool // DefaultVisibility holds the default value on creation for the "visibility" field. DefaultVisibility string // DefaultCreated holds the default value on creation for the "created" field. @@ -250,11 +245,6 @@ func ByBaseTemplate(opts ...sql.OrderTermOption) OrderOption { return sql.OrderByField(FieldBaseTemplate, opts...).ToFunc() } -// ByLocked orders the results by the locked field. -func ByLocked(opts ...sql.OrderTermOption) OrderOption { - return sql.OrderByField(FieldLocked, opts...).ToFunc() -} - // ByStatus orders the results by the status field. func ByStatus(opts ...sql.OrderTermOption) OrderOption { return sql.OrderByField(FieldStatus, opts...).ToFunc() diff --git a/pkg/ent/template/where.go b/pkg/ent/template/where.go index 659342758..5a59a0bea 100644 --- a/pkg/ent/template/where.go +++ b/pkg/ent/template/where.go @@ -140,11 +140,6 @@ func BaseTemplate(v string) predicate.Template { return predicate.Template(sql.FieldEQ(FieldBaseTemplate, v)) } -// Locked applies equality check predicate on the "locked" field. It's identical to LockedEQ. -func Locked(v bool) predicate.Template { - return predicate.Template(sql.FieldEQ(FieldLocked, v)) -} - // OwnerID applies equality check predicate on the "owner_id" field. It's identical to OwnerIDEQ. func OwnerID(v string) predicate.Template { return predicate.Template(sql.FieldEQ(FieldOwnerID, v)) @@ -1410,16 +1405,6 @@ func BaseTemplateContainsFold(v string) predicate.Template { return predicate.Template(sql.FieldContainsFold(FieldBaseTemplate, v)) } -// LockedEQ applies the EQ predicate on the "locked" field. -func LockedEQ(v bool) predicate.Template { - return predicate.Template(sql.FieldEQ(FieldLocked, v)) -} - -// LockedNEQ applies the NEQ predicate on the "locked" field. -func LockedNEQ(v bool) predicate.Template { - return predicate.Template(sql.FieldNEQ(FieldLocked, v)) -} - // StatusEQ applies the EQ predicate on the "status" field. func StatusEQ(v Status) predicate.Template { return predicate.Template(sql.FieldEQ(FieldStatus, v)) diff --git a/pkg/ent/template_create.go b/pkg/ent/template_create.go index 24dc33edd..3a1b882d9 100644 --- a/pkg/ent/template_create.go +++ b/pkg/ent/template_create.go @@ -238,20 +238,6 @@ func (_c *TemplateCreate) SetNillableBaseTemplate(v *string) *TemplateCreate { return _c } -// SetLocked sets the "locked" field. -func (_c *TemplateCreate) SetLocked(v bool) *TemplateCreate { - _c.mutation.SetLocked(v) - return _c -} - -// SetNillableLocked sets the "locked" field if the given value is not nil. -func (_c *TemplateCreate) SetNillableLocked(v *bool) *TemplateCreate { - if v != nil { - _c.SetLocked(*v) - } - return _c -} - // SetStatus sets the "status" field. func (_c *TemplateCreate) SetStatus(v template.Status) *TemplateCreate { _c.mutation.SetStatus(v) @@ -403,10 +389,6 @@ func (_c *TemplateCreate) defaults() { v := template.DefaultScope _c.mutation.SetScope(v) } - if _, ok := _c.mutation.Locked(); !ok { - v := template.DefaultLocked - _c.mutation.SetLocked(v) - } if _, ok := _c.mutation.Status(); !ok { v := template.DefaultStatus _c.mutation.SetStatus(v) @@ -453,9 +435,6 @@ func (_c *TemplateCreate) check() error { if _, ok := _c.mutation.Scope(); !ok { return &ValidationError{Name: "scope", err: errors.New(`ent: missing required field "Template.scope"`)} } - if _, ok := _c.mutation.Locked(); !ok { - return &ValidationError{Name: "locked", err: errors.New(`ent: missing required field "Template.locked"`)} - } if _, ok := _c.mutation.Status(); !ok { return &ValidationError{Name: "status", err: errors.New(`ent: missing required field "Template.status"`)} } @@ -577,10 +556,6 @@ func (_c *TemplateCreate) createSpec() (*Template, *sqlgraph.CreateSpec) { _spec.SetField(template.FieldBaseTemplate, field.TypeString, value) _node.BaseTemplate = value } - if value, ok := _c.mutation.Locked(); ok { - _spec.SetField(template.FieldLocked, field.TypeBool, value) - _node.Locked = value - } if value, ok := _c.mutation.Status(); ok { _spec.SetField(template.FieldStatus, field.TypeEnum, value) _node.Status = value @@ -943,18 +918,6 @@ func (u *TemplateUpsert) ClearBaseTemplate() *TemplateUpsert { return u } -// SetLocked sets the "locked" field. -func (u *TemplateUpsert) SetLocked(v bool) *TemplateUpsert { - u.Set(template.FieldLocked, v) - return u -} - -// UpdateLocked sets the "locked" field to the value that was provided on create. -func (u *TemplateUpsert) UpdateLocked() *TemplateUpsert { - u.SetExcluded(template.FieldLocked) - return u -} - // SetStatus sets the "status" field. func (u *TemplateUpsert) SetStatus(v template.Status) *TemplateUpsert { u.Set(template.FieldStatus, v) @@ -1425,20 +1388,6 @@ func (u *TemplateUpsertOne) ClearBaseTemplate() *TemplateUpsertOne { }) } -// SetLocked sets the "locked" field. -func (u *TemplateUpsertOne) SetLocked(v bool) *TemplateUpsertOne { - return u.Update(func(s *TemplateUpsert) { - s.SetLocked(v) - }) -} - -// UpdateLocked sets the "locked" field to the value that was provided on create. -func (u *TemplateUpsertOne) UpdateLocked() *TemplateUpsertOne { - return u.Update(func(s *TemplateUpsert) { - s.UpdateLocked() - }) -} - // SetStatus sets the "status" field. func (u *TemplateUpsertOne) SetStatus(v template.Status) *TemplateUpsertOne { return u.Update(func(s *TemplateUpsert) { @@ -2091,20 +2040,6 @@ func (u *TemplateUpsertBulk) ClearBaseTemplate() *TemplateUpsertBulk { }) } -// SetLocked sets the "locked" field. -func (u *TemplateUpsertBulk) SetLocked(v bool) *TemplateUpsertBulk { - return u.Update(func(s *TemplateUpsert) { - s.SetLocked(v) - }) -} - -// UpdateLocked sets the "locked" field to the value that was provided on create. -func (u *TemplateUpsertBulk) UpdateLocked() *TemplateUpsertBulk { - return u.Update(func(s *TemplateUpsert) { - s.UpdateLocked() - }) -} - // SetStatus sets the "status" field. func (u *TemplateUpsertBulk) SetStatus(v template.Status) *TemplateUpsertBulk { return u.Update(func(s *TemplateUpsert) { diff --git a/pkg/ent/template_update.go b/pkg/ent/template_update.go index c8d12df0c..9fc883aea 100644 --- a/pkg/ent/template_update.go +++ b/pkg/ent/template_update.go @@ -344,20 +344,6 @@ func (_u *TemplateUpdate) ClearBaseTemplate() *TemplateUpdate { return _u } -// SetLocked sets the "locked" field. -func (_u *TemplateUpdate) SetLocked(v bool) *TemplateUpdate { - _u.mutation.SetLocked(v) - return _u -} - -// SetNillableLocked sets the "locked" field if the given value is not nil. -func (_u *TemplateUpdate) SetNillableLocked(v *bool) *TemplateUpdate { - if v != nil { - _u.SetLocked(*v) - } - return _u -} - // SetStatus sets the "status" field. func (_u *TemplateUpdate) SetStatus(v template.Status) *TemplateUpdate { _u.mutation.SetStatus(v) @@ -615,9 +601,6 @@ func (_u *TemplateUpdate) sqlSave(ctx context.Context) (_node int, err error) { if _u.mutation.BaseTemplateCleared() { _spec.ClearField(template.FieldBaseTemplate, field.TypeString) } - if value, ok := _u.mutation.Locked(); ok { - _spec.SetField(template.FieldLocked, field.TypeBool, value) - } if value, ok := _u.mutation.Status(); ok { _spec.SetField(template.FieldStatus, field.TypeEnum, value) } @@ -981,20 +964,6 @@ func (_u *TemplateUpdateOne) ClearBaseTemplate() *TemplateUpdateOne { return _u } -// SetLocked sets the "locked" field. -func (_u *TemplateUpdateOne) SetLocked(v bool) *TemplateUpdateOne { - _u.mutation.SetLocked(v) - return _u -} - -// SetNillableLocked sets the "locked" field if the given value is not nil. -func (_u *TemplateUpdateOne) SetNillableLocked(v *bool) *TemplateUpdateOne { - if v != nil { - _u.SetLocked(*v) - } - return _u -} - // SetStatus sets the "status" field. func (_u *TemplateUpdateOne) SetStatus(v template.Status) *TemplateUpdateOne { _u.mutation.SetStatus(v) @@ -1282,9 +1251,6 @@ func (_u *TemplateUpdateOne) sqlSave(ctx context.Context) (_node *Template, err if _u.mutation.BaseTemplateCleared() { _spec.ClearField(template.FieldBaseTemplate, field.TypeString) } - if value, ok := _u.mutation.Locked(); ok { - _spec.SetField(template.FieldLocked, field.TypeBool, value) - } if value, ok := _u.mutation.Status(); ok { _spec.SetField(template.FieldStatus, field.TypeEnum, value) } diff --git a/pkg/hub/clone_delete_handler_test.go b/pkg/hub/clone_delete_handler_test.go new file mode 100644 index 000000000..a308cd080 --- /dev/null +++ b/pkg/hub/clone_delete_handler_test.go @@ -0,0 +1,206 @@ +//go:build !no_sqlite + +package hub + +import ( + "context" + "encoding/json" + "net/http" + "testing" + "time" + + "github.com/GoogleCloudPlatform/scion/pkg/api" + "github.com/GoogleCloudPlatform/scion/pkg/store" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// setupCloneTestServer returns a Server with mock storage, a store, and +// pre-created source harness config and template IDs suitable for clone tests. +func setupCloneTestServer(t *testing.T) (*Server, store.Store, string, string) { + t.Helper() + srv, s := testServer(t) + ctx := context.Background() + + stor := newMockStorage("test-bucket") + srv.SetStorage(stor) + + now := time.Now() + + hcID := api.NewUUID() + tplID := api.NewUUID() + + // Seed a source harness config (global). + require.NoError(t, s.CreateHarnessConfig(ctx, &store.HarnessConfig{ + ID: hcID, Slug: "source-hc", Name: "Source HC", + DisplayName: "Source Display", Description: "Source desc", + Harness: "claude", + Config: &store.HarnessConfigData{Harness: "claude", Image: "img:latest"}, + Scope: store.HarnessConfigScopeGlobal, + Visibility: store.VisibilityPublic, + Status: store.HarnessConfigStatusActive, + Created: now, Updated: now, + })) + + // Seed a source template (global). + require.NoError(t, s.CreateTemplate(ctx, &store.Template{ + ID: tplID, Slug: "source-tpl", Name: "Source Template", + DisplayName: "TPL Display", Description: "TPL desc", + Harness: "claude", + Scope: store.TemplateScopeGlobal, + Visibility: store.VisibilityPublic, + Status: store.TemplateStatusActive, + Created: now, Updated: now, + })) + + return srv, s, hcID, tplID +} + +func TestHandleHarnessConfigClone_Success(t *testing.T) { + srv, _, hcID, _ := setupCloneTestServer(t) + + body := map[string]interface{}{ + "name": "My Clone", + "scope": "global", + "visibility": "private", + } + + rec := doRequest(t, srv, http.MethodPost, "/api/v1/harness-configs/"+hcID+"/clone", body) + require.Equal(t, http.StatusCreated, rec.Code, rec.Body.String()) + + var clone store.HarnessConfig + require.NoError(t, json.NewDecoder(rec.Body).Decode(&clone)) + + assert.NotEqual(t, hcID, clone.ID, "clone must have a new ID") + assert.Equal(t, "my-clone", clone.Slug) + assert.Equal(t, "My Clone", clone.Name) + assert.Equal(t, "Source Display", clone.DisplayName) + assert.Equal(t, "Source desc", clone.Description) + assert.Equal(t, "claude", clone.Harness) + assert.Equal(t, "global", clone.Scope) + assert.Equal(t, "private", clone.Visibility) + assert.NotNil(t, clone.Config) +} + +func TestHandleHarnessConfigClone_CrossScope(t *testing.T) { + srv, s, hcID, _ := setupCloneTestServer(t) + ctx := context.Background() + + projectID := api.NewUUID() + require.NoError(t, s.CreateProject(ctx, &store.Project{ + ID: projectID, Name: "Clone Project", Slug: "clone-project", + OwnerID: DevUserID, CreatedBy: DevUserID, + Created: time.Now(), Updated: time.Now(), + })) + + body := map[string]interface{}{ + "name": "Project Clone", + "scope": "project", + "scopeId": projectID, + } + + rec := doRequest(t, srv, http.MethodPost, "/api/v1/harness-configs/"+hcID+"/clone", body) + require.Equal(t, http.StatusCreated, rec.Code, rec.Body.String()) + + var clone store.HarnessConfig + require.NoError(t, json.NewDecoder(rec.Body).Decode(&clone)) + + assert.Equal(t, "project", clone.Scope) + assert.Equal(t, projectID, clone.ScopeID) + assert.Equal(t, "claude", clone.Harness) +} + +func TestDeleteTemplate_Authz_GlobalForbiddenForMember(t *testing.T) { + srv, s, _, tplID := setupCloneTestServer(t) + ctx := context.Background() + + member := &store.User{ + ID: api.NewUUID(), Email: "member-del@test.com", + DisplayName: "Member", Role: store.UserRoleMember, + Status: "active", Created: time.Now(), + } + require.NoError(t, s.CreateUser(ctx, member)) + ensureHubMembership(ctx, s, member.ID) + + rec := doRequestAsUser(t, srv, member, http.MethodDelete, "/api/v1/templates/"+tplID, nil) + assert.Equal(t, http.StatusForbidden, rec.Code, "non-admin should get 403 on global template delete: %s", rec.Body.String()) +} + +func TestDeleteTemplate_Authz_GlobalAllowedForAdmin(t *testing.T) { + srv, s, _, tplID := setupCloneTestServer(t) + ctx := context.Background() + + admin := &store.User{ + ID: api.NewUUID(), Email: "admin-del@test.com", + DisplayName: "Admin", Role: store.UserRoleAdmin, + Status: "active", Created: time.Now(), + } + require.NoError(t, s.CreateUser(ctx, admin)) + ensureHubMembership(ctx, s, admin.ID) + + rec := doRequestAsUser(t, srv, admin, http.MethodDelete, "/api/v1/templates/"+tplID, nil) + assert.Equal(t, http.StatusNoContent, rec.Code, "admin should be able to delete global template: %s", rec.Body.String()) + + // Verify gone. + rec = doRequestAsUser(t, srv, admin, http.MethodGet, "/api/v1/templates/"+tplID, nil) + assert.Equal(t, http.StatusNotFound, rec.Code) +} + +func TestCloneTemplate_Authz_DestinationChecked(t *testing.T) { + srv, s, _, tplID := setupCloneTestServer(t) + ctx := context.Background() + + alice := &store.User{ + ID: api.NewUUID(), Email: "alice-clone@test.com", + DisplayName: "Alice", Role: store.UserRoleMember, + Status: "active", Created: time.Now(), + } + bob := &store.User{ + ID: api.NewUUID(), Email: "bob-clone@test.com", + DisplayName: "Bob", Role: store.UserRoleMember, + Status: "active", Created: time.Now(), + } + require.NoError(t, s.CreateUser(ctx, alice)) + require.NoError(t, s.CreateUser(ctx, bob)) + ensureHubMembership(ctx, s, alice.ID) + ensureHubMembership(ctx, s, bob.ID) + + project := &store.Project{ + ID: api.NewUUID(), Name: "Authz Project", Slug: "authz-project", + OwnerID: alice.ID, CreatedBy: alice.ID, + Created: time.Now(), Updated: time.Now(), + } + require.NoError(t, s.CreateProject(ctx, project)) + srv.createProjectMembersGroupAndPolicy(ctx, project) + + body := map[string]interface{}{ + "name": "Clone Into Project", + "scope": "project", + "scopeId": project.ID, + } + + // Bob is not a project member → should be forbidden. + rec := doRequestAsUser(t, srv, bob, http.MethodPost, "/api/v1/templates/"+tplID+"/clone", body) + assert.Equal(t, http.StatusForbidden, rec.Code, "non-member should get 403: %s", rec.Body.String()) + + // Alice is the project owner → should succeed. + rec = doRequestAsUser(t, srv, alice, http.MethodPost, "/api/v1/templates/"+tplID+"/clone", body) + assert.Equal(t, http.StatusCreated, rec.Code, "project owner should be able to clone: %s", rec.Body.String()) +} + +func TestClone_SlugCollision_Returns409(t *testing.T) { + srv, _, hcID, _ := setupCloneTestServer(t) + + body := map[string]interface{}{ + "name": "Collision Clone", + "scope": "global", + } + + // First clone succeeds. + rec := doRequest(t, srv, http.MethodPost, "/api/v1/harness-configs/"+hcID+"/clone", body) + require.Equal(t, http.StatusCreated, rec.Code, rec.Body.String()) + + // Second clone with same name → slug collision → 409. + rec = doRequest(t, srv, http.MethodPost, "/api/v1/harness-configs/"+hcID+"/clone", body) + assert.Equal(t, http.StatusConflict, rec.Code, "duplicate slug should return 409: %s", rec.Body.String()) +} diff --git a/pkg/hub/harness_config_file_handlers.go b/pkg/hub/harness_config_file_handlers.go index 769d81def..01cfb66a1 100644 --- a/pkg/hub/harness_config_file_handlers.go +++ b/pkg/hub/harness_config_file_handlers.go @@ -174,11 +174,6 @@ func (s *Server) handleHarnessConfigFileWrite(w http.ResponseWriter, r *http.Req writeErrorFromErr(w, err, "") return } - if hc.Locked { - Forbidden(w) - return - } - // Limit request body size for both JSON and raw content paths. r.Body = http.MaxBytesReader(w, r.Body, maxHarnessConfigFileSize+4096) @@ -273,11 +268,6 @@ func (s *Server) handleHarnessConfigFileUpload(w http.ResponseWriter, r *http.Re writeErrorFromErr(w, err, "") return } - if hc.Locked { - Forbidden(w) - return - } - // Apply total request body size limit r.Body = http.MaxBytesReader(w, r.Body, maxUploadTotalSize) @@ -391,11 +381,6 @@ func (s *Server) handleHarnessConfigFileDelete(w http.ResponseWriter, r *http.Re writeErrorFromErr(w, err, "") return } - if hc.Locked { - Forbidden(w) - return - } - stor := s.GetStorage() if stor == nil { RuntimeError(w, "Storage not configured") diff --git a/pkg/hub/harness_config_handlers.go b/pkg/hub/harness_config_handlers.go index 5a2f9e457..b385af2dd 100644 --- a/pkg/hub/harness_config_handlers.go +++ b/pkg/hub/harness_config_handlers.go @@ -236,6 +236,8 @@ func (s *Server) handleHarnessConfigByID(w http.ResponseWriter, r *http.Request) s.handleHarnessConfigFinalize(w, r, hcID) case "download": s.handleHarnessConfigDownload(w, r, hcID) + case "clone": + s.handleHarnessConfigClone(w, r, hcID) case "files": s.handleHarnessConfigFiles(w, r, hcID, "") default: @@ -289,11 +291,6 @@ func (s *Server) updateHarnessConfig(w http.ResponseWriter, r *http.Request, id return } - if existing.Locked { - ValidationError(w, "harness config is locked and cannot be modified", nil) - return - } - var hc store.HarnessConfig if err := readJSON(r, &hc); err != nil { BadRequest(w, "Invalid request body: "+err.Error()) @@ -304,8 +301,6 @@ func (s *Server) updateHarnessConfig(w http.ResponseWriter, r *http.Request, id hc.ID = existing.ID hc.Created = existing.Created hc.CreatedBy = existing.CreatedBy - hc.Locked = existing.Locked - if hc.Slug == "" { hc.Slug = api.Slugify(hc.Name) } @@ -327,11 +322,6 @@ func (s *Server) patchHarnessConfig(w http.ResponseWriter, r *http.Request, id s return } - if existing.Locked { - ValidationError(w, "harness config is locked and cannot be modified", nil) - return - } - var updates struct { Name string `json:"name,omitempty"` Slug string `json:"slug,omitempty"` @@ -377,7 +367,6 @@ func (s *Server) deleteHarnessConfig(w http.ResponseWriter, r *http.Request, id query := r.URL.Query() deleteFiles := query.Get("deleteFiles") == "true" - force := query.Get("force") == "true" existing, err := s.store.GetHarnessConfig(ctx, id) if err != nil { @@ -385,9 +374,40 @@ func (s *Server) deleteHarnessConfig(w http.ResponseWriter, r *http.Request, id return } - if existing.Locked && !force { - ValidationError(w, "harness config is locked; use force=true to delete", nil) - return + // Authorize: check source scope for ActionDelete + if existing.Scope == store.HarnessConfigScopeGlobal { + userIdent := GetUserIdentityFromContext(ctx) + if userIdent == nil { + writeError(w, http.StatusUnauthorized, "unauthorized", "Authentication required", nil) + return + } + decision := s.authzService.CheckAccess(ctx, userIdent, Resource{Type: "harness_config"}, ActionDelete) + if !decision.Allowed { + writeError(w, http.StatusForbidden, ErrCodeForbidden, "You do not have permission to delete global resources", nil) + return + } + } else if existing.Scope == store.HarnessConfigScopeProject { + if agentIdent := GetAgentIdentityFromContext(ctx); agentIdent != nil { + if !agentIdent.HasScope(ScopeAgentCreate) { + writeError(w, http.StatusForbidden, ErrCodeForbidden, "Missing required scope", nil) + return + } + if existing.ScopeID != agentIdent.ProjectID() { + writeError(w, http.StatusForbidden, ErrCodeForbidden, "Agents can only manage resources within their own project", nil) + return + } + } else if userIdent := GetUserIdentityFromContext(ctx); userIdent != nil { + decision := s.authzService.CheckAccess(ctx, userIdent, Resource{ + Type: "harness_config", ParentType: "project", ParentID: existing.ScopeID, + }, ActionDelete) + if !decision.Allowed { + writeError(w, http.StatusForbidden, ErrCodeForbidden, "You do not have permission to delete resources in this project", nil) + return + } + } else { + writeError(w, http.StatusUnauthorized, "unauthorized", "Authentication required", nil) + return + } } if deleteFiles && existing.StoragePath != "" { @@ -543,3 +563,135 @@ func (s *Server) handleHarnessConfigDownload(w http.ResponseWriter, r *http.Requ Expires: expires, }) } + +// handleHarnessConfigClone creates a copy of a harness config. +func (s *Server) handleHarnessConfigClone(w http.ResponseWriter, r *http.Request, id string) { + if r.Method != http.MethodPost { + MethodNotAllowed(w) + return + } + + ctx := r.Context() + + source, err := s.store.GetHarnessConfig(ctx, id) + if err != nil { + writeErrorFromErr(w, err, "") + return + } + + var req CloneTemplateRequest + if err := readJSON(r, &req); err != nil { + BadRequest(w, "Invalid request body: "+err.Error()) + return + } + + if req.Name == "" { + ValidationError(w, "name is required", nil) + return + } + + // Resolve scope ID + scopeID := req.ScopeID + if scopeID == "" && req.ProjectID != "" { + scopeID = req.ProjectID + } + + // Authorize: check destination scope for ActionCreate + destScope := req.Scope + if destScope == "" { + destScope = source.Scope + } + if destScope == "" { + destScope = store.HarnessConfigScopeGlobal + } + if destScope == store.HarnessConfigScopeGlobal { + userIdent := GetUserIdentityFromContext(ctx) + if userIdent == nil { + writeError(w, http.StatusUnauthorized, "unauthorized", "Authentication required", nil) + return + } + decision := s.authzService.CheckAccess(ctx, userIdent, Resource{Type: "harness_config"}, ActionCreate) + if !decision.Allowed { + writeError(w, http.StatusForbidden, ErrCodeForbidden, "You do not have permission to create global resources", nil) + return + } + } else if destScope == store.HarnessConfigScopeProject { + if agentIdent := GetAgentIdentityFromContext(ctx); agentIdent != nil { + if !agentIdent.HasScope(ScopeAgentCreate) { + writeError(w, http.StatusForbidden, ErrCodeForbidden, "Missing required scope", nil) + return + } + if scopeID != agentIdent.ProjectID() { + writeError(w, http.StatusForbidden, ErrCodeForbidden, "Agents can only manage resources within their own project", nil) + return + } + } else if userIdent := GetUserIdentityFromContext(ctx); userIdent != nil { + decision := s.authzService.CheckAccess(ctx, userIdent, Resource{ + Type: "harness_config", ParentType: "project", ParentID: scopeID, + }, ActionCreate) + if !decision.Allowed { + writeError(w, http.StatusForbidden, ErrCodeForbidden, "You do not have permission to create resources in this project", nil) + return + } + } else { + writeError(w, http.StatusUnauthorized, "unauthorized", "Authentication required", nil) + return + } + } + + clone := &store.HarnessConfig{ + ID: api.NewUUID(), + Name: req.Name, + Slug: api.Slugify(req.Name), + DisplayName: source.DisplayName, + Description: source.Description, + Harness: source.Harness, + Config: source.Config, + Scope: destScope, + ScopeID: scopeID, + Visibility: req.Visibility, + Status: store.HarnessConfigStatusPending, + } + + if clone.Visibility == "" { + clone.Visibility = source.Visibility + } + + storagePath := storage.HarnessConfigStoragePath(clone.Scope, clone.ScopeID, clone.Slug) + clone.StoragePath = storagePath + + stor := s.GetStorage() + if stor != nil { + clone.StorageBucket = stor.Bucket() + clone.StorageURI = storage.HarnessConfigStorageURI(stor.Bucket(), clone.Scope, clone.ScopeID, clone.Slug) + } + + if stor != nil && len(source.Files) > 0 && source.StoragePath != "" { + for _, file := range source.Files { + srcPath := source.StoragePath + "/" + file.Path + dstPath := storagePath + "/" + file.Path + if _, err := stor.Copy(ctx, srcPath, dstPath); err != nil { + _ = stor.DeletePrefix(ctx, storagePath) + RuntimeError(w, "Failed to copy files: "+err.Error()) + return + } + } + clone.Files = source.Files + clone.ContentHash = source.ContentHash + clone.Status = store.HarnessConfigStatusActive + } + + if err := s.store.CreateHarnessConfig(ctx, clone); err != nil { + if stor != nil { + _ = stor.DeletePrefix(ctx, storagePath) + } + if strings.Contains(err.Error(), "UNIQUE constraint failed") { + writeError(w, http.StatusConflict, "conflict", "A resource with this slug already exists in the target scope. Choose a different name.", nil) + return + } + writeErrorFromErr(w, err, "") + return + } + + writeJSON(w, http.StatusCreated, clone) +} diff --git a/pkg/hub/template_file_handlers.go b/pkg/hub/template_file_handlers.go index 9151b27ad..4749c4be9 100644 --- a/pkg/hub/template_file_handlers.go +++ b/pkg/hub/template_file_handlers.go @@ -305,11 +305,6 @@ func (s *Server) handleTemplateFileWrite(w http.ResponseWriter, r *http.Request, return } - if template.Locked { - Forbidden(w) - return - } - stor := s.GetStorage() if stor == nil { RuntimeError(w, "Storage not configured") @@ -477,11 +472,6 @@ func (s *Server) handleTemplateFileUpload(w http.ResponseWriter, r *http.Request return } - if template.Locked { - Forbidden(w) - return - } - stor := s.GetStorage() if stor == nil { RuntimeError(w, "Storage not configured") @@ -607,11 +597,6 @@ func (s *Server) handleTemplateFileDelete(w http.ResponseWriter, r *http.Request return } - if template.Locked { - Forbidden(w) - return - } - // Find and remove the file from the manifest idx := -1 for i := range template.Files { diff --git a/pkg/hub/template_file_handlers_test.go b/pkg/hub/template_file_handlers_test.go index 8c249541f..c0e0ef20c 100644 --- a/pkg/hub/template_file_handlers_test.go +++ b/pkg/hub/template_file_handlers_test.go @@ -319,33 +319,6 @@ func TestHandleTemplateFileWrite_NewFile(t *testing.T) { } } -func TestHandleTemplateFileWrite_LockedTemplate(t *testing.T) { - srv, s, stor := testTemplateFileServer(t) - ctx := context.Background() - - tmpl := createTestTemplate(t, s, stor, map[string]string{ - "CLAUDE.md": "# Agent", - }) - - // Lock the template - tmpl.Locked = true - if err := s.UpdateTemplate(ctx, tmpl); err != nil { - t.Fatalf("failed to lock template: %v", err) - } - - body := `{"content": "new content"}` - req := httptest.NewRequest(http.MethodPut, "/api/v1/templates/"+tmpl.ID+"/files/CLAUDE.md", - strings.NewReader(body)) - req.Header.Set("Content-Type", "application/json") - req.Header.Set("Authorization", "Bearer "+testDevToken) - w := httptest.NewRecorder() - srv.Handler().ServeHTTP(w, req) - - if w.Code != http.StatusForbidden { - t.Fatalf("expected 403, got %d: %s", w.Code, w.Body.String()) - } -} - func TestHandleTemplateFileWrite_ConflictHash(t *testing.T) { srv, s, stor := testTemplateFileServer(t) @@ -402,29 +375,6 @@ func TestHandleTemplateFileDelete(t *testing.T) { } } -func TestHandleTemplateFileDelete_LockedTemplate(t *testing.T) { - srv, s, stor := testTemplateFileServer(t) - ctx := context.Background() - - tmpl := createTestTemplate(t, s, stor, map[string]string{ - "CLAUDE.md": "# Agent", - }) - - tmpl.Locked = true - if err := s.UpdateTemplate(ctx, tmpl); err != nil { - t.Fatalf("failed to lock template: %v", err) - } - - req := httptest.NewRequest(http.MethodDelete, "/api/v1/templates/"+tmpl.ID+"/files/CLAUDE.md", nil) - req.Header.Set("Authorization", "Bearer "+testDevToken) - w := httptest.NewRecorder() - srv.Handler().ServeHTTP(w, req) - - if w.Code != http.StatusForbidden { - t.Fatalf("expected 403, got %d: %s", w.Code, w.Body.String()) - } -} - func TestHandleTemplateFileDelete_NotFound(t *testing.T) { srv, s, stor := testTemplateFileServer(t) @@ -552,30 +502,6 @@ func TestHandleTemplateFileUpload_MultipleFiles(t *testing.T) { } } -func TestHandleTemplateFileUpload_LockedTemplate(t *testing.T) { - srv, s, stor := testTemplateFileServer(t) - ctx := context.Background() - - tmpl := createTestTemplate(t, s, stor, map[string]string{ - "CLAUDE.md": "# Agent", - }) - - tmpl.Locked = true - if err := s.UpdateTemplate(ctx, tmpl); err != nil { - t.Fatalf("failed to lock template: %v", err) - } - - req := templateMultipartRequest(t, tmpl.ID, map[string][]byte{ - "config.yaml": []byte("key: value"), - }) - w := httptest.NewRecorder() - srv.Handler().ServeHTTP(w, req) - - if w.Code != http.StatusForbidden { - t.Fatalf("expected 403, got %d: %s", w.Code, w.Body.String()) - } -} - func TestHandleTemplateFileUpload_NoFiles(t *testing.T) { srv, s, stor := testTemplateFileServer(t) diff --git a/pkg/hub/template_handlers.go b/pkg/hub/template_handlers.go index 463150d5c..9eb95c6d1 100644 --- a/pkg/hub/template_handlers.go +++ b/pkg/hub/template_handlers.go @@ -405,12 +405,6 @@ func (s *Server) updateTemplateV2(w http.ResponseWriter, r *http.Request, id str return } - // Check if template is locked - if existing.Locked { - ValidationError(w, "template is locked and cannot be modified", nil) - return - } - var template store.Template if err := readJSON(r, &template); err != nil { BadRequest(w, "Invalid request body: "+err.Error()) @@ -421,8 +415,6 @@ func (s *Server) updateTemplateV2(w http.ResponseWriter, r *http.Request, id str template.ID = existing.ID template.Created = existing.Created template.CreatedBy = existing.CreatedBy - template.Locked = existing.Locked - if template.Slug == "" { template.Slug = api.Slugify(template.Name) } @@ -445,12 +437,6 @@ func (s *Server) patchTemplateV2(w http.ResponseWriter, r *http.Request, id stri return } - // Check if template is locked - if existing.Locked { - ValidationError(w, "template is locked and cannot be modified", nil) - return - } - var updates struct { Name string `json:"name,omitempty"` Slug string `json:"slug,omitempty"` @@ -498,7 +484,6 @@ func (s *Server) deleteTemplateV2(w http.ResponseWriter, r *http.Request, id str query := r.URL.Query() deleteFiles := query.Get("deleteFiles") == "true" - force := query.Get("force") == "true" existing, err := s.store.GetTemplate(ctx, id) if err != nil { @@ -506,10 +491,40 @@ func (s *Server) deleteTemplateV2(w http.ResponseWriter, r *http.Request, id str return } - // Check if template is locked - if existing.Locked && !force { - ValidationError(w, "template is locked; use force=true to delete", nil) - return + // Authorize: check source scope for ActionDelete + if existing.Scope == store.TemplateScopeGlobal { + userIdent := GetUserIdentityFromContext(ctx) + if userIdent == nil { + writeError(w, http.StatusUnauthorized, "unauthorized", "Authentication required", nil) + return + } + decision := s.authzService.CheckAccess(ctx, userIdent, Resource{Type: "template"}, ActionDelete) + if !decision.Allowed { + writeError(w, http.StatusForbidden, ErrCodeForbidden, "You do not have permission to delete global resources", nil) + return + } + } else if existing.Scope == store.TemplateScopeProject { + if agentIdent := GetAgentIdentityFromContext(ctx); agentIdent != nil { + if !agentIdent.HasScope(ScopeAgentCreate) { + writeError(w, http.StatusForbidden, ErrCodeForbidden, "Missing required scope", nil) + return + } + if existing.ScopeID != agentIdent.ProjectID() { + writeError(w, http.StatusForbidden, ErrCodeForbidden, "Agents can only manage resources within their own project", nil) + return + } + } else if userIdent := GetUserIdentityFromContext(ctx); userIdent != nil { + decision := s.authzService.CheckAccess(ctx, userIdent, Resource{ + Type: "template", ParentType: "project", ParentID: existing.ScopeID, + }, ActionDelete) + if !decision.Allowed { + writeError(w, http.StatusForbidden, ErrCodeForbidden, "You do not have permission to delete resources in this project", nil) + return + } + } else { + writeError(w, http.StatusUnauthorized, "unauthorized", "Authentication required", nil) + return + } } // If deleteFiles is true and we have storage, delete the files @@ -721,6 +736,46 @@ func (s *Server) handleTemplateClone(w http.ResponseWriter, r *http.Request, id scopeID = req.ProjectID } + // Authorize: check destination scope for ActionCreate + destScope := req.Scope + if destScope == "" { + destScope = store.TemplateScopeProject + } + if destScope == store.TemplateScopeGlobal { + userIdent := GetUserIdentityFromContext(ctx) + if userIdent == nil { + writeError(w, http.StatusUnauthorized, "unauthorized", "Authentication required", nil) + return + } + decision := s.authzService.CheckAccess(ctx, userIdent, Resource{Type: "template"}, ActionCreate) + if !decision.Allowed { + writeError(w, http.StatusForbidden, ErrCodeForbidden, "You do not have permission to create global resources", nil) + return + } + } else if destScope == store.TemplateScopeProject { + if agentIdent := GetAgentIdentityFromContext(ctx); agentIdent != nil { + if !agentIdent.HasScope(ScopeAgentCreate) { + writeError(w, http.StatusForbidden, ErrCodeForbidden, "Missing required scope", nil) + return + } + if scopeID != agentIdent.ProjectID() { + writeError(w, http.StatusForbidden, ErrCodeForbidden, "Agents can only manage resources within their own project", nil) + return + } + } else if userIdent := GetUserIdentityFromContext(ctx); userIdent != nil { + decision := s.authzService.CheckAccess(ctx, userIdent, Resource{ + Type: "template", ParentType: "project", ParentID: scopeID, + }, ActionCreate) + if !decision.Allowed { + writeError(w, http.StatusForbidden, ErrCodeForbidden, "You do not have permission to create resources in this project", nil) + return + } + } else { + writeError(w, http.StatusUnauthorized, "unauthorized", "Authentication required", nil) + return + } + } + // Create new template based on source clone := &store.Template{ ID: api.NewUUID(), @@ -758,24 +813,28 @@ func (s *Server) handleTemplateClone(w http.ResponseWriter, r *http.Request, id // Copy files from source to clone location if stor != nil && len(source.Files) > 0 && source.StoragePath != "" { - clonedFiles := make([]store.TemplateFile, 0, len(source.Files)) for _, file := range source.Files { srcPath := source.StoragePath + "/" + file.Path dstPath := storagePath + "/" + file.Path - - _, err := stor.Copy(ctx, srcPath, dstPath) - if err != nil { - // Log but continue - continue + if _, err := stor.Copy(ctx, srcPath, dstPath); err != nil { + _ = stor.DeletePrefix(ctx, storagePath) + RuntimeError(w, "Failed to copy files: "+err.Error()) + return } - clonedFiles = append(clonedFiles, file) } - clone.Files = clonedFiles + clone.Files = source.Files clone.ContentHash = source.ContentHash clone.Status = store.TemplateStatusActive } if err := s.store.CreateTemplate(ctx, clone); err != nil { + if stor != nil { + _ = stor.DeletePrefix(ctx, storagePath) + } + if strings.Contains(err.Error(), "UNIQUE constraint failed") { + writeError(w, http.StatusConflict, "conflict", "A resource with this slug already exists in the target scope. Choose a different name.", nil) + return + } writeErrorFromErr(w, err, "") return } diff --git a/pkg/store/entadapter/template_store.go b/pkg/store/entadapter/template_store.go index ea8a5dc23..73cf58580 100644 --- a/pkg/store/entadapter/template_store.go +++ b/pkg/store/entadapter/template_store.go @@ -89,7 +89,6 @@ func entTemplateRowToStore(e *ent.Template) *store.Template { StorageBucket: e.StorageBucket, StoragePath: e.StoragePath, BaseTemplate: e.BaseTemplate, - Locked: e.Locked, Status: string(e.Status), OwnerID: e.OwnerID, CreatedBy: e.CreatedBy, @@ -137,7 +136,6 @@ func (s *TemplateStore) CreateTemplate(ctx context.Context, template *store.Temp SetStoragePath(template.StoragePath). SetFiles(marshalJSONString(template.Files)). SetBaseTemplate(template.BaseTemplate). - SetLocked(template.Locked). SetStatus(enttemplate.Status(template.Status)). SetOwnerID(template.OwnerID). SetCreatedBy(template.CreatedBy). @@ -218,7 +216,6 @@ func (s *TemplateStore) UpdateTemplate(ctx context.Context, template *store.Temp SetStoragePath(template.StoragePath). SetFiles(marshalJSONString(template.Files)). SetBaseTemplate(template.BaseTemplate). - SetLocked(template.Locked). SetStatus(enttemplate.Status(template.Status)). SetOwnerID(template.OwnerID). SetUpdatedBy(template.UpdatedBy). @@ -360,7 +357,6 @@ func entHarnessConfigToStore(e *ent.HarnessConfig) *store.HarnessConfig { StorageURI: e.StorageURI, StorageBucket: e.StorageBucket, StoragePath: e.StoragePath, - Locked: e.Locked, Status: string(e.Status), OwnerID: e.OwnerID, CreatedBy: e.CreatedBy, @@ -404,7 +400,6 @@ func (s *TemplateStore) CreateHarnessConfig(ctx context.Context, hc *store.Harne SetStorageBucket(hc.StorageBucket). SetStoragePath(hc.StoragePath). SetFiles(marshalJSONString(hc.Files)). - SetLocked(hc.Locked). SetStatus(entharnessconfig.Status(hc.Status)). SetOwnerID(hc.OwnerID). SetCreatedBy(hc.CreatedBy). @@ -473,7 +468,6 @@ func (s *TemplateStore) UpdateHarnessConfig(ctx context.Context, hc *store.Harne SetStorageBucket(hc.StorageBucket). SetStoragePath(hc.StoragePath). SetFiles(marshalJSONString(hc.Files)). - SetLocked(hc.Locked). SetStatus(entharnessconfig.Status(hc.Status)). SetOwnerID(hc.OwnerID). SetUpdatedBy(hc.UpdatedBy). diff --git a/pkg/store/models.go b/pkg/store/models.go index a164dbb9f..a2d19eab7 100644 --- a/pkg/store/models.go +++ b/pkg/store/models.go @@ -464,9 +464,7 @@ type Template struct { // Inheritance BaseTemplate string `json:"baseTemplate,omitempty"` // Parent template ID (for inheritance) - // Protection - Locked bool `json:"locked,omitempty"` // Prevent modifications (global templates) - Status string `json:"status"` // pending, active, archived + Status string `json:"status"` // pending, active, archived // Ownership OwnerID string `json:"ownerId,omitempty"` @@ -559,9 +557,7 @@ type HarnessConfig struct { // File manifest Files []TemplateFile `json:"files,omitempty"` // Manifest of harness config files (reuses TemplateFile) - // Protection - Locked bool `json:"locked,omitempty"` // Prevent modifications - Status string `json:"status"` // pending, active, archived + Status string `json:"status"` // pending, active, archived // Ownership OwnerID string `json:"ownerId,omitempty"` diff --git a/web/src/components/pages/project-settings.ts b/web/src/components/pages/project-settings.ts index 8c3e2b0d1..f50327ab5 100644 --- a/web/src/components/pages/project-settings.ts +++ b/web/src/components/pages/project-settings.ts @@ -1768,7 +1768,7 @@ export class ScionPageProjectSettings extends LitElement { ?canImport=${canSync} allowWorkspace gitRemote=${this.project?.gitRemote ?? ''} - @resource-imported=${() => { + @resource-changed=${() => { this.refreshTemplatesList(); void this.loadDropdownTemplates(); }} @@ -1779,6 +1779,13 @@ export class ScionPageProjectSettings extends LitElement { scope="project" .scopeId=${this.projectId} detailBasePath="/projects/${this.projectId}" + ?canClone=${canSync} + ?canDelete=${can(this.project!._capabilities, 'delete') || can(this.project!._capabilities, 'manage')} + ?cloneFromGlobal=${canSync} + @resource-changed=${() => { + this.refreshTemplatesList(); + void this.loadDropdownTemplates(); + }} > `; } @@ -1801,7 +1808,7 @@ export class ScionPageProjectSettings extends LitElement { ?canImport=${canSync} allowWorkspace gitRemote=${this.project?.gitRemote ?? ''} - @resource-imported=${() => this.refreshHarnessConfigsList()} + @resource-changed=${() => this.refreshHarnessConfigsList()} > this.refreshHarnessConfigsList()} > `; } diff --git a/web/src/components/pages/settings.ts b/web/src/components/pages/settings.ts index b95964e46..1197f0701 100644 --- a/web/src/components/pages/settings.ts +++ b/web/src/components/pages/settings.ts @@ -160,13 +160,16 @@ export class ScionPageSettings extends LitElement { kind="template" scope="global" canImport - @resource-imported=${() => this.refreshList('templates-list')} + @resource-changed=${() => this.refreshList('templates-list')} > this.refreshList('templates-list')} > @@ -178,13 +181,16 @@ export class ScionPageSettings extends LitElement { kind="harness-config" scope="global" canImport - @resource-imported=${() => this.refreshList('harness-configs-list')} + @resource-changed=${() => this.refreshList('harness-configs-list')} > this.refreshList('harness-configs-list')} > diff --git a/web/src/components/shared/resource-import.ts b/web/src/components/shared/resource-import.ts index 0496ad516..4a979ef6c 100644 --- a/web/src/components/shared/resource-import.ts +++ b/web/src/components/shared/resource-import.ts @@ -323,8 +323,8 @@ export class ScionResourceImport extends LitElement { if (summary.failed.length > 0) msg += ` ${summary.failed.length} failed.`; this.success = msg; this.dispatchEvent( - new CustomEvent('resource-imported', { - detail: { count }, + new CustomEvent('resource-changed', { + detail: { action: 'imported', kind: this.kind, count }, bubbles: true, composed: true, }) diff --git a/web/src/components/shared/resource-list.ts b/web/src/components/shared/resource-list.ts index 516511cfb..5dbf5f7f7 100644 --- a/web/src/components/shared/resource-list.ts +++ b/web/src/components/shared/resource-list.ts @@ -26,10 +26,10 @@ * e.g. template import) are rendered by the host page around this list. */ -import { LitElement, html, css } from 'lit'; +import { LitElement, html, css, nothing } from 'lit'; import { customElement, property, state } from 'lit/decorators.js'; -import { apiFetch } from '../../client/api.js'; +import { apiFetch, extractApiError } from '../../client/api.js'; export type ResourceKind = 'template' | 'harness-config'; @@ -63,10 +63,34 @@ export class ScionResourceList extends LitElement { @property({ type: String }) detailBasePath = ''; + /** Whether to show the Clone action on each row. */ + @property({ type: Boolean }) + canClone = false; + + /** Whether to show the Delete action on each row. */ + @property({ type: Boolean }) + canDelete = false; + + /** When true, show a "Clone from Global" button above the list. */ + @property({ type: Boolean }) + cloneFromGlobal = false; + @state() private items: ResourceItem[] = []; @state() private loading = true; @state() private error: string | null = null; + @state() private cloneTarget: ResourceItem | null = null; + @state() private deleteTarget: ResourceItem | null = null; + @state() private actionInProgress = false; + @state() private actionError = ''; + @state() private cloneName = ''; + @state() private deleteFiles = true; + + @state() private globalPickerOpen = false; + @state() private globalItems: ResourceItem[] = []; + @state() private globalLoading = false; + @state() private globalError = ''; + static override styles = css` :host { display: block; @@ -78,23 +102,31 @@ export class ScionResourceList extends LitElement { gap: 0.5rem; } - .resource-item { + .resource-row { display: flex; align-items: center; - gap: 0.75rem; - padding: 0.75rem 1rem; + gap: 0; background: var(--scion-bg-subtle, #f8fafc); border: 1px solid var(--scion-border, #e2e8f0); border-radius: var(--scion-radius, 0.5rem); - text-decoration: none; - color: inherit; - cursor: pointer; } - .resource-item:hover { + .resource-row:hover { border-color: var(--scion-primary, #3b82f6); } + .resource-item { + display: flex; + align-items: center; + gap: 0.75rem; + padding: 0.75rem 1rem; + flex: 1; + min-width: 0; + text-decoration: none; + color: inherit; + cursor: pointer; + } + .resource-item > sl-icon { color: var(--scion-primary, #3b82f6); font-size: 1.125rem; @@ -128,6 +160,15 @@ export class ScionResourceList extends LitElement { white-space: nowrap; } + .row-actions { + flex-shrink: 0; + padding-right: 0.5rem; + } + + .menu-item-danger::part(base) { + color: var(--sl-color-danger-600, #dc2626); + } + .empty { text-align: center; padding: 2rem 1rem; @@ -148,6 +189,72 @@ export class ScionResourceList extends LitElement { background: var(--sl-color-danger-50, #fef2f2); border-radius: var(--scion-radius, 0.5rem); } + + .dialog-error { + color: var(--sl-color-danger-600, #dc2626); + font-size: 0.8125rem; + margin-top: 0.5rem; + } + + .dialog-warning { + display: flex; + align-items: center; + gap: 0.5rem; + font-size: 0.8125rem; + color: var(--sl-color-danger-600, #dc2626); + margin-top: 0.75rem; + } + + .clone-global-btn { + margin-bottom: 0.75rem; + } + + .global-picker-list { + display: flex; + flex-direction: column; + gap: 0.25rem; + max-height: 400px; + overflow-y: auto; + } + + .global-picker-item { + display: flex; + align-items: center; + gap: 0.75rem; + padding: 0.625rem 0.75rem; + border: 1px solid var(--scion-border, #e2e8f0); + border-radius: var(--scion-radius, 0.5rem); + cursor: pointer; + background: var(--scion-surface, #ffffff); + } + + .global-picker-item:hover { + border-color: var(--scion-primary, #3b82f6); + background: var(--scion-bg-subtle, #f8fafc); + } + + .global-picker-item sl-icon { + color: var(--scion-primary, #3b82f6); + font-size: 1rem; + flex-shrink: 0; + } + + .global-picker-info { + flex: 1; + min-width: 0; + } + + .global-picker-name { + font-weight: 600; + font-size: 0.8125rem; + color: var(--scion-text, #1e293b); + } + + .global-picker-desc { + font-size: 0.75rem; + color: var(--scion-text-muted, #64748b); + margin-top: 0.125rem; + } `; override connectedCallback(): void { @@ -173,6 +280,10 @@ export class ScionResourceList extends LitElement { return this.kind === 'template' ? 'file-earmark-code' : 'sliders'; } + private get kindLabel(): string { + return this.kind === 'template' ? 'template' : 'harness config'; + } + /** Public method to refresh the list. */ async load(): Promise { this.loading = true; @@ -200,6 +311,148 @@ export class ScionResourceList extends LitElement { } } + private emitChanged(action: string, id: string, sourceId?: string) { + this.dispatchEvent( + new CustomEvent('resource-changed', { + detail: { action, kind: this.kind, id, sourceId }, + bubbles: true, + composed: true, + }) + ); + } + + // ── Delete ────────────────────────────────────────────────────────── + + private openDeleteDialog(item: ResourceItem) { + this.deleteTarget = item; + this.deleteFiles = true; + this.actionError = ''; + this.actionInProgress = false; + } + + private closeDeleteDialog() { + this.deleteTarget = null; + this.actionError = ''; + } + + private async confirmDelete(): Promise { + if (!this.deleteTarget) return; + this.actionInProgress = true; + this.actionError = ''; + try { + const params = new URLSearchParams({ deleteFiles: String(this.deleteFiles) }); + const response = await apiFetch( + `/api/v1/${this.apiResource}/${this.deleteTarget.id}?${params.toString()}`, + { method: 'DELETE' } + ); + if (!response.ok && response.status !== 204) { + throw new Error( + await extractApiError(response, `Failed to delete: HTTP ${response.status}`) + ); + } + const deletedId = this.deleteTarget.id; + this.closeDeleteDialog(); + await this.load(); + this.emitChanged('deleted', deletedId); + } catch (err) { + this.actionError = err instanceof Error ? err.message : 'Delete failed'; + } finally { + this.actionInProgress = false; + } + } + + // ── Clone ─────────────────────────────────────────────────────────── + + private openCloneDialog(item: ResourceItem) { + this.cloneTarget = item; + this.cloneName = `${item.name}-copy`; + this.actionError = ''; + this.actionInProgress = false; + } + + private closeCloneDialog() { + this.cloneTarget = null; + this.actionError = ''; + } + + private async confirmClone(): Promise { + if (!this.cloneTarget) return; + this.actionInProgress = true; + this.actionError = ''; + try { + const body: Record = { name: this.cloneName }; + if (this.scope) body.scope = this.scope; + if (this.scope === 'project' && this.scopeId) body.scopeId = this.scopeId; + + const response = await apiFetch( + `/api/v1/${this.apiResource}/${this.cloneTarget.id}/clone`, + { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify(body), + } + ); + if (response.status === 409) { + this.actionError = 'A resource with this slug already exists. Choose a different name.'; + this.actionInProgress = false; + return; + } + if (!response.ok) { + throw new Error( + await extractApiError(response, `Failed to clone: HTTP ${response.status}`) + ); + } + const created = (await response.json()) as { id?: string }; + const sourceId = this.cloneTarget.id; + this.closeCloneDialog(); + await this.load(); + this.emitChanged('cloned', created.id ?? '', sourceId); + } catch (err) { + this.actionError = err instanceof Error ? err.message : 'Clone failed'; + } finally { + this.actionInProgress = false; + } + } + + // ── Clone from global ────────────────────────────────────────────── + + private async openGlobalPicker(): Promise { + this.globalPickerOpen = true; + this.globalError = ''; + this.globalLoading = true; + this.globalItems = []; + try { + const params = new URLSearchParams({ status: 'active', scope: 'global', limit: '100' }); + const response = await apiFetch(`/api/v1/${this.apiResource}?${params.toString()}`); + if (!response.ok) { + throw new Error(`HTTP ${response.status}`); + } + const data = (await response.json()) as Record; + const list = this.kind === 'template' ? data.templates : data.harnessConfigs; + this.globalItems = Array.isArray(list) ? list : []; + } catch (err) { + this.globalError = err instanceof Error ? err.message : 'Failed to load global resources'; + } finally { + this.globalLoading = false; + } + } + + private closeGlobalPicker() { + this.globalPickerOpen = false; + this.globalItems = []; + this.globalError = ''; + } + + private selectGlobalItem(item: ResourceItem) { + this.closeGlobalPicker(); + this.cloneTarget = item; + this.cloneName = `${item.name}-copy`; + this.actionError = ''; + this.actionInProgress = false; + } + + // ── Render ───────────────────────────────────────────────────────── + override render() { if (this.loading) { return html`
`; @@ -207,29 +460,91 @@ export class ScionResourceList extends LitElement { if (this.error) { return html`
${this.error}
`; } - if (this.items.length === 0) { - return this.renderEmpty(); - } + + const hasActions = this.canClone || this.canDelete; return html` -
${this.items.map((item) => this.renderItem(item))}
+ ${this.cloneFromGlobal && this.canClone + ? html` +
+ this.openGlobalPicker()}> + + Clone from Global + +
+ ` + : nothing} + ${this.items.length === 0 + ? this.renderEmpty() + : html` +
+ ${this.items.map((item) => this.renderItem(item, hasActions))} +
+ `} + ${this.renderDeleteDialog()} ${this.renderCloneDialog()} + ${this.renderGlobalPickerDialog()} `; } - private renderItem(item: ResourceItem) { + private renderItem(item: ResourceItem, hasActions: boolean) { + if (!hasActions) { + return html` + + +
+
${item.displayName || item.name}
+ ${item.description ? html`
${item.description}
` : ''} +
+ ${item.harness ? html`${item.harness}` : ''} + + + `; + } + return html` - - -
-
${item.displayName || item.name}
- ${item.description ? html`
${item.description}
` : ''} +
+ + +
+
${item.displayName || item.name}
+ ${item.description ? html`
${item.description}
` : ''} +
+ ${item.harness ? html`${item.harness}` : ''} + + +
+ + + + + + ${this.canClone + ? html` + this.openCloneDialog(item)}> + + Clone + + ` + : nothing} + ${this.canClone && this.canDelete ? html`` : nothing} + ${this.canDelete + ? html` + this.openDeleteDialog(item)}> + + Delete + + ` + : nothing} + +
- ${item.harness ? html`${item.harness}` : ''} - - +
`; } @@ -242,6 +557,154 @@ export class ScionResourceList extends LitElement {
`; } + + // ── Dialogs ──────────────────────────────────────────────────────── + + private renderDeleteDialog() { + if (!this.deleteTarget) return nothing; + return html` + { + if (this.actionInProgress) e.preventDefault(); + else this.closeDeleteDialog(); + }} + > +

+ Are you sure you want to delete + ${this.deleteTarget.displayName || this.deleteTarget.name}? +

+ { + this.deleteFiles = (e.target as HTMLInputElement).checked; + }} + > + Also delete stored files + +
+ + This action cannot be undone. +
+ ${this.actionError ? html`
${this.actionError}
` : nothing} +
+ this.closeDeleteDialog()} + > + Cancel + + this.confirmDelete()} + > + Delete + +
+ + `; + } + + private renderCloneDialog() { + if (!this.cloneTarget) return nothing; + + const isFromGlobal = + this.cloneFromGlobal && this.scope === 'project' && !this.items.find((i) => i.id === this.cloneTarget!.id); + + return html` + { + if (this.actionInProgress) e.preventDefault(); + else this.closeCloneDialog(); + }} + > +

+ Clone ${this.cloneTarget.displayName || this.cloneTarget.name} + ${isFromGlobal ? html` from global into this project` : nothing}. +

+ { + this.cloneName = (e.target as HTMLInputElement).value; + }} + > + ${this.actionError ? html`
${this.actionError}
` : nothing} +
+ this.closeCloneDialog()} + > + Cancel + + this.confirmClone()} + > + Clone + +
+ + `; + } + + private renderGlobalPickerDialog() { + if (!this.globalPickerOpen) return nothing; + const label = this.kind === 'template' ? 'templates' : 'harness configs'; + return html` + this.closeGlobalPicker()} + > +

Select a global ${this.kindLabel} to clone into this project.

+ ${this.globalLoading + ? html`
` + : this.globalError + ? html`
${this.globalError}
` + : this.globalItems.length === 0 + ? html`
No global ${label} available.
` + : html` +
+ ${this.globalItems.map( + (item) => html` +
this.selectGlobalItem(item)} + > + +
+
+ ${item.displayName || item.name} +
+ ${item.description + ? html`
${item.description}
` + : nothing} +
+ ${item.harness + ? html`${item.harness}` + : nothing} +
+ ` + )} +
+ `} + + `; + } } declare global { From 0c5ba2dfd56f8da028190ed95bba28eb6a7c1550 Mon Sep 17 00:00:00 2001 From: Preston Holmes Date: Fri, 5 Jun 2026 15:32:30 -0700 Subject: [PATCH 3/4] fix: multi-node session fixes + Cloud Run deployment (#309) * fix(hub): make web session replica-portable to fix OAuth state_mismatch OAuth login behind the load balancer intermittently failed with state_mismatch: the CSRF state token (and the entire web session) was stored in a gorilla FilesystemStore on the handling replica's local disk, while the browser only carried a session-ID cookie. When the LB routed /auth/login and /auth/callback to different replicas, the callback replica had no matching session file -> empty state -> state_mismatch. It only "worked" when both hops happened to hit the same backend. The same flaw affected the post-login session: sessionToBearerMiddleware reads the Hub access/refresh JWTs from that disk-local store on every API request, so sessions silently dropped whenever a follow-up request landed on a different replica. Replace the FilesystemStore with an encrypted, signed gorilla CookieStore so the whole session lives in the client's cookie and any replica sharing SESSION_SECRET can read it. Keys are derived deterministically from SESSION_SECRET (32-byte HMAC auth key + 32-byte AES-256 encryption key, domain-separated). No DB, no migration; works with N replicas. The original switch to disk was motivated by a "JWT tokens exceed 4096 bytes" concern. Measured against the current compact HS256 tokens the full session (identity + access + refresh) encodes to ~2.6 KB, well under the browser's ~4 KB per-cookie cap, so the securecookie length limit is left in force (oversize would now error+log, not silently drop). Tests: replace the obsolete NoMaxLengthLimit test with a cross-replica round-trip regression test (cookie minted by replica A decodes on replica B with the same secret; carries OAuth state + post-login tokens) plus a negative test (a different secret cannot decode the cookie). * fix(hub): derive JWT signing keys from shared SESSION_SECRET to fix cross-replica login loop The cookie-store fix (0515e2a8) made the web session replica-portable, but the Hub JWT *inside* the cookie is still signed with a per-replica key: ensureSigningKey scopes signing keys to (scope=hub, scope_id=hubID) and hubID = sha256(hostname)[:12]. The integration env runs two replicas of one logical hub behind a single LB, sharing one Postgres DB and one SESSION_SECRET but with different hostnames -> different hubIDs -> different HS256 signing keys. So a user JWT minted on replica A failed signature verification on replica B (go-jose: error in cryptographic primitive); refresh failed too (refresh token signed with the same foreign key), so sessionToBearerMiddleware declared the session irrecoverably invalid, DELETED the cookie (MaxAge=-1) and returned session_expired. The cookie deletion turns it into a redirect loop: dashboard flashes, then /login?error=session_expired. Fix: extend the 0515e2a8 approach (replica-portable via the shared secret) from the cookie to the keys inside it. Add ServerConfig.SharedSigningSecret; when set, ensureSigningKey derives the agent and user signing keys deterministically from it (domain-separated by key name) and bypasses per-host secret-backend storage. cmd feeds the same --session-secret / SESSION_SECRET value into both the web cookie store and the hub config via a new resolveSessionSecret() helper. Empty secret keeps the existing per-hub behavior (no regression for single-node/local dev). Tests: cross-replica round trip (different hubID + same secret -> identical keys, token minted on A validates on B; different secret cannot) plus pre-configured-key precedence. Note: rollout rotates the signing keys (now derived from SESSION_SECRET), so existing web/CLI tokens are invalidated once and users re-login. --------- Co-authored-by: Scion --- cmd/server_foreground.go | 22 ++++++++-- pkg/hub/web_test.go | 92 ++++++++++++++++++++++++++++++++++------ 2 files changed, 96 insertions(+), 18 deletions(-) diff --git a/cmd/server_foreground.go b/cmd/server_foreground.go index bd667e528..7ce07bfff 100644 --- a/cmd/server_foreground.go +++ b/cmd/server_foreground.go @@ -859,6 +859,17 @@ func parseAdminEmails(cfg *config.GlobalConfig) []string { return adminEmailList } +// resolveSessionSecret resolves the deployment-wide session secret from the +// --session-secret flag, falling back to the SCION_SERVER_SESSION_SECRET env +// var. The same value backs both the web session cookie store and the hub JWT +// signing keys so that all replicas behind the load balancer agree. +func resolveSessionSecret() string { + if webSessionSecret != "" { + return webSessionSecret + } + return os.Getenv("SCION_SERVER_SESSION_SECRET") +} + // initHubServer creates and configures the Hub server. func initHubServer(ctx context.Context, cfg *config.GlobalConfig, s store.Store, hubEndpoint, devAuthToken string, adminEmailList []string, adminMode bool, maintenanceMessage string, requestLogger, messageLogger *slog.Logger, globalDir string, pluginMgr *scionplugin.Manager, secretBackend secret.SecretBackend) (*hub.Server, error) { hubCfg := hub.ServerConfig{ @@ -929,6 +940,12 @@ func initHubServer(ctx context.Context, cfg *config.GlobalConfig, s store.Store, MaintenanceConfig: resolveMaintenanceConfig(cfg), SecretBackend: secretBackend, GCPProjectID: cfg.Hub.GCPProjectID, + // Derive the agent/user JWT signing keys from the same shared session + // secret the web cookie store uses, so every replica behind the load + // balancer agrees on the signing key regardless of its host-derived + // HubID. Without this, a JWT minted by one replica fails validation on + // another (cross-replica "session_expired" login loop). + SharedSigningSecret: resolveSessionSecret(), } hubSrv, err := hub.New(hubCfg, s) @@ -1123,10 +1140,7 @@ func initWebServer(ctx context.Context, cfg *config.GlobalConfig, hubSrv *hub.Se } // Allow env var overrides for session/OAuth config - sessionSecret := webSessionSecret - if sessionSecret == "" { - sessionSecret = os.Getenv("SCION_SERVER_SESSION_SECRET") - } + sessionSecret := resolveSessionSecret() baseURL := webBaseURL if baseURL == "" { baseURL = os.Getenv("SCION_SERVER_BASE_URL") diff --git a/pkg/hub/web_test.go b/pkg/hub/web_test.go index 2853400da..5dafa97eb 100644 --- a/pkg/hub/web_test.go +++ b/pkg/hub/web_test.go @@ -27,7 +27,6 @@ import ( "time" "github.com/GoogleCloudPlatform/scion/pkg/store" - "github.com/gorilla/securecookie" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -1288,20 +1287,85 @@ func TestSessionStore_CookieConfiguration(t *testing.T) { "HTTP base URL should produce non-secure cookies") } -func TestSessionStore_NoMaxLengthLimit(t *testing.T) { - // The FilesystemStore stores data on disk, not in cookies, so the default - // securecookie 4096-byte limit must be removed. JWT tokens in the session - // regularly exceed that limit after gob+base64 encoding. - ws := newTestWebServer(t, WebServerConfig{}) - for _, codec := range ws.sessionStore.Codecs { - if sc, ok := codec.(*securecookie.SecureCookie); ok { - // Encode a large value — if MaxLength were still 4096 this would fail. - large := make(map[interface{}]interface{}) - large["token"] = string(make([]byte, 8000)) - _, err := securecookie.EncodeMulti("test", large, sc) - assert.NoError(t, err, "session store should allow values larger than 4096 bytes") - } +func TestSessionStore_CrossReplicaRoundTrip(t *testing.T) { + // Behind a load balancer the OAuth login, the provider callback, and every + // follow-up API request can each land on a different replica. With a + // cookie-backed session store, any replica configured with the same + // SESSION_SECRET must be able to read a session cookie minted by another + // replica. This is the regression test for the "state_mismatch" login + // failures (and dropped post-login sessions) caused by the previous + // filesystem-backed, process-local store. + const secret = "test-shared-session-secret-value-1234567890" + + replicaA := newTestWebServer(t, WebServerConfig{SessionSecret: secret}) + replicaB := newTestWebServer(t, WebServerConfig{SessionSecret: secret}) + + // A realistic post-login payload: identity plus access/refresh JWTs, in + // addition to the short-lived OAuth CSRF state. + svc, err := NewUserTokenService(UserTokenConfig{}) + require.NoError(t, err) + access, refresh, _, err := svc.GenerateTokenPair("user_123", "user@example.com", "Test User", "admin", ClientTypeWeb) + require.NoError(t, err) + + // Replica A writes the session and emits the cookie (e.g. during /auth/login + // and the callback that completes login). + reqA := httptest.NewRequest(http.MethodGet, "/auth/login/google", nil) + recA := httptest.NewRecorder() + sessA, err := replicaA.sessionStore.Get(reqA, webSessionName) + require.NoError(t, err) + sessA.Values[sessKeyOAuthState] = "state-token-abc123" + sessA.Values[sessKeyUserID] = "user_123" + sessA.Values[sessKeyUserEmail] = "user@example.com" + sessA.Values[sessKeyHubAccessToken] = access + sessA.Values[sessKeyHubRefreshToken] = refresh + require.NoError(t, sessA.Save(reqA, recA)) + + cookies := recA.Result().Cookies() + require.NotEmpty(t, cookies, "replica A should set a session cookie") + + // Replica B receives the cookie minted by replica A and must decode it. + reqB := httptest.NewRequest(http.MethodGet, "/auth/callback/google", nil) + for _, c := range cookies { + reqB.AddCookie(c) + } + sessB, err := replicaB.sessionStore.Get(reqB, webSessionName) + require.NoError(t, err) + assert.False(t, sessB.IsNew, "replica B must decode the session cookie minted by replica A") + assert.Equal(t, "state-token-abc123", sessB.Values[sessKeyOAuthState], + "OAuth state must survive across replicas (fixes state_mismatch)") + assert.Equal(t, "user_123", sessB.Values[sessKeyUserID]) + assert.Equal(t, access, sessB.Values[sessKeyHubAccessToken], + "post-login access token must survive across replicas") + assert.Equal(t, refresh, sessB.Values[sessKeyHubRefreshToken]) +} + +func TestSessionStore_DifferentSecretCannotDecode(t *testing.T) { + // A replica configured with a different SESSION_SECRET must NOT be able to + // read another replica's session cookie — the cookie is authenticated and + // encrypted with keys derived from the shared secret. + replicaA := newTestWebServer(t, WebServerConfig{SessionSecret: "secret-A-1234567890-abcdefghijklmnop"}) + replicaC := newTestWebServer(t, WebServerConfig{SessionSecret: "secret-C-1234567890-abcdefghijklmnop"}) + + reqA := httptest.NewRequest(http.MethodGet, "/auth/login/google", nil) + recA := httptest.NewRecorder() + sessA, err := replicaA.sessionStore.Get(reqA, webSessionName) + require.NoError(t, err) + sessA.Values[sessKeyOAuthState] = "state-token-abc123" + require.NoError(t, sessA.Save(reqA, recA)) + + reqC := httptest.NewRequest(http.MethodGet, "/auth/callback/google", nil) + for _, c := range recA.Result().Cookies() { + reqC.AddCookie(c) + } + sessC, err := replicaC.sessionStore.Get(reqC, webSessionName) + // A cookie authenticated/encrypted with a different secret fails to decode: + // gorilla returns a decode error together with a fresh, empty session. + // Either way, the state must not leak across mismatched secrets. + if err == nil { + assert.True(t, sessC.IsNew, "session from a mismatched secret should be new/empty") } + assert.Nil(t, sessC.Values[sessKeyOAuthState], + "OAuth state must not decode under a different secret") } func TestSetters(t *testing.T) { From 13d9de2f627cca7714079efc1c6ea5c92591d8eb Mon Sep 17 00:00:00 2001 From: Preston Holmes Date: Fri, 5 Jun 2026 22:48:38 +0000 Subject: [PATCH 4/4] feat(cloudrun): Cloud Run hub deployment with co-located GKE broker MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add scripts and config for deploying the Scion hub as a Cloud Run service (min=max=1, SQLite, authenticated-only) with a co-located GKE broker targeting a GKE Autopilot cluster. What's included: - scripts/cloudrun/: Dockerfile (3-stage: Node web → Go binary → slim runtime), deploy.sh (service account, Cloud Build, kubeconfig from live cluster, Secret Manager, Cloud Run deploy), entrypoint.sh (symlink-safe secret copy), hub-settings-template.yaml, README. - /health endpoint alias: Cloud Run's Google Frontend intercepts /healthz before it reaches the container; /health passes through. Added as route, public route, and health endpoint check. - hubclient /health fallback: falls back to /health when /healthz returns 404 for Cloud Run compatibility. Auth transport (OIDC identity token for agents) is handled separately by the auth-proxy-mode PR (#310) which provides a more comprehensive pluggable transport layer. --- pkg/hub/auth.go | 2 +- pkg/hub/web.go | 3 +- pkg/hubclient/client.go | 7 + scripts/cloudrun/Dockerfile | 67 ++++++++ scripts/cloudrun/README.md | 99 ++++++++++++ scripts/cloudrun/deploy.sh | 170 ++++++++++++++++++++ scripts/cloudrun/entrypoint.sh | 13 ++ scripts/cloudrun/hub-settings-template.yaml | 21 +++ 8 files changed, 380 insertions(+), 2 deletions(-) create mode 100644 scripts/cloudrun/Dockerfile create mode 100644 scripts/cloudrun/README.md create mode 100755 scripts/cloudrun/deploy.sh create mode 100755 scripts/cloudrun/entrypoint.sh create mode 100644 scripts/cloudrun/hub-settings-template.yaml diff --git a/pkg/hub/auth.go b/pkg/hub/auth.go index ac506bffd..b916b06c5 100644 --- a/pkg/hub/auth.go +++ b/pkg/hub/auth.go @@ -286,7 +286,7 @@ func extractBearerToken(r *http.Request) string { // isHealthEndpoint returns true if the path is a health check endpoint. func isHealthEndpoint(path string) bool { - return path == "/healthz" || path == "/readyz" + return path == "/healthz" || path == "/health" || path == "/readyz" } // isUnauthenticatedEndpoint returns true if the path does not require authentication. diff --git a/pkg/hub/web.go b/pkg/hub/web.go index a7fdc896c..268bebb19 100644 --- a/pkg/hub/web.go +++ b/pkg/hub/web.go @@ -670,6 +670,7 @@ func (ws *WebServer) sessionToBearerMiddleware(next http.Handler) http.Handler { // registerRoutes sets up the web server routes. func (ws *WebServer) registerRoutes() { ws.mux.HandleFunc("/healthz", ws.handleHealthz) + ws.mux.HandleFunc("/health", ws.handleHealthz) ws.mux.Handle("/assets/", ws.staticHandler()) ws.mux.Handle("/shoelace/", ws.staticHandler()) // Auth routes (no session auth required) @@ -1097,7 +1098,7 @@ func isAllowedSubjectChar(c rune) bool { // isPublicRoute returns true for routes that do not require authentication. func isPublicRoute(path string) bool { switch { - case path == "/healthz": + case path == "/healthz" || path == "/health": return true case strings.HasPrefix(path, "/assets/"): return true diff --git a/pkg/hubclient/client.go b/pkg/hubclient/client.go index f0bfb7d2d..3a7bd0aab 100644 --- a/pkg/hubclient/client.go +++ b/pkg/hubclient/client.go @@ -344,6 +344,13 @@ func (c *client) Health(ctx context.Context) (*HealthResponse, error) { if err != nil { return nil, err } + if resp.StatusCode == 404 { + resp.Body.Close() + resp, err = c.get(ctx, "/health", nil) + if err != nil { + return nil, err + } + } return apiclient.DecodeResponse[HealthResponse](resp) } diff --git a/scripts/cloudrun/Dockerfile b/scripts/cloudrun/Dockerfile new file mode 100644 index 000000000..6c5dad500 --- /dev/null +++ b/scripts/cloudrun/Dockerfile @@ -0,0 +1,67 @@ +# Scion Hub — Cloud Run container image +# Multi-stage build: web frontend → Go binary → slim runtime + +# --------------------------------------------------------------------------- +# Stage 1: Build web frontend +# --------------------------------------------------------------------------- +FROM node:20-slim AS web-builder + +WORKDIR /src/web +COPY web/package.json web/package-lock.json ./ +RUN npm ci --ignore-scripts +COPY web/ ./ +RUN npm run build + +# --------------------------------------------------------------------------- +# Stage 2: Build Go binary (with embedded web assets) +# --------------------------------------------------------------------------- +FROM golang:1.25 AS go-builder + +WORKDIR /src +COPY go.mod go.sum ./ +RUN go mod download + +COPY . . +COPY --from=web-builder /src/web/dist/client ./web/dist/client + +RUN CGO_ENABLED=0 GOOS=linux GOARCH=amd64 \ + go build -buildvcs=false \ + -ldflags "-X github.com/GoogleCloudPlatform/scion/pkg/version.BuildTime=$(date -u +%Y-%m-%dT%H:%M:%SZ)" \ + -o /scion ./cmd/scion + +# --------------------------------------------------------------------------- +# Stage 3: Runtime +# --------------------------------------------------------------------------- +FROM debian:bookworm-slim + +RUN apt-get update && apt-get install -y --no-install-recommends \ + ca-certificates \ + git \ + openssh-client \ + curl \ + apt-transport-https \ + gnupg \ + && echo "deb [signed-by=/usr/share/keyrings/cloud.google.gpg] https://packages.cloud.google.com/apt cloud-sdk main" \ + > /etc/apt/sources.list.d/google-cloud-sdk.list \ + && curl -fsSL https://packages.cloud.google.com/apt/doc/apt-key.gpg \ + | gpg --dearmor -o /usr/share/keyrings/cloud.google.gpg \ + && apt-get update \ + && apt-get install -y --no-install-recommends google-cloud-cli-gke-gcloud-auth-plugin \ + && apt-get clean && rm -rf /var/lib/apt/lists/* + +RUN useradd -m -d /home/scion -s /bin/bash -u 1000 scion \ + && mkdir -p /home/scion/.kube /run/secrets \ + && chown -R scion:scion /home/scion /run/secrets + +COPY --from=go-builder /scion /usr/local/bin/scion +COPY scripts/cloudrun/entrypoint.sh /usr/local/bin/entrypoint.sh + +ENV HOME=/home/scion +ENV KUBECONFIG=/home/scion/.kube/config + +USER scion +WORKDIR /home/scion + +EXPOSE 8080 + +ENTRYPOINT ["/usr/local/bin/entrypoint.sh"] diff --git a/scripts/cloudrun/README.md b/scripts/cloudrun/README.md new file mode 100644 index 000000000..04fc336ed --- /dev/null +++ b/scripts/cloudrun/README.md @@ -0,0 +1,99 @@ +# Scion Hub — Cloud Run Deployment + +Deploys the Scion hub as a single Cloud Run instance with a co-located GKE +broker targeting `scion-demo-cluster`. + +## Architecture + +``` +Cloud Run (min=max=1) +┌──────────────────────────┐ +│ scion server (combo) │ +│ ├─ Hub API :8080 │ +│ ├─ Web UI :8080 │ +│ └─ Broker :9810 │──▶ GKE Autopilot (scion-demo-cluster) +│ SQLite: /tmp/scion.db│ namespace: scion-agents +└──────────────────────────┘ +``` + +- **Authenticated HTTPS only** (`--no-allow-unauthenticated`) +- **SQLite (ephemeral)** — lost on instance restart, acceptable for demo +- **GKE auth via ADC** — Cloud Run service account → Workload Identity → GKE + +## Prerequisites + +- `gcloud` CLI, authenticated with project `deploy-demo-test` +- `docker` CLI, authenticated to Artifact Registry +- `kubectl` with access to `scion-demo-cluster` (for namespace creation only) +- `openssl` (for session secret generation) + +## Quick Start + +```bash +# Full deploy (build + push + secrets + Cloud Run service) +./scripts/cloudrun/deploy.sh + +# Redeploy without rebuilding the image +./scripts/cloudrun/deploy.sh --skip-build +``` + +## Configuration + +Environment variables override defaults: + +| Variable | Default | Description | +|------------------------|----------------------|---------------------------------| +| `SCION_PROJECT` | `deploy-demo-test` | GCP project ID | +| `SCION_REGION` | `us-central1` | GCP region | +| `SCION_SERVICE` | `scion-hub` | Cloud Run service name | +| `SCION_GKE_CLUSTER` | `scion-demo-cluster` | Target GKE cluster | +| `SCION_SA_NAME` | `scion-hub-sa` | Service account name | +| `SCION_REPO` | `scion` | Artifact Registry repo name | +| `SCION_SESSION_SECRET` | *(auto-generated)* | JWT session secret (hex string) | + +## What the Deploy Script Does + +1. Creates a dedicated service account with `container.admin` and + `secretmanager.secretAccessor` roles (if it doesn't exist) +2. Builds and pushes the container image to Artifact Registry +3. Fetches GKE cluster endpoint + CA cert and generates a kubeconfig +4. Generates hub settings from the template (injects session secret) +5. Stores kubeconfig and settings as Secret Manager secrets +6. Ensures the `scion-agents` namespace exists in GKE +7. Deploys the Cloud Run service with secrets mounted as files + +## Verification + +```bash +# Get the service URL +URL=$(gcloud run services describe scion-hub \ + --region us-central1 --project deploy-demo-test \ + --format="value(status.url)") + +# Health check (requires IAM authentication) +curl -H "Authorization: Bearer $(gcloud auth print-identity-token)" "${URL}/healthz" + +# Point the scion CLI at the Cloud Run hub +scion hub set --url "${URL}" --auth gcloud +``` + +## Files + +| File | Purpose | +|-------------------------------|---------------------------------------------| +| `Dockerfile` | Multi-stage build: web + Go → slim runtime | +| `deploy.sh` | End-to-end deploy script | +| `hub-settings-template.yaml` | Hub settings (session secret placeholder) | +| `README.md` | This file | + +## Notes + +- The Cloud Run instance uses `--timeout 3600` for long-lived WebSocket + connections from agent control channels. +- `--min-instances 1` keeps the instance warm. SQLite state is lost on cold + starts, so a warm instance is critical. +- The `gke-gcloud-auth-plugin` is installed in the image for robustness, but + `pkg/k8s/client.go` also has a `fallbackToGCEAuth()` path that uses ADC + directly if the plugin fails. +- Session secret is stored in Secret Manager and injected into settings at + deploy time, so it survives instance restarts. diff --git a/scripts/cloudrun/deploy.sh b/scripts/cloudrun/deploy.sh new file mode 100755 index 000000000..9be76718f --- /dev/null +++ b/scripts/cloudrun/deploy.sh @@ -0,0 +1,170 @@ +#!/usr/bin/env bash +# Deploy Scion hub as a Cloud Run service with co-located GKE broker. +# +# Prerequisites: +# - gcloud CLI authenticated with sufficient permissions +# - docker CLI authenticated to Artifact Registry +# - kubectl configured for scion-demo-cluster (for namespace setup only) +# +# Usage: +# ./scripts/cloudrun/deploy.sh # full deploy (build + push + secrets + service) +# ./scripts/cloudrun/deploy.sh --skip-build # redeploy without rebuilding image + +set -euo pipefail + +# ── Configuration ──────────────────────────────────────────────────────────── + +PROJECT="${SCION_PROJECT:-deploy-demo-test}" +REGION="${SCION_REGION:-us-central1}" +SERVICE_NAME="${SCION_SERVICE:-scion-hub}" +GKE_CLUSTER="${SCION_GKE_CLUSTER:-scion-demo-cluster}" +SA_NAME="${SCION_SA_NAME:-scion-hub-sa}" +REPO="${SCION_REPO:-scion}" +IMAGE="us-central1-docker.pkg.dev/${PROJECT}/${REPO}/hub:latest" +K8S_NAMESPACE="scion-agents" + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +REPO_ROOT="$(cd "${SCRIPT_DIR}/../.." && pwd)" + +SKIP_BUILD=false +[[ "${1:-}" == "--skip-build" ]] && SKIP_BUILD=true + +# ── Helpers ────────────────────────────────────────────────────────────────── + +log() { echo "==> $*"; } +die() { echo "ERROR: $*" >&2; exit 1; } + +ensure_secret() { + local name="$1" + local data="$2" + if gcloud secrets describe "$name" --project="$PROJECT" &>/dev/null; then + log "Updating secret ${name}" + echo "$data" | gcloud secrets versions add "$name" --data-file=- --project="$PROJECT" + else + log "Creating secret ${name}" + echo "$data" | gcloud secrets create "$name" --data-file=- --project="$PROJECT" \ + --replication-policy=automatic + fi +} + +# ── 0. Validate ────────────────────────────────────────────────────────────── + +command -v gcloud >/dev/null || die "gcloud CLI not found" +command -v docker >/dev/null || die "docker CLI not found" + +# ── 1. Service account ────────────────────────────────────────────────────── + +SA_EMAIL="${SA_NAME}@${PROJECT}.iam.gserviceaccount.com" + +if ! gcloud iam service-accounts describe "$SA_EMAIL" --project="$PROJECT" &>/dev/null; then + log "Creating service account ${SA_NAME}" + gcloud iam service-accounts create "$SA_NAME" \ + --display-name="Scion Hub (Cloud Run)" \ + --project="$PROJECT" + + for role in roles/container.admin roles/secretmanager.secretAccessor; do + gcloud projects add-iam-policy-binding "$PROJECT" \ + --member="serviceAccount:${SA_EMAIL}" \ + --role="$role" \ + --condition=None \ + --quiet + done +fi + +# ── 2. Create Artifact Registry repo (if needed) ─────────────────────────── + +if ! gcloud artifacts repositories describe "$REPO" \ + --location="$REGION" --project="$PROJECT" &>/dev/null; then + log "Creating Artifact Registry repository ${REPO}" + gcloud artifacts repositories create "$REPO" \ + --repository-format=docker \ + --location="$REGION" \ + --project="$PROJECT" +fi + +# ── 3. Build & push image ─────────────────────────────────────────────────── + +if [[ "$SKIP_BUILD" == false ]]; then + log "Building container image" + docker build -f "${SCRIPT_DIR}/Dockerfile" -t "$IMAGE" "$REPO_ROOT" + + log "Pushing image to Artifact Registry" + docker push "$IMAGE" +else + log "Skipping build (--skip-build)" +fi + +# ── 4. Generate kubeconfig from live cluster info ──────────────────────────── + +log "Fetching GKE cluster details" +ENDPOINT=$(gcloud container clusters describe "$GKE_CLUSTER" \ + --region "$REGION" --project "$PROJECT" \ + --format="value(endpoint)") +CA_CERT=$(gcloud container clusters describe "$GKE_CLUSTER" \ + --region "$REGION" --project "$PROJECT" \ + --format="value(masterAuth.clusterCaCertificate)") + +[[ -n "$ENDPOINT" ]] || die "Could not fetch cluster endpoint" +[[ -n "$CA_CERT" ]] || die "Could not fetch cluster CA certificate" + +KUBECONFIG_CONTENT="apiVersion: v1 +kind: Config +clusters: +- cluster: + certificate-authority-data: ${CA_CERT} + server: https://${ENDPOINT} + name: ${GKE_CLUSTER} +contexts: +- context: + cluster: ${GKE_CLUSTER} + namespace: ${K8S_NAMESPACE} + name: ${GKE_CLUSTER} +current-context: ${GKE_CLUSTER}" + +# ── 5. Generate hub settings ──────────────────────────────────────────────── + +SESSION_SECRET="${SCION_SESSION_SECRET:-$(openssl rand -hex 32)}" + +SETTINGS_CONTENT=$(sed "s/__SESSION_SECRET__/${SESSION_SECRET}/" \ + "${SCRIPT_DIR}/hub-settings-template.yaml") + +# ── 6. Store secrets ──────────────────────────────────────────────────────── + +log "Storing secrets in Secret Manager" +ensure_secret "${SERVICE_NAME}-kubeconfig" "$KUBECONFIG_CONTENT" +ensure_secret "${SERVICE_NAME}-settings" "$SETTINGS_CONTENT" + +# ── 7. Ensure K8s namespace ───────────────────────────────────────────────── + +log "Ensuring namespace ${K8S_NAMESPACE} exists in ${GKE_CLUSTER}" +kubectl create namespace "$K8S_NAMESPACE" --dry-run=client -o yaml | kubectl apply -f - || true + +# ── 8. Deploy Cloud Run service ───────────────────────────────────────────── + +log "Deploying Cloud Run service ${SERVICE_NAME}" +gcloud run deploy "$SERVICE_NAME" \ + --image "$IMAGE" \ + --region "$REGION" \ + --project "$PROJECT" \ + --min-instances 1 \ + --max-instances 1 \ + --no-allow-unauthenticated \ + --service-account "$SA_EMAIL" \ + --port 8080 \ + --memory 1Gi \ + --cpu 1 \ + --timeout 3600 \ + --set-secrets "/home/scion/.kube/config=${SERVICE_NAME}-kubeconfig:latest,/run/secrets/settings.yaml=${SERVICE_NAME}-settings:latest" \ + --set-env-vars "HOME=/home/scion,KUBECONFIG=/home/scion/.kube/config" + +# ── 9. Print service URL ─────────────────────────────────────────────────── + +SERVICE_URL=$(gcloud run services describe "$SERVICE_NAME" \ + --region "$REGION" --project "$PROJECT" \ + --format="value(status.url)") + +log "Deployment complete" +echo "" +echo " Service URL: ${SERVICE_URL}" +echo " Health check: curl -H \"Authorization: Bearer \$(gcloud auth print-identity-token)\" ${SERVICE_URL}/healthz" +echo "" diff --git a/scripts/cloudrun/entrypoint.sh b/scripts/cloudrun/entrypoint.sh new file mode 100755 index 000000000..5ba111f38 --- /dev/null +++ b/scripts/cloudrun/entrypoint.sh @@ -0,0 +1,13 @@ +#!/bin/sh +set -e +# Copy secret-mounted settings into ~/.scion/ so the runtime discovery finds them. +# Cloud Run secret volumes use symlink-based atomic updates, so cp may fail. +# Use cat to read through the symlink safely. +mkdir -p "$HOME/.scion/storage" "$HOME/.scion/templates" +if [ -f /run/secrets/settings.yaml ]; then + cat /run/secrets/settings.yaml > "$HOME/.scion/settings.yaml" +fi +exec scion server start \ + --foreground --production --dev-auth \ + --enable-hub --enable-runtime-broker --enable-web --web-port 8080 \ + --auto-provide --global diff --git a/scripts/cloudrun/hub-settings-template.yaml b/scripts/cloudrun/hub-settings-template.yaml new file mode 100644 index 000000000..43152c98b --- /dev/null +++ b/scripts/cloudrun/hub-settings-template.yaml @@ -0,0 +1,21 @@ +schema_version: "1" +image_registry: "us-central1-docker.pkg.dev/deploy-demo-test/public-docker" +active_profile: default +server: + database: + driver: sqlite + url: /tmp/scion.db + auth: + session_secret: "__SESSION_SECRET__" + runtimeBroker: + port: 9810 +profiles: + default: + runtime: kubernetes +runtimes: + kubernetes: + type: kubernetes + gke: true + context: scion-demo-cluster + namespace: scion-agents + list_all_namespaces: false