Add generic Do[T] function for compile-time pointer enforcement by sean- · Pull Request #809 · opensearch-project/opensearch-go

sean- · 2026-03-24T13:34:18Z

Add generic Do[T] function for compile-time pointer enforcement

Client.Do() accepts any for its dataPointer parameter, deferring
pointer validation to json.Unmarshal at runtime. Add a top-level
opensearch.DoT function whose *T signature catches non-pointer
arguments at compile time. The generic wrapper is trivially inlined
by the compiler — zero runtime overhead.

Mark Client.Do() with a Deprecated doc annotation to nudge callers
toward the safer alternative via staticcheck SA1019 and IDE tooling.
The method remains fully functional and will not be removed.

Convert the unexported do method in opensearchapi, plugins/security,
and plugins/ism to package-level generic functions, enforcing pointer
safety across all ~200 internal call sites. Requests that expect no
response body use a separate doRequest() function.

Fixes: #808

Signed-off-by: Sean Chittenden sean.chittenden@crowdstrike.com

codecov · 2026-03-24T13:35:56Z

Codecov Report

❌ Patch coverage is 95.39171% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.91%. Comparing base (1dbe719) to head (d1098ad).

Files with missing lines	Patch %	Lines
opensearchapi/api_cluster.go	85.71%	2 Missing ⚠️
opensearchapi/opensearchapi.go	60.00%	1 Missing and 1 partial ⚠️
plugins/ism/api.go	60.00%	1 Missing and 1 partial ⚠️
plugins/security/api.go	60.00%	1 Missing and 1 partial ⚠️
opensearchapi/api_cat.go	95.00%	1 Missing ⚠️
opensearchapi/api_nodes.go	80.00%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #809   +/-   ##
=======================================
  Coverage   72.90%   72.91%           
=======================================
  Files         428      428           
  Lines       15647    15648    +1     
=======================================
+ Hits        11408    11410    +2     
+ Misses       2600     2597    -3     
- Partials     1639     1641    +2

Flag	Coverage Δ
integration	`65.74% <94.47%> (-0.01%)`	⬇️
unit	`43.75% <2.48%> (+<0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
opensearch.go	`93.58% <100.00%> (+0.16%)`	⬆️
opensearchapi/api_aliases.go	`100.00% <100.00%> (ø)`
opensearchapi/api_bulk.go	`100.00% <100.00%> (ø)`
opensearchapi/api_component_template.go	`100.00% <100.00%> (ø)`
opensearchapi/api_dangling.go	`100.00% <100.00%> (ø)`
opensearchapi/api_datastream.go	`100.00% <100.00%> (ø)`
opensearchapi/api_document.go	`100.00% <100.00%> (ø)`
opensearchapi/api_index.go	`100.00% <100.00%> (ø)`
opensearchapi/api_index_template.go	`100.00% <100.00%> (ø)`
opensearchapi/api_indices-alias.go	`99.00% <100.00%> (ø)`
... and 53 more

... and 4 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Add opensearch.Do[T]() that enforces pointer response types at compile time, preventing a class of bugs where non-pointer values silently fail JSON unmarshaling at runtime. Mark Client.Do() as deprecated in favor of the generic alternative. Ref: opensearch-project#809 Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com>

@tender-barbarian

Address PR opensearch-project#809 review feedback from @tender-barbarian: - Add a nil check in Do[T] before widening dataPointer to any. A typed nil (e.g. (*MyResp)(nil)) produces a non-nil interface that bypasses the guard in Client.Do and reaches json.Unmarshal. Forwarding as untyped nil lets Client.Do skip unmarshalling as intended. - Fix the "Generic Do() nil pointer" test to use Path "/" so the mock returns a JSON body. Previously the empty path produced a nil body, causing unmarshal to be skipped regardless of the nil guard. - Use t.Context() instead of context.TODO() in both generic Do tests. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com>

Jakob3xD · 2026-04-30T08:27:14Z

Could it be an alternative to introduce a 'NoBody' type of type nil, that we use for endpoints that return nothing? So we don't need the additional doRequest() function?

  // NoBody is a marker type for do() calls that expect no response body.                                                                                                                      
  type NoBody struct{}                                                                                                                                                                         
                                                                                                                                                                                               
  // noBody is a pre-declared typed nil — passed to do() when unmarshaling                                                                                                                     
  // should be skipped. Type inference resolves T=NoBody; the nil pointer                                                                                                                      
  // triggers the dataPointer == nil guard in opensearch.Do, skipping json.Unmarshal.                                                                                                          
  var noBody *NoBody

Introduce opensearch.NoBody as a sentinel type for Do[T] calls that expect no response body. Replace the separate doRequest() function in opensearchapi with do(ctx, c, req, noBody), routing all internal dispatch through a single generic path. This eliminates the dual-function split and leverages the existing nil-pointer guard in opensearch.Do to skip json.Unmarshal when the typed-nil noBody sentinel is passed. Resolves review feedback from opensearch-project#809. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com>

sean- · 2026-04-30T14:19:27Z

Could it be an alternative to introduce a 'NoBody' type of type nil, that we use for endpoints that return nothing? So we don't need the additional doRequest() function?

Oh, that's a good idea. Yeah, that removes the doRequest/do[T] split and yields a single dispatch path through the generic function. Cleaner and harder to misuse. Can you PTAL?

Thanks for the nudge on this, @Jakob3xD.

Jakob3xD

I would suggest waiting for a go release that allow generics

OpenSearch < 2.2.0 with the security plugin throws java.io.OptionalDataException on shard-routed requests due to non-thread-safe HashSet/HashMap in User serialization. Fixed in 2.2.0 by opensearch-project/security#1970 (50a94b47). Widen the existing 2.1.0-only skip to cover all versions below 2.2.0. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com>

Client.Do() accepts `any` for its dataPointer parameter, deferring pointer validation to json.Unmarshal at runtime. Add a top-level opensearch.Do[T]() function whose *T signature catches non-pointer arguments at compile time. The generic wrapper is trivially inlined by the compiler -- zero runtime overhead. Mark Client.Do() with a Deprecated doc annotation to nudge callers toward the safer alternative via staticcheck SA1019 and IDE tooling. The method remains fully functional and will not be removed. Convert the unexported `do` method in opensearchapi, plugins/security, and plugins/ism to package-level generic functions, enforcing pointer safety across all internal call sites. Fixes: opensearch-project#808 Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com>

Replace magic status codes with http.StatusOK constants, use t.Context() instead of manual context.WithCancel, and prefer require.Len/require.Positive over less specific assertions. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com>

Replace require.NotNil/require.Nil with the semantically correct error assertion helpers. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com>

Introduce opensearch.NoBody as a marker type for Do[T] calls that expect no response body, replacing the separate doRequest() function with a unified dispatch path through the generic function. Guard Do[T] against typed-nil pointers that would otherwise widen into a non-nil any interface and reach json.Unmarshal. When dataPointer is nil and the error response has no body, return a quoted-status error instead of the misleading ErrUnexpectedEmptyBody. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com>

sean- · 2026-05-04T18:26:46Z

I would suggest waiting for a go release that allow generics

go.mod already requires Go 1.25.9, which has full generics support including generic type aliases. Is there a specific language feature you're thinking of that isn't available yet?

Jakob3xD · 2026-05-05T05:45:38Z

go.mod already requires Go 1.25.9, which has full generics support including generic type aliases. Is there a specific language feature you're thinking of that isn't available yet?

I meant the “Generic Methods for Go” that is planned for GO 1.27. With that we could have just adjusted the Do Functions with a generic one.

However, as we don't try to run this lib on the latest go version, we can merge this one and maybe have a look later on.

sean- requested review from Jakob3xD, VachaShah and VijayanB as code owners March 24, 2026 13:34

sean- force-pushed the gh-808 branch 3 times, most recently from 5808bb9 to 6f3f8de Compare April 16, 2026 17:29

VijayanB previously approved these changes Apr 16, 2026

View reviewed changes

sean- dismissed VijayanB’s stale review via b7f14b5 April 16, 2026 19:28

sean- force-pushed the gh-808 branch from 6f3f8de to b7f14b5 Compare April 16, 2026 19:28

sean- mentioned this pull request Apr 21, 2026

Call for New Maintainers — Help Wanted #813

Closed

tender-barbarian suggested changes Apr 22, 2026

View reviewed changes

Comment thread opensearch.go

tender-barbarian reviewed Apr 22, 2026

View reviewed changes

Comment thread opensearch_internal_test.go Outdated

tender-barbarian suggested changes Apr 22, 2026

View reviewed changes

sean- requested review from VijayanB and tender-barbarian April 22, 2026 22:19

tender-barbarian approved these changes Apr 23, 2026

View reviewed changes

sean- force-pushed the gh-808 branch from 73d494a to bfa6d62 Compare April 30, 2026 14:13

Jakob3xD requested changes May 4, 2026

View reviewed changes

Comment thread opensearchtransport/coverage_async_internal_test.go

Comment thread opensearchtransport/discovery_internal_test.go

Comment thread plugins/security/api.go

Comment thread error_test.go

Comment thread plugins/ism/api.go

Comment thread opensearchapi/opensearchapi.go

sean- added 4 commits May 4, 2026 11:18

Improve transport test assertions

808bd5c

Replace magic status codes with http.StatusOK constants, use t.Context() instead of manual context.WithCancel, and prefer require.Len/require.Positive over less specific assertions. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com>

Use require.Error/require.NoError in ParseError test

9b8e803

Replace require.NotNil/require.Nil with the semantically correct error assertion helpers. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com>

sean- force-pushed the gh-808 branch from bfa6d62 to d1098ad Compare May 4, 2026 18:22

Jakob3xD approved these changes May 5, 2026

View reviewed changes

Jakob3xD merged commit 983f252 into opensearch-project:main May 5, 2026
75 checks passed

sean- deleted the gh-808 branch May 5, 2026 05:47

Navigation Menu

Uh oh!

Add generic Do[T] function for compile-time pointer enforcement - #809

Add generic Do[T] function for compile-time pointer enforcement#809
Jakob3xD merged 5 commits into
opensearch-project:mainfrom
sean-:gh-808

sean- commented Mar 24, 2026

Uh oh!

codecov Bot commented Mar 24, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Jakob3xD commented Apr 30, 2026

Uh oh!

sean- commented Apr 30, 2026 •

edited

Loading

Uh oh!

Jakob3xD left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sean- commented May 4, 2026

Uh oh!

Jakob3xD commented May 5, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

sean- commented Mar 24, 2026

Uh oh!

codecov Bot commented Mar 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Jakob3xD commented Apr 30, 2026

Uh oh!

sean- commented Apr 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Jakob3xD left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sean- commented May 4, 2026

Uh oh!

Jakob3xD commented May 5, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov Bot commented Mar 24, 2026 •

edited

Loading

sean- commented Apr 30, 2026 •

edited

Loading