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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 21 additions & 4 deletions src/@types/Services.ts
Original file line number Diff line number Diff line change
Expand Up @@ -152,18 +152,35 @@ export interface ServicePayment {
// so a plaintext secret can never be forwarded unencrypted.
export type ServiceUserData = Record<string, unknown>

export interface ServiceStartParams {
environment: string // required: the envId to run the service on
image: string // base image name (or build label when dockerfile is set)
// The container specification shared by serviceStart and serviceRestart. Every field is
// optional here; the consuming type decides which are required (serviceStart re-declares
// `image` as mandatory). `userData` is always ECIES-encrypted to the node before sending.
export interface ServiceContainerSpec {
image?: string // base image name (or build label when dockerfile is set)
tag?: string // pull by name:tag
checksum?: string // pull by digest: "sha256:<64 hex>"
dockerfile?: string // build from inline Dockerfile; requires allowImageBuild on the env
additionalDockerFiles?: Record<string, string>
userData?: ServiceUserData
dockerCmd?: string[] // exec-form CMD override (no shell)
dockerEntrypoint?: string[]
}

// Optional container-spec overrides for serviceRestart. The node treats the restart
// atomically ("all-or-nothing"): providing ANY of image/tag/checksum/dockerfile/
// additionalDockerFiles switches it into RESPEC mode — the container is rebuilt entirely
// from these values (`image` becomes mandatory, and exactly one of tag/checksum/dockerfile
// applies) instead of being bounced on the stored spec (REUSE mode). Passing none of them
// reuses the stored container spec unchanged. `userData`/`dockerCmd`/`dockerEntrypoint`
// keep their replace-when-supplied semantics: an omitted value reuses the stored one, an
// explicit value (including `[]`) REPLACES it.
export type ServiceRestartParams = ServiceContainerSpec
Comment on lines +169 to +177

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Make invalid RESPEC combinations unrepresentable.

ServiceRestartParams = ServiceContainerSpec accepts { tag }, { additionalDockerFiles }, or multiple source selectors without image. The documented protocol requires RESPEC requests to include image and exactly one of tag/checksum/dockerfile; both providers forward these invalid shapes to the node. Model REUSE and RESPEC as distinct union members and validate untyped JS inputs before dispatch.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/`@types/Services.ts around lines 169 - 177, Update ServiceRestartParams
to model REUSE and RESPEC as a discriminated union: REUSE must allow no
container-spec override fields, while RESPEC requires image and exactly one of
tag, checksum, or dockerfile, with additionalDockerFiles only valid in RESPEC.
Add runtime validation at the serviceRestart dispatch boundary so untyped
JavaScript inputs matching invalid combinations are rejected before being
forwarded to the node.


export interface ServiceStartParams extends ServiceContainerSpec {
environment: string // required: the envId to run the service on
image: string // required for start (base image name, or build label when dockerfile is set)
exposedPorts?: number[]
resources?: ComputeResourceRequest[]
duration: number // seconds; capped by serviceOnDemand.maxDurationSeconds
userData?: ServiceUserData
payment: ServicePayment
}
10 changes: 3 additions & 7 deletions src/services/providers/BaseProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,9 @@ import {
ServiceJob,
ServiceJobListed,
ServiceListFilters,
ServiceRestartParams,
ServiceTemplatePublic,
ServiceStartParams,
ServiceUserData,
ServicePayment,
OceanNode,
NodeP2P,
Expand Down Expand Up @@ -860,18 +860,14 @@ export class BaseProvider {
nodeUri: OceanNode,
signerOrAuthToken: SignerOrAuthTokenOrSignature,
serviceId: string,
userData?: ServiceUserData,
dockerCmd?: string[],
dockerEntrypoint?: string[],
params?: ServiceRestartParams,
signal?: AbortSignal
): Promise<ServiceJob[]> {
return this.getImpl(nodeUri).serviceRestart(
nodeUri,
signerOrAuthToken,
serviceId,
userData,
dockerCmd,
dockerEntrypoint,
params,
signal
)
}
Expand Down
39 changes: 30 additions & 9 deletions src/services/providers/HttpProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import {
ServiceJob,
ServiceJobListed,
ServiceListFilters,
ServiceRestartParams,
ServiceTemplatePublic,
ServiceStartParams,
ServiceUserData,
Expand Down Expand Up @@ -2123,21 +2124,36 @@ export class HttpProvider {
}

/**
* Restarts a running service (recreates the container). When `userData`, `dockerCmd` or
* `dockerEntrypoint` is supplied it REPLACES the stored value;
* otherwise the stored one is reused. Passing a new `dockerCmd` swaps the launch command
* (e.g. a different model) while keeping the same serviceId, host port and expiry.
* Restarts a running service (recreates the container) while keeping the same serviceId,
* payment, resources, host port(s) and expiry.
*
* The node treats the restart atomically. Passing no container-spec fields in `params`
* (REUSE mode) bounces the container on its stored spec unchanged. Passing ANY of
* `image`/`tag`/`checksum`/`dockerfile`/`additionalDockerFiles` (RESPEC mode) rebuilds the
* container entirely from `params`: `image` becomes mandatory, exactly one of
* `tag`/`checksum`/`dockerfile` applies, and a `dockerfile` requires `allowImageBuild` on
* the env (else the node replies 403). `userData`/`dockerCmd`/`dockerEntrypoint` are only
* sent when supplied — an omitted override reuses the node's stored value, whereas an
* explicit value (including `[]`) REPLACES it (matches ocean-node's restartService semantics).
* @return {Promise<ServiceJob[]>} The restarted service job (single-element array).
*/
public async serviceRestart(
nodeUri: string,
signerOrAuthToken: SignerOrAuthTokenOrSignature,
serviceId: string,
userData?: ServiceUserData,
dockerCmd?: string[],
dockerEntrypoint?: string[],
params?: ServiceRestartParams,
signal?: AbortSignal
): Promise<ServiceJob[]> {
const {
image,
tag,
checksum,
dockerfile,
additionalDockerFiles,
userData,
dockerCmd,
dockerEntrypoint
} = params ?? {}
const providerEndpoints = await this.getEndpoints(nodeUri)
const serviceEndpoints = await this.getServiceEndpoints(nodeUri, providerEndpoints)
const route = this.resolveServiceRoute(nodeUri, serviceEndpoints, 'serviceRestart')
Expand All @@ -2160,8 +2176,13 @@ export class HttpProvider {
...authPayload,
serviceId,
userData: await this.encryptServiceUserData(nodeUri, userData),
// Only send when supplied — an omitted override reuses the node's stored value, whereas an
// explicit [] REPLACES it with "no override" (matches ocean-node's restartService semantics).
// Only send when supplied — an omitted field reuses the node's stored value, whereas an
// explicit value REPLACES it (matches ocean-node's restartService REUSE/RESPEC semantics).
...(image !== undefined ? { image } : {}),
...(tag !== undefined ? { tag } : {}),
...(checksum !== undefined ? { checksum } : {}),
...(dockerfile !== undefined ? { dockerfile } : {}),
...(additionalDockerFiles !== undefined ? { additionalDockerFiles } : {}),
...(dockerCmd !== undefined ? { dockerCmd } : {}),
...(dockerEntrypoint !== undefined ? { dockerEntrypoint } : {})
}),
Expand Down
24 changes: 19 additions & 5 deletions src/services/providers/P2pProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ import {
ServiceJob,
ServiceJobListed,
ServiceListFilters,
ServiceRestartParams,
ServiceTemplatePublic,
ServiceStartParams,
ServiceUserData,
Expand Down Expand Up @@ -1866,11 +1867,19 @@ export class P2pProvider {
nodeUri: OceanNode,
signerOrAuthToken: SignerOrAuthTokenOrSignature,
serviceId: string,
userData?: ServiceUserData,
dockerCmd?: string[],
dockerEntrypoint?: string[],
params?: ServiceRestartParams,
signal?: AbortSignal
): Promise<ServiceJob[]> {
const {
image,
tag,
checksum,
dockerfile,
additionalDockerFiles,
userData,
dockerCmd,
dockerEntrypoint
} = params ?? {}
const authPayload = await this.getSignedCommandParams(
nodeUri,
signerOrAuthToken,
Expand All @@ -1884,8 +1893,13 @@ export class P2pProvider {
...authPayload,
serviceId,
userData: await this.encryptServiceUserData(nodeUri, userData),
// Only send when supplied — an omitted override reuses the node's stored value, whereas an
// explicit [] REPLACES it with "no override" (matches ocean-node's restartService semantics).
// Only send when supplied — an omitted field reuses the node's stored value, whereas an
// explicit value REPLACES it (matches ocean-node's restartService REUSE/RESPEC semantics).
...(image !== undefined ? { image } : {}),
...(tag !== undefined ? { tag } : {}),
...(checksum !== undefined ? { checksum } : {}),
...(dockerfile !== undefined ? { dockerfile } : {}),
...(additionalDockerFiles !== undefined ? { additionalDockerFiles } : {}),
...(dockerCmd !== undefined ? { dockerCmd } : {}),
...(dockerEntrypoint !== undefined ? { dockerEntrypoint } : {})
},
Expand Down
2 changes: 0 additions & 2 deletions test/integration/Services.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -373,8 +373,6 @@ describe('Service on Demand flow tests', () => {
consumerAccount,
serviceId,
undefined,
undefined,
undefined,
opSignal()
)
const running = await pollUntil(
Expand Down
Loading