Support no_prefix in flag

[noahhai]

Allow json for secret/prefix flag for full configuration.

[hashicorp-cla]

All committers have signed the CLA.

[noahhai]

#198

[eikenb]

Hey @noahhai, thanks for the submission.

While testing I ran into this same issue, no way to specify no-prefix without using a config file. My gut was to just add -no-prefix, that is when using it you'd want a single, flat namespace. The issue about this, #198, seems to be asking for something similar.

Your approach would allow no-prefix to be different per-prefix. Can you provide a case where this would be useful. I'd hate to add the additional complications without at least having an idea of how useful it would be. Sorry to have to ask, I'm just new to this tool and am reliant upon the community to help me get a good understanding of all the use cases.

Thanks.

[noahhai]

Hey @eikenb, yeah I think adding -no-prefix global flag would work fine in my use case as well. I I gravitated towards making all options available for multiple secrets|prefixes because of the way I'm using envconsul; a base config file in kubernetes via a ConfigMap, shared by different containers/pods, and then for the actual container args, mixing in multiple secrets to the binary's environment. So yeah, to answer your question, I can't really think of a case where you'd want -no-prefix for some secrets but not others, guess I was just erring on the side of greater configurability (also with the format parameter, though I haven't needed to use that personally).

[noahhai]

And yeah, not very familiar with using the format parameter. Maybe if someone were also doing mixins, they might want different formats on a per-secret basis? But of course they could solve this using config file, so I see where this might be too edge of a use case to want to do in this way :)

[eikenb]

Thanks for the detailed response @noahhai, it's useful to know not only your use-case but your reasoning as well.

Given what you said I think I'd prefer to go with the simpler -no-prefix option. Though there is a new consideration, that no-prefix is now supported for both vault secrets and consul keys. The twist is that each vault defaults to no-prefix=false while consul defaults to no-prefix=true. This is unfortunate, but when adding no-prefix to consul keys we didn't want to break existing uses and previously consul didn't support no-prefix and just never added the prefixes.

So, if you still want to implement this, what I think would be good now would be to add -no-prefix but have it not set the option when it wasn't passed (to it can remain nil and maintain the funky differing default behaviors). The best way to do this would be using the funcBoolVar type in flags.go, similar to how -consul-retry and -consul-ssl are done.

Thanks.

PS. If you don't want to do this just let me know and I can do it. I just wanted to give you first stab since you've gotten things this far.

[eikenb]

The best way to do this would be using the funcBoolVar type in flags.go, similar to how -consul-retry and -consul-ssl are done.

I started playing with this and I'm not sure that it is the way to go. The NoPrefix configuration option is set in the PrefixConfig sub-struct that isn't present until the -prefix arguments have been parsed... and having the order of arguments be important is bad.

So it looks like another pattern might work better. Something that works better with a delayed handling of the -no-prefix argument to make it easier to apply to the already parsed -prefix's passed.

[eikenb]

The other pattern I usually use for post-processing optional flags (using flag.Visit) doesn't work with the tests. So back to the funcBoolVar and setting a variable to be post processed instead of setting it directly.

[eikenb]

@noahhai I played around at this until I basically had it done, so I finished it off and pushed it up in a branch. As you initiated the work I still want to give you a chance to do it if you want and you can look at my branch as an example if you'd like. If you don't care just close this PR and I'll create a pull request from the branch.

[eikenb]

I created PR #208 for my version.

[eikenb]

@noahhai Thanks for the initial version and push to get it added.

[noahhai]

Hey @eikenb thanks for getting this in! Sorry I didn't get back to you on it, ended up a little busy, but excited to have this functionality added now!

[eikenb]

No problem re: getting back to me. I just like to let people contribute if they want to and get the credit if they like. So I wanted to be sure to give you the opportunity. Thanks.