Skip to content

Commit 4120f08

Browse files
sean-Jakob3xDssunno
authored
feat: Add code generation and fix double-slash URL path bug(#804)
* Fix alias/mapping/settings/block API URL path when Indices is empty (#650) When Indices is empty, path construction produced a double-slash (e.g. "//_alias/myalias") which http.NewRequest misparses per RFC 3986, treating "_alias" as the URL authority. After setReqURL overwrites the host, the path loses the API prefix entirely, causing requests to hit the wrong endpoint (e.g. creating an index instead of an alias). Extract buildPath helper that skips empty segments, eliminating the class of bug and the magic-number Grow hints. Benchmarked at 1 alloc / ~21ns vs path.Join's 3 allocs / ~63ns. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Fix double-slash URL path bug in 74 GetRequest methods (#650) When optional path segments (Indices, Index, Repo, etc.) are empty, path construction produced "//" which http.NewRequest misparses per RFC 3986, treating the next segment as the URL authority. After the transport overwrites the host, the path loses its API prefix, causing requests to hit the wrong endpoint. Extract opensearch.BuildPath helper that skips empty segments (1 alloc, ~21ns - benchmarked against path.Join's 3 allocs, ~63ns; see BenchmarkBuildPath in opensearchapi/path_test.go). Convert all 74 vulnerable GetRequest methods across opensearchapi, plugins/security, and plugins/ism to use it. Fixes: #617, #650 Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Add typed path segment types as a foundation for URL construction safety Introduce 21 domain-specific string types (Index, Indices, Action, DocumentID, Alias, Repo, Snapshot, NodeID, Plugin, Policy, Block, Prefix, Suffix, Name, Resource, Attr, Value, Metric, IndexMetric, Metrics, NodeFilter) and 25 struct-per-shape path builders. Build() methods skip empty optional segments, eliminating the double-slash URL bug (#617, #650) at the type level. Indices filters empty-string elements during comma-join so mixed slices like {"", "x", ""} produce "/x" rather than "/,x,"; required-slice fields are validated via hasNonEmpty() to reject all-empty inputs. Segment encoding (url.PathEscape) is layered on by subsequent commits in this series; the typed shapes make that layering possible. Published API types in opensearchapi/ and plugins/ remain string/ []string. Casts to domain types happen internally in GetRequest() methods, preserving backward compatibility. Fixes #617, Fixes #650 Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Add Docker image pre-pull with retry for CI resilience Add retry loop (30 attempts, 10s backoff) to pre-pull the OpenSearch base image before docker compose build. Handles transient Docker Hub 504 timeouts and BuildKit gRPC disconnects (as seen in CI failures in #804). Applied in the Makefile cluster.docker-build target so both CI and local invocations get the same protection. Makefile reliability improvements (exit-on-final-failure guard, and dropping the redundant workflow-level pre-pull step now that the Makefile target handles it) folded in from #833 review feedback by @Jakob3xD and @ssunno. Fixes: #833 Co-authored-by: Jakob <jakob.hahn@hetzner.com> Co-authored-by: ssunno <ssunno@users.noreply.github.com> Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Pool path builder buffers to eliminate per-request allocation churn Replace strings.Builder in all 25 typed path Build() methods with a sync.Pool'd []byte buffer. Each Build() acquires a pooled buffer, writes path segments via append, copies to a final string on release, and returns the buffer for reuse. Under steady state the working buffers are recycled so there is no allocation for the buffer itself. Because append handles growth, the entire pre-compute length calculation phase is no longer needed. This removes reqSegLen, optSegLen, reqSegWrite, optSegWrite, Indices.joinLen, Indices.join, Indices.optSegLen, and Indices.optSegWrite. Buffers exceeding 4 KiB are discarded rather than returned to the pool to bound memory growth from pathological inputs. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Add spec-driven path builder generator with pooled zero-alloc construction Generate type-safe path builder structs from the OpenAPI specification. Each builder validates required fields at build time and uses a sync.Pool-backed byte buffer to construct URL paths with zero per-request heap allocations. Includes benchmark tests covering single-index, multi-index, multi-segment, and static-path patterns. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Migrate all consumer files to typed path builders Replace hand-built URL paths in 146 API consumer files with generated ospath.*Path{} builders, gaining compile-time segment validation and eliminating fmt.Sprintf allocation at call sites. The typed builders accept []string for multi-value segments directly, removing the need for callers to pre-join with strings.Join. Add internal/build for [http.Request] construction without the url.Parse overhead of [http.NewRequest]. RawPath + Path are set together so url.URL.EscapedPath() honors the percent-encoded form produced by the path builders. build.Request: - Skip the default Content-Type seed when the caller supplies one, so callers that need application/x-ndjson (e.g. _bulk, _msearch) don't end up with two Content-Type values on the wire. - Reject paths with invalid percent-encoding (e.g. "%ZZ"). Falling through to "Path = raw" plus "RawPath = raw" makes EscapedPath() silently re-encode the literal '%' as "%25", corrupting the wire. - Tighten validMethod to RFC 7230 token chars only ("get", "GET,POST", "PROP[]FIND" are now rejected, matching net/http's isTokenTable). Export HeaderContentType, ContentTypeJSON, and ContentTypeNDJSON constants so generated client code can reference them by name. Remove the monolithic root-level path.go that predated the generated builders. Rewrite api_indices_response_test.go as table-driven tests. Add api_path_integration_test.go covering GetRequest() path correctness for representative API types. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Add api subcommand and restructure cmd/osgen for paths/api split Reorganize cmd/osgen so a single binary generates both typed path builders (paths subcommand) and API consumer structs (api subcommand) from the OpenAPI spec. Rename analyze.go/group.go/testgen.go to paths_analyze.go/paths_group.go/paths_testgen.go to make room for the api_*.go counterparts. Add shared naming.go for consistent acronym expansion (ID, UUID, HTTP, SQL, etc.) across both subcommands. Add internal/apiutil with FormatDuration and the shared Inspect type used by generated API consumer code. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Add emit/ template fragments and ir/ IR layer to osgen Restructure the API code generator with two new sub-packages: - cmd/osgen/emit/ holds template fragments for request structs, test scaffolding, plugin dispatch, and shared build code (~2400 lines split across frag_req.go, frag_tests.go, frag_plugin.go, frag_dispatch.go, build.go). - cmd/osgen/ir/ defines the intermediate representation that the api subcommand walks; cmd/osgen/ir_bridge.go converts the parsed OpenAPI spec into IR. Key behaviors: - Deterministic output: ops sorted by URL then operationId, with the spec's declared primary preserved as a stable tiebreaker. - Stale _gen.go files not produced by the current run are removed. - Output directories are validated against the git working tree root via os.OpenRoot to prevent accidental writes outside the repository. Drop internal/path/builders_gen.go; the next commit regenerates it through the unified osgen pipeline. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Add integration test generator and per-plugin client generation to osgen Extend the osgen api subcommand to produce integration tests for all 423 operations and per-plugin Client types with typed dispatch methods. - api_testgen_integ.go: template and classification logic for integration tests with fixture management, version skipping, and NeedImport* flags - api_render_plugin_client.go: per-plugin Client struct, generic do[T], dispatch methods, and internal/test helpers (NewClient, CreateFailingClient) - api_cmd.go: plugin detection fix (opensearchAPIPkgName vs corePkg), generation loop for plugin clients and test helpers - route.go: modulePath const, _common core group, methodNameFromSuffix - Hand-written test renames to TestManual_ prefix to avoid conflicts - VerifyInspect: require instead of assert, nil-safe Response check - testutil: fix context cancel leak in WaitForClusterReady Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Generate osapi integration tests and plugin clients from spec Output of: cmd/osgen api -spec opensearch-openapi.yaml -out osapi -pkg osapi -plugins-out osapi/plugins - 167 core integration tests (osapi/*_integ_gen_test.go) - 256 plugin integration tests (osapi/plugins/*/*_integ_gen_test.go) - 21 plugin client types with dispatch methods (client_gen.go) - 21 plugin test helpers (internal/test/helpers_gen.go) - Compat, types, unions, unit tests, and params for all 423 operations Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Use http.Status constants in production code Replace magic-number HTTP status codes with http.Status* constants in opensearchtransport/logger.go and opensearchtransport/opensearchtransport.go. The logger range checks tighten from "0-299" to "100-299" (status codes under 100 are invalid HTTP and were previously colored as success). opensearchutil/bulk_indexer.go also widens the per-item success range from {200,201} to 200-299 to match HTTP success semantics. OpenSearch bulk per-item responses are 200 or 201 in practice, so this is a no-op on the live response surface; the change closes a latent gap if a future server returns 202-299 for an accepted-but-still-in-progress item. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Use http.Status constants in test code Replace magic-number HTTP status codes with http.Status* constants in unit and integration tests across opensearchapi, opensearchtransport, and opensearchutil. Mirrors the same substitution applied to production code in the prior commit. Six files touched, no behavior change: every literal maps 1:1 to its named constant (502 -> http.StatusBadGateway, 404 -> http.StatusNotFound, etc.). Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Use http.Method and http.Status constants in samples Replace HTTP method and status-code literals in _samples/ with their http.Method* and http.Status* constants: - _samples/json.go: "GET"/"PUT"/"POST"/"DELETE" -> http.MethodGet/Put/ Post/Delete in http.NewRequest calls. - _samples/bulk.go: `resp.Status > 299` -> `resp.Status >= http.StatusMultipleChoices` (semantically identical for integers). No behavior change. The samples are reference material for end users, so the named-constant form is more instructive than literals. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Add patched OpenSearch OpenAPI specification for code generation Vendor the bundled OpenAPI spec from opensearch-api-specification (x-api-version 2.16.0) as the input for cmd/osgen. Five upstream-pending fixes are applied locally and removed from the patch set as each lands upstream: - Fix ISM index parameters to use Indices type instead of IndexName - opensearch-project/opensearch-api-specification#1098 - Fix ltr CacheStatsResponse to use NodesResponseBase via allOf - opensearch-project/opensearch-api-specification#1099 - Add missing properties to nodes.info settings schemas - opensearch-project/opensearch-api-specification#1101 - Mark create_pit keep_alive query parameter as required - opensearch-project/opensearch-api-specification#1102 - Add max_score to HitsMetadata required fields - opensearch-project/opensearch-api-specification#1103 A sixth fix (#1107, CatResponseFormat schema unification) lands in the follow-up regen commit. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Add union types, version filtering, and field constants to osgen Extend cmd/osgen with three substantial features: - Union types: api_render_union.go and resp_union.go generate Go union-shaped types from anyOf/oneOf schemas with disambiguating branch identifiers. - Version filtering: cmd/osgen/version_filter.go applies the bundled spec's x-version-added / x-version-removed extensions to emit only operations valid for the targeted OpenSearch version. - Path field constants: fieldIndex, fieldID, fieldMetric, etc. replace magic strings throughout the emitter. Regenerate internal/path/builders_gen.go and builders_gen_test.go; this is where percent-encoding (encSeg) and hasNonEmpty validation land in the generated path builders. Add internal/params/params.go for shared query parameter helpers used by generated client code. Refinements: - unionConstName always concatenates unionName + branchName + "Type" (no de-stutter): a de-stutter shortcut would let two branches in the same union ("Bar" and "FooBar" under union "Foo") collide on "FooBarType". Uniqueness wins over elegance. - mapValueTypeName recurses through nested map / slice / pointer prefixes so types like "map[string]map[string]X" or "[]*X" resolve to a valid base identifier instead of feeding "*X" or "map[string]X" into baseGoName. - deriveBranchName runs the goTypeName fallback through baseGoName so dotted package qualifiers don't leak as identifier fragments. - Exclusion guards an empty versionAdded so the breadcrumb message doesn't render "requires OpenSearch >= " (empty version). - Drop the unused (and inverted) MinSatisfied helper. Propagate the spec's IsNDJSON flag through the IR (ir.Operation, ir_bridge.go) into the Req template so generated GetRequest methods inject Content-Type: application/x-ndjson before calling build.Request for _bulk, _msearch, and _msearch/template. Caller-provided Content-Type still wins. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Document osapi rename in cmd/osgen/README.md Update the api subcommand documentation so it matches what `make gen` actually invokes: - `-out ../../opensearchapi` -> `-out ../../osapi` in both example invocations (full regen and single-operation). - `-plugins-out ../../plugins` -> `-plugins-out ../../osapi/plugins`. - Add `-pkg` to the api subcommand flag table; was previously documented for paths but absent here, so a reader following the README would generate into the wrong directory under the wrong package name. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Document osapi v5-track surface in DEVELOPER_GUIDE.md Two changes to the Code Generation section: - Update the output-path paragraph from `opensearchapi/`/`plugins/` to the actual destinations of `make gen` at this point in the branch: `osapi/` and `osapi/plugins/`. (A later commit relocates these under `v5preview/opensearchapi/`; the guide is updated again at that step.) - Add a paragraph identifying `osapi/` as the v5-track API surface that coexists with hand-written `opensearchapi/` during the v4 -> v5 transition, with pointers to `osapi/README.md` and `UPGRADING.md` for usage and migration guidance. Drive-by: trailing-whitespace realignment of the GEN_* variable table (cosmetic, no content change). Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Sync osgen flag help, godoc, and test fixtures with osapi/ output Mechanical rename of "opensearchapi" -> "osapi" in places where the generator's own metadata had drifted from the actual output package. No behavior change. - cmd/osgen/api_cmd.go: -out and -plugins-out flag help text now show the osapi/ paths; the usage error string updates likewise. - cmd/osgen/README.md: routing description points at osapi. - cmd/osgen/naming.go: pkgScopedName godoc now references the osapi package by name. Cosmetic gofmt re-alignment of scalarAliases map literals. - cmd/osgen/naming_test.go: golden expectations switch to osapi/*_gen.go paths; godoc reflects osapi/ and osapi/plugins/. - cmd/osgen/cmd_test.go: test calls pass "osapi" explicitly instead of the opensearchAPIPkgName const, and the stale-file fixture writes `package osapi` instead of `package opensearchapi`. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Regenerate osapi from osgen with applied spec patches Output of `make gen` after the union types, version filtering, and field constants added to cmd/osgen in the previous commit. The bundled spec at this point includes six upstream-pending fixes: - opensearch-project/opensearch-api-specification#1098 - opensearch-project/opensearch-api-specification#1099 - opensearch-project/opensearch-api-specification#1101 - opensearch-project/opensearch-api-specification#1102 - opensearch-project/opensearch-api-specification#1103 - opensearch-project/opensearch-api-specification#1107 The regenerated tree covers ~480 typed API operations across osapi/ and osapi/plugins/ with happy-path integration test scaffolding. Drive-by: a one-line trim in cmd/osgen/paths_render.go to keep the generator's output stable, an osapi/README.md describing the package layout, and an osapi/bench_getrequest_test.go for path-construction benchmarks. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Default integration cluster to SECURE_INTEGRATION=true The secure configuration (TLS + basic auth) is the production-realistic default. Callers who need insecure testing explicitly set SECURE_INTEGRATION=false. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Add container runtime detection and test output capture to Makefile Detect nerdctl or docker at build time (overridable via CONTAINER_RUNTIME). Pipe test output through tee to test-unit.log and test-integ.log for post-failure inspection while preserving exit codes via PIPESTATUS. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Pin CONTAINER_RUNTIME=docker in CI workflows GitHub Actions runners use docker, not nerdctl. Explicitly set the variable so the new Makefile runtime detection selects the correct backend. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Add CI workflow to verify generated code is up to date Runs make gen and surfaces a diff of the working tree afterward, catching cases where someone modifies the spec or generator without regenerating. The job is marked continue-on-error to surface staleness as a warning without blocking PRs while the generator stabilizes; flip to blocking once the regen is reliably hermetic. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Bump golangci-lint to 2.12.2 - Rename gomodguard to gomodguard_v2 in golangci-lint config. - Resolve new warnings introduced by the bump: extract lifecycle state names to lcName* constants in connection_lifecycle.go; similar magic-string extraction in policy_override_env.go and related files. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Modernize opensearchtransport with Go 1.25 idioms Use slices.Backward for reverse iteration in pool dead-list removal. Replace manual wg.Add/go/defer patterns with sync.WaitGroup.Go in tests. Drop a now-unneeded //nolint:nestif annotation. Drive-bys discovered while exercising the modernized tests: - Add nil-Header guards in setReqUserAgent and setReqGlobalHeader so synthetic *http.Request values (Header == nil) no longer panic. - Switch t.Context() to context.Background() inside t.Cleanup so the cleanup request isn't canceled by test-end before completing. - Split the api_tasks_test.go cleanup so a require-failure Goexit in the task-cancel cleanup doesn't skip the index-delete cleanup. - Skip TestTasksClient on OpenSearch <2.18 (.tasks index lacked cancellation_time_millis and resource_stats fields, OpenSearch#16201). Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Extract shared integration helpers and add version guards to shard routing tests - Add opensearchtransport/integration_test_helpers_test.go for helpers shared across the shard routing integration tests (selector readiness predicates, connection-count requirements, etc.). - Expand opensearchtransport/testutil/helpers.go: ConnPollOpts + DefaultConnPollOpts + RequireMinConns for parameterized polling, and thread an explicit context.Context through RequireMinNodes (callers pass t.Context() or another scope-appropriate ctx). - Skip shard routing tests on OpenSearch <2.0 where routing_num_shards is unavailable from _cluster/state/metadata. - Replace magic 500 with http.StatusInternalServerError throughout the shard routing tests. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Add TestFlag bitfield framework for integration test behavior Replace the ad-hoc TestVersionOverrides map and integSkipReason() function with declarative TestRule records (glob NamePattern, Version constraint, TestFlag bitfield). The bitfield form lets a single rule both skip on old versions and inject a cluster-readiness poll, avoiding the previous duplication and making it cheaper to add new test-time behaviors (timeouts, fixture setup, etc.) without touching the emitter. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Regenerate integration tests with TestFlag framework Output of `make gen` after the prior commit added the TestFlag bitfield framework. The framework's TestRule table emits two kinds of additions into existing _integ_gen_test.go files: - WaitForAllNodesReady prologue for tests sensitive to lagging node registration: cat-nodes, nodes-info, nodes-stats. - t.Skip directives with bug links for tests that depend on fixtures the integration cluster doesn't provide: - delete_all_pits - malformed error body when no PITs exist (OpenSearch#11711) - field_caps - requires an index with mapped fields - get_script_languages - KNN plugin NPE in getSupportedContexts (k-NN#560) Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Add Response.RawBody for buffered body access and modernize error tests Response gains a rawBody field, RawBody() accessor, HijackBody() ownership-transfer accessor, and NewResponse() constructor so generated response types can buffer and re-expose the raw bytes without consuming the stream. RawBody returns a pointer-stable view of the buffer for inspection; HijackBody clears r.rawBody and transfers ownership to the caller for log/background-process scenarios that outlive the Response. error_test.go is rewritten: - switch from assert to require so a broken expectation halts the subtest before subsequent dereferences panic - consolidate around table-driven cases for the multi-shape error bodies (StructError, StringError, error.cause chains, etc.) Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Add unified security guide and eliminate raw JSON injection patterns Add guides/security.md covering TLS, credential management, path-param validation, request body construction, error-handling/info-disclosure, and transport configuration. Replace fmt.Sprintf-based JSON construction in guides/search.md and guides/tasks.md with opensearchutil.NewJSONReader so the examples demonstrate the safe pattern. Add cross-reference notes to the other existing guides pointing at security.md. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Harden link checker with retries and exclude flaky badge URL Add --max-retries=3 --retry-wait-time=10 to the lychee args so transient 504s from upstream don't fail the run, and exclude the codecov badge URL which intermittently 502s. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Add cmd/osgen coverage upload to unit CI workflow Bump Go CI timeouts to exceed the internal FSM timeouts (~6.5min) Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Add golden workflow integration tests for core osapi operations Hand-written table-driven tests covering Bulk, Count, Cat, Alias, Settings, Mapping, Scroll, IndexTemplate, PIT, Aggregation, DocumentGet, Mget, Update, and DeleteByQuery. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Add per-directory codecov coverage targets The osapi/ package contains ~194k lines of generated code covering every API in the OpenSearch spec. Coverage sits at ~25% because many generated APIs target plugin-specific endpoints (ISM, anomaly detection, k-NN, security, notifications) that require plugins not present in the CI cluster. Unit-level roundtrip tests exercise request construction and dispatch for all 167 APIs, but full response parsing coverage requires integration tests against a cluster with the relevant plugins enabled. Expanding the CI cluster configuration and adding plugin-aware integration tests would raise this target over time. Component-level targets: core (transport, utils, signer): 80% - mature, hand-written code internal/path: 95% - small, fully testable osapi: 25% - limited by CI plugin set opensearchapi (legacy): 14% - frozen, being replaced Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Use ospath builders instead of fmt.Sprintf for URL construction in tests Replace all fmt.Sprintf URL path construction with typed ospath builders (IndicesCreatePath, IndicesDeletePath, SearchPath, SearchShardsPath, IndexPath, ClusterHealthPath, ClusterStatePath) for compile-time safety and proper percent-encoding of path segments. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Add internal/test/readiness package for layered FSM-based test gating Each per-node FSM packs an ordinal Layer (lower 16 bits, cumulative) and a free-form ClientState (upper 16 bits) into one atomic uint32 so the satisfaction check collapses to s & target == target. Layers gate each other (no LayerHTTP without LayerTCP); ClientState bits flap freely. Per-layer budgets are tuned for CI pessimism. NodeFSM.Advance captures the caller's intent at the first state observation: a forward call (next > initialLayer) that loses the CAS to a peer that already advanced past next is a silent no-op, not a regression. Without this, two concurrent forward Advance calls would mis-classify the loser's retry against a now-higher layer as a deliberate regression and trip MaxRegressions. Cluster.lastPolledAt sits inside c.mu alongside lastErr/lastResp/polls with SetLastPolledAt/LastPolledAt accessors so the diagnostic dump reads a consistent snapshot. NodeFSM.Snapshot returns state, enteredAt, regressions, and history under the FSM's mu so renderNode emits a coherent per-node line. The old four independent loads could skew across a concurrent transition mid-render. The misconfig guard fails fast on any non-zero target without a LayerCheck or FSMCheck (was: only layer-bearing targets), so a state-only target with no driver no longer silently times out. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Replace WaitForAllNodesReady inline polling with readiness.Wait The previous implementation polled cat-nodes for 60s with a single boolean predicate, producing "Condition never satisfied" on timeout. Switch to readiness.Wait(t, ctx, TargetClusterReady, WithCluster(c)) which observes per-node progression through LayerHTTP, LayerClusterJoin, and LayerStatsReady, and dumps a structured per-node diagnostic with the full last cat-nodes response on failure. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Preserve in-flight shards in shardMap across /_cat/shards relocation window /_cat/shards parsing skips RELOCATING and INITIALIZING rows, so a shard mid-relocation is absent from placement.ShardToNodes for the duration of the relocation window. The previous unconditional store would clobber a complete cached map with this partial response, leaving Shards[N] == nil for the in-flight shard and disabling shard-exact routing for it. Carry forward entries the new response is missing: a shard in RELOCATING state still serves reads from the source until the destination flips to STARTED, so the prior entry remains a valid routing target for the relocation window. New data wins where present. Use CompareAndSwap with a bounded retry to bound the visible Load+merge+Store window: a CAS loss can drop a strictly fresher in-flight observation, so cheap retries (3 attempts) are preferred over a single attempt with silent loss. Log on retry exhaustion to surface persistent contention from a hot health-check goroutine; the next merge cycle will self-heal. Compute NumberOfPrimaryShards from the cross-state primary set (counting distinct shard numbers across STARTED + RELOCATING + INITIALIZING rows) rather than from ShardToNodes alone. ShardToNodes is filtered to STARTED entries, so a relocating shard would otherwise undercount the primary count and cause the post-merge prune to drop the very entry that carry-forward just preserved. The prune now fires only when the placement covers every primary shard (a real shrink), never on a partial view. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Mark singleServerPool's connection lcActive at construction Route all four singleServerPool construction sites through a newSingleServerPool helper that CAS's lcActive (clearing lcUnknown|lcStandby) under conn.mu; without it the no-op OnSuccess/OnFailure leave the connection lcUnknown for the lifetime of the pool, so observers classify the only available connection as not-ready. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Set lcReady when warmup completes guides/routing.md documents the post-warmup state as lcReady|lcActive but tryWarmupSkip's completion path only cleared lcNeedsWarmup, leaving lcReady unset for the lifetime of the connection. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Bump AWS SDK deps config v1.32.17 -> v1.32.18 credentials v1.19.16 -> v1.19.17 ssooidc v1.35.21 -> v1.36.0 Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Mark ToPointer() in advance of v5 Add a package-local unexported ptr() helper to the opensearch and opensearchapi packages and route ToPointer through it. ToPointer is flagged Deprecated; the helper is removed in v5. Once the module's go directive moves to 1.26, ptr() itself can be deleted and call sites can switch to the native new(value) form (e.g. new(false)). Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Parallelize osgen generated output - Target rendering uses runtime.NumCPU() goroutines with sorted output for determinism. - Added sync.WaitGroup.Go() workers in api_cmd renderloop - Split Makefile gen into gen-paths and gen-api targets for make -j gen support Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Build bool query params as *bool - All 352 boolean query parameters now emit as *bool, enabling callers to send both true and false values. Fixes inability to override server-default-true params (wait_for_completion, flush, etc). - classifyParamSchema returns "*bool" instead of "bool" for booleans - Encoder: if r.X != nil { set("name", strconv.FormatBool(*r.X)) } - All test expectations updated Fixes: #840 Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Lint cmd/osgen as a separate Go module Add a `cmd/osgen/` exclusion to .golangci.yml's goconst rule (the generator uses many short repeated literal strings as identifiers and import paths inside template input that don't benefit from const extraction). Extend the `lint.local` Makefile target to run golangci-lint inside cmd/osgen too, since it's a separate module not covered by the root invocation. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Tighten path builder generator: visibility, determinism, error paths - F18: emit opExplainCheck for case=1 + positional deps, closing a silent-fall-through path the explainXxx helper was meant to catch. - F24: unexport the path.Builder interface; the assertions are intra-package and no external caller writes path.Builder. - F25: drop the unused paramSet accumulator in pathParamInfo. - F26: iterate spec.Paths and per-path Operations() in sorted order so description, versionAdded, and docsURL captured for multi-method URLs are deterministic instead of depending on map iteration order. - F27: panic on unknown HTTP methods in httpMethodConst instead of silently falling through to MethodGet, so a spec typo or new verb fails the generator loudly. - F36: clarify alwaysImplies vacuous-case behavior; the function intentionally returns false when no path contains the dependent. Regenerated internal/path/builders_gen.go and builders_gen_test.go. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Generate per-required-field error cases and *bool false-case in path tests Improve generator-emitted test coverage: - Path test emit (cmd/osgen/paths_testgen.go): for builders with 2+ required fields, emit a "missing <Field>" case per required field with the others set; for builders with list fields, emit a multi-value case for each list field, not just the first; for builders with only optional fields, emit "only <FieldName>" cases per field. Replace require.Error with require.ErrorIs against errRequired so the public error contract is verified, not just presence. - Params test emit (cmd/osgen/emit/build.go, frag_tests.go): emit two scenarios for each *bool query param ("=true" and "=false") so the wire-level encoding of an explicit false is exercised. Regenerate internal/path/builders_gen_test.go and the osapi/*_internal_gen_test.go family. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Strengthen assertPathRoundTrip URL invariant checks Replace the per-segment self-consistency round-trip with a stricter helper that catches structural bugs the encoder is meant to prevent: - url.ParseRequestURI must succeed (valid request-URI shape). - Path must not contain "//" (the original #650 double-slash bug). - Decoded segments must not still decode further (no double-encoding). - Decoded segments must not contain "/", "?", "#" (path injection). - Stable encode-decode round-trip on every sub-segment. Used by every generated builder test. Catches encoder regressions that the previous self-consistency check would have silently accepted. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Run golangci-lint --fix on cmd/osgen and remove dead render pipeline - Run golangci-lint --fix across cmd/osgen (a separate Go module). Mostly comment additions and minor formatting; no behavior change. - Remove the dead pre-emit/ render pipeline: cmd/osgen/api_render.go (843 lines) cmd/osgen/api_render_plugin_client.go (207 lines) cmd/osgen/api_testgen.go (448 lines) cmd/osgen/api_testgen_integ.go (421 lines) cmd/osgen/render_integration_test.go (596 lines, the only consumer) These were superseded by cmd/osgen/emit/. The active code path runs through emit.Build; the deleted files were called only from the legacy integration test that goes with them. resolveFieldPath, the only function that survived as a live dependency from this set, is preserved next to subClientHierarchy in cmd/osgen/route.go. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Fix reqXxxReq concatenation in dispatch fragment template The dispatch template at cmd/osgen/emit/frag_dispatch.go used `{{- ""}}` followed by a `{{- if ...}}` to strip whitespace, which also stripped the space between "req" and the type name. The generator emitted "func (c Client) Bulk(ctx context.Context, reqBulkReq)" - a syntactically invalid parameter list - whenever IsPointerReq was false (value-receiver Req struct). Replace the multi-line whitespace-stripping construct with a single inline `{{if $.IsPointerReq}}*{{end}}` so the template emits "req X" for value Req and "req *X" for pointer Req correctly. The bug only surfaced when generating the value-receiver dispatch path (Bulk, Create, Delete, etc.); pointer-receiver Req types worked because the leading `*` masked the missing space. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Move osapi to v5preview/opensearchapi Relocate the generated v5-track API surface from `osapi/` to `v5preview/opensearchapi/` ahead of merging this PR. Two motivations: - Signal preview status: the `v5preview/` parent directory makes the package's stability guarantee explicit at the import path. Callers importing `.../v5preview/opensearchapi` cannot mistake it for the stable v5 surface. - Reserve the canonical name: the package is already named `opensearchapi` (matching the legacy hand-written package). Once v5 is cut and the legacy package is removed, the v5preview/ prefix gets dropped and consumers move with a single import-path edit; the type qualifiers (`opensearchapi.IndexReq`, etc.) stay constant. Generator changes that make the relocation a one-line config swap later: - cmd/osgen/ir/defaults.go (new): centralizes ModulePath, DefaultCorePkgName, DefaultCoreSubpath, DefaultCoreImportPath, and DefaultPluginsImportBase. Promoting v5preview/opensearchapi to the module root reduces to editing DefaultCoreSubpath. - cmd/osgen/emit/build.go: introduces coreImportPath() to centralize import-path resolution; importPathForGroup and the dispatch test emitter route through it. - Plugin test helper directory now uses `internal/<pkg>test` instead of `internal/test`, so the helper subpackage is unique per plugin (avoids collisions when multiple generated packages share the same module). Companion edits across CHANGELOG.md, UPGRADING.md, DEVELOPER_GUIDE.md, .codecov.yml, .github/workflows/check-gen.yml, .golangci.yml, Makefile, guides/security.md, and cmd/osgen/README.md update path references; the file moves themselves are pure git renames preserving history. The follow-up commit regenerates the package contents at the new location. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Regenerate osapi as v5preview/opensearchapi Pure regeneration output: `make gen` after the prior commit moved the package and updated cmd/osgen's defaults to emit v5preview/opensearchapi/. No semantic change. The 6042/6042 line balance reflects two mechanical substitutions across 1590 generated files: - `package osapi` -> `package opensearchapi` (and `osapi_test` -> `opensearchapi_test` for external test files). - Import path `github.com/opensearch-project/opensearch-go/v4/osapi` -> `.../v4/v5preview/opensearchapi`, with the qualifier `osapi.` -> `opensearchapi.` updated at every reference inside the regenerated test fixtures. No hand-edits in this commit; any review should focus on the prior move/codegen-config commit. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Fix data race in pooledConns Release-then-read test The "release clears pointer references across full capacity" subtest read the backing array after b.Release(), racing with parallel rendezvousTopK calls that re-acquired the same buffer from connSlicePool. Factor the clear-and-reset logic into a shared clearConns helper and test it directly; same invariant coverage with no use-after-Release. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> * Set ?timeout=10s on cat-nodes readiness probe clusterLensFSMCheck polls cat-nodes to advance LayerStatsReady. Without ?timeout=, cat-nodes' inner NodesInfo+NodesStats RPCs (server-side at RestNodesAction.java:128,140) inherit the unbounded request default. A freshly joined node whose first stats cycle stalls leaves the row with cpu=null, heap.percent=null indefinitely, and the readiness gate hangs until the per-layer budget elapses; CI run 26578837681 hit this on 2.13.0 insecure with TestNodesInfo + TestNodesStats consuming the full package timeout. 10s bounds each server-side fan-out and lets the Go-side polling cadence drive retries instead of relying on individual server-side calls to complete. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> --------- Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> Co-authored-by: Jakob <jakob.hahn@hetzner.com> Co-authored-by: ssunno <ssunno@users.noreply.github.com>
1 parent b6134ac commit 4120f08

2,157 files changed

Lines changed: 321283 additions & 4160 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.

.ci/opensearch/docker-compose.yml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ services:
99
context: .
1010
dockerfile: Dockerfile.opensearch
1111
args:
12-
- SECURE_INTEGRATION=${SECURE_INTEGRATION:-false}
12+
- SECURE_INTEGRATION=${SECURE_INTEGRATION:-true}
1313
- OPENSEARCH_VERSION=${OPENSEARCH_VERSION:-latest}
1414
- OPENSEARCH_INITIAL_ADMIN_PASSWORD=myStrongPassword123!
1515
environment:
@@ -55,7 +55,7 @@ services:
5555
context: .
5656
dockerfile: Dockerfile.opensearch
5757
args:
58-
- SECURE_INTEGRATION=${SECURE_INTEGRATION:-false}
58+
- SECURE_INTEGRATION=${SECURE_INTEGRATION:-true}
5959
- OPENSEARCH_VERSION=${OPENSEARCH_VERSION:-latest}
6060
- OPENSEARCH_INITIAL_ADMIN_PASSWORD=myStrongPassword123!
6161
environment:
@@ -101,7 +101,7 @@ services:
101101
context: .
102102
dockerfile: Dockerfile.opensearch
103103
args:
104-
- SECURE_INTEGRATION=${SECURE_INTEGRATION:-false}
104+
- SECURE_INTEGRATION=${SECURE_INTEGRATION:-true}
105105
- OPENSEARCH_VERSION=${OPENSEARCH_VERSION:-latest}
106106
- OPENSEARCH_INITIAL_ADMIN_PASSWORD=myStrongPassword123!
107107
environment:

.codecov.yml

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,5 +11,29 @@ coverage:
1111
status:
1212
project:
1313
default:
14-
threshold: 5% # allow 5% coverage decrease
14+
target: auto
15+
threshold: 5%
16+
core:
17+
paths:
18+
- "opensearch*.go"
19+
- "opensearchtransport/"
20+
- "opensearchutil/"
21+
- "signer/"
22+
target: 80%
23+
threshold: 2%
24+
path:
25+
paths:
26+
- "internal/path/"
27+
target: 95%
28+
threshold: 2%
29+
opensearchapi:
30+
paths:
31+
- "opensearchapi/"
32+
target: 14%
33+
threshold: 2%
34+
opensearchapiv5:
35+
paths:
36+
- "v5preview/opensearchapi/"
37+
target: 25%
38+
threshold: 5%
1539
patch: off

.github/workflows/check-gen.yml

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
name: Check generated code
2+
3+
on: [push, pull_request]
4+
5+
jobs:
6+
check-gen:
7+
name: Generated code up to date
8+
runs-on: ubuntu-latest
9+
continue-on-error: true
10+
steps:
11+
- uses: actions/checkout@v6
12+
- uses: actions/setup-go@v6
13+
with: { go-version-file: 'go.mod' }
14+
- name: Fetch OpenAPI spec
15+
run: make fetch-opensearch-spec
16+
- name: Regenerate code from spec
17+
run: make -j gen
18+
- name: Check for diff
19+
run: |
20+
if ! git diff --exit-code internal/path/builders_gen.go internal/path/builders_gen_test.go v5preview/opensearchapi/; then
21+
echo "::warning::Generated code is out of date. Run 'make gen' and commit the result."
22+
exit 1
23+
fi

.github/workflows/links.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ jobs:
1313
id: lychee
1414
uses: lycheeverse/lychee-action@a8c4c7cb88f0c7386610c35eb25108e448569cb0 # v2.7.0
1515
with:
16-
args: --accept=200,403,429 "**/*.html" "**/*.md" "**/*.txt" "**/*.json" --exclude "https://opensearch-domain.region.com/" --exclude "https://github.com/\[your*" --exclude "https://localhost:9200"
16+
args: --accept=200,403,429 --max-retries=3 --retry-wait-time=10 "**/*.html" "**/*.md" "**/*.txt" "**/*.json" --exclude "https://opensearch-domain.region.com/" --exclude "https://github.com/\[your*" --exclude "https://localhost:9200" --exclude "https://codecov.io/gh/opensearch-project/opensearch-go/branch/main/graph/badge.svg"
1717
env:
1818
GITHUB_TOKEN: ${{secrets.GITHUB_TOKEN}}
1919
- name: Fail if there were link errors

.github/workflows/lint.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ jobs:
1616
- name: golangci-lint
1717
uses: golangci/golangci-lint-action@82606bf257cbaff209d206a39f5134f0cfbfd2ee # v9
1818
with:
19-
version: v2.11.4
19+
version: v2.12.2
2020
args: --fix --build-tags "integration core plugins plugin_security plugin_index_management multinode"
2121

2222
prettify:

.github/workflows/test-compatibility.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ on: [push, pull_request]
44

55
env:
66
OPENSEARCH_NODE_COUNT: 3
7+
CONTAINER_RUNTIME: docker
78

89
jobs:
910
integ-test-compat:

.github/workflows/test-integration-unreleased.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@ jobs:
9191
env:
9292
OPENSEARCH_GO_SKIP_JSON_COMPARE: true
9393
OPENSEARCH_NODE_COUNT: 1
94+
SECURE_INTEGRATION: false
9495

9596
- name: Save server logs
9697
if: failure()

.github/workflows/test-integration.yml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ env:
66
GITHUB_ACTIONS: true
77
OPENSEARCH_VERSION: 3.6.0
88
OPENSEARCH_NODE_COUNT: 3
9+
SECURE_INTEGRATION: true
10+
CONTAINER_RUNTIME: docker
911

1012
jobs:
1113
test:

.github/workflows/test-unit.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ jobs:
2525
- uses: codecov/codecov-action@75cd11691c0faa626561e295848008c8a7dddffe # v5
2626
with:
2727
token: ${{ secrets.CODECOV_TOKEN }}
28-
files: tmp/unit.cov
28+
files: tmp/unit.cov,tmp/osgen.cov
2929
flags: unit
3030
if: matrix.os == 'ubuntu-latest'
3131

.gitignore

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
tmp/
22
*.test
3+
coverage.out
4+
test-*.log
35

46
# editors
57
.idea
@@ -10,3 +12,6 @@ bin/
1012
# Admin certificates for testing
1113
admin.pem
1214
admin.key
15+
16+
# Compiled generator binaries
17+
cmd/osgen/osgen

0 commit comments

Comments
 (0)