Deploy infisical instance as a service #3141
Conversation
WalkthroughThe changes introduce a dedicated instances feature spanning both backend and frontend components. On the backend, new dependencies are added for AWS SDK CloudFormation and AWS CDK, enabling interaction with AWS resources. Type declarations are updated to extend Fastify services with dedicated instance functionality, and additional Knex table definitions and migrations create and modify a Suggested reviewers
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 ESLint
backend/src/@types/knex.d.tsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "@typescript-eslint/eslint-plugin". (The package "@typescript-eslint/eslint-plugin" was not found when loaded as a Node module from the directory "/backend".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "@typescript-eslint/eslint-plugin" was referenced from the config file in "backend/.eslintrc.js". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. backend/src/db/migrations/20240321000000_dedicated_instances.tsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "@typescript-eslint/eslint-plugin". (The package "@typescript-eslint/eslint-plugin" was not found when loaded as a Node module from the directory "/backend".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "@typescript-eslint/eslint-plugin" was referenced from the config file in "backend/.eslintrc.js". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. backend/src/db/schemas/dedicated-instances.tsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "@typescript-eslint/eslint-plugin". (The package "@typescript-eslint/eslint-plugin" was not found when loaded as a Node module from the directory "/backend".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "@typescript-eslint/eslint-plugin" was referenced from the config file in "backend/.eslintrc.js". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team.
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
maidul98
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (10)
backend/src/db/migrations/20240321000000_dedicated_instances.ts (1)
13-13: Consider adding documentation for enum values.The status enum includes several states (RUNNING, UPGRADING, PROVISIONING, FAILED), but there's no inline documentation explaining when each state is used and what transitions are valid.
Adding a comment describing each status would make future maintenance easier:
- t.enum("status", ["RUNNING", "UPGRADING", "PROVISIONING", "FAILED"]).notNullable(); + // Status values: + // RUNNING: Instance is operational + // UPGRADING: Instance is being upgraded to a new version + // PROVISIONING: Instance is being initially provisioned + // FAILED: Instance provisioning/upgrading failed, see error field + t.enum("status", ["RUNNING", "UPGRADING", "PROVISIONING", "FAILED"]).notNullable();backend/src/ee/services/dedicated-instance/dedicated-instance-dal.ts (1)
13-22: Consider adding pagination support for listing instances.The
findInstancesByOrgIdmethod currently returns all instances for an organization without pagination. As the number of instances grows, this could lead to performance issues.Consider adding pagination parameters:
- const findInstancesByOrgId = async (orgId: string, tx?: Knex) => { + const findInstancesByOrgId = async (orgId: string, tx?: Knex, limit?: number, offset?: number) => { try { - const instances = await (tx || db.replicaNode())(TableName.DedicatedInstances) - .where({ orgId }) - .select("*"); + let query = (tx || db.replicaNode())(TableName.DedicatedInstances) + .where({ orgId }) + .select("*"); + + if (limit !== undefined) { + query = query.limit(limit); + } + if (offset !== undefined) { + query = query.offset(offset); + } + + const instances = await query; return instances; } catch (error) { throw new DatabaseError({ error, name: "Find instances by org ID" }); } };frontend/src/pages/organization/DedicatedInstancesPage/DedicatedInstancesPage.tsx (2)
79-83: Remove or integrateINSTANCE_SIZES.Currently,
INSTANCE_SIZESis not being referenced in the UI. Either remove it if redundant or use it in the cluster size selection to maintain consistent definitions across the code.-const INSTANCE_SIZES = [ - { value: "small", label: "Small (2 vCPU, 8GB RAM)" }, - { value: "medium", label: "Medium (4 vCPU, 16GB RAM)" }, - { value: "large", label: "Large (8 vCPU, 32GB RAM)" } -];
564-573: Ensure consistent cluster size definitions.There's a mismatch between the
INSTANCE_SIZESarray and the options in the<SelectItem>for cluster size. Align them to avoid confusion about resource specs or remove the unused definitions.backend/src/ee/services/permission/org-permission.ts (2)
15-16: Combine or clarify "Edit" vs. "Update" actions.
Both "Edit" and "Update" are present in theOrgPermissionActionsenum, and they may serve overlapping purposes. Consider using just one action if the semantics are identical, or documenting the difference if they serve distinct use cases.
54-55: Check naming consistency.
Other subjects (e.g.,secret-scanning,incident-contact) use dash-case. For consistency, consider renamingdedicatedInstanceto something likededicated-instance.backend/src/ee/services/dedicated-instance/dedicated-instance-service.ts (4)
278-289: Confirm strict security group rules.
Currently, the code only opens the necessary ports for RDS (5432) and Redis (6379) from ECS tasks. Confirm if you need further inbound/outbound restrictions or a unique approach for public vs. private subnets to avoid unintended exposure.
334-336: Dry run feature is useful—consider additional logging.
You are logging the stack template, but you could also log other computed details (subnet IDs, security groups) for easier auditing in a dry run.
416-434: Outputs are saved only once stack reaches CREATE_COMPLETE.
Make sure partial or in-progress stacks do not require storing partial outputs. If needed, you can periodically retrieve partial outputs to update the instance record.
457-460: Silent catch for stack status errors.
You log an error but still return the instance details. If the stack status is crucial, consider returning a partial error or using structured error data to help the caller diagnose stack issues.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
backend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (29)
backend/package.json(2 hunks)backend/src/@types/fastify.d.ts(2 hunks)backend/src/@types/knex.d.ts(1 hunks)backend/src/db/migrations/20240321000000_dedicated_instances.ts(1 hunks)backend/src/db/migrations/20240324000000_add_failed_status_to_dedicated_instances.ts(1 hunks)backend/src/db/schemas/dedicated-instances.ts(1 hunks)backend/src/db/schemas/models.ts(2 hunks)backend/src/ee/migrations/dedicated-instance.ts(1 hunks)backend/src/ee/routes/v1/dedicated-instance-router.ts(1 hunks)backend/src/ee/routes/v1/index.ts(2 hunks)backend/src/ee/services/dedicated-instance/dedicated-instance-dal.ts(1 hunks)backend/src/ee/services/dedicated-instance/dedicated-instance-service.ts(1 hunks)backend/src/ee/services/permission/org-permission.ts(6 hunks)backend/src/server/routes/index.ts(3 hunks)frontend/src/layouts/OrganizationLayout/OrganizationLayout.tsx(1 hunks)frontend/src/layouts/OrganizationLayout/components/MinimizedOrgSidebar/MinimizedOrgSidebar.tsx(2 hunks)frontend/src/pages/organization/$organizationId/dedicated-instances/$instanceId.tsx(1 hunks)frontend/src/pages/organization/$organizationId/dedicated-instances/index.tsx(1 hunks)frontend/src/pages/organization/DedicatedInstancesPage/DedicatedInstanceDetailsPage.tsx(1 hunks)frontend/src/pages/organization/DedicatedInstancesPage/DedicatedInstanceDetailsPage/route.tsx(1 hunks)frontend/src/pages/organization/DedicatedInstancesPage/DedicatedInstancesPage.tsx(1 hunks)frontend/src/pages/organization/DedicatedInstancesPage/index.ts(1 hunks)frontend/src/pages/organization/DedicatedInstancesPage/route.tsx(1 hunks)frontend/src/routeTree.gen.ts(19 hunks)frontend/src/routes.ts(1 hunks)frontend/src/routes/index.tsx(1 hunks)frontend/src/routes/organization.$organizationId.dedicated-instances.$instanceId.tsx(1 hunks)frontend/src/routes/organization.$organizationId.dedicated-instances.tsx(1 hunks)frontend/src/routes/organization/dedicated-instances.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- backend/src/ee/migrations/dedicated-instance.ts
- frontend/src/pages/organization/DedicatedInstancesPage/index.ts
- backend/src/db/schemas/dedicated-instances.ts
🧰 Additional context used
🪛 GitHub Actions: Check Frontend Type and Lint check
frontend/src/pages/organization/DedicatedInstancesPage/DedicatedInstanceDetailsPage.tsx
[error] 5-5: 'FormControl' is declared but its value is never read.
🪛 GitHub Actions: Check Backend PR types and lint
backend/src/ee/routes/v1/dedicated-instance-router.ts
[error] 96-96: Argument of type '{ orgId: string; instanceName: string; subdomain: string; region: string; publiclyAccessible: boolean; provider: "aws"; dryRun: false; }' is not assignable to parameter of type 'CreateInstanceParams'.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Check API Changes
🔇 Additional comments (45)
frontend/src/pages/organization/$organizationId/dedicated-instances/$instanceId.tsx (1)
1-6: Route implementation looks correctThe route is properly set up with authentication and organization context, and correctly associates the DedicatedInstanceDetailsPage component. This follows the application's file-based routing pattern using TanStack Router.
Note that there appear to be multiple route definitions for dedicated instances in the codebase (as seen in the AI summary). Ensure this is intentional and part of your routing architecture.
frontend/src/routes/organization.$organizationId.dedicated-instances.tsx (1)
1-6: Route implementation for dedicated instances list view looks goodThe route correctly maps to the DedicatedInstancesPage component and follows the application's routing patterns. This new route enables navigation to the dedicated instances list for a specific organization.
frontend/src/layouts/OrganizationLayout/OrganizationLayout.tsx (1)
77-83: UI navigation item properly implementedThe navigation link for "Dedicated Instances" has been correctly added to the Organization Control menu group. The implementation follows the same pattern as other menu items with proper icon and active state handling.
frontend/src/pages/organization/DedicatedInstancesPage/DedicatedInstanceDetailsPage/route.tsx (1)
1-6: Route implementation is correct but appears duplicativeThe route correctly maps to the DedicatedInstanceDetailsPage component, but appears to duplicate functionality with the route in
frontend/src/pages/organization/$organizationId/dedicated-instances/$instanceId.tsx. While this may be intentional due to your routing architecture, consider whether maintaining two routes to the same component is necessary, as it could lead to confusion during maintenance.Is this duplicate route intentional as part of your routing architecture? If so, ensure there's documentation explaining the purpose of having multiple route definitions for the same component.
frontend/src/pages/organization/DedicatedInstancesPage/route.tsx (1)
1-9: LGTM: Route configuration follows project standards.The route implementation correctly uses the TanStack Router's
createFileRoutefunction and properly associates it with theDedicatedInstancesPagecomponent. The route path follows the established authentication and organization layout structure used throughout the application.frontend/src/routes.ts (1)
19-22: Well-structured route integration.The dedicated instances routes are properly integrated within the existing organization routes structure. The implementation follows the project's pattern of having an index route and specific detail routes with dynamic parameters. Routes are correctly organized in the navigation hierarchy.
frontend/src/pages/organization/$organizationId/dedicated-instances/index.tsx (1)
1-6: LGTM: Clean implementation of organization-specific route.The route implementation correctly follows the application's routing patterns. The import from
@app/pagesrather than a relative path maintains consistency with project's module aliasing practices.backend/package.json (2)
120-120:❓ Verification inconclusive
Verify CloudFormation SDK version compatibility.
The CloudFormation client will be used for AWS infrastructure management related to dedicated instances. Ensure this version is compatible with your AWS environment.
🏁 Script executed:
#!/bin/bash # Check for any known compatibility issues with the CloudFormation SDK version # Check for documented issues or deprecated features curl -s https://github.com/aws/aws-sdk-js-v3/tree/main/clients/client-cloudformation | grep -i "version\|deprecat\|breaking" # Check if there are newer versions available npm view @aws-sdk/client-cloudformation versions --json | jq 'keys[-5:]'Length of output: 82121
Action Required: Manually Verify CloudFormation SDK Compatibility
Although our automated check for the "@aws-sdk/client-cloudformation" version (^3.750.0) did not return any explicit deprecated features or breaking changes, the output was inconclusive due to the HTML response received from the GitHub page. Please ensure that this version is fully compatible with your AWS environment—especially regarding dedicated instance management—by conducting a manual review of the SDK’s documentation and testing against your AWS setup.
165-165:✅ Verification successful
Verify AWS CDK library version compatibility.
The AWS CDK is being added to support infrastructure as code for dedicated instances. This is a significant architectural choice that will affect how AWS resources are provisioned.
🏁 Script executed:
#!/bin/bash # Check for version compatibility with the AWS CDK library # Check if AWS CDK version is compatible with Node.js version in project node -e "console.log('Node.js version:', process.version)" npm view aws-cdk-lib engines --json # Check if there are newer stable versions available npm view aws-cdk-lib versions --json | jq 'keys[-5:]'Length of output: 250
AWS CDK Library Version Compatibility Verified
- The project's Node.js version (v22.9.0) satisfies the AWS CDK library's engine requirement (
>= 14.15.0).- The dependency
"aws-cdk-lib": "^2.180.0"is compatible. Note that the latest version indices (keys [291–295]) suggest newer releases are available; if you plan an upgrade, please review their release notes to check for any breaking changes or beneficial improvements.No further action is required regarding compatibility at this time.
frontend/src/routes/organization.$organizationId.dedicated-instances.$instanceId.tsx (1)
1-6: Route implementation looks good!Clean implementation of a dynamic route for dedicated instance details. The component assignment and route pattern are well-structured.
backend/src/@types/knex.d.ts (1)
933-937: Table definition properly integratedThe
DedicatedInstancestable definition follows the established pattern used throughout this file, properly extending the Knex types with the necessary composite table type.backend/src/ee/routes/v1/index.ts (2)
7-7: Import added correctlyThe import for the dedicated instance router is added in the appropriate alphabetical order with other router imports.
42-42: Router registration implemented properlyThe dedicated instance router is registered with the "/organizations" prefix, which is consistent with the organization-scoped nature of this feature.
frontend/src/routes/organization/dedicated-instances.tsx (2)
1-7: List route implementation looks goodThe list route for dedicated instances is well-structured, using the correct path pattern and component assignment.
9-12: Details route implementation looks goodThe details route properly extends the list route pattern with the instanceId parameter, and correctly assigns the details component.
frontend/src/layouts/OrganizationLayout/components/MinimizedOrgSidebar/MinimizedOrgSidebar.tsx (2)
16-17: Correctly imported FontAwesome icon for dedicated instances.Adding the
faServericon from FontAwesome is appropriate for the new dedicated instances feature.
336-340: Well-structured navigation link added for dedicated instances.The new navigation link follows the established pattern used throughout the sidebar component and is placed logically within the organization controls section.
backend/src/@types/fastify.d.ts (2)
22-22: Appropriate import for dedicated instance service type.The import is correctly placed with other similar service imports.
232-232: Correctly extended Fastify services interface.The dedicated instance service is properly added to the FastifyInstance services object, following the established pattern for service registration.
backend/src/db/migrations/20240324000000_add_failed_status_to_dedicated_instances.ts (1)
1-16: Well-structured migration for adding FAILED status.The migration follows best practices:
- Uses descriptive naming with date prefix
- Properly implements both up and down methods
- Safely drops existing constraints with IF EXISTS
- Uses table name constants instead of hardcoded strings
- Correctly handles constraint recreation
Adding a FAILED status is a logical enhancement for dedicated instances, ensuring the system can properly track instances that fail during provisioning.
backend/src/db/schemas/models.ts (1)
139-140: Correctly added DedicatedInstances to TableName enum.The DedicatedInstances entry is properly added to the TableName enum, following the naming convention of using snake_case for database table names.
backend/src/db/migrations/20240321000000_dedicated_instances.ts (2)
5-51: Well-designed table schema for dedicated instances.The migration creates a comprehensive
DedicatedInstancestable with appropriate fields for tracking AWS resources, configuration parameters, and instance status. Good practices include:
- UUID for primary key and proper foreign key relationship
- Meaningful default values where appropriate
- Unique constraint on orgId and instanceName
- Proper error field for tracking failure details
53-56: Clean down migration correctly handles dropping tables and triggers.The down migration properly removes both the table and its associated update trigger in the correct order.
frontend/src/routes/index.tsx (2)
1-27: Routes are correctly defined and integrated into the routing system.The dedicated instance routes are properly set up with the correct paths and components. The module augmentation for TypeScript is correctly implemented.
17-18:❓ Verification inconclusive
Verify that routeTree is properly defined before use.
The code adds children to a
routeTreevariable that isn't defined in the visible portion of code. Ensure it's correctly defined and initialized before this point.
🏁 Script executed:
#!/bin/bash # Check if routeTree is properly defined before use rg -A 3 -B 10 "const\s+routeTree|let\s+routeTree|var\s+routeTree" frontend/src/routes/Length of output: 88
Action Required: Confirm
routeTreeInitializationThe code calls
routeTree.addChildren(...)on a variable that isn’t visibly defined in the file. Since the initial search in thefrontend/src/routes/folder didn’t return any definitions, please manually verify thatrouteTreeis either imported from another module or defined in a parent scope before this point. Ensure that its initialization is correct and that the variable is available at runtime.
- Double-check if there’s an import statement or global definition for
routeTreeelsewhere in the repository.- If
routeTreeshould be defined locally in this file, add the proper declaration and initialization.backend/src/ee/services/dedicated-instance/dedicated-instance-dal.ts (1)
1-28: Well-structured DAL following project patterns.The dedicated instance DAL is implemented using the factory pattern consistent with other DALs in the project. It properly handles error cases and includes transaction support.
backend/src/server/routes/index.ts (3)
235-236: Correctly imports dedicated instance modules.The import statements for the dedicated instance DAL and service factories are properly placed with other similar imports.
1462-1466: Dedicated instance services are properly initialized.The DAL and service are initialized correctly, with the service depending on the DAL and the permission service.
1568-1569: Service is correctly added to the server's service registry.The dedicated instance service is properly added to the services object, making it available throughout the application.
frontend/src/pages/organization/DedicatedInstancesPage/DedicatedInstanceDetailsPage.tsx (1)
241-259: Confirm distinct private/public endpoints.Both the "Private" and "Public" sections display the same endpoint URL (
https://{instance.subdomain}.infisical.com). If the intention is to differentiate endpoints, ensure correct values are populated or remove one if not applicable.backend/src/ee/routes/v1/dedicated-instance-router.ts (1)
7-28: Validate date fields against JSON input.
z.date()can fail if the incoming JSON is in a string format not automatically parsed by Fastify. Convert or parse fields likecreatedAtandupdatedAtfrom strings to Dates, or consider usingz.string().datetime()with a transform to keep them in sync.frontend/src/routeTree.gen.ts (9)
62-64: Routes imported for new dedicated instances feature.These imports bring in the routes for the dedicated instances feature, including both the main page and the details page.
214-217: New route creation for dedicated instances.This block creates a new file route for the dedicated instances feature, properly structured within the organization layout.
463-471: Route configuration for dedicated instances.The route is properly configured with ID, path, and parent route relationships, following the application's routing pattern by making it a child of the organization route.
636-644: Details page route definition.This correctly defines the route for viewing individual dedicated instance details, using the
$instanceIdparameter for dynamic routing.
654-660: Main page route definition.This sets up the main landing page route for the dedicated instances feature, properly connecting it to its parent route.
2958-2974: Interface and implementation for dedicated instances routes.This establishes the type definitions and relationships between the routes, correctly implementing the parent-child relationships needed for the router.
3010-3011: Inclusion in organization route children.The dedicated instances route is properly included in the organization route children, ensuring it's accessible within the organization navigation structure.
4943-4943: Registration in route manifest.The routes are properly registered in the route manifest section, which is crucial for the TanStack Router to correctly generate the routing logic.
Also applies to: 5015-5022
1-9: Note about auto-generated file.This is an automatically generated file by TanStack Router. The changes introduced for dedicated instances follow the established patterns of the codebase, maintaining consistency with existing features.
backend/src/ee/services/permission/org-permission.ts (4)
86-87: No immediate concerns—good addition to the union type.
Adding[OrgPermissionActions, OrgPermissionSubjects.DedicatedInstance]cleanly extends the permission set to include this new subject.
181-190: Great schema extension.
The dedicated instance schema definition follows the existing style, ensuring consistent validations inOrgPermissionSchema.
281-284: Expanded admin permissions look thorough.
Allowing Create/Read/Update/Delete for "DedicatedInstance" aligns with admin-level privileges. Verify that no further restriction is required for your use case.
316-316: Member read permission is consistent.
GrantingReadaccess to members for a dedicated instance subject seems appropriate, ensuring standard visibility without full privileges.backend/src/ee/services/dedicated-instance/dedicated-instance-service.ts (1)
240-255: Replace hard-coded container specs with cluster config.
In the Fargate task definition, container CPU/memory are directly taken fromclusterConfig, but the method of referencing them is correct. Verify that all references match your final resource requirements and consider error handling if the user provides an unsupported cluster size.
| import { | ||
| Button, | ||
| Card, | ||
| FormControl, |
There was a problem hiding this comment.
Remove unused import triggering lint error.
FormControl is imported but never used. This is causing the lint error in the pipeline. Please remove it if it's not needed.
-import {
- Button,
- Card,
- FormControl,
- Input,
- ...
+import {
+ Button,
+ Card,
+ Input,
+ ...📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| FormControl, | |
| import { | |
| Button, | |
| Card, | |
| Input, | |
| ... |
🧰 Tools
🪛 GitHub Actions: Check Frontend Type and Lint check
[error] 5-5: 'FormControl' is declared but its value is never read.
| {PROVIDERS.map(({ value, label, image, description }) => ( | ||
| <button | ||
| key={value} | ||
| type="button" | ||
| onClick={() => { | ||
| if (value === "aws") { | ||
| setNewInstance({ ...newInstance, provider: value, region: "" }); | ||
| } | ||
| }} | ||
| className={twMerge( | ||
| "group relative flex h-24 cursor-pointer flex-col items-center justify-center rounded-md border border-mineshaft-600 bg-mineshaft-700 p-4 duration-200 hover:bg-mineshaft-600", | ||
| newInstance.provider === value && "border-primary/50 bg-primary/10" | ||
| )} | ||
| > | ||
| <div className="flex h-10 w-10 items-center justify-center rounded-lg bg-mineshaft-800"> | ||
| <img | ||
| src={image} | ||
| alt={`${label} logo`} | ||
| className="h-7 w-7 object-contain" | ||
| /> | ||
| </div> | ||
| <div className="mt-2 max-w-xs text-center text-sm font-medium text-gray-300 duration-200 group-hover:text-gray-200"> | ||
| {label} | ||
| </div> | ||
| </button> | ||
| ))} | ||
| </div> |
There was a problem hiding this comment.
Guard against unsupported providers in the UI.
The UI lists aws, gcp, and azure while the backend schema restricts provider to 'aws' only. Selecting gcp or azure will fail at backend validation. Consider removing them from the UI or adding backend support.
...
{
- value: "gcp",
- label: "Google Cloud Platform",
- ...
- },
- {
- value: "azure",
- label: "Microsoft Azure",
- ...
- }
+ // For now, remove other providers until backend supports them
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {PROVIDERS.map(({ value, label, image, description }) => ( | |
| <button | |
| key={value} | |
| type="button" | |
| onClick={() => { | |
| if (value === "aws") { | |
| setNewInstance({ ...newInstance, provider: value, region: "" }); | |
| } | |
| }} | |
| className={twMerge( | |
| "group relative flex h-24 cursor-pointer flex-col items-center justify-center rounded-md border border-mineshaft-600 bg-mineshaft-700 p-4 duration-200 hover:bg-mineshaft-600", | |
| newInstance.provider === value && "border-primary/50 bg-primary/10" | |
| )} | |
| > | |
| <div className="flex h-10 w-10 items-center justify-center rounded-lg bg-mineshaft-800"> | |
| <img | |
| src={image} | |
| alt={`${label} logo`} | |
| className="h-7 w-7 object-contain" | |
| /> | |
| </div> | |
| <div className="mt-2 max-w-xs text-center text-sm font-medium text-gray-300 duration-200 group-hover:text-gray-200"> | |
| {label} | |
| </div> | |
| </button> | |
| ))} | |
| </div> | |
| // Somewhere at the top of the file, update the provider definitions: | |
| const PROVIDERS = [ | |
| { | |
| value: "aws", | |
| label: "Amazon Web Services", | |
| image: "/images/aws-logo.png", | |
| description: "AWS description" | |
| }, | |
| // For now, remove other providers until backend supports them | |
| ]; | |
| // ... later in the component's render/return code ... | |
| <div> | |
| {PROVIDERS.map(({ value, label, image, description }) => ( | |
| <button | |
| key={value} | |
| type="button" | |
| onClick={() => { | |
| if (value === "aws") { | |
| setNewInstance({ ...newInstance, provider: value, region: "" }); | |
| } | |
| }} | |
| className={twMerge( | |
| "group relative flex h-24 cursor-pointer flex-col items-center justify-center rounded-md border border-mineshaft-600 bg-mineshaft-700 p-4 duration-200 hover:bg-mineshaft-600", | |
| newInstance.provider === value && "border-primary/50 bg-primary/10" | |
| )} | |
| > | |
| <div className="flex h-10 w-10 items-center justify-center rounded-lg bg-mineshaft-800"> | |
| <img | |
| src={image} | |
| alt={`${label} logo`} | |
| className="h-7 w-7 object-contain" | |
| /> | |
| </div> | |
| <div className="mt-2 max-w-xs text-center text-sm font-medium text-gray-300 duration-200 group-hover:text-gray-200"> | |
| {label} | |
| </div> | |
| </button> | |
| ))} | |
| </div> |
| provider: z.literal('aws'), // Only allow 'aws' as provider | ||
| region: z.string(), | ||
| publiclyAccessible: z.boolean().default(false) | ||
| }); |
There was a problem hiding this comment.
Include clusterSize in schema to match frontend payload.
The frontend sends a clusterSize field, but the current schema doesn't accept it. Consider adding it for consistency if the backend is meant to handle it.
const CreateDedicatedInstanceSchema = z.object({
instanceName: z.string().min(1),
subdomain: z.string().min(1),
provider: z.literal('aws'), // Only allow 'aws' as provider
region: z.string(),
publiclyAccessible: z.boolean().default(false),
+ clusterSize: z.enum(["small", "medium", "large"]).default("small")
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| provider: z.literal('aws'), // Only allow 'aws' as provider | |
| region: z.string(), | |
| publiclyAccessible: z.boolean().default(false) | |
| }); | |
| const CreateDedicatedInstanceSchema = z.object({ | |
| instanceName: z.string().min(1), | |
| subdomain: z.string().min(1), | |
| provider: z.literal('aws'), // Only allow 'aws' as provider | |
| region: z.string(), | |
| publiclyAccessible: z.boolean().default(false), | |
| clusterSize: z.enum(["small", "medium", "large"]).default("small") | |
| }); |
| const instance = await server.services.dedicatedInstance.createInstance({ | ||
| orgId: organizationId, | ||
| instanceName, | ||
| subdomain, | ||
| region, | ||
| publiclyAccessible, | ||
| provider: provider, | ||
| dryRun: false, | ||
| }); | ||
|
|
There was a problem hiding this comment.
Fix type mismatch in createInstance call.
The pipeline error indicates that the argument passed to createInstance doesn't match the service's expected CreateInstanceParams. Ensure all required fields (e.g., clusterSize) are included or remove unrecognized fields (e.g., dryRun) based on the service interface.
const instance = await server.services.dedicatedInstance.createInstance({
orgId: organizationId,
instanceName,
subdomain,
region,
publiclyAccessible,
provider: provider,
- dryRun: false,
+ // If 'dryRun' is part of the service definition, add it to the interface. Otherwise remove it.
+ // clusterSize, if needed, must be passed here to match the updated schema:
+ // clusterSize,
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const instance = await server.services.dedicatedInstance.createInstance({ | |
| orgId: organizationId, | |
| instanceName, | |
| subdomain, | |
| region, | |
| publiclyAccessible, | |
| provider: provider, | |
| dryRun: false, | |
| }); | |
| const instance = await server.services.dedicatedInstance.createInstance({ | |
| orgId: organizationId, | |
| instanceName, | |
| subdomain, | |
| region, | |
| publiclyAccessible, | |
| provider: provider, | |
| // If 'dryRun' is part of the service definition, add it to the interface. Otherwise remove it. | |
| // clusterSize, if needed, must be passed here to match the updated schema: | |
| // clusterSize, | |
| }); |
🧰 Tools
🪛 GitHub Actions: Check Backend PR types and lint
[error] 96-96: Argument of type '{ orgId: string; instanceName: string; subdomain: string; region: string; publiclyAccessible: boolean; provider: "aws"; dryRun: false; }' is not assignable to parameter of type 'CreateInstanceParams'.
| const CLUSTER_SIZES = { | ||
| small: { | ||
| containerCpu: 1024, // 1 vCPU | ||
| containerMemory: 2048, // 2GB | ||
| rdsInstanceType: 'db.t3.small', | ||
| elasticCacheType: 'cache.t3.micro', | ||
| desiredContainerCount: 1, | ||
| displayName: '1 vCPU, 2GB RAM' | ||
| }, | ||
| medium: { | ||
| containerCpu: 2048, // 2 vCPU | ||
| containerMemory: 4096, // 4GB | ||
| rdsInstanceType: 'db.t3.medium', | ||
| elasticCacheType: 'cache.t3.small', | ||
| desiredContainerCount: 2, | ||
| displayName: '2 vCPU, 4GB RAM' | ||
| }, | ||
| large: { | ||
| containerCpu: 4096, // 4 vCPU | ||
| containerMemory: 8192, // 8GB | ||
| rdsInstanceType: 'db.t3.large', | ||
| elasticCacheType: 'cache.t3.medium', | ||
| desiredContainerCount: 4, | ||
| displayName: '4 vCPU, 8GB RAM' | ||
| } | ||
| }; |
There was a problem hiding this comment.
Cluster size config is unused when creating resources.
Although CLUSTER_SIZES defines different CPU/memory specs, lines 191-206 and 219-225 hardcode the instance type and Redis cache node type as T3.medium and T3.micro, respectively. Reuse clusterConfig.rdsInstanceType and clusterConfig.elasticCacheType here.
Example fix snippet:
- instanceType: ec2.InstanceType.of(ec2.InstanceClass.T3, ec2.InstanceSize.MEDIUM),
+ instanceType: ec2.InstanceType.of(
+ ec2.InstanceClass.T3,
+ ec2.InstanceSize[clusterConfig.rdsInstanceType.replace('db.t3.', '').toUpperCase()]
+ ),Committable suggestion skipped: line range outside the PR's diff.
Description 📣
Service to deploy a dedicated instance of Infisical
Type ✨
Tests 🛠️
# Here's some code block to paste some code snippets