fix linter issues throughout the repo

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

Author: sean-

CHANGELOG.md

@@ -29,6 +29,12 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 - **BREAKING**: Enhanced node discovery to match OpenSearch server behavior ([#765](https://github.com/opensearch-project/opensearch-go/issues/765))
   - Dedicated cluster manager nodes are now excluded from client request routing by default (best practice)
   - Node selection logic now matches Java client `NodeSelector.SKIP_DEDICATED_CLUSTER_MASTERS` behavior
+- **BREAKING**: Add context support to discovery and client lifecycle management
+  - `opensearchtransport.Discoverable` interface now requires `context.Context` parameter: `DiscoverNodes(ctx context.Context) error`
+  - `opensearch.Client.DiscoverNodes()` and `opensearchtransport.Client.DiscoverNodes()` now require `context.Context` parameter
+  - `opensearch.Config` and `opensearchtransport.Config` now accept optional `Context` and `CancelFunc` fields
+  - `opensearchutil.BulkIndexerConfig` now accepts optional `Context` and `CancelFunc` fields
+  - Enables proper context propagation for timeouts, cancellation, and graceful shutdown
 
 ### Deprecated
 

internal/test/helper.go

@@ -57,7 +57,7 @@ func waitForClusterReady(t *testing.T, client *opensearchapi.Client) error {
 	)
 
 	// Get version for informational logging
-	major, minor, patch, err := GetVersion(client, t)
+	major, minor, patch, err := GetVersion(t, client)
 	if err != nil {
 		return fmt.Errorf("failed to get OpenSearch version: %w", err)
 	}
@@ -116,7 +116,8 @@ func extendedReadinessCheck(ctx context.Context, client *opensearchapi.Client) e
 }
 
 // GetVersion gets cluster info and returns version as int's
-func GetVersion(client *opensearchapi.Client, t *testing.T) (int64, int64, int64, error) {
+func GetVersion(t *testing.T, client *opensearchapi.Client) (int64, int64, int64, error) {
+	t.Helper()
 	if client == nil {
 		return 0, 0, 0, fmt.Errorf("client cannot be nil")
 	}
@@ -130,7 +131,7 @@ func GetVersion(client *opensearchapi.Client, t *testing.T) (int64, int64, int64
 // SkipIfBelowVersion skips a test if the cluster version is below a given version
 func SkipIfBelowVersion(t *testing.T, client *opensearchapi.Client, majorVersion, patchVersion int64, testName string) {
 	t.Helper()
-	major, patch, _, err := GetVersion(client, t)
+	major, patch, _, err := GetVersion(t, client)
 	assert.Nil(t, err)
 	if major < majorVersion || (major == majorVersion && patch < patchVersion) {
 		t.Skipf("Skipping %s as version %d.%d.x does not support this endpoint", testName, major, patch)

internal/test/readiness_test.go

@@ -5,7 +5,7 @@
 // compatible open source license.
 //go:build integration
 
-package ostest
+package ostest_test
 
 import (
 	"context"
@@ -16,13 +16,14 @@ import (
 	"github.com/stretchr/testify/require"
 	"github.com/stretchr/testify/suite"
 
+	ostest "github.com/opensearch-project/opensearch-go/v4/internal/test"
 	"github.com/opensearch-project/opensearch-go/v4/opensearchapi"
 	"github.com/opensearch-project/opensearch-go/v4/opensearchtransport/testutil"
 )
 
 // TestNewClient demonstrates the enhanced client creation with automatic readiness checks
 func TestNewClient(t *testing.T) {
-	client, err := NewClient(t)
+	client, err := ostest.NewClient(t)
 	require.NoError(t, err, "Failed to create client")
 	require.NotNil(t, client, "Client should not be nil")
 
@@ -42,23 +43,23 @@ func TestNewClient(t *testing.T) {
 
 // TestConfigFunctions tests the configuration utility functions
 func TestConfigFunctions(t *testing.T) {
-	// Test IsSecure function
-	t.Run("IsSecure", func(t *testing.T) {
+	// Test ostest.IsSecure function
+	t.Run("ostest.IsSecure", func(t *testing.T) {
 		// Save original value
 		original := os.Getenv("SECURE_INTEGRATION")
 		defer os.Setenv("SECURE_INTEGRATION", original)
 
 		// Test false case
 		os.Setenv("SECURE_INTEGRATION", "false")
-		assert.False(t, IsSecure())
+		assert.False(t, ostest.IsSecure())
 
 		// Test true case
 		os.Setenv("SECURE_INTEGRATION", "true")
-		assert.True(t, IsSecure())
+		assert.True(t, ostest.IsSecure())
 
 		// Test empty case
 		os.Unsetenv("SECURE_INTEGRATION")
-		assert.False(t, IsSecure())
+		assert.False(t, ostest.IsSecure())
 	})
 
 	// Test GetPassword function
@@ -76,31 +77,31 @@ func TestConfigFunctions(t *testing.T) {
 
 		// Test default admin password for older versions
 		os.Setenv("OPENSEARCH_VERSION", "2.0.0")
-		password, err := GetPassword()
+		password, err := ostest.GetPassword()
 		assert.NoError(t, err)
 		assert.Equal(t, "admin", password)
 
 		// Test strong password for newer versions
 		os.Setenv("OPENSEARCH_VERSION", "2.12.0")
-		password, err = GetPassword()
+		password, err = ostest.GetPassword()
 		assert.NoError(t, err)
 		assert.Equal(t, "myStrongPassword123!", password)
 
 		// Test latest version
 		os.Setenv("OPENSEARCH_VERSION", "latest")
-		password, err = GetPassword()
+		password, err = ostest.GetPassword()
 		assert.NoError(t, err)
 		assert.Equal(t, "myStrongPassword123!", password)
 
 		// Test empty version
 		os.Unsetenv("OPENSEARCH_VERSION")
-		password, err = GetPassword()
+		password, err = ostest.GetPassword()
 		assert.NoError(t, err)
 		assert.Equal(t, "myStrongPassword123!", password)
 
 		// ERROR PATH: Test invalid version format
 		os.Setenv("OPENSEARCH_VERSION", "invalid.version")
-		_, err = GetPassword()
+		_, err = ostest.GetPassword()
 		assert.Error(t, err, "Should error with invalid version format")
 	})
 
@@ -125,7 +126,7 @@ func TestConfigFunctions(t *testing.T) {
 
 		// Test insecure config (should return valid HTTP config)
 		os.Setenv("SECURE_INTEGRATION", "false")
-		config, err := ClientConfig()
+		config, err := ostest.ClientConfig()
 		assert.NoError(t, err)
 		assert.NotNil(t, config)
 		assert.Equal(t, []string{"http://localhost:9200"}, config.Client.Addresses)
@@ -136,7 +137,7 @@ func TestConfigFunctions(t *testing.T) {
 		// Test secure config - success case
 		os.Setenv("SECURE_INTEGRATION", "true")
 		os.Setenv("OPENSEARCH_VERSION", "2.12.0") // Valid version
-		config, err = ClientConfig()
+		config, err = ostest.ClientConfig()
 		assert.NoError(t, err)
 		if config != nil {
 			assert.Equal(t, "admin", config.Client.Username)
@@ -148,29 +149,29 @@ func TestConfigFunctions(t *testing.T) {
 		// ERROR PATH: Test secure config with invalid version
 		os.Setenv("SECURE_INTEGRATION", "true")
 		os.Setenv("OPENSEARCH_VERSION", "not.a.version")
-		config, err = ClientConfig()
+		config, err = ostest.ClientConfig()
 		assert.Error(t, err, "Should propagate GetPassword error")
 		assert.Nil(t, config, "Config should be nil on error")
 	})
 }
 
 // TestHelperFunctions tests utility functions with different scenarios
 func TestHelperFunctions(t *testing.T) {
-	client, err := NewClient(t)
+	client, err := ostest.NewClient(t)
 	require.NoError(t, err)
 
-	// Test SkipIfBelowVersion - this will not skip since we're running on 3.4.0
-	t.Run("SkipIfBelowVersion", func(t *testing.T) {
+	// Test ostest.SkipIfBelowVersion - this will not skip since we're running on 3.4.0
+	t.Run("ostest.SkipIfBelowVersion", func(t *testing.T) {
 		// This should not skip on current version (3.4.0+)
-		SkipIfBelowVersion(t, client, 2, 0, "TestFeature")
+		ostest.SkipIfBelowVersion(t, client, 2, 0, "TestFeature")
 		// If we reach here, the test didn't skip
 		assert.True(t, true, "Test should continue for supported version")
 	})
 
 	// Test SkipIfNotSecure
 	t.Run("SkipIfNotSecure", func(t *testing.T) {
-		if IsSecure() {
-			SkipIfNotSecure(t)
+		if ostest.IsSecure() {
+			ostest.SkipIfNotSecure(t)
 			// If we reach here, it's a secure cluster and test continued
 			assert.True(t, true, "Test should continue for secure cluster")
 		}
@@ -184,27 +185,27 @@ func TestHelperFunctions(t *testing.T) {
 		require.NoError(t, err)
 
 		// This tests the JSON comparison utility
-		CompareRawJSONwithParsedJSON(t, resp, resp.Inspect().Response)
+		ostest.CompareRawJSONwithParsedJSON(t, resp, resp.Inspect().Response)
 	})
 
 	// Test GetVersion function
 	t.Run("GetVersion", func(t *testing.T) {
 		// Test successful version retrieval
-		major, minor, patch, err := GetVersion(client, t)
+		major, minor, patch, err := ostest.GetVersion(t, client)
 		assert.NoError(t, err, "GetVersion should succeed with valid client")
 		assert.True(t, major >= 1, "Major version should be at least 1")
 		assert.True(t, minor >= 0, "Minor version should be non-negative")
 		assert.True(t, patch >= 0, "Patch version should be non-negative")
 
 		// ERROR PATH: Test GetVersion with nil client
-		_, _, _, err = GetVersion(nil, t)
+		_, _, _, err = ostest.GetVersion(t, nil)
 		assert.Error(t, err, "GetVersion should error with nil client")
 	})
 
 	// Test NewClient function
 	t.Run("NewClient", func(t *testing.T) {
 		// Test successful client creation (already done above)
-		client, err := NewClient(t)
+		client, err := ostest.NewClient(t)
 		assert.NoError(t, err, "NewClient should succeed in normal conditions")
 		assert.NotNil(t, client, "Client should not be nil")
 
@@ -229,30 +230,14 @@ func TestHelperFunctions(t *testing.T) {
 		os.Setenv("SECURE_INTEGRATION", "true")
 		os.Setenv("OPENSEARCH_VERSION", "invalid.format")
 
-		_, err = NewClient(t)
+		_, err = ostest.NewClient(t)
 		assert.Error(t, err, "NewClient should propagate ClientConfig errors")
 	})
-
-	// Test extendedReadinessCheck function
-	t.Run("extendedReadinessCheck", func(t *testing.T) {
-		// Test successful validation (using working client)
-		ctx := context.Background()
-		err := extendedReadinessCheck(ctx, client)
-		assert.NoError(t, err, "extendedReadinessCheck should succeed with healthy cluster")
-
-		// ERROR PATH: Test with cancelled context
-		cancelledCtx, cancel := context.WithCancel(context.Background())
-		cancel() // Cancel immediately
-
-		err = extendedReadinessCheck(cancelledCtx, client)
-		assert.Error(t, err, "extendedReadinessCheck should error with cancelled context")
-		assert.Contains(t, err.Error(), "failed", "Error should contain descriptive message")
-	})
 }
 
 // ExampleTestSuite demonstrates using the testify suite pattern
 type ExampleTestSuite struct {
-	OpenSearchTestSuite
+	ostest.OpenSearchTestSuite
 }
 
 func (s *ExampleTestSuite) TestClusterHealthWithSuite() {
@@ -300,7 +285,7 @@ func (s *ExampleTestSuite) TestVersionSkipping() {
 
 // Test secure cluster skipping if not secure
 func (s *ExampleTestSuite) TestSecureSkipping() {
-	if !IsSecure() {
+	if !ostest.IsSecure() {
 		s.SkipIfNotSecure()
 		// This line should not be reached for insecure clusters
 		s.T().Error("This test should have been skipped for insecure cluster")

internal/test/suite.go

@@ -36,7 +36,7 @@ func (s *OpenSearchTestSuite) SetupSuite() {
 	s.Client = client
 
 	// Get and store version information for test use
-	major, minor, patch, err := GetVersion(s.Client, t)
+	major, minor, patch, err := GetVersion(t, s.Client)
 	require.NoError(t, err, "Failed to get OpenSearch version")
 
 	s.Major, s.Minor, s.Patch = major, minor, patch

opensearch.go

@@ -100,6 +100,12 @@ type Config struct {
 	Logger    opensearchtransport.Logger   // The logger object.
 	Selector  opensearchtransport.Selector // The selector object.
 
+	// Context for background operations. If nil, context.Background() will be used.
+	// This is only used during client initialization and is not stored long-term.
+	//nolint:containedctx // Config struct is short-lived, context extracted during New()
+	Context    context.Context
+	CancelFunc context.CancelFunc
+
 	// Optional constructor function for a custom ConnectionPool. Default: nil.
 	ConnectionPoolFunc func([]*opensearchtransport.Connection, opensearchtransport.Selector) opensearchtransport.ConnectionPool
 }
@@ -139,6 +145,11 @@ func NewClient(cfg Config) (*Client, error) {
 		addrs = append(addrs, cfg.Addresses...)
 	}
 
+	// Initialize context if not provided
+	if cfg.Context == nil {
+		cfg.Context, cfg.CancelFunc = context.WithCancel(context.Background())
+	}
+
 	urls, err := addrsToURLs(addrs)
 	if err != nil {
 		return nil, fmt.Errorf("%w: %w", ErrCreateClient, err)
@@ -148,24 +159,11 @@ func NewClient(cfg Config) (*Client, error) {
 		//nolint:errcheck // errcheck exclude ???
 		u, _ := url.Parse(defaultURL)
 		urls = append(urls, u)
-	} else if cfg.Username == "" || cfg.Password == "" {
-		// Extract credentials from the first URL that has them (only if not already configured)
-		for _, u := range urls {
-			if u.User != nil {
-				if cfg.Username == "" {
-					cfg.Username = u.User.Username()
-				}
-				if cfg.Password == "" {
-					if pw, ok := u.User.Password(); ok {
-						cfg.Password = pw
-					}
-				}
-				// Stop after finding the first URL with credentials
-				break
-			}
-		}
 	}
 
+	// Extract credentials from the first URL that has them (only if not already configured)
+	extractCredentialsFromURLs(&cfg, urls)
+
 	tp, err := opensearchtransport.New(opensearchtransport.Config{
 		URLs:     urls,
 		Username: cfg.Username,
@@ -193,6 +191,8 @@ func NewClient(cfg Config) (*Client, error) {
 		Logger:             cfg.Logger,
 		Selector:           cfg.Selector,
 		ConnectionPoolFunc: cfg.ConnectionPoolFunc,
+		Context:            cfg.Context,
+		CancelFunc:         cfg.CancelFunc,
 	})
 	if err != nil {
 		return nil, fmt.Errorf("%w: %w", ErrCreateTransport, err)
@@ -201,8 +201,15 @@ func NewClient(cfg Config) (*Client, error) {
 	client := &Client{Transport: tp}
 
 	if cfg.DiscoverNodesOnStart {
-		//nolint:errcheck // goroutine discards return values
-		go client.DiscoverNodes()
+		go func() {
+			start := time.Now()
+			if err := client.DiscoverNodes(cfg.Context); err != nil {
+				if cfg.Logger != nil {
+					//nolint:errcheck // Logger errors are not critical for discovery
+					cfg.Logger.LogRoundTrip(nil, nil, err, start, time.Since(start))
+				}
+			}
+		}()
 	}
 
 	return client, err
@@ -294,9 +301,9 @@ func (c *Client) Metrics() (opensearchtransport.Metrics, error) {
 }
 
 // DiscoverNodes reloads the client connections by fetching information from the cluster.
-func (c *Client) DiscoverNodes() error {
+func (c *Client) DiscoverNodes(ctx context.Context) error {
 	if dt, ok := c.Transport.(opensearchtransport.Discoverable); ok {
-		return dt.DiscoverNodes()
+		return dt.DiscoverNodes(ctx)
 	}
 
 	return ErrTransportMissingMethodDiscoverNodes
@@ -335,6 +342,31 @@ func addrsToURLs(addrs []string) ([]*url.URL, error) {
 	return urls, nil
 }
 
+// extractCredentialsFromURLs extracts username and password from the first URL that has them.
+// Only extracts credentials that are not already configured in cfg.
+func extractCredentialsFromURLs(cfg *Config, urls []*url.URL) {
+	if len(urls) == 0 || (cfg.Username != "" && cfg.Password != "") {
+		return // No URLs or credentials already fully configured
+	}
+
+	for _, u := range urls {
+		if u.User == nil {
+			continue
+		}
+
+		if cfg.Username == "" {
+			cfg.Username = u.User.Username()
+		}
+		if cfg.Password == "" {
+			if pw, ok := u.User.Password(); ok {
+				cfg.Password = pw
+			}
+		}
+		// Stop after finding the first URL with credentials
+		break
+	}
+}
+
 // ToPointer converts any value to a pointer, mainly used for request parameters
 func ToPointer[V any](value V) *V {
 	return &value