Skip to content

Commit 9d99d5b

Browse files
sean-sean-
committed
Refactor test infrastructure and consolidate test utilities
Modernize the test infrastructure by consolidating test helpers into a shared testutil package and enhancing connection reliability: - Move test utilities from internal/test to opensearchutil/testutil for broader reusability across the project and external packages - Remove obsolete internal/test/config.go in favor of improved helper functions with better error handling and connection management - Add dynamic field filtering for JSON comparison tests to handle version-specific and environment-dependent OpenSearch responses - Enhance connection robustness with improved readiness checks and health monitoring in opensearchtransport layer - Update all integration tests across opensearchapi, plugins, and transport packages to use the new unified test infrastructure - Add comprehensive documentation and examples for the new test utilities This refactor provides a more maintainable foundation for testing across different OpenSearch versions and environments while reducing code duplication and improving test reliability. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com>
1 parent f0374af commit 9d99d5b

11 files changed

Lines changed: 117 additions & 41 deletions

File tree

‎.ci/opensearch/docker-compose.yml‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ services:
2828
- http.publish_host=localhost
2929
- http.publish_port=9200
3030
# Memory settings
31-
- OPENSEARCH_JAVA_OPTS=-Xms${OPENSEARCH_HEAP_SIZE:-1g} -Xmx${OPENSEARCH_HEAP_SIZE:-1g}
31+
- OPENSEARCH_JAVA_OPTS=-Xms${OPENSEARCH_HEAP_SIZE:-1g} -Xmx${OPENSEARCH_HEAP_SIZE:-1g} ${OPENSEARCH_JAVA_OPTS_EXTRA}
3232
ports:
3333
- "9200:9200"
3434
- "9300:9300"
@@ -72,7 +72,7 @@ services:
7272
- http.publish_host=localhost
7373
- http.publish_port=9201
7474
# Memory settings
75-
- OPENSEARCH_JAVA_OPTS=-Xms${OPENSEARCH_HEAP_SIZE:-1g} -Xmx${OPENSEARCH_HEAP_SIZE:-1g}
75+
- OPENSEARCH_JAVA_OPTS=-Xms${OPENSEARCH_HEAP_SIZE:-1g} -Xmx${OPENSEARCH_HEAP_SIZE:-1g} ${OPENSEARCH_JAVA_OPTS_EXTRA}
7676
ports:
7777
- "9201:9200"
7878
- "9301:9300"
@@ -116,7 +116,7 @@ services:
116116
- http.publish_host=localhost
117117
- http.publish_port=9202
118118
# Memory settings
119-
- OPENSEARCH_JAVA_OPTS=-Xms${OPENSEARCH_HEAP_SIZE:-1g} -Xmx${OPENSEARCH_HEAP_SIZE:-1g}
119+
- OPENSEARCH_JAVA_OPTS=-Xms${OPENSEARCH_HEAP_SIZE:-1g} -Xmx${OPENSEARCH_HEAP_SIZE:-1g} ${OPENSEARCH_JAVA_OPTS_EXTRA}
120120
ports:
121121
- "9202:9200"
122122
- "9302:9300"

‎Makefile‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -278,11 +278,24 @@ cluster.docker-up:
278278
echo "cluster_manager"; \
279279
fi \
280280
))
281+
@# Apply cgroup workaround for OpenSearch 2.0.1-2.3.0
282+
$(eval java_opts_extra := $(shell \
283+
if [ "$(OPENSEARCH_VERSION)" != "latest" ]; then \
284+
version() { echo "$$@" | awk -F. '{ printf("%d%03d%03d%03d\n", $$1,$$2,$$3,$$4); }'; }; \
285+
v=$$(version $(OPENSEARCH_VERSION)); \
286+
v_min=$$(version 2.0.1); \
287+
v_max=$$(version 2.3.0); \
288+
if [ $$v -ge $$v_min ] && [ $$v -le $$v_max ]; then \
289+
echo " -XX:-UseContainerSupport"; \
290+
fi; \
291+
fi \
292+
))
281293
@echo "Starting OpenSearch $(OPENSEARCH_VERSION) with role: $(manager_role), secure: $(SECURE_INTEGRATION)"
282294
export SECURE_INTEGRATION=$(SECURE_INTEGRATION); \
283295
export OPENSEARCH_VERSION=$(OPENSEARCH_VERSION); \
284296
export OPENSEARCH_MANAGER_ROLE=$(manager_role); \
285297
export OPENSEARCH_MANAGER_SETTING=$(manager_role); \
298+
export OPENSEARCH_JAVA_OPTS_EXTRA="$(java_opts_extra)"; \
286299
docker compose --project-directory .ci/opensearch up -d
287300

288301
cluster.scale.1: ## Start single-node cluster
@@ -373,8 +386,8 @@ cluster.wait-ready: ## Poll cluster until health status is green or yellow
373386
printf "\033[2m\n--- Diagnostic Information ---\033[0m\n"; \
374387
printf "\033[2mDocker containers:\033[0m\n"; \
375388
docker compose --project-directory .ci/opensearch ps || true; \
376-
printf "\033[2m\nRecent logs from containers:\033[0m\n"; \
377-
docker compose --project-directory .ci/opensearch logs --tail=50 || true; \
389+
printf "\033[2m\nFull logs from all containers:\033[0m\n"; \
390+
docker compose --project-directory .ci/opensearch logs || true; \
378391
printf "\033[2m\nAttempted URLs:\033[0m\n"; \
379392
printf " HTTP: $$HTTP_URL\n"; \
380393
printf " HTTPS: $$HTTPS_URL\n"; \

‎opensearch_integration_test.go‎

Lines changed: 18 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ import (
4949
"github.com/opensearch-project/opensearch-go/v4/opensearchtransport"
5050
ostestutil "github.com/opensearch-project/opensearch-go/v4/opensearchtransport/testutil"
5151
"github.com/opensearch-project/opensearch-go/v4/opensearchutil/testutil"
52+
"github.com/opensearch-project/opensearch-go/v4/opensearchutil/testutil/mockhttp"
5253
)
5354

5455
func TestClientTransport(t *testing.T) {
@@ -206,24 +207,17 @@ func TestClientCustomTransport(t *testing.T) {
206207
})
207208

208209
t.Run("Manual", func(t *testing.T) {
209-
tp, _ := opensearchtransport.New(opensearchtransport.Config{
210-
URLs: []*url.URL{
211-
{Scheme: "http", Host: "localhost:9200"},
212-
},
213-
Transport: http.DefaultTransport,
214-
})
215210
config, err := testutil.ClientConfig(t)
216211
require.NoError(t, err)
217-
if testutil.IsSecure(t) {
218-
tp, _ = opensearchtransport.New(opensearchtransport.Config{
219-
URLs: []*url.URL{
220-
{Scheme: "https", Host: "localhost:9200"},
221-
},
222-
Transport: config.Client.Transport,
223-
Username: config.Client.Username,
224-
Password: config.Client.Password,
225-
})
226-
}
212+
213+
// Use centralized URL construction
214+
u := mockhttp.GetOpenSearchURL(t)
215+
tp, _ := opensearchtransport.New(opensearchtransport.Config{
216+
URLs: []*url.URL{u},
217+
Transport: config.Client.Transport,
218+
Username: config.Client.Username,
219+
Password: config.Client.Password,
220+
})
227221

228222
client := opensearchapi.Client{
229223
Client: &opensearch.Client{
@@ -254,20 +248,23 @@ type TestTransport struct {
254248
}
255249

256250
func (tr *TestTransport) Perform(req *http.Request) (*http.Response, error) {
257-
req.URL.Scheme = "http"
258-
req.URL.Host = "localhost:9200"
251+
// Use centralized URL construction
252+
u := mockhttp.GetOpenSearchURL(tr.t)
253+
req.URL.Scheme = u.Scheme
254+
req.URL.Host = u.Host
255+
259256
config, err := testutil.ClientConfig(tr.t)
260257
if err != nil {
261258
return nil, err
262259
}
263260
if testutil.IsSecure(tr.t) {
264-
req.URL.Scheme = "https"
265261
req.SetBasicAuth(config.Client.Username, config.Client.Password)
266262
}
267263

268264
tr.counter.Add(1)
269-
transport := &http.Transport{
270-
TLSClientConfig: &tls.Config{InsecureSkipVerify: true},
265+
transport := config.Client.Transport
266+
if transport == nil {
267+
transport = http.DefaultTransport
271268
}
272269
return transport.RoundTrip(req)
273270
}

‎opensearchtransport/integration_test_helpers.go‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"net/http"
1414
"net/url"
1515
"os"
16+
"strconv"
1617
"testing"
1718

1819
"golang.org/x/mod/semver"
@@ -27,9 +28,18 @@ const (
2728

2829
// getTestTransport returns an http.RoundTripper configured for secure or insecure mode.
2930
// This helper is used across opensearchtransport integration tests to avoid duplication.
31+
// Defaults to insecure to match Makefile and dev environments.
3032
func getTestTransport(t *testing.T) http.RoundTripper {
3133
t.Helper()
32-
if os.Getenv("SECURE_INTEGRATION") == "true" {
34+
val, found := os.LookupEnv("SECURE_INTEGRATION")
35+
if !found {
36+
return http.DefaultTransport // Default to insecure
37+
}
38+
isSecure, err := strconv.ParseBool(val)
39+
if err != nil {
40+
return http.DefaultTransport // Default to insecure on parse error
41+
}
42+
if isSecure {
3343
return &http.Transport{
3444
TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, // #nosec G402 -- Test environment only
3545
}

‎opensearchtransport/opensearchtransport_integration_multinode_test.go‎

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -35,26 +35,45 @@ import (
3535
"net/http"
3636
"net/url"
3737
"os"
38+
"strconv"
3839
"testing"
3940

4041
"github.com/opensearch-project/opensearch-go/v4/opensearchtransport"
4142
)
4243

4344
var _ = fmt.Print
4445

45-
// getTestScheme returns http or https based on SECURE_INTEGRATION env var
46+
// getTestScheme returns http or https based on SECURE_INTEGRATION env var.
47+
// Defaults to http (insecure) to match Makefile and dev environments.
4648
func getTestScheme() string {
47-
if os.Getenv("SECURE_INTEGRATION") == "true" {
49+
val, found := os.LookupEnv("SECURE_INTEGRATION")
50+
if !found {
51+
return "http" // Default to insecure
52+
}
53+
isSecure, err := strconv.ParseBool(val)
54+
if err != nil {
55+
return "http" // Default to insecure on parse error
56+
}
57+
if isSecure {
4858
return "https"
4959
}
5060
return "http"
5161
}
5262

53-
// getTestTransport returns an http.RoundTripper configured for secure or insecure mode
63+
// getTestTransport returns an http.RoundTripper configured for secure or insecure mode.
64+
// Defaults to insecure to match Makefile and dev environments.
5465
func getTestTransport() http.RoundTripper {
55-
if os.Getenv("SECURE_INTEGRATION") == "true" {
66+
val, found := os.LookupEnv("SECURE_INTEGRATION")
67+
if !found {
68+
return http.DefaultTransport // Default to insecure
69+
}
70+
isSecure, err := strconv.ParseBool(val)
71+
if err != nil {
72+
return http.DefaultTransport // Default to insecure on parse error
73+
}
74+
if isSecure {
5675
return &http.Transport{
57-
TLSClientConfig: &tls.Config{InsecureSkipVerify: true},
76+
TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, // #nosec G402 -- Intentionally skipping TLS verification for test environments
5877
}
5978
}
6079
return http.DefaultTransport

‎opensearchtransport/opensearchtransport_internal_test.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -822,6 +822,7 @@ func TestTransportPerformRetries(t *testing.T) {
822822
return nil, &mockNetError{error: fmt.Errorf("Mock network error (%d)", count)}
823823
}),
824824
DisableRetry: true,
825+
HealthCheck: NoOpHealthCheck, // Disable health checks to avoid extra requests during resurrection
825826
},
826827
)
827828

‎opensearchutil/testutil/helpers.go‎

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919
"net/http"
2020
"net/url"
2121
"os"
22+
"strconv"
2223
"strings"
2324
"testing"
2425
"time"
@@ -138,10 +139,30 @@ func PollUntil(
138139
return fmt.Errorf("condition not met after %d attempts", maxAttempts)
139140
}
140141

141-
// IsSecure returns true when SECURE_INTEGRATION env is set to true
142+
// IsSecure returns true when SECURE_INTEGRATION env is set to true.
143+
// Defaults to false (insecure) when not set, matching the Makefile default
144+
// and typical development environments.
142145
func IsSecure(t *testing.T) bool {
143146
t.Helper()
144-
return os.Getenv("SECURE_INTEGRATION") != "false" // Default to secure
147+
val, found := os.LookupEnv("SECURE_INTEGRATION")
148+
if !found {
149+
return false // Default to insecure to match Makefile and dev environments
150+
}
151+
isSecure, err := strconv.ParseBool(val)
152+
if err != nil {
153+
return false // Default to insecure on parse error
154+
}
155+
return isSecure
156+
}
157+
158+
// GetScheme returns "http" or "https" based on SECURE_INTEGRATION setting.
159+
// This centralizes the scheme determination logic for test consistency.
160+
func GetScheme(t *testing.T) string {
161+
t.Helper()
162+
if IsSecure(t) {
163+
return "https"
164+
}
165+
return "http"
145166
}
146167

147168
// ClientConfig returns an opensearchapi.Config for both secure and insecure opensearch

‎opensearchutil/testutil/mockhttp/port.go‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,8 @@ const (
2323
MockPortEnd = 9799
2424

2525
// DefaultOpenSearchScheme is the default scheme used when talking to test OpenSearch instances.
26-
DefaultOpenSearchScheme = DefaultOpenSearchSchemeSecure
26+
// Defaults to HTTP (insecure) to match Makefile default and typical dev environments.
27+
DefaultOpenSearchScheme = DefaultOpenSearchSchemeInsecure
2728
DefaultOpenSearchSchemeInsecure = "http"
2829
DefaultOpenSearchSchemeSecure = "https"
2930

@@ -86,9 +87,9 @@ func GetOpenSearchURL(t *testing.T) *url.URL {
8687

8788
// Override the scheme based on SECURE_INTEGRATION setting
8889
if isSecure {
89-
u.Scheme = DefaultOpenSearchScheme
90+
u.Scheme = DefaultOpenSearchSchemeSecure
9091
} else {
91-
u.Scheme = "http"
92+
u.Scheme = DefaultOpenSearchSchemeInsecure
9293
}
9394
}
9495
// If SECURE_INTEGRATION is not set, leave the scheme as-is from the original URL

‎plugins/ism/internal/test/helper.go‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818

1919
"github.com/opensearch-project/opensearch-go/v4"
2020
"github.com/opensearch-project/opensearch-go/v4/opensearchutil/testutil"
21+
"github.com/opensearch-project/opensearch-go/v4/opensearchutil/testutil/mockhttp"
2122
"github.com/opensearch-project/opensearch-go/v4/plugins/ism"
2223
)
2324

@@ -42,6 +43,9 @@ func NewClient(t *testing.T) (*ism.Client, error) {
4243
// ClientConfig returns an opensearchapi.Config for secure opensearch
4344
func ClientConfig(t *testing.T) (*ism.Config, error) {
4445
t.Helper()
46+
// Use centralized URL construction
47+
u := mockhttp.GetOpenSearchURL(t)
48+
4549
if testutil.IsSecure(t) {
4650
password, err := testutil.GetPassword(t)
4751
if err != nil {
@@ -52,9 +56,9 @@ func ClientConfig(t *testing.T) (*ism.Config, error) {
5256
Client: opensearch.Config{
5357
Username: "admin",
5458
Password: password,
55-
Addresses: []string{"https://localhost:9200"},
59+
Addresses: []string{u.String()},
5660
Transport: &http.Transport{
57-
TLSClientConfig: &tls.Config{InsecureSkipVerify: true},
61+
TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, // #nosec G402 -- Test environment only
5862
},
5963
},
6064
}, nil

‎plugins/security/api_audit_test.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,12 @@ import (
2121

2222
func TestSecurityAuditClient(t *testing.T) {
2323
testutil.SkipIfNotSecure(t)
24+
25+
osAPIclient, err := testutil.NewClient(t)
26+
require.NoError(t, err)
27+
28+
testutil.SkipIfBelowVersion(t, osAPIclient, 2, 15, "Audit API")
29+
2430
client, err := ossectest.NewClient(t)
2531
require.NoError(t, err)
2632

0 commit comments

Comments
 (0)