Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AutomaticEnv does not seem to pull ENV anymore. Broken since 1.19.0, working in 1.18.0 #1895

Open
3 tasks done
bludot opened this issue Jul 31, 2024 · 6 comments
Open
3 tasks done
Labels
kind/bug Something isn't working

Comments

@bludot
Copy link

bludot commented Jul 31, 2024

Preflight Checklist

  • I have searched the issue tracker for an issue that matches the one I want to file, without success.
  • I am not looking for support or already pursued the available support channels without success.
  • I have checked the troubleshooting guide for my problem, without success.

Viper Version

1.19.0

Go Version

1.21

Config Source

Environment variables, Files

Format

INI

Repl.it link

https://replit.com/@jamestrotter4/Go

Code reproducing the issue

package main

import (
	"errors"
	"fmt"
	"log"
	"os"
	"path/filepath"
	"reflect"
	"runtime"
	"strings"

	"github.com/go-playground/validator"
	"github.com/spf13/viper"
)

// config struct

type Config struct {
	Name   string `mapstructure:"name" validate:"required"`
	Nested Nested `mapstructure:"nested"`
}

type Nested struct {
	Val string `mapstructure:"value" validate:"required"`
}

func main() {
	var config Config
	// set env for test
	os.Setenv("NAME", "test")
	os.Setenv("NESTED_VALUE", "test")

	// get the path of the current file
	_, b, _, _ := runtime.Caller(0)
	basepath := filepath.Dir(b)
	err := GetConfig(&config, basepath)
	log.Println(err)

}

func GetConfig(config *Config, cwd string) error {
	if reflect.ValueOf(config).Kind() != reflect.Ptr {
		return errors.New("config must be a pointer")
	}

	//config should be in the same folder that config.go is called from
	viper.AddConfigPath(cwd)
	viper.SetConfigName("config")
	viper.SetConfigType("ini")
	viper.SetEnvKeyReplacer(strings.NewReplacer(`.`, `_`))
	viper.AutomaticEnv()

	// Will not load the config file in development, staging, and production environment. Instead will use configmap

	if err := viper.ReadInConfig(); err != nil {
		if _, ok := err.(viper.ConfigFileNotFoundError); ok {
			log.Println(err)
			log.Println("error loading server env config file (expected)")
		}
	}

	err := viper.Unmarshal(config)
	if err != nil {
		return errors.New("unable to decode into struct")
	}

	// should log values
	log.Println("name: ", config.Name)
	log.Println("nested field: ", config.Nested.Val)

	validate := validator.New()
	if err := validate.Struct(config); err != nil {
		fmt.Print(fmt.Sprintf("%v", err))
		return errors.New("missing required attributes")
	}

	return nil
}

Expected Behavior

Expected to not throw error on validation and for name and nested.val to have a value in logs.

Actual Behavior

config in the end does not have any value, env seems to not be read.

Steps To Reproduce

  1. use version 1.19.0 of viper
  2. use above code (or similar, with AutomaticEnv)
  3. Expect to see no values added

Additional Information

Provided code works with viper version 1.18.0. Ideally I would not want to add BindEnv for every field I want to use. I've tried prefixing the env and other things like that thinking maybe it was missing something. In the end I was not able to get it working.

2024/07/31 12:18:16 Config File "config" Not Found in "[/home/runner/Go]"
2024/07/31 12:18:16 error loading server env config file (expected)

these logs are expected when there is no file. I expect to then get from ENV

Actual

2024/07/31 12:18:16 name:  
2024/07/31 12:18:16 nested field:  
Key: 'Config.Name' Error:Field validation for 'Name' failed on the 'required' tag
Key: 'Config.Nested.Val' Error:Field validation for 'Val' failed on the 'required' tag2024/07/31 12:18:16 missing required attributes

Expected:

2024/07/31 12:18:16 Config File "config" Not Found in "[/home/runner/Go]"
2024/07/31 12:18:16 error loading server env config file (expected)
2024/07/31 12:18:16 name:  test
2024/07/31 12:18:16 nested field:  test
@bludot bludot added the kind/bug Something isn't working label Jul 31, 2024
Copy link

👋 Thanks for reporting!

A maintainer will take a look at your issue shortly. 👀

In the meantime: We are working on Viper v2 and we would love to hear your thoughts about what you like or don't like about Viper, so we can improve or fix those issues.

⏰ If you have a couple minutes, please take some time and share your thoughts: https://forms.gle/R6faU74qPRPAzchZ9

📣 If you've already given us your feedback, you can still help by spreading the news,
either by sharing the above link or telling people about this on Twitter:

https://twitter.com/sagikazarmark/status/1306904078967074816

Thank you! ❤️

@tandem97
Copy link

Same problem. Downgrade to 1.18 solves this problem.

@vyas-git
Copy link

vyas-git commented Aug 28, 2024

Same issue in 1.19 unable to map env to struct.

I checked in changelog after v1.18.2 we need to use viper_bind_struct in tags .

try go run --tags=viper_bind_struct main.go . It works !!

@parnic
Copy link

parnic commented Sep 1, 2024

Confirmed, same thing happening here. Starting with version 1.18.2 and continuing into 1.19.0, the only way an environment variable override gets applied in my case is if a config file exists with the target key in it. If there's no config file, or it doesn't have the key I'm trying to override via env, its value isn't being pulled from the env var.

type AlertConfig struct {
	NotificationType string `mapstructure:"NOTIFICATION_TYPE"`
	NotificationURL  string `mapstructure:"NOTIFICATION_URL"`
}

func loadConfig(path string) (config AlertConfig, err error) {
	viper.AddConfigPath(path)
	viper.SetConfigName("alerts")
	viper.SetConfigType("env")

	viper.AutomaticEnv()

	err = viper.ReadInConfig()
	if err != nil {
		if _, ok := err.(viper.ConfigFileNotFoundError); !ok {
			return
		}
	}

	err = viper.Unmarshal(&config)
	return
}

version <1.18.2: whether an alerts.env exists or not, the final value of config.NotificationType is whatever is in the env var.
version >= 1.18.2: env var override only works if alerts.env exists containing a NOTIFICATION_TYPE=something entry.

parnic added a commit to parnic/beszel that referenced this issue Sep 1, 2024
This provides users the ability to use a wide variety of notification platforms instead of just email. If there's a problem sending a notification via shoutrrr, an error is logged and email is attempted as a fallback.

Since this uses Viper, users can set a notification type and URL via either a config file or environment variable. In the beszel_data folder (where the sqlite dbs reside), create an alerts.env file to set values that way.

Values:
* NOTIFICATION_TYPE
  * If this is `shoutrrr`, then the shoutrrr library is used. Any other value, including not being set, uses the fallback email behavior.
* NOTIFICATION_URL
  * If NOTIFICATION_TYPE is shoutrrr, this is the URL given to shoutrrr to send the alert. See list of supported services: https://containrrr.dev/shoutrrr/services/overview/

Note: there's currently a bug in viper v1.18.2+ where environment variable overrides aren't functioning when no config file exists, so this library should remain pinned to 1.18.1 until that's fixed. See: spf13/viper#1895

Fixes henrygd#71
parnic added a commit to parnic/beszel that referenced this issue Sep 6, 2024
This provides users the ability to use a wide variety of notification platforms instead of just email. If there's a problem sending a notification via shoutrrr, an error is logged and email is attempted as a fallback.

Since this uses Viper, users can set a notification type and URL via either a config file or environment variable. In the beszel_data folder (where the sqlite dbs reside), create an alerts.env file to set values that way.

Values:
* NOTIFICATION_TYPE
  * If this is `shoutrrr`, then the shoutrrr library is used. Any other value, including not being set, uses the fallback email behavior.
* NOTIFICATION_URL
  * If NOTIFICATION_TYPE is shoutrrr, this is the URL given to shoutrrr to send the alert. See list of supported services: https://containrrr.dev/shoutrrr/services/overview/

Note: there's currently a bug in viper v1.18.2+ where environment variable overrides aren't functioning when no config file exists, so this library should remain pinned to 1.18.1 until that's fixed. See: spf13/viper#1895

Fixes henrygd#71
henrygd pushed a commit to henrygd/beszel that referenced this issue Sep 9, 2024
* Add support for alerts via shoutrrr

This provides users the ability to use a wide variety of notification platforms instead of just email. If there's a problem sending a notification via shoutrrr, an error is logged and email is attempted as a fallback.

Since this uses Viper, users can set a notification type and URL via either a config file or environment variable. In the beszel_data folder (where the sqlite dbs reside), create an alerts.env file to set values that way.

Values:
* NOTIFICATION_TYPE
  * If this is `shoutrrr`, then the shoutrrr library is used. Any other value, including not being set, uses the fallback email behavior.
* NOTIFICATION_URL
  * If NOTIFICATION_TYPE is shoutrrr, this is the URL given to shoutrrr to send the alert. See list of supported services: https://containrrr.dev/shoutrrr/services/overview/

Note: there's currently a bug in viper v1.18.2+ where environment variable overrides aren't functioning when no config file exists, so this library should remain pinned to 1.18.1 until that's fixed. See: spf13/viper#1895

* Update documentation

* Log shoutrrr URL instead of unused "to" var
@florianrusch
Copy link

I found a workaround to avoid the config file:

// The following is required to introduce the paths to viper. Else viper
// would not be able to fetch these configurations from environment
// variables as it does not know the key to search for. Only configs which
// don't have any default value (`viper.SetDefault`) needs to be mentioned
// here.
var configsWithoutDefaultValues = []byte(`
nested:
  var:
name:
`)
viper.SetConfigType("yaml")
viper.ReadConfig(bytes.NewBuffer(configsWithoutDefaultValues))

@florianrusch
Copy link

Or another solution would be to set the default e.g. to an empty string:

viper.SetDefault("nested.var", "")

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants