From e7a701b9ba8a4f6c554216318d84c29ae7398fff Mon Sep 17 00:00:00 2001 From: Christian Heimes Date: Thu, 21 Sep 2023 08:10:34 +0200 Subject: [PATCH] HMS-2606 fix: Fix config loading from env (part 2) Fix 065eb0c4 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 --- internal/config/config.go | 30 +++++++++++++++++++++--------- internal/test/config_helper.go | 2 +- scripts/mk/ephemeral.mk | 16 ++++++++-------- scripts/mk/variables.mk | 4 ++-- 4 files changed, 32 insertions(+), 20 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index 84de3b62..34834211 100644 --- 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,14 +274,13 @@ 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()) } @@ -287,7 +288,7 @@ func Load(cfg *Config) *Config { 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 index 3ed8832a..6342bad9 100644 --- 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 index 89ad6e7c..19cb6229 100644 --- 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 index 63c966e0..4dc3c20f 100644 --- 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)