refactor: migrate /apply calls to REST endpoints for component and workload creation#253
refactor: migrate /apply calls to REST endpoints for component and workload creation#253binoyPeries wants to merge 1 commit intoopenchoreo:mainfrom
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@plugins/scaffolder-backend-module-openchoreo/src/actions/component.ts`:
- Line 448: The use of a blanket type assertion "body: workloadBody as any"
masks a schema mismatch between the OpenAPI createWorkload contract and the
WorkloadBody interface; fix by making the contract and runtime shape consistent:
either update the OpenAPI schema for createWorkload to explicitly declare the
containers (required) and endpoints (optional) properties to match the
WorkloadBody interface, or validate workloadBody at runtime before sending
(e.g., check that the variable workloadBody contains the required containers
array and optional endpoints) and only then pass it as body without as any;
locate references to workloadBody, the createWorkload call, and the WorkloadBody
interface to implement the schema change or add validation.
🧹 Nitpick comments (5)
plugins/scaffolder-backend-module-openchoreo/src/actions/component.ts (2)
370-378: Consider including error response body in failure messages.The error message only includes status code and status text, but the API likely returns useful error details in the response body. This could make debugging failures more difficult.
Suggested improvement
if (createError || !createResponse.ok) { + const errorBody = createError ? JSON.stringify(createError) : 'No error details'; throw new Error( - `Failed to create component: ${createResponse.status} ${createResponse.statusText}`, + `Failed to create component: ${createResponse.status} ${createResponse.statusText}. Details: ${errorBody}`, ); }
452-457: Workload creation errors don't fail the action but log errors.The workload creation failure is logged but doesn't throw, allowing the scaffolder action to complete successfully even when the workload setup fails. This is intentional (component was created), but the error message could be more actionable.
The current message mentions "manual configuration" but doesn't specify how. Consider adding a link or specific instructions.
plugins/scaffolder-backend-module-openchoreo/src/actions/componentResourceBuilder.test.ts (1)
228-255: Consider adding test coverage for workload edge cases.The workload tests cover basic scenarios but could benefit from additional cases:
- Workload without image (should have empty containers)
- Workload with envVars and fileMounts
- Workload with endpoints map (not just legacy port)
Suggested additional tests
it('should build workload body with empty containers when no image provided', () => { const result = buildWorkloadResource({ endpoints: { api: { type: 'HTTP', port: 3000 } }, }); expect(result.containers).toEqual({}); expect(result.endpoints).toEqual({ api: { type: 'HTTP', port: 3000 } }); }); it('should add env vars and file mounts to container', () => { const result = buildWorkloadResource({ containerImage: 'nginx:latest', envVars: [{ key: 'NODE_ENV', value: 'production' }], fileMounts: [{ key: 'config', mountPath: '/etc/config', value: 'data' }], }); expect(result.containers.main.env).toEqual([{ key: 'NODE_ENV', value: 'production' }]); expect(result.containers.main.files).toEqual([{ key: 'config', mountPath: '/etc/config', value: 'data' }]); }); it('should prefer endpoints map over legacy port', () => { const result = buildWorkloadResource({ containerImage: 'nginx:latest', port: 8080, endpoints: { api: { type: 'REST', port: 3000 } }, }); expect(result.endpoints).toEqual({ api: { type: 'REST', port: 3000 } }); });plugins/scaffolder-backend-module-openchoreo/src/actions/componentResourceBuilder.ts (2)
91-91: Type assertionas anyon systemParameters.The
systemParameters: { repository } as anycast bypasses type checking. This is likely due to thesecretReffield not being in the official OpenAPI schema (as mentioned in component.ts line 543-544).Consider documenting why this assertion is needed, or update the OpenAPI schema to include
secretRefif it's a supported feature.
144-147: WorkloadBody interface could be derived from OpenAPI schema.Similar to
CreateComponentRequest, theWorkloadBodyinterface could potentially be derived from the OpenAPI-generated types to ensure consistency. However, since the workload endpoint acceptsadditionalProperties: true, a local interface may be acceptable.
| componentName: ctx.input.componentName, | ||
| }, | ||
| }, | ||
| body: workloadBody as any, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, check the OpenAPI spec for workload-related schemas
rg -n -A 10 'createWorkload|WorkloadRequest|workloads:' packages/openchoreo-client-node/openapi/openchoreo-api.yamlRepository: openchoreo/backstage-plugins
Length of output: 531
🏁 Script executed:
#!/bin/bash
# Get context around line 448 in component.ts
cat -n plugins/scaffolder-backend-module-openchoreo/src/actions/component.ts | sed -n '430,465p'Repository: openchoreo/backstage-plugins
Length of output: 1618
🏁 Script executed:
#!/bin/bash
# Search for WorkloadBody interface definition
rg -n 'interface WorkloadBody|type WorkloadBody' plugins/scaffolder-backend-module-openchoreo/Repository: openchoreo/backstage-plugins
Length of output: 192
🏁 Script executed:
#!/bin/bash
# Get the WorkloadBody interface definition
cat -n plugins/scaffolder-backend-module-openchoreo/src/actions/componentResourceBuilder.ts | sed -n '144,180p'Repository: openchoreo/backstage-plugins
Length of output: 1550
🏁 Script executed:
#!/bin/bash
# Get the request body schema from OpenAPI spec for createWorkload
rg -n -A 30 'operationId: createWorkload' packages/openchoreo-client-node/openapi/openchoreo-api.yaml | grep -A 25 'requestBody'Repository: openchoreo/backstage-plugins
Length of output: 460
Type assertion as any indicates schema mismatch that warrants review.
The body: workloadBody as any cast is necessary because the OpenAPI schema for createWorkload (defined as a generic object with additionalProperties: true) does not align with the specific WorkloadBody interface structure (containers property required, endpoints optional). The permissive OpenAPI schema forces the cast to bypass type checking, which can mask type errors if either the interface or API contract changes.
Consider either:
- Updating the OpenAPI schema to define explicit
containersandendpointsproperties matching theWorkloadBodyinterface, or - Adding a runtime validation that the constructed
workloadBodymatches the expected shape before casting.
🤖 Prompt for AI Agents
In `@plugins/scaffolder-backend-module-openchoreo/src/actions/component.ts` at
line 448, The use of a blanket type assertion "body: workloadBody as any" masks
a schema mismatch between the OpenAPI createWorkload contract and the
WorkloadBody interface; fix by making the contract and runtime shape consistent:
either update the OpenAPI schema for createWorkload to explicitly declare the
containers (required) and endpoints (optional) properties to match the
WorkloadBody interface, or validate workloadBody at runtime before sending
(e.g., check that the variable workloadBody contains the required containers
array and optional endpoints) and only then pass it as body without as any;
locate references to workloadBody, the createWorkload call, and the WorkloadBody
interface to implement the schema change or add validation.
binoyPeries
force-pushed
the
role-ui-revamp
branch
from
2e4a054 to
7c7ce0a
Compare
…rkload creation Signed-off-by: binoyPeries <binoyperies98@gmail.com>
binoyPeries
force-pushed
the
role-ui-revamp
branch
from
7c7ce0a to
a428e24
Compare
1 participant
Purpose
Replace the generic POST /apply (K8s-style resource apply) calls in the openchoreo:component:create scaffolder action with the dedicated REST endpoints: createComponent and createWorkload.
Goals
Approach
User stories
Release note
Documentation
Training
Certification
Marketing
Automation tests
Security checks
Samples
Related PRs
Migrations (if applicable)
Test environment
Learning
Summary by CodeRabbit
New Features
Refactor