Merge pull request #2295 from pwittrock/slfn

refactor function filters
This commit is contained in:
Kubernetes Prow Robot
2020-03-24 13:55:02 -07:00
committed by GitHub
4 changed files with 206 additions and 202 deletions

View File

@@ -389,8 +389,7 @@ type IsReconcilerFilter struct {
func (c *IsReconcilerFilter) Filter(inputs []*yaml.RNode) ([]*yaml.RNode, error) {
var out []*yaml.RNode
for i := range inputs {
img, _ := GetContainerName(inputs[i])
isContainerResource := img != ""
isContainerResource := GetFunctionSpec(inputs[i]) != nil
if isContainerResource && !c.ExcludeReconcilers {
out = append(out, inputs[i])
}
@@ -408,52 +407,67 @@ const (
var functionAnnotationKeys = []string{FunctionAnnotationKey, oldFunctionAnnotationKey}
// GetFunction parses the config function from the object if it is found
func GetFunction(n *yaml.RNode, meta yaml.ResourceMeta) (*yaml.RNode, error) {
// getFunction parses the config function from the object if it is found
func getFunction(n *yaml.RNode, meta yaml.ResourceMeta) *FunctionSpec {
var fs FunctionSpec
for _, s := range functionAnnotationKeys {
fn := meta.Annotations[s]
if fn != "" {
return yaml.Parse(fn)
_ = yaml.Unmarshal([]byte(fn), &fs)
return &fs
}
}
return n.Pipe(yaml.Lookup("metadata", "configFn"))
n, err := n.Pipe(yaml.Lookup("metadata", "configFn"))
if err != nil || yaml.IsEmpty(n) {
return nil
}
s, err := n.String()
if err != nil {
return nil
}
_ = yaml.Unmarshal([]byte(s), &fs)
return &fs
}
// GetContainerName returns the container image for an API if one exists
func GetContainerName(n *yaml.RNode) (string, string) {
meta, _ := n.GetMeta()
type ContainerSpec struct {
Image string `json:"image,omitempty" yaml:"image,omitempty"`
Network ContainerNetwork `json:"network,omitempty" yaml:"network,omitempty"`
}
type FunctionSpec struct {
Path string `json:"path,omitempty" yaml:"path,omitempty"`
Network string `json:"network,omitempty" yaml:"network,omitempty"`
Container ContainerSpec `json:"container,omitempty" yaml:"container,omitempty"`
}
type ContainerNetwork struct {
Required bool `json:"required,omitempty" yaml:"required,omitempty"`
}
// GetFunctionSpec returns the FunctionSpec for a resource. Returns
// nil if the resource does not have a FunctionSpec.
//
// The FunctionSpec is read from the resource metadata.annotation
// "config.kubernetes.io/function"
func GetFunctionSpec(n *yaml.RNode) *FunctionSpec {
meta, err := n.GetMeta()
if err != nil {
return nil
}
// path to the function, this will be mounted into the container
path := meta.Annotations[kioutil.PathAnnotation]
fn, _ := GetFunction(n, meta)
if fn != nil {
image, _ := fn.Pipe(yaml.Lookup("container", "image"))
return yaml.GetValue(image), path
if fn := getFunction(n, meta); fn != nil {
fn.Network = ""
fn.Path = path
return fn
}
// legacy function specification for backwards compatibility
container := meta.Annotations["config.kubernetes.io/container"]
if container != "" {
return container, path
return &FunctionSpec{
Path: path, Container: ContainerSpec{Image: container}}
}
image, err := n.Pipe(yaml.Lookup("metadata", "configFn", "container", "image"))
if err != nil || yaml.IsMissingOrNull(image) {
return "", path
}
return yaml.GetValue(image), path
}
// GetContainerNetworkRequired returns whether or not networking is required for the container
func GetContainerNetworkRequired(n *yaml.RNode) (bool, error) {
meta, err := n.GetMeta()
if err != nil {
return false, err
}
f, err := GetFunction(n, meta)
if err != nil {
return false, err
}
networkRequired, _ := f.Pipe(yaml.Lookup("container", "network", "required"))
return yaml.GetValue(networkRequired) == "true", nil
return nil
}

View File

@@ -326,9 +326,71 @@ kind: Example
metadata:
annotations:
config.kubernetes.io/function: |-
container: foo:v1.0.0
container:
image: foo:v1.0.0
`,
expectedFn: `
container:
image: foo:v1.0.0`,
},
{
name: "network",
resource: `
apiVersion: v1beta1
kind: Example
metadata:
annotations:
config.kubernetes.io/function: |-
container:
image: foo:v1.0.0
network:
required: true
`,
expectedFn: `
container:
image: foo:v1.0.0
network:
required: true
`,
},
{
name: "path",
resource: `
apiVersion: v1beta1
kind: Example
metadata:
annotations:
config.kubernetes.io/function: |-
path: foo
container:
image: foo:v1.0.0
`,
// path should be erased
expectedFn: `
container:
image: foo:v1.0.0
`,
},
{
name: "network",
resource: `
apiVersion: v1beta1
kind: Example
metadata:
annotations:
config.kubernetes.io/function: |-
network: foo
container:
image: foo:v1.0.0
`,
// network should be erased
expectedFn: `
container:
image: foo:v1.0.0
`,
expectedFn: `container: foo:v1.0.0`,
},
// legacy fn style
@@ -338,9 +400,13 @@ apiVersion: v1beta1
kind: Example
metadata:
configFn:
container: foo:v1.0.0
container:
image: foo:v1.0.0
`,
expectedFn: `
container:
image: foo:v1.0.0
`,
expectedFn: `container: foo:v1.0.0`,
},
// no fn
@@ -361,20 +427,19 @@ metadata:
tt := tests[i]
t.Run(tt.name, func(t *testing.T) {
resource := yaml.MustParse(tt.resource)
meta, err := resource.GetMeta()
if !assert.NoError(t, err) {
t.FailNow()
}
fn, err := GetFunction(resource, meta)
if !assert.NoError(t, err) {
t.FailNow()
}
fn := GetFunctionSpec(resource)
if tt.missingFn {
if !assert.Nil(t, fn) {
t.FailNow()
}
} else {
if !assert.Equal(t, strings.TrimSpace(fn.MustString()), strings.TrimSpace(tt.expectedFn)) {
b, err := yaml.Marshal(fn)
if !assert.NoError(t, err) {
t.FailNow()
}
if !assert.Equal(t,
strings.TrimSpace(tt.expectedFn),
strings.TrimSpace(string(b))) {
t.FailNow()
}
}
@@ -382,61 +447,6 @@ metadata:
}
}
func Test_GetContainerName(t *testing.T) {
// make sure gcr.io works
n, err := yaml.Parse(`apiVersion: v1beta1
kind: MyThing
metadata:
configFn:
container:
image: gcr.io/foo/bar:something
`)
if !assert.NoError(t, err) {
return
}
c, _ := GetContainerName(n)
assert.Equal(t, "gcr.io/foo/bar:something", c)
// container from config.kubernetes.io/container annotation
n, err = yaml.Parse(`apiVersion: v1
kind: MyThing
metadata:
annotations:
config.kubernetes.io/container: gcr.io/foo/bar:something
`)
if !assert.NoError(t, err) {
return
}
c, _ = GetContainerName(n)
assert.Equal(t, "gcr.io/foo/bar:something", c)
// container from config.kubernetes.io/function annotation
n, err = yaml.Parse(`apiVersion: v1
kind: MyThing
metadata:
annotations:
config.kubernetes.io/function: |
container:
image: gcr.io/foo/bar:something
`)
if !assert.NoError(t, err) {
return
}
c, _ = GetContainerName(n)
assert.Equal(t, "gcr.io/foo/bar:something", c)
// doesn't have a container
n, err = yaml.Parse(`apiVersion: v1
kind: MyThing
metadata:
`)
if !assert.NoError(t, err) {
return
}
c, _ = GetContainerName(n)
assert.Equal(t, "", c)
}
func Test_GetContainerNetworkRequired(t *testing.T) {
tests := []struct {
input string
@@ -501,9 +511,10 @@ metadata:
if !assert.NoError(t, err) {
return
}
required, err := GetContainerNetworkRequired(cfg)
assert.NoError(t, err)
assert.Equal(t, tc.required, required)
meta, _ := cfg.GetMeta()
fn := getFunction(cfg, meta)
assert.Equal(t, tc.required, fn.Container.Network.Required)
}
}

View File

@@ -57,8 +57,10 @@ type RunFns struct {
// and only use explicit sources
NoFunctionsFromInput *bool
// for testing purposes only
containerFilterProvider func(string, string, string, *yaml.RNode) kio.Filter
// functionFilterProvider provides a filter to perform the function.
// this is a variable so it can be mocked in tests
functionFilterProvider func(
filter filters.FunctionSpec, api *yaml.RNode) kio.Filter
}
// Execute runs the command
@@ -110,21 +112,21 @@ func (r RunFns) getNodesAndFilters() (
func (r RunFns) getFilters(nodes []*yaml.RNode) ([]kio.Filter, error) {
var fltrs []kio.Filter
// implicit filters from the input Resources
// fns from annotations on the input resources
f, err := r.getFunctionsFromInput(nodes)
if err != nil {
return nil, err
}
fltrs = append(fltrs, f...)
// explicit filters from a list of directories
// fns from directories specified on the struct
f, err = r.getFunctionsFromFunctionPaths()
if err != nil {
return nil, err
}
fltrs = append(fltrs, f...)
// explicit filters from a list of directories
// explicit fns specified on the struct
f, err = r.getFunctionsFromFunctions()
if err != nil {
return nil, err
@@ -156,7 +158,6 @@ func (r RunFns) getFunctionsFromInput(nodes []*yaml.RNode) ([]kio.Filter, error)
return nil, nil
}
var fltrs []kio.Filter
buff := &kio.PackageBuffer{}
err := kio.Pipeline{
Inputs: []kio.Reader{&kio.PackageBuffer{Nodes: nodes}},
@@ -167,95 +168,50 @@ func (r RunFns) getFunctionsFromInput(nodes []*yaml.RNode) ([]kio.Filter, error)
return nil, err
}
sortFns(buff)
for i := range buff.Nodes {
api := buff.Nodes[i]
network := ""
img, path := filters.GetContainerName(api)
required, err := filters.GetContainerNetworkRequired(api)
if err != nil {
return nil, err
}
if required {
if !r.Network {
// TODO(eddizane): Provide error info about which function needs the network
return fltrs, errors.Errorf("network required but not enabled with --network")
}
network = r.NetworkName
}
fltrs = append(fltrs, r.containerFilterProvider(img, path, network, api))
}
return fltrs, nil
return r.getFunctionFilters(false, buff.Nodes...)
}
// getFunctionsFromFunctionPaths returns the set of functions read from r.FunctionPaths
// as a slice of Filters
func (r RunFns) getFunctionsFromFunctionPaths() ([]kio.Filter, error) {
var fltrs []kio.Filter
buff := &kio.PackageBuffer{}
for i := range r.FunctionPaths {
err := kio.Pipeline{
Inputs: []kio.Reader{kio.LocalPackageReader{PackagePath: r.FunctionPaths[i]}},
Inputs: []kio.Reader{
kio.LocalPackageReader{PackagePath: r.FunctionPaths[i]},
},
Outputs: []kio.Writer{buff},
}.Execute()
if err != nil {
return nil, err
}
}
for i := range buff.Nodes {
api := buff.Nodes[i]
network := ""
img, path := filters.GetContainerName(api)
required, err := filters.GetContainerNetworkRequired(api)
if err != nil {
return nil, err
}
if required {
if !r.Network {
// TODO(eddiezane): Provide error info about which function needs the network
return fltrs, errors.Errorf("network required but not enabled with --network")
}
network = r.NetworkName
}
c := r.containerFilterProvider(img, path, network, api)
cf, ok := c.(*filters.ContainerFilter)
if ok {
// functions provided by FunctionPaths are globally scoped
cf.GlobalScope = true
}
fltrs = append(fltrs, c)
}
return fltrs, nil
return r.getFunctionFilters(true, buff.Nodes...)
}
// getFunctionsFromFunctions returns the set of explicitly provided functions as
// Filters
func (r RunFns) getFunctionsFromFunctions() ([]kio.Filter, error) {
var fltrs []kio.Filter
for i := range r.Functions {
api := r.Functions[i]
network := ""
img, path := filters.GetContainerName(api)
return r.getFunctionFilters(true, r.Functions...)
}
required, err := filters.GetContainerNetworkRequired(api)
if err != nil {
return nil, err
}
if required {
func (r RunFns) getFunctionFilters(global bool, fns ...*yaml.RNode) (
[]kio.Filter, error) {
var fltrs []kio.Filter
for i := range fns {
api := fns[i]
spec := filters.GetFunctionSpec(api)
if spec.Container.Network.Required {
if !r.Network {
// TODO(eddizane): Provide error info about which function needs the network
// TODO(eddiezane): Provide error info about which function needs the network
return fltrs, errors.Errorf("network required but not enabled with --network")
}
network = r.NetworkName
spec.Network = r.NetworkName
}
c := r.containerFilterProvider(img, path, network, api)
c := r.functionFilterProvider(*spec, api)
cf, ok := c.(*filters.ContainerFilter)
if ok {
// functions provided by Functions are globally scoped
if global && ok {
cf.GlobalScope = true
}
fltrs = append(fltrs, c)
@@ -327,17 +283,26 @@ func (r *RunFns) init() {
}
}
// if containerFilterProvider hasn't been set, use the default
if r.containerFilterProvider == nil {
r.containerFilterProvider = func(image, path, network string, api *yaml.RNode) kio.Filter {
cf := &filters.ContainerFilter{
Image: image,
Config: api,
Network: network,
StorageMounts: r.StorageMounts,
GlobalScope: r.GlobalScope,
}
return cf
}
// functionFilterProvider set the filter provider
if r.functionFilterProvider == nil {
r.functionFilterProvider = r.ffp
}
}
// ffp provides function filters
func (r *RunFns) ffp(spec filters.FunctionSpec, api *yaml.RNode) kio.Filter {
if spec.Container.Image != "" {
return &filters.ContainerFilter{
Image: spec.Container.Image,
Config: api,
Network: spec.Network,
StorageMounts: r.StorageMounts,
GlobalScope: r.GlobalScope,
}
}
return noOpFilter
}
var noOpFilter = kio.FilterFunc(func(in []*yaml.RNode) ([]*yaml.RNode, error) {
return in, nil
})

View File

@@ -47,10 +47,15 @@ func TestRunFns_init(t *testing.T) {
api, err := yaml.Parse(`apiVersion: apps/v1
kind:
`)
spec := filters.FunctionSpec{
Container: filters.ContainerSpec{
Image: "example.com:version",
},
}
if !assert.NoError(t, err) {
return
}
filter := instance.containerFilterProvider("example.com:version", "", "", api)
filter := instance.functionFilterProvider(spec, api)
assert.Equal(t, &filters.ContainerFilter{Image: "example.com:version", Config: api}, filter)
}
@@ -69,7 +74,16 @@ kind:
if !assert.NoError(t, err) {
return
}
filter := instance.containerFilterProvider("example.com:version", "", "", api)
spec := filters.FunctionSpec{
Container: filters.ContainerSpec{
Image: "example.com:version",
},
}
if !assert.NoError(t, err) {
return
}
filter := instance.functionFilterProvider(spec, api)
assert.Equal(t, &filters.ContainerFilter{
Image: "example.com:version", Config: api, GlobalScope: true}, filter)
}
@@ -131,7 +145,7 @@ func TestRunFns_Execute__initDefault(t *testing.T) {
tt := tests[i]
t.Run(tt.name, func(t *testing.T) {
(&tt.instance).init()
(&tt.instance).containerFilterProvider = nil
(&tt.instance).functionFilterProvider = nil
if !assert.Equal(t, tt.expected, tt.instance) {
t.FailNow()
}
@@ -505,7 +519,7 @@ func TestCmd_Execute(t *testing.T) {
return
}
instance := RunFns{Path: dir, containerFilterProvider: getFilterProvider(t)}
instance := RunFns{Path: dir, functionFilterProvider: getFilterProvider(t)}
if !assert.NoError(t, instance.Execute()) {
t.FailNow()
}
@@ -534,9 +548,9 @@ func TestCmd_Execute_setFunctionPaths(t *testing.T) {
// run the functions, providing the path to the directory of filters
instance := RunFns{
FunctionPaths: []string{tmpF.Name()},
Path: dir,
containerFilterProvider: getFilterProvider(t),
FunctionPaths: []string{tmpF.Name()},
Path: dir,
functionFilterProvider: getFilterProvider(t),
}
// initialize the defaults
instance.init()
@@ -566,9 +580,9 @@ func TestCmd_Execute_setOutput(t *testing.T) {
out := &bytes.Buffer{}
instance := RunFns{
Output: out, // write to out
Path: dir,
containerFilterProvider: getFilterProvider(t),
Output: out, // write to out
Path: dir,
functionFilterProvider: getFilterProvider(t),
}
// initialize the defaults
instance.init()
@@ -614,9 +628,9 @@ func TestCmd_Execute_setInput(t *testing.T) {
}
instance := RunFns{
Input: input, // read from input
Path: outDir,
containerFilterProvider: getFilterProvider(t),
Input: input, // read from input
Path: outDir,
functionFilterProvider: getFilterProvider(t),
}
// initialize the defaults
instance.init()
@@ -659,8 +673,8 @@ func setupTest(t *testing.T) string {
// getFilterProvider fakes the creation of a filter, replacing the ContainerFiler with
// a filter to s/kind: Deployment/kind: StatefulSet/g.
// this can be used to simulate running a filter.
func getFilterProvider(t *testing.T) func(string, string, string, *yaml.RNode) kio.Filter {
return func(s, _, _ string, node *yaml.RNode) kio.Filter {
func getFilterProvider(t *testing.T) func(filters.FunctionSpec, *yaml.RNode) kio.Filter {
return func(f filters.FunctionSpec, node *yaml.RNode) kio.Filter {
// parse the filter from the input
filter := yaml.YFilter{}
b := &bytes.Buffer{}