ISV template design #1028
ISV template design #1028
Conversation
… type definitions, and implementation plan for a new centralized configuration system
ziyang-lin-404
force-pushed
the
tmp/isv-design
branch
from
231073d to
ef92cf3
Compare
docs/ISV_TEMPLATE_SYSTEM_DESIGN.md
Outdated
| interface SingleMutation { | ||
| id: string; | ||
| label: string; | ||
| action: string; // Built-in action name |
There was a problem hiding this comment.
Using string for action names bypasses TypeScript's type checking. A typo like 'createAcount' won't be caught until runtime. Consider using a const enum or union type: type ActionName = 'createAccount' | 'createBucket' | ...
| { | ||
| id: 'veeamFolder', | ||
| label: 'Prepare Veeam repository: {{name}}', | ||
| action: 'putObject', | ||
| variables: (form, prev, item) => ({ | ||
| Bucket: item.name, | ||
| Key: `${VEEAM_XML_PREFIX}/`, | ||
| Body: '', | ||
| }), | ||
| }, | ||
| { | ||
| id: 'veeamSystem', | ||
| label: 'Enforce Veeam repository: {{name}}', | ||
| action: 'putObject', | ||
| variables: (form, prev, item) => ({ | ||
| Bucket: item.name, | ||
| Key: `${VEEAM_XML_PREFIX}/system.xml`, | ||
| Body: SYSTEM_XML_CONTENT, | ||
| ContentType: 'text/xml', | ||
| }), | ||
| }, | ||
| { | ||
| id: 'veeamCapacity', | ||
| label: 'Set repository capacity: {{name}}', | ||
| action: 'putObject', | ||
| variables: (form, prev, item) => ({ | ||
| Bucket: item.name, | ||
| Key: `${VEEAM_XML_PREFIX}/capacity.xml`, | ||
| Body: `<capacity>${item.capacityBytes}</capacity>`, | ||
| ContentType: 'text/xml', | ||
| }), | ||
| when: (form, item) => item.capacity > 0, | ||
| }, | ||
| ], |
There was a problem hiding this comment.
mixing low-level S3 operations with high-level business concepts. This exposes low-level S3 API details (putObject, Key, ContentType) in the template. Template authors shouldn't need to know Veeam's internal folder structure. Maybe consider a composite action that encapsulates these operations.
There was a problem hiding this comment.
These are not “implementation details” but “business requirements.”
Veeam requires system.xml to be in a specific path—that’s not our implementation choice, it’s Veeam’s specification. Hiding it inside a composite action only shifts the complexity from A to B; it doesn’t eliminate it.
There was a problem hiding this comment.
You're right that the Veeam XML structure is a business requirement, not an implementation detail we're hiding. The putObject calls directly reflect Veeam's specification.
My concern was primarily about reuse if multiple Veeam templates (VBR, VBO) share this logic. However, the Composable Blocks pattern (Section 11) already addresses this — Veeam-specific bucketSteps can be extracted as a reusable block rather than a composite action.
The current design also provides better retry granularity at each step, which is a valid trade-off.
docs/ISV_TEMPLATE_SYSTEM_DESIGN.md
Outdated
| | Field | Description | Source | | ||
| |-------|-------------|--------| | ||
| | `_sosApiAvailable` | Whether SOS API is available | `useCheckSOSAPIStatus` | | ||
| | `_existingAccountArn` | Selected existing account's role ARN | Account selection | | ||
| | `_existingAccountId` | Selected existing account's ID | Account selection | | ||
| | `_iamUserType` | 'create' or 'existing' | IAM user selection | | ||
|
|
||
| These are prefixed with `_` to indicate they are system-injected, not user input. |
There was a problem hiding this comment.
These _-prefixed fields are "magic" runtime injections invisible in the template definition. This creates hidden dependencies. Consider:
- Documenting all injected fields in the template type
- Or making injection explicit via a context parameter separate from form
| label: string; // Supports template syntax: {{name}}, {{capacity}} | ||
| action: string; | ||
| variables: LoopVariablesConfig; | ||
| when?: (form: FormData, item: any) => boolean; |
There was a problem hiding this comment.
Inconsistent when signatures: SingleMutation.when receives (form), but LoopStep.when receives (form, item). This asymmetry could confuse developers. Consider a unified signature or clearly distinct naming (when vs itemWhen).
There was a problem hiding this comment.
The signatures differ because the contexts differ - just like Array.reduce(acc, item) vs Array.map(item), and TypeScript will error if you misuse them.
No description provided.