From 1f39998ff86c823a144b3e56aee11572707c7910 Mon Sep 17 00:00:00 2001
From: nielskrijger <niels@kryger.nl>
Date: Sat, 11 Sep 2021 12:54:27 +0200
Subject: [PATCH] fix: allow elasticsearch settings to be read from ENV vars if
 no defaults are defined within yaml files

For more information see: https://github.com/spf13/viper/issues/761
---
 Makefile                                         |  6 +++++-
 config.go                                        |  2 +-
 config_test.go                                   |  2 ++
 docker-compose.yml                               |  2 +-
 elasticsearch.go                                 | 16 +++++++---------
 elasticsearch_test.go                            | 15 ++++++++++++---
 .../elasticsearch/config.using-env-vars.yaml     |  3 +++
 7 files changed, 31 insertions(+), 15 deletions(-)
 create mode 100644 testdata/elasticsearch/config.using-env-vars.yaml

diff --git a/Makefile b/Makefile
index 4e339ad..25a5743 100644
--- a/Makefile
+++ b/Makefile
@@ -14,7 +14,11 @@ integration:
 
 .PHONY: humantest
 humantest:
-	LOG_HUMAN=true richgo test -v -p=1 -timeout=60s $(PKGS)
+ifndef run
+	LOG_DEBUG=true LOG_HUMAN=true richgo test -v -p=1 -timeout=60s $(PKGS)
+else
+	LOG_DEBUG=true LOG_HUMAN=true richgo test -v -p=1 -timeout=60s $(PKGS) -run $(run)
+endif
 
 .PHONY: coverage
 coverage:
diff --git a/config.go b/config.go
index 403b5b3..2b34407 100644
--- a/config.go
+++ b/config.go
@@ -58,7 +58,7 @@ func LoadConfig(log zerolog.Logger, dir string, env string) (*viper.Viper, error
 	}
 
 	// Viper ignores environment variables when unmarshalling if no defaults are set.
-	// This should that, see also https://github.com/spf13/viper/issues/188
+	// This should fix that in some scenarios, see also https://github.com/spf13/viper/issues/188
 	for _, key := range v.AllKeys() {
 		val := v.Get(key)
 		v.Set(key, val)
diff --git a/config_test.go b/config_test.go
index a27239d..cdc16cb 100644
--- a/config_test.go
+++ b/config_test.go
@@ -63,4 +63,6 @@ func TestConfig_OverrideEnvVariables(t *testing.T) {
 	err = cfg.Sub("vars").Unmarshal(cfgStruct)
 	assert.Nil(t, err)
 	assert.Equal(t, "from-env", cfgStruct.Filename)
+
+	os.Clearenv()
 }
diff --git a/docker-compose.yml b/docker-compose.yml
index 4c2475b..0f47e66 100644
--- a/docker-compose.yml
+++ b/docker-compose.yml
@@ -14,7 +14,7 @@ services:
       - "5432:5432"
 
   elasticsearch:
-    image: docker.elastic.co/elasticsearch/elasticsearch:7.12.0
+    image: docker.elastic.co/elasticsearch/elasticsearch:7.14.1
     environment:
       - bootstrap.memory_lock=true
       - discovery.type=single-node
diff --git a/elasticsearch.go b/elasticsearch.go
index 7ab0682..8598865 100644
--- a/elasticsearch.go
+++ b/elasticsearch.go
@@ -44,15 +44,13 @@ func (s *Elasticsearch) Name() string {
 func (s *Elasticsearch) Configure(ctx *AppEnv) error {
 	s.log = ctx.Log
 
-	// unmarshal config and set defaults
-	s.Config = &elasticsearch7.Config{}
-
-	if !ctx.Config.InConfig("elasticsearch") {
-		return errMissingElasticsearchConfig
-	}
-
-	if err := ctx.Config.Sub("elasticsearch").Unmarshal(&s.Config); err != nil {
-		return fmt.Errorf("parsing elasticsearch configuration: %w", err)
+	// Fetch config from viper. Avoid unmarshal directly into elasticsearch7.Config
+	// as it doesn't work with env vars:
+	// https://github.com/spf13/viper/issues/761
+	s.Config = &elasticsearch7.Config{
+		Addresses: ctx.Config.GetStringSlice("elasticsearch.addresses"),
+		Username:  ctx.Config.GetString("elasticsearch.username"),
+		Password:  ctx.Config.GetString("elasticsearch.password"),
 	}
 
 	if len(s.Config.Addresses) == 0 {
diff --git a/elasticsearch_test.go b/elasticsearch_test.go
index 6ce4166..94f1a67 100644
--- a/elasticsearch_test.go
+++ b/elasticsearch_test.go
@@ -2,6 +2,7 @@ package goboot_test
 
 import (
 	"context"
+	"os"
 	"testing"
 
 	"github.com/nielskrijger/goboot"
@@ -32,10 +33,18 @@ func TestElasticsearch_Success(t *testing.T) {
 	assert.NotNil(t, s.Config)
 }
 
-func TestElasticsearch_ErrorMissingConfig(t *testing.T) {
+func TestElasticsearch_SuccessEnvs(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping integration test")
+	}
+
 	s := &goboot.Elasticsearch{}
-	err := s.Configure(goboot.NewAppEnv("./testdata/elasticsearch", ""))
-	assert.EqualError(t, err, "missing \"elasticsearch\" configuration")
+	_ = os.Setenv("ELASTICSEARCH_USERNAME", "elastic")
+	_ = os.Setenv("ELASTICSEARCH_PASSWORD", "secret")
+
+	err := s.Configure(goboot.NewAppEnv("./testdata/elasticsearch", "using-env-vars"))
+	assert.Nil(t, err)
+	os.Clearenv()
 }
 
 func TestElasticsearch_ErrorNoAddresses(t *testing.T) {
diff --git a/testdata/elasticsearch/config.using-env-vars.yaml b/testdata/elasticsearch/config.using-env-vars.yaml
new file mode 100644
index 0000000..e1dccda
--- /dev/null
+++ b/testdata/elasticsearch/config.using-env-vars.yaml
@@ -0,0 +1,3 @@
+elasticsearch:
+  addresses:
+    - http://localhost:9200