Repository: opensearch-project/opensearch-go
Pull request #61 (2021-08-20 02:49:38)

Accept OPENSEARCH_URL for address
### Description
Accept OPENSEARCH_URL and ELASTICSEARCH_URL for backward compatibility, but not together.
 
### Issues Resolved
#49
 
### Check List
- [x] Commits are signed per the DCO using --signoff 

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check [here](https://github.com/opensearch-project/OpenSearch/blob/main/CONTRIBUTING.md#developer-certificate-of-origin).



Patch:
diff --git a/doc.go b/doc.go
index fa3ccaf..c6c44dc 100644
--- a/doc.go
+++ b/doc.go
@@ -31,8 +31,9 @@ Create the client with the NewDefaultClient function:
 
 		opensearch.NewDefaultClient()
 
-The ELASTICSEARCH_URL environment variable is used instead of the default URL, when set.
+The OPENSEARCH_URL/ELASTICSEARCH_URL environment variable is used instead of the default URL, when set.
 Use a comma to separate multiple URLs.
+It is an error to set both environment variable.
 
 To configure the client, pass a Config object to the NewClient function:
 
diff --git a/opensearch.go b/opensearch.go
index d48075a..073fd58 100644
--- a/opensearch.go
+++ b/opensearch.go
@@ -42,9 +42,9 @@ import (
 	"sync"
 	"time"
 
+	"github.com/opensearch-project/opensearch-go/internal/version"
 	"github.com/opensearch-project/opensearch-go/opensearchapi"
 	"github.com/opensearch-project/opensearch-go/opensearchtransport"
-	"github.com/opensearch-project/opensearch-go/internal/version"
 )
 
 var (
@@ -57,9 +57,12 @@ func init() {
 }
 
 const (
-	defaultURL         = "http://localhost:9200"
-	openSearch         = "opensearch"
-	unsupportedProduct = "the client noticed that the server is not a supported distribution"
+	defaultURL          = "http://localhost:9200"
+	openSearch          = "opensearch"
+	unsupportedProduct  = "the client noticed that the server is not a supported distribution"
+	envOpenSearchURL    = "OPENSEARCH_URL"
+	envElasticsearchURL = "ELASTICSEARCH_URL"
+
 )
 
 // Version returns the package version as a string.
@@ -132,9 +135,11 @@ type info struct {
 //
 // It will use http://localhost:9200 as the default address.
 //
-// It will use the ELASTICSEARCH_URL environment variable, if set,
+// It will use the OPENSEARCH_URL/ELASTICSEARCH_URL environment variable, if set,
 // to configure the addresses; use a comma to separate multiple URLs.
 //
+// It's an error to set both OPENSEARCH_URL and ELASTICSEARCH_URL.
+//
 func NewDefaultClient() (*Client, error) {
 	return NewClient(Config{})
 }
@@ -143,14 +148,20 @@ func NewDefaultClient() (*Client, error) {
 //
 // It will use http://localhost:9200 as the default address.
 //
-// It will use the ELASTICSEARCH_URL environment variable, if set,
+// It will use the OPENSEARCH_URL/ELASTICSEARCH_URL environment variable, if set,
 // to configure the addresses; use a comma to separate multiple URLs.
 //
+// It's an error to set both OPENSEARCH_URL and ELASTICSEARCH_URL.
+//
 func NewClient(cfg Config) (*Client, error) {
 	var addrs []string
 
 	if len(cfg.Addresses) == 0 {
-		addrs = addrsFromEnvironment()
+		envAddress, err := getAddressFromEnvironment()
+		if err != nil {
+			return nil, err
+		}
+		addrs = envAddress
 	} else {
 		addrs = append(addrs, cfg.Addresses...)
 	}
@@ -212,6 +223,19 @@ func NewClient(cfg Config) (*Client, error) {
 	return client, err
 }
 
+func getAddressFromEnvironment() ([]string, error) {
+	fromOpenSearchEnv := addrsFromEnvironment(envOpenSearchURL)
+	fromElasticsearchEnv := addrsFromEnvironment(envElasticsearchURL)
+
+	if len(fromElasticsearchEnv) > 0 && len(fromOpenSearchEnv) > 0 {
+		return nil, fmt.Errorf("cannot create client: both %s and %s are set", envOpenSearchURL, envElasticsearchURL)
+	}
+	if len(fromOpenSearchEnv) > 0 {
+		return fromOpenSearchEnv, nil
+	}
+	return fromElasticsearchEnv, nil
+}
+
 // checkCompatibleInfo validates the information given by OpenSearch
 //
 func checkCompatibleInfo(info info) error {
@@ -343,12 +367,12 @@ func (c *Client) DiscoverNodes() error {
 }
 
 // addrsFromEnvironment returns a list of addresses by splitting
-// the ELASTICSEARCH_URL environment variable with comma, or an empty list.
+// the given environment variable with comma, or an empty list.
 //
-func addrsFromEnvironment() []string {
+func addrsFromEnvironment(name string) []string {
 	var addrs []string
 
-	if envURLs, ok := os.LookupEnv("ELASTICSEARCH_URL"); ok && envURLs != "" {
+	if envURLs, ok := os.LookupEnv(name); ok && envURLs != "" {
 		list := strings.Split(envURLs, ",")
 		for _, u := range list {
 			addrs = append(addrs, strings.TrimSpace(u))


Test patch:
diff --git a/opensearch_internal_test.go b/opensearch_internal_test.go
index 2d39234..595fb35 100644
--- a/opensearch_internal_test.go
+++ b/opensearch_internal_test.go
@@ -30,6 +30,7 @@ package opensearch
 
 import (
 	"errors"
+	"github.com/stretchr/testify/assert"
 	"io/ioutil"
 	"net/http"
 	"net/http/httptest"
@@ -103,9 +104,9 @@ func TestClientConfiguration(t *testing.T) {
 		}
 	})
 
-	t.Run("With URL from environment", func(t *testing.T) {
-		os.Setenv("ELASTICSEARCH_URL", "http://example.com")
-		defer func() { os.Setenv("ELASTICSEARCH_URL", "") }()
+	t.Run("With URL from ELASTICSEARCH_URL", func(t *testing.T) {
+		os.Setenv(envElasticsearchURL, "http://elasticsearch.com")
+		defer func() { os.Setenv(envElasticsearchURL, "") }()
 
 		c, err := NewClient(Config{Transport: &mockTransp{}})
 		if err != nil {
@@ -114,14 +115,46 @@ func TestClientConfiguration(t *testing.T) {
 
 		u := c.Transport.(*opensearchtransport.Client).URLs()[0].String()
 
-		if u != "http://example.com" {
-			t.Errorf("Unexpected URL, want=http://example.com, got=%s", u)
+		if u != "http://elasticsearch.com" {
+			t.Errorf("Unexpected URL, want=http://elasticsearch.com, got=%s", u)
+		}
+	})
+
+	t.Run("With URL from OPENSEARCH_URL", func(t *testing.T) {
+		os.Setenv(envOpenSearchURL, "http://opensearch.com")
+		defer func() { os.Setenv(envOpenSearchURL, "") }()
+
+		c, err := NewClient(Config{Transport: &mockTransp{}})
+		if err != nil {
+			t.Errorf("Unexpected error: %s", err)
+		}
+
+		u := c.Transport.(*opensearchtransport.Client).URLs()[0].String()
+
+		if u != "http://opensearch.com" {
+			t.Errorf("Unexpected URL, want=http://opensearch.com, got=%s", u)
+		}
+	})
+
+	t.Run("With URL from OPENSEARCH_URL and ELASTICSEARCH_URL", func(t *testing.T) {
+		os.Setenv(envOpenSearchURL, "http://opensearch.com")
+		defer func() { os.Setenv(envOpenSearchURL, "") }()
+
+		os.Setenv(envElasticsearchURL, "http://elasticsearch.com")
+		defer func() { os.Setenv(envElasticsearchURL, "") }()
+
+		_, err := NewClient(Config{Transport: &mockTransp{}})
+		assert.Error(t, err, "Expected error")
+
+		match, _ := regexp.MatchString("both .* are set", err.Error())
+		if !match {
+			t.Errorf("Expected error when addresses from OPENSEARCH_URL and ELASTICSEARCH_URL are used together, got: %v", err)
 		}
 	})
 
 	t.Run("With URL from environment and cfg.Addresses", func(t *testing.T) {
-		os.Setenv("ELASTICSEARCH_URL", "http://example.com")
-		defer func() { os.Setenv("ELASTICSEARCH_URL", "") }()
+		os.Setenv(envElasticsearchURL, "http://example.com")
+		defer func() { os.Setenv(envElasticsearchURL, "") }()
 
 		c, err := NewClient(Config{Addresses: []string{"http://localhost:8080//"}, Transport: &mockTransp{}})
 		if err != nil {
@@ -145,8 +178,8 @@ func TestClientConfiguration(t *testing.T) {
 	})
 
 	t.Run("With invalid URL from environment", func(t *testing.T) {
-		os.Setenv("ELASTICSEARCH_URL", ":foobar")
-		defer func() { os.Setenv("ELASTICSEARCH_URL", "") }()
+		os.Setenv(envElasticsearchURL, ":foobar")
+		defer func() { os.Setenv(envElasticsearchURL, "") }()
 
 		c, err := NewDefaultClient()
 		if err == nil {