P81 142146 update perimeter 81 sdk to use v 3 openapi - #2
Open
chkp-valentynk wants to merge 27 commits into
Open
P81 142146 update perimeter 81 sdk to use v 3 openapi#2chkp-valentynk wants to merge 27 commits into
chkp-valentynk wants to merge 27 commits into
Conversation
v3 ships an empty securitySchemes and declares Authorization as a header parameter on all 114 operations. Left as-is, openapi-generator emits a required .Authorization(string) argument on every method and the SDK's central bearer injection becomes unreachable. Introduces api/v3.upstream.yaml (pristine), api/overlay.yaml (reviewable corrections with removal conditions), and scripts/build_spec.py which composes them into api/swagger.yaml.
securitySchemes is entirely absent from v3.upstream.yaml, not present-but- empty as originally assumed. Replace test_upstream_is_untouched (which asserted the false premise and could never pass) with test_upstream_lacks_the_security_scheme_the_overlay_adds, which asserts absence of both components.securitySchemes and top-level security. Also correct overlay entry A1a's reason text to match. No change to build_spec.py or the overlay's behavior: op_set already creates the securitySchemes key regardless of whether it pre-exists.
The six /v3/gum/custom-roles operations declare neither tags nor operationIds, so they would generate into DefaultAPIService with path-derived, version-unstable method names. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Each of the eight v2.3-era hand-patches is re-tested against the v3 spec. Overlay entries are written only for defects still present; already-fixed ones are recorded in api/AUDIT-2026-08-10.md so they are not carried forward as dead corrections.
The audit script checked only each schema's top-level properties/required. v3 composes heavily with allOf, so that missed peakBandwidth (declared inside allOf branches of EnhancedTunnelBase/StaticTunnelCreate) and the nested required fields on IPSecRedundantTunnel/NetworkTunnelIpsecRedundant, producing three false ALREADY-FIXED verdicts (A8 and two A6 sub-candidates). declares_property()/required_fields() now walk the full allOf/oneOf/anyOf composition tree. Re-auditing with them flips those three to CONFIRMED (8/10 total, up from 5/10) and adds overlay entries for each, using targeted transform functions rather than a plain set/merge: - rename_peak_bandwidth renames the property wherever it is nested. - flatten_and_trim_required decouples IPSecRedundantTunnel (GET-response shape) from its ancestor schemas, which are shared with the create request body and must keep requiring passphrase there. - trim_network_tunnel_ipsec_redundant_required drops the three over-declared fields from NetworkTunnelIpsecRedundant's own private inline allOf branch in place. api/AUDIT-2026-08-10.md documents the correction transparently: what the first pass got wrong, why, and the corrected verdicts. The regression test now covers all 8 CONFIRMED candidates.
The API returns fields the spec omits, so DisallowUnknownFields turns undocumented fields into deserialisation failures. Overriding the template makes this an input to codegen rather than a post-generation edit that regeneration silently discards. utils.mustache covers the oneOf decode path (newStrictDecoder), which no generator flag reaches and which the v2.3 OPEN-04 Application-decoder bug and overlay entry A4's discriminator depend on. The second strict-decode site, in model_simple.mustache's per-model UnmarshalJSON, is instead suppressed via generate.sh's disallowAdditionalPropertiesIfNotPresent=false flag, avoiding an override of that much larger, heavily-parameterized template.
disallowAdditionalPropertiesIfNotPresent=false does not reach schemas that set additionalProperties: false explicitly (e.g. SupportOptionsRequest, a request-only schema whose decoder is never exercised against an API payload) nor the oneOf decode path (handled separately by templates/utils.mustache). The comment overstated coverage; name both gaps and their justification so neither is later "cleaned up" as noise. Also cd to the repo root as the first executable line so the script is correct regardless of the caller's cwd, ahead of Task 8 fixing a CI invocation convention.
BREAKING CHANGE: module path is now
github.com/CheckPointSW/perimeter-81-client-sdk/v3.
Service names follow v3's tag set: the four standard-tunnel services
collapse into StandardTunnelsAPI, the two object services into ObjectsAPI,
Regions -> StandardRegionsAPI, RouteTable -> StandardRouteTablesAPI,
Application -> ApplicationsAPI, Gateways folds into StandardNetworksAPI.
client.go (hand-written, protected by .openapi-generator-ignore) is
updated by hand to wire up the new service set, since it is never
regenerated.
Also removes .swagger-codegen-ignore, which listed 21 files and claimed
openapi-generator honoured it. It does not; only .openapi-generator-ignore
is read, so those files were unprotected. Their corrections now live in
api/overlay.yaml.
This is the first end-to-end run of scripts/generate.sh against the
corrected spec, and it surfaced four problems the pipeline had not hit
before, all fixed here:
- scripts/generate.sh was missing --git-user-id/--git-repo-id, so
test/*_test.go and README.md fell back to the literal placeholders
GIT_USER_ID/GIT_REPO_ID, which `go mod tidy`/`go build`/`go vet` cannot
resolve. Added both, with gitRepoId carrying the /v3 suffix so the
emitted import path matches go.mod's module line.
- HarmonySaseRegionsList.items is `allOf: [$ref, {description}]` with a
redundant sibling `additionalProperties: true` (HarmonySaseRegion, the
$ref target, already sets it). openapi-generator 7.24.0's Go codegen
mis-resolves that combination into invalid Go
(`[]HarmonySaseRegion[string]interface{}`); 7.21.0 rendered the same
upstream schema correctly. New overlay entry A12 drops the redundant
key (zero semantic change) to restore the pre-existing, already-shipped
`[]HarmonySaseRegion` shape.
- RoutingType/RoutingTypeUpdate share the enum ['route','policy'], and
StandardHealthCheckType/EnhancedHealthCheckType both contain 'tunnel'.
The Go enum template names constants from the value text with no
per-schema scoping, so both pairs collided at package scope
("ROUTE/POLICY/TUNNEL redeclared in this block"). New overlay entries
A13/A14 use x-enum-varnames to rename the narrower schema's constants
in each pair, without touching wire values.
- Stock model_oneof.mustache (openapi-generator 7.24.0) has a template
bug: its "no match" branch sits inside the {{#oneOf}} loop instead of
outside it, so any oneOf with N>=2 variants emits N copies of the same
branch, and `go vet` flags copies 2..N as unreachable dead code. Not
fixed via a local model_oneof.mustache override: that template is the
exact mechanism the A4 discriminator finding (see task-5-report.md)
depends on, so it must stay stock and be inspected as generated, not
patched for an unrelated cosmetic warning. generate.sh now runs
`go vet -unreachable=false ./...` instead, with the reasoning recorded
inline.
- Added templates/api_test.mustache: the stock template calls
NewConfiguration() with no arguments, but this SDK's hand-written
configuration.go (also protected) replaced that with a two-arg
NewConfiguration(apiKey, basePath). Every generated test is
unconditionally t.Skip'd (documentation, not exercised), so this only
fixes compilation, using placeholder argument values.
Verification: `python3 -m pytest scripts/test_build_spec.py` (6/6),
`go build ./...`, `go vet -unreachable=false ./...`, and manual checks
for api_default.go absence, zero bearer Authorization( occurrences, and
the full v3 service set all pass. Full findings, including the A4
discriminator investigation, are in task-5-report.md.
Overlay A4 added a discriminator to the Application GET 200 response's inline oneOf, but the generator gates any discriminator-based decode shortcut behind useOneOfDiscriminatorLookup (default false) regardless of whether a discriminator is present. Task 5's initial regeneration left this off, so model_get_application_by_id_200_response.go still generated the try-every-variant, match++ decoder — the exact OPEN-04 shape v2.3 had to hand-patch. A4 was a no-op. With the flag on, that decoder now switches on jsonDict["type"] and returns on the first successful branch, matching the v2.3 hand-patch's intent exactly. Cost, measured rather than assumed from the flag's doc string: 13 oneOf model files change in total (including the Application fix itself). 9 are type-disjoint primitive unions (ApplicationPort.value, FixedHost.value, RemoteID, RdpAttributes.maxConnections, NetworkIpsecBase.rightID, both `port` variants, `host`) where a JSON scalar is either a string or a number, never both, so the "exactly one match" validation this flag relaxes could never have fired anyway — nothing is lost. The remaining 3 non-Application object unions (CommonCreateApplication, CreateApplicationRequest, ObjectServiceProtocolTCPUDP, SourcesAndDestinations — 4, not 3) move from strict-decode-plus-validate matching to plain json.Unmarshal matching (still exactly-one-match-required, just without the strict decoder or gopkg.in/validator.v2 validation step — see model_sources_and_destinations.go for the shape). ObjectServiceProtocolTCPUDP was one of the 21 files v2.3 had to hand-patch specifically because strict oneOf matching failed on it, so the looser matching is closer to already-shipped, already-validated behaviour than the strict alternative it replaces. CommonCreateApplication/CreateApplicationRequest are request-only shapes serialized outbound, so their UnmarshalJSON is essentially never exercised against a real payload. SourcesAndDestinations is the residual risk carried forward: it is a genuine object union that appears in firewall-policy response shapes, not request-only, so the loosened matching there has real (if small) mis-selection potential. Flagged for Phase 4's firewall/SWG work to test rather than assume. go.mod/go.sum: gopkg.in/validator.v2 dropped, no generated oneOf decoder calls validator.Validate anymore under this flag; go mod tidy picked that up correctly. The go vet -unreachable=false workaround (unrelated upstream model_oneof.mustache template bug, see prior commit) stands unchanged and unaffected: the same duplicated-branch bug persists at shifted line numbers in the models that still lack a discriminator, confirmed by running plain `go vet ./...` before applying the flag to generate.sh. Verification: go build ./..., go vet -unreachable=false ./..., and python3 -m pytest scripts/test_build_spec.py (6/6) all pass. Full before/after decoder listing in task-5-report.md.
…um names
Fix round 2, addressing five review findings.
1. CRITICAL — generate.sh was not idempotent, breaking Task 8's planned
`git diff --exit-code` CI gate on its first run. Cause: openapi-generator
refuses to overwrite an existing test/*_test.go ("Test files never
overwrite an existing file of the same name"), so a second run against
an already-committed tree silently drops all 19 test/ entries from
.openapi-generator/FILES even though nothing else changed. The committed
FILES was only reproducible from a tree where test/ had been manually
deleted first, which Step 3 did by hand and the script itself never did.
Fix: --global-property=apiTests=false stops generating test/ entirely;
git rm -r test/; delete templates/api_test.mustache (no longer needed —
it existed solely to fix an argument-count mismatch in generated test
scaffolding that no longer exists). apiDocs/modelDocs are deliberately
left at their default: verified with two consecutive runs into an
already-populated tree that docs/'s regeneration is already fully
idempotent (doc files don't have the "never overwrite" behaviour), so
disabling them would only freeze docs/ out of sync with the API surface
for zero idempotency benefit.
Verified the actual acceptance criterion: two consecutive `generate.sh`
runs against the committed tree now produce byte-identical output across
all 573 relevant files (api/, docs/, model_*.go, .openapi-generator/).
--git-user-id/--git-repo-id are still needed even with test/ gone:
git_push.sh hard-codes their defaults and README.md's example import
uses them.
Side effect: go.mod/go.sum now have zero dependencies (testify was only
ever imported by the now-deleted test scaffolding); go build/vet still
pass with no *_test.go files anywhere in the tree.
2. A13/A14 deleted; replaced by enumClassPrefix=true. A13 disambiguated
RoutingType/RoutingTypeUpdate's shared enum by renaming the *narrower*
schema's constants (RoutingTypeUpdate -> ROUTE_UPDATE/POLICY_UPDATE) and
leaving the base schema's constants bare (ROUTE/POLICY) — but the v2.3
SDK always shipped these class-prefixed, and
terraform-provider-checkpointsase/checkpointsase/resource_enhanced_static_tunnel.go
references perimeter81Sdk.ROUTINGTYPE_ROUTE, which A13 deleted outright.
enumClassPrefix=true prefixes every enum constant unconditionally,
restoring ROUTINGTYPE_ROUTE exactly, resolving both collisions
(RoutingType/RoutingTypeUpdate, StandardHealthCheckType/
EnhancedHealthCheckType) as a side effect, and is collision-proof against
future shared enum values without a new overlay entry per collision.
3. Corrected generate.sh's useOneOfDiscriminatorLookup comment: it claimed
the flag swaps "exactly one match" validation for first-match-wins across
the SDK's other 13 oneOf models. That mechanism was wrong — those models
still count matches and still error on match > 1 or match == 0; what the
flag actually drops is strict field decoding (newStrictDecoder) plus
validator.Validate, in favour of plain json.Unmarshal + non-empty check.
Ambiguity detection is intact; only what counts as a candidate match is
looser.
4. Documented the unknown-discriminator fallthrough as an accepted cost:
model_get_application_by_id_200_response.go's discriminator if-chain
ends in a bare `return nil` when `type` is unrecognised or absent, so an
application of an unknown type decodes to an all-nil union with a nil
error, silently — not fixed here (would need a model_oneof.mustache
override, deliberately avoided), flagged for Phase 4 to test.
5. api/overlay.yaml A12's `op: remove` in scripts/build_spec.py used
pop(key, None), silently doing nothing if the target key were ever
already absent — exactly the failure mode that would hide A12's
remove_when condition coming true. op_remove now raises unless the entry
sets `optional: true` (no entry does today). Added
test_a12_harmony_sase_regions_list_items_have_no_sibling_additional_properties
to scripts/test_build_spec.py, asserting the spec-level precondition
(and, for contrast, that the one other items:allOf+sibling-
additionalProperties occurrence in the spec — DynamicTunnelUpdate's
updateTunnels — is left alone because its allOf's non-$ref branch is
substantive, forcing model materialization instead of the buggy
$ref-collapse).
Minor: corrected A12's reason (the shape occurs twice, not once; stated the
actual distinguishing condition); corrected the go vet comment's stale
example (model_get_application_by_id_200_response.go no longer triggers the
warning after fix #2's discriminator dispatch — model_sources_and_destinations.go
does).
Verification: go build ./..., go vet -unreachable=false ./...,
python3 -m pytest scripts/test_build_spec.py (7/7), and two consecutive
./scripts/generate.sh runs producing a byte-identical tree, all pass.
v3 documents POST /v3/auth/authorize served from the /api/rest base, so the v2.3-era '/rest' strip must go. Both files are in .openapi-generator-ignore, so codegen cannot update them; authorizeURL() isolates the path so it is unit-testable. Adds BaseURLCA for the new Canada region and reports 3.0.0 in the UA.
…erride README.md is itself generated output (stock go-generator README.mustache, not covered by .openapi-generator-ignore), so the "Regenerating the SDK" section is added through templates/README.mustache rather than hand-edited into README.md directly — a direct edit would be silently discarded by the next ./scripts/generate.sh run, which is exactly the failure mode this section warns about. Covers the upstream+overlay->swagger.yaml->generate.sh pipeline, why make verify is the gate to run before pushing, why the v2.3 SDK's 21 hand-patches were unprotected (.swagger-codegen-ignore is not read by openapi-generator), the 7.24.0 generator pin, and why go vet needs -unreachable=false (model_oneof.mustache emits unreachable code for oneOf schemas with 2+ variants). Also records that the Maven Central JAR for 7.24.0 is byte-identical (verified by MD5) to the one Homebrew installs and prints an exact "7.24.0" with no wrapper banner text, as a clean CI install path if a CI workflow is ever added -- none is wired up now; that was a deliberate choice, not an oversight, so the gate is make verify run locally.
…late override templates/README.mustache (added in the previous commit) was the wrong mechanism: it vendored a 286-line copy of the stock Go-generator template, frozen against future generator upgrades, and grew templates/ from one override to two for no real benefit -- contradicting the "templates/ holds only deliberately-overridden templates; fewer is better" constraint that Task 5's fix round 2 already established by deleting api_test.mustache. README.md is now listed in .openapi-generator-ignore instead, the same mechanism already protecting client.go, configuration.go, response.go, and the two token models. It is a plain hand-authored file again -- no mustache placeholders -- and ./scripts/generate.sh no longer touches it (verified: two consecutive runs produce identical README.md content and zero additional diff). One tradeoff worth it: the endpoint/model listing tables in README.md no longer auto-refresh on regeneration, since the whole file is now frozen. Noted inline in the "Regenerating the SDK" section. Also documents the sharper form of the drift-experiment finding: an uncommitted hand-edit to a generated file makes `make verify` exit 0 while silently destroying the edit (regeneration overwrites the working tree before the diff check runs), whereas a staged/committed hand-edit exits 2 with the "committed SDK differs" error -- which is the case CI/a checkout actually sees. Destroying the edit is intended; the silent exit-0 in the uncommitted case is the footgun worth calling out.
Skipped unless CHECKPOINT_SASE_API_KEY is set, so it stays out of the offline tier. Asserts /v3/auth/authorize returns a token and that the token is accepted on a follow-up authenticated call. Doc comment notes the base URL path prefix is environment-dependent: production regions include /api/rest, while the solo.safersoftware.net gateway used for live verification serves /v3/... at the root.
The live auth smoke test caught a real defect: GET /v3/status returns HTTP 200 with content-type text/plain (body "Ok"), which the v3 spec correctly declares and the generator correctly types as a plain string return. But decode() in client.go only handled application/xml and application/json, so every text/plain endpoint failed with "undefined response type" despite a successful, authenticated response. Add a text/plain branch to decode() that writes the raw body into a *string target, or returns a clear error for any other target type. client.go is in .openapi-generator-ignore, so this survives regeneration. Add unit tests in client_test.go covering: text/plain into *string, text/plain with a charset parameter, text/plain into a non-string target, application/json into a struct (regression guard), and an unrecognized content type (fallback error path). Also correct smoke_auth_test.go's diagnostic: a GetStatus failure does not by itself mean the token was rejected, as this defect proved - the token was accepted; decoding the response body was what failed.
Phase 1 hit 111 build errors where the plan predicted ~38. The prior audit was built from LEFTOVERS.md/DEVELOPER-GUIDE.md prose instead of the v2.3 hand-patched files themselves (recoverable from .swagger-codegen-ignore and git history on main), and was both incomplete and partly mis-described: - A7 (object-service protocol anyOf shape, BUG-17) was a candidate in the design spec's carried-forward table but had no check at all in audit_carried_patches.py. v3 still declares ObjectsServicesProtocolRequestObj/ ResponseObj as a two-member anyOf that openapi-generator still turns into a pointer-pair wrapper unable to round-trip the real flat wire payload. Fixed via a flat-object overlay replacement (A7-*-flatten). - A9 (ASN/RemoteASN, BUG-24) was checked via `type != "integer"`, a proxy for the defect rather than the defect itself. Both schemas declare `type: integer` at top level (so the proxy check passed) but also carry a sibling `oneOf` of narrower integer ranges that drives openapi-generator 7.24.0 into an empty-struct wrapper regardless. Fixed by removing the redundant oneOf (A9-*-remove-redundant-oneof). Empirically verified (see api/AUDIT-2026-08-10.md's round-2 section) that the A9 fix does NOT restore ASN/RemoteASN as distinct Go types — openapi-generator has no mechanism to alias a bare scalar schema to a named type, confirmed four independent ways. Every field that referenced them now generates as a correct, real-value-carrying int32/*int32 instead of an always-zero empty struct, but `ASN(int32(x))`-style conversions (already present in the checkpointsase provider repo) will need a small mechanical follow-up in Phase 1. Recorded as a decision point, not papered over. Also verified all 8 BUG-23 (peakBandwidth->peakBandwidthMbps) files beyond the 3 A8 already covers: v3 dropped the field entirely from all 8, so no new entries are needed there — writing entries for already-fixed defects was the explicit lesson of the prior round. Overlay entries: 24 -> 28. scripts/test_build_spec.py grows two new regression tests asserting the derived postcondition in the built spec, not merely that the entries exist. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Product of scripts/generate.sh against the corrected api/swagger.yaml (previous commit). ASN/RemoteASN no longer generate a model file at all (openapi-generator has no scalar-alias mechanism for Go) — every field that referenced them now generates as int32/*int32 instead of an unusable empty struct. ObjectsServicesProtocolRequestObj/ResponseObj generate as flat structs (protocol required; valueType, value, protocolOptions optional) instead of an anyOf pointer-pair wrapper. Verified: go build ./..., go vet -unreachable=false ./..., and ./scripts/generate.sh run three times in a row all produce zero diff. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
GET /v3/networks/standard/{id} returns tunnel types the spec does not
enumerate (`connector` is the documented example), and every one of them
failed the whole anyOf(NetworkTunnel) decode since v2.3's hand-patched
fallback was dropped when this SDK moved to a generated pipeline. Add
overlay entry A15 to append NetworkTunnelBase — already a full v3 schema —
as a fifth anyOf member.
Regenerating exposed a real generator constraint the overlay ordering alone
can't fix: openapi-generator 7.24.0 collapses anyOf into a
java.util.TreeSet<String> (alphabetical, hardcoded in CodegenModel, not
configurable), so "last in the YAML list" does not mean "tried last" in the
generated decoder — NetworkTunnelBase sorted first and would have silently
swallowed concrete-variant payloads. templates/model_anyof.mustache now
overrides the stock decoder to defer the alphabetically-first anyOf member
(today, exactly NetworkTunnelBase) to the end; NetworkTunnel is the only
anyOf-composed schema in this spec, so the blast radius is limited to it.
Known gap, left for a decision rather than improvised: NetworkTunnelBase's
generated required-field check still rejects the realistic case where
createdAt is absent from the wire payload (the exact shape v2.3's hand-patch
handled). See TestNetworkTunnel_UnrecognizedTypeWithoutCreatedAt_StillErrors.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…back actually catches unrecognised tunnel types The A15 fallback still required 9 fields composition-resolved (its own 8 plus createdAt via allOf -> BaseDates), so it rejected the realistic connector payload it was meant to catch (createdAt absent on the wire). Add A16: flatten NetworkTunnelBase and trim its required list to [id] — reproducing the v2.3 hand-patch's exact guard (non-empty id, nothing else) declaratively. BaseDates itself is left untouched (shared by other schemas). NetworkTunnelBase is also the allOf ancestor of all four concrete tunnel variants, and openapi-generator unions required fields across allOf branches, so A16 alone would have silently cascaded into NetworkTunnelOpenvpn/Wireguard/IpsecSingle/IpsecRedundant's composed required sets too. A17a-A17d flatten each of those four with their exact pre-A16 required set restored explicitly, decoupling them from NetworkTunnelBase's required-ness. Verified exhaustively: diffing every model's composed required set before/after shows exactly one line changed (NetworkTunnelBase, 9 fields -> [id]) across all 261 model files — the four concrete variants' generated .go files are byte-for-byte unchanged. The connector-without-createdAt decode test now passes (previously documented as a known gap). Added an empty-payload regression test to prove the [id] guard still rejects garbage. Also strengthened templates/model_anyof.mustache with a template-level (non-rendering) header comment explaining the TreeSet-alphabetical-ordering rationale for readers who open the template file directly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
generate.sh ran openapi-generator with --skip-validate-spec since inception, suppressing "Undefined array inner type for `null`" (x4) and "CommonCreateApplication.items is missing" (x1) with no record of which schemas they touched. All five trace to one defect: CommonCreateApplication's own oneOf branches redeclare properties.users/properties.groups as bare arrays with no items, unlike the schema's own correctly-typed top-level users/groups. Provider-exposure analysis (grepped both this SDK and the provider): the three Go types this oneOf materializes are dead code referenced by nothing else in the SDK or the provider. The provider-used create- application types get Users/Groups from this schema's own top-level properties instead, so they were never affected by this defect. Fixed in api/overlay.yaml (A18) by giving both oneOf branches the same items ref the schema's top-level properties already use. Verified this resolves all five diagnostics (`openapi-generator validate --recommend` now reports 0 errors) and regenerates byte-identical Go output, since the broken properties' "default to string" fallback already produced the same []string type a correct ref would. Because every error is now genuinely resolved, removed --skip-validate-spec from generate.sh and proved the full script still succeeds without it. Provider build and tests confirmed green (35 pass / 14 skip / 0 fail, unchanged). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A live GET /v3/networks/enhanced/{networkId}/tunnels captured 2026-08-16
against a real static tunnel returns ikeLifeTime, lifetime, dpdDelay,
dpdTimeout, phase1, phase2, remotePublicIP, remoteID, description and
isHA as top-level keys, and returns no advancedSettings object at all.
EnhancedTunnelBase declares the opposite: one advancedSettings ref and
none of those ten fields.
The generated EnhancedTunnel.AdvancedSettings pointer was therefore nil
on every real response. Its nil-safe getters returned "", so the
provider's Read functions blanked the user's configured timing values on
every refresh, and remotePublicIP/remoteID had no field to read at all
(the separately-recorded "enhanced_static_tunnel.remote_id can never
populate" defect is the same root cause).
Overlay entry A19 replaces EnhancedTunnelBase with the shape the server
actually returns. The whole schema is replaced rather than edited in
place because both the additions and the advancedSettings removal live
inside its second allOf branch, and build_spec.py's path walker
addresses mapping keys only.
The write path cannot be affected: EnhancedTunnelBase has exactly one
reference in the spec (EnhancedTunnel's allOf), and StaticTunnelCreate,
StaticTunnelUpdate, DynamicTunnelCreate and DynamicTunnelUpdate compose
TunnelAuthConfig and IPSecAdvancedSettingsV2_3 directly. Confirmed
empirically: regeneration touches only model_enhanced_tunnel.go,
model_enhanced_tunnel_base.go and their two docs files; all six
write-model files hash byte-identical to before.
required is trimmed to the six fields the generator already enforced, so
the added fields generate as nil-safe optional pointers and the
generated required-property check is unchanged. features is
deliberately left undeclared (the capture abridged its contents) and
lands in AdditionalProperties instead. See A19's reason for all three
decisions.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
GET /v3/objects/web-category answers 200 with a full, valid catalog in which every entry carries exactly id and name. The string "codes" appears nowhere in the payload. Measured live 2026-08-19 during the first acceptance run of checkpointsase_web_categories. The spec declares codes required, so it lands in the generated requiredProperties list and WebCategory.UnmarshalJSON checks for the key before unmarshalling anything — returning "no value given for required property codes". That rejects the WHOLE payload, not the offending entry, so the entire catalog read fails and the data source reports "Unable to get Web categories" on a response that is entirely well-formed. There is no partial-success path, which is why it cannot be worked around above the SDK. Only codes is dropped from required; id and name stay. Deliberately narrower than A6 and A10, which emptied their required lists outright: the live data positively supports keeping id and name, and emptying the list would turn the generated Id/Name into *string and push a pointer-dereference change into the provider's flattenWebCategories for no measured benefit. Committed on behalf of the implementer, which was killed by an upstream 500 after making the change and regenerating but before committing. I verified the entry, the regenerated requiredProperties (now id, name) and that make verify passes on the committed tree. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
parameterAddToHeaderOrQuery dereferenced a pointer argument into `v` (`v = reflect.ValueOf(obj).Elem()`) and then formatted the ORIGINAL `obj` in its scalar branch, so any pointer argument reached the wire as its address: `?page=0x14000112028` instead of `?page=1`. The generated request methods pass a POINTER exactly when the caller has explicitly set a scalar query parameter, and a plain VALUE on the schema-default `else` branch -- which is why omitting a parameter worked and setting one did not, and why this went unnoticed. Measured against /v3/objects/updatable-objects: `?page=1&limit=1000` returns 200 with 1000 rows, while the same call through this SDK returned 422 "limit must be >= 1, page must be >= 1". The server was never at fault; the provider's own error text prints its intent, not what it sent. Blast radius: every explicitly-set scalar query parameter in the SDK. No caller has ever successfully set one. The immediate casualty is the objects-catalog data source (page/limit on updatable-objects), but the same shape appears across api_objects.go, api_team.go, api_applications.go, api_enhanced_tunnels.go and api_enhanced_route_tables.go -- 35 query-parameter call sites in total -- so several provider data sources have accepted page/limit arguments they could never actually use. MECHANISM: this is a hand edit to client.go, NOT a third templates/ override, and that is a deliberate correction to the task's stated plan. client.go is listed in .openapi-generator-ignore, so ./scripts/generate.sh never writes it -- the generator says so itself in its log: Ignored .../client.go (Ignored by rule in ignore file.) A templates/client.mustache override would therefore have been inert: the template is never rendered to disk in this repo. It would also have been unnecessary, because stock openapi-generator 7.24.0's go/client.mustache does not carry this defect at all -- its `case reflect.Ptr` recurses on `v.Elem().Interface()` (go/client.mustache:214-216 inside the jar). The bug is local to the v2.3-era hand-maintained client.go this repo carries (its header still says "API version: 2.3.0"), not to the generator, and the ignore rule is exactly what makes a hand edit durable here. This follows the precedent set in c873b1a, which chose .openapi-generator-ignore over a vendored template for the same reason: fewer frozen template copies is better. templates/ stays at two entries. Note the ordering this implies for `make verify`: because client.go survives regeneration, an UNCOMMITTED edit to it shows up as a diff and fails the gate, and a COMMITTED one passes. That is the mirror image of the drift behaviour c873b1a documented for genuinely generated files, and it is the case CI sees. Verified green after committing. `v` rather than `v.Interface()` is deliberate: fmt replaces a reflect.Value operand with the concrete value it holds, and unlike v.Interface() it cannot panic on the zero Value that Elem() yields for a typed-nil pointer (the `obj == nil` guard does not catch a typed nil inside an interface). No generated call site can reach that path -- all of them guard with `if r.<param> != nil` -- but the helper is package-level and the cheaper spelling is also the safer one. Tests (client_test.go, hand-maintained -- there is no client_test.mustache in the Go generator): * TestParameterAddToHeaderOrQueryFormatsPointerScalars -- table over *int32/*int64/*float64/*bool/*string plus the non-pointer rows that always worked, so the fix cannot regress the default branch. * TestGetUpdatableObjectsSendsExplicitPageAndLimitAsNumbers -- the same defect end to end through the real generated request builder against an httptest server, with BearerTokenData pre-seeded so the test stays offline. * TestParameterAddToHeaderOrQueryKeepsSliceHandling -- guards the collection branch, which was already correct. * TestParameterAddToHeaderOrQueryTypedNilDoesNotPanic -- pins the one behavioural edge the fix touches. Teeth verified by reverting the single changed expression and re-running: 6 of 9 subtests and the end-to-end test fail with real addresses (`query value = "0x6ba6074f6510", want "1"`); the non-pointer rows keep passing, which is the correct signature for this bug. KNOWN, NOT FIXED HERE: the adjacent `map[string]string` branch has the identical `fmt.Sprintf("%v", obj)` defect for HEADER parameters, and it is live -- api_settings.go sends the *string `x-auth-lambda-authorization` through it at two call sites, so an explicitly-set value there becomes a pointer address in the header. Left alone to keep this commit to the one line the task authorised; noted inline and reported for a follow-up decision. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The header-parameter twin of 9b20b40, and the follow-up that commit named. Same one-word cause: the map[string]string branch of parameterAddToHeaderOrQuery formatted `obj`, the original pointer, where `v` already held the dereferenced value. Why this one matters more than the query-string case it mirrors. The only live callers are api_settings.go:94 and :211, both passing the *string x-auth-lambda-authorization. An explicitly-set value therefore travelled as "0x14000112028" in an HTTP auth header. That does not fail loudly the way the query-string defect did against /v3/objects/updatable-objects, which answered a clear 422 "must be >= 1"; it presents a meaningless credential and comes back unauthorized, which is a far harder failure to trace to serialisation. No secret is exposed either way -- an address is not the token. Nothing in the provider sets that header today, so this was latent rather than active. client_header_test.go pins it, and was proven to bite: with the fix reverted it reports `header = "0x52f3688104d0", want "Bearer abc123"`, while the non-pointer case keeps passing. Also corrects the NOT CHANGED HERE, DELIBERATELY note left in 9b20b40's comment, which described this branch as still broken and is now false. A hand edit rather than a templates/ override, for the reason 9b20b40 established: client.go is line 1 of .openapi-generator-ignore, absent from .openapi-generator/FILES, and never written by the generator, so an override would be inert. Stock 7.24.0 go/client.mustache carries neither defect -- both are local to this repo's hand-carried v2.3-era client.go. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…lists, expand deepObject query params Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…d-to-end Review follow-ups on de8917b: - A21b/A22b become `op: remove` on the whole `required` list rather than `op: set` down to one field. Keeping `email` rested on CreateUserDto marking it required, but IdP-synced accounts never pass through that DTO and AD's `mail` attribute is optional, so a mail-less account would still have failed the entire GET /v3/users page. `required: []` is not the alternative: OpenAPI 3.0 inherits JSON-Schema draft-4's "at least one element". Removing the key also gives both entries a self-firing disposal trigger, since op_remove raises once its target is absent. - Both `remove_when` blocks rewritten to an evaluable condition. The previous text ("all eight keys on every record") was a universal claim that could only fail to be falsified, so the entries would have lived forever. - New end-to-end test drives the real generated ListUsers builder against an httptest server and asserts the emitted query string. The three helper tests never observed api_team.go, so a regeneration that dropped the "deepObject" style argument would have left them all green. - Three more tests pin the map-of-slices, map-of-pointers and nil-map shapes the client.go comment claims recursion buys for free. - client.go comment narrowed: it claimed the local fix and stock 7.24.0 were identical implementations. They match for the map-in-query case this fix covers, but stock also expands map HEADER parameters and indexes slice elements under deepObject. That claim is load-bearing for the disposal condition, so it now says only what is true. - pytest: upstream-defect guard extended from three entries to all six; A24/A24b now pin maxLength and the exact key set, not just the pattern. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The entry's reason claimed flattenWebCategories' nil coercion was the only thing standing between an absent `codes` and a nil slice in state. A direct probe disproved it: schema.ResourceData.Set normalises a nil slice to an empty list by itself, including for a list nested inside a list element. What the entry IS load-bearing for is unchanged and still measured -- without it the whole catalog response fails to unmarshal and the data source errors on a valid 200. Only the downstream claim was wrong. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
chkp-olegkre
approved these changes
Aug 26, 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.