diff --git a/.yarn/changelogs/service.0cbd66e9.md b/.yarn/changelogs/service.0cbd66e9.md new file mode 100644 index 0000000..d640088 --- /dev/null +++ b/.yarn/changelogs/service.0cbd66e9.md @@ -0,0 +1,46 @@ + +# service + +## ๐Ÿ› Bug Fixes + +### Reject duplicate IDs with `409 Conflict` on create endpoints + +Creating an entity with a client-supplied `id`/`name` that already exists now fails fast with a `409` response and a descriptive message instead of surfacing a generic store-level error (or, worse, silently overwriting state during a stack import). Affected endpoints: + +- `POST /github-repositories` โ€” rejects when `body.id` matches an existing GitHub repository. +- `POST /prerequisites` โ€” rejects when `body.id` matches an existing prerequisite. +- `POST /services` โ€” rejects when `body.id` matches an existing service definition. +- `POST /stacks` โ€” rejects when `body.name` matches an existing stack. + +### Pre-flight conflict check on stack import + +`POST /stacks/import` now collects all id collisions (`stack.name`, `services[].id`, `repositories[].id`, `prerequisites[].id`) before touching any store, and returns a single `409` listing every conflicting id. Previously, the importer could partially apply changes and then enter the rollback path, which would delete the pre-existing records that shared the colliding ids. + +### Atomic rollback on partial-create failures + +Multi-step create flows now undo the records they have already inserted when a later step fails, so a failed create no longer leaves orphaned rows in the data store: + +- `POST /prerequisites` โ€” removes the prerequisite if seeding the initial `PrerequisiteCheckResult` throws. +- `POST /services` โ€” removes the partially-inserted `ServiceDefinition`, `ServiceConfig`, `ServiceStatus`, and link rows if any later insert throws. +- `POST /stacks` โ€” removes the `StackDefinition` if inserting the `StackConfig` throws. + +Errors that are not already a `RequestError` are wrapped into a `500` with the original message preserved. + +## ๐Ÿ”ง Refactor + +The duplicate-id check and the multi-step create + rollback logic for `POST /services`, `POST /stacks`, and `POST /github-repositories` were moved out of the inline `setup-*-rest-api.ts` handlers into named, testable `RequestAction` modules โ€” matching the existing `ImportStackAction` / `CreatePrerequisiteAction` shape: + +- `service/src/app-models/services/actions/create-service-action.ts` โ€” exports `CreateServiceAction`, with `assertNoServiceIdCollision`, `buildServiceArtifacts`, and `createServiceWithRollback` as module-local helpers. +- `service/src/app-models/stacks/actions/create-stack-action.ts` โ€” exports `CreateStackAction` with an `assertNoStackNameCollision` helper. +- `service/src/app-models/github-repositories/actions/create-github-repo-action.ts` โ€” exports `CreateGitHubRepoAction`. + +The three `setup-*-rest-api.ts` files now wire the action through `Validate(...)` with no inline logic, which removes the High/Medium complexity findings introduced by the original fix on the services and stacks endpoints. No behavioral change. + +## ๐Ÿงช Tests + +- Added prerequisite-creation specs covering the duplicate-id `409` path and the check-result rollback path. +- Added stack-import specs covering duplicate `stack.name` and cross-stack duplicate `service.id` rejection, asserting the pre-existing records remain untouched. +- Added `create-github-repo-action.spec.ts` covering happy-path persistence (`201`) and duplicate-id `409`. +- Added `create-stack-action.spec.ts` covering duplicate-name `409` and the `StackDefinition` rollback when `StackConfig.add` throws. +- Added `create-service-action.spec.ts` covering duplicate-id `409`, full rollback when the `ServiceStatus` insert throws, and link-row rollback when a later `ServiceDependencyLink` insert throws. +- Extended `import-export-actions.spec.ts` with duplicate-`repository.id` and combined service+repo+prerequisite collision tests, asserting `assertNoImportConflicts` aggregates every conflicting id into a single `409` message. diff --git a/.yarn/changelogs/stack-craft.0cbd66e9.md b/.yarn/changelogs/stack-craft.0cbd66e9.md new file mode 100644 index 0000000..2bcbba7 --- /dev/null +++ b/.yarn/changelogs/stack-craft.0cbd66e9.md @@ -0,0 +1,12 @@ + +# stack-craft + +## ๐Ÿ› Bug Fixes + +### Prevent silent overwrites when creating or importing entities with duplicate IDs + +Creating a GitHub repository, prerequisite, service, or stack with an `id`/`name` that already exists now fails with a clear `409 Conflict` error instead of overwriting the existing record or returning an opaque store error. Importing a stack performs the same collision check up-front across the stack name and all nested service, repository, and prerequisite ids, so a conflicting import is rejected before any data is mutated. + +Partially-created entities are also rolled back when a later step in the create flow fails (e.g. service definition + config + status + links), so failed creates no longer leave orphan rows behind. + +See the `service` changelog for the affected endpoints and response details. diff --git a/.yarn/versions/0cbd66e9.yml b/.yarn/versions/0cbd66e9.yml new file mode 100644 index 0000000..3d28169 --- /dev/null +++ b/.yarn/versions/0cbd66e9.yml @@ -0,0 +1,3 @@ +releases: + service: patch + stack-craft: patch diff --git a/service/src/app-models/github-repositories/actions/create-github-repo-action.spec.ts b/service/src/app-models/github-repositories/actions/create-github-repo-action.spec.ts new file mode 100644 index 0000000..cfe6112 --- /dev/null +++ b/service/src/app-models/github-repositories/actions/create-github-repo-action.spec.ts @@ -0,0 +1,65 @@ +import { getDataSetFor } from '@furystack/repository' +import { describe, expect, it } from 'vitest' + +import { createMockActionContext, withTestInjector } from '../../../test-helpers.js' +import { GitHubRepositoryDataSet } from '../../data-store/tokens.js' +import { CreateGitHubRepoAction } from './create-github-repo-action.js' + +type CreateRepoBody = Parameters[0] extends { getBody: () => Promise } + ? B + : never + +const baseRepo = { + id: 'repo-1', + stackName: 'my-stack', + url: 'https://github.com/example/repo', + displayName: 'Example Repo', + description: '', +} + +describe('CreateGitHubRepoAction', () => { + it('should persist the repository and return it with statusCode 201', async () => { + await withTestInjector(async ({ elevated }) => { + const result = await CreateGitHubRepoAction({ + ...createMockActionContext({ injector: elevated, body: baseRepo }), + request: {} as never, + }) + + expect(result.statusCode).toBe(201) + expect(result.chunk.id).toBe('repo-1') + + const stored = await getDataSetFor(elevated, GitHubRepositoryDataSet).find(elevated, {}) + expect(stored).toHaveLength(1) + expect(stored[0]?.url).toBe('https://github.com/example/repo') + }) + }) + + it('should reject with 409 when a repository with the same id already exists', async () => { + await withTestInjector(async ({ elevated }) => { + const ts = new Date().toISOString() + await getDataSetFor(elevated, GitHubRepositoryDataSet).add(elevated, { + ...baseRepo, + displayName: 'Original', + createdAt: ts, + updatedAt: ts, + }) + + await expect( + CreateGitHubRepoAction({ + ...createMockActionContext({ + injector: elevated, + body: { ...baseRepo, displayName: 'Duplicate' }, + }), + request: {} as never, + }), + ).rejects.toMatchObject({ + message: expect.stringContaining('"repo-1" already exists'), + responseCode: 409, + }) + + const stored = await getDataSetFor(elevated, GitHubRepositoryDataSet).find(elevated, {}) + expect(stored).toHaveLength(1) + expect(stored[0]?.displayName).toBe('Original') + }) + }) +}) diff --git a/service/src/app-models/github-repositories/actions/create-github-repo-action.ts b/service/src/app-models/github-repositories/actions/create-github-repo-action.ts new file mode 100644 index 0000000..97721d4 --- /dev/null +++ b/service/src/app-models/github-repositories/actions/create-github-repo-action.ts @@ -0,0 +1,35 @@ +import type { WithOptionalId } from '@furystack/core' +import { RequestError } from '@furystack/rest' +import { JsonResult, type RequestAction } from '@furystack/rest-service' +import type { GitHubRepoWritableFields, PostGitHubRepoEndpoint } from 'common' +import { GitHubRepository } from 'common' + +import { legacyRepository as getRepository } from '../../../utils/legacy-repository.js' + +/** + * POST action that registers a new {@link GitHubRepository}. + * + * Rejects with `409 Conflict` when the client-supplied `body.id` matches an + * existing record, so the store-level `add` cannot silently overwrite it. + */ +export const CreateGitHubRepoAction: RequestAction = async ({ injector, getBody }) => { + const body = await getBody() + const ds = getRepository(injector).getDataSetFor< + GitHubRepository, + 'id', + WithOptionalId + >(GitHubRepository, 'id') + + if (body.id !== undefined) { + const existing = await ds.get(injector, body.id) + if (existing) { + throw new RequestError(`A GitHub repository with id "${body.id}" already exists. Choose a different id.`, 409) + } + } + + const { created } = await ds.add(injector, body) + if (!created?.length) { + throw new RequestError('Repository not created', 500) + } + return JsonResult(created[0], 201) +} diff --git a/service/src/app-models/github-repositories/setup-github-repos-rest-api.ts b/service/src/app-models/github-repositories/setup-github-repos-rest-api.ts index f5b23a3..fd0a79c 100644 --- a/service/src/app-models/github-repositories/setup-github-repos-rest-api.ts +++ b/service/src/app-models/github-repositories/setup-github-repos-rest-api.ts @@ -5,7 +5,6 @@ import { createGetCollectionEndpoint, createGetEntityEndpoint, createPatchEndpoint, - createPostEndpoint, useRestService, Validate, } from '@furystack/rest-service' @@ -16,6 +15,7 @@ import { GitHubRepositoryDataSet } from '../data-store/tokens.js' import { getCorsOptions } from '../../get-cors-options.js' import { getHost } from '../../get-host.js' import { getPort } from '../../get-port.js' +import { CreateGitHubRepoAction } from './actions/create-github-repo-action.js' import { ValidateRepoAction } from './actions/validate-repo-action.js' export const setupGitHubReposRestApi = async (injector: Injector) => { @@ -40,7 +40,7 @@ export const setupGitHubReposRestApi = async (injector: Injector) => { '/github-repositories': Validate({ schema: githubReposApiSchema, schemaName: 'PostGitHubRepoEndpoint', - })(createPostEndpoint(GitHubRepositoryDataSet)), + })(CreateGitHubRepoAction), '/github-repositories/:id/validate': Validate({ schema: githubReposApiSchema, schemaName: 'ValidateRepoEndpoint', diff --git a/service/src/app-models/prerequisites/actions/prerequisite-lifecycle-actions.spec.ts b/service/src/app-models/prerequisites/actions/prerequisite-lifecycle-actions.spec.ts index 47210bc..78af790 100644 --- a/service/src/app-models/prerequisites/actions/prerequisite-lifecycle-actions.spec.ts +++ b/service/src/app-models/prerequisites/actions/prerequisite-lifecycle-actions.spec.ts @@ -46,6 +46,74 @@ describe('CreatePrerequisiteAction', () => { }) }) + it('should reject with 409 when a prerequisite with the same id already exists', async () => { + await withTestInjector(async ({ elevated }) => { + const ts = new Date().toISOString() + await getDataSetFor(elevated, PrerequisiteDataSet).add(elevated, { + id: 'existing-prereq', + stackName: 'stack', + name: 'Existing', + type: 'custom-script', + config: { script: 'echo existing' }, + installationHelp: '', + createdAt: ts, + updatedAt: ts, + }) + + mockedReadPostBody.mockResolvedValue({ + id: 'existing-prereq', + stackName: 'stack', + name: 'Duplicate', + type: 'custom-script' as const, + config: { script: 'echo dup' }, + installationHelp: '', + }) + + await expect( + CreatePrerequisiteAction({ + ...createMockActionContext({ injector: elevated }), + request: {} as never, + }), + ).rejects.toMatchObject({ + message: expect.stringContaining('"existing-prereq" already exists'), + responseCode: 409, + }) + + const prereqs = await getDataSetFor(elevated, PrerequisiteDataSet).find(elevated, {}) + expect(prereqs).toHaveLength(1) + expect(prereqs[0].name).toBe('Existing') + }) + }) + + it('should roll back the prerequisite when seeding the check result fails', async () => { + await withTestInjector(async ({ elevated }) => { + mockedReadPostBody.mockResolvedValue({ + id: 'rollback-prereq', + stackName: 'stack', + name: 'Rollback', + type: 'custom-script' as const, + config: { script: 'echo rollback' }, + installationHelp: '', + }) + + const checkResultDs = getDataSetFor(elevated, PrerequisiteCheckResultDataSet) + vi.spyOn(checkResultDs, 'add').mockRejectedValueOnce(new Error('check-result store down')) + + await expect( + CreatePrerequisiteAction({ + ...createMockActionContext({ injector: elevated }), + request: {} as never, + }), + ).rejects.toMatchObject({ + message: expect.stringContaining('check-result store down'), + responseCode: 500, + }) + + const prereqs = await getDataSetFor(elevated, PrerequisiteDataSet).find(elevated, {}) + expect(prereqs).toHaveLength(0) + }) + }) + it('should throw 500 when prerequisite creation returns empty', async () => { await withTestInjector(async ({ elevated }) => { mockedReadPostBody.mockResolvedValue({ diff --git a/service/src/app-models/prerequisites/actions/prerequisite-lifecycle-actions.ts b/service/src/app-models/prerequisites/actions/prerequisite-lifecycle-actions.ts index 0ad4ade..acba42d 100644 --- a/service/src/app-models/prerequisites/actions/prerequisite-lifecycle-actions.ts +++ b/service/src/app-models/prerequisites/actions/prerequisite-lifecycle-actions.ts @@ -19,18 +19,36 @@ export const CreatePrerequisiteAction: RequestAction = const checkResultDs = repository.getDataSetFor(PrerequisiteCheckResult, 'prerequisiteId') const body = await readPostBody>(request) + + if (body.id !== undefined) { + const existing = await prereqDs.get(injector, body.id) + if (existing) { + throw new RequestError(`A prerequisite with id "${body.id}" already exists. Choose a different id.`, 409) + } + } + const { created } = await prereqDs.add(injector, body) if (!created?.length) { throw new RequestError('Prerequisite not created', 500) } const newPrereq = created[0] - await checkResultDs.add(injector, { - prerequisiteId: newPrereq.id, - status: 'unchecked', - output: '', - checkedAt: '', - }) + try { + await checkResultDs.add(injector, { + prerequisiteId: newPrereq.id, + status: 'unchecked', + output: '', + checkedAt: '', + }) + } catch (error) { + await prereqDs.remove(injector, newPrereq.id).catch(() => undefined) + throw error instanceof RequestError + ? error + : new RequestError( + `Failed to create prerequisite: ${error instanceof Error ? error.message : 'unknown error'}`, + 500, + ) + } return JsonResult(newPrereq, 201) } diff --git a/service/src/app-models/services/actions/create-service-action.spec.ts b/service/src/app-models/services/actions/create-service-action.spec.ts new file mode 100644 index 0000000..bc026a9 --- /dev/null +++ b/service/src/app-models/services/actions/create-service-action.spec.ts @@ -0,0 +1,164 @@ +import { getDataSetFor } from '@furystack/repository' +import { randomBytes } from 'crypto' +import { beforeAll, describe, expect, it, vi } from 'vitest' + +import { + ServiceConfigDataSet, + ServiceDefinitionDataSet, + ServiceDependencyLinkDataSet, + ServicePrerequisiteLinkDataSet, + ServiceStatusDataSet, +} from '../../data-store/tokens.js' +import { createMockActionContext, withTestInjector } from '../../../test-helpers.js' +import { CreateServiceAction } from './create-service-action.js' + +type CreateServiceBody = Parameters[0] extends { getBody: () => Promise } + ? B + : never + +const baseBody: CreateServiceBody = { + id: 'svc-1', + stackName: 'my-stack', + displayName: 'Service 1', + description: '', + workingDirectory: 'svc', + runCommand: 'echo hello', + files: [], + autoFetchEnabled: false, + autoFetchIntervalMinutes: 60, + autoRestartOnFetch: false, + environmentVariableOverrides: {}, + localFiles: [], + prerequisiteIds: [], + prerequisiteServiceIds: [], +} + +beforeAll(() => { + process.env.STACK_CRAFT_ENCRYPTION_KEY = randomBytes(32).toString('base64') +}) + +describe('CreateServiceAction', () => { + it('should persist the service definition, config, status, and link rows', async () => { + await withTestInjector(async ({ elevated }) => { + const body: CreateServiceBody = { + ...baseBody, + prerequisiteIds: ['prereq-a'], + prerequisiteServiceIds: ['svc-dep'], + } + + const result = await CreateServiceAction({ + ...createMockActionContext({ injector: elevated, body }), + request: {} as never, + }) + + expect(result.chunk.id).toBe('svc-1') + + const defs = await getDataSetFor(elevated, ServiceDefinitionDataSet).find(elevated, {}) + expect(defs).toHaveLength(1) + + const configs = await getDataSetFor(elevated, ServiceConfigDataSet).find(elevated, {}) + expect(configs).toHaveLength(1) + + const statuses = await getDataSetFor(elevated, ServiceStatusDataSet).find(elevated, {}) + expect(statuses).toHaveLength(1) + expect(statuses[0]?.runStatus).toBe('stopped') + + const prereqLinks = await getDataSetFor(elevated, ServicePrerequisiteLinkDataSet).find(elevated, {}) + expect(prereqLinks).toHaveLength(1) + expect(prereqLinks[0]?.prerequisiteId).toBe('prereq-a') + + const depLinks = await getDataSetFor(elevated, ServiceDependencyLinkDataSet).find(elevated, {}) + expect(depLinks).toHaveLength(1) + expect(depLinks[0]?.dependsOnServiceId).toBe('svc-dep') + }) + }) + + it('should reject with 409 when a service with the same id already exists', async () => { + await withTestInjector(async ({ elevated }) => { + const ts = new Date().toISOString() + await getDataSetFor(elevated, ServiceDefinitionDataSet).add(elevated, { + id: 'svc-1', + stackName: 'my-stack', + displayName: 'Original', + description: '', + workingDirectory: 'svc', + runCommand: 'echo original', + files: [], + createdAt: ts, + updatedAt: ts, + }) + + await expect( + CreateServiceAction({ + ...createMockActionContext({ + injector: elevated, + body: { ...baseBody, displayName: 'Duplicate' }, + }), + request: {} as never, + }), + ).rejects.toMatchObject({ + message: expect.stringContaining('"svc-1" already exists'), + responseCode: 409, + }) + + const defs = await getDataSetFor(elevated, ServiceDefinitionDataSet).find(elevated, {}) + expect(defs).toHaveLength(1) + expect(defs[0]?.displayName).toBe('Original') + + const configs = await getDataSetFor(elevated, ServiceConfigDataSet).find(elevated, {}) + expect(configs).toHaveLength(0) + }) + }) + + it('should roll back every inserted row when the ServiceStatus insert fails', async () => { + await withTestInjector(async ({ elevated }) => { + const statusDs = getDataSetFor(elevated, ServiceStatusDataSet) + vi.spyOn(statusDs, 'add').mockRejectedValueOnce(new Error('status store down')) + + await expect( + CreateServiceAction({ + ...createMockActionContext({ + injector: elevated, + body: { ...baseBody, prerequisiteIds: ['prereq-a'], prerequisiteServiceIds: ['svc-dep'] }, + }), + request: {} as never, + }), + ).rejects.toMatchObject({ + message: expect.stringContaining('status store down'), + responseCode: 500, + }) + + expect(await getDataSetFor(elevated, ServiceDefinitionDataSet).find(elevated, {})).toHaveLength(0) + expect(await getDataSetFor(elevated, ServiceConfigDataSet).find(elevated, {})).toHaveLength(0) + expect(await getDataSetFor(elevated, ServiceStatusDataSet).find(elevated, {})).toHaveLength(0) + expect(await getDataSetFor(elevated, ServicePrerequisiteLinkDataSet).find(elevated, {})).toHaveLength(0) + expect(await getDataSetFor(elevated, ServiceDependencyLinkDataSet).find(elevated, {})).toHaveLength(0) + }) + }) + + it('should roll back already-written link rows when a later link insert fails', async () => { + await withTestInjector(async ({ elevated }) => { + const depLinkDs = getDataSetFor(elevated, ServiceDependencyLinkDataSet) + vi.spyOn(depLinkDs, 'add').mockRejectedValueOnce(new Error('dep-link store down')) + + await expect( + CreateServiceAction({ + ...createMockActionContext({ + injector: elevated, + body: { ...baseBody, prerequisiteIds: ['prereq-a'], prerequisiteServiceIds: ['svc-dep'] }, + }), + request: {} as never, + }), + ).rejects.toMatchObject({ + message: expect.stringContaining('dep-link store down'), + responseCode: 500, + }) + + expect(await getDataSetFor(elevated, ServiceDefinitionDataSet).find(elevated, {})).toHaveLength(0) + expect(await getDataSetFor(elevated, ServiceConfigDataSet).find(elevated, {})).toHaveLength(0) + expect(await getDataSetFor(elevated, ServiceStatusDataSet).find(elevated, {})).toHaveLength(0) + expect(await getDataSetFor(elevated, ServicePrerequisiteLinkDataSet).find(elevated, {})).toHaveLength(0) + expect(await getDataSetFor(elevated, ServiceDependencyLinkDataSet).find(elevated, {})).toHaveLength(0) + }) + }) +}) diff --git a/service/src/app-models/services/actions/create-service-action.ts b/service/src/app-models/services/actions/create-service-action.ts new file mode 100644 index 0000000..54790e6 --- /dev/null +++ b/service/src/app-models/services/actions/create-service-action.ts @@ -0,0 +1,40 @@ +import type { Injector } from '@furystack/inject' +import { RequestError } from '@furystack/rest' +import { JsonResult, type RequestAction } from '@furystack/rest-service' +import type { PostServiceEndpoint } from 'common' +import { ServiceDefinition } from 'common' + +import { CryptoService } from '../../../utils/crypto-service.js' +import { legacyRepository as getRepository } from '../../../utils/legacy-repository.js' +import { buildMaskedServiceView, buildServiceArtifacts } from './service-artifacts.js' +import { createServiceWithRollback } from './service-create-with-rollback.js' + +const assertNoServiceIdCollision = async (injector: Injector, id: string | undefined): Promise => { + if (id === undefined) return + const ds = getRepository(injector).getDataSetFor(ServiceDefinition, 'id') + const existing = await ds.get(injector, id) + if (existing) { + throw new RequestError(`A service with id "${id}" already exists. Choose a different id.`, 409) + } +} + +/** + * POST action that creates a {@link ServiceDefinition} along with its + * companion `ServiceConfig`, `ServiceStatus` and the requested prerequisite / + * dependency link rows. + * + * Rejects with `409 Conflict` when the client-supplied `body.id` matches an + * existing service. If any later insert throws, every record written for the + * service is removed before the error propagates (see + * {@link createServiceWithRollback}). + */ +export const CreateServiceAction: RequestAction = async ({ injector, getBody }) => { + const body = await getBody() + const crypto = injector.get(CryptoService) + const now = new Date().toISOString() + + await assertNoServiceIdCollision(injector, body.id) + const artifacts = buildServiceArtifacts(body, crypto, now) + await createServiceWithRollback(injector, artifacts) + return JsonResult(buildMaskedServiceView(artifacts, crypto)) +} diff --git a/service/src/app-models/services/actions/service-artifacts.ts b/service/src/app-models/services/actions/service-artifacts.ts new file mode 100644 index 0000000..bf9e17e --- /dev/null +++ b/service/src/app-models/services/actions/service-artifacts.ts @@ -0,0 +1,97 @@ +import type { PostServiceEndpoint, ServiceRelations } from 'common' +import { mergeServiceView, type ServiceConfig, type ServiceDefinition, type ServiceStatus } from 'common' +import { randomUUID } from 'crypto' + +import { SENSITIVE_VALUE_MASK, type CryptoService } from '../../../utils/crypto-service.js' +import { + encryptEnvValues, + encryptLocalFiles, + maskLocalFiles, + maskSensitiveEnvValues, +} from '../../../utils/env-encryption-helpers.js' + +/** + * Materialised rows ready to be persisted for a new service, plus the + * relational ids that drive link-row creation. Built once by + * {@link buildServiceArtifacts} so the persistence and view-shaping helpers + * share a single source of truth. + */ +export type ServiceArtifacts = { + def: ServiceDefinition + config: ServiceConfig + status: ServiceStatus + prerequisiteIds: string[] + prerequisiteServiceIds: string[] +} + +/** + * Pure data assembly: turns a validated `POST /services` body into the four + * concrete rows + relational ids that need to be persisted. Falls back to a + * `randomUUID()` when the client omits `body.id`. + */ +export const buildServiceArtifacts = ( + body: PostServiceEndpoint['body'], + crypto: CryptoService, + now: string, +): ServiceArtifacts => { + const id = body.id ?? randomUUID() + return { + def: { + id, + stackName: body.stackName, + displayName: body.displayName, + description: body.description ?? '', + workingDirectory: body.workingDirectory, + repositoryId: body.repositoryId, + installCommand: body.installCommand, + buildCommand: body.buildCommand, + runCommand: body.runCommand, + files: body.files ?? [], + createdAt: now, + updatedAt: now, + }, + config: { + serviceId: id, + autoFetchEnabled: body.autoFetchEnabled ?? false, + autoFetchIntervalMinutes: body.autoFetchIntervalMinutes ?? 60, + autoRestartOnFetch: body.autoRestartOnFetch ?? false, + environmentVariableOverrides: encryptEnvValues(crypto, body.environmentVariableOverrides ?? {}), + localFiles: encryptLocalFiles(crypto, body.localFiles ?? []), + createdAt: now, + updatedAt: now, + }, + status: { + serviceId: id, + cloneStatus: 'not-cloned', + installStatus: 'not-installed', + buildStatus: 'not-built', + runStatus: 'stopped', + updatedAt: now, + }, + prerequisiteIds: body.prerequisiteIds ?? [], + prerequisiteServiceIds: body.prerequisiteServiceIds ?? [], + } +} + +/** + * Shapes the response body for `POST /services`: merges the freshly-written + * artifacts into a `ServiceView` and masks every encrypted secret so the + * caller never receives ciphertext. + */ +export const buildMaskedServiceView = ( + artifacts: ServiceArtifacts, + crypto: CryptoService, +): ReturnType => { + const relations: ServiceRelations = { + prerequisiteIds: artifacts.prerequisiteIds, + prerequisiteServiceIds: artifacts.prerequisiteServiceIds, + } + const merged = mergeServiceView(artifacts.def, artifacts.config, artifacts.status, undefined, relations) + merged.environmentVariableOverrides = maskSensitiveEnvValues( + crypto, + merged.environmentVariableOverrides, + SENSITIVE_VALUE_MASK, + ) + merged.localFiles = maskLocalFiles(crypto, merged.localFiles) + return merged +} diff --git a/service/src/app-models/services/actions/service-create-with-rollback.ts b/service/src/app-models/services/actions/service-create-with-rollback.ts new file mode 100644 index 0000000..a674c15 --- /dev/null +++ b/service/src/app-models/services/actions/service-create-with-rollback.ts @@ -0,0 +1,65 @@ +import type { Injector } from '@furystack/inject' +import { getLogger } from '@furystack/logging' +import { RequestError } from '@furystack/rest' +import { ServiceConfig, ServiceDefinition, ServiceDependencyLink, ServicePrerequisiteLink, ServiceStatus } from 'common' + +import { legacyRepository as getRepository } from '../../../utils/legacy-repository.js' +import type { ServiceArtifacts } from './service-artifacts.js' + +/** + * Persists the four service rows + link rows in dependency order. If any + * insert throws, every record already written for the service is removed + * before the error is re-thrown โ€” so a failed create never leaves orphan + * rows behind. Errors that are not already a {@link RequestError} are + * wrapped into a 500 with the original message preserved. + * + * Insertion order: def โ†’ config โ†’ status โ†’ prereq links โ†’ dep links. Rollback + * walks the inverse, with link rows discovered via filtered `find` (link + * primary keys are composite strings, not the service id). + */ +export const createServiceWithRollback = async (injector: Injector, artifacts: ServiceArtifacts): Promise => { + const repo = getRepository(injector) + const svcDefDs = repo.getDataSetFor(ServiceDefinition, 'id') + const svcConfigDs = repo.getDataSetFor(ServiceConfig, 'serviceId') + const svcStatusDs = repo.getDataSetFor(ServiceStatus, 'serviceId') + const prereqLinkDs = repo.getDataSetFor(ServicePrerequisiteLink, 'id') + const depLinkDs = repo.getDataSetFor(ServiceDependencyLink, 'id') + const { def, config, status, prerequisiteIds, prerequisiteServiceIds } = artifacts + const { id } = def + + let defAdded = false + let configAdded = false + let statusAdded = false + try { + await svcDefDs.add(injector, def) + defAdded = true + await svcConfigDs.add(injector, config) + configAdded = true + await svcStatusDs.add(injector, status) + statusAdded = true + + for (const prereqId of prerequisiteIds) { + await prereqLinkDs.add(injector, { id: `${id}::${prereqId}`, serviceId: id, prerequisiteId: prereqId }) + } + for (const depId of prerequisiteServiceIds) { + await depLinkDs.add(injector, { id: `${id}::${depId}`, serviceId: id, dependsOnServiceId: depId }) + } + } catch (error) { + const rollbackLogger = getLogger(injector).withScope('CreateService') + const pLinks = await prereqLinkDs.find(injector, { filter: { serviceId: { $eq: id } } }).catch(() => []) + if (pLinks.length > 0) { + await prereqLinkDs.remove(injector, ...pLinks.map((l) => l.id)).catch(() => undefined) + } + const dLinks = await depLinkDs.find(injector, { filter: { serviceId: { $eq: id } } }).catch(() => []) + if (dLinks.length > 0) { + await depLinkDs.remove(injector, ...dLinks.map((l) => l.id)).catch(() => undefined) + } + if (statusAdded) await svcStatusDs.remove(injector, id).catch(() => undefined) + if (configAdded) await svcConfigDs.remove(injector, id).catch(() => undefined) + if (defAdded) await svcDefDs.remove(injector, id).catch(() => undefined) + await rollbackLogger.warning({ message: `Service creation rolled back for "${id}"`, data: { error } }) + throw error instanceof RequestError + ? error + : new RequestError(`Failed to create service: ${error instanceof Error ? error.message : 'unknown error'}`, 500) + } +} diff --git a/service/src/app-models/services/setup-services-rest-api.ts b/service/src/app-models/services/setup-services-rest-api.ts index 320e502..f292ce3 100644 --- a/service/src/app-models/services/setup-services-rest-api.ts +++ b/service/src/app-models/services/setup-services-rest-api.ts @@ -15,7 +15,6 @@ import { ServiceStatus, } from 'common' import servicesApiSchema from 'common/schemas/services-api.json' with { type: 'json' } -import { randomUUID } from 'crypto' import { getCorsOptions } from '../../get-cors-options.js' import { getHost } from '../../get-host.js' @@ -29,6 +28,7 @@ import { maskSensitiveEnvValues, } from '../../utils/env-encryption-helpers.js' import { ClearServiceLogsAction } from './actions/clear-service-logs-action.js' +import { CreateServiceAction } from './actions/create-service-action.js' import { ServiceBranchesAction } from './actions/service-branches-action.js' import { ServiceCheckoutAction } from './actions/service-checkout-action.js' import { ServiceDeleteBranchAction } from './actions/service-delete-branch-action.js' @@ -82,24 +82,6 @@ const resolveRelationsForServices = async ( return relationsMap } -const setServiceLinks = async ( - injector: Injector, - serviceId: string, - prerequisiteIds: string[], - prerequisiteServiceIds: string[], -) => { - const repo = getRepository(injector) - const prereqDs = repo.getDataSetFor(ServicePrerequisiteLink, 'id') - const depDs = repo.getDataSetFor(ServiceDependencyLink, 'id') - - for (const prereqId of prerequisiteIds) { - await prereqDs.add(injector, { id: `${serviceId}::${prereqId}`, serviceId, prerequisiteId: prereqId }) - } - for (const depId of prerequisiteServiceIds) { - await depDs.add(injector, { id: `${serviceId}::${depId}`, serviceId, dependsOnServiceId: depId }) - } -} - const replaceServiceLinks = async ( injector: Injector, serviceId: string, @@ -214,61 +196,7 @@ export const setupServicesRestApi = async (injector: Injector) => { ), }, POST: { - '/services': Validate({ schema: servicesApiSchema, schemaName: 'PostServiceEndpoint' })( - async ({ injector: i, getBody }) => { - const body = await getBody() - const repo = getRepository(i) - const crypto = i.get(CryptoService) - const now = new Date().toISOString() - const id = body.id ?? randomUUID() - - const prerequisiteIds = body.prerequisiteIds ?? [] - const prerequisiteServiceIds = body.prerequisiteServiceIds ?? [] - - const def = { - id, - stackName: body.stackName, - displayName: body.displayName, - description: body.description ?? '', - workingDirectory: body.workingDirectory, - repositoryId: body.repositoryId, - installCommand: body.installCommand, - buildCommand: body.buildCommand, - runCommand: body.runCommand, - files: body.files ?? [], - createdAt: now, - updatedAt: now, - } - - const config = { - serviceId: id, - autoFetchEnabled: body.autoFetchEnabled ?? false, - autoFetchIntervalMinutes: body.autoFetchIntervalMinutes ?? 60, - autoRestartOnFetch: body.autoRestartOnFetch ?? false, - environmentVariableOverrides: encryptEnvValues(crypto, body.environmentVariableOverrides ?? {}), - localFiles: encryptLocalFiles(crypto, body.localFiles ?? []), - createdAt: now, - updatedAt: now, - } - - const status = { - serviceId: id, - cloneStatus: 'not-cloned' as const, - installStatus: 'not-installed' as const, - buildStatus: 'not-built' as const, - runStatus: 'stopped' as const, - updatedAt: now, - } - - await repo.getDataSetFor(ServiceDefinition, 'id').add(i, def) - await repo.getDataSetFor(ServiceConfig, 'serviceId').add(i, config) - await repo.getDataSetFor(ServiceStatus, 'serviceId').add(i, status) - await setServiceLinks(i, id, prerequisiteIds, prerequisiteServiceIds) - - const relations = { prerequisiteIds, prerequisiteServiceIds } - return JsonResult(mergeServiceViewMasked(def, config, status, undefined, crypto, relations)) - }, - ), + '/services': Validate({ schema: servicesApiSchema, schemaName: 'PostServiceEndpoint' })(CreateServiceAction), '/services/:id/start': Validate({ schema: servicesApiSchema, schemaName: 'ServiceActionEndpoint' })( ServiceLifecycleAction('start'), ), diff --git a/service/src/app-models/stacks/actions/create-stack-action.spec.ts b/service/src/app-models/stacks/actions/create-stack-action.spec.ts new file mode 100644 index 0000000..d161f2c --- /dev/null +++ b/service/src/app-models/stacks/actions/create-stack-action.spec.ts @@ -0,0 +1,98 @@ +import { getDataSetFor } from '@furystack/repository' +import { randomBytes } from 'crypto' +import { beforeAll, describe, expect, it, vi } from 'vitest' + +import { StackConfigDataSet, StackDefinitionDataSet } from '../../data-store/tokens.js' +import { createMockActionContext, withTestInjector } from '../../../test-helpers.js' +import { CreateStackAction } from './create-stack-action.js' + +type CreateStackBody = Parameters[0] extends { getBody: () => Promise } ? B : never + +const baseBody: CreateStackBody = { + name: 'my-stack', + displayName: 'My Stack', + description: 'Example', + mainDirectory: '/tmp/my-stack', + environmentVariables: {}, +} + +beforeAll(() => { + process.env.STACK_CRAFT_ENCRYPTION_KEY = randomBytes(32).toString('base64') +}) + +describe('CreateStackAction', () => { + it('should persist the stack definition and config', async () => { + await withTestInjector(async ({ elevated }) => { + const result = await CreateStackAction({ + ...createMockActionContext({ injector: elevated, body: baseBody }), + request: {} as never, + }) + + expect(result.chunk.name).toBe('my-stack') + + const defs = await getDataSetFor(elevated, StackDefinitionDataSet).find(elevated, {}) + expect(defs).toHaveLength(1) + expect(defs[0]?.displayName).toBe('My Stack') + + const configs = await getDataSetFor(elevated, StackConfigDataSet).find(elevated, {}) + expect(configs).toHaveLength(1) + expect(configs[0]?.mainDirectory).toBe('/tmp/my-stack') + }) + }) + + it('should reject with 409 when a stack with the same name already exists', async () => { + await withTestInjector(async ({ elevated }) => { + const ts = new Date().toISOString() + await getDataSetFor(elevated, StackDefinitionDataSet).add(elevated, { + name: 'my-stack', + displayName: 'Original', + description: '', + createdAt: ts, + updatedAt: ts, + }) + + await expect( + CreateStackAction({ + ...createMockActionContext({ + injector: elevated, + body: { ...baseBody, displayName: 'Duplicate' }, + }), + request: {} as never, + }), + ).rejects.toMatchObject({ + message: expect.stringContaining('"my-stack" already exists'), + responseCode: 409, + }) + + const defs = await getDataSetFor(elevated, StackDefinitionDataSet).find(elevated, {}) + expect(defs).toHaveLength(1) + expect(defs[0]?.displayName).toBe('Original') + + const configs = await getDataSetFor(elevated, StackConfigDataSet).find(elevated, {}) + expect(configs).toHaveLength(0) + }) + }) + + it('should roll back the StackDefinition when the StackConfig insert fails', async () => { + await withTestInjector(async ({ elevated }) => { + const configDs = getDataSetFor(elevated, StackConfigDataSet) + vi.spyOn(configDs, 'add').mockRejectedValueOnce(new Error('config store down')) + + await expect( + CreateStackAction({ + ...createMockActionContext({ injector: elevated, body: baseBody }), + request: {} as never, + }), + ).rejects.toMatchObject({ + message: expect.stringContaining('config store down'), + responseCode: 500, + }) + + const defs = await getDataSetFor(elevated, StackDefinitionDataSet).find(elevated, {}) + expect(defs).toHaveLength(0) + + const configs = await getDataSetFor(elevated, StackConfigDataSet).find(elevated, {}) + expect(configs).toHaveLength(0) + }) + }) +}) diff --git a/service/src/app-models/stacks/actions/create-stack-action.ts b/service/src/app-models/stacks/actions/create-stack-action.ts new file mode 100644 index 0000000..148cd42 --- /dev/null +++ b/service/src/app-models/stacks/actions/create-stack-action.ts @@ -0,0 +1,67 @@ +import type { Injector } from '@furystack/inject' +import { RequestError } from '@furystack/rest' +import { JsonResult, type RequestAction } from '@furystack/rest-service' +import type { PostStackEndpoint } from 'common' +import { StackConfig, StackDefinition } from 'common' +import { randomUUID } from 'crypto' + +import { CryptoService } from '../../../utils/crypto-service.js' +import { encryptEnvValues } from '../../../utils/env-encryption-helpers.js' +import { legacyRepository as getRepository } from '../../../utils/legacy-repository.js' + +const assertNoStackNameCollision = async (injector: Injector, name: string | undefined): Promise => { + if (name === undefined) return + const stackDefDs = getRepository(injector).getDataSetFor(StackDefinition, 'name') + const existing = await stackDefDs.get(injector, name) + if (existing) { + throw new RequestError(`A stack named "${name}" already exists. Choose a different name.`, 409) + } +} + +/** + * POST action that creates a new {@link StackDefinition} together with its + * companion {@link StackConfig}. The two inserts are atomic from the caller's + * point of view: if `StackConfig.add` throws, the just-written + * `StackDefinition` row is removed before the error is re-thrown. + */ +export const CreateStackAction: RequestAction = async ({ injector, getBody }) => { + const body = await getBody() + const repo = getRepository(injector) + const crypto = injector.get(CryptoService) + const now = new Date().toISOString() + const stackDefDs = repo.getDataSetFor(StackDefinition, 'name') + const stackConfigDs = repo.getDataSetFor(StackConfig, 'stackName') + + await assertNoStackNameCollision(injector, body.name) + + const name = body.name ?? randomUUID() + const def = { + name, + displayName: body.displayName, + description: body.description ?? '', + createdAt: now, + updatedAt: now, + } + const config = { + stackName: name, + mainDirectory: body.mainDirectory, + environmentVariables: encryptEnvValues(crypto, body.environmentVariables ?? {}), + createdAt: now, + updatedAt: now, + } + + await stackDefDs.add(injector, def) + try { + await stackConfigDs.add(injector, config) + } catch (error) { + await stackDefDs.remove(injector, name).catch(() => undefined) + throw error instanceof RequestError + ? error + : new RequestError( + `Failed to create stack "${name}": ${error instanceof Error ? error.message : 'unknown error'}`, + 500, + ) + } + + return JsonResult({ ...def, ...config }) +} diff --git a/service/src/app-models/stacks/actions/import-export-actions.spec.ts b/service/src/app-models/stacks/actions/import-export-actions.spec.ts index 294b3e7..8bd9c67 100644 --- a/service/src/app-models/stacks/actions/import-export-actions.spec.ts +++ b/service/src/app-models/stacks/actions/import-export-actions.spec.ts @@ -458,6 +458,256 @@ describe('Import/Export Stack Actions', () => { }) }) + it('should reject import when a stack with the same name already exists', async () => { + const { injector, stackDefStore, serviceDefStore } = createSetup() + await usingAsync(injector, async () => { + const ts = now() + await stackDefStore.add({ + name: 'duplicate-stack', + displayName: 'Original', + description: 'Original description', + createdAt: ts, + updatedAt: ts, + }) + await serviceDefStore.add({ + id: 'original-svc', + stackName: 'duplicate-stack', + displayName: 'Original Service', + description: '', + workingDirectory: 'svc', + runCommand: 'echo original', + files: [], + createdAt: ts, + updatedAt: ts, + }) + + const importBody = { + stack: { + name: 'duplicate-stack', + displayName: 'Imported', + description: 'Imported description', + }, + services: [], + repositories: [], + prerequisites: [], + config: { mainDirectory: '/tmp/dup' }, + } + + const elevated = useSystemIdentityContext({ injector }) + await expect( + ImportStackAction(createMockActionContext({ injector: elevated, body: importBody })), + ).rejects.toMatchObject({ + message: expect.stringContaining('"duplicate-stack" already exists'), + responseCode: 409, + }) + + const stackDefs = await stackDefStore.find({}) + expect(stackDefs).toHaveLength(1) + expect(stackDefs[0]?.displayName).toBe('Original') + + const serviceDefs = await serviceDefStore.find({}) + expect(serviceDefs).toHaveLength(1) + expect(serviceDefs[0]?.id).toBe('original-svc') + }) + }) + + it('should reject import when a service id already exists in another stack', async () => { + const { injector, stackDefStore, serviceDefStore } = createSetup() + await usingAsync(injector, async () => { + const ts = now() + await stackDefStore.add({ + name: 'existing-stack', + displayName: 'Existing', + description: '', + createdAt: ts, + updatedAt: ts, + }) + await serviceDefStore.add({ + id: 'shared-svc-id', + stackName: 'existing-stack', + displayName: 'Existing Service', + description: '', + workingDirectory: 'svc', + runCommand: 'echo existing', + files: [], + createdAt: ts, + updatedAt: ts, + }) + + const importBody = { + stack: { + name: 'new-stack', + displayName: 'New Stack', + description: '', + }, + services: [ + { + id: 'shared-svc-id', + stackName: 'new-stack', + displayName: 'New Service', + description: '', + workingDirectory: 'svc', + runCommand: 'echo new', + prerequisiteIds: [], + prerequisiteServiceIds: [], + files: [], + }, + ], + repositories: [], + prerequisites: [], + config: { mainDirectory: '/tmp/conflict' }, + } + + const elevated = useSystemIdentityContext({ injector }) + await expect( + ImportStackAction(createMockActionContext({ injector: elevated, body: importBody })), + ).rejects.toMatchObject({ + message: expect.stringContaining('shared-svc-id'), + responseCode: 409, + }) + + const stackDefs = await stackDefStore.find({}) + expect(stackDefs).toHaveLength(1) + expect(stackDefs[0]?.name).toBe('existing-stack') + + const serviceDefs = await serviceDefStore.find({}) + expect(serviceDefs).toHaveLength(1) + expect(serviceDefs[0]?.displayName).toBe('Existing Service') + }) + }) + + it('should reject import when a repository id already exists', async () => { + const { injector, repoStore } = createSetup() + await usingAsync(injector, async () => { + const ts = now() + await repoStore.add({ + id: 'shared-repo-id', + stackName: 'other-stack', + url: 'https://github.com/existing/repo', + displayName: 'Existing Repo', + description: '', + createdAt: ts, + updatedAt: ts, + }) + + const importBody = { + stack: { name: 'new-stack', displayName: 'New Stack', description: '' }, + services: [], + repositories: [ + { + id: 'shared-repo-id', + stackName: 'new-stack', + url: 'https://github.com/new/repo', + displayName: 'New Repo', + description: '', + }, + ], + prerequisites: [], + config: { mainDirectory: '/tmp/repo-conflict' }, + } + + const elevated = useSystemIdentityContext({ injector }) + await expect( + ImportStackAction(createMockActionContext({ injector: elevated, body: importBody })), + ).rejects.toMatchObject({ + message: expect.stringContaining('repository(ies) [shared-repo-id]'), + responseCode: 409, + }) + + const repos = await repoStore.find({}) + expect(repos).toHaveLength(1) + expect(repos[0]?.url).toBe('https://github.com/existing/repo') + }) + }) + + it('should reject import when a prerequisite id already exists and list every conflicting id in the message', async () => { + const { injector, repoStore, prereqStore, serviceDefStore } = createSetup() + await usingAsync(injector, async () => { + const ts = now() + await prereqStore.add({ + id: 'shared-prereq-id', + stackName: 'other-stack', + name: 'Existing Prereq', + type: 'custom-script', + config: { script: 'echo existing' }, + installationHelp: '', + createdAt: ts, + updatedAt: ts, + }) + await repoStore.add({ + id: 'shared-repo-id', + stackName: 'other-stack', + url: 'https://github.com/existing/repo', + displayName: 'Existing Repo', + description: '', + createdAt: ts, + updatedAt: ts, + }) + await serviceDefStore.add({ + id: 'shared-svc-id', + stackName: 'other-stack', + displayName: 'Existing Service', + description: '', + workingDirectory: 'svc', + runCommand: 'echo existing', + files: [], + createdAt: ts, + updatedAt: ts, + }) + + const importBody = { + stack: { name: 'new-stack', displayName: 'New Stack', description: '' }, + services: [ + { + id: 'shared-svc-id', + stackName: 'new-stack', + displayName: 'New Service', + description: '', + workingDirectory: 'svc', + runCommand: 'echo new', + prerequisiteIds: [], + prerequisiteServiceIds: [], + files: [], + }, + ], + repositories: [ + { + id: 'shared-repo-id', + stackName: 'new-stack', + url: 'https://github.com/new/repo', + displayName: 'New Repo', + description: '', + }, + ], + prerequisites: [ + { + id: 'shared-prereq-id', + stackName: 'new-stack', + name: 'New Prereq', + type: 'custom-script' as const, + config: { script: 'echo new' }, + installationHelp: '', + }, + ], + config: { mainDirectory: '/tmp/multi-conflict' }, + } + + const elevated = useSystemIdentityContext({ injector }) + await expect( + ImportStackAction(createMockActionContext({ injector: elevated, body: importBody })), + ).rejects.toMatchObject({ + message: expect.stringMatching( + /service\(s\) \[shared-svc-id\].*repository\(ies\) \[shared-repo-id\].*prerequisite\(s\) \[shared-prereq-id\]/, + ), + responseCode: 409, + }) + + expect(await prereqStore.find({})).toHaveLength(1) + expect(await repoStore.find({})).toHaveLength(1) + expect(await serviceDefStore.find({})).toHaveLength(1) + }) + }) + it('should persist environment variable config during import', async () => { const { injector, stackConfigStore, serviceConfigStore } = createSetup() await usingAsync(injector, async () => { diff --git a/service/src/app-models/stacks/actions/import-stack-action.ts b/service/src/app-models/stacks/actions/import-stack-action.ts index fb7d740..bbc6b8f 100644 --- a/service/src/app-models/stacks/actions/import-stack-action.ts +++ b/service/src/app-models/stacks/actions/import-stack-action.ts @@ -1,3 +1,4 @@ +import type { Injector } from '@furystack/inject' import { getLogger } from '@furystack/logging' import { RequestError } from '@furystack/rest' import { JsonResult, type RequestAction } from '@furystack/rest-service' @@ -20,6 +21,47 @@ import { CryptoService } from '../../../utils/crypto-service.js' import { encryptEnvValues, encryptLocalFiles } from '../../../utils/env-encryption-helpers.js' import { legacyRepository as getRepository } from '../../../utils/legacy-repository.js' +// Pre-flight check: surface conflicts BEFORE any mutation so the destructive +// rollback path is never entered for an avoidable collision (which would +// otherwise delete the existing records that share the colliding ids/name). +const assertNoImportConflicts = async (injector: Injector, body: ImportStackEndpoint['body']): Promise => { + const repository = getRepository(injector) + const stackDefDs = repository.getDataSetFor(StackDefinition, 'name') + const svcDefDs = repository.getDataSetFor(ServiceDefinition, 'id') + const repoDs = repository.getDataSetFor(GitHubRepository, 'id') + const prereqDs = repository.getDataSetFor(Prerequisite, 'id') + + const stackName = body.stack.name + const existingStack = await stackDefDs.get(injector, stackName) + if (existingStack) { + throw new RequestError( + `A stack named "${stackName}" already exists. Rename or delete the existing stack before importing.`, + 409, + ) + } + + const [svcHits, repoHits, prereqHits] = await Promise.all([ + Promise.all(body.services.map(async (s) => ((await svcDefDs.get(injector, s.id)) ? s.id : null))), + Promise.all(body.repositories.map(async (r) => ((await repoDs.get(injector, r.id)) ? r.id : null))), + Promise.all(body.prerequisites.map(async (p) => ((await prereqDs.get(injector, p.id)) ? p.id : null))), + ]) + + const conflictingServiceIds = svcHits.filter((id): id is string => id !== null) + const conflictingRepoIds = repoHits.filter((id): id is string => id !== null) + const conflictingPrereqIds = prereqHits.filter((id): id is string => id !== null) + + const parts: string[] = [] + if (conflictingServiceIds.length > 0) parts.push(`service(s) [${conflictingServiceIds.join(', ')}]`) + if (conflictingRepoIds.length > 0) parts.push(`repository(ies) [${conflictingRepoIds.join(', ')}]`) + if (conflictingPrereqIds.length > 0) parts.push(`prerequisite(s) [${conflictingPrereqIds.join(', ')}]`) + if (parts.length > 0) { + throw new RequestError( + `Cannot import "${stackName}": the following entity IDs already exist: ${parts.join('; ')}. Each entity must have a unique ID.`, + 409, + ) + } +} + export const ImportStackAction: RequestAction = async ({ injector, getBody }) => { const logger = getLogger(injector).withScope('ImportStack') const body = await getBody() @@ -30,6 +72,8 @@ export const ImportStackAction: RequestAction = async ({ in await logger.information({ message: `Importing stack: ${stackName}` }) + await assertNoImportConflicts(injector, body) + const stackDefDs = repository.getDataSetFor(StackDefinition, 'name') const stackConfigDs = repository.getDataSetFor(StackConfig, 'stackName') const repoDs = repository.getDataSetFor(GitHubRepository, 'id') diff --git a/service/src/app-models/stacks/setup-stacks-rest-api.ts b/service/src/app-models/stacks/setup-stacks-rest-api.ts index 6e06082..61c48e5 100644 --- a/service/src/app-models/stacks/setup-stacks-rest-api.ts +++ b/service/src/app-models/stacks/setup-stacks-rest-api.ts @@ -14,7 +14,6 @@ import { StackDefinition, } from 'common' import stacksApiSchema from 'common/schemas/stacks-api.json' with { type: 'json' } -import { randomUUID } from 'crypto' import { getCurrentUser } from '@furystack/core' @@ -24,6 +23,7 @@ import { getPort } from '../../get-port.js' import { ProcessManager } from '../../services/process-manager.js' import { CryptoService, SENSITIVE_VALUE_MASK } from '../../utils/crypto-service.js' import { encryptEnvValues, maskSensitiveEnvValues } from '../../utils/env-encryption-helpers.js' +import { CreateStackAction } from './actions/create-stack-action.js' import { ExportStackAction } from './actions/export-stack-action.js' import { ImportStackAction } from './actions/import-stack-action.js' import { legacyRepository as getRepository } from '../../utils/legacy-repository.js' @@ -96,32 +96,7 @@ export const setupStacksRestApi = async (injector: Injector) => { ), }, POST: { - '/stacks': Validate({ schema: stacksApiSchema, schemaName: 'PostStackEndpoint' })( - async ({ injector: i, getBody }) => { - const body = await getBody() - const repo = getRepository(i) - const crypto = i.get(CryptoService) - const now = new Date().toISOString() - const name = body.name ?? randomUUID() - const def = { - name, - displayName: body.displayName, - description: body.description ?? '', - createdAt: now, - updatedAt: now, - } - const config = { - stackName: name, - mainDirectory: body.mainDirectory, - environmentVariables: encryptEnvValues(crypto, body.environmentVariables ?? {}), - createdAt: now, - updatedAt: now, - } - await repo.getDataSetFor(StackDefinition, 'name').add(i, def) - await repo.getDataSetFor(StackConfig, 'stackName').add(i, config) - return JsonResult({ ...def, ...config }) - }, - ), + '/stacks': Validate({ schema: stacksApiSchema, schemaName: 'PostStackEndpoint' })(CreateStackAction), '/stacks/import': Validate({ schema: stacksApiSchema, schemaName: 'ImportStackEndpoint' })(ImportStackAction), '/stacks/:id/setup': Validate({ schema: stacksApiSchema, schemaName: 'StackSetupEndpoint' })( async ({ injector: i, getUrlParams }) => {