diff --git a/.changeset/30762-cloud-binding-service-name.md b/.changeset/30762-cloud-binding-service-name.md new file mode 100644 index 00000000000..506ad4b780a --- /dev/null +++ b/.changeset/30762-cloud-binding-service-name.md @@ -0,0 +1,20 @@ +--- +'@objectstack/spec': patch +'@objectstack/cloud-connection': patch +'@objectstack/service-push': patch +--- + +fix(cloud-connection): name the `cloud-binding` service once, in its contract, and read an empty `controlPlaneUrl` one way + +Clause-②: yes (widening) +Metadata-protocol: none + +`@objectstack/spec/contracts` now exports `CLOUD_BINDING_SERVICE` (`'cloud-binding'`) beside `ICloudBindingService`, the way the sibling contracts export their service names. `@objectstack/cloud-connection` registers the binding under it and `@objectstack/service-push`'s relay sender reads it, so the registrar and the reader can no longer spell the name two ways. `@objectstack/service-push` keeps exporting `CLOUD_BINDING_SERVICE`, now a re-export of the contract's constant with the same value. + +`CloudConnectionPlugin`'s routes now take their control-plane URL from the `cloud-binding` service they register, so the routes and the binding read the `controlPlaneUrl` option the same way: + +- An explicit empty (or blank) `controlPlaneUrl` means "cloud not configured" for both: the routes answer `CLOUD_UNCONFIGURED` as before, and `controlPlaneUrl()` now answers `''` instead of the production default. +- A declined `OS_CLOUD_URL` (`off`, `none`, `local`, `disabled`) under an absent option is "cloud not configured" for the routes too, as it already was for the binding. +- An absent option falls through to `OS_CLOUD_URL`, then the ObjectStack-operated default, for the routes too, as it already did for the binding and for the marketplace plugins. + +`objectstack serve` and the showcase app are unchanged: they compose the plugin only with `resolveCloudUrl()`'s non-empty answer, which both readings return as is. diff --git a/packages/cloud-connection/src/cloud-binding.test.ts b/packages/cloud-connection/src/cloud-binding.test.ts index d6ba71ede76..14df7419eed 100644 --- a/packages/cloud-connection/src/cloud-binding.test.ts +++ b/packages/cloud-connection/src/cloud-binding.test.ts @@ -11,8 +11,8 @@ import { mkdtempSync, rmSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -import type { ICloudBindingService } from '@objectstack/spec/contracts'; -import { CLOUD_BINDING_SERVICE, createCloudBinding } from './cloud-binding.js'; +import { CLOUD_BINDING_SERVICE, type ICloudBindingService } from '@objectstack/spec/contracts'; +import { createCloudBinding } from './cloud-binding.js'; import { CloudConnectionPlugin } from './cloud-connection-plugin.js'; import { ConnectionCredentialStore } from './connection-credential-store.js'; import { DEFAULT_CLOUD_URL } from './cloud-url.js'; @@ -70,9 +70,22 @@ describe('createCloudBinding — the control-plane URL', () => { vi.stubEnv('OS_CLOUD_URL', 'off'); expect(createCloudBinding({ store }).controlPlaneUrl()).toBe(''); }); + + // [#30762] An explicit empty option is the host saying "no control plane", + // the reading the plugin's routes always gave it. It does not fall through + // to OS_CLOUD_URL or the default the way an absent option does. + it.each(['', ' '])('is empty for an explicit empty option %j, over OS_CLOUD_URL and the default', (option) => { + expect(createCloudBinding({ store, controlPlaneUrl: option }).controlPlaneUrl()).toBe(''); + vi.stubEnv('OS_CLOUD_URL', 'https://env.example.com'); + expect(createCloudBinding({ store, controlPlaneUrl: option }).controlPlaneUrl()).toBe(''); + }); }); describe('CloudConnectionPlugin — registers the binding in init()', () => { + it('registers it under the name the contract exports, the one service-push reads', () => { + expect(CLOUD_BINDING_SERVICE).toBe('cloud-binding'); + }); + it('registers cloud-binding, declared in providesServices, reading the plugin\'s own credential store', async () => { const services = new Map(); const plugin = new CloudConnectionPlugin({ credentialPath: join(dir, 'cloud-connection.json'), controlPlaneUrl: 'https://cp.example.com' }); diff --git a/packages/cloud-connection/src/cloud-binding.ts b/packages/cloud-connection/src/cloud-binding.ts index 918402fc12a..653f54d3576 100644 --- a/packages/cloud-connection/src/cloud-binding.ts +++ b/packages/cloud-connection/src/cloud-binding.ts @@ -19,13 +19,14 @@ import type { ICloudBindingService } from '@objectstack/spec/contracts'; import { resolveCloudUrl } from './cloud-url.js'; import type { ConnectionCredentialStore } from './connection-credential-store.js'; -/** The kernel service name {@link createCloudBinding}'s answer is registered under. */ -export const CLOUD_BINDING_SERVICE = 'cloud-binding'; - export interface CloudBindingOptions { /** The plugin's `controlPlaneApiKey` option (the env→cloud service key); `OS_CLOUD_API_KEY` when absent. */ controlPlaneApiKey?: string; - /** The plugin's `controlPlaneUrl` option; `OS_CLOUD_URL`, then the default, when absent. */ + /** + * The plugin's `controlPlaneUrl` option; `OS_CLOUD_URL`, then the default, + * when absent. An explicit empty (or blank) value declares that this + * server has no control plane, the same as a decline spelling. + */ controlPlaneUrl?: string; /** Where a self-hosted bind persisted its `oscc_` bearer. */ store: Pick; @@ -40,6 +41,13 @@ export interface CloudBindingOptions { * derivation `isCloudManaged` reads in `@objectstack/core` * (`resolveControlPlaneApiKey`), so "platform-managed" and "which bearer" * cannot disagree. + * + * The control-plane URL is {@link resolveCloudUrl}'s answer, with one + * exception: an explicit empty option answers `''`, "no control plane". + * `resolveCloudUrl` reads that `''` as unset and falls through to the + * environment and the default, while this plugin's own routes read it as + * "cloud not configured". The routes read their URL through this member, so + * the binding and the routes give the one option one reading. */ export function createCloudBinding(options: CloudBindingOptions): ICloudBindingService { const credential = (): string => @@ -47,6 +55,12 @@ export function createCloudBinding(options: CloudBindingOptions): ICloudBindingS return { credential, bound: () => credential() !== '', - controlPlaneUrl: () => resolveCloudUrl(options.controlPlaneUrl), + controlPlaneUrl: () => resolveBindingUrl(options.controlPlaneUrl), }; } + +/** {@link resolveCloudUrl}, except that an explicit empty (or blank) option answers `''`. */ +function resolveBindingUrl(explicit: string | undefined): string { + if (typeof explicit === 'string' && explicit.trim() === '') return ''; + return resolveCloudUrl(explicit); +} diff --git a/packages/cloud-connection/src/cloud-connection-plugin.test.ts b/packages/cloud-connection/src/cloud-connection-plugin.test.ts index 53716e3e333..23cc59b21c5 100644 --- a/packages/cloud-connection/src/cloud-connection-plugin.test.ts +++ b/packages/cloud-connection/src/cloud-connection-plugin.test.ts @@ -10,6 +10,7 @@ */ import { describe, expect, it, vi, beforeEach, afterEach } from 'vitest'; +import { CLOUD_BINDING_SERVICE, type ICloudBindingService } from '@objectstack/spec/contracts'; import { CloudConnectionPlugin, createCloudConnectionPlugin } from './cloud-connection-plugin.js'; import { bindCatalogFromTables } from './__tests__/security-catalog.testkit.js'; @@ -268,3 +269,64 @@ describe('single-environment mode (host auth, fixed env id)', () => { expect(res.payload.data.environmentId).toBe('env-from-env-var'); }); }); + +/** + * [#30762] One reading of the control-plane URL. The routes read it through + * the `cloud-binding` service the plugin registers, the same answer the push + * relay sender reads, so the two cannot disagree on an option. An explicit + * empty option is "cloud not configured" for both, and does not fall through + * to `OS_CLOUD_URL` or the default. + */ +describe('CloudConnectionPlugin — the routes and the binding read the control-plane URL once', () => { + const ENV = 'env-123'; + const services = { + 'env-registry': { resolveByHostname: async () => ({ environmentId: ENV }) }, + 'kernel-manager': { getOrCreate: async () => ({ getServiceAsync: async () => sessionAuth('user-1') }) }, + }; + + afterEach(() => { vi.unstubAllEnvs(); }); + + /** Boot the plugin, drive `GET /installed` once, and answer where it went and what the binding says. */ + async function readBoth(controlPlaneUrl: string | undefined) { + const rawApp = makeRawApp(); + const { ctx, fireKernelReady } = makeCtx({ rawApp, services }); + const registered = new Map(); + const fetchSpy = vi.fn(async (_url: string) => new Response(JSON.stringify({ success: true, data: { packages: [], total: 0 } }), { status: 200 })); + vi.stubGlobal('fetch', fetchSpy); + const plugin = new CloudConnectionPlugin({ controlPlaneApiKey: 'svc-key', ...(controlPlaneUrl === undefined ? {} : { controlPlaneUrl }) }); + await plugin.init({ ...ctx, registerService: (name: string, svc: unknown) => registered.set(name, svc) } as any); + await plugin.start(ctx as any); + await fireKernelReady(); + const res = await rawApp.routes.get('GET /api/v1/cloud-connection/installed')!( + makeC('https://t1.example.com/api/v1/cloud-connection/installed'), + ); + const binding = registered.get(CLOUD_BINDING_SERVICE) as ICloudBindingService; + const called = fetchSpy.mock.calls.map(([url]) => String(url)); + return { res, binding: binding.controlPlaneUrl(), called }; + } + + it.each(['', ' '])('an explicit empty option %j is "cloud not configured" for both, over OS_CLOUD_URL', async (option) => { + vi.stubEnv('OS_CLOUD_URL', 'https://env.example.com'); + const { res, binding, called } = await readBoth(option); + expect(binding).toBe(''); + expect(called).toEqual([]); + expect(res.payload).toEqual({ success: true, data: { packages: [], total: 0, connected: false } }); + }); + + it('a declined OS_CLOUD_URL under an absent option is "cloud not configured" for both', async () => { + vi.stubEnv('OS_CLOUD_URL', 'off'); + const { binding, called } = await readBoth(undefined); + expect(binding).toBe(''); + expect(called).toEqual([]); + }); + + it.each([ + ['the configured URL', 'https://cp.example.com/', 'https://env.example.com', 'https://cp.example.com'], + ['OS_CLOUD_URL under an absent option', undefined, 'https://env.example.com/', 'https://env.example.com'], + ])('%s: the routes call the URL the binding answers', async (_label, option, env, expected) => { + vi.stubEnv('OS_CLOUD_URL', env); + const { binding, called } = await readBoth(option); + expect(binding).toBe(expected); + expect(called).toEqual([`${expected}/api/v1/cloud/environments/${ENV}/packages`]); + }); +}); diff --git a/packages/cloud-connection/src/cloud-connection-plugin.ts b/packages/cloud-connection/src/cloud-connection-plugin.ts index 8fcd5d514ed..a955339a953 100644 --- a/packages/cloud-connection/src/cloud-connection-plugin.ts +++ b/packages/cloud-connection/src/cloud-connection-plugin.ts @@ -149,8 +149,8 @@ import { import type { AuthSessionApi, IAuthService, IObjectQLEngine } from '@objectstack/spec/contracts'; import { inProcessSessionReadInput } from '@objectstack/types'; import { ConnectionCredentialStore } from './connection-credential-store.js'; -import { CLOUD_BINDING_SERVICE, createCloudBinding } from './cloud-binding.js'; -import type { ICloudBindingService } from '@objectstack/spec/contracts'; +import { createCloudBinding } from './cloud-binding.js'; +import { CLOUD_BINDING_SERVICE, type ICloudBindingService } from '@objectstack/spec/contracts'; import { CLOUD_CONNECTION_CAPABILITY, CLOUD_CONNECTION_UI_BUNDLE } from './cloud-connection-ui.js'; import { readJsonRequestBody } from './request-json-body.js'; @@ -266,7 +266,13 @@ interface ResolvedSession { const ENVIRONMENT_NOT_FOUND_MESSAGE = 'Could not resolve an environment for this request host.'; export interface CloudConnectionPluginConfig { - /** Control-plane base URL. Default: `OS_CLOUD_URL` (read lazily at kernel:ready). */ + /** + * Control-plane base URL. Default: `OS_CLOUD_URL` (read lazily at + * kernel:ready), then the ObjectStack-operated default. An explicit `''`, + * or a decline spelling (`off`, `none`, `local`, `disabled`), means "cloud + * not configured": the routes answer `CLOUD_UNCONFIGURED` and the + * `cloud-binding` service's `controlPlaneUrl()` answers `''`. + */ controlPlaneUrl?: string; /** env→cloud service credential. Default: `OS_CLOUD_API_KEY`. */ controlPlaneApiKey?: string; @@ -333,8 +339,12 @@ export class CloudConnectionPlugin implements Plugin { const rawApp = httpServer.getRawApp(); // Env vars are read here (not at construction) so a host can set // them between config load and kernel boot — preserves the - // behavior of the original app-level wiring. - const cloudUrl = (this.cfg.controlPlaneUrl ?? process.env.OS_CLOUD_URL ?? '').trim().replace(/\/+$/, ''); + // behavior of the original app-level wiring. [#30762] The URL is + // the `cloud-binding` service's answer, the one the push relay + // sender reads too: an explicit empty option means "cloud not + // configured" for both, and an absent one falls through to + // `OS_CLOUD_URL`, then the default, as `resolveCloudUrl` does. + const cloudUrl = this.binding.controlPlaneUrl(); const cloudApiKey = resolveControlPlaneApiKey(this.cfg.controlPlaneApiKey); const deviceClientId = (this.cfg.deviceClientId ?? process.env.OS_CLI_CLIENT_ID ?? 'objectstack-cli').trim(); const deviceScope = 'openid profile email'; diff --git a/packages/services/service-push/src/push-plugin.test.ts b/packages/services/service-push/src/push-plugin.test.ts index c10ef7e428c..db10034bc59 100644 --- a/packages/services/service-push/src/push-plugin.test.ts +++ b/packages/services/service-push/src/push-plugin.test.ts @@ -6,7 +6,7 @@ */ import { afterEach, beforeEach, describe, it, expect, vi } from 'vitest'; -import type { ICloudBindingService } from '@objectstack/spec/contracts'; +import { CLOUD_BINDING_SERVICE, type ICloudBindingService } from '@objectstack/spec/contracts'; import { PushServicePlugin, PUSH_RELAY_AIR_GAPPED_MESSAGE } from './push-plugin.js'; import { PushService, type IPushTransport, type PushMessage } from './push-service.js'; import { envPushCredentialResolver } from './credential.js'; @@ -98,7 +98,7 @@ describe('PushServicePlugin — the hosted relay', () => { it('bound: lists both vendors and sends one POST carrying the binding credential, which no log line carries', async () => { const { ctx, services, lines, fire } = context(); - services.set('cloud-binding', cloudBinding(() => SECRET)); + services.set(CLOUD_BINDING_SERVICE, cloudBinding(() => SECRET)); const calls: Array<{ url: string; headers: Record }> = []; const relayFetch = (async (url: string, init: RequestInit) => { calls.push({ url, headers: init.headers as Record }); @@ -122,7 +122,7 @@ describe('PushServicePlugin — the hosted relay', () => { it('unbound at kernel:ready: warns once, and the bind takes effect without a restart', async () => { const { ctx, services, lines, fire } = context(); let credential = ''; - services.set('cloud-binding', cloudBinding(() => credential)); + services.set(CLOUD_BINDING_SERVICE, cloudBinding(() => credential)); const plugin = new PushServicePlugin({ config: {} }); await plugin.init(ctx); await plugin.start(ctx); diff --git a/packages/services/service-push/src/transports/relay.ts b/packages/services/service-push/src/transports/relay.ts index e8864ce45fb..0287acf9860 100644 --- a/packages/services/service-push/src/transports/relay.ts +++ b/packages/services/service-push/src/transports/relay.ts @@ -7,7 +7,7 @@ import { type PushRelayPayload, type PushRelayRequest, } from '@objectstack/spec/api'; -import type { ICloudBindingService } from '@objectstack/spec/contracts'; +import { CLOUD_BINDING_SERVICE, type ICloudBindingService } from '@objectstack/spec/contracts'; import type { PushRelayTransportConfigParsed, PushVendor } from '@objectstack/spec/system'; import type { IPushTransport, PushMessage, PushSendResult } from '../push-service.js'; @@ -22,8 +22,13 @@ import type { IPushTransport, PushMessage, PushSendResult } from '../push-servic */ export const PUSH_RELAY_PATH = '/api/v1/cloud/push/send'; -/** The kernel service the binding credential is read from (`ICloudBindingService`, `@objectstack/cloud-connection`). */ -export const CLOUD_BINDING_SERVICE = 'cloud-binding'; +/** + * The kernel service the binding credential is read from (`ICloudBindingService`, + * registered by `@objectstack/cloud-connection`). The name lives beside the + * contract in `@objectstack/spec/contracts`; re-exported here so this + * package's existing public export keeps its spelling and its value. + */ +export { CLOUD_BINDING_SERVICE }; /** * The visible line a relay delivery shows when the caller names none: the diff --git a/packages/services/service-push/vitest.config.ts b/packages/services/service-push/vitest.config.ts index 89ca038fe8c..fd7a9ad4751 100644 --- a/packages/services/service-push/vitest.config.ts +++ b/packages/services/service-push/vitest.config.ts @@ -24,6 +24,10 @@ export default defineConfig({ // from `@objectstack/core`: both from source, for the same reason. { find: /^@objectstack\/spec\/api$/, replacement: path.resolve(__dirname, '../../spec/src/api/index.ts') }, { find: /^@objectstack\/core$/, replacement: path.resolve(__dirname, '../../core/src/index.ts') }, + // The relay sender reads the `cloud-binding` service name from the + // contract (`CLOUD_BINDING_SERVICE`, `@objectstack/spec/contracts`): from + // source, for the same reason. + { find: /^@objectstack\/spec\/contracts$/, replacement: path.resolve(__dirname, '../../spec/src/contracts/index.ts') }, ], }, }); diff --git a/packages/spec/api-surface/contracts.json b/packages/spec/api-surface/contracts.json index 705bfd732df..923ee5c020a 100644 --- a/packages/spec/api-surface/contracts.json +++ b/packages/spec/api-surface/contracts.json @@ -58,6 +58,7 @@ "AuthoredRowWriteVerdict (type)", "AutomationContext (interface)", "AutomationResult (interface)", + "CLOUD_BINDING_SERVICE (const)", "CRYPTO_CONTEXT_SCOPES (const)", "CacheStats (interface)", "ChatWithToolsOptions (interface)", diff --git a/packages/spec/export-origins/contracts.json b/packages/spec/export-origins/contracts.json index e9d1ae8ed9d..e02bbb4de6f 100644 --- a/packages/spec/export-origins/contracts.json +++ b/packages/spec/export-origins/contracts.json @@ -58,6 +58,7 @@ "AuthoredRowWriteVerdict": "src/contracts/security-service.ts#AuthoredRowWriteVerdict (type)", "AutomationContext": "src/contracts/automation-service.ts#AutomationContext (interface)", "AutomationResult": "src/contracts/automation-service.ts#AutomationResult (interface)", + "CLOUD_BINDING_SERVICE": "src/contracts/cloud-binding-service.ts#CLOUD_BINDING_SERVICE (const)", "CRYPTO_CONTEXT_SCOPES": "src/contracts/crypto-provider.ts#CRYPTO_CONTEXT_SCOPES (const)", "CacheStats": "src/contracts/cache-service.ts#CacheStats (interface)", "ChatWithToolsOptions": "src/contracts/ai-service.ts#ChatWithToolsOptions (interface)", diff --git a/packages/spec/src/contracts/cloud-binding-service.ts b/packages/spec/src/contracts/cloud-binding-service.ts index 401c87bb668..9b7882a7325 100644 --- a/packages/spec/src/contracts/cloud-binding-service.ts +++ b/packages/spec/src/contracts/cloud-binding-service.ts @@ -39,7 +39,16 @@ export interface ICloudBindingService { * The control-plane base URL the binding is made against, without a * trailing slash: the configured value, else `OS_CLOUD_URL`, else the * ObjectStack-operated default. `''` when the deployment declared that it - * has no control plane (`OS_CLOUD_URL=off`, `none`, `local`, `disabled`). + * has no control plane (`OS_CLOUD_URL=off`, `none`, `local`, `disabled`, + * or an explicitly empty or blank configured value). */ controlPlaneUrl(): string; } + +/** + * Service-registry slot for {@link ICloudBindingService}: the one spelling of + * the name. `@objectstack/cloud-connection` registers the binding under it and + * a reader (`@objectstack/service-push`'s relay sender) asks for it, so the + * registrar and the reader cannot drift apart. + */ +export const CLOUD_BINDING_SERVICE = 'cloud-binding';