consul-template icon indicating copy to clipboard operation
consul-template copied to clipboard

secretOrDefault

Open pdbogen opened this issue 7 years ago • 25 comments

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.

pdbogen avatar May 25 '17 23:05 pdbogen

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.

phaer avatar May 29 '17 14:05 phaer

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.

pdbogen avatar May 30 '17 17:05 pdbogen

Hi there,

This is a quasi-duplicate of https://github.com/hashicorp/consul-template/issues/776. If you are reading a secret, you expect the secret to exist. We consider a missing secret to be an error.

sethvargo avatar May 30 '17 20:05 sethvargo

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

pdbogen avatar May 30 '17 23:05 pdbogen

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

bbriggs avatar Jun 19 '17 15:06 bbriggs

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).

pdbogen avatar Sep 14 '17 19:09 pdbogen

+1 for this

vaidik avatar Jul 19 '19 20:07 vaidik

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

eikenb avatar Jul 19 '19 22:07 eikenb

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

bbriggs avatar Jul 25 '19 02:07 bbriggs

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?

jsok avatar Nov 01 '19 22:11 jsok

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 avatar Nov 01 '19 22:11 pdbogen

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?

jsok avatar Nov 02 '19 21:11 jsok

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 avatar Nov 08 '19 18:11 pdbogen

(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 avatar Nov 08 '19 18:11 pdbogen

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.

pdbogen avatar Nov 08 '19 18:11 pdbogen

Also in need of this feature, following!

tommyalatalo avatar Oct 06 '20 14:10 tommyalatalo

@tommyalatalo, thanks for adding your :+1: to the top issue post.

eikenb avatar Oct 06 '20 19:10 eikenb

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?

eikenb avatar Mar 23 '22 19:03 eikenb

@eikenb I believe there would be some downsides to the secretExists road based on this comment https://github.com/hashicorp/consul-template/issues/942#issuecomment-305038780 , 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?

weeezes avatar Jun 15 '22 07:06 weeezes

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!

eikenb avatar Jun 22 '22 22:06 eikenb

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!

weeezes avatar Jul 13 '22 05:07 weeezes

@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!

eikenb avatar Jul 13 '22 21:07 eikenb

@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.))

pdbogen avatar Jul 14 '22 06:07 pdbogen

((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 :).

weeezes avatar Jul 14 '22 06:07 weeezes

Any plan to add this feature ?

vimal3271 avatar May 31 '23 05:05 vimal3271