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

Support externally defined providers #285

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

vetsin
Copy link

@vetsin vetsin commented Jun 3, 2024

A mockup for #280.

Any external provider should manage it's own auth, e.g. maybe it will use /var/run/secrets/kubernetes.io/serviceaccount/token

Looking for feedback on this idea/code.

Questions:

  • Should an example/library external provider exist?
  • As per Provider Wishlist #279 rename to exec?
  • Do we want to allow arbitrary programs or just teller specifc ones? if so would external != exec

@jondot
Copy link
Contributor

jondot commented Jun 14, 2024

Looks great as a start, some feedback:

  • Yes, perhaps name to exec
  • While reviewing the code, an idea poped into mind, maybe have each of the get, put, list commands in configuration?
get: some-bin get -k {{KEY_PATH}}
put: some-bin put -k {{KEY_PATH}} -v {{VALUE}}
list: some-bin list

This can allow people to more extensively customize how they run their custom binaries. A custom binary can be some app like 1password or it can be just some shell script they made.

I believe some thought should be made to the "protocol" where the command returns results. Is it in JSON? what would be the shape of it?

  • Another recommendation is to take a real command line app and try to build a provider for it. For example gopass might fit (also lastpass or 1password if you have accounts for them).

@vetsin
Copy link
Author

vetsin commented Jun 14, 2024

  • No problem with -k and -v flags instead of positional arguments, though what's the benefit? It seems it would just make writing the script or binary harder, since you have to parse flags not not just a #!/bin/sh with $2 and $3
  • As for protocol I was using docker-credentials-helpers as the model which uses a struct as JSON -- would could do the same and provide a base rust impl

How about just:

struct Credential {
    path: String,
    key: String,
    value: String,
}

@jondot
Copy link
Contributor

jondot commented Jun 18, 2024

You're correct on -k and -v, it would indeed require someone to have a concept of a parser or use a parsing library. your suggestion sounds good.

For the Credential struct, it sounds good as well, a few notes:

  • Let's just call it KV (for key-value, since some secrets can be (a) just configuration settings (b) not secrets but confidential)
  • I believe as long as we're open to extension of this struct, the initial suggestion looks great

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

Successfully merging this pull request may close these issues.

2 participants