P81 142148 update checkpoint sase provider to use v 3 public api - #5
Merged
chkp-valentynk merged 138 commits intoAug 28, 2026
Merged
Conversation
classifyAPIError buckets SDK errors into not-found, conflict, transient and other, which the drift and idempotency conventions depend on. Previously every error went through appendErrorDiags uniformly. Adds a temporary replace directive for local SDK development; removed before release.
v3 collapses Wireguard, OpenVPN, IPSec-Single and IPSec-Redundant into the StandardTunnels tag and folds Gateways into Standard Networks. Method names and signatures are unchanged, so this is a receiver rename only.
v3 merges Objects Addresses and Objects Services into one Objects tag, renames Regions -> Standard Regions, Route Table -> Standard Route Tables, and Application -> Applications. Two operationIds also changed: getObjectsAddresses -> getAddresses, postObjectsAddresses -> createAddress. Receiver renames also applied to the two acceptance test files, whose "conn." receiver isn't compiled by `go build` but should stay consistent with the production code. Also, three small targeted fixes surfaced by the retarget: - PeakBandwidth -> PeakBandwidthMbps on the 4 sites where the spec property was genuinely renamed (EnhancedTunnel/StaticTunnelCreate); left the other 5 PeakBandwidth sites alone since those structs (IPSecSharedSettings family, StaticTunnelUpdate) dropped the field entirely in v3 rather than renaming it -- that's a structural change for a later tunnel task, not this one. - Dropped the now-nonexistent perimeter81Sdk.ASN/RemoteASN wrapper types (Phase 0 removed the redundant oneOf that forced codegen into unusable wrappers); parseASNString now returns plain int32, and the three call sites assign directly since the generated SDK fields (DynamicTunnelDetails.RemoteASN, EnhancedIPSecSharedSettingsCreate.LeftASN, IPSecRedundantTunnelPayload.RemoteASN) are all plain int32, not pointers. FirewallPolicyAPI, enhanced-tunnel field/type churn, objects/addresses payload types, application host/port oneOf types, and all_networks are left for their own tasks -- errors there are expected and unchanged.
Fix round 1 finding: unlike PutObjectsAddresses (now UpdateAddress, whose builder needs the Address type from Task 12B), DeleteObjectsAddresses (now DeleteAddress) takes no request body at all -- addressId is a plain positional string parameter. No dependency on the Address type rework, so it did not need to wait for 12B.
…types v3 moved IkeLifeTime/Lifetime/DpdDelay/DpdTimeout/Phase1/Phase2 off EnhancedTunnel and onto a nested *AdvancedSettings, renamed PeakBandwidth to PeakBandwidthMbps, and dropped RemotePublicIP/RemoteID/Description from the read shape entirely (write-only now). Update the static/dynamic tunnel resources and the enhanced-tunnels data source accordingly, preserving prior state for the three fields that can no longer be read back (same pattern as resource_gateway.go's name/idle handling) rather than blanking them, and drop the bandwidth field references that no longer exist anywhere in the IPSecSharedSettings family or on StaticTunnelUpdate. Build errors: 58 -> 21. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…Data Task 12A fix round 1: removing the read of IPSecSharedSettings.PeakBandwidth (which v3 dropped) was leaving the key out of the flattened map entirely, which zeroes shared_settings.peak_bandwidth on checkpointsase_ipsec_redundant on every refresh instead of preserving it. Thread the prior "shared_settings" state into flattenSharedSettingsData so the caller can't forget to preserve it, and document on the schema attribute itself that it is accepted and kept in state but never sent to the v3 API (no such field exists anywhere in the IPSecSharedSettings family under v3). No schema fields changed, no error-count change (21) — this is a behavior fix for an already-broken read path, not a compile fix. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… data source Task 12A fix round 2: data sources never receive prior state from Terraform (ReadDataSource always gets a nil state, unlike ReadResource's CurrentState), so the priorTunnels/priorByID merge added to flattenEnhancedTunnelsData in the original pass was dead code that could never carry anything forward — remote_public_ip/remote_id/description were already always "" on every refresh, not just "the first time a tunnel is observed" as the removed comment claimed. Delete the merge machinery and emit "" directly, with an honest comment explaining why no preservation mechanism is possible here. Also extends the existing comment in flattenDynamicTunnelDetails to note the same "required non-pointer field forces empty-string serialization" behavior already documented for StaticTunnelCreate, applying to DynamicTunnelDetails' AuthType/RemotePublicIP too (comment-only, no logic change). Resource-level preservation (resource_enhanced_static_tunnel.go's Read, flattenSharedSettingsData in utils.go) is untouched — that pattern is correct because resources, unlike data sources, do receive prior state. No error-count change (21); no schema literals altered; gofmt clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Address flips Name/ValueType to *string (Value/Description unchanged); CreateAddress/UpdateAddress now build perimeter81Sdk.Address via .Address(...) and their Execute() returns *DBAddress, whose top-level Id is what gets written to resource state. PutObjectsAddresses is replaced by UpdateAddress. Read paths that assigned the now-pointer Name/ValueType straight into d.Set/flatten maps are switched to the Get*() accessors to avoid storing pointer values in state. GetRouteTable200ResponseInner is renamed to StandardGetRouteTable200ResponseInner (fields unchanged) in data_source_route_table.go. resource_ipsec_redundant.go drops the PeakBandwidth write in the redundant-tunnel create payload — v3 removed the field from IPSecSharedSettingsCreate and every other IPSecSharedSettings-family type, with no replacement. The peak_bandwidth schema attribute and flattenSharedSettingsData's read-side prior-state preservation are left untouched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… maps
flattenObjectServicesProtocols and flattenProtocolsDataSourceData assigned
ObjectsServicesProtocolResponseObj.ValueType straight into a
map[string]interface{}. ValueType flipped from string to *string in v3;
the map sink accepts anything so this compiled but stored the pointer,
not the string. Use GetValueType() (nil-safe) instead. Protocol on the
same struct stayed a required string and is unaffected.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…tructors
Replace the removed NullableOneOfFixedHostIdpHost/NullableOneOfFixedPortIdpPort
interface{}-boxing constructors with v3's typed FixedHostAsCommonCreateApplicationHost/
FixedPortAsCommonCreateApplicationPort, matching the plain (non-Nullable) Host/Port field
types on HttpCreateApplication/HttpsCreateApplication/RdpCreateApplication. Also fix
ApplicationAuth.AuthEnabled, now *bool in v3, by taking the address of a named local
instead of a literal, preserving the hardcoded auth-disabled behavior for RDP creates.
v3 deletes GET /networks with no replacement. Rather than removing a registered data source and breaking customer configs, this fans out to the two surviving list endpoints and merges client-side. Adds a computed network_kind attribute so consumers can tell the two apart, and replaces the timestamp-derived ID with a stable one. Populates every shared field (including dns, access_type, is_default) for both standard and enhanced rows, since Network and EnhancedNetwork expose identical accessors for all ten shared fields.
The previous polling loops only failed on statusCode 500, so a 400 or 409 completion was reported to Terraform as success. They also had no retry budget (one transient EOF orphaned a real network from state, LEFTOVERS L3) and ignored the context deadline. pollAsync requires a 2xx completion, retries transient errors within a budget using exponential backoff, honours ctx, and surfaces result.reason.
v3 replaces /networks/{id}/policy with /networks/{networkId}/firewall-policy,
returns GranularFirewallPolicy, and makes the update asynchronous.
policyLoggingEnabled is already carried by the existing trace attribute, so
only per-rule logEnabled is new. It is Optional with Default false so that
configurations written against v2.3 continue to apply unchanged; trace is
now always sent because v3 declares it required.
The update polls via pollAsync, so a non-2xx completion now fails the apply
instead of being reported as success.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The four per-protocol tunnel services collapsed into StandardTunnelsAPI, and ObjectsAddressObj was replaced by Address. Several fields also flipped from T to *T, so the affected comparisons move to the generated nil-safe getters rather than dereferencing an optional field. Assertions are unchanged in meaning; this only restores compilation, which had been blocking go vet and the whole test binary for the package.
Task 12D's pointer-into-interface{} sweep found two sites where a v3 field
that flipped from string to *string was assigned straight into map/state
destinations, silently corrupting the stored value with a pointer instead
of its contents: ObjectsServicesResponseObj.Description in
resource_object_services.go, and IPSecRedundantTunnel.Passphrase in
flattenTunnelData. The passphrase one is a credential field, so the leak
also meant the IPSec pre-shared key never made it into state correctly.
Both now go through the generated nil-safe GetX() getters instead of the
raw field, matching the pattern already used for the neighbouring Name/
GatewayID accesses on the same structs. A raw dereference was not an
option: both fields are omitempty and the server can omit them.
Adds a regression test for the passphrase case covering both the present
and omitted paths. Also known but deliberately untouched, out of scope for
this task: the very expression this test is written against
(tunnelItem.RemoteID.String) panics on a nil RemoteID rather than leaking
a pointer, because RemoteID is a *RemoteID that flattenTunnelData
dereferences unconditionally two lines below the fix. The test's fixtures
set a non-nil RemoteID to route around that pre-existing bug so the
assertions here actually exercise the passphrase fix instead of crashing.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
flattenTunnelData checked only tunnelItem.RemoteID.String != nil, missing the
tunnelItem.RemoteID != nil guard on the outer *RemoteID pointer. RemoteID is
omitempty, so any tunnel response that omits remoteID panicked on read,
crashing the provider plugin and taking down the whole apply.
A second type-aware sweep for this bug class (unguarded dereferences of an
SDK pointer field, as opposed to Task 12D's pointer-into-interface{} leaks)
checked all 70 reads through an SDK pointer field in the package and found
this was the only unguarded one; resource_ipsec_single.go:390 already does
the same two-level check for the same RemoteID union type, so this just
brings the two read paths in line with each other.
Restores the regression test's RemoteID-less fixtures (present/omitted) to
their originally intended form now that the fix makes them safe, and adds a
third case for the other half of the two-level check: a non-nil RemoteID
wrapper with a nil inner string.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
v3 adds a fifth region (Canada). The Makefile still used NAME=perimeter81 and hashicorp.com/edu, so make install wrote to the pre-rebrand plugin path. Adds a testunit target for the offline test tier.
…tive One test per invariant across every registered type, so new resources in later phases are checked automatically instead of relying on per-resource review. The Sensitive check caught five attributes shipping without the marker, not the two the design spec anticipated: wireguard.vault, wireguard.request_config_token, openvpn.access_key_id, enhanced_static_tunnel.customer_root_ca, and enhanced_dynamic_tunnel.tunnel.customer_root_ca. The first two are why WireGuard private key material appeared in cleartext in provider run logs. Sensitive affects rendering only, so this produces no plan diff. Also fixes the 45-attribute Description gap the new TestSchemaEveryAttributeHasADescription test surfaced, entirely concentrated in the deprecated checkpointsase_networks data source's nested networks/regions/instances/tunnels shape, which had never had per-field descriptions filled in.
Regenerated with `go generate ./...` (tfplugindocs v0.25.0, pinned in go.mod). Eight files change: the new all_networks network_kind and firewall_policy log_enabled rows, the five newly-Sensitive fields now rendering as (String, Sensitive), and the expanded base_url region list. Verified idempotent: a second and third `go generate` produce byte-identical output, so a docs-drift CI gate will not thrash. The CI drift gate itself is NOT included here. Creating .github/workflows/verify.yml was declined, so it is deferred pending a decision rather than worked around.
…pletion checkNetworkStatus only failed on statusCode 500, so a completed 400/409/422 was reported to Terraform as a successful apply. Add pollStandardNetworkStatus, a pollAsync-backed helper that requires a 2xx completion, is context-aware, and retries transient errors, then convert addGatewayToRegion's polling loop to use it. checkNetworkStatus itself is untouched; 25 other call sites still use it and will be migrated in later tasks. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Migrates the remaining checkNetworkStatus call sites in resource_network.go, resource_openvpn.go, resource_wireguard.go, resource_ipsec_single.go, and resource_ipsec_redundant.go to the pollAsync-based helpers in async.go. checkNetworkStatus only failed on statusCode 500, so a completed operation carrying 400/409/422 was reported to Terraform as a successful apply; pollStandardNetworkStatus requires a 2xx completion. Behaviour changes, both deliberate: - Non-2xx async completions now fail the apply at all 13 sites instead of being silently treated as success. - Since pollStandardNetworkStatus returns only an error (no status object to read a message from), every converted error path now appends its own operation-specific diagnostic summary via appendErrorDiags, replacing the old generic "Unable to get Network Status". A bare `return diags` would otherwise make the failure invisible to the operator. async.go: split the single poll interval into standardNetworkPollInterval (60s, network create/update) and standardTunnelPollInterval (20s, tunnel CRUD) so each call site keeps its original cadence explicitly rather than inheriting a hardcoded one; added pollStandardNetworkStatusForResource, which also returns result.resource for the one create path (resourceNetworkCreate) that needs the new object's ID. Remaining checkNetworkStatus call sites drop from 25 to 12 (all in enhanced_* resources, out of scope here). Added TestPollStandardNetworkStatusForResourceReturnsResourceOnCompletedTwoHundred to cover the new resource-returning helper. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ll sites Completes the async-polling migration: converts the last 12 checkNetworkStatus call sites across resource_enhanced_network.go, resource_enhanced_region.go, resource_enhanced_route_table.go, resource_enhanced_static_tunnel.go, and resource_enhanced_dynamic_tunnel.go to pollStandardNetworkStatus / pollStandardNetworkStatusForResource, and deletes checkNetworkStatus now that no callers remain. checkNetworkStatus treated an async operation as failed only when statusCode == 500, so a completion carrying 400/409/422 was reported to Terraform as a successful apply. Every converted site now appends an operation-specific diagnostic on failure instead of silently returning, so non-2xx completions fail the apply everywhere instead of at just the 14 sites migrated previously. Preserves each site's d.Partial(true) placement, post-completion work (id extraction, list-by-name fallbacks), and enclosing control flow exactly; only the polling mechanism and failure semantics changed. Drops the time import from resource_enhanced_region.go, the only file left with no other time.* usage after its four time.Sleep(60s) loops were removed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…lication.go
Converts resourceApplicationCreate's bare for{} loop around
ApplicationsAPI.GetApplicationStatus to pollApplicationStatusForResource,
completing the async-polling migration started in the checkNetworkStatus
call sites.
This site was worse than the others: it checked only GetCompleted() and
never inspected result.statusCode, so a completed 409 fell through to the
list-by-name fallback and could bind Terraform state to a pre-existing
application of the same name instead of failing the apply. Non-2xx
completions now error out before reaching that fallback; the fallback
remains for its legitimate case of a 2xx completion with no resource URL.
Same 30s poll cadence, same diagnostic text, same state writes as before.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…harness testAccProviders registered the provider under local name "sase" while every resource in this provider is checkpointsase_*, so Terraform could never resolve a provider for any TestAcc* configuration -- all 14 acceptance tests failed before running, and the TF_ACC skip guard was masking it. Register under "checkpointsase" and add testAccProviderFactories for steps that need it. Add scripts/upg01.sh, a CLI-driven harness for UPG-01 (upgrading from the v2.3-API provider to v3 must produce an empty plan). The public registry has never had this provider published, so ExternalProviders is not an option; instead the script builds the real v2.3 baseline (commit d4e7810, SDK v2 v2.3.0) and the current tree into a project-local Terraform filesystem mirror and drives real terraform init/apply/plan/destroy against it. UPG01_DRY_RUN=1 stops after `terraform init` so the mirror/build mechanics can be verified without tenant credentials. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…tignore scope Review found that if git worktree remove ever fell through to the bare rm -rf fallback in build_baseline()/cleanup(), the directory would be gone but the administrative entry under .git/worktrees/ would survive, wedging every subsequent run with "fatal: '<path>' is a missing but already registered worktree" until someone pruned by hand. A single Ctrl-C during go build was enough to trigger it. Add a defensive `git worktree prune` at the top of build_baseline() -- a no-op when nothing is stale. Also document that .gitignore's /.upg01-work/ entry only covers the default UPG01_WORK_DIR location; a caller-supplied path elsewhere in the repo must be gitignored by the caller, since the work directory holds Terraform state with tenant data. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…alable
Two release blockers found by the whole-branch review, both invisible to
every existing test because nothing in this branch marshalled a request
body.
firewall_policy sent a zero-value SourcesAndDestinations, a oneOf wrapper
whose MarshalJSON returns (nil, nil) when no variant is set, while ToMap
writes the field unconditionally. encoding/json turns that into a hard
error, so any config with a policy_rules block failed in setBody before a
request was sent. Rule-less policies worked, which is why it went
unnoticed. Now sends an empty address list -- the only shape the provider
can express, since sources/destinations have no schema attributes yet; the
semantics of an empty list still need confirming against a live tenant.
enhanced_dynamic_tunnel never assigned RoutingType, which v3 made a
required value type with no omitempty and an enum of route|policy, so
every create sent "". The sibling static-tunnel path was fixed for this
exact pointer-to-value flip and this one was missed.
Adds golden-body marshal tests over the create/update payloads. Both
fixes are proven falsifiable by reverting them. The goldens also record
two behaviours deliberately deferred: several v3-required strings lost
omitempty and serialize as "" when unset, and NetworkFeaturesCreate{}
now explicitly sends enabled: false for three features v2.3 left at the
server default.
…ailure pollAsync's migration to failing on any non-2xx completion (not just 500) meant a 409 "name already in use" now fell into the network/enhanced-network create paths' adopt-on-failure recovery. That recovery lists existing networks and adopts the first name match into state -- which a 409 guarantees will be the very object that caused the conflict. The adopted object's ID then landed in state via resourceNetworkRead's return, silently discarding the error diagnostic appended moments earlier, so `terraform apply` reported clean success and a later `terraform destroy` would delete a network Terraform never created. Add a typed asyncFailedError (byte-identical Error() string, so existing async_test.go assertions are unaffected) so callers can tell a conflict from any other async failure via the new isAsyncConflict predicate, which reuses errors.go's existing errKindConflict (409) classification instead of inventing a second status list. Both create paths now refuse adoption on a conflict and fail the apply; adoption on any other failure still proceeds but now appends a Warning diagnostic (naming the adopted object and its ID) instead of discarding the create error, and the Read call's diagnostics are appended to -- not returned in place of -- the accumulated diagnostics. Add unit tests for asyncFailedError's message, pollAsync's typed-error return, and isAsyncConflict's status handling. The adoption branches themselves need a live schema.ResourceData and client, so that integration remains covered only by the (TF_ACC-gated) acceptance tests.
…dynamic/redundant
enhanced_static_tunnel's peak_bandwidth is settable only at create in v3 —
StaticTunnelUpdate has no bandwidth field — so an in-place change plans a
diff that apply can never satisfy: Read always writes the unchanged server
value back, and the config never reaches "No changes". Adding ForceNew
routes a change through destroy+create instead, whose create path does
send the new value, restoring convergence. An unchanged config still
plans empty, since ForceNew only triggers when the value actually differs
from prior state.
enhanced_dynamic_tunnel and ipsec_redundant (shared_settings.peak_bandwidth)
have no path to send the value at all, on create or update, so ForceNew
would just be destructive with no corrective benefit. Marked Deprecated
instead and rewrote each Description (plus ipsec_redundant.md's stale
"Required by the downstream service" line) to say plainly that the value
is inert in v3 and kept only for configuration compatibility. Attributes
are kept, not removed, to avoid a breaking schema change.
Regenerated docs/resources/{enhanced_static_tunnel,enhanced_dynamic_tunnel,
ipsec_redundant}.md via go generate; a second run is byte-identical
(deterministic). 34 unit tests still pass, 14 acceptance tests still skip,
0 failures; go build/vet/gofmt clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… secret guard data_source_networks.go rendered the WireGuard config-retrieval token in cleartext via `requestconfigtoken`, unlike its counterpart in resource_wireguard.go (`request_config_token`, already Sensitive). Terraform plan/show and TF_LOG=debug therefore leaked it for any config using data.checkpointsase_networks. The guard meant to catch exactly this (TestSchemaSecretAttributesAreMarkedSensitive) never fired: it did an exact leaf-name lookup against a secretNames set spelled with underscores (request_config_token), while this attribute is spelled without them (requestconfigtoken). The mismatch let the test report zero unmarked secrets despite a real one being present, making it decorative rather than load-bearing. Normalize both the secretNames keys and the extracted leaf name (lowercase, strip underscores/hyphens) before comparing, so spelling variants collapse to the same key. The failure message still reports the original attribute path. Re-run after normalizing surfaced exactly one previously-invisible violation (requestconfigtoken); every other secretNames entry was already marked Sensitive under the old exact-match lookup. Also note in the affected tunnel descriptions (leftallowedip, leftendpoint, requestconfigtoken) that flattenNetworkTunnelsData only populates them for WireGuard tunnels; IPsec/OpenVPN entries read back empty.
pollAsync documented that it honours ctx so a Terraform timeout produces a deadline error instead of hanging or recording a false success, but that was unreachable in production: every CRUD and data-source entry point reassigned ctx to context.Background() as its first statement, discarding Terraform's real context (including any deadline) before it ever reached the poller. On top of that, no resource declared a Timeouts meta-argument, so even a fixed ctx would still fall back to the SDK's undeclared 20-minute default with no way for users to override it in HCL. Remove all 72 ctx = context.Background() reassignments (two of them a nil-guard variant) across 30 files, verified site-by-site to confirm each is a genuine Terraform-invoked entry point rather than a helper shared with a non-Terraform path. Declare Timeouts (Create/Update/Delete, 30m default, centralized as asyncResourceTimeout in async.go) on the 13 resources whose lifecycle polls an async operation, including checkpointsase_gateway, which polls indirectly through utils.go's addGatewayToRegion/deleteGatewayFromRegion. Timeouts is a meta-argument, not a schema attribute, so no resource's attribute surface changed; go generate only added a Timeouts doc section to those 13 resources. Add TestPollAsyncReturnsPromptlyOnAlreadyCancelledContext to cover the other half of ctx handling that the existing TestPollAsyncHonoursContextDeadline didn't: an already-cancelled context returns promptly wrapping context.Canceled without polling again. Both tests would have passed in isolation before this fix too — pollAsync itself always honoured ctx correctly, the callers just never let a real one reach it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ceNew
Every other invariant of this class is pinned offline -- TypeList-not-TypeSet,
Create-is-Update, id/priority Computed-only. "Nothing is ForceNew" was not, on
either policy resource, although SAP-02 and SHI-02 both assert it, and its blast
radius is the largest of the four: a ForceNew on any rule attribute turns every
edit of that attribute into destroy-then-create, and Delete here is
DELETE /v3/ia/access/policy -- "all internet traffic will be allowed after
deletion". Renaming one rule would empty the tenant's policy and re-POST it.
There was behavioural coverage in
TestAccCheckpointsaseAccessPolicy_removingTheMiddleRuleRenumbersPriority, which
would fail because the recreated rules would carry new ids -- but only under
TF_ACC, and the HTTPS-inspection suite has no equivalent.
The walk is recursive, so a ForceNew added inside sources, destinations or
conditions is caught too. Watched fail with ForceNew: true on rule.name:
rule.name is ForceNew. On a whole-policy resource that turns an edit into
destroy-then-create, and this resource's Delete issues the endpoint's DELETE
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…rten the policy
The two expanders and expandAccessPolicyConditions all `continue` past a failed
type assertion. The branch is unreachable — SDKv2 hands back
map[string]interface{} for every element of a TypeList of *schema.Resource, and
HCL cannot emit a null block — but if it ever were reached it would drop a rule
from the POST with no error, no diagnostic and no trace, which is the one outcome
the whole design exists to prevent, and in expandAccessPolicyConditions it would
silently widen a rule's hours instead.
Making it loud changes behaviour and needs a decision this provider has not taken
anywhere else: it panics in no file, so the choice is between introducing that
idiom and threading an error return through both expanders and their callers.
Recorded as LEFTOVERS.md L30, and now named at the branches so the next reader
meets the argument rather than assuming nobody looked.
Also names L30 at the flatteners' `break`, whose false rationale was removed in
0d8a81e, and corrects a test's row citation from SHI-N02 to SHI-N03 — the empty
name is N03's case.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…te API-FINDINGS instead Every measurement in this package was sourced to .superpowers/sdd/2026-08-10-v3-api-implementation/phase4-verification.md. `.superpowers/sdd/.gitignore` is `*`, and `git ls-files` on that directory returns nothing: the file is not in the repository. Anyone reading these comments on another machine — a reviewer, a maintainer, anyone who clones this — follows the citation and finds no such path. Shipped code that cites an untracked document has, in effect, no citation at all. The load-bearing measurements were moved into API-FINDINGS.md, which is tracked, as §1.23 (the one-field write surface of POST /v3/ia/status), §1.24 (`controlledBy`, returned and undefined), §1.25 (`cleanupBypassRuleDefaultAction` answering 422 rather than ignoring the field), §1.26 (the action/appliedOn matrix and why only one row of it is safe to enforce) and §1.27 (a rule's id surviving a whole-array rewrite), plus extensions to §1.15 (the HTTPS list canonicalises the same way) and §1.18 (VALIDATION_BYPASS_RULES_REQUIRED measured, not inferred from the sibling's schema). Nothing already recorded was duplicated: the citations for the HTTPS server-assigned field set and the two bucket vocabularies re-point to §1.20, and the enveloped GET re-points to §1.22. All 32 sites re-pointed, 15 in production code and 17 in tests. `grep -rn phase4-verification checkpointsase/` now returns nothing. Two citations were also narrowed while being moved, because the tracked wording is more careful than the one they carried: "keeps rule ids stable across a rewrite that left the rule in place" became "keeps a rewrite from minting new ids for rules that did not change", which is the property §1.27 actually establishes — ids follow array position, so only index 0 is guaranteed to keep its own. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The plan's Out-of-scope section required a resource-level caveat about concurrent applies. It was never written, and the mechanism the plan credited for the in-process half — `policyMutex` — was deleted in 9cb7b65. Both resource descriptions said "Only one Terraform resource, in one configuration, can manage this policy", which covers OWNERSHIP and not the race. The race is the thing that bites. Two `terraform apply` runs against the same tenant are two writers of one array: each composes the whole list from its own configuration and replaces whatever is there, so whichever POST lands second wins outright and the other run's rules are gone. Nothing reports it — no conflict, no error, no diff afterwards, because each apply read a policy that was correct when it read it and wrote one that was correct when it wrote it. Terraform's state locking does not prevent it, because the race is on the server rather than on state. The caveat now says all of that, and says to serialise. This was briefly filed as LEFTOVERS L25. That was the wrong call: a caveat the phase's own plan required is a spec requirement, not an optional improvement, and deferring it left the phase short of its own plan. L25 is closed in the same round, following L8's precedent — heading kept with a CLOSED marker, dropped from the action rows, number not reused. docs/ regenerated. The second `go generate ./...` produced byte-identical output, verified by staging the first run and confirming the second leaves an empty working-tree diff. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The unreachable-branch note in expandAccessPolicyConditions was pasted verbatim from the rules expander, where "it drops a rule from the POST" is accurate. In the conditions loop it is not: dropping a time window leaves the rule in place and WIDENS the hours it applies to, which is a different and quieter failure -- a rule meant for Monday mornings would silently apply whenever its surviving windows allow. The comment also called conditions a TypeList; it is a TypeSet, with its own recorded reasoning twelve hundred lines up. Found by the scoped re-review of the final-review fix round.
… not an empty array
Adds checkpointsase/private_dns.go, the machinery both enhanced private-DNS
resources will share: the async PUT, the expander that builds its body, and the
flattener that reads the result back.
Two silent failures live here, which is why it ships before either resource.
Both enhanced private-DNS PUTs declare ONLY a 202 (swagger.yaml:633, :699), so
the write has not happened when the call returns. putPrivateDNSAndWait polls
/v3/networks/status/{id} until the operation reports completed with a 2xx.
Completion alone is not success: checkNetworkStatus, the helper pollAsync
replaced, failed only on 500, so a 400 or 409 completion reached Terraform as a
successful apply.
The status id is the last path segment of statusUrl, resolved against the
configured client -- never the URL itself. Measured 2026-08-26 (API-FINDINGS.md
1.28): that URL is absolute, names a host the request did not go to, and carries
an /api/rest/v2.3/ path. Following it leaves the operator's configured BASE_URL
and polls whatever tenant lives at the other host, invisibly, because that
deployment answers with a well-formed {"completed":...} too.
TestPutPrivateDNSAndWaitResolvesStatusUrlAgainstTheConfiguredClient stands up
that other deployment and asserts it is never reached.
A 202 with no statusUrl now returns (true, error), NOT the (false, nil) that
putGranularFirewallPolicy inherits. That is a decision, recorded with its reason
on errPrivateDNSNoStatusUrl and tested: a write this provider cannot follow is a
write it cannot confirm, and reporting it as applied is the failure the whole
helper exists to prevent. Nine probes never saw a 202 without one.
expandCustomDnsUpdate always sends `attributes`, and always sends its arrays as
[] rather than null. Both are measured, not inferred (API-FINDINGS.md 1.31):
{"enabled": false} alone is a 422 -- and is exactly what the corresponding GET
returns for an unconfigured network, so the read body cannot be echoed back as a
write -- while omitting servers is a 400 naming servers. Servers, SearchDomains
and both DnsPolicy domain lists are declared without omitempty, so a nil slice
reaches the wire as null and fails the same @isarray. Both shapes marshal without
error, so the tests assert on json.Marshal output rather than on the struct.
samplingTime is 120 on every measured 202 while every poller here uses 10s. Left
alone deliberately and recorded on privateDNSPollInterval: retiming one endpoint
in isolation is worse than retiming none.
Nine offline tests, all httptest or marshalled bodies, none TestAcc-prefixed.
248 PASS / 53 SKIP / 0 FAIL, from 239 / 53 / 0 at b49555a.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds privateDNSSchema() and the attribute-name constants both it and the expander read, so the enhanced network and region resources consume the shared declaration instead of restating it. The hole this closes: expandCustomDnsUpdate reads attributes BY NAME, and a name it reads that the schema does not declare returns a zero value with no error, no diff and nothing in the logs. This provider has shipped that class of defect more than once -- enhanced_dynamic_tunnel sent routingType: "" against a required enum on every create; EnhancedTunnel's read blanked eight fields of user config. Declaring and reading through the same constant makes the mismatch unrepresentable rather than unlikely: one spelling per name, so a rename is one edit and a typo does not compile. TestPrivateDNSSchemaAndExpanderAgreeOnEveryAttribute covers what constants alone cannot. Its config is synthesised FROM the schema, so an attribute added later gets a sentinel automatically and fails if the expander drops it -- the guard is not a second hand-written list. Watched fail three ways: the expander reading "searchDomains" instead of the declared search_domains; an attribute added to the schema and to neither the path list nor the expander; and one added to both the schema and the path list but still unread. It also checks Descriptions. TestSchemaEveryAttributeHasADescription walks only REGISTERED resources, and nothing registers this schema until Task 2, so without this Task 2 inherits a conformance failure it did not cause on the commit that merely wires the schema up. Every limit in the schema is sourced rather than chosen, and two of them correct the plan: dns_policy's two domains lists are maxItems 100 (swagger.yaml:4601, :4626), not the 4 the plan implies, and `domains` is Required inside `public` and `private`, as are `mode` and `public_fallback` inside `private` (swagger.yaml:4595, :4609). Two rules a schema cannot express -- servers non-empty only WHEN enabled is true, and uniqueItems on four lists whose order is load-bearing -- are recorded on privateDNSSchema for the resources' CustomizeDiff rather than left to be rediscovered. The mirrored schema in private_dns_test.go is gone; it was the second definition this change exists to remove. The test now builds the resource schema the way Tasks 2 and 3 will: privateDNSSchema() plus the object's own address. 249 PASS / 53 SKIP / 0 FAIL, from 248 / 53 / 0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…, and only one of them converges
Adds checkpointsase_enhanced_network_private_dns: GET/PUT
/v3/networks/enhanced/{networkId}/privateDNS, wired onto Task 1's shared
schema, expander, flattener and async PUT rather than a second copy of them.
The defect worth naming in a subject line is the one the schema fixes.
API-FINDINGS 1.31 said "GET on an unconfigured network returns exactly
{"enabled": false}" -- true, and incomplete. Two shapes were captured in the
same probe run:
never written to -> {"enabled":false} attributes ABSENT
explicitly disabled -> {"enabled":false,"attributes":{...empty}} attributes PRESENT
So the key returns as soon as anything has been written. `enabled = false` with
no attributes block -- the configuration this resource's own description
recommends -- therefore wrote cleanly, read back with attributes present, and
stored one block against a configuration holding none: a diff on every plan for
ever and an apply that re-PUT the same body every time. Observed as
`attributes.# : "1" => "0"`. `attributes` is now Optional AND Computed, which is
sound rather than a workaround: attributes is required on write, so the server
can never report "no attributes" for a network Terraform has applied to. The
nested blocks stay Optional-only, so dropping dns_policy still clears it.
Delete is a no-op that explains itself (D9): it clears the id, issues zero
requests, and warns. The only write it could make is turning private DNS off,
which would change how a live network resolves names because somebody removed a
Terraform resource. A test asserts the whole request list is empty, not that
there was no PUT -- a GET would pass the weaker assertion.
A 404 clears the id and returns no error. That is the opposite of the Phase 3/4
lesson and deliberately so: those were collection reads where a 404 meant a
wrong URL, and treating it as drift emptied state on a misconfiguration. This is
a single object addressed by a user-supplied id, and P10 measured the body --
"Network doesn't exist." names the object, not the route. The comment says so;
the companion test asserts a 500 does NOT clear the id, so the branch cannot
degrade into "clear on any error".
Two rules no schema can express live in a shared CustomizeDiff. servers must be
non-empty only when enabled is true -- MinItems cannot see a sibling, and
MinItems: 1 would make the measured legal "off" body unwritable. And all four
uniqueItems lists are checked at diff time rather than made TypeSet, because the
write round-trips byte-exactly with non-alphabetical order preserved. Servers
compare by address, not whole element: P8's "All servers's elements must be
unique" fired for two elements whose only shared value was an absent address,
which is @ArrayUnique projecting onto one field.
listAttributeEmptyPolicy gains EIGHT entries, not the four the dispatch predicted
or the six the brief did. Every MaxItems-1 block is a TypeList too, so the sweep
demands `attributes`, `dns_policy`, `dns_policy.public` and
`dns_policy.private` as well as the four leaf lists. All eight are mayBeEmpty;
servers cannot be mustReject because that verdict forces the MinItems that would
break the off body.
Also closes two gaps the Task 1 review found in the shared file: flattenDnsPolicy
had zero coverage -- the constants make a misspelling impossible but not an
omission, and a flattener that never assigned `mode` would have written "" into a
required enum with nothing failing -- and one header comment named a test that
does not exist while another advertised a mirrored test schema that no longer
exists. private_dns_test.go's testPrivateDNSResourceSchema now returns the
registered resource's schema instead of assembling a copy.
Offline: 266 PASS / 54 SKIP / 0 FAIL, from 249/53/0. Every one of the 52
TestAcc-prefixed names skips without TF_ACC, so none is an offline test in
disguise. The acceptance test's step order is load-bearing and says so: step 1
must be the first write the network ever receives or it stops exercising the
synthesised empty arrays, and step 2 must follow it or it stops being able to see
the permanent diff.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… them
checkpointsase_enhanced_region_private_dns: GET/PUT
/v3/networks/enhanced/{networkId}/regions/{regionId}/privateDNS.
It calls privateDNSSchema, expandCustomDnsUpdate, flattenCustomDnsAttributes,
putPrivateDNSAndWait, validatePrivateDNSDiff and singleBlock rather than
restating any of them, and adds network_id, region_id and its own address. Both
are Required + ForceNew; Delete makes no request (D9) so the replace is free.
The two path parameters are positional arguments to the SDK, so transposing them
compiles, vets and answers 404 -- indistinguishable from a region that does not
exist. Two tests assert the full URL for that reason.
The composite id is <network_id>:<region_id>, not the <network_id>-<region_id>
row EPD-I01 currently writes. A hyphen cannot separate these: network ids contain
hyphens, so SplitN("net-abc-reg-def", "-", 2) yields ("net", "abc-reg-def") and
the provider would go looking for a network called "net". A mutation to "-" fails
seven rows of the parser test. The split takes the FIRST colon, so only
network_id has to be colon-free -- unlike group_membership, whose ids the API
document constrains to EnglishNumericId, this document types both as bare string
with no pattern, and no charset check is invented here on the strength of that.
THE REGION ENDPOINT HAS NEVER BEEN PROBED. Every capture behind API-FINDINGS.md
1.31 -- the two disabled read shapes, the 422 for a body with no attributes, the
400 for a null array, the byte-exact round trip -- was taken against the NETWORK
path. The offline fixtures reuse those bodies because the spec declares the same
request model, the same response model and the same 202-only response map, which
is a reason to expect the same behaviour and not evidence of it. Every one is
labelled spec-derived; the acceptance test is the first thing that will ever
touch the path, and its header says a failure there is information first.
Two shared constants had to be reworded for the same reason: rendered by this
resource, privateDNSWriteRefused's "the two rejections measured on this endpoint"
asserted measurements that do not exist, and privateDNSNoOpDeleteNote's "leaves
the network exactly as it is" named the wrong object on this resource's registry
page. Both now say what there is evidence for.
Eight listAttributeEmptyPolicy entries, not four: the four leaf lists plus the
four MaxItems-1 wrapper blocks, which are TypeList too. All mayBeEmpty, each
sourced to a swagger line verified for this commit. For `attributes` the verdict
is about the HCL only -- on the wire an absent attributes object is a 422, and
mayBeEmpty is legal solely because the expander synthesises the two empty arrays.
The schema test walks FULL PATHS rather than top-level keys. The first version
looped over privateDNSSchema()'s two keys and a mutation deleting
attributes.search_domains passed it green; the sibling's copy still has that hole.
Offline: 285 PASS / 55 SKIP / 0 FAIL, from 266/54/0. The one new TestAcc name is
a real acceptance test; all 53 in the package skip without TF_ACC and call
resource.Test.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…s that were not true
The review found no Criticals and no behaviour defects. What it found was text
asserting things that are false, and constants asserted by nothing. Both are
fixed here, each with a mutation watched failing first.
THE COVERAGE GAP (review Minor 6). MaxItems 4 -> 100 on servers AND
search_domains, 100 -> 4 on both domains lists, and the mode enum flipped to
case-insensitive, all in one edit, left the suite at 285 PASS / 55 SKIP / 0 FAIL.
Reproduced before fixing. The limits DIFFER per attribute -- 4 for servers and
search_domains, 100 for both domains lists -- so a uniform table would be wrong
whichever number it picked.
MaxItems is enforced by schemaMap.Validate, NOT by Resource.Diff, which is why
every plan-time test in this package was blind to it. The new coverage drives the
validation path: a closed table of all eight MaxItems, and boundary rows at max
and max+1 for each of the four lists so neither direction can pass by accident.
mode stays case-sensitive (StringInSlice ignoreCase=false) -- the Phase 3
access/"Read" lesson -- pinned by four rejected spellings.
THE SHALLOW SCHEMA TEST. It looped over privateDNSSchema's two top-level keys, so
deleting attributes.search_domains passed it green. Now walks full paths in both
directions through the package's one walkSchema, which is what the region
resource's equivalent already did.
I2 the importer comment called its one load-bearing d.Set "a no-op on every
path today", contradicting the doc comment fifteen lines above. Deleting the line
gives GET /v3/networks/enhanced//privateDNS. Verified by deleting it.
I3 four texts said the provider sends the empty arrays "on every write". The
Computed fix made that false after the first apply. Removing Computed shows the
body reverting to {"servers":[],"searchDomains":[]} -- the docs were describing
pre-fix behaviour. Corrected, docs regenerated, and pinned by a test whose two
rows cover both halves of the corrected wording.
I4 the plan-time duplicate error claimed the API refuses duplicate ADDRESSES as
measured. P8's two rejected servers were content-identical after whitelist
stripping, so that capture cannot separate "unique by address" from the spec's
whole-object uniqueItems. Behaviour kept, claim reworded to what is known.
I1 is CLOSED, not open: the server returns publicFallback explicitly
(API-FINDINGS.md 1.34). Several comments still called dns_policy unmeasured;
corrected to cite 1.34, with the region path still correctly marked unprobed.
Minors 5, 7, 8, 9, 10: Timeouts on both resources (Create/Update only -- Delete
makes no request); plan LIMITS citations 4393/4401 -> 4489/4496; a note that a
completed:true with no result is an unmeasured assumption; a nil guard on Read,
which panicked on a 200 with a null body (SIGSEGV watched at the dereference);
and the two thin conformance why strings brought up to the region's standard.
290 PASS / 55 SKIP / 0 FAIL. All 53 TestAcc names skip without TF_ACC.
go generate twice byte-identical across 48 doc files. SDK untouched at af8c6fd.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…obe that decided nothing
The two standard private-DNS data sources, read-only because the API is: the
standard family declares `get` and nothing else on both paths. No expander, no
async poll, no Delete, and every attribute Computed.
The brief said P3's captured body would decide whether the standard
`dnsPolicy.private` carries `mode`, `publicFallback` and `domains`. P3's body is
`{"enabled":false}` -- no `attributes` key, therefore no `dnsPolicy` key. It is
silent. The decision is made instead on the spec, where `DnsPolicyResponse` is
`allOf[DnsPolicy, {private: {forwardDNSUpdate}}]` and allOf means merge, and on
the asymmetry of the two errors: reading a key the server never sends yields the
zero value that omitting it would, while omitting a key the server does send is
the 2.1 failure and is invisible. So the three are read out of
AdditionalProperties, and every comment that touches them says spec-derived
rather than measured. The SDK gap is LEFTOVERS L35, in the A19 class.
The 404 is an error here and drift in the enhanced resources, and both are
right: a resource can clear its id and plan a recreation, a data source has
nothing to clear and would hand every downstream reference a zero value instead.
flattenCustomDnsServers moves into private_dns.go so both families call it
rather than spelling `address` and `is_tls` twice; the enclosing flattener
cannot be shared because Go will not take both structs.
P10's 404 body is NOT the same text on the standard path as on the enhanced one
-- "network doesnt exists" against "Network doesn't exist." -- so this fixture
declares its own constant instead of importing the neighbour's.
290 -> 300 PASS, 55 -> 56 SKIP, 0 FAIL. One new TestAcc name, and it is one.
…d has a policy 1.36 read both standard paths from a network configured out of band, because 1.35 established v3 has no write method on this family at all. The three fields the generated struct drops really do arrive in AdditionalProperties. What was argued from allOf is now measured, and the two new fixtures are verbatim from the captures. The labels move only as far as the captures reach, which is not far. Between them the two bodies cover one server with isTLS false, empty searchDomains, empty public.domains, both mode values, both publicFallback values, and a private domains list of one element and of zero. Server order, is_tls true, non-alphabetical search domains and a populated public.domains are still spec-derived, so specStandardPrivateDNSConfigured stays -- it is the only fixture that exercises the ordering contract TypeList rests on. forwardDNSUpdate was not returned by either endpoint. The one field the generator kept is the one field nothing has measured, so forward_dns_update reads false on every body anyone has seen -- the absence of a key, not a value the server stated. L29 means a nested attribute's Description never reaches the rendered page, so that caveat rides on both data sources' top-level Description, which is the only text tfplugindocs renders. The third disabled shape is the surprise: a region nobody configured comes back enabled false with attributes PRESENT and a fully populated dnsPolicy whose defaults do not match its own network's. enabled predicts neither attributes nor dns_policy in either direction now, and nothing may shortcut on it. The consequence is on the region data source's own page, and the acceptance shape helper carries a do-not-tighten-this warning, because "disabled means no attributes" is exactly the tidy-up the tenant contradicts. Two comments corrected on the way through, both false as written. measuredPrivateDNSNetworkGone claimed its 404 BODY was verified on all three Phase 5 paths; only the status was, and P10 captured three different message strings. private_dns.go claimed a mis-spelled attribute name is silent; that is the expander direction only -- d.Set on an undeclared nested key fails loudly, which mutation M14 measured. 300 -> 302 PASS, 56 SKIP, 0 FAIL. No new TestAcc name; still 54, all genuine.
checkpointsase_split_tunneling: GET /v3/networks/{id}/split-tunneling and
PUT .../split-tunneling/async, polled to completion. network_id takes EITHER
family -- P1 measured byte-identical bodies and accepted writes on an enhanced
and a standard network, so this is the one Phase 5 resource that is not
family-specific.
except_data.exceptions ships Computed-only (D5). The v3 write MERGES that field,
so nothing the provider could send would ever remove a saved exception: [] merges,
omitting is unchanged, shrinking the covering cidr leaves it -- contradicting the
schema's own promised cleanup -- and via_tunnel HIDES rather than deletes, so a
resource that stored the empty list a via_tunnel network returns would resurrect
bypass rules on the switch back. The cost is stated in the resource description
and not only in a comment: an exception cannot be created here either, and stays
a console operation until L33 is resolved.
Two divergences from the shipped private-DNS siblings, both deliberate:
- Create sets the id AFTER the write, not before. SPT-N02 and SPT-N03 both
require that a failed or timed-out apply record no id, and unlike private DNS
this resource is fully recoverable without state -- its id IS the required
network_id argument and every write is a full replacement.
- No CustomizeDiff. The server refuses via_tunnel + exceptions with a 400 that
names the reason (API-FINDINGS.md 1.30), and Computed-only exceptions make
the rejected body unreachable from HCL anyway.
FOUR listAttributeEmptyPolicy entries, not the five the plan predicted and not
the three the brief did: except_data is a TypeList wrapper too, and exceptions
needs none because the sweep skips Computed && !Optional. Measured by deleting
them and reading which paths the test named.
Also corrected, from this task's own reading: API-FINDINGS.md 1.31 gains the
addressObjectIds length discrepancy (swagger says 11 characters, the live 400
says 10, and the capture cannot settle which -- so no length validator ships),
and the plan's two claims that all three Phase 5 paths return the same 404 body
are corrected to the three different messages P10 actually captured.
26 offline tests, one acceptance test that builds its own standard network and
assumes nothing about the tenant -- the leftover probe network still holds the
exception that cannot be deleted. 302 -> 328 PASS / 57 SKIP / 0 FAIL. 26 single
mutations watched fail as tests, plus one deliberate control that survived and
corrected a comment claiming singleBlock was load-bearing here. SDK untouched at
af8c6fd.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… one family
C2 is answered. The coordinator probed the endpoint rather than leaving it for
the first live run: three cidr entries sent deliberately non-ascending
("10.80.0.0/16", "10.10.0.0/16", "10.50.0.0/16") came back in exactly that order,
so a sorting server would have moved 10.80 to the end and did not. TypeList was
the right call, and an index-based assertion is now a regression check rather
than a probe.
The four places that said the ordering was unmeasured now cite API-FINDINGS.md
1.31 -- and each of them says how far one probe reaches, which is the part worth
the words. It measured `cidr`, on ONE network, of the ENHANCED family.
addressObjectIds and updatableObjectIds were sent EMPTY in that request, so
nothing was observed about their ordering, and the standard family was not
exercised at all. TypeList stays right for all three under either outcome -- it
is the conservative choice, and list-to-set is a state migration later -- but
only the first of them has evidence behind it.
That distinction matters most at the acceptance test's step 5, which builds a
STANDARD network: its index assertions are a regression check for cidr's measured
behaviour and still the open question for the family the probe did not cover. If
that step ever fails, the answer is a finding, not a code change.
No behaviour change. Comments only, plus the blank lines gofmt wants between list
items once one of them carries a second paragraph. 328 PASS / 57 SKIP / 0 FAIL
unchanged, and `go generate` output byte-identical -- nothing here touches a
schema Description.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The standard private-DNS shape check failed an acceptance step when `enabled`
was true and `attributes.#` was not 1, and again when `servers.#` was 0, citing
swagger.yaml:4492. That line is CustomDnsUpdateAttributes.servers -- the PUT
body. The read decodes CustomDnsResponse (:4419) -> CustomDnsAttributesResponse
(:4409) -> CustomDnsAttributes (:4387), where `servers` carries uniqueItems and
maxItems: 4 and no minimum of any kind, and CustomDns (:4376) requires `enabled`
alone. So {"enabled":true} with no `attributes` is a legal response, and this is
the one family whose only writer is the console (1.35: the standard paths are
GET-only in v3), so the provider cannot know what wrote the body it is reading.
Both rows were offline-green and could only ever have fired live, against a
legal body, as "the block was dropped on the way into state".
Re-citing them would not have helped; they are removed. What replaces them is a
constraint the READ model really declares: CustomDnsServer (:4429-4442) requires
both address and isTLS, so every server element must carry them -- vacuous for
an empty list, and the live twin of M10.
The finding under the finding is the asymmetry: the check refused to infer shape
from `enabled = false` on measured grounds (1.36's third disabled shape), then
inferred shape from `enabled = true` on write-model grounds. The doc comment now
warns against tightening in either direction, and
TestStandardPrivateDNSShapeCheckMatchesTheReadModel pins both -- 5 of its 13 rows
failed against the unfixed check, and four mutations of the check confirm the
retained rows still fail for the reasons they claim.
Also three labels that claimed less measurement than the captures support:
- SPD-02 no longer calls itself "the first request anything has ever made to
this path" 60 lines below a comment saying the path has been read exactly
once. It is the POSITIVE row; "the first negative request" belongs to SPD-N02
and is already there. What is new here is the read going through the provider.
- TestFlattenDnsPolicyResponsePrivateToleratesWrongTypes no longer opens
"Nothing in this project has seen a standard private-DNS body at all" above
two fixtures in its own file that are exactly that. Three have been captured
and none is malformed, which is the honest reason the readers stay total.
- The spec-decode test's topic sentence agreed with neither 1.36 nor its own
bullet four lines down.
And four minors: the shared servers flattener's ordering claim is 1.31's
ENHANCED write measurement and is now scoped as such at the point of sharing;
the region example uses the regions catalogue rather than a fake "us-east-1",
which is what every other example does and what makes the file applyable; and
the id row that cannot fail says so, measured by mutating the id to a timestamp
and watching rows 1 and 4 fire while it stayed silent.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…t all three Task 6's review found no behavioural defect and three claims that were not true. One of them, "omitting ANY ONE of the three exceptData arrays returns a 400 naming all three", had reached an operator-facing diagnostic. It has now been probed directly (API-FINDINGS.md 1.37): omitting only updatableObjectIds returns three errors, all about updatableObjectIds. The validator names exactly the arrays you omitted, which is narrower AND more useful -- the error tells you which field to add. Corrected in all five places, starting with splitTunnelingWriteRefused. measuredSplitTunnelingArraysMissing was a hand-trimmed three-error body sitting under a header that promises "None was authored to fit the code". Restored to the whole eleven-error capture, byte-identical to p4-via-tunnel.json. Nothing depended on the trimmed version -- its one caller asserts accepted == false and zero polls -- and the full body is the only place two things are visible: one missing array produces five complaints, two of which are the contradictory length rules that are the reason no id-length validator ships. A null async status body panicked the poll closure. status.Result is a direct field access and json sets the pointer to nil for `null`; reproduced, guarded, and pinned by a test that fails with the panic recovered into a message. The same shape survives in async.go:229, :269 and private_dns.go:479 -- shared machinery, left for one pass, tracked as L37. TestSplitTunnelingExceptionsAreComputedOnly's "fourth, behavioural" assertion could not fail against the defect its comment named, and the review's suggested remedy could not either: with exceptions Optional and its leaves Computed-only, SDKv2 still refuses the config, naming except_data.0.exceptions.0.type -- which contains "exceptions". Measured both by mutation. The assertion now requires the LIST to be named. Also: samplingTime cited L13, which is about 403s during polling and says nothing about cadence (now L38); utils.go:334 is the SDK's, not this package's; eight 202s carried a statusUrl, not six, and six were on one network; and "full replacement" is measured for cidr only. The region private-DNS files stop claiming their endpoint has never been read. API-FINDINGS.md 1.37 read it: an untouched region returns `attributes` present with a populated dnsPolicy, byte-for-byte what the STANDARD region returns -- so the third disabled read shape is a property of REGIONS, not of the standard family, and the enhanced region resource needs the same allowance. Its 404 is a fourth distinct spelling, "Region with ID <id> not found.". Both are now fixtures captured from that route, and the convergence test gained the row its own comment said belonged there once the shape was measured. 330 PASS / 57 SKIP / 0 FAIL, TF_ACC unset. No acceptance test was run. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… it names
Four corrections, each traceable to a measurement or to the SDK's own source.
D1. docs/data-sources/standard_networks.md said the data source uses
/v2.3/networks/standard. StandardGetNetworks builds /v3/networks/standard
(api_standard_networks.go:172). Fixed at the Go Description and the doc
comment above it, then regenerated -- never hand-edited. go generate ./...
twice, 51 files, shasum-identical over both runs.
D3. API-FINDINGS 1.2 was undocumented on the resource that has it. A static
tunnel created with remote_gateway_subnets = ["0.0.0.0/0"] applies cleanly
and can then NEVER be updated: every later PUT returns
404 "Remote gateway subnets not found", including one that touches neither
subnet list. Measured 2026-08-17 on three tunnels differing only in that
field. The 0.0.0.0/0 text already on that page belongs to
p81_gateway_subnets, where 0.0.0.0/0 is legal and recommended -- the two
attributes are adjacent and have OPPOSITE correct answers, so a reader
skimming for the string was actively misled. Documented on the attribute,
the resource description and the example. NOT validated at plan time: the
API accepts the value at create, and refusing a create the server allows is
the worse error.
D4. ipsec_redundant's Description claimed "every attribute change forces full
replacement". IT DOES NOT, and the truth is worse. Two reasons:
- last_updated is Optional+Computed with no ForceNew.
- helper/schema's diffList copies a list's ForceNew only for
Elem: *Schema and for the synthetic <key>.# count. For Elem: *Resource
it diffs each nested field against that field's OWN schema, and none of
the nested schemas here sets ForceNew. All four blocks are
Elem: *Resource.
Measured against terraform-plugin-sdk/v2 v2.26.1 on 2026-08-26: changing
tunnel1.0.passphrase yields RequiresNew=false on both the attribute diff
and the plan. So editing a passphrase or an IKE lifetime plans an UPDATE,
reaches UpdateContext, and fails the apply. That error was NOT dead code,
and its diagnostic summary said "Unable to delete ipsec-redundant tunnel" --
a copy-paste from Delete telling an operator the provider could not destroy
their tunnel. Corrected to "Unable to update". Behaviour is unchanged: the
SDK's PUT for this tunnel type exists and stays unwired, tracked in
LEFTOVERS L6.
D2 (half). data_source_standard_private_dns.go borrowed the expander's argument
for a file that has no expander: "a key spelled differently in a FLATTENER
surfaces a zero value with no error". d.Set REJECTS an undeclared nested key
and appendErrorDiags surfaces it -- measured by mutating
flattenDnsPolicyResponsePrivate to emit "dns_mode". private_dns.go already
said so; this file did not.
Also: four comments counted THREE 404 spellings where API-FINDINGS 1.37 now
tabulates FOUR, and three headers said the async PUT "declares only a 202" when
the response maps also list 400/401/403/404/409/422/429 -- the true claim is
that no SUCCESS response but 202 is declared.
330 PASS / 57 SKIP / 0 FAIL, unchanged. 55 TestAcc names, diffed against the
baseline and identical. No acceptance test was run. SDK untouched at af8c6fd.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…subnet
The live run failed both private-DNS acceptance tests at step 3 of 8 with
400 {"message":"Invalid IP address","messageCode":"BAD_REQUEST","status":400}
on an address that is perfectly well-formed. API-FINDINGS.md 1.38 measured what
the server means by "invalid": the address was inside the network's OWN subnet.
Same network, three writes -- 10.254.0.53 and 10.254.9.9 refused on a
10.254.0.0/16 network, 10.96.0.53 accepted on it.
Both tests had chosen server addresses to MATCH the subnet they built, which
reads as tidy and is exactly the forbidden case: 10.96.0.53 / 10.96.1.53 on
10.96.0.0/22, and 10.97.0.53 / 10.97.1.53 on 10.97.0.0/22. Only the four server
constants move; the subnets stay, because they were chosen to avoid every other
acceptance test's subnet and the two probe networks on the tenant.
network test 10.96.0.0/22 -> servers 10.200.0.53, 10.200.1.53
region test 10.97.0.0/22 -> servers 10.201.0.53, 10.201.1.53
Each of the four is RFC1918 private, outside its own test's subnet, outside the
other test's subnet and servers, and outside 10.254.0.0/16 and 10.255.0.0/16.
The pair within each test stays distinct because the step asserts two servers
with DIFFERENT is_tls values and the assertions are positional. Every reference
is through the constants -- no literal address appears in an assertion, a config
builder or a CheckDestroy helper -- so the two constant lines per file are the
whole behavioural change. The non-alphabetical search domains (b before a,
API-FINDINGS.md 1.31) are untouched.
The rest of the diff is a comment at each const block, because the next reader
will otherwise tidy these back to match the subnet. It says the address must NOT
be inside the network's own subnet, cites 1.38, and points out that the server
calls this "Invalid IP address" -- which sends you to check your typing when the
problem is your addressing.
No provider source changed: the provider surfaced the server's error unchanged
and did not corrupt state, so there is nothing here to fix on our side. SDK
untouched at af8c6fd.
330 PASS / 57 SKIP / 0 FAIL, TF_ACC unset, identical to the 40c6d29 baseline.
No acceptance test was run.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… ordering assertions Final whole-branch review returned APPROVE WITH FIXES from two reviewers. This is the provider half of the fix round. F-2, the only crash in either report. private_dns.go's poll closure reached status.Result -- a direct field access -- without checking the pointer. A 200 carrying the literal `null` decodes cleanly and leaves it nil, so the provider panicked mid-apply with no diagnostic, no statusId and no statement of whether the write landed. The three-line guard already existed in putSplitTunnelingAndWait from 8b554df; it is ported rather than reinvented, so the two shapes match. Reproduced as a panic first, then fixed: TestPutPrivateDNSAndWaitSurvivesANullStatusBody. F-1/C2, examples that cannot be applied. Both enhanced private-DNS examples put their DNS servers inside the network's own subnet -- 10.120.0.53 in 10.120.0.0/22, 10.121.x in 10.121.0.0/22 -- which API-FINDINGS.md 1.38 measured as 400 "Invalid IP address". validate and plan pass; the apply fails AFTER the network is created, leaving a partial apply. 4c8e016 fixed the tests and missed these because it grepped for the four old literals. Moved to 10.200.x/10.201.x and swept BY SHAPE instead: every `subnet = "<cidr>"` against every `address = "<ip>"` in the same file, across examples/, docs/, demo/ and checkpointsase/. Zero violations remain. F-3, four ordering assertions that could not fail, each mutation-proven blind first and mutation-proven live after. `servers` read side (sorting flattenCustomDnsServers left all 330 tests green), `servers` write side (sorting what expandCustomDnsUpdate sends: green), split-tunneling `cidr` read side (one element, so sort AND reverse both passed), standard `private.domains` (one element). Measured fixtures are untouched -- p8b really is byte-exact and its servers really are ascending -- so the discriminating fixtures are new, authored and labelled as such. The acceptance pairs are now descending, which is what makes D9's own instance able to catch a sorting server rather than only a deduplicating one. F-8, operator-facing text wrong twice. privateDNSWriteRefused blamed "an array that arrived as `null`" when the measured 400 came from an OMITTED key -- no request capture in the phase contains a JSON null, and HCL cannot express one -- and told the operator the field named first "is not necessarily the field you got wrong" when all three errors name the field that was actually missing. Both errors pointed away from the cause. Mirrors splitTunnelingWriteRefused, fixed in 8b554df. F-6, "there is no uniqueness rule" is contradicted by a capture this file already quotes: @ArrayUnique() is declared on all three exceptData arrays. Duplicates now fail at plan instead of one async write later, sharing the private-DNS helper. The evidence limit is stated at the check: the captured errors fired on ABSENT arrays, so they prove the decorator exists and rejection of a populated duplicate follows from class-validator rather than from the wire. F-4/I6, 1.38's constraint now reaches operators on both writable pages via privateDNSAttrAddress. It cannot reach the two data-source pages the same way: tfplugindocs drops nested descriptions under `List of Object`, which is L29, and that is recorded at the schema so nobody thinks the job is done. F-5, the fixture-provenance headers promised more than they could keep. No async status body has ever been captured -- `grep -l completed` over all 115 probes returns nothing, including the bare {"completed":false} that 1.28 quotes as a wire body -- so all 24 of them are renamed to a synthetic* prefix that survives a reader who skips the header. F-9, the region separator argument rested on `net-` plus a UUID. Every id in all 115 captures is 10 alphanumerics with no hyphen, so "demonstrably" was backing the one false clause in the paragraph. The decision is unchanged and now rests on the observed shape, and three shipped import.sh files stop printing an id shape that does not exist. Minors: m1 (spec-vs-server is the contradiction, not the two length rules), m2 (nine writes, not eight), m3 (p16 measured the standard family), m4 (one citation for utils.go:1283), m7 (the id check builds `want` from a literal colon so a separator change can fail it), m9 (this test drives Diff, so it pins over-refusal and not MinItems), m11 (ipsecRedundant01 is 16 characters against a 15 limit and fails validate), m12 (10.92 is not taken), m13 (thirty-one call sites, not twelve), m14 (no 200 in the response map, not "only a 202"), m16 (three stale "no probe has ever" negatives that p11, p14 and the live run refuted). Declined, with reasons in the fix report: m5 and m8 (placement and harness refactors, no claim is wrong), m6 and m10 (coverage additions rather than corrections), m15 (a ~40-line date sweep, recorded once in the findings instead). Offline suite 335 PASS / 57 SKIP / 0 FAIL, up from 330/57/0 by exactly the five new tests. TestAcc list diffed against the baseline: identical. go generate run twice, byte-identical by checksum. No acceptance test was run. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…d say so where it is read I3's smaller half, from the docs-side final review. `resource_wireguard`'s descriptions told operators to "pair with `vault`" and "retrieve the WireGuard client config from the SASE management console". Neither is measured, and the retrieval path that IS measured uses neither: API-FINDINGS.md 1.32 got a URL from GET .../config-token whose last path segment is this very token, which needs no authentication, lives 6 hours, and serves an executable carrying the tunnel's key material. Unverified guidance sitting beside a measured alternative. The consequence is the part worth surfacing. `request_config_token` is Computed + Sensitive and set on every Read; `network_id` is in state and the resource id IS the tunnel id. So all three variable parts of that URL are already in state, and anyone who can read the state file can rebuild a working unauthenticated link. `Sensitive: true` does not protect a state file. The three descriptions now say that plainly rather than pointing at a console flow nobody measured. This is also why LEFTOVERS.md L36's deferral rationale had to be rewritten -- the deferral stands, but not on the ground that it keeps this credential out of state. `vault`'s description now says its role is undocumented and unmeasured instead of asserting a workflow. Also M7: the split-tunneling example's `address_object_ids` line is the one argument across all five examples with no measurement behind it -- the array was empty in every capture, and spec and server disagree about the id length by one. Said so at the line, including that applying the example once would settle it. 335 PASS / 57 SKIP / 0 FAIL. go generate run twice, byte-identical by checksum. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
m16's closing paragraph, verified individually. model_custom_dns_update_attributes.go:23 is the doc comment above `Servers`, not a declaration. The claim built on it -- that Servers and SearchDomains carry no omitempty -- is true, and the declarations are at :24 and :26. Cited in two files. resource_split_tunneling_test.go cited resource_split_tunneling.go:694 as the `status == nil` guard it pins by mutation; :694 is the `statusId == ""` guard a few lines earlier. Named rather than renumbered, because the guard moved again in this very fix round -- which is the argument for naming constructs over counting lines. The same reasoning de-numbers private_dns.go's new cross-reference to its sibling. Both were wrong pointers behind correct claims, which is the benign form. Numbers in this branch drift; names do not. 335 PASS / 57 SKIP / 0 FAIL. go generate byte-identical across two runs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
chkp-olegkre
approved these changes
Aug 26, 2026
…on absence
Two deferred gap fixes from the design spec's section 8.3, both with the test
watched failing first.
L6 -- created_at and updated_at were Optional+Computed on wireguard, openvpn and
ipsec_single. Both are only ever set from the server, so a value written into HCL
was accepted at plan time and silently ignored. Now Computed-only, six attributes
across three resources.
The gate for it already existed and was the more important half. It read
t.Skipf("KNOWN (deferred to Phase 6)"), so it reported the violation and passed
green -- a test that skips when it finds what it looks for is not a guard. It is
now t.Errorf, and a reintroduced Optional on one attribute makes it fail naming
that attribute.
OA-N02 -- object_addresses Delete reported every error including a 404, so
destroying an object somebody had already removed failed the apply and stranded
the resource in state, needing a manual state rm. It now swallows a 404 only,
matching resourceUserDelete and resourceGroupDelete. The 500 row of the new test
is the half that matters as much: swallowing a 404 must not become swallowing
everything.
Evidence: removing the guard fails the 404 row on both assertions (error raised,
id not cleared) and leaves the 200 and 500 rows passing.
Verified already done and not touched: F5's Sensitive markers on wireguard.vault,
openvpn.access_key_id and secret_access_key are all present; L1/APP-D03's groups
population; L3's pollAsync transient-error budget.
337 PASS / 56 SKIP / 0 FAIL, from 335/57/0 -- the SKIP becoming a PASS is the
timestamp gate. Docs regenerated; the six attributes move from Optional to
Read-Only. go generate twice byte-identical.
SI-D02. resourceGatewayRead treated a failed parent-network lookup as fatal, so deleting the network out of band left every gateway under it unreadable: plan could not even report that the gateway was gone, and recovering meant a manual terraform state rm per gateway. A 404 on the parent is now drift -- clear the id, return no error. Why this is safe here and is NOT safe on a collection endpoint, stated in the code because the distinction has cost this project three defects: the request addresses ONE network by id, so a 404 means that network is absent. On a collection a 404 means the URL was wrong, and treating it as drift cleared live ids on a misconfiguration. The 500 row of the test is the necessary other half. A server error means the network may well still exist, and clearing the id there would tell Terraform to recreate a gateway that is already running. Evidence: removing the guard fails the 404 row on both assertions -- error raised, id not cleared -- and leaves the 500 row passing. 338 PASS / 56 SKIP / 0 FAIL. No doc change: the fix is in Read, not the schema.
…on nobody read
ERT-I01. resourceEnhancedRouteTableImportState called Read directly, but Read
builds its request from d.Get("network_id") and d.Id(), and on import only d.Id()
is set. So an import issued GET /v3/networks/enhanced//route-table/<id> -- an
EMPTY network segment, a different route rather than a 404 on this one, and an
error naming nothing an operator could act on. The id is now composite,
<network_id>:<route_id>, matching group_membership and enhanced_region_private_dns;
both chose a colon over a hyphen because a hyphen is ambiguous the moment either
half contains one.
This matters more here than on a resource that can be created. API-FINDINGS.md
1.1 measured that the create endpoint cannot succeed for any tunnel that exists,
so the resource refuses every configuration at plan time and import is the ONLY
way an entry reaches state. Evidence: reverting the importer reproduces the empty
segment verbatim in the request path.
No character class is imposed on either half. The API types both ids as bare
strings, so any charset rejected here would be this file's invention -- the same
restraint resource_enhanced_region_private_dns.go records for the same reason.
ETD-04. Update sends description; Read never read it back, so a description
edited in the console was invisible and the local value diverged silently. Now
read under a HasDescription() guard rather than setIfPresent, because that helper
refuses to overwrite state with an empty value -- right for write-once
credentials, wrong here, since clearing a description is a real edit that should
show as drift.
Two attributes stay unread, each for its own reason, both stated in the code:
peak_bandwidth is Deprecated and never sent to v3 at all, so a server value would
overwrite a local one this resource does not manage; and left_asn has no field on
EnhancedTunnel to read, which is why Update already warns it cannot be changed.
Both new tests derive from the CAPTURED live fixture rather than hand-written
bodies. The first drafts of both did hand-write, and both failed on a required
property -- propagated, then authType -- with an error naming a field unrelated
to what was being tested. That is the trap API-FINDINGS.md 1.34 records, hit
twice in one sitting.
340 PASS / 56 SKIP / 0 FAIL. 55 TestAcc names unchanged. No doc change: both
fixes are in behaviour, not schema.
…t be managed The resource creates two real tunnels on two real gateways and then fails on the read that follows, leaving infrastructure the provider can neither manage nor destroy. Every attempt leaked a pair; one such leak was cleared off the test tenant by hand this morning. Root cause, measured on a live pair (API-FINDINGS.md 1.39): read, update and delete are all addressed by an haTunnelId -- the HA PAIR's id -- and the API returns that value from no endpoint. Not the network read, not the network list, not the gateway or region reads, not the single-tunnel read, and there is no collection route to list it from. The pair endpoint was tested with BOTH member ids and answered 404 to each. utils.go:1004 asserted the opposite as fact. Refused in CustomizeDiff, matching resourceEnhancedRouteTableCustomizeDiff which handles the same class of problem for the same reason (1.1). One difference is recorded in the code because it changes the stakes: there, create FAILS, so refusing early only improves the message. Here create WORKS, so refusing early prevents orphaned infrastructure. The resource stays REGISTERED. Unregistering it would fail every existing config and every v2.3 upgrade with "resource type not supported", which explains nothing and offers no path; a refusal can name the reason, the finding and the alternative. TestAccIpsecRedundant_basic is skipped with its reason rather than deleted -- its config is known-good and did create a real pair, and rebuilding that fixture costs two gateways and ~26 minutes. A permanently red test trains people to ignore a red suite, and this project has already lost a phase's acceptance coverage to evidence that looked present and measured something else. Evidence: removing the CustomizeDiff registration fails the new gate, naming what it re-enables. 341 PASS / 56 SKIP / 0 FAIL.
…work families
The server derives each tunnel's interfaceName from tunnel_name and refuses
anything outside [a-zA-Z0-9] with a 422 that names only the derived field --
a field that appears nowhere in a Terraform configuration. Until now the only
plan-time check was StringLenBetween(0, 15), which got the character class and
the bottom of the range wrong: the server's minLength is 3, so an empty name
was reaching POST.
All six tunnel resources now share tunnelNamePattern, ^[a-zA-Z0-9]{3,15}$.
The four standard-network resources take it from the API's own TunnelName
schema (swagger.yaml:7517), reached from BaseTunnelValues,
CreateIPSecRedundantPayload and IPSecRedundantTunnels.
The two enhanced resources are here ON EVIDENCE, NOT SYMMETRY. Their payload
schemas declare tunnelName as a bare string with no pattern and no length
(swagger.yaml:4651, :5059), so they were deliberately excluded at first --
refusing a name the server would have accepted is worse than the 422 it
replaces, because the 422 comes with a workaround and a plan-time refusal does
not. An apply of demo/enhanced_dynamic_tunnel, which touches enhanced endpoints
and nothing else, then returned the identical 422 at the identical Joi path
(regions[0].instances[0].attributes.tunnels[0]). instances appears in the public
spec only on NetworkRegion, the STANDARD region model, so the two families share
one internal network document and the spec's silence on the enhanced side is a
gap rather than a looser server.
What is NOT measured on the enhanced side is the bounds. 15 was already enforced
there and is unchanged; the floor of 3 is carried over from the standard family
and has never been tested against an enhanced endpoint. The code comment says so.
Tests drive the REGISTERED resources rather than the pattern in isolation -- a
pattern nothing is wired to would pass either way. TestEveryTunnelNameIsGuarded
walks the whole ResourcesMap so a seventh tunnel resource added without the
check fails here rather than during someone's live apply.
Both tests were watched failing against the old schema before the fix landed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
chkp-olegkre
self-requested a review
August 28, 2026 12:24
chkp-olegkre
approved these changes
Aug 28, 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
2 participants
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.
No description provided.