Repository navigation
feat(compute): public-access command and private addresses - #373
Conversation
- `insta compute public-access [on|off] [service]` and `insta <redis|mysql|mongodb> public-access [on|off] [service]`: show the service's public access and its private address (<name>.insta.internal, <name>-<type>.insta.internal for a database); off makes it private-only, asks first, --yes when non-interactive; the platform's coded refusals (private_network_disabled, private_network_unavailable_in_region, no_private_name, private_name_taken) read as CLI instructions - `insta service list` shows `private <host>:<port>` and `public off` Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
c6ee9aa to
bfc1da8
Compare
There was a problem hiding this comment.
1 issue found and verified against the latest diff
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="test/private-network.test.ts">
<violation number="1" location="test/private-network.test.ts:23">
P3: These fixtures mirror the name-derived address convention, so they do not catch a regression that composes the host or FQDN instead of using the platform-reported fields. Use deliberately non-derived host and FQDN values in the show and list fixtures.</violation>
</file>
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
| import { closeWarning, publicAccessErrorMessage, publicAccessLines, servicePublicAccess } from '../src/commands/private-network.js' | ||
| import { serviceListLine } from '../src/commands/services.js' | ||
|
|
||
| const API = { id: 's1', type: 'compute', name: 'api', public: true, port: 8080, private_host: 'api.insta.internal', private_fqdn: 'api.0123abcd.insta.internal' } |
There was a problem hiding this comment.
P3: These fixtures mirror the name-derived address convention, so they do not catch a regression that composes the host or FQDN instead of using the platform-reported fields. Use deliberately non-derived host and FQDN values in the show and list fixtures.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At test/private-network.test.ts, line 23:
<comment>These fixtures mirror the name-derived address convention, so they do not catch a regression that composes the host or FQDN instead of using the platform-reported fields. Use deliberately non-derived host and FQDN values in the show and list fixtures.</comment>
<file context>
@@ -0,0 +1,134 @@
+import { closeWarning, publicAccessErrorMessage, publicAccessLines, servicePublicAccess } from '../src/commands/private-network.js'
+import { serviceListLine } from '../src/commands/services.js'
+
+const API = { id: 's1', type: 'compute', name: 'api', public: true, port: 8080, private_host: 'api.insta.internal', private_fqdn: 'api.0123abcd.insta.internal' }
+const CACHE = { id: 's2', type: 'redis', name: 'cache', public: true, port: 6379, private_host: 'cache-redis.insta.internal', private_fqdn: 'cache-redis.0123abcd.insta.internal' }
+const printed = () => vi.mocked(info).mock.calls.map((c) => c[0]).join('\n')
</file context>
jwfing
left a comment
There was a problem hiding this comment.
Summary
No blocking defects found in the reviewed head commit. The commands follow existing access-control conventions and match the linked platform contract. Local validation was blocked by sandbox startup failure; review used GitHub files pinned to the PR head.
Requirements context
Assessed against the PR description, platform PR #672’s explicit networking contract, README, supplied AGENTS.md, and the repository’s developing-insta-cli guide. The existing Postgres public-access implementation establishes confirmation and approval conventions. No separate CLI-specific networking design document was verified.
Findings
Critical
(none)
Suggestion
- test/private-network.test.ts: Cover JSON, branch selection, and pending approval - The tests cover confirmation, refusals, rendering, and mutation requests well. Add cases for JSON show/change output, an explicit branch override, and approval_required responses that must not print success. These paths are implemented but currently unasserted.
Information
- Functionality: Implementation matches the platform contract - Service resolution uses the selected branch and service type; addresses come from platform fields; closing requires confirmation; reopening skips confirmation; all four specified refusal codes have CLI guidance. No blocking functionality finding.
- Security: Existing authentication and approval handling retained - Requests use the authenticated API client and handleApproval before reporting success. The change introduces no shell execution, SQL, dependencies, or credential-value logging. No security finding.
- Performance: Bounded request count and rendering work - Show performs one service-list request; mutations add one PUT. List rendering adds constant work per row without additional requests. No performance finding.
- Rollout: Coordinate the platform migration and reference update - Platform #672 changes compute’s public column from its historical false value to actual access state. Before that migration, this CLI can label a publicly reachable compute service as public off. Deploy the platform migration before releasing this surface, and include the separately described cli-reference.md update.
Tests
No tests or typecheck ran: shell commands failed before execution with a bwrap namespace-permission error. Inspected all changed files through GitHub at bfc1da8, plus relevant helpers and the platform contract. Could not cross-check local checkout contents. The reported 2189 passing tests were not independently verified.
Verdict
approved: No Critical findings were identified; Suggestions and Information do not block approval under the specified rule.
jwfing
left a comment
There was a problem hiding this comment.
Summary
No blocking defects were identified in the reviewed changes. The commands implement the stated confirmation policy, platform error mappings, and private-address display using existing CLI helpers. Local verification was blocked by the shell sandbox; review continued through read-only GitHub access.
Requirements context
Assessed against the supplied PR description, linked platform PR #672 and its API changes, the repository README, development skill, and existing Postgres access implementation. No standalone CLI private-network requirements document was identified in the inspected material. The separate skills-reference update was reported by the author but not verified.
Findings
Critical
(none)
Suggestion
- test/private-network.test.ts: Cover JSON output, branch selection, and pending approvals - The tests cover confirmation, refusal mappings, and list formatting, but do not exercise successful JSON output, explicit branch selection, or an approval_required response. Add cases asserting the exact branch query, JSON-only success output, and absence of a success message when approval is pending.
- PR description: Document MCP availability for the new commands - The development guide asks new leaf commands to name their matching MCP tool or explain why none is appropriate, explicitly as a non-blocking expectation. The description does not record that decision; add the tool or companion issue reference.
Information
- Functionality: Core behavior matches the platform contract - The handler selects services within the requested/current branch, submits a boolean public value to the service-ID access endpoint, and prints platform-reported private names without deriving them. The four refusal codes match the linked platform change. Confirmation and reopening behavior follow the existing Postgres conventions.
- Security: Existing authentication and approval paths are retained - Requests use the authenticated ApiClient and shared approval handler. Closing without --yes refuses in noninteractive or JSON mode before issuing the PUT. No new shell/SQL execution, dependencies, or explicit credential-value logging was introduced in the reviewed diff.
- Performance: No material performance issue identified - Showing access performs one service-list request; changing it adds one PUT. Service selection is linear in the returned list, and the added list formatting performs constant work per row without extra network requests.
- Testing: Static review completed with execution limitations - Inspected the changed source and tests at commit bfc1da8 through GitHub, plus surrounding helpers and API behavior. The new pure rendering helpers and injected confirmation dependency fit the repository's testable-command conventions. The checked-out files could not be independently cross-checked.
Tests
No tests or typecheck ran: shell commands failed before execution with 'bwrap: No permissions to create a new namespace'. The reported 2189 passing tests and negative controls were not independently reproduced. No files or external state were modified.
Verdict
approved: No Critical findings were identified; Suggestions and Information do not block approval under the specified verdict rule.
Summary
The CLI side of compute private networking (platform InsForge/instacloud-platform#672; the plane has had it since insta-compute #413).
insta compute public-access [on|off] [service]andinsta <redis|mysql|mongodb> public-access [on|off] [service]:private_host:port, plusprivate_fqdn). It never composes one.offmakes the service private-only:*_URLstops connecting; apps use*_PRIVATE_URL.--json, it refuses (exit 2) unless--yesis passed. This is the same contract asinsta postgres public-access off.onre-opens without asking.private_network_disabled,private_network_unavailable_in_region,no_private_name(namesinsta service rename), andprivate_name_taken.insta service listshowsprivate <host>:<port>and, for a closed compute service,public off. A row with noprivate_hostprints exactly as before.Command surface mirrored in skills (
cli-reference.md, separate PR).Tests
test/private-network.test.ts:--yes(TTY and--json);--yes;test/help-surface.test.ts: the managed database verb list and the compute verbs includepublic-access.--yesgate.public offbadge.npm run typecheckandnpm testpass: 2189 tests, after updating help-surface.🤖 Generated with Claude Code
Summary by cubic
Adds the CLI side of compute private networking: new
public-accesscommands for compute and managed database services, plus a private address shown ininsta service list.insta compute public-access [on|off] [service]andinsta <redis|mysql|mongodb> public-access [on|off] [service]show public access and the platform-reported private address;offmakes the service private-only (compute URLs answer 403 and stop waking the service; database*_URLs stop connecting, apps use*_PRIVATE_URL), andonre-opens without asking.--yeswhen there is no terminal or with--json; the platform's coded refusals now surface as CLI instructions (for example, a missing private name suggestsinsta service rename).insta service listshowsprivate <host>:<port>and apublic offbadge for a closed compute service; rows without a private host print exactly as before.Written for commit bfc1da8. Summary will update on new commits.