add .env file support

[dolanor]

Hi,

Given that I use Docker quite a lot when coding, I also use the .env file to pass configuration to the docker-compose.
But given the speed of go run compared to docker-compose up -d --build, I want to test more with the former.

So here is the package that helps me do that without sourcing the .env file in my shell (and mostly, forgetting about re-sourcing it).

So now, running the app with a docker-compose up -d --build or go run does not require another workflow.

It is a copy paste from the ff.PlainParser that use = as the key/value split.
It also has an optional parser that accepts a prefix as the .env often has the variable with prefix to avoid conflicts with other services, as in env variables. I'm not sure it's the best way to do it, but I thought it was maybe the less intrusive since it's quite specific to the handling of .env files. I'm open to suggestions.

[peterbourgon]

Ah, cool! Let me give this a closer review. Are there existing env file format parsers in Go that I can review as a comparison?

[dolanor]

Maybe in docker as the v3+ compose file must be parsed by it to execute in a docker swarm environment. Otherwise, the logic is in the docker-compose tool that is only in python.

Otherwise I found this one: https://github.com/joho/godotenv

[peterbourgon]

I think in this parser we don't want to interpret lines of the form

FOO_BAR

as setting boolean flags to true. I think they should probably be errors.

[dolanor]

I think that's fair.

[dolanor]

this is fixed in 23a2085

[peterbourgon]

It's not a given that a _ in an env var should match to only a - in a flag name. Quoting from the docs

Flag names are matched to environment variables by capitalizing the flag name, and replacing separator characters like periods or hyphens with underscores.

We would need the same kind of matching behavior here.

[dolanor]

So for each separator in []string{"-", ".", "/"}, I should check that a flag exist with that format, or should I set it for each separator?
So given: MY_FOO_LISTEN_ADDR=value with prefix MY_FOO, I should set listen.addr, listen-addr and listen/addr, hoping there is one corresponding, and there is no conflict either.
Or iterate through existing flags (how in this context?) and do the other way around (like in the ff.Parse func)?

[peterbourgon]

I'm not sure what the correct implementation is, my point is that the behavior here should try to be consistent with env vars specified directly. If you're stuck, let me know, and I can dig in to the details and make a more direct suggestion.

[dolanor]

I went on the path where the option needs a flag.FlagSet passed, so it can match with the env var.
Concerning #69 , when the env var from a .env file is applied, it will apply itself on every flag.
So, FOO_VAR will get applied to -foo_var, -foo-var, -foo.var and -foo/var. When using purely the flags, it can determine exactly which to update.
The updated code has been pushed in the branch now.

[peterbourgon]

Please find a way to test these corner cases of env files without modifying fftest.Vars.

[dolanor]

I added them in their own local struct and removed reference to them in other part of the module.

[xeoncross]

I'm interested in seeing this PR resolved, however, after looking at the code I'm curious why PlainParser and EnvParser aren't just merged since I am (mostly) using PlainParser to parse .env files with a couple small tweaks.

Original:

ff/parse.go

Lines 231 to 268 in 556df77

	// PlainParser is a parser for config files in an extremely simple format. Each
	// line is tokenized as a single key/value pair. The first whitespace-delimited
	// token in the line is interpreted as the flag name, and all remaining tokens
	// are interpreted as the value. Any leading hyphens on the flag name are
	// ignored.
	func PlainParser(r io.Reader, set func(name, value string) error) error {
	s := bufio.NewScanner(r)
	for s.Scan() {
	line := strings.TrimSpace(s.Text())
	if line == "" {
	continue // skip empties
	}
	if line[0] == '#' {
	continue // skip comments
	}
	var (
	name string
	value string
	index = strings.IndexRune(line, ' ')
	)
	if index < 0 {
	name, value = line, "true" // boolean option
	} else {
	name, value = line[:index], strings.TrimSpace(line[index:])
	}
	if i := strings.Index(value, " #"); i >= 0 {
	value = strings.TrimSpace(value[:i])
	}
	if err := set(name, value); err != nil {
	return err
	}
	}
	return nil
	}

// EnvParser is a parser for config files in an extremely simple format. Each
// line is tokenized as a single key/value pair. The first whitespace-delimited
// token in the line is interpreted as the flag name, and all remaining tokens
// are interpreted as the value. Any leading hyphens on the flag name are
// ignored.
func EnvParser(r io.Reader, set func(name, value string) error) error {
	s := bufio.NewScanner(r)
	for s.Scan() {
		line := strings.TrimSpace(s.Text())
		if line == "" {
			continue // skip empties
		}

		if line[0] == '#' {
			continue // skip comments
		}

		var (
			name  string
			value string
			index = strings.IndexRune(line, '=')
		)

		if index < 0 {
			name, value = line, "true" // boolean option
		} else {
			name, value = line[:index], strings.TrimSpace(line[index+1:])
		}

		// remove ending comment
		if i := strings.Index(value, " #"); i >= 0 {
			value = strings.TrimSpace(value[:i])
		}

		if err := set(name, value); err != nil {
			return err
		}
	}
	return nil
}

[peterbourgon]

Looks like the main tweak is swapping for =, any others?

[xeoncross]

@peterbourgon yes, you have to exclude the '=' (line[index+1:] vs line[index:]) but other than that, you already wrote an .env parser. I'm kind of surprised you decided to roll your own simple format here instead of just using .env files.

Here's a full demo example: https://gist.github.com/Xeoncross/707ee4a6a7fff8b73baa74a1071c382e

In fact, I didn't like having all that duplicate code so I just made a wasteful wrapper that reads the whole massive 200-500 byte .env file stream into memory, removes the equals signs, then feeds it to ff.PlainParser.

func EnvParser(r io.Reader, set func(name, value string) error) error {
	b, err := ioutil.ReadAll(r)
	if err != nil {
		return err
	}
	r = bytes.NewReader(bytes.ReplaceAll(b, []byte("="), []byte(" ")))
	return ff.PlainParser(r, set)
}

[dolanor]

In fact, I didn't like having all that duplicate code so I just made a wasteful wrapper that reads the whole massive 200-500 byte .env file stream into memory, removes the equals signs, then feeds it to ff.PlainParser.

func EnvParser(r io.Reader, set func(name, value string) error) error {
	b, err := ioutil.ReadAll(r)
	if err != nil {
		return err
	}
	r = bytes.NewReader(bytes.ReplaceAll(b, []byte("="), []byte(" ")))
	return ff.PlainParser(r, set)
}

Hi,
I think it's a smart trick depending on your cases, but if you have some urls with query parameters as env var, you'll break them.

DB_URL=postgres://localhost:5432/db?sslmode=disable

[xeoncross]

Yes, I'd rather see proper env config parsing, but I construct the DSN in-app and don't have any other URL params in the config and if I did I'd just use the first function so either of these two methods works for now until this PR or some related one gets merged.

[peterbourgon]

I explicitly wanted a config file format whose keys were tokenized with spaces, so the PlainParser would have existed in any case. But to be honest I hadn't ever heard of the env file format until people started bringing it up in issues on this repo. Ignorance! 😇

I'll add support for env files shortly. @xeoncross is your "proper" link a good canonical parser? I'll use it as a reference for a local implementation, if so.

[dolanor]

What was missing from this PR to be merged, actually? I thought I dealt with every comments/suggestions.

[peterbourgon]

@dolanor Lots of stuff :\ ConfigFileParsers don't get access to the flag set, and certainly can't mutate it. You're re-implementing env var name logic rather than reusing it. Using break labels in loops. Other stuff. No big deal, I'm planning on adding support. Thanks for the contribution in any case! 👍

[xeoncross]

Quick update for search results, I still prefer spinning up new services using ff over cobra given the lighter weight codebase, simpler setup, and easier auditability. However, since ff doesn't support environment files (which are commonly used with docker or local dev) an easy workaround is to also use the github.com/joho/godotenv package which you can setup yourself, or just use the default, 1-line autoloader.

package main

import (
	...

	_ "github.com/joho/godotenv/autoload"
	"github.com/peterbourgon/ff"
)

Once godotenv loads everything into os.Env, then ff picks it up.

[peterbourgon]

Fixed by #89.

@xeoncross, @dolanor — if you get a chance can you test out the RC I'm about to publish?