-
Notifications
You must be signed in to change notification settings - Fork 38
Improve functionRunner interface #599
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
base: master
Are you sure you want to change the base?
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,15 @@ | ||
apiVersion: config.kubernetes.io/v1 | ||
kind: ResourceList | ||
items: | ||
- apiVersion: v1 | ||
kind: Service | ||
metadata: | ||
name: example | ||
functionConfig: | ||
apiVersion: v1 | ||
kind: ConfigMap | ||
metadata: | ||
name: runner-fn-config | ||
data: | ||
project-id: kpt-dev | ||
managed-by: kpt |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
apiVersion: config.kubernetes.io/v1 | ||
kind: ResourceList | ||
items: | ||
- apiVersion: v1 | ||
kind: Service | ||
metadata: | ||
name: example | ||
functionConfig: | ||
apiVersion: v1 | ||
kind: ConfigMap | ||
metadata: | ||
name: customConfig | ||
data: | ||
owner: kpt | ||
org: google |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,43 +19,108 @@ import ( | |
"github.com/GoogleContainerTools/kpt-functions-sdk/go/fn" | ||
) | ||
|
||
var _ fn.Runner = &CustomFnConfig{} | ||
|
||
type CustomFnConfig struct { | ||
Owner string | ||
Org string | ||
} | ||
|
||
// Run is the main function logic. | ||
// `ctx` provides easy methods to add info, error or warning result to `ResourceList.Results`. | ||
// `items` is parsed from the STDIN "ResourceList.Items". | ||
// `functionConfig` is from the STDIN "ResourceList.FunctionConfig". The value is parsed from a `CustomFnConfig` type | ||
// "owner" and "org" field. | ||
func (r *CustomFnConfig) Run(ctx *fn.Context, functionConfig *fn.KubeObject, items fn.KubeObjects) { | ||
for _, o := range items { | ||
o.SetName(r.Owner) | ||
o.SetNamespace(r.Org) | ||
} | ||
ctx.ResultInfo("updated namespace and name", nil) | ||
} | ||
|
||
// This example uses a CustomFnConfig object, which implements `Runner.Run` methods. | ||
func Example_asMainCustomFnConfig() { | ||
file, _ := os.Open("./data/runner-customFnConfig.yaml") | ||
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'd move these to golden tests, because that keeps the function itself cleaner and shows how to test! |
||
defer file.Close() | ||
os.Stdin = file | ||
|
||
if err := fn.AsMain(&CustomFnConfig{}); err != nil { | ||
os.Exit(1) | ||
} | ||
// Output: | ||
// apiVersion: config.kubernetes.io/v1 | ||
// kind: ResourceList | ||
// items: | ||
// - apiVersion: v1 | ||
// kind: Service | ||
// metadata: | ||
// name: kpt | ||
// namespace: google | ||
// functionConfig: | ||
// apiVersion: fn.kpt.dev/v1alpha1 | ||
// kind: CustomFnConfig | ||
// metadata: | ||
// name: runner-fn-config | ||
// owner: kpt | ||
// org: google | ||
// results: | ||
// - message: updated namespace and name | ||
// severity: info | ||
} | ||
|
||
func Example_asMainConfigMap() { | ||
file, _ := os.Open("./data/runner-configmap.yaml") | ||
defer file.Close() | ||
os.Stdin = file | ||
|
||
if err := fn.AsMain(&CustomFnConfig{}); err != nil { | ||
os.Exit(1) | ||
} | ||
// Output: | ||
// apiVersion: config.kubernetes.io/v1 | ||
// kind: ResourceList | ||
// items: | ||
// - apiVersion: v1 | ||
// kind: Service | ||
// metadata: | ||
// name: kpt | ||
// namespace: google | ||
// functionConfig: | ||
// apiVersion: v1 | ||
// kind: ConfigMap | ||
// metadata: | ||
// name: customConfig | ||
// data: | ||
// owner: kpt | ||
// org: google | ||
// results: | ||
// - message: updated namespace and name | ||
// severity: info | ||
} | ||
|
||
var _ fn.Runner = &SetLabels{} | ||
|
||
type SetLabels struct { | ||
Labels map[string]string `json:"labels,omitempty"` | ||
Data map[string]string | ||
} | ||
|
||
// Run is the main function logic. | ||
// `ctx` provides easy methods to add info, error or warning result to `ResourceList.Results`. | ||
// `items` is parsed from the STDIN "ResourceList.Items". | ||
// `functionConfig` is from the STDIN "ResourceList.FunctionConfig". The value has been assigned to the r.Labels | ||
// the functionConfig is validated to have kind "SetLabels" and apiVersion "fn.kpt.dev/v1alpha1" | ||
func (r *SetLabels) Run(ctx *fn.Context, functionConfig *fn.KubeObject, items []*fn.KubeObject) { | ||
// `functionConfig` is from the STDIN "ResourceList.FunctionConfig". The value is parsed from a `ConfigMap` type | ||
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. Are we switching away from the "CRD" style for backwards compatibility? I find the ConfigMap approach much harder to think about, and we don't get any type-assistance. I'd rather think about how to map from a ConfigMap to a CRD, which I think will also help with kptdev/kpt#3351 |
||
// "data" field. | ||
func (r *SetLabels) Run(ctx *fn.Context, functionConfig *fn.KubeObject, items fn.KubeObjects) { | ||
for _, o := range items { | ||
for k, newLabel := range r.Labels { | ||
o.SetLabel(k, newLabel) | ||
for k, v := range r.Data { | ||
o.SetLabel(k, v) | ||
} | ||
} | ||
ctx.ResultInfo("updated labels", nil) | ||
} | ||
|
||
// This example uses a SetLabels object, which implements `Runner.Run` methods. | ||
// | ||
// The input from ./data/setlabels-resourcelist.yaml: | ||
// apiVersion: config.kubernetes.io/v1 | ||
// kind: ResourceList | ||
// items: | ||
// - apiVersion: v1 | ||
// kind: Service | ||
// metadata: | ||
// name: example | ||
// functionConfig: | ||
// apiVersion: fn.kpt.dev/v1alpha1 | ||
// kind: SetLabels | ||
// metadata: | ||
// name: setlabel-fn-config | ||
func Example_asMain() { | ||
file, _ := os.Open("./data/setlabels-resourcelist.yaml") | ||
func Example_asMain_configMapData() { | ||
file, _ := os.Open("./data/runner-configmap-general.yaml") | ||
defer file.Close() | ||
os.Stdin = file | ||
|
||
|
@@ -70,11 +135,17 @@ func Example_asMain() { | |
// kind: Service | ||
// metadata: | ||
// name: example | ||
// labels: | ||
// project-id: kpt-dev | ||
// managed-by: kpt | ||
// functionConfig: | ||
// apiVersion: fn.kpt.dev/v1alpha1 | ||
// kind: SetLabels | ||
// apiVersion: v1 | ||
// kind: ConfigMap | ||
// metadata: | ||
// name: setlabel-fn-config | ||
// name: runner-fn-config | ||
// data: | ||
// project-id: kpt-dev | ||
// managed-by: kpt | ||
// results: | ||
// - message: updated labels | ||
// severity: info | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,5 +15,5 @@ | |
package fn | ||
|
||
type Runner interface { | ||
Run(context *Context, functionConfig *KubeObject, items []*KubeObject) | ||
Run(context *Context, functionConfig *KubeObject, items KubeObjects) | ||
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 it! |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,6 +17,7 @@ package fn | |
import ( | ||
"fmt" | ||
"reflect" | ||
"strings" | ||
) | ||
|
||
type runnerProcessor struct { | ||
|
@@ -39,9 +40,19 @@ func (r *runnerProcessor) config(ctx *Context, o *KubeObject) { | |
data := o.NestedStringMapOrDie("data") | ||
fnRunnerElem := reflect.ValueOf(r.fnRunner).Elem() | ||
for i := 0; i < fnRunnerElem.NumField(); i++ { | ||
if fnRunnerElem.Field(i).Kind() == reflect.Map { | ||
fnRunnerElem.Field(i).Set(reflect.ValueOf(data)) | ||
break | ||
switch fnRunnerElem.Field(i).Kind() { | ||
case reflect.String: | ||
for k, v := range data { | ||
lowerKey := strings.ToLower(k) | ||
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 this approach. We might consider also using the |
||
if lowerKey == strings.ToLower(fnRunnerElem.Type().Field(i).Name) { | ||
fnRunnerElem.Field(i).SetString(v) | ||
break | ||
} | ||
} | ||
case reflect.Map: | ||
if "Data" == fnRunnerElem.Type().Field(i).Name { | ||
fnRunnerElem.Field(i).Set(reflect.ValueOf(data)) | ||
} | ||
} | ||
} | ||
case o.IsGVK("fn.kpt.dev", "v1alpha1", fnName): | ||
|
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.
Do we need
functionConfig
?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.
I'm thinking about some special handling. Like set-namespace supports variant constructor by accepting a
kptfile.kpt.dev
named "ConfigMap"