cta: narrow down docker API #209
Conversation
| docker: Dockerode | ||
| }) => { | ||
| const handler = tryHandler({ log }, async (req, res) => { | ||
| const { pathname: path } = url.parse(req.url || '') |
There was a problem hiding this comment.
Need to take more careful look, but I think that since this api is know behaving very similar to a regular rest-api, it might worth using express/fastify or other package to reduce the noise and high-level implementations of routing.
| const handler = tryHandler({ log }, async (req, res) => { | ||
| const { pathname: path } = url.parse(req.url || '') | ||
|
|
||
| if (req.method === 'GET' && path === '/healthz') { |
There was a problem hiding this comment.
Not specifically for this PR, but teed to add support for CORS, so these apis can be fetched also from client.
@eliyabar currently add it locally for testing
There was a problem hiding this comment.
CORS can work without the cookie and only by using Authorization header, so no need for access-control-allow-credentials. (Apparently WS doesn't respect CORS anyhow)
| if (!await dockerFilter.inspectContainer(containerId)) { | ||
| throw new NotFoundError() | ||
| } | ||
| const { obj: { tty: ttyQueryParam }, search } = parseQueryParams(req.url ?? '', { tty: true }) |
There was a problem hiding this comment.
It might worth splitting it to two ops in the future:
One for creating the exec/logs handler (return handle url), the other for reading in websocket based on the handle.
The handle can up-for-grabs and available in a limited time window. (can be saved even in CTA memory, or localfs)
It can be a bit safer as POST requests can work with authorization header and CORS.
| @@ -70,6 +72,10 @@ export const addComposeTunnelAgentService = ( | |||
| machineStatusCommand?: MachineStatusCommand | |||
| envMetadata: EnvMetadata | |||
| composeModelPath: string | |||
| composeProject: string | |||
| profileThumbprint?: string | |||
| privateMode: boolean | |||
There was a problem hiding this comment.
The privateMode can be derived from the defaultAccess paramter (or the opposite), it's a label with product semantics, while defaultAccess is a CTA env var, but they are basically the same. (The label is currently only read by the Docker extension AFAIK)
There was a problem hiding this comment.
can the privateMode be read from the /env-metadata endpoint instead of the label?
royra
force-pushed
the
cta-narrow-docker-api
branch
from
68b5d29 to
d58bd24
Compare
replace generic docker API proxy with specific endpoints
royra
force-pushed
the
cta-narrow-docker-api
branch
from
d58bd24 to
e7a2fb1
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| app.get('/compose-model', async ({ log }, res) => { | ||
| log.debug('compose-model handler') | ||
| void res | ||
| .header('Content-Type', 'application/x-yaml') | ||
| .send(await fs.promises.readFile(composeModelPath, { encoding: 'utf-8' })) | ||
| }) |
Check failure
Code scanning / CodeQL
Missing rate limiting
replace generic docker API proxy with specific endpoints