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

Docs: common section and root vars injection rules should be better documented #93

Open
achiang opened this issue Jan 8, 2019 · 5 comments

Comments

@achiang
Copy link

achiang commented Jan 8, 2019

Given the following secrets.yml:

common:
    FOO: foo

production:
    BAR: bar

Here is some observed behavior:

$ summon -e production printenv | egrep 'FOO|BAR'
BAR=bar
FOO=foo
$ summon -e common printenv | egrep 'FOO|BAR'
FOO=foo
$ summon printenv | egrep 'FOO|BAR'
$

The first two observations match my expectations based on the documentation.

The third example's behavior isn't mentioned in the documentation, so I won't go so far as to call it a bug.

However, it is mildly surprising. Since common is always inherited by other sections, I kinda had a weak expectation that its values would always get injected, even if we do not pass -e on the command line.

Docs should be elaborated with examples to describe the behavior in more details between root vars and common section as it changes when using and omitting the -e variable.

@sgnn7
Copy link
Contributor

sgnn7 commented Jan 8, 2019

This one is a bit tricky since we would need to figure out if the user really want to use the common section.

This example kind of illustrates it:

FOO: foo-base

common:
    FOO: foo-common

production:
    BAR: bar

Would the result of summon printenv | egrep 'FOO|BAR' be FOO=foo-base or FOO=foo-common?

I think if you use -e, we assume that you have split out things into sections but if you don't specify it, it gets ambiguous as to what you intended. I think a solution could be implemented easy but we would need to know what precedence to apply to the common section if you have a variable in both toplevel and common grouping.

Let me chew on this a bit this week.

@achiang
Copy link
Author

achiang commented Jan 9, 2019

Would the result of summon printenv | egrep 'FOO|BAR' be FOO=foo-base or FOO=foo-common?

Based on some other observations, what I've seen is that "last thing defined, wins". So in your example, I would expect to see FOO=foo-common. But that is just my intuition, and I do not feel strongly about that opinion at all.

To be honest though, I hadn't thought to combine an implicit base section along with an explicit common section. The docs kinda sorta made me think that you could either define sections xor everything was required to live in the base section.

Perhaps this issue can simply be converted into a request for more explicit examples in the docs?

@sgnn7
Copy link
Contributor

sgnn7 commented Jan 9, 2019

Yeah I think that putting this in the docs would be the best place since current behavior iirc from playing around with this yesterday is that it's doing an xor on evaluation already which matches the docs but maybe that's not clear enough? I'll reword the issue/title a bit to match.

@sgnn7 sgnn7 changed the title common / default sections should always be injected Docs: common section and root vars injection rules should be better documented Jan 9, 2019
@achiang
Copy link
Author

achiang commented Jan 9, 2019

Works for me, thanks!

@ckolumbus
Copy link
Contributor

I'd like to give this a little push... I do have quite some secrets to inject an only for special purposes I need to overwrite some of them. So the combination of section-less common part with a section for specific secrets would help me a lot. with the current implementation I'd always need to provide an explicit -e common parameter to actually read te default values. that's not very intuitive.

For me It would already be sufficient to support one of the following solutions: either allow section-less common part (and always load it) OR have a common section, but load it also in case NO environment has been given.

P.S.: another problem currently is that if you have a section-less part and a section summon throws lots of errors in case you actually provide the -e section parameter. So a section-less common part with some additional section secrets does not work right now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

3 participants