-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
add kep for kustomize secret generator plugins #811
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,256 @@ | ||
--- | ||
title: Kustomize Secret Generator Plugins | ||
authors: | ||
- "@sethpollack" | ||
owning-sig: sig-cli | ||
participating-sigs: | ||
- sig-apps | ||
- sig-architecture | ||
reviewers: | ||
- "@monopole" | ||
- "@Liujingfang1" | ||
approvers: | ||
- "@monopole" | ||
- "@Liujingfang1" | ||
- "@pwittrock" | ||
editor: "@sethpollack" | ||
creation-date: 2019-02-04 | ||
last-updated: 2019-02-04 | ||
status: implementable | ||
--- | ||
|
||
# Kustomize Secret Generator Plugins | ||
|
||
## Table of Contents | ||
* [Table of Contents](#table-of-contents) | ||
* [Summary](#summary) | ||
* [Motivation](#motivation) | ||
* [Goals](#goals) | ||
* [Non-Goals](#non-goals) | ||
* [Proposal](#proposal) | ||
* [Risks and Mitigations](#risks-and-mitigations) | ||
* [Graduation Criteria](#graduation-criteria) | ||
* [Implementation History](#implementation-history) | ||
|
||
## Summary | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggest say what the goal is, and its probably not useful to assume reader has history, e.g.
|
||
|
||
Kustomize wants to generate Kubernetes Secret objects from general key-value pairs where the value is supposed to be a secret. Currently Kustomize only supports reading secret values from local files which raises security concerns about file lifetime and access. Reading secret values from the execution of arbitrary "commands" in a kustomization.yaml file introduces concerns in a world where kustomization files can be used directly from the internet when a user runs `kustomize build`. | ||
|
||
This proposal describes obtaining secret values safely using golang [plugins](https://golang.org/pkg/plugin/). | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. link to golang plugins, and maybe a sentence or two as to why they are different from any old executable There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. From the go plugins doc:
Shouldn't the solution work on Windows since kustomize ships for Windows? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
## Motivation | ||
|
||
Not having a way to integrate `SecretGenerator` with secret management tools requires hacky workarounds. | ||
|
||
### Goals | ||
|
||
- Give users a way to intergrate `SecretGenerator` with secret management tools in a safe way. | ||
|
||
### Non-Goals | ||
|
||
- Kustomize will not handle plugin installation/management. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If kustomize is going to add plugins should it try the same model as kubectl for management? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What do you think? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Some options for leveraging kubectl plugin model (discussed during/after sig-cli meeting):
The downside to all these options is that a plugin intended only for kustomize secret generation would appear in kubectl help, and plugin lists, etc. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The above comments assume familiarity with how kubectl plugins work. if interested, see https://kubernetes.io/docs/tasks/extend-kubectl/kubectl-plugins There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like option 3 the best. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think there are risks of command collisions here. Not right now, because nobody uses kubectl plugins ;) but imagine a world where it is normal to have several unknown Given that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do those extensions have to be in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @anguslees I don't want to use kubectl style plugins. Was just listing the options as a thought experiment. Your point taken though; if we did, we could avoid collisions in kubectl with more ugly namespace. When I said the kustomize use case doesn't have to worry about command collisions, i was referring to colliding with other native kustomize commands, which isn't possible since the kustomize plugin wouldn't be used as a cobra-style command. The only trouble would be plugin-plugin collision, and we just error out if the loader detects that. I'd never use a kustomize.yaml file from an untrusted source. Is there something in the docs that suggests such behavior is recommended? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I think the larger kustomize proposal is to have 3rd parties publish remote manifest repositories that may include I agree that actually applying the remote manifests necessarily means we are trusting the remote source to give us code/config that will be executed in our local cluster. There are differences between what (and when) the remote source is able to execute in the cluster vs on the local machine, however. In particular, we need to be able to do "--dry-run" -style (See also: the reasons the previous arbitrary-command-based kustomize secret generator was rolled back) |
||
|
||
## Proposal | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please add an example of what the new |
||
|
||
In the proposed design `SecretGenerator` would load golang plugins from `~/.config/kustomization_plugins`. This would limit the scope of what kustomize can execute to the plugins on the users machine. Also using golang plugins forces a strict interface with well-defined responsibilities. | ||
|
||
The current API looks like this: | ||
|
||
``` | ||
secretGenerator: | ||
- name: app-tls | ||
files: | ||
- secret/tls.cert | ||
- secret/tls.key | ||
type: "kubernetes.io/tls" | ||
|
||
- name: env_file_secret | ||
env: env.txt | ||
type: Opaque | ||
|
||
- name: myJavaServerEnvVars | ||
literals: | ||
- JAVA_HOME=/opt/java/jdk | ||
- JAVA_TOOL_OPTIONS=-agentlib:hprof | ||
``` | ||
|
||
The proposed API would look like this: | ||
|
||
``` | ||
secretGenerator: | ||
- name: app-tls | ||
kvSources: | ||
- pluginType: builtin // builtin is the default value of pluginType | ||
name: files | ||
args: | ||
- secret/tls.cert | ||
- secret/tls.key | ||
- name: env_file_secret | ||
kvSources: | ||
- name: env // this is a builtin | ||
args: | ||
- env.txt | ||
- name: myJavaServerEnvVars | ||
kvSources: | ||
- name: literals // this is a builtin | ||
args: | ||
- JAVA_HOME=/opt/java/jdk | ||
- JAVA_TOOL_OPTIONS=-agentlib:hprof | ||
- name: secretFromPlugins | ||
kvSources: | ||
- pluginType: go // described by this KEP | ||
name: someLocalGoPlugin | ||
args: | ||
- someArg | ||
- someOtherArg | ||
- pluginType: kubectl // some future KEP can write this | ||
name: someKubectlPlugin | ||
args: | ||
- anotherArg | ||
- yetAnotherArg | ||
``` | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How about kustomization file changes from this:
to the following:
The above reformulates the existing name=value pair generators ( This stanza suggests two new strategies, There are two instances of Then a strategy instance graduates to builtin by making its name a new reserved word in the strategy field, e.g. this:
could graduate to
This way we can add goplugins, open the door to other kinds of plugins, and allow a graduation strategy to become a builtin like the existing We could deprecate the existing Also, if we do the impl in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, thats basically what I had in mind. I just went with For the first phase I ported We can also make
I already updated the PR, but I can rename those fields if you would like. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks! W/r to the word plugin - i want to bury it a little deeper. Because everything that makes sense about this approach to KV pair generation makes even more sense for transformers - a completely different "plugin" thing. So we should eschew a highly visible package, interface, or type called called simply Also suggest we likewise push the word Nice stuff in PR kubernetes-sigs/kustomize#760Modulo the use of the bare word plugin and pluginFactory, the overall approach is great.
Instead of
suggest:
i.e. i prefer pushing the word plugin down a level in the kustomization file, and using the reserved words There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok sounds like a plan, I'll update the PR. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @monopole Can we go with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sure. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 1
2
3
4
3 is brief, but 4 is self-explanatory. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I'm ok with 3 or 4 |
||
The implementation would look something like this: | ||
|
||
``` | ||
func keyValuesFromPlugins(ldr ifc.Loader, plugins []types.Plugin) ([]kv.Pair, error) { | ||
var allKvs []kv.Pair | ||
|
||
for _, plugin := range plugins { | ||
secretFunc, err := findPlugin(plugin.Name) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
kvs, err := secretFunc(ldr.Root(), plugin.Args) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
allKvs = append(allKvs, kvs...) | ||
} | ||
|
||
return allKvs, nil | ||
} | ||
|
||
func findPlugin(name string) (func(string, []string) ([]kv.Pair, error), error) { | ||
allPlugins, err := filepath.Glob(os.ExpandEnv("$HOME/.config/kustomization_plugins/*.so")) | ||
if err != nil { | ||
return nil, fmt.Errorf("error loading plugins") | ||
} | ||
|
||
for _, filename := range allPlugins { | ||
p, err := plugin.Open(filename) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
symbol, err := p.Lookup(name) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we must go with golang plugins, please don't just allow the kustomization.yaml file to invoke any symbol that happens to be exported. A better way to do this is to have each plugin export a well-known symbol (perhaps As a bonus, it allows the same plugin .so to be used to provide other sets of entry points (with different function signatures) in the future. This includes versioning the well-known symbol so we can cleanly evolve the ABI over time. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @anguslees Policy around exposed vars sounds reasonable. |
||
if err != nil { | ||
continue | ||
} | ||
|
||
if secretFunc, exists := symbol.(func(string, []string) ([]kv.Pair, error)); exists { | ||
return secretFunc, nil | ||
} | ||
} | ||
|
||
return nil, fmt.Errorf("plugin %s not found", name) | ||
} | ||
``` | ||
|
||
and an example plugin would look like this: | ||
|
||
``` | ||
package main | ||
|
||
import ( | ||
"os" | ||
|
||
"sigs.k8s.io/kustomize/pkg/kv" | ||
) | ||
|
||
func Env(root string, args []string) ([]kv.Pair, error) { | ||
var kvs []kv.Pair | ||
for _, arg := range args { | ||
kvs = append(kvs, kv.Pair{Key: arg, Value: os.Getenv(arg)}) | ||
} | ||
return kvs, nil | ||
} | ||
``` | ||
|
||
Users would install the plugin with `go build -buildmode=plugin ~/.config/kustomization_plugins/myplugin.so` | ||
|
||
### Risks and Mitigations | ||
|
||
#### Several people want an _exec-style_ plugin | ||
_exec-style_ means execute arbitrary code from some file. | ||
|
||
Most the lines of code written to implement this KEP | ||
accommodate the notion of KV generation via a _generic_ | ||
notion of a plugin, and a generic interface, in the | ||
kustomization file and associated structs and code. | ||
|
||
This code recharactizes the three existing KV | ||
generation methods as `pluginType: builtin` | ||
(_literals_, _env_ and _files_), introduces the new | ||
`pluginType: Go`, and leaves room for someone to easily | ||
add, say `pluginType: kubectl` to look for a kubectl | ||
plugin, and `pluginType: whatever` to handles some | ||
completely new style, e.g. look for the plugin name as | ||
an executable in some hardcoded location. | ||
|
||
Actual implementation of these other kinds of plugins | ||
are out of scope for this KEP, however this KEP's | ||
implementation will make it much easier to create other | ||
kinds of plugins. | ||
|
||
Go Plugins that become popular have a clear | ||
path to becoming a builting - so if someone writes, say, | ||
a general Exec plugin, we can easily promote it to a | ||
builtin (by virtue of the fact that it's written in Go, | ||
and because of the choices made in this KEP for | ||
describing a plugin in the kustomization file). | ||
|
||
#### No current windows support for golang plugins. | ||
This may be implemented by the Go team later in which | ||
case it will just work. | ||
|
||
Also, as noted above, someone can write an exec style plugin | ||
which will work on Windows. | ||
|
||
#### General symbol execution from the plugin | ||
A Go plugin will be cast to a particular hardcoded interface, e.g. | ||
|
||
``` | ||
type KvSourcePlugin { | ||
Get []kvPairs | ||
} | ||
``` | ||
and accessed exclusively through that interface. | ||
|
||
#### Existing KV generators continue as an option | ||
We leave in place the existing three mechanisms | ||
(_literals_, _env_ and _files_) for generating KV pairs, but | ||
additionally allow these mechanisms to be expressed as | ||
`builtin` plugins (see example above). | ||
|
||
If plugins - both Go and other styles - prove unpopular | ||
or problematic, we can remove them per API change | ||
policies. | ||
|
||
If they do prove popular/useful, we can remove deprecate | ||
the legacy form for (_literals_, _env_ and _files_), | ||
and help people convert to the new "builtin" form. | ||
|
||
## Graduation Criteria | ||
|
||
Many users have been requesting a better way to integrate with secret management tools https://github.com/kubernetes-sigs/kustomize/issues/692 | ||
|
||
* A higher level feature test (like those in [pkg/target](https://github.com/kubernetes-sigs/kustomize/tree/master/pkg/target)) | ||
* Documentation of fields in the [canonical example file](https://github.com/kubernetes-sigs/kustomize/blob/master/docs/kustomization.yaml) | ||
* Usage [example](https://github.com/kubernetes-sigs/kustomize/tree/master/examples) | ||
|
||
## Implementation History | ||
|
||
This design uses the golang [plugin](https://golang.org/pkg/plugin/) system. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should the approvers be the kustomize admin and/or maintainers? https://github.com/kubernetes-sigs/kustomize/blob/master/OWNERS_ALIASES
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to answer a question from @lavalamp, it is my intention that
approvers
andreviewers
are the people who did not the people who can which is the problem that theOWNERS
mechanism addressesThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@calebamiles Then the fields should be called "reviewed-by" and "approved-by"? And it is weird to have those people added by the author rather than add themselves. I would not be surprised if we have many KEPs created with a different understanding of those fields.