-
Notifications
You must be signed in to change notification settings - Fork 146
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
chore: allow to use custom environment #214
base: master
Are you sure you want to change the base?
Conversation
imo, this testing aspect does not really belong into the provider. I think that this is a pretty common use case where you can use the flexibility of koanf and multiple providers to achieve your goal: package main
import (
"encoding/json"
"fmt"
"os"
"strings"
"github.com/knadh/koanf/providers/confmap"
"github.com/knadh/koanf/providers/env"
"github.com/knadh/koanf/v2"
)
type Config struct {
Name string `koanf:"svc.name"`
Port int `koanf:"svc.port"`
Hostname string `koanf:"svc.hostname"`
}
func loadEnv(cfg any, override ...map[string]any) error {
const (
koanfDelim = "."
envDelim = "_"
)
envToKoanf := func(s string) string {
return strings.ToLower(strings.ReplaceAll(s, envDelim, koanfDelim))
}
k := koanf.New(koanfDelim)
err := k.Load(env.Provider("", envDelim, envToKoanf), nil)
if err != nil {
return err
}
for _, o := range override {
mapped := make(map[string]any, len(o))
for k, v := range o {
mapped[envToKoanf(k)] = v
}
_ = k.Load(confmap.Provider(mapped, envDelim), nil)
}
return k.Unmarshal("", cfg)
}
func main() {
// testing with
os.Setenv("SVC_PORT", "1234")
os.Setenv("SVC_NAME", "my-service")
cfg := Config{}
err := loadEnv(&cfg, map[string]any{
"SVC_PORT": 3306,
"SVC_HOSTNAME": "localhost",
})
if err != nil {
panic(err)
}
data, _ := json.MarshalIndent(cfg, "", " ")
fmt.Println(string(data))
/*
expected:
{
"Name": "my-service",
"Port": 3306,
"Hostname": "localhost"
}
*/
} https://go.dev/play/p/ZsxOgp4YgBh Maybe confmap might need an additional provider constructor that maps the keys before setting them to its internal map. Koanf allows you to merge different configurations with more or less any kind of logic in between. package main
import (
"encoding/json"
"fmt"
"os"
"strings"
"github.com/knadh/koanf/providers/confmap"
"github.com/knadh/koanf/providers/env"
"github.com/knadh/koanf/v2"
)
type Config struct {
Name string `koanf:"svc.name"`
Port int `koanf:"svc.port"`
Hostname string `koanf:"svc.hostname"`
}
func loadEnv(cfg any, override ...map[string]any) error {
const (
koanfDelim = "."
envDelim = "_"
)
envToKoanf := func(s string) string {
return strings.ToLower(strings.ReplaceAll(s, envDelim, koanfDelim))
}
k := koanf.New(koanfDelim)
num := 0
for _, m := range override {
num += len(m)
}
if num == 0 {
err := k.Load(env.Provider("", envDelim, envToKoanf), nil)
if err != nil {
return err
}
} else {
for _, o := range override {
mapped := make(map[string]any, len(o))
for k, v := range o {
mapped[envToKoanf(k)] = v
}
_ = k.Load(confmap.Provider(mapped, envDelim), nil)
}
}
return k.Unmarshal("", cfg)
}
func main() {
// testing with
os.Setenv("SVC_PORT", "1234")
os.Setenv("SVC_NAME", "my-service")
cfg := Config{}
err := loadEnv(&cfg, map[string]any{
"SVC_PORT": 3306,
"SVC_HOSTNAME": "localhost",
})
if err != nil {
panic(err)
}
data, _ := json.MarshalIndent(cfg, "", " ")
fmt.Println(string(data))
/*
expected:
{
"Name": "",
"Port": 3306,
"Hostname": "localhost"
}
*/
cfg2 := Config{}
err = loadEnv(&cfg2)
if err != nil {
panic(err)
}
data, _ = json.MarshalIndent(cfg2, "", " ")
fmt.Println(string(data))
/*
expected:
{
"Name": "my-service",
"Port": 3306,
"Hostname": ""
}
*/
} |
Well you are correct, but you won’t be able to test the whole env logic, namely the callback functionality and whether the environment variable values land on the right path or not. |
do you have an example of such a test? my first thoughts would be:
|
Simple example: package main
import (
"os"
"strings"
"testing"
"github.com/knadh/koanf/providers/env"
koanf "github.com/knadh/koanf/v2"
"github.com/mitchellh/mapstructure"
"github.com/pkg/errors"
"github.com/stretchr/testify/require"
)
type Config struct {
Users []string `conf_key:"users_credentials"`
Admins []string `conf_key:"admins_credentials"`
}
func LoadConfig() (*Config, error) {
k := koanf.New(".")
err := k.Load(env.ProviderWithValue("APP_", ".", func(s, v string) (string, any) {
s = strings.ToLower(s)
s = strings.Replace(s, "app_", "", 1)
parts := strings.FieldsFunc(v, func(r rune) bool {
return r == ',' || r == ';' || r == '|'
})
switch len(parts) {
case 0:
return s, nil
case 1:
return s, parts[0]
default:
return s, parts
}
}), nil)
if err != nil {
return nil, errors.Wrapf(err, "unable to load config from environment")
}
var out Config
unmarshalConf := koanf.UnmarshalConf{
Tag: "config_key",
FlatPaths: false,
DecoderConfig: &mapstructure.DecoderConfig{
DecodeHook: nil,
TagName: "config_key",
Result: &out,
WeaklyTypedInput: true,
},
}
if err := k.UnmarshalWithConf("", &out, unmarshalConf); err != nil {
return nil, errors.Wrap(err, "unable to unmarshal config")
}
return &out, nil
}
func TestLoadConfig(t *testing.T) {
os.Setenv("APP_USERS", "Joe,Alice")
os.Setenv("APP_ADMINS", "root")
defer os.Unsetenv("APP_USERS")
defer os.Unsetenv("APP_ADMINS")
cfg, err := LoadConfig()
require.NoError(t, err)
require.Equal(t, cfg, &Config{
Users: []string{"Joe", "Alice"},
Admins: []string{"root"},
})
} |
This might work for your tests: package main
import (
"strings"
"testing"
"github.com/knadh/koanf/providers/confmap"
"github.com/knadh/koanf/providers/env"
"github.com/knadh/koanf/v2"
"github.com/mitchellh/mapstructure"
"github.com/pkg/errors"
"github.com/stretchr/testify/require"
)
type Config struct {
Users []string `conf_key:"users_credentials"`
Admins []string `conf_key:"admins_credentials"`
}
// if override is set no env variables are used
func LoadConfig(override ...map[string]string) (*Config, error) {
k, err := loadEnv(envToKoanfValue, override...)
if err != nil {
return nil, errors.Wrap(err, "unable to load env variables")
}
var out Config
unmarshalConf := koanf.UnmarshalConf{
Tag: "config_key",
FlatPaths: false,
DecoderConfig: &mapstructure.DecoderConfig{
DecodeHook: nil,
TagName: "config_key",
Result: &out,
WeaklyTypedInput: true,
},
}
if err := k.UnmarshalWithConf("", &out, unmarshalConf); err != nil {
return nil, errors.Wrap(err, "unable to unmarshal config")
}
return &out, nil
}
func loadEnv(cb func(string, string) (string, any), override ...map[string]string) (*koanf.Koanf, error) {
k := koanf.New(".")
num := 0
for _, m := range override {
num += len(m)
}
if num == 0 {
err := k.Load(env.ProviderWithValue("APP_", ".", cb), nil)
if err != nil {
return nil, errors.Wrapf(err, "unable to load config from environment")
}
} else {
for _, o := range override {
mapped := make(map[string]any, len(o))
for k, v := range o {
mk, mv := cb(k, v)
mapped[mk] = mv
}
err := k.Load(confmap.Provider(mapped, "."), nil)
if err != nil {
return nil, err
}
}
}
return k, nil
}
func envToKoanfValue(k, v string) (string, any) {
k = strings.ToLower(k)
k = strings.Replace(k, "app_", "", 1)
parts := strings.FieldsFunc(v, func(r rune) bool {
return r == ',' || r == ';' || r == '|'
})
switch len(parts) {
case 0:
return k, nil
case 1:
return k, parts[0]
default:
return k, parts
}
}
func TestLoadConfig(t *testing.T) {
cfg, err := LoadConfig(map[string]string{
"APP_USERS": "Joe,Alice",
"APP_ADMINS": "root",
})
require.NoError(t, err)
require.Equal(t, cfg, &Config{
Users: []string{"Joe", "Alice"},
Admins: []string{"root"},
})
} |
I am well aware that I can restructure my code in a way that the logic can be tested, I am not looking for a solution like this because it breaks the simplicity of the code. |
in regard to the pr code:
my two cents :). |
I agree that having an addition constructor would be the cleanest, but what about the callback methods (that are used in the other two constructors)? Which one should we choose? |
I think you would need to extend ProviderWithValue to have an additional parameter map[string]string constructor implementation: |
I am fine with the option implementation. Do you think its feasible to break api? |
We can't break the constructor as There's an issue of ambiguity with this approach. With an additional variable, be it a constructor or a variable in the config, the provider switches to using os.Environ automatically if the variable is empty. What if it has to be legitimately empty? Here, the empty/len vs. nil check isn't explicit. |
Good catch @knadh!
|
I'd say yes. Update your PR code and you will get feedback as fas as I can say. |
I made the changes, please have a look now. I removed some tests because the main logic now sits in |
Could you add an option that's called WithEnvironmentMap. |
(Y) |
This works well, but thinking about the option function pattern. |
Yes that is correct, but what are the alternatives? |
Sorry, picking this up again. You can maintain this implementation separately on your repo. We can list it on the README in case others want to use your implementation. |
So this means you don't want to merge because of? |
Don't want to introduce the functional option pattern into this provider which would be inconsistent with other providers. If there's more demand for this particular usecase, we can consider it. |
Ok, would you accept a pr with a function like |
Sorry, lost track of this PR. Yes, |
@knadh Ok thanks for confirmation. |
When using the
env
provider in tests it is always necessary to set environment values beforehand:With this change it would be possible to have a pattern like this:
The reason for having this is to stop pollutiong the global environment state.
Especially on test failures the current design becomes a source of problems for future tests.
Following alternatives are also possible: