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

add kep for kustomize secret generator plugins #811

Merged
merged 6 commits into from
Mar 5, 2019
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
256 changes: 256 additions & 0 deletions keps/sig-cli/kustomize-secret-generator-plugins.md
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"
Copy link
Contributor

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

Copy link
Contributor

@calebamiles calebamiles Feb 27, 2019

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 and reviewers are the people who did not the people who can which is the problem that the OWNERS mechanism addresses

Copy link
Member

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.

- "@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
Copy link
Contributor

Choose a reason for hiding this comment

The 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 Secret objects (link to api?), which are general key-value pairs where the value is supposed to be a secret. Reading secret values from local files raises security concerns about file lifetime and access. Reading secret values from the execution of arbitrary commands in a kustomization file introduces concerns in a world where kustomization files can be used directly from the internet. This proposal describes obtaining secret values from specific kustomize plugins...


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

Copy link
Contributor

Choose a reason for hiding this comment

The 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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From the go plugins doc:

Currently plugins are only supported on Linux and macOS.

Shouldn't the solution work on Windows since kustomize ships for Windows?

Copy link
Contributor

Choose a reason for hiding this comment

The 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.
Copy link
Contributor

Choose a reason for hiding this comment

The 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?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

The 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):

  • Require installation of kubectl, and have kustomize execute the secret generation code as a kubectl sub-process, i.e. exec.Command(“kubectl”, “foo”, …) using the plugin via kubectl.

  • Arrange via cli-runtime refactoring to vendor kubectl plugin code into kustomize, to track changes to argument and flag parsing and the notion of plugin subcommands.

  • Vendor no code, and use a simplified kubctl plugin model: given a secret generation command foo bar, just look for a binary called kubectl-foo and run it, passing bar as an argument. This works because the kustomize use case doesn’t have to worry about command collisions, sub-commands, listing plugins, etc.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The 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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like option 3 the best.

Copy link
Member

Choose a reason for hiding this comment

The 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 kubectl-something commands in $PATH. It is highly likely that these plugins have interesting side effects when invoked, because that's the whole point of plugins.

Given that kustomization.yaml files will come from relatively-untrusted external sources, I would much prefer that this specific category of plugin was cleanly namespaced off from other possible kubectl plugins. This should be as simple as invoking kubectl-kustomize-plugin-foo ..., rather than just kubectl-foo.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do those extensions have to be in $PATH? If users are not meant to invoke them directly, then they shouldn't be.

Copy link
Contributor

Choose a reason for hiding this comment

The 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?

Copy link
Member

@anguslees anguslees Feb 21, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd never use a kustomize.yaml file from an untrusted source.

I think the larger kustomize proposal is to have 3rd parties publish remote manifest repositories that may include kustomize.yaml (since they are in turn derived from manifests coming from another 4th party). That means we are pulling kustomize.yaml from a sort-of-untrusted source.

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 kubectl operations on remote yaml without endangering the local user.

(See also: the reasons the previous arbitrary-command-based kustomize secret generator was rolled back)


## Proposal
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add an example of what the new secretGenerator stanza would look like in https://github.com/kubernetes-sigs/kustomize/blob/master/docs/kustomization.yaml


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
```

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about kustomization file changes from this:

secretGenerator:
  - name: bob
    literals:
    - fruit=apple
    - vegetable=broccoli
    env: foo.env
    files:
    - passphrase=phrase.dat
    - forces.txt
  - name: sally
     ...

to the following:

  secretGenerator:
  - name: bob
    sources:
    - strategy: literal
      args:
      - fruit=apple
      - vegetable=broccoli
    - strategy: env
      args:
      - foo.env
    - strategy: files
      args:
      - passphrase=phrase.dat
      - forces.txt
    - strategy: goplugin
      name: awesome
      args:
      - foo
      - bar
    - stratagy: goplugin
      name: usegrpc
      args:
      - foo
      - bar
    - strategy: kubectlplugin
      name: whatever
      args:
      - foo
      - bar
  - name: sally
     ...

The above reformulates the existing name=value pair generators (literals, files and env) as builtin strategies.

This stanza suggests two new strategies, goplugin and kubectlplugin.

There are two instances of goplugin called awesome and usegrpc, and one instance of kubectlplugin called whatever.

Then a strategy instance graduates to builtin by making its name a new reserved word in the strategy field, e.g. this:

    - strategy: goplugin
      name: awesome
      args:
      - foo
      - bar

could graduate to

    - strategy: awesome
      args:
      - foo
      - bar

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 literals, files and env.

We could deprecate the existing DataSources format, and modify the existing fix command to make this change trivial. We've done this before.

Also, if we do the impl in GeneratorArgs, you get all these benefits in ConfigMap as well as Secret.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, thats basically what I had in mind. I just went with plugins instead of sources and type instead of strategy.

For the first phase I ported DataSources over to a new BuiltIn plugin type and then in another PR we can deprecate DataSources and add a fix command.

We can also make kustomize add secret/configmap more generic with --plugin-type, --plugin-name, and --args which would work for all plugins.

  secretGenerator:
  - name: bob
    plugins:
    - type: BuiltIn
      name: literal
      args:
      - fruit=apple
      - vegetable=broccoli
    - type: BuiltIn  
      name: env
      args:
      - foo.env
    - type: BuiltIn  
      name: files
      args:
      - passphrase=phrase.dat
      - forces.txt
    - type: Go
      name: awesome
      args:
      - foo
      - bar
    - type: Go
      name: usegrpc
      args:
      - foo
      - bar
    - type: Kubectl
      name: whatever
      args:
      - foo
      - bar

I already updated the PR, but I can rename those fields if you would like.

Copy link
Contributor

Choose a reason for hiding this comment

The 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 plugin or Plugin. Better to call the interface KvSourcePlugin, and put the code under, say,
k8sdeps/kv/plugin instead of in pkg/plugin.

Also suggest we likewise push the word plugin a little deeper in the kustomization file - see below.

Nice stuff in PR kubernetes-sigs/kustomize#760

Modulo the use of the bare word plugin and pluginFactory, the overall approach is great.

  • there's a Plugin interface with a Get method that returns kv pairs (prefer a rename to KvSourcePlugin).
  • it's clear that there are two types of plugins, Go and BuiltIn, and a factory for making instances.
  • the legacy sources - literals, env, and files - are converted to Builtin form
  • there's a clear path (clear to me anyway) for making exec style plugins, or a kubectl plugin runner.
  • Would prefer some changes to field names in the kustomization file:

Instead of

 plugins:
 - type: BuiltIn
   name: env
   ...
 - type: BuiltIn
   name: literals
   ...
 - type: Kubectl
   name: whatever
   ...
 - type: Go
   name: awesome
   ...

suggest:

 kvSources:
  - name: env
    ...
  - name: literals
    ...
  - plugin: kubectl
    name: whatever
    ...
  - plugin: go
    name: awesome
    ...

i.e. i prefer pushing the word plugin down a level in the kustomization file, and using the reserved words builtin, go, kubectl instead of BuiltIn, Go, Kubectl, and i'd allow the plugin field to default to builtin (so it can be optional).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok sounds like a plan, I'll update the PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@monopole Can we go with type instead of plugin? It makes it seem like it is a single plugin vs an instance of a plugin type.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1

plugin: kubectl
plugin: go

2

type: kubectlPlugin
type: goPlugin

3

type: kubectl
type: go

4

pluginType: kubectl
pluginType: go

3 is brief, but 4 is self-explanatory.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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)
Copy link
Member

Choose a reason for hiding this comment

The 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 KustomizationSecretGenerators) that is a map (or array for simplicity) of name => function. That way only explicitly marked functions will be accessible.

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.

Copy link
Contributor

@monopole monopole Feb 23, 2019

Choose a reason for hiding this comment

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