Skip to content

feat(networking): add an Azure Virtual Networks adapter - #166

Open
TheSaifZaman wants to merge 4 commits into
floci-io:mainfrom
TheSaifZaman:feat/azure-networking
Open

feat(networking): add an Azure Virtual Networks adapter#166
TheSaifZaman wants to merge 4 commits into
floci-io:mainfrom
TheSaifZaman:feat/azure-networking

Conversation

@TheSaifZaman

Copy link
Copy Markdown
Contributor

Extends the existing networking category to Azure.

No catalog row and no new CloudResource type — a VNet is normalized with type
vpc, the same type an AWS VPC uses, so the shared CIDR column renders for both
clouds without widening the union.

Create and delete are available for Azure while AWS stays partial

That asymmetry is deliberate, and the schema is per-cloud so it costs nothing. A
VNet needs a name, a location and an address prefix — a flat form expresses that
fine. The AWS flows need the dependent selectors that pushed them into the
Networking panel.

Both verbs were verified against the runtime rather than inferred from the AWS
shape: PUT returns 200 with provisioningState: Succeeded, and DELETE returns
200 with the VNet gone. This is worth stating because the sibling AKS adapter in
#164 went the other way — there, create returns 201 but the cluster is permanently
Failed, so it is refused. Same runtime, opposite conclusions, both from probing.

Notes from probing

  • Resources are addressed as resourceGroup/name, like the other Azure adapters:
    ARM cannot address a VNet without its resource group.
  • The resource group is matched case-insensitively, and the id carries the
    runtime's spelling.
    This applies up front the fix review asked for on the
    compute adapter in feat(compute): add an Azure Virtual Machines adapter #161, rather than waiting to repeat the same mistake.
  • create validates that the address space is an IPv4 CIDR, and that the optional
    first subnet has both its fields or neither — half a subnet is not something
    ARM can act on, and silently dropping the one value the user supplied would be
    worse than saying so.
  • ARM returns no creation time for a virtual network, so createdAt is null
    rather than an invented value.

Verification

lint, type-check, test and build pass from the repo root. 17 adapter tests;
hermetic — the networking, capability-guard and catalog suites (101 tests) pass with
globalThis.fetch replaced by a throw.

End to end through the route: the Azure nav entry is available, create with a
first subnet using a differently-cased resource group (RG-VM → id rg-vm/…),
inspect, delete, and all three validation rejections returning 400.

Merge note

Branched off main. Touches only networkingSchema.ts (adds a function, leaves
awsNetworkingSchema untouched), the new adapter, and one registry line. No SPI or
type change, so it conflicts with nothing else open.

Comment thread packages/api/src/cloud-spi/networkingSchema.ts Outdated
Comment thread packages/api/src/cloud-spi/networkingSchema.ts
@greptile-apps

greptile-apps Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Adds an Azure Virtual Networks adapter under the existing networking category (type vpc), with create/delete/list/inspect wired through the registry and schema.

  • New AzureNetworkingAdapter with per-resource-group list aggregation, case-insensitive resource-group resolution, and create/delete against ARM.
  • azureNetworkingSchema() with tightened IPv4 CIDR and VNet name validation; AWS schema unchanged.
  • README matrix and NetworkingPanel copy updated for Azure availability.

Confidence Score: 5/5

This PR appears safe to merge; prior CIDR and VNet name validation issues are addressed and no blocking failures remain.

No blocking failure remains. CIDR octets and prefix length are bounded and enforced on address and subnet prefixes; VNet names require 2–64 characters with start/end rules, with tests covering the previously reported cases.

Important Files Changed

Filename Overview
packages/api/src/cloud-spi/networkingSchema.ts Azure networking schema with bounded CIDR pattern (0–255 octets, /0–32) and 2–64 VNet name pattern matching the prior review fixes.
packages/api/src/adapter-azure/AzureNetworkingAdapter.ts Full Azure VNet adapter using the shared validation helpers; create/delete/list/get and resource-group casing handled as described.
packages/api/src/adapter-azure/AzureNetworkingAdapter.test.ts Hermetic tests cover mapping, list scope, subnet pairing, RG case, out-of-range CIDRs, and VNet name edges.
packages/api/src/cloudProxy.ts Registers AzureNetworkingAdapter in the cloud adapter registry.
packages/frontend/src/components/NetworkingPanel.tsx Azure-specific panel copy so “coming soon” is not shown under a live VNet table.
README.md Service matrix marks Azure networking as list/inspect/create/delete.

Reviews (4): Last reviewed commit: "fix(networking): list Azure VNets per re..." | Re-trigger Greptile

@hectorvent hectorvent left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for another carefully probed adapter. Applying the case-insensitive resource group handling from the #161 review up front, validating CIDRs with real octet and prefix bounds instead of a digits-and-slashes pattern, and refusing half a subnet are all exactly the right calls, and the hermetic test suite is genuinely strong.

One blocker, and it hides well:

list() will always come back empty against the runtime. The adapter lists at subscription scope (GET /subscriptions/{s}/providers/Microsoft.Network/virtualNetworks), but floci-az only supports the resource group scoped VNet list (its network service docs list only .../resourceGroups/{rg}/providers/Microsoft.Network/virtualNetworks). The runtime routes the subscription scoped path into its network service with a placeholder resource group, matches it strictly, and returns 200 with an empty value, so the call succeeds while the table stays empty even after a successful create. Your verification notes line up with this: create, inspect, delete, and the nav entry are all confirmed, but a created VNet showing up in the list is not. Could you switch list() to enumerate resource groups (you already fetch them in resolveResourceGroup) and aggregate the per group results, then confirm a created VNet actually appears in the table? The README's Yes (list, ...) claim rides on this too.

Two small things, neither blocking:

  • With this adapter live, the NetworkingPanel still shows "Networking management coming soon for AZURE" underneath a now functional Azure table. You deliberately touched no frontend here, which I appreciate, so a follow-up is fine, but a one-line gating or copy tweak for azure would also be welcome in this PR if you prefer.
  • Agreed with your comment that lifting the shared json helper onto AzureRuntimeClient should wait until the open Azure PRs land; thanks for writing that reasoning down in the code.

Everything else looks right: registration is the single line the catalog pattern needs, reusing the vpc type keeps the shared CIDR column meaningful without widening the union, and createdAt: null instead of an invented time is exactly the no fake data rule from AGENTS.md.

Extends the existing networking category to Azure. No catalog row and no
new CloudResource type: a VNet is normalized with type `vpc`, the same
type an AWS VPC uses, so the shared CIDR column renders for both.

Create and delete are advertised as available for Azure while AWS keeps
them `partial`. That asymmetry is deliberate and the schema is per-cloud:
a VNet needs a name, a location and an address prefix, which a flat form
expresses fine, whereas the AWS flows need the dependent selectors that
pushed them into the Networking panel. Both verbs were verified end to end
rather than inferred from the AWS shape — create returns Succeeded and
delete removes the VNet.

Notes from probing:

- Resources are addressed as `resourceGroup/name`, as the other Azure
  adapters are: ARM cannot address a VNet without its resource group.
- The resource group is matched case-insensitively and the id carries the
  runtime's spelling, so the returned id is the one list() reports. This
  applies the fix review asked for on the compute adapter up front rather
  than waiting to repeat the mistake.
- create validates that the address space is a CIDR, and that the optional
  first subnet has both its fields or neither — half a subnet is not
  something ARM can act on, and dropping the one supplied value silently
  would be worse than saying so.
- ARM returns no creation time for a virtual network, so createdAt is null
  rather than an invented value.

Verified end to end through the route: the Azure nav entry is available,
create with a first subnet using a differently-cased resource group,
inspect, delete, and the three validation rejections returning 400.
Three review findings, all cases where an invalid create reached ARM and
failed with an opaque runtime error instead of a clear ValidationError.

- CIDR_PATTERN bounded octets to 0-255 and the prefix length to 0-32.
  The previous digits-and-slashes pattern accepted 999.999.999.999/99.
  Applies to the subnet prefix too.
- VNet names now follow Azure's rule rather than a loose approximation:
  2-64 characters, starting with a letter or digit and ending with a
  letter, digit or underscore. A one-character name or a trailing hyphen
  previously only failed on the PUT. This is the provider's rule, not the
  runtime's — floci-az accepts names real Azure rejects.
- json() now sends `accept: application/json` and maps 204 to null,
  matching the azureJson and cosmosJson helpers the other Azure adapters
  already use.

Five near-identical copies of that helper now exist under adapter-azure/.
Worth lifting onto AzureRuntimeClient once the open Azure PRs merge; doing
it across three parallel branches would only conflict.

Verified through the route: all four invalid inputs return 400 before
reaching ARM, and a valid VNet still creates.
@TheSaifZaman
TheSaifZaman force-pushed the feat/azure-networking branch from ad4233d to da0e4ed Compare July 29, 2026 03:09
@TheSaifZaman

Copy link
Copy Markdown
Contributor Author

Rebased onto fd3bd2f after #147, #152 and #155 merged — no textual conflicts.

Same README gap as #161 and #164: the branch registers the Azure VNet adapter but the committed table still read No for Azure, where service-matrix.ts generates Yes (list, inspect, create, delete). Regenerated in its own commit.

Gate green after the rebase: lint, type-check, 448 tests, build.

The subscription-scoped ARM path answers 200 with an empty value on floci-az, so the table stayed empty after a successful create. Enumerate resource groups and aggregate instead, and make the stub mirror the runtime so the permissive catch-all cannot hide this again.
@TheSaifZaman

TheSaifZaman commented Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

@hectorvent You were right, and it reproduced exactly as you described. Thank you for catching it — this was a silent failure, which is the worst kind.

Verified against the runtime before changing anything (2026-07-29): created vnet-review in rg-probe, then

  • GET /subscriptions/{s}/providers/Microsoft.Network/virtualNetworks{"value":[]}
  • GET /subscriptions/{s}/resourceGroups/rg-probe/providers/Microsoft.Network/virtualNetworks["vnet-review"]

So the call succeeded and the table stayed empty, precisely as you said.

Fixed in 592361c. list() now enumerates resource groups and aggregates the per-group results, reusing the lookup resolveResourceGroup already needed (refactored into resourceGroupNames() so both share it). One unreadable group degrades to empty rather than blanking the whole table.

Confirmed end to end through the console API, which is the check that was missing from my original verification:

GET /api/clouds/azure/services/networking/resources
[{"id":"rg-probe/vnet-review","name":"vnet-review"}]

Why the tests did not catch this, since that matters more than the fix: the stub's fallback branch answered any URL with the VNet list, so subscription-scope and group-scope were indistinguishable to it. It now mirrors floci-az — only the group-scoped collection returns VNets, the subscription-scoped path returns {"value":[]} — plus two new tests: one asserting every collection call is group-scoped, one asserting aggregation across two groups. The old permissive stub would fail against the new assertions.

I also took the NetworkingPanel copy tweak rather than deferring it, since "coming soon" sat directly under a table that now works. Azure gets: "Virtual networks are managed in the table above. Subnets, security groups, gateways and route tables are AWS-only today." Other clouds keep the original wording.

Gate green: lint, type-check, 450 tests, build.

@fredpena

fredpena commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Hi @TheSaifZaman, thank you for this contribution. The overall direction is good: using the existing networking category, normalizing Azure VNets as vpc, and listing per resource group all fit the current multi-cloud explorer well.

Before we merge, could you please make these adjustments?

  1. Prevent a duplicate create from updating an existing VNet. The adapter currently sends a PUT directly, and the runtime treats that as an upsert. Since the UI exposes this as “Create VNet” and has no update flow, it should first check whether the VNet already exists and return a ConflictError instead of modifying it.

  2. Validate that subnetPrefix is contained within addressPrefix, or adjust the schema text accordingly. The current UI says the first subnet must be inside the address space, but the adapter only validates that both values are valid IPv4 CIDRs.

  3. Please add focused tests for:

    • duplicate VNet creation returning a conflict without issuing the PUT;
    • a subnet CIDR outside the VNet address space returning a validation error without issuing the PUT.

The JSON helper duplication is noted, but it does not need to be refactored in this PR. Keeping this PR focused on Azure Virtual Networks is preferable.

Once these points are addressed, please run the full checks and verify create, list, inspect, and delete against Floci-AZ. Let me know if anything in the current architecture is unclear. Thanks again for the solid groundwork.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants