feat(aurora-portal): security group rule create #601
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughReplaces a non-index security-group route with a trailing-slash index variant and implements a full security-group rule creation flow (AddRuleModal, form sections, presets, constants, validation), client create mutation and hook updates, UI/table adjustments, server schema simplification, and many tests. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant UI as "AddRuleModal (UI)"
participant Form as "TanStack Form"
participant Validation as "Zod Schema"
participant Client as "TRPC Client"
participant Server as "Backend"
User->>UI: open modal
UI->>Form: init DEFAULT_VALUES
User->>UI: fill form fields
UI->>Form: update fields
User->>UI: submit
UI->>Validation: validate (createRuleFormSchema)
alt validation fails
Validation-->>UI: errors
UI-->>User: show errors
else validation passes
UI->>Client: network.securityGroupRule.create(payload)
Client->>Server: send request
Server->>Server: (server-side processing)
Server-->>Client: response
Client->>Client: invalidate queries
alt success
Client-->>UI: success
UI->>UI: reset & close
else error
Client-->>UI: error
UI-->>User: show error
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
KirylSAP
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: 2
🧹 Nitpick comments (11)
apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/-components/SecurityGroups/-components/SecurityGroupTableRow.tsx (1)
44-49: Use accessible semantics for row navigation.The whole row is now clickable, but this change only adds a mouse handler. If
DataGridRowis not already keyboard-interactive, keyboard users will miss the new “open details” affordance. Prefer a real link/button for navigation, or add focus plus Enter/Space handling explicitly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/aurora-portal/src/client/routes/_auth/accounts/`$accountId/projects/$projectId/network/-components/SecurityGroups/-components/SecurityGroupTableRow.tsx around lines 44 - 49, The row is only clickable via mouse; make it keyboard-accessible by ensuring DataGridRow (or its wrapper) is focusable and activates on Enter/Space: add tabIndex={0}, role="button" (or use an actual <button>/<a> instead) and implement an onKeyDown handler that calls the existing handleShowDetails when key is "Enter" or " " (Space); also include an accessible name/aria-label if needed so screen reader users know it opens details. Target the DataGridRow usage and the handleShowDetails handler when making these changes.apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/-components/SecurityGroups/-components/SecurityGroupListContainer.tsx (1)
123-124: Please verify the empty-state create path.This branch no longer uses
permissions.canCreateoronCreateClick. If the page does not surface another create action when the list is empty, users land on a dead-end state. If creation intentionally moved elsewhere, please also drop the now-dead prop fromSecurityGroupListContainerProps.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/aurora-portal/src/client/routes/_auth/accounts/`$accountId/projects/$projectId/network/-components/SecurityGroups/-components/SecurityGroupListContainer.tsx around lines 123 - 124, The empty-state early return in SecurityGroupListContainer (when securityGroups.length === 0) removed usage of permissions.canCreate and onCreateClick — verify whether creation was intentionally moved; if not, restore a visible create action in this branch by rendering the create UI (checking permissions.canCreate and invoking onCreateClick) so users aren’t stuck, otherwise remove the now-unused onCreateClick and permissions.canCreate props from SecurityGroupListContainerProps and update all callers to stop passing them.apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-modals/AddRuleModal/AddRuleModal.test.tsx (1)
177-188: Test provides limited verification due to mocking.The comment notes the prop is "verified by mock" but the mock doesn't capture or assert on props. This test only confirms the section renders, not that
availableSecurityGroupsis correctly passed. Consider either removing this test or enhancing the mock to capture props for assertion.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/aurora-portal/src/client/routes/_auth/accounts/`$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-modals/AddRuleModal/AddRuleModal.test.tsx around lines 177 - 188, The test "passes availableSecurityGroups to RemoteSourceSection" currently only checks that RemoteSourceSection renders; update the test to actually verify availableSecurityGroups is passed by either enhancing the RemoteSourceSection mock to capture and expose its props (e.g. mockImplementation that renders a data-testid containing JSON or count of props) or by asserting rendered output that depends on availableSecurityGroups; locate the test and the helper renderModal and change the mock for RemoteSourceSection (or its module) so you can assert on the passed availableSecurityGroups prop instead of only checking getByTestId("remote-source-section").apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-modals/AddRuleModal/rulePresets.ts (1)
62-68: Consider adding a UDP variant for DNS.DNS commonly uses both TCP (for zone transfers and large responses) and UDP (for standard queries). The current preset only covers TCP on port 53. If users frequently need UDP-based DNS rules, consider adding a separate "DNS (UDP)" preset.
💡 Optional: Add DNS UDP preset
{ value: "dns", - label: "DNS", + label: "DNS (TCP)", protocol: "tcp", portRangeMin: 53, portRangeMax: 53, }, + { + value: "dns-udp", + label: "DNS (UDP)", + protocol: "udp", + portRangeMin: 53, + portRangeMax: 53, + },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/aurora-portal/src/client/routes/_auth/accounts/`$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-modals/AddRuleModal/rulePresets.ts around lines 62 - 68, The DNS preset currently only covers TCP (the object with value "dns" in rulePresets.ts); add a separate DNS UDP preset object (e.g., value "dns-udp", label "DNS (UDP)") with protocol set to "udp" and portRangeMin/portRangeMax set to 53 so users can quickly select a UDP-based DNS rule alongside the existing TCP entry.apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-modals/AddRuleModal/sections/PortRangeSection.tsx (1)
18-19: Consider defensive handling for undefined fieldMeta.While the optional chaining handles missing errors,
fieldMeta.portFromorfieldMeta.portTocould be undefined if the field hasn't been touched yet. This currently works but consider adding explicit nullish checks for robustness.💡 More explicit null handling
- const portFromError = useStore(form.store, (state) => state.fieldMeta.portFrom?.errors[0]?.message) - const portToError = useStore(form.store, (state) => state.fieldMeta.portTo?.errors[0]?.message) + const portFromError = useStore(form.store, (state) => state.fieldMeta.portFrom?.errors?.[0]?.message) + const portToError = useStore(form.store, (state) => state.fieldMeta.portTo?.errors?.[0]?.message)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/aurora-portal/src/client/routes/_auth/accounts/`$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-modals/AddRuleModal/sections/PortRangeSection.tsx around lines 18 - 19, The current selectors assume state.fieldMeta exists; update the two useStore selectors to defensively guard fieldMeta itself so they don't throw when fieldMeta is undefined—replace state.fieldMeta.portFrom?.errors[0]?.message and state.fieldMeta.portTo?.errors[0]?.message with state.fieldMeta?.portFrom?.errors?.[0]?.message and state.fieldMeta?.portTo?.errors?.[0]?.message (or equivalent nullish checks) in the useStore calls to compute portFromError and portToError.apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-modals/AddRuleModal/sections/RemoteSourceSection.tsx (2)
45-71: Deep nesting of form fields impacts readability.The three-level nesting of
form.Fieldcomponents (remoteSourceType → remoteCidr → ethertype) makes the code harder to follow. Consider usingform.Subscribeto watch multiple field values at once, which can flatten the structure.♻️ Alternative approach using form.Subscribe
{/* Remote CIDR (conditional) */} <form.Subscribe selector={(state) => ({ remoteSourceType: state.values.remoteSourceType, ethertype: state.values.ethertype })}> {({ remoteSourceType, ethertype }) => remoteSourceType === "cidr" ? ( <form.Field name="remoteCidr"> {(remoteCidrField) => ( <FormRow className="mb-6"> <TextInput id="remoteCidr" name="remoteCidr" label={t`Remote IP Prefix`} value={remoteCidrField.state.value} onChange={(e) => remoteCidrField.handleChange(e.target.value)} errortext={remoteCidrField.state.meta.errors[0]?.message} placeholder={ethertype === "IPv4" ? DEFAULT_IPV4_CIDR : DEFAULT_IPV6_CIDR} disabled={disabled} /> </FormRow> )} </form.Field> ) : null } </form.Subscribe>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/aurora-portal/src/client/routes/_auth/accounts/`$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-modals/AddRuleModal/sections/RemoteSourceSection.tsx around lines 45 - 71, Deeply nested form.Field calls (remoteSourceType → remoteCidr → ethertype) reduce readability; replace them by using form.Subscribe with a selector that returns { remoteSourceType: state.values.remoteSourceType, ethertype: state.values.ethertype } and conditionally render a single form.Field for "remoteCidr" when remoteSourceType === "cidr", then pass remoteCidrField.state.value, remoteCidrField.handleChange, remoteCidrField.state.meta.errors[0]?.message and use ethertype to choose the placeholder (DEFAULT_IPV4_CIDR / DEFAULT_IPV6_CIDR) and keep disabled/label/id/name as before on the TextInput to flatten the structure.
86-90: Consider adding error display for security group selection.The
Selectcomponent for remote security group doesn't display validation errors like the CIDR input does (line 60). If validation requires a security group to be selected, users won't see the error message.♻️ Add error text to Select
<Select id="remoteSecurityGroupId" label={t`Remote Security Group`} value={remoteSecurityGroupIdField.state.value} onChange={(value) => remoteSecurityGroupIdField.handleChange(String(value))} disabled={disabled} + errortext={remoteSecurityGroupIdField.state.meta.errors[0]?.message} >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/aurora-portal/src/client/routes/_auth/accounts/`$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-modals/AddRuleModal/sections/RemoteSourceSection.tsx around lines 86 - 90, The Select for remote security group in RemoteSourceSection should show validation errors like the CIDR input does; update the Select (the one rendering SelectOption and mapping availableSecurityGroups) to accept and render the form validation state and message (e.g., pass error={Boolean(errors.remoteSecurityGroupId)} and helperText or errorText={errors.remoteSecurityGroupId?.message} or the equivalent props your Select supports), sourcing the message from the same form state/validator used for CIDR, so a missing/invalid security group selection displays the error below the Select.apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-components/-details/SecurityGroupRulesTable.test.tsx (2)
499-527: Test coverage reduced for delete functionality.The tests for delete functionality now only verify that rows render, but don't actually test the popup menu interaction or that clicking "Delete" opens the dialog. Consider adding a test that opens the popup menu and clicks the delete option to ensure the full flow works.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/aurora-portal/src/client/routes/_auth/accounts/`$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-components/-details/SecurityGroupRulesTable.test.tsx around lines 499 - 527, Add a test that exercises the full delete flow by rendering SecurityGroupRulesTable with mockRules and a spy on onDeleteRule, then open the PopupMenu for a specific rule (e.g., find the row and its menu button), click the "Delete" menu item, and assert that the DeleteRuleDialog appears (or that the onDeleteRule callback is invoked after confirming deletion). Use the existing components (SecurityGroupRulesTable, PopupMenu, DeleteRuleDialog) and testing helpers (screen, userEvent or fireEvent, findByText/findByRole) to interact with the menu button, select the "Delete" option, and verify the expected UI/dialog or callback behavior to restore coverage for the delete interaction.
270-274: Consider using fake timers for more reliable debounce testing.Using
setTimeoutwith a hardcoded delay (600ms) to wait for debounce is fragile and can cause flaky tests. Consider using Vitest's fake timers for more deterministic testing.♻️ Suggested improvement using fake timers
it("calls onSearchChange when search input changes", async () => { + vi.useFakeTimers() const onSearchChange = vi.fn() render( <SecurityGroupRulesTable rules={mockRules} onDeleteRule={vi.fn()} isDeletingRule={false} deleteError={null} onSearchChange={onSearchChange} />, { wrapper: createWrapper() } ) const searchInput = screen.getByPlaceholderText("Search...") const user = userEvent.setup() await user.type(searchInput, "HTTP") - // Wait for debounce (500ms) - await new Promise((resolve) => setTimeout(resolve, 600)) + // Advance timers past debounce threshold + await vi.advanceTimersByTimeAsync(500) // After debounce, onSearchChange should be called with the final value expect(onSearchChange).toHaveBeenCalledWith("HTTP") + vi.useRealTimers() })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/aurora-portal/src/client/routes/_auth/accounts/`$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-components/-details/SecurityGroupRulesTable.test.tsx around lines 270 - 274, The test currently waits for debounce with a real timeout then asserts onSearchChange, which is flaky; update the test in SecurityGroupRulesTable.test.tsx to use Vitest fake timers: call vi.useFakeTimers() before triggering the input change, advance timers by the debounce duration (e.g., vi.advanceTimersByTime(500)) instead of awaiting setTimeout, then assert onSearchChange was called with "HTTP" and finally call vi.useRealTimers(); ensure you reference the existing mocked callback onSearchChange and the debounce duration used by the component when advancing timers.apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-components/-details/SecurityGroupRulesTable.tsx (1)
84-89: useEffect may trigger unnecessarily on initial render.The effect runs when
isDeletingRuleisfalseanddeleteErrorisnull, which is also true on initial mount. WhilesetRuleToDelete(null)whenruleToDeleteis alreadynullis a no-op, addingruleToDeleteto the condition or deps would make the intent clearer.♻️ More explicit condition
// Close dialog after successful deletion useEffect(() => { - if (!isDeletingRule && !deleteError) { + if (ruleToDelete && !isDeletingRule && !deleteError) { setRuleToDelete(null) } -}, [isDeletingRule, deleteError]) +}, [isDeletingRule, deleteError, ruleToDelete])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/aurora-portal/src/client/routes/_auth/accounts/`$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-components/-details/SecurityGroupRulesTable.tsx around lines 84 - 89, The effect currently clears the dialog on mount because isDeletingRule is false and deleteError is null; update the useEffect to include ruleToDelete in the dependency array and add ruleToDelete !== null to the condition so setRuleToDelete(null) only runs when there is actually a rule to clear (i.e., when ruleToDelete is non-null, isDeletingRule is false, and deleteError is falsy). This change involves the existing useEffect and the symbols isDeletingRule, deleteError, ruleToDelete, and setRuleToDelete.apps/aurora-portal/src/server/Network/types/securityGroup.ts (1)
8-91: Extract shared validation utilities to reduce duplication between client and server.The functions
detectCIDRFamily,validatePortRange,validateIcmpTypeCode, andisValidCIDRare duplicated identically in both the client-sidevalidationHelpers.tsand server-sidesecurityGroup.ts. This duplication creates maintenance overhead—any future fixes or enhancements require updates in two locations, increasing the risk of validation logic diverging between client and server.Consider extracting these utilities into a shared package or module that both client and server can import from, ensuring consistent validation behavior across the application.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/aurora-portal/src/server/Network/types/securityGroup.ts` around lines 8 - 91, The same CIDR/port/ICMP validators (detectCIDRFamily, validatePortRange, validateIcmpTypeCode, and isValidCIDR) are duplicated in client validationHelpers.ts and server securityGroup.ts; extract them into one shared module (e.g., a new utilities package or shared lib) and import from it in both places. Create a shared file that exports detectCIDRFamily, validatePortRange, validateIcmpTypeCode, isValidCIDR with the same signatures and types, replace the local copies in validationHelpers.ts and securityGroup.ts with imports from the shared module, remove the duplicate implementations, and run/typecheck/tests to ensure no typing or runtime regressions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/aurora-portal/src/client/routes/_auth/accounts/`$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-modals/AddRuleModal/AddRuleModal.test.tsx:
- Around line 160-174: The test AddRuleModal.test.tsx currently named "resets
form when modal is closed" only asserts onClose was called; either rename the
test to reflect behavior (e.g., "calls onClose when Cancel is clicked after form
interaction") or add assertions to verify the form actually resets: after using
renderModal and interacting with ruleTypeSelect (selecting "custom-tcp"), click
cancel via cancelButton, assert onClose called and then re-render/open the modal
or inspect the form elements (ruleTypeSelect value and any other fields
manipulated) to confirm they returned to their default values; update the test
name or assertions accordingly and keep references to renderModal,
ruleTypeSelect, cancelButton, and onClose.
In
`@apps/aurora-portal/src/client/routes/_auth/accounts/`$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-modals/AddRuleModal/validation/formSchema.ts:
- Around line 152-179: The ICMP range check block incorrectly runs when
icmpType/icmpCode are NaN because the condition uses !== null; update the guard
before calling validateIcmpTypeCode to ensure values are valid numbers (e.g.,
typeof icmpType === "number" && !Number.isNaN(icmpType) or same for icmpCode) so
validateIcmpTypeCode isn't called with NaN. Locate the block that references
icmpType, icmpCode and validateIcmpTypeCode and change the if condition to only
run when at least one of the fields is a real number (not NaN) so the earlier
parsing errors remain the authoritative validation results.
---
Nitpick comments:
In
`@apps/aurora-portal/src/client/routes/_auth/accounts/`$accountId/projects/$projectId/network/-components/SecurityGroups/-components/SecurityGroupListContainer.tsx:
- Around line 123-124: The empty-state early return in
SecurityGroupListContainer (when securityGroups.length === 0) removed usage of
permissions.canCreate and onCreateClick — verify whether creation was
intentionally moved; if not, restore a visible create action in this branch by
rendering the create UI (checking permissions.canCreate and invoking
onCreateClick) so users aren’t stuck, otherwise remove the now-unused
onCreateClick and permissions.canCreate props from
SecurityGroupListContainerProps and update all callers to stop passing them.
In
`@apps/aurora-portal/src/client/routes/_auth/accounts/`$accountId/projects/$projectId/network/-components/SecurityGroups/-components/SecurityGroupTableRow.tsx:
- Around line 44-49: The row is only clickable via mouse; make it
keyboard-accessible by ensuring DataGridRow (or its wrapper) is focusable and
activates on Enter/Space: add tabIndex={0}, role="button" (or use an actual
<button>/<a> instead) and implement an onKeyDown handler that calls the existing
handleShowDetails when key is "Enter" or " " (Space); also include an accessible
name/aria-label if needed so screen reader users know it opens details. Target
the DataGridRow usage and the handleShowDetails handler when making these
changes.
In
`@apps/aurora-portal/src/client/routes/_auth/accounts/`$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-components/-details/SecurityGroupRulesTable.test.tsx:
- Around line 499-527: Add a test that exercises the full delete flow by
rendering SecurityGroupRulesTable with mockRules and a spy on onDeleteRule, then
open the PopupMenu for a specific rule (e.g., find the row and its menu button),
click the "Delete" menu item, and assert that the DeleteRuleDialog appears (or
that the onDeleteRule callback is invoked after confirming deletion). Use the
existing components (SecurityGroupRulesTable, PopupMenu, DeleteRuleDialog) and
testing helpers (screen, userEvent or fireEvent, findByText/findByRole) to
interact with the menu button, select the "Delete" option, and verify the
expected UI/dialog or callback behavior to restore coverage for the delete
interaction.
- Around line 270-274: The test currently waits for debounce with a real timeout
then asserts onSearchChange, which is flaky; update the test in
SecurityGroupRulesTable.test.tsx to use Vitest fake timers: call
vi.useFakeTimers() before triggering the input change, advance timers by the
debounce duration (e.g., vi.advanceTimersByTime(500)) instead of awaiting
setTimeout, then assert onSearchChange was called with "HTTP" and finally call
vi.useRealTimers(); ensure you reference the existing mocked callback
onSearchChange and the debounce duration used by the component when advancing
timers.
In
`@apps/aurora-portal/src/client/routes/_auth/accounts/`$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-components/-details/SecurityGroupRulesTable.tsx:
- Around line 84-89: The effect currently clears the dialog on mount because
isDeletingRule is false and deleteError is null; update the useEffect to include
ruleToDelete in the dependency array and add ruleToDelete !== null to the
condition so setRuleToDelete(null) only runs when there is actually a rule to
clear (i.e., when ruleToDelete is non-null, isDeletingRule is false, and
deleteError is falsy). This change involves the existing useEffect and the
symbols isDeletingRule, deleteError, ruleToDelete, and setRuleToDelete.
In
`@apps/aurora-portal/src/client/routes/_auth/accounts/`$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-modals/AddRuleModal/AddRuleModal.test.tsx:
- Around line 177-188: The test "passes availableSecurityGroups to
RemoteSourceSection" currently only checks that RemoteSourceSection renders;
update the test to actually verify availableSecurityGroups is passed by either
enhancing the RemoteSourceSection mock to capture and expose its props (e.g.
mockImplementation that renders a data-testid containing JSON or count of props)
or by asserting rendered output that depends on availableSecurityGroups; locate
the test and the helper renderModal and change the mock for RemoteSourceSection
(or its module) so you can assert on the passed availableSecurityGroups prop
instead of only checking getByTestId("remote-source-section").
In
`@apps/aurora-portal/src/client/routes/_auth/accounts/`$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-modals/AddRuleModal/rulePresets.ts:
- Around line 62-68: The DNS preset currently only covers TCP (the object with
value "dns" in rulePresets.ts); add a separate DNS UDP preset object (e.g.,
value "dns-udp", label "DNS (UDP)") with protocol set to "udp" and
portRangeMin/portRangeMax set to 53 so users can quickly select a UDP-based DNS
rule alongside the existing TCP entry.
In
`@apps/aurora-portal/src/client/routes/_auth/accounts/`$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-modals/AddRuleModal/sections/PortRangeSection.tsx:
- Around line 18-19: The current selectors assume state.fieldMeta exists; update
the two useStore selectors to defensively guard fieldMeta itself so they don't
throw when fieldMeta is undefined—replace
state.fieldMeta.portFrom?.errors[0]?.message and
state.fieldMeta.portTo?.errors[0]?.message with
state.fieldMeta?.portFrom?.errors?.[0]?.message and
state.fieldMeta?.portTo?.errors?.[0]?.message (or equivalent nullish checks) in
the useStore calls to compute portFromError and portToError.
In
`@apps/aurora-portal/src/client/routes/_auth/accounts/`$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-modals/AddRuleModal/sections/RemoteSourceSection.tsx:
- Around line 45-71: Deeply nested form.Field calls (remoteSourceType →
remoteCidr → ethertype) reduce readability; replace them by using form.Subscribe
with a selector that returns { remoteSourceType: state.values.remoteSourceType,
ethertype: state.values.ethertype } and conditionally render a single form.Field
for "remoteCidr" when remoteSourceType === "cidr", then pass
remoteCidrField.state.value, remoteCidrField.handleChange,
remoteCidrField.state.meta.errors[0]?.message and use ethertype to choose the
placeholder (DEFAULT_IPV4_CIDR / DEFAULT_IPV6_CIDR) and keep
disabled/label/id/name as before on the TextInput to flatten the structure.
- Around line 86-90: The Select for remote security group in RemoteSourceSection
should show validation errors like the CIDR input does; update the Select (the
one rendering SelectOption and mapping availableSecurityGroups) to accept and
render the form validation state and message (e.g., pass
error={Boolean(errors.remoteSecurityGroupId)} and helperText or
errorText={errors.remoteSecurityGroupId?.message} or the equivalent props your
Select supports), sourcing the message from the same form state/validator used
for CIDR, so a missing/invalid security group selection displays the error below
the Select.
In `@apps/aurora-portal/src/server/Network/types/securityGroup.ts`:
- Around line 8-91: The same CIDR/port/ICMP validators (detectCIDRFamily,
validatePortRange, validateIcmpTypeCode, and isValidCIDR) are duplicated in
client validationHelpers.ts and server securityGroup.ts; extract them into one
shared module (e.g., a new utilities package or shared lib) and import from it
in both places. Create a shared file that exports detectCIDRFamily,
validatePortRange, validateIcmpTypeCode, isValidCIDR with the same signatures
and types, replace the local copies in validationHelpers.ts and securityGroup.ts
with imports from the shared module, remove the duplicate implementations, and
run/typecheck/tests to ensure no typing or runtime regressions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 12dc2e0f-89a3-4bc2-95a2-dcef3379b9e2
📒 Files selected for processing (43)
apps/aurora-portal/src/client/routeTree.gen.tsapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/-components/SecurityGroups/-components/-details/SecurityGroupRulesTable.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/-components/SecurityGroups/-components/SecurityGroupListContainer.test.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/-components/SecurityGroups/-components/SecurityGroupListContainer.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/-components/SecurityGroups/-components/SecurityGroupTableRow.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/-components/SecurityGroups/List.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/-components/SecurityGroups/constants.tsapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-components/-details/SecurityGroupBasicInfo.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-components/-details/SecurityGroupHeader.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-components/-details/SecurityGroupRulesTable.test.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-components/-details/SecurityGroupRulesTable.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-components/-details/SecurityGroupTabs.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-components/-details/index.tsapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-components/SecurityGroupDetailsView.test.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-components/SecurityGroupDetailsView.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-hooks/useSecurityGroupDetails.tsapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-modals/AddRuleModal/AddRuleModal.test.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-modals/AddRuleModal/AddRuleModal.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-modals/AddRuleModal/constants.tsapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-modals/AddRuleModal/rulePresets.tsapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-modals/AddRuleModal/sections/DescriptionSection.test.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-modals/AddRuleModal/sections/DescriptionSection.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-modals/AddRuleModal/sections/DirectionEthertypeSection.test.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-modals/AddRuleModal/sections/DirectionEthertypeSection.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-modals/AddRuleModal/sections/IcmpSection.test.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-modals/AddRuleModal/sections/IcmpSection.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-modals/AddRuleModal/sections/PortRangeSection.test.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-modals/AddRuleModal/sections/PortRangeSection.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-modals/AddRuleModal/sections/ProtocolSection.test.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-modals/AddRuleModal/sections/ProtocolSection.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-modals/AddRuleModal/sections/RemoteSourceSection.test.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-modals/AddRuleModal/sections/RemoteSourceSection.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-modals/AddRuleModal/sections/RuleTypeSection.test.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-modals/AddRuleModal/sections/RuleTypeSection.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-modals/AddRuleModal/types.tsapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-modals/AddRuleModal/validation/formSchema.tsapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-modals/AddRuleModal/validation/index.tsapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-modals/AddRuleModal/validation/validationHelpers.test.tsapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-modals/AddRuleModal/validation/validationHelpers.tsapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-modals/DeleteRuleDialog.tsxapps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/index.tsxapps/aurora-portal/src/server/Network/types/securityGroup.test.tsapps/aurora-portal/src/server/Network/types/securityGroup.ts
💤 Files with no reviewable changes (2)
- apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/-components/SecurityGroups/constants.ts
- apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/-components/SecurityGroups/-components/-details/SecurityGroupRulesTable.tsx
...projectId/network/securitygroups/$securityGroupId/-modals/AddRuleModal/AddRuleModal.test.tsx
Outdated
Show resolved
Hide resolved
...jectId/network/securitygroups/$securityGroupId/-modals/AddRuleModal/validation/formSchema.ts
Show resolved
Hide resolved
KirylSAP
commented
Mar 25, 2026
...cts/$projectId/network/securitygroups/$securityGroupId/-modals/AddRuleModal/AddRuleModal.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/aurora-portal/src/locales/de/messages.ts`:
- Line 3: Replace the remaining English strings in the German messages bundle by
providing German translations for the specific message keys used by the new Add
Rule flow: update b2BLBa ("Add Security Group Rule"), bmQLn5 ("Add Rule"),
YuGQWb ("Rule Type"), MRB7nI ("Direction"), 6+OdGi ("Protocol"), LMdsuJ ("Port
(from)"), a12lSo ("Port (to)"), I5kZVK ("Remote Source"), pbzA+s ("Optional
description") and QAUa4B ("Enter a single port, or define a range by also
filling \"Port (to)\". \"Port (to)\" is optional.") with proper German text so
the modal rendered by DirectionEthertypeSection.tsx, PortRangeSection.tsx,
ProtocolSection.tsx and IcmpSection.tsx shows localized labels and helper copy;
keep the same message keys (no code changes) and only replace the English
strings with their German equivalents in the messages.ts JSON object.
- Line 3: Summary: The German catalog mis-keys oDkgME to the welcome message
(conflicting with English) and must be corrected and the Lingui catalog
regenerated. Fix: update the German messages entry for the key oDkgME so it
contains the German translation that matches the English authorization error
(e.g. "Sie sind nicht berechtigt, Flavors zu erstellen. Bitte melden Sie sich
erneut an."), ensure Ou8mIC remains the welcome message with the ["username"]
placeholder, then regenerate/compile the Lingui catalog (run lingui
extract/compile or your repo's i18n build steps) so the ID→text contract is
consistent across locales; search for keys oDkgME and Ou8mIC in messages.ts to
locate and update the mapping.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 731f7334-bac4-48d2-9a1b-95c94c622162
📒 Files selected for processing (4)
apps/aurora-portal/src/locales/de/messages.poapps/aurora-portal/src/locales/de/messages.tsapps/aurora-portal/src/locales/en/messages.poapps/aurora-portal/src/locales/en/messages.ts
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/aurora-portal/src/server/Network/types/securityGroup.ts`:
- Around line 109-110: The schema for port_range_min and port_range_max
currently allows negative integers because it only applies .max(65535); update
both validators (port_range_min and port_range_max in securityGroup.ts) to
enforce the lower bound by adding .min(0) (e.g., use
z.number().int().min(0).max(65535).nullable().optional()) so port values are
constrained to the valid 0–65535 range.
- Around line 103-127: Update createSecurityGroupRuleInputSchema to enforce
server-side port ordering by adding a refinement/superRefine that checks when
both port_range_min and port_range_max are present and not null that
port_range_min <= port_range_max and otherwise adds a clear validation error;
locate the schema exported as createSecurityGroupRuleInputSchema and implement
the check there (use z.preprocess/superRefine or .refine on the object) so
direct tRPC/API calls are validated, and if the intent is to keep other
invariants on the client, add a short comment above
createSecurityGroupRuleInputSchema documenting that decision.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1928c68f-59dd-4577-a4fb-a3e97ad9ae21
📒 Files selected for processing (2)
apps/aurora-portal/src/server/Network/types/securityGroup.test.tsapps/aurora-portal/src/server/Network/types/securityGroup.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/aurora-portal/src/server/Network/types/securityGroup.test.ts
KirylSAP
requested review from
TilmanHaupt,
andypf,
mark-karnaukh-extern-sap and
vlad-schur-external-sap
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds an end-to-end “Add Security Group Rule” flow to Aurora Portal, including a new modal-based form with presets and frontend validation, plus client-side integration to create rules via the Neutron-backed tRPC mutation.
Changes:
- Introduces
AddRuleModalwith modular form sections, rule presets, and Zod-based frontend validation utilities + extensive component/unit tests. - Wires rule creation into the Security Group details page (new “Add rule” entry point, mutation + cache invalidation, and supporting UI updates).
- Simplifies the server-side
createSecurityGroupRuleInputSchemafrom cross-field validation to primarily type checking.
Reviewed changes
Copilot reviewed 40 out of 47 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| apps/aurora-portal/src/server/Network/types/securityGroup.ts | Updates security group rule creation input schema (validation behavior changed). |
| apps/aurora-portal/src/server/Network/types/securityGroup.test.ts | Updates schema tests to align with the simplified server-side validation. |
| apps/aurora-portal/src/locales/en/messages.ts | Adds/updates i18n strings used by new dialogs/modals and UI text. |
| apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/index.tsx | Integrates rule creation into the SG details route and fetches SGs for remote-source dropdown. |
| apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-modals/DeleteRuleDialog.tsx | Adds a confirmation dialog for deleting security group rules. |
| apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-modals/AddRuleModal/validation/validationHelpers.ts | Adds reusable frontend validators (CIDR/ports/ICMP). |
| apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-modals/AddRuleModal/validation/validationHelpers.test.ts | Adds unit tests for the validation helpers. |
| apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-modals/AddRuleModal/validation/index.ts | Adds a validation barrel export for the feature. |
| apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-modals/AddRuleModal/validation/formSchema.ts | Adds the Zod schema with conditional validation for the Add Rule form. |
| apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-modals/AddRuleModal/types.ts | Adds default form values for AddRuleModal. |
| apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-modals/AddRuleModal/sections/RuleTypeSection.tsx | Adds rule preset selector behavior (sets protocol/ports/ICMP fields). |
| apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-modals/AddRuleModal/sections/RuleTypeSection.test.tsx | Tests RuleTypeSection behavior. |
| apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-modals/AddRuleModal/sections/RemoteSourceSection.tsx | Adds remote source selector (CIDR vs remote SG) and inputs. |
| apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-modals/AddRuleModal/sections/RemoteSourceSection.test.tsx | Tests RemoteSourceSection behavior. |
| apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-modals/AddRuleModal/sections/ProtocolSection.tsx | Adds protocol input for “Other Protocol” rule type. |
| apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-modals/AddRuleModal/sections/ProtocolSection.test.tsx | Tests ProtocolSection behavior. |
| apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-modals/AddRuleModal/sections/PortRangeSection.tsx | Adds port range UI and cross-field validation triggers. |
| apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-modals/AddRuleModal/sections/PortRangeSection.test.tsx | Tests PortRangeSection behavior. |
| apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-modals/AddRuleModal/sections/IcmpSection.tsx | Adds ICMP type/code inputs. |
| apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-modals/AddRuleModal/sections/IcmpSection.test.tsx | Tests IcmpSection behavior. |
| apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-modals/AddRuleModal/sections/DirectionEthertypeSection.tsx | Adds direction and IP version selectors. |
| apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-modals/AddRuleModal/sections/DirectionEthertypeSection.test.tsx | Tests direction/ethertype selection interactions. |
| apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-modals/AddRuleModal/sections/DescriptionSection.tsx | Adds optional description textarea. |
| apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-modals/AddRuleModal/sections/DescriptionSection.test.tsx | Tests DescriptionSection behavior. |
| apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-modals/AddRuleModal/rulePresets.ts | Adds rule preset definitions (services + custom types). |
| apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-modals/AddRuleModal/constants.ts | Adds constants for defaults and validation ranges. |
| apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-modals/AddRuleModal/AddRuleModal.tsx | Implements the main Add Rule modal, payload mapping, and conditional sections. |
| apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-modals/AddRuleModal/AddRuleModal.test.tsx | Tests AddRuleModal rendering and basic interactions. |
| apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-hooks/useSecurityGroupDetails.ts | Adds create rule mutation + cache invalidation and exposes create state/errors. |
| apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-components/SecurityGroupDetailsView.tsx | Plumbs add-rule props through to the rules table. |
| apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-components/SecurityGroupDetailsView.test.tsx | Updates tests for the new props/empty state text. |
| apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-components/-details/index.ts | Adds barrel exports for SG details subcomponents. |
| apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-components/-details/SecurityGroupTabs.tsx | Adds tab UI for “Rules” vs “RBAC Policies”. |
| apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-components/-details/SecurityGroupRulesTable.tsx | Replaces rules table with a version that includes AddRuleModal + popup delete action. |
| apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-components/-details/SecurityGroupRulesTable.test.tsx | Updates tests to match new toolbar/filter behavior and actions. |
| apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-components/-details/SecurityGroupHeader.tsx | Adds a new SG header component. |
| apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/securitygroups/$securityGroupId/-components/-details/SecurityGroupBasicInfo.tsx | Adds a new “Basic Info” details panel. |
| apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/-components/SecurityGroups/constants.ts | Removes shared filter constants file. |
| apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/-components/SecurityGroups/List.tsx | Inlines shared filter constants. |
| apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/-components/SecurityGroups/-components/SecurityGroupTableRow.tsx | Makes table rows clickable and tweaks styling. |
| apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/-components/SecurityGroups/-components/SecurityGroupListContainer.tsx | Simplifies empty state rendering and trims imports/props usage. |
| apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/-components/SecurityGroups/-components/SecurityGroupListContainer.test.tsx | Updates tests for the new empty state text. |
| apps/aurora-portal/src/client/routes/_auth/accounts/$accountId/projects/$projectId/network/-components/SecurityGroups/-components/-details/SecurityGroupRulesTable.tsx | Removes the previous rules table implementation. |
| apps/aurora-portal/src/client/routeTree.gen.ts | Updates generated route tree due to route file move/indexing. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
andypf
approved these changes
Mar 26, 2026
mark-karnaukh-extern-sap
approved these changes
Mar 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR implements a comprehensive "Add Security Group Rule" feature with a full-featured modal form, allowing users to create firewall rules for security groups to control ingress and egress traffic. The implementation includes dynamic field validation, 20+ rule type presets, complete test coverage, and integration with the OpenStack Neutron API.
Changes Made
Frontend Implementation (Client)
AddRuleModal Component System
Form Sections (7 focused, testable components)
RuleTypeSection (66 lines): Dropdown with 20+ predefined rule presets
DirectionSection (65 lines): Ingress/Egress traffic direction selection with radio buttons
EthertypeSection (65 lines): IPv4/IPv6 selection (conditionally shown for Security Group remote source)
ProtocolSection (31 lines): Protocol dropdown for "Other Protocol" rule type
PortRangeSection (97 lines): Port range inputs for TCP/UDP
IcmpSection (47 lines): ICMP Type and Code inputs
RemoteSourceSection (100 lines): Remote traffic source configuration
DescriptionSection (31 lines): Optional description field with comprehensive help text
Supporting Files
Validation Layer
formSchema.ts (209 lines): Zod schema with complex conditional validation
validationHelpers.ts (200 lines): Reusable validation utilities
validatePortRange: Port number and range validation with protocol-specific rulesvalidateIcmpTypeCode: ICMP field validation (0-255 range)isValidCIDR: IPv4 and IPv6 CIDR format validation with regexdetectCIDRFamily: Auto-detect IP family from CIDR notationBackend Implementation (Server)
securityGroupRuleRouter.ts: Enhanced
createmutationPOST /v2.0/security-group-rulessecurity_group_rulekeywithErrorHandlingsecurityGroup.ts: Type definitions and schemas
createSecurityGroupRuleInputSchema: Zod schema for API input validationCreateSecurityGroupRuleInput: TypeScript type for type-safe rule creationsecurityGroupHelpers.ts: Error handling utilities
SecurityGroupRuleErrorHandlers.create(): Specific error messages for creation failuresIntegration Points
Testing (1,800+ lines of test code)
Component Tests
Validation Tests
Related Issues
POST /v2.0/security-group-rulesScreenshots
Testing Instructions
Automated Testing
Install dependencies:
Run the test suite:
pnpm run testExpected: All tests pass including 11 new test files for AddRuleModal
Run type checking:
Expected: No TypeScript errors
Manual Testing
Setup
Start development server:
Navigate to: Network → Security Groups → Click on any security group
Test Cases
1. Rule Type Presets
2. Custom Rules
3. Validation Testing
Port validation:
ICMP validation:
CIDR validation:
4. Remote Source Types
5. Direction and Ethertype
6. Form Behavior
7. Integration
Checklist
Summary by CodeRabbit
New Features
Improvements
Bug Fixes / Behavior Changes
Tests