HMS-2606 fix: Fix config loading from env (part 2)

Fix 065eb0c was wrong. `v.AutomaticEnv` is required. It turned out that the root cause of the issue was Makefile's env var `APP`. It conflicts with Viper's map structure `app`. Due to `APP=idmsvc`, Viper resets the entire `Config.Application` struct to empty defaults, then applies additioanl env var settings. Fix: - Rename Makefile variable and export from `APP` to `APP_NAME` - Enable automatic env again Signed-off-by: Christian Heimes <[email protected]>
podengo-project · Sep 21, 2023 · e7a701b · e7a701b
1 parent 6c061b7
commit e7a701b
Show file tree

Hide file tree

Showing 4 changed files with 32 additions and 20 deletions.
diff --git a/internal/config/config.go b/internal/config/config.go
@@ -7,6 +7,7 @@
 package config
 
 import (
+	"encoding/json"
 	"fmt"
 	"os"
 	"strings"
@@ -213,6 +214,7 @@ func setDefaults(v *viper.Viper) {
 	// which is used as HMAC key. The string "random" creates an random,
 	// ephemeral key for testing.
 	v.SetDefault("app.domain_reg_key", "")
+	v.SetDefault("app.debug", false)
 }
 
 func setClowderConfiguration(v *viper.Viper, clowderConfig *clowder.AppConfig) {
@@ -258,7 +260,7 @@ func setClowderConfiguration(v *viper.Viper, clowderConfig *clowder.AppConfig) {
 	v.Set("metrics.port", clowderConfig.MetricsPort)
 }
 
-func Load(cfg *Config) *Config {
+func Load(cfg *Config) *viper.Viper {
 	var (
 		err error
 	)
@@ -272,22 +274,21 @@ func Load(cfg *Config) *Config {
 	v.SetConfigName("config.yaml")
 	v.SetConfigType("yaml")
 	v.SetEnvKeyReplacer(strings.NewReplacer(".", "_"))
-	// HMS-2606: Do not set v.AutomaticEnv(). Viper looks up values with
-	// precedence "env", "config", "default". Automatic env seems to make
-	// Viper ignore config and default when at least one env var is set for
-	// a sub-struct.
+	v.AutomaticEnv()
+
 	setDefaults(v)
 	if clowder.IsClowderEnabled() {
 		setClowderConfiguration(v, clowder.LoadedConfig)
 	}
+
 	if err = v.ReadInConfig(); err != nil {
 		log.Warn().Msgf("Not using config.yaml: %s", err.Error())
 	}
 	if err = v.Unmarshal(cfg); err != nil {
 		log.Warn().Msgf("Mapping to configuration: %s", err.Error())
 	}
 
-	return cfg
+	return v
 }
 
 func reportError(err error) {
@@ -296,12 +297,11 @@ func reportError(err error) {
 	for _, err := range err.(validator.ValidationErrors) {
 		fmt.Fprintf(
 			os.Stderr,
-			"    '%s': rule: %v %v, got: %v (type: %v)\n",
+			"    %s: rule: %v %v, got: '%v' (type: %v)\n",
 			err.Namespace(),
 			err.Tag(), err.Value(), err.Param(), err.Kind(),
 		)
 	}
-	panic("Validation failed")
 }
 
 func Validate(cfg *Config) (err error) {
@@ -315,9 +315,21 @@ func Get() *Config {
 		return config
 	}
 	config = &Config{}
-	config = Load(config)
+	v := Load(config)
+
+	// Dump configuration as JSON
+	if config.Logging.Level == "debug" {
+		c := v.AllSettings()
+		b, err := json.MarshalIndent(c, "", "  ")
+		if err != nil {
+			panic(err)
+		}
+		fmt.Println(string(b))
+	}
+
 	if err := Validate(config); err != nil {
 		reportError(err)
+		panic("Invalid configuration")
 	}
 	return config
 }

diff --git a/internal/test/config_helper.go b/internal/test/config_helper.go
@@ -5,7 +5,7 @@ import "github.com/podengo-project/idmsvc-backend/internal/config"
 // Config for testing
 func GetTestConfig() (cfg *config.Config) {
 	cfg = &config.Config{}
-	cfg = config.Load(cfg)
+	config.Load(cfg)
 	// override some default settings
 	cfg.Application.DomainRegTokenKey = "random"
 	cfg.Application.PaginationDefaultLimit = 10

diff --git a/scripts/mk/ephemeral.mk b/scripts/mk/ephemeral.mk
@@ -3,8 +3,8 @@
 # ephemeral-setup: $(BONFIRE) ## Configure bonfire to run locally
 # 	$(BONFIRE) config write-default > $(PROJECT_DIR)/config/bonfire-config.yaml
 
-ifeq (,$(APP))
-$(error APP is empty; did you miss to set APP=my-app at your scripts/mk/variables.mk)
+ifeq (,$(APP_NAME))
+$(error APP_NAME is empty; did you miss to set APP_NAME=my-app at your scripts/mk/variables.mk)
 endif
 
 APP_COMPONENT ?= backend
@@ -115,7 +115,7 @@ ephemeral-deploy: $(EPHEMERAL_DEPS) ## Deploy application using 'config/bonfire.
 		--set-parameter "$(APP_COMPONENT)/IMAGE=$(CONTAINER_IMAGE_BASE)" \
 		--set-parameter "$(APP_COMPONENT)/IMAGE_TAG=$(CONTAINER_IMAGE_TAG)" \
 		$(EPHEMERAL_OPTS) \
-		"$(APP)"
+		"$(APP_NAME)"
 
 # NOTE Changes to config/bonfire.yaml could impact to this rule
 .PHONY: ephemeral-undeploy
@@ -127,7 +127,7 @@ ephemeral-undeploy: $(BONFIRE) $(JSON2YAML) ## Undeploy application from the cur
 		--set-parameter "$(APP_COMPONENT)/IMAGE=$(CONTAINER_IMAGE_BASE)" \
 		--set-parameter "$(APP_COMPONENT)/IMAGE_TAG=$(CONTAINER_IMAGE_TAG)" \
 		$(EPHEMERAL_OPTS) \
-		"$(APP)" 2>/dev/null | $(JSON2YAML) | oc delete -f -
+		"$(APP_NAME)" 2>/dev/null | $(JSON2YAML) | oc delete -f -
 	! oc get secrets/content-sources-certs &>/dev/null || oc delete secrets/content-sources-certs
 
 .PHONY: ephemeral-process
@@ -139,11 +139,11 @@ ephemeral-process: $(BONFIRE) $(JSON2YAML) ## Process application from the curre
 		--set-parameter "$(APP_COMPONENT)/IMAGE=$(CONTAINER_IMAGE_BASE)" \
 		--set-parameter "$(APP_COMPONENT)/IMAGE_TAG=$(CONTAINER_IMAGE_TAG)" \
 		$(EPHEMERAL_OPTS) \
-		"$(APP)" 2>/dev/null | $(JSON2YAML)
+		"$(APP_NAME)" 2>/dev/null | $(JSON2YAML)
 
 .PHONY: ephemeral-db-cli
 ephemeral-db-cli: ## Open a database client
-	POD="$(shell oc get pods -l service=db,app=$(APP)-$(APP_COMPONENT) -o jsonpath='{.items[0].metadata.name}')" \
+	POD="$(shell oc get pods -l service=db,app=$(APP_NAME)-$(APP_COMPONENT) -o jsonpath='{.items[0].metadata.name}')" \
 	&& oc exec -it pod/"$${POD}" -- bash -c 'exec psql -U $$POSTGRESQL_USER -d $$POSTGRESQL_DATABASE'
 
 # TODO Add command to specify to bonfire the clowdenv template to be used
@@ -188,9 +188,9 @@ ephemeral-build-deploy:  ## Build and deploy image using 'build_deploy.sh' scrip
 ephemeral-test-backend: $(BONFIRE) ## Run IQE tests in the ephemeral environment (require to run ephemeral-deploy before)
 	$(BONFIRE) deploy-iqe-cji \
 	  --env clowder_smoke \
-	  --cji-name "$(APP)-$(APP_COMPONENT)" \
+	  --cji-name "$(APP_NAME)-$(APP_COMPONENT)" \
 	  --namespace "$(NAMESPACE)" \
-	  "$(APP)"
+	  "$(APP_NAME)"
 
 # https://kubernetes.io/docs/tasks/administer-cluster/dns-debugging-resolution/
 .PHONY: ephemeral-run-dnsutil

diff --git a/scripts/mk/variables.mk b/scripts/mk/variables.mk
@@ -1,8 +1,8 @@
 ##
 # Set default variable values for the project
 ##
-APP ?= idmsvc
-export APP
+APP_NAME ?= idmsvc
+export APP_NAME
 
 BIN ?= bin
 PATH := $(CURDIR)/$(BIN):$(PATH)