Ignore the ELASTICSEARCH_URL variable when address is passed in configuration

Fixes #113
Closes #115

(cherry picked from commit 86a626c)

Author: karmi

doc.go

@@ -30,6 +30,7 @@ To configure the client, pass a Config object to the NewClient function:
 		elasticsearch.NewClient(cfg)
 
 When using the Elastic Service (https://elastic.co/cloud), you can use CloudID instead of Addresses.
+When either Addresses or CloudID is set, the ELASTICSEARCH_URL environment variable is ignored.
 
 See the elasticsearch_integration_test.go file and the _examples folder for more information.
 

elasticsearch.go

@@ -83,36 +83,30 @@ func NewDefaultClient() (*Client, error) {
 // It will use the ELASTICSEARCH_URL environment variable, if set,
 // to configure the addresses; use a comma to separate multiple URLs.
 //
-// It's an error to set both cfg.Addresses and the ELASTICSEARCH_URL
-// environment variable.
+// If either cfg.Addresses or cfg.CloudID is set, the ELASTICSEARCH_URL
+// environment variable is ignored.
+//
+// It's an error to set both cfg.Addresses and cfg.CloudID.
 //
 func NewClient(cfg Config) (*Client, error) {
 	var addrs []string
 
-	envAddrs := addrsFromEnvironment()
-
-	if len(envAddrs) > 0 && len(cfg.Addresses) > 0 {
-		return nil, errors.New("cannot create client: both ELASTICSEARCH_URL and Addresses are set")
-	}
-
-	if len(envAddrs) > 0 && cfg.CloudID != "" {
-		return nil, errors.New("cannot create client: both ELASTICSEARCH_URL and CloudID are set")
-	}
-
-	if len(cfg.Addresses) > 0 && cfg.CloudID != "" {
-		return nil, errors.New("cannot create client: both Addresses and CloudID are set")
-	}
+	if len(cfg.Addresses) == 0 && cfg.CloudID == "" {
+		addrs = addrsFromEnvironment()
+	} else {
+		if len(cfg.Addresses) > 0 && cfg.CloudID != "" {
+			return nil, errors.New("cannot create client: both Addresses and CloudID are set")
+		}
 
-	if cfg.CloudID != "" {
-		cloudAddrs, err := addrFromCloudID(cfg.CloudID)
-		if err != nil {
-			return nil, fmt.Errorf("cannot create client: cannot parse CloudID: %s", err)
+		if cfg.CloudID != "" {
+			cloudAddr, err := addrFromCloudID(cfg.CloudID)
+			if err != nil {
+				return nil, fmt.Errorf("cannot create client: cannot parse CloudID: %s", err)
+			}
+			addrs = append(addrs, cloudAddr)
 		}
-		addrs = append(addrs, cloudAddrs)
-	} else {
-		if len(envAddrs) > 0 {
-			addrs = append(addrs, envAddrs...)
-		} else if len(cfg.Addresses) > 0 {
+
+		if len(cfg.Addresses) > 0 {
 			addrs = append(addrs, cfg.Addresses...)
 		}
 	}
@@ -236,5 +230,10 @@ func addrFromCloudID(input string) (string, error) {
 		return "", err
 	}
 	parts := strings.Split(string(data), "$")
+
+	if len(parts) < 2 {
+		return "", fmt.Errorf("invalid encoded value: %s", parts)
+	}
+
 	return fmt.Sprintf("%s%s.%s", scheme, parts[1], parts[0]), nil
 }

elasticsearch_internal_test.go

@@ -22,14 +22,20 @@ func TestClientConfiguration(t *testing.T) {
 	t.Parallel()
 
 	t.Run("With empty", func(t *testing.T) {
-		_, err := NewDefaultClient()
+		c, err := NewDefaultClient()
 
 		if err != nil {
 			t.Errorf("Unexpected error: %s", err)
 		}
+
+		u := c.Transport.(*estransport.Client).URLs()[0].String()
+
+		if u != defaultURL {
+			t.Errorf("Unexpected URL, want=%s, got=%s", defaultURL, u)
+		}
 	})
 
-	t.Run("With address", func(t *testing.T) {
+	t.Run("With URL from Addresses", func(t *testing.T) {
 		c, err := NewClient(Config{Addresses: []string{"http://localhost:8080//"}})
 		if err != nil {
 			t.Fatalf("Unexpected error: %s", err)
@@ -62,32 +68,36 @@ func TestClientConfiguration(t *testing.T) {
 		os.Setenv("ELASTICSEARCH_URL", "http://example.com")
 		defer func() { os.Setenv("ELASTICSEARCH_URL", "") }()
 
-		_, err := NewClient(Config{Addresses: []string{"http://localhost:8080//"}})
-		if err == nil {
-			t.Fatalf("Expected error, got: %v", err)
+		c, err := NewClient(Config{Addresses: []string{"http://localhost:8080//"}})
+		if err != nil {
+			t.Fatalf("Unexpected error: %s", err)
 		}
-		match, _ := regexp.MatchString("both .* are set", err.Error())
-		if !match {
-			t.Errorf("Expected error when addresses from environment and configuration are used together, got: %v", err)
+
+		u := c.Transport.(*estransport.Client).URLs()[0].String()
+
+		if u != "http://localhost:8080" {
+			t.Errorf("Unexpected URL, want=http://localhost:8080, got=%s", u)
 		}
 	})
 
 	t.Run("With URL from environment and cfg.CloudID", func(t *testing.T) {
 		os.Setenv("ELASTICSEARCH_URL", "http://example.com")
 		defer func() { os.Setenv("ELASTICSEARCH_URL", "") }()
 
-		_, err := NewClient(Config{CloudID: "foobar="})
-		if err == nil {
-			t.Fatalf("Expected error, got: %v", err)
+		c, err := NewClient(Config{CloudID: "foo:YmFyLmNsb3VkLmVzLmlvJGFiYzEyMyRkZWY0NTY="})
+		if err != nil {
+			t.Fatalf("Unexpected error: %s", err)
 		}
-		match, _ := regexp.MatchString("both .* are set", err.Error())
-		if !match {
-			t.Errorf("Expected error when addresses from environment and configuration are used together, got: %v", err)
+
+		u := c.Transport.(*estransport.Client).URLs()[0].String()
+
+		if u != "https://abc123.bar.cloud.es.io" {
+			t.Errorf("Unexpected URL, want=https://abc123.bar.cloud.es.io, got=%s", u)
 		}
 	})
 
 	t.Run("With cfg.Addresses and cfg.CloudID", func(t *testing.T) {
-		_, err := NewClient(Config{Addresses: []string{"http://localhost:8080//"}, CloudID: "foobar="})
+		_, err := NewClient(Config{Addresses: []string{"http://localhost:8080//"}, CloudID: "foo:ABC="})
 		if err == nil {
 			t.Fatalf("Expected error, got: %v", err)
 		}
@@ -111,6 +121,25 @@ func TestClientConfiguration(t *testing.T) {
 		}
 	})
 
+	t.Run("With invalid CloudID", func(t *testing.T) {
+		var err error
+
+		_, err = NewClient(Config{CloudID: "foo:ZZZ==="})
+		if err == nil {
+			t.Errorf("Expected error for CloudID, got: %v", err)
+		}
+
+		_, err = NewClient(Config{CloudID: "foo:Zm9v"})
+		if err == nil {
+			t.Errorf("Expected error for CloudID, got: %v", err)
+		}
+
+		_, err = NewClient(Config{CloudID: "foo:"})
+		if err == nil {
+			t.Errorf("Expected error for CloudID, got: %v", err)
+		}
+	})
+
 	t.Run("With invalid URL", func(t *testing.T) {
 		u := ":foo"
 		_, err := NewClient(Config{Addresses: []string{u}})