opts icon indicating copy to clipboard operation
opts copied to clipboard

env vars should have the program name as prefix

Open marco-m opened this issue 5 years ago • 2 comments

Hello,

thanks for opts!

I think that when generating environment variable names (either when using opts:"env" without the =FOO part or when using UseEnv(), the environment variables should always have a prefix.

The default prefix should be the auto-detected program name (as already used in the --help output), and maybe (but personally I am not convinced about this) with a way to explicitly set it to what the user wants.

The rationale for requiring the prefix is to avoid unexpected errors caused by common env var names that can be present in the enviroment. Names such as MAX_FOO, MIN_FOO, FILE, EDITOR, HOST, PORT, USER, PASSWORD, ... are way to easy to find around. It is too dangerous to allow to pick them up. This problem already happened in other Go packages (cannot find the reference right now).

This would be a breaking change, and in my personal opinion this would be fine, it is for the greater good :-)

marco-m avatar Jun 28 '19 16:06 marco-m

Interesting, I can see where this would be useful, like in very large CLIs where you want to expose everything

Usually when I find clashes, like "User string" I usually explicitly set "env=HTTP_USER" etc. Also a common convention is to listen on PORT, this would break if it was prefixed with program name.

This could be an extra method: PrefixEnv() and/or PrefixEnvWith("PROG_")

On Sat, 29 Jun 2019 at 2:45 am Marco Molteni [email protected] wrote:

Hello,

thanks for opts!

I think that when generating environment variable names (either when using opts:"env" without the =FOO part or when using UseEnv(), the environment variables should always have a prefix.

The default prefix should be the auto-detected program name (as already used in the --help output), and maybe (but personally I am not convinced about this) with a way to explicitly set it to what the user wants.

The rationale for requiring the prefix is to avoid unexpected errors caused by common env var names that can be present in the enviroment. Names such as MAX_FOO, MIN_FOO, FILE, EDITOR, HOST, PORT, USER, PASSWORD, ... are way to easy to find around. It is too dangerous to allow to pick them up. This problem already happened in other Go packages (cannot find the reference right now).

This would be a breaking change, and in my personal opinion this would be fine, it is for the greater good :-)

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/jpillora/opts/issues/20?email_source=notifications&email_token=AAE2X4YCM3FTDX32OY5WS2LP4Y52PA5CNFSM4H4G6JAKYY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4G4LNKEA, or mute the thread https://github.com/notifications/unsubscribe-auth/AAE2X4YTJCSKQGZWAKPHDALP4Y52PANCNFSM4H4G6JAA .

jpillora avatar Jun 28 '19 17:06 jpillora

I can see where this would be useful, like in very large CLIs where you want to expose everything

I don't think it is a question of large/small. One clash is enough. The problem is that a clash introduces a subtle bug and makes certain things impossible for the user of the program.

One example. Say I am configuring a Docker container with a system that actually needs two programs, foo and bar. Say that both support a port. If the only way I have is to set the env var PORT, I have no way out. If on the other hand the names are FOO_PORT and BAR_PORT, the problem is solved.

This is not the same as your other example:

Usually when I find clashes, like "User string" I usually explicitly set "env=HTTP_USER"

because I am thinking about the user of my program, not about me as the programmer :-)

Also a common convention is to listen on PORT, this would break if it was prefixed with program name.

IMHO this convention is wrong :-)

Another problem is not a clash, is a subtle bug. This is what I was trying to convey in my first writeup. Imagine program hello that has a setting more subtle than port with a default value. I run it and it works, I don't even have to pass any options, all the defaults are fine. Then the program is run in an environment where an enviroment variable is set, a leftover of something else. This can change the behavior of the program in a subtle way that requires time to troubleshoot.

Another way of looking at it is that env vars are global state. They should at least have a namespace.

At least this is my point of view, from years of experience seeing things going wrong for the most unexpected reasons :-)

marco-m avatar Jun 28 '19 18:06 marco-m