Skip to content

Commit 869a579

Browse files
ryanyuansean-
andauthored
fix(osgen): json.RawMessage guard + collision/nullable/alias/enum fixes (#890)
A json.RawMessage in generated output is the symptom of a type the generator could not resolve. Most are legitimate freeform JSON (_source, _meta), but a generator bug can silently widen the raw-JSON surface of the public API by spawning many at once. This PR adds a guard that pins the permitted set, then burns the list down by fixing the underlying generator defects. Guard: - Add a checked-in allowlist (cmd/osgen/rawmessage_allowlist.txt) keyed GoTypeName/jsonFieldName. collectRawMessageUses walks the IR -- struct fields plus the synthetic whole-response raw/map/array shapes -- and generation fails (non-zero exit) when an unlisted use appears, exposing silent regressions into a gen-time error. classifyRawForm pushes pointer/slice/map wrappers to the leaf, so nested raw (e.g. [][]json.RawMessage for SQL/PPL Datarows) cannot escape the guard. Flags: -raw-message-allowlist / -update-raw-message-allowlist / -allow-unlisted-raw-message. Generator fixes (each removes a class of unresolved-to-raw degradation): - Union/struct name collisions: a oneOf/anyOf field whose parent-scoped union name collided with the parent struct's Go name dropped the struct and degraded the response to raw. Such unions are now re-keyed by their referenced schema, and any remaining collisions are reported to stderr instead of silently dropping types. Re-types the tasks family (tasks.list, tasks.cancel, delete_by_query_rethrottle) and the CommonMapping family (_common.mapping___DynamicTemplate.mapping -> *CommonMappingProperty). - OpenAPI 3.1 nullable scalars: ["null","<primitive>"] fell to raw because kin-openapi Type.Is is false for a 2-element set. nullablePrimitiveGoType resolves {null,<primitive>} to the pointer primitive, clearing the CAT *Record cluster. - Bare-$ref alias responses: a response whose component schema is a bare $ref alias missed the registry lookup under its alias key. resolveSchemaAlias follows the chain to the terminal key (cycle-safe), fixing ISM add/delete/get/remove_policy + retry_index and all 7 ml.search_*. - Typed enums: emit int-backed (const iota) enums for string fields carrying an x-enum-name marker + enum: constraint, with an <Name>Unknown sentinel, name<->value maps, String(), MarshalJSON, and a closed-set UnmarshalJSON (unknown values recoverable via *Unknown<Name>Error). Markers are shared (registered once, reused across fields) and a marker reused with a conflicting value set fails generation rather than silently merging. Honored whether the schema arrives inline or via a component $ref. Applies to the security status field (RestStatus). - Phantom SQL/PPL stats body: remove the requestBody from the _sql/stats and _ppl/stats POST operations (the server ignores it; the schema is an upstream spec defect), dropping the dead SQLStats type. Signed-off-by: Ryan Yuan <ryan.yuan@crowdstrike.com> Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> Co-authored-by: Ryan Yuan <ryan.yuan@crowdstrike.com> Co-authored-by: Sean Chittenden <sean.chittenden@crowdstrike.com>
1 parent 58d0301 commit 869a579

100 files changed

Lines changed: 7851 additions & 8049 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
66

77
### Added
88

9+
- `cmd/osgen`: guard `json.RawMessage` in generated request/response types behind a checked-in allowlist (`cmd/osgen/rawmessage_allowlist.txt`). Because a `json.RawMessage` is the symptom of a type the generator could not resolve, a generator bug can silently widen the raw-JSON surface of the public API; generation now fails (non-zero exit) when any `json.RawMessage` use is not listed, including nested forms such as `[]json.RawMessage`, `map[string]json.RawMessage`, and `[][]json.RawMessage` (the leaf is detected at any wrapper depth). Entries are keyed `GoTypeName/jsonFieldName` (whole-response raw bodies use `<Prefix>Resp/-`, and map/array responses whose element type is unresolved use `<Prefix>Resp/[entries]` and `<Prefix>Resp/[records]`). Add `-update-raw-message-allowlist` to regenerate the allowlist from current output (sorted and grouped for minimal diffs), and `-allow-unlisted-raw-message` to downgrade the check to a warning ([#890](https://github.com/opensearch-project/opensearch-go/pull/890))
10+
- `cmd/osgen`: emit int-backed (const `iota`) enum types for string fields carrying an `x-enum-name` marker alongside an `enum:` constraint. Each enum generates a named int type with a zero-value `<Name>Unknown` sentinel, name<->value lookup maps, `String()`, `MarshalJSON`, and a closed-set `UnmarshalJSON` that rejects unknown wire values via a typed `*Unknown<Name>Error` (recoverable through `errors.As`). The marker is shared, so a single enum type is registered once and reused across every referencing field; a marker reused with a conflicting value set fails generation rather than silently merging. Applied to the security `status` field, which becomes a typed `RestStatus` enum ([#890](https://github.com/opensearch-project/opensearch-go/pull/890))
911
- Add `OPENSEARCH_GO_POLICY_DUMP` environment variable: when set with `OPENSEARCH_GO_DEBUG=true`, dumps the router's policy tree (the dot-delimited node paths that `OPENSEARCH_GO_POLICY_*` matchers target, each labeled with its pool or role) to the debug logger at client initialization. The dump walks the structural tree so router wrappers that share an inner policy instance are each rendered in full. ([#883](https://github.com/opensearch-project/opensearch-go/issues/883))
1012
- Add a `build-samples` Makefile target and a CI job that compiles and vets every `_samples/*.go` program, so example breakage is caught (the `_samples` directory is excluded from `go build ./...` because Go ignores `_`-prefixed paths)
1113
- Group document operations under a `client.Doc` sub-client and point-in-time operations under `client.PIT` (`Create`/`Delete`/`GetAll`/`DeleteAll`); `client.Document` and `client.PointInTime` remain as field aliases. The indices sub-client's canonical field is `client.Index`, with `client.Indices` and `client.Indexes` as aliases. `cmd/osgen` gains `--emit-v4-compat` (default true) to emit backward-compatibility forwarders so top-level `client.Bulk`/`MGet`/`Update`, `client.Document.Source`, and `client.PointInTime.Get` keep working (`client.Index` is not forwarded -- it is the indices sub-client field; use `client.Doc.Index`), and `--emit-v4-deprecation` (default false) to mark those forwarders deprecated
@@ -220,6 +222,9 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
220222
### Fixed
221223

222224
- Cache credentials in the `signer/awsv2` constructors. A raw `CredentialsProvider` is wrapped in an `aws.CredentialsCache` (an already-cached provider, such as one from `config.LoadDefaultConfig`, is left as-is), so SigV4 signing no longer calls `Credentials.Retrieve` on every request. For STS-backed providers (assume-role, web identity, IRSA) the previous behavior was a per-request STS call that could exhaust the account's STS rate limits under load. `signer/awsv2` shipped without this in v4.6.0.
225+
- Fix `cmd/osgen` silently dropping a response struct when a response schema has a `oneOf`/`anyOf` field whose parent-scoped union name collides with the parent struct's own Go name. The union registered first and the parent struct was then dropped by the type registry (its name already taken), degrading the response to raw `json.RawMessage`. Such a union is now re-keyed by its referenced schema so the parent struct survives. The generator also reports any remaining Go type name collisions to stderr at generation time instead of dropping types silently. Regenerating fixes two type families: `tasks.list`, `tasks.cancel`, and `delete_by_query_rethrottle` change from raw `Body json.RawMessage` to typed structs (`NodeFailures`, `TaskFailures`, `Nodes map[string]TasksTaskExecutingNode`, `Tasks *TasksTaskInfos`), and the `_common.mapping___DynamicTemplate.mapping` field becomes typed `*CommonMappingProperty` (accounting for the large `unions_gen.go`/`indices-put_mapping_gen.go` churn). ([#890](https://github.com/opensearch-project/opensearch-go/pull/890))
226+
- Fix `cmd/osgen` degrading two more schema shapes to raw `json.RawMessage`: an OpenAPI 3.1 nullable scalar (`type: ["null", "<primitive>"]`) fell through because kin-openapi's `Type.Is` matches only single-element type sets, and a response whose component schema is a bare `$ref` alias (`Foo: {$ref: Bar}`) missed the registry lookup under its alias key. Nullable scalars now resolve to the pointer primitive (`*string`/`*int`/`*bool`/`*float64`), clearing the CAT `*Record` cluster, and alias responses follow the `$ref` chain to the registered struct, fixing ISM `add`/`delete`/`get`/`remove_policy` + `retry_index` and the seven `ml.search_*` responses. ([#890](https://github.com/opensearch-project/opensearch-go/pull/890))
227+
- Fix `cmd/osgen` generating a phantom request body for the `_sql/stats` and `_ppl/stats` POST operations. The server (`RestSqlStatsAction`/`RestPPLStatsAction`) ignores the request body, so the spec's body schema is a defect; removing it drops the dead `SQLStats` type. The typed client no longer sends a body to these endpoints. ([#890](https://github.com/opensearch-project/opensearch-go/pull/890))
223228
- Fix `BulkIndexer` `OnFailure` nil pointer dereference when reading `BulkRespItem.Error` on status-only failures (e.g. HTTP 404 without an `error` object) or transport-level flush errors by ensuring callbacks always receive a non-nil `Error` ([#679](https://github.com/opensearch-project/opensearch-go/issues/679))
224229
- Generate query parameters whose value `0` is meaningful as `*int` instead of `int` so a deliberate `0` reaches the wire. These params previously used the `!= 0` emission guard shared by all integer params, which silently dropped a deliberate `0` -- breaking optimistic-concurrency writes with `if_seq_no=0` (the sequence number of the first document written to a shard) and search `size=0` (aggregations with no hits). `cmd/osgen` now promotes such params to `*int` with a nil guard, mirroring the existing `*bool` treatment. The promotion is scoped per operation (currently `if_seq_no`/`if_primary_term` on `delete`/`index`/`update` and the plugin policy writes `ism.put_policy`/`ism.put_policies`/`rollups.put`/`sm.update_policy`/`transforms.put`, plus `size` on `search`), since the same wire name is a page-size with no meaningful `0` on other operations. The core `_create` operation does not accept `if_seq_no`/`if_primary_term`, so it is intentionally excluded
225230
- Fix `BulkIndexerStats.NumAdded` overcounting items rejected by `Add()` when the caller's context is cancelled before the item could be enqueued: increment `NumAdded` only after the queue accepts the item, and add a new `BulkAddFailCount` counter for items dropped on the `<-ctx.Done()` branch. Migrate `bulkIndexerStats` fields to `sync/atomic.Uint64` typed values so future direct access is a compile-time error rather than a `-race`-only finding ([#783](https://github.com/opensearch-project/opensearch-go/issues/783))

Makefile

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,18 @@ gen-api: fetch-opensearch-spec ## Regenerate API consumer files only
223223
-max-version=$(GEN_MAX_VERSION) \
224224
-remove-deprecated=$(GEN_REMOVE_DEPRECATED)
225225

226+
gen-api-update-rawlist: fetch-opensearch-spec ## Regenerate API files and refresh the json.RawMessage allowlist
227+
@printf "\033[2m-> Regenerating API consumer files and json.RawMessage allowlist...\033[0m\n"
228+
cd $(REPO_ROOT)/cmd/osgen && go run . api \
229+
-spec $(OPENAPI_SPEC) \
230+
-out $(GEN_OSAPI_DIR) \
231+
-pkg opensearchapi \
232+
-plugins-out $(GEN_PLUGINS_DIR) \
233+
-min-version=$(GEN_MIN_VERSION) \
234+
-max-version=$(GEN_MAX_VERSION) \
235+
-remove-deprecated=$(GEN_REMOVE_DEPRECATED) \
236+
-update-raw-message-allowlist
237+
226238
gen: gen-paths gen-api ## Regenerate all code from OpenAPI spec (run gen-paths and gen-api in parallel with `make -j gen`)
227239

228240
regen: clean-gen gen ## Clean generated files then regenerate from spec

_samples/usage-tasks.go

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -144,16 +144,7 @@ func example() error {
144144
if err != nil {
145145
return err
146146
}
147-
var taskList struct {
148-
Nodes map[string]struct {
149-
Name string `json:"name"`
150-
Tasks map[string]any `json:"tasks"`
151-
} `json:"nodes"`
152-
}
153-
if err := json.Unmarshal(listResp.Body, &taskList); err != nil {
154-
return err
155-
}
156-
for nodeID, node := range taskList.Nodes {
147+
for nodeID, node := range listResp.Nodes {
157148
fmt.Printf("Node %s (%s): %d tasks\n", node.Name, nodeID, len(node.Tasks))
158149
}
159150

cmd/osgen/api_cmd.go

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,12 @@ func runAPI() error {
5353
"emit backward-compatibility forwarder methods (e.g. top-level Client.Bulk forwarding to Doc.Bulk)")
5454
emitV4Deprecation := fs.Bool("emit-v4-deprecation", false,
5555
"mark the v4 compatibility forwarders with a Deprecated doc comment (requires -emit-v4-compat)")
56+
rawAllowlist := fs.String("raw-message-allowlist", "rawmessage_allowlist.txt",
57+
"path to the checked-in json.RawMessage allowlist (relative to cwd)")
58+
updateRawAllowlist := fs.Bool("update-raw-message-allowlist", false,
59+
"rewrite the json.RawMessage allowlist from current output instead of checking it")
60+
allowUnlistedRaw := fs.Bool("allow-unlisted-raw-message", false,
61+
"downgrade the json.RawMessage allowlist check from fatal to a warning")
5662
if err := fs.Parse(os.Args[1:]); err != nil {
5763
return err
5864
}
@@ -80,7 +86,8 @@ func runAPI() error {
8086
}
8187

8288
return generateAPI(*specPath, filter, *outDir, *pluginsDir, *pkg, vrange, bc,
83-
CompatConfig{V4Compat: *emitV4Compat, V4Deprecation: *emitV4Deprecation})
89+
CompatConfig{V4Compat: *emitV4Compat, V4Deprecation: *emitV4Deprecation},
90+
RawMessageConfig{AllowlistPath: *rawAllowlist, Update: *updateRawAllowlist, AllowUnlisted: *allowUnlistedRaw})
8491
}
8592

8693
// generateAPI uses the two-phase pipeline (Parse -> IR -> Emit -> Targets).
@@ -98,6 +105,7 @@ func generateAPI(
98105
vrange VersionRange,
99106
bc BreadcrumbConfig,
100107
compat CompatConfig,
108+
rawCfg RawMessageConfig,
101109
) error {
102110
if bc.Types != BreadcrumbAll {
103111
return fmt.Errorf("--version-breadcrumb-types is not implemented for `osgen api`: " +
@@ -113,6 +121,7 @@ func generateAPI(
113121
registry := newTypeRegistry(corePkg)
114122
respFieldExc := populateResponseTypes(ops, spec, registry, vrange)
115123
reqFieldExc := populateRequestBodyTypes(ops, spec, registry, vrange)
124+
reportCollisions(os.Stderr, registry)
116125
fieldExclusions := append(respFieldExc, reqFieldExc...) //nolint:gocritic // intentional concat into new slice
117126
sort.Slice(fieldExclusions, func(i, j int) bool { return fieldExclusions[i].Name < fieldExclusions[j].Name })
118127

@@ -123,6 +132,12 @@ func generateAPI(
123132
Params: filterExclusions(paramExclusions, bc.Params),
124133
}
125134

135+
// Guard against the generator silently widening the raw-JSON surface. Run
136+
// before any file is written so an unlisted use aborts cleanly.
137+
if err := guardRawMessages(os.Stderr, irSpec, rawCfg); err != nil {
138+
return err
139+
}
140+
126141
// Apply the v4 compatibility-forwarder policy before sub-client filtering so
127142
// dropped forwarders don't keep an otherwise-dead sub-client alive.
128143
applyCompatPolicy(irSpec.Operations, compat)

cmd/osgen/api_extract.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -255,7 +255,7 @@ func buildAPIOperation(group string, ops []struct {
255255
op *openapi3.Operation
256256
path *openapi3.PathItem
257257
url string
258-
}, _ *openapi3.T, vrange VersionRange,
258+
}, spec *openapi3.T, vrange VersionRange,
259259
) (apiOperation, []ir.Exclusion) {
260260
// Sort ops by URL for determinism, then by operationId within the same URL
261261
// to preserve the spec's declared primary ordering (e.g. search.0 is POST,
@@ -456,6 +456,12 @@ func buildAPIOperation(group string, ops []struct {
456456
if apiOp.ResponseRef == "" {
457457
apiOp.ResponseRef = group + respBodySuffix
458458
}
459+
// Follow bare-$ref alias chains (a component schema that is just
460+
// `$ref: <other>` with no own properties) to the terminal schema key.
461+
// The walker registers the terminal type under its own key, so an
462+
// unresolved alias key would miss registry.lookup and degrade the whole
463+
// response to raw json.RawMessage.
464+
apiOp.ResponseRef = resolveSchemaAlias(apiOp.ResponseRef, spec)
459465
apiOp.ResponseSchemaRef = mt.Schema
460466
break
461467
}

0 commit comments

Comments
 (0)