Conversation
| chartsPath?: string, | ||
| ): Promise<any | null> { | ||
| const uuid = uuidv4() | ||
| const helmChartsDir = `/tmp/otomi/charts/${catalogName}/${branch}/chart/${uuid}` |
There was a problem hiding this comment.
I would merge getBYOWorkloadCatalogChart and fetchCatalogChart into a single function. getBYOWorkloadCatalogChart only constructs the helmChartsDir path using a UUID and passes it to fetchCatalogChart, which is not used anywhere else.
There was a problem hiding this comment.
Indeed, the original functions only have it this way so that the v1 endpoint still works. For this new endpoint the functions can be merged.
| $ref: '#/components/schemas/AplCatalogChartResponse' | ||
| '400': | ||
| $ref: '#/components/responses/BadRequest' | ||
| content: |
There was a problem hiding this comment.
Both ref and inline content for this error response seem problematic, we should consider returning only the ref.
'400':
$ref: '#/components/responses/BadRequest'
| content: | ||
| application/json: | ||
| schema: | ||
| $ref: '#/components/schemas/OpenApiValidationError' |
There was a problem hiding this comment.
This endpoint might return a 200 OK with a null body instead of a 404, we should consider adding a 404 error response.
| return true | ||
| } | ||
|
|
||
| export async function sparseCheckoutChart( |
There was a problem hiding this comment.
It seems this function is unused.
|
|
||
| const chartDir = checkoutResult.checkoutPath | ||
|
|
||
| // RBAC check if needed |
There was a problem hiding this comment.
We should consider removing this RBAC check:
- We use catalog-level RBAC rather than chart-level RBAC,
- This check will almost certainly fail for single-chart checkout since
rbac.yamlwon't be fetched with sparse checkout, causing bothresolvedHelmChartsDirandfallbackDirto always return{}.
| export async function fetchWorkloadCatalogChart( | ||
| url: string, | ||
| helmChartsDir: string, | ||
| branch: string = 'main', |
There was a problem hiding this comment.
Since the 3rd param has a default value of main, it should be moved to the last position of the required params, otherwise the 4th param becomes required and callers can't skip the 3rd param to use its default.
console: linode/apl-console#728