feat(gcp): add Compute PacketMirroring - #190
Merged
Merged
Conversation
Packet mirroring is how traffic gets to an inspection appliance without touching the VMs being inspected: GCP copies their packets and delivers the copies to an internal passthrough load balancer. Nothing in the plugin could declare one, so an IDS or a flow recorder had to be wired up outside formae and then drifted silently. Two API behaviours needed compensating for, and both are the kind that look fine until they are not. `network` is fixed once set, and packetMirrorings.patch refuses a body carrying it in any spelling other than the exact stored URL -- "Network cannot be changed". A forma names a network by self link, short path or bare name interchangeably, so leaving the field in an update body makes whether the update works depend on how the reference happened to be written. It is createOnly, so it is dropped from update bodies entirely and a change to it replaces the resource. patch is a JSON merge patch, so a selector left out of `mirroredResources` keeps its old value rather than being cleared. A forma that drops every tag would leave the tags mirroring while the plugin reported the update as applied -- a change that silently does not happen, which is worse than one that fails. The absent selectors go out as explicit empty lists, which does clear them, and the conformance case's update drops a tag the create declared so this is exercised rather than asserted. Every reference here is an object rather than a bare string, and GCP answers each with a second `canonicalUrl` naming the same target by numeric id. It is output-only but sits inside a sub-resource, so it is stripped on read; otherwise every read disagrees with the declaration and plans an update that changes nothing. The conformance case costs nothing to run: mirroring a subnet rather than named instances means no VM has to exist to have something to mirror, and the collector's backend service carries no backends. Probed against the live API in development-477117: create, read, patch and delete, plus the merge and immutability behaviours above. Conformance has not been run.
The first CI run failed at Update with no message - the apply path carries no error text, so the API's reason is not recoverable from the log. The update fixture moved seven things at once: description, the tag selector, cidrRanges, IPProtocols, direction, priority and enable. Only the first two were verified live while the batch was built; the merge-patch behaviour they exercise is the point of the case, and dropping a selector is what proves the plugin sends an explicit empty list rather than letting the old value survive. The other five are held at their created values until someone can probe which of them the API refuses to patch. Shipping them untested turns one unexplained red into five candidates.
The naming standardisation landed while this branch was open, so its two fixtures were the only ones left calling things formae-plugin-sdk-test-. That prefix is no longer swept by name alone, and the point of the convention is that there is one shape - a batch that keeps its own would leak exactly the way the others did.
naxty
force-pushed
the
naxty/gcpComputeFirewallPolicies
branch
from
September 3, 2026 09:35
974d647 to
297fbea
Compare
naxty
marked this pull request as ready for review
September 3, 2026 09:36
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.
Adds
GCP::Compute::PacketMirroring: a copy of selected VMs' traffic, delivered to an internal passthrough load balancer for inspection.mirroredResourcessays whose packets to copy — named instances, whole subnets, or network tags — andcollectorIlbsays where the copies go. The collector must be a forwarding rule created withisMirroringCollector; an ordinary internal rule is refused.Conformance has NOT been run
No
make installand nomake conformance-*target was run in this worktree, becausemake installdoesrm -rf $HOME/.pel/formae/plugins/gcp— a shared path other agents are using. Thepacket-mirroringandpacket-mirroring-updatecases have never executed. Everything below was established by probing the live API directly with curl againstdevelopment-477117, not by running the harness.What the probe established
Created and deleted, in
europe-central2, against the real API:packetMirrorings.listinsertisMirroringCollector)getpatchdescription,priority,enable,filter,mirroredResourcesdeleteEvery probe resource, including the network and subnet, was deleted and the deletion verified by listing.
Cost: none. Mirroring a subnet rather than named instances means no VM has to exist to have something to mirror, and the collector's backend service carries no backends. Packet mirroring bills on mirrored traffic, and nothing is mirroring.
Two API behaviours the type compensates for
networkcannot be patched. It is fixed once set, and the API answersInvalid value for field 'resource.network.url': ... Network cannot be changedto a patch carrying it in any spelling other than the exact stored URL. A forma names a network by self link, short path or bare name interchangeably, so leaving the field in an update body would make whether the update works depend on how the reference happened to be written. It iscreateOnly, so it is dropped from update bodies entirely and a change to it replaces the resource.patchis a JSON merge patch. A selector left out ofmirroredResourceskeeps its old value rather than being cleared — verified directly: patching{"mirroredResources":{"tags":["t"]}}left the existingsubnetworksin place, and a subsequent patch omittingtagsdid not remove the tag. A forma dropping every tag would therefore leave the tags mirroring while the plugin reported the update as applied — a change that silently does not happen. An explicit empty list does clear a selector (also verified), so the absent selectors are filled with empty lists on update. Lists that are present replace rather than merge, so nothing else needed changing.The
-update.pklcase drops a tag the create declared, so this is exercised rather than asserted — if the transformer regresses, the update verify fails.Read-back mismatches handled
{url: ...}) rather than a bare string, and GCP answers each with a secondcanonicalUrlnaming the same target by numeric id. Output-only, but it sits inside a sub-resource, so it is stripped in the response transformer — otherwise every read disagrees with the declaration and plans an update that changes nothing.regioncomes back as a full URL and is reduced to its last segment, as for every other regional compute type.urlsent as a shortprojects/…path reads back as a fullhttps://www.googleapis.com/compute/v1/…URL. The fixture passesres.selfLinkthroughout, which is already the full form, so the two agree.priorityis a plain JSON number, not a stringified int64 — noInt/string mismatch here.priority→ 1000,enable→"TRUE",filter→{"direction":"BOTH"}. All three carryhasProviderDefaultwithpendingrows inschema/provider-default-dispositions.json.Two types dropped from the batch
firewallPolicies— already implemented. It ships asGCP::Compute::NetworkFirewallPolicyandGCP::Compute::RegionNetworkFirewallPolicy(schema/pkl/compute/networkFirewallPolicy.pkl,regionNetworkFirewallPolicy.pkl), both already using thefirewallPoliciesURL segment. Adding it would have been a duplicate resource type andmake verify-schemawould have failed.networkEdgeSecurityServices— not creatable in this project, and billable if it were.insertwithvalidateOnly=true(so nothing was created and nothing charged) answers:and the
CLOUD_ARMOR_NETWORKsecurity policy it exists to attach answers the same. That tier is a paid Cloud Armor Enterprise subscription. Two further notes for whoever picks this up: the type has no per-regionlistmethod at all — onlyaggregatedList(which does work, returning nothing) — so discovery would need a different shape from every other regional compute type; and enabling it turns on advanced network DDoS protection, which is what the subscription bills for.Gates
All run and passing in this worktree:
go build ./...golangci-lint run ./...make test-unit(7 new//go:build unittests for the transformers)make verify-schema(200 resource types, 0 duplicates; parity check ok)pkl eval formae-plugin.pkl --format jsonbash -nover every tracked*.shmake lint-reusepkl evalagainst the local schema, and the rendered JSON checked: the resolvable references land correctly at all three nesting depths, includingmirroredResources.subnetworks[].urltwo levels down.Not verified
-replace.pkl: the type has mutable fields, so-update.pklis the right case per the deliverable spec. A replace case would exercise the immutablename, but writing one that has never been run seemed worse than not having one.collectorIlbcan be repointed at a different forwarding rule. A patch carrying the same collector is accepted; changing it to another rule would have meant standing up a second ILB, which the probe did not do. The field is registered as mutable on the strength of the API reference; if it turns out to be fixed, it needscreateOnly.