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

secretOrDefault #942

Open
pdbogen opened this issue May 25, 2017 · 25 comments
Open

secretOrDefault #942

pdbogen opened this issue May 25, 2017 · 25 comments
Labels
enhancement vault Related to the Vault integration waiting-reply

Comments

@pdbogen
Copy link

pdbogen commented May 25, 2017

secret, for the case of (for example) a generic secret, blocks rendering of the template when the requested path does not exist, as in this case:

{{ with secret "www-bbs-dev/static/db_demo2" }}{{ .Data.db_demo2_pass }}{{ end }}

This produces the consul error (on latest git):

2017/05/25 23:25:31.775568 [WARN] (view) vault.read(www-bbs-dev/static/db_demo2): no secret exists at www-bbs-dev/static/db_demo2 (retry attempt 1 after "250ms")

It would be helpful for my purposes if there was the equivalent of keyOrDefault for secret that would let me emit a blank string and continue rendering the template.

@phaer
Copy link

phaer commented May 29, 2017

According to https://golang.org/pkg/text/template/, there's not only {{with pipeline}}[...]{{end}}, but {{with pipeline}}[...]{{else}}[...]{{end}} where the second part servers is executed when the pipeline is empty. I think thats what one would use for default values here.

@pdbogen
Copy link
Author

pdbogen commented May 30, 2017

Thanks for the advice; unfortunately it doesn't seem to help. Even with an {{else}} block in the template, consul-template still reports:

2017/05/30 10:38:55 [ERR] (view) "secret(www-bbs-dev/static/db_demo2)" no secret exists at path "\"secret(www-bbs-dev/static/db_demo2)\""

...and the file is not rendered, rather than being rendered with the contents of the {{else}}.

I believe the issue is that consul-template is processing the pipeline callback and returning an error, rather than returning an empty pipeline. An empty pipeline -- i.e., if consul-template's secret "path" returned empty rather than an error when the path did not exist -- would work properly for me.

@sethvargo
Copy link
Contributor

Hi there,

This is a quasi-duplicate of #776. If you are reading a secret, you expect the secret to exist. We consider a missing secret to be an error.

@pdbogen
Copy link
Author

pdbogen commented May 30, 2017

Hello.

I agree that this is similar to #776 (I read that one before I contemplated open a ticket), but I disagree that the answer should be the same, or even that many of the same considerations apply.

The secret_exists requested in #776 is fundamentally unworkable because, as pointed out, there's an implication that secret_exists should not have side-effects (ed: but that reading any vault path cannot be guaranteed to not have side-effects).

However, secret already potentially has (desirable) side effects, and secretOrDefault is merely a request for alternate error handling. The semantics vis-a-vis effects on / interaction with Vault are the same in the case of secretOrDefault as it is in the existing secret.

Can you explain why you consider a nonexistent secret to be a fatal error, and more to the point, why you're not willing to add a mechanism that would allow a user to explicitly indicate that in a particular case, a nonexistent secret is not an error? Do you consider a nonexistent consul key to be an error? If so, why is there a keyOrDefault?

Thanks,

  • Tricia

@bbriggs
Copy link

bbriggs commented Jun 19, 2017

@sethvargo But clearly people are asking for it and have workflows where this would not be considered an error.

@pdbogen
Copy link
Author

pdbogen commented Sep 14, 2017

Hello, @sethvargo. Can we reopen this ticket please? As I indicate in my message from May (and as I continue to believe), this request is substantially different from the request secret_exists; is fundamentally satisfiable within the constraints and concerns preesnted in issue 776, and furthermore is parallel to functionality that is provided for consul keys (keyOrDefault).

@vaidik
Copy link

vaidik commented Jul 19, 2019

+1 for this

@eikenb
Copy link
Contributor

eikenb commented Jul 19, 2019

Not promising anything, but I'm going to re-open this so I can look into it.

@eikenb eikenb reopened this Jul 19, 2019
@bbriggs
Copy link

bbriggs commented Jul 25, 2019

That was one heck of a necrobump. Looking forward to this!

@jsok
Copy link

jsok commented Nov 1, 2019

How do you all envisage the calling convention for this function to look like?

The hard part is how to specify the default value, since secret actually returns a map and we grab values out of {{ .Data }}. This is even more complex for KV v2 results: {{ .Data.data }}.

The best I could come up with is:

{{ with defaultSecret "foo=bar" "baz=quux" | secretOrDefault "/secret" "other=params" }}
{{ .Data.foo }}
{{ .Data.baz }}
{{ end }}

But this also raises the question, should the default map be merged with the result? e.g. if foo was found in the actual secret, but baz wasn't, should we return the real foo and default baz, or should we return all the defaults?

consul-template currently doesn't have the defaultSecret helper functionality - but it was the cleanest way I could think of implementing this.
Something like sprig/lists would need to be included or implemented to support the ability to create list and map variables as this isn't possible with vanilla text/template.

Finally, would this helper also cover the use case of permission denied as well as missing values?

@pdbogen
Copy link
Author

pdbogen commented Nov 1, 2019

I think a reasonable implementation might be to accept an even-numbered list of key-value pairs that are populated into sub-fields of .Data, i.e.,

{{ with secretOrDefault "/secret" "key" "NOT FOUND!!!!" }}
{{ .Data.key }}
{{ end }}

@jsok
Copy link

jsok commented Nov 2, 2019

I think a reasonable implementation might be to accept an even-numbered list of key-value pairs that are populated into sub-fields of .Data, i.e.,

{{ with secretOrDefault "/secret" "key" "NOT FOUND!!!!" }}
{{ .Data.key }}
{{ end }}

@pdbogen secret accepts arbitrary data arguments after the path, i.e. {{ secret "<PATH>" "<DATA>" }}. How do you differentiate <DATA> args and default value args?

@pdbogen
Copy link
Author

pdbogen commented Nov 8, 2019

Yep, that's a great point. It doesn't seem like secretOrDefault would be reasonable with write semantics, so maybe secretOrDefault should not worry about the subsequent data arguments.

A slightly more radical proposal might be something like:

pipeline | orSecret "/secret" [<data>]

where orSecret works exactly like secret, except that if there's an error reading (or writing?) the secret, it uses the incoming pipeline instead.

(But my preference would be to not have write semantics for secretOrDefault.)

@pdbogen
Copy link
Author

pdbogen commented Nov 8, 2019

(which I see now is largely what you had previously suggested! so yeah, I think if we want to keep write semantics, that would be the best option.)

As for

But this also raises the question, should the default map be merged with the result? e.g. if foo was found in the actual secret, but baz wasn't, should we return the real foo and default baz, or should we return all the defaults?

I do not think it should. secretOrDefault is more about error handling, and isn't itself aware of the contents of secrets, anyway.

consul-template currently doesn't have the defaultSecret helper functionality

I think we can leave it up to the users at this point to decide how to stream in structured data. parseJSON would do it, though it's not the most graceful option. Or they can write their defaults to scratch, presumably.

Finally, would this helper also cover the use case of permission denied as well as missing values?

Yes, I think it should. There's some set of errors it maybe shouldn't cover, which we know are transient errors (probably we wouldn't want the template rendering to flip back and forth), but I'm not sure how easy it is to distinguish those. (HTTP 4xx vs 5xx might be enough?)

@pdbogen
Copy link
Author

pdbogen commented Nov 8, 2019

to expand more on merging, etc; msising keys in .Data can be handled already within consul-template, but a missing or inaccessible vault path entirely can't, it just causes rendering to fail with no workaround; which is what this issue requests be fixed. tbqh, even just something like maybeSecret that returns an empty .Data would meet my needs.

@tommyalatalo
Copy link

Also in need of this feature, following!

@eikenb
Copy link
Contributor

eikenb commented Oct 6, 2020

@tommyalatalo, thanks for adding your 👍 to the top issue post.

@eikenb eikenb added the vault Related to the Vault integration label Jan 24, 2022
@eikenb
Copy link
Contributor

eikenb commented Mar 23, 2022

With Go 1.18's introducing lazy evaluate of the and and or operators I think we might have a solution in sight for this. Instead of secretOrDefault, using secretExists with the lazy logical operators should work.

So I propose changing this feature request to secretExists along with upping the Go version to 1.18. Thoughts?

@weeezes
Copy link

weeezes commented Jun 15, 2022

@eikenb I believe there would be some downsides to the secretExists road based on this comment #942 (comment) , but for my use case just having secretExists would be enough. But the content of the comment still sounds relevant and reasonable, what do you think?

@eikenb
Copy link
Contributor

eikenb commented Jun 22, 2022

Well.. we have the choice of having something like this and not using it with those endpoints with side effects or not having it. There are no ways to tell which secret paths will end up with side effects (depends on the secret engine) as querying them has the side effect.

IMO if we need this (secretExists or secretOrDefault) we will have to accept that if you use it with a path with side effects, you'll get those side effects. That is there is no way to tell this universally without just reading the docs, which would be on the user.

If anyone has an idea for how to differentiate (I'm not a Vault expert) between the side-effect paths and not please let me know and I'll consider it. Otherwise it is decision time. It is worth this feature to have bugs around using it with side-effecting paths? We'd mention it in the docs of course.

Thanks for any input!

@weeezes
Copy link

weeezes commented Jul 13, 2022

Well.. we have the choice of having something like this and not using it with those endpoints with side effects or not having it. There are no ways to tell which secret paths will end up with side effects (depends on the secret engine) as querying them has the side effect.

IMO if we need this (secretExists or secretOrDefault) we will have to accept that if you use it with a path with side effects, you'll get those side effects. That is there is no way to tell this universally without just reading the docs, which would be on the user.

If anyone has an idea for how to differentiate (I'm not a Vault expert) between the side-effect paths and not please let me know and I'll consider it. Otherwise it is decision time. It is worth this feature to have bugs around using it with side-effecting paths? We'd mention it in the docs of course.

Thanks for any input!

That's a good point, there are no way to guarantee that there's no side effects because that's not really a part of any "contract" made by be APIs. Just one example is https://www.vaultproject.io/api-docs/secret/consul#generate-credential , maybe it's even a requirement that there are side effects when by nature some of the secrets are short-lived and created during call time.

I'd give my vote for secretOrDefault, it would be very helpful :).

Sorry it took so long to reply!

@eikenb
Copy link
Contributor

eikenb commented Jul 13, 2022

@pdbogen, @vaidik, @bbriggs, @altosys ... sorry for the direct pings but I'm looking to get feedback on this. If you have any thoughts on my last question please let me know. Thanks!

@pdbogen
Copy link
Author

pdbogen commented Jul 14, 2022

@eikenb no worries, and thanks for taking a look at this after so long.

In our environment, we don't use mounts that have side effects. So for us, it's a non-issue.

I continue to think that side effects being triggered by secretOrDefault are appropriate- in these cases presumably we'd just never hit the OrDefault aspect, perhaps unless the side effect fails?

I would not expect secretExists to trigger side effects --- ironically, perhaps we could implement it, but require backends to opt-in to supporting it (e.g., kv / kv2 can support it, but consul/creds can't?) and error out otherwise. Though, this would imply changes to the vault API to support a new verb or at least request parameter.

(So, secretOrDefault continues to better meet our specific needs, and in additional is implementable cleanly without changes to the vault API.)

((amendment: to clarify our use case, for context: we have separation of duties between the folks that maintain vault (interact with it directly, configure mounts, and write secrets & policies); from those folks that write consul-template to read from vault. So we from time to time run into situations where the template owners write unsatisfiable templates, e.g., due to requests for secrets that are mis-typed or where the request to create them hasn't been actioned yet- it's really hard for us to fail gracefully in this situation, and secretOrDefault could help our template authors both handle these situations more cleanly, and also more easily write templates that are portable across environments.))

@weeezes
Copy link

weeezes commented Jul 14, 2022

((amendment: to clarify our use case, for context: we have separation of duties between the folks that maintain vault (interact with it directly, configure mounts, and write secrets & policies); from those folks that write consul-template to read from vault. So we from time to time run into situations where the template owners write unsatisfiable templates, e.g., due to requests for secrets that are mis-typed or where the request to create them hasn't been actioned yet- it's really hard for us to fail gracefully in this situation, and secretOrDefault could help our template authors both handle these situations more cleanly, and also more easily write templates that are portable across environments.))

Another use case which we had was that it would have made moving secrets around a lot easier, as during the process of migrating secrets from one place to another one could have supported the same secret from two different paths and picked the preferred one if it already existed for the environment. All kinds of small maintanance/refactoring tasks could be made easier through this :).

@eikenb eikenb added this to the v0.30.0 milestone Jul 15, 2022
@eikenb eikenb removed this from the v0.30.0 milestone Jan 3, 2023
@vimal3271
Copy link

Any plan to add this feature ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement vault Related to the Vault integration waiting-reply
Projects
None yet
Development

No branches or pull requests

10 participants