New Endpoint: /upload or /publish extension
We currently have the Add cli command, would be beneficial to expose this via API. This would prevent the need to exec into the pods running in the cluster to add a new extension.
As a part of our internal project we need to implement new /add and /remove endpoints. Please, see POC below (you can copy the yaml to editor.swagger.io). The idea is to repeat CLI interface for add and remove commands with one exception: the user can provide "extension" field (to upload extensions) or "extensionUrl" (to add extension using URL). Please, check and comment, any suggestions would be really useful. Thanks.
openapi: 3.0.3
info:
title: Coder Marketplace New APIs
description: |-
This is a proposal for new "Add extension" and "Remove extension" API endpoints
version: 0.0.1
servers:
- url: https://example.com/api
paths:
/add:
post:
tags:
- extensions
summary: Add extension(s)
description: Add new extension(s) to the marketplace, extension or extension-url parameter should be provided
operationId: addExtension
requestBody:
content:
multipart/form-data:
schema:
type: object
properties:
extensions-dir:
type: string
artifactory:
type: string
repo:
type: string
extension:
type: string
format: binary
extensionUrl:
type: string
required: true
responses:
'201':
description: Extension added
'400':
description: Unable to add extension
content:
application/json:
schema:
$ref: '#/components/schemas/AddApiResponse'
/remove/{extensionId}:
delete:
tags:
- extensions
summary: Remove extension
description: Remove an extension, one version or all versions, if "all" keyword provided
operationId: removeExtension
parameters:
- name: extensionId
in: path
description: Extension ID or "all" keyword
required: true
schema:
type: string
responses:
'400':
description: Unable to remove extension
content:
application/json:
schema:
$ref: '#/components/schemas/RemoveApiResponse'
components:
schemas:
RemoveApiResponse:
type: object
properties:
message:
type: string
example: Unable to remove extension
AddApiResponse:
type: object
properties:
message:
type: string
example: Unable to add extension
This looks really good! Some thoughts:
- Can we omit
extensions-dir,artifactory, andrepofor/add? The marketplace would already know all these values. If we do need them, then I imagine the remove endpoint also needs them. - I think I would expect the
/removeendpoint to takeallas a query parameter to mimic how the cli does it, so something like/remove/{id}?all. - While the
/add,/removeendpoints do make sense and I see how they mirror the cli, I think it would be more REST-like to use/extensionsand/extensions/{id}. - Authentication is something we will have to figure out if this gets added. Maybe it can even just be basic http auth. Or, we could make the new endpoints opt-in and users have to secure the endpoints themselves.
Thank you for your response.
- You are right. Just confirmed we can re-use them.
- Yes, I have mentioned "all" in the description and going to support it.
- Yes, agree with that. I will use POST
/extensionsand DELETE /extensions{extensionId} or DELETE /extensions{extensionId@all} (@ because is used for versions now: https://github.com/coder/code-marketplace/pull/24) as you suggested. - Can we do authentication as a next step or do you think it should be the part of the same PR?
Yes, I have mentioned "all" in the description and going to support it. /extensions{extensionId@all}
Ohhhh I see, yes this makes sense, I misread it as /extensions/all. I like that, maybe we should do that in the cli as well, right now the cli expects remove ext --all rather than remove ext@all, but we can think about changing the cli later.
Can we do authentication as a next step or do you think it should be the part of the same PR?
It does not need to be part of the same PR, but I would say at the very least we disable these endpoints by default, just to make sure we do not accidentally release with the endpoints exposed.
Ohhhh I see, yes this makes sense, I misread it as
/extensions/all. I like that, maybe we should do that in the cli as well, right now the cli expectsremove ext --allrather thanremove ext@all, but we can think about changing the cli later.
Agreed, let's think about cli changes later.
Can we do authentication as a next step or do you think it should be the part of the same PR?
It does not need to be part of the same PR, but I would say at the very least we disable these endpoints by default, just to make sure we do not accidentally release with the endpoints exposed.
Noted, I will disable the endpoints by default. I think I will add some environment variables support: ENABLE_PUBLISHING_ENDPOINTS, MAX_UPLOAD_FILE_SIZE, MAX_FILES and UPLOADS_FOLDER.
I think I will add some environment variables support: ENABLE_PUBLISHING_ENDPOINTS, MAX_UPLOAD_FILE_SIZE, MAX_FILES and UPLOADS_FOLDER.
Works for me! Should we prefix these with MARKETPLACE or CODE_MARKETPLACE (a bit long but being unique seems like a good idea)?
I think I will add some environment variables support: ENABLE_PUBLISHING_ENDPOINTS, MAX_UPLOAD_FILE_SIZE, MAX_FILES and UPLOADS_FOLDER.
Works for me! Should we prefix these with
MARKETPLACEorCODER_MARKETPLACE(a bit long but being unique seems like a good idea)?
Sure, I will add the MARKETPLACE prefix.
Quick update on this one. Functionally, everything is done and UT added but the changes are still inside our organisation. We are going to setup the process to contribute the changes back, using our organisation's account. It will take some time (2-3 weeks, I hope not more). In the mean time, I'm starting to work on protecting these endpoints by authentication.