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

improve fn authoring #4319

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
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
32 changes: 32 additions & 0 deletions kyaml/fn/framework/command/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,3 +166,35 @@ func (fr *deferredFileReader) Read(dest []byte) (int, error) {
}
return fr.srcReader.Read(dest)
}

// AsMain reads the resourceList in yaml format from stdin, evaluates the
// function and write the updated resourceList in yaml to stdout. Errors if any
// will be printed to stderr.
func AsMain(p framework.ResourceListProcessor) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the intended use case for this vs. command.Build above? It seems like they do the same thing, but Build has more batteries included (e.g. it wires up a CLI with debugging flags for you, helps you create a dockerfile for your function, and accepts deconstructed input as well as ResourceList on stdin). Most of the functions I've written myself have entrypoints that look like:

func main() {
	cmd := command.Build(provider.New(), command.StandaloneEnabled, false)
	cmd.Execute()
}

That strikes me as pretty easy, though the concepts of standalone mode and error printing might be unnecessary (we could always enable standalone mode, always print the error and eliminate the legacy env variable).

Copy link
Member Author

Choose a reason for hiding this comment

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

IMO the concepts of standalone mode and error printing are unnecessary complexity for users, I'd suggest dropping them if they don't affect kustomize.

Quoted from godoc of Build method:

// By default, invoking the returned cobra.Command with arguments triggers "standalone" mode.
// In this mode:
// - The first argument must be the name of a file containing the FunctionConfig.
// - The remaining arguments must be filenames containing input resources for ResourceList.Items.
// - The argument "-", if present, will cause resources to be read from STDIN as well.
// The output will be a raw stream of resources (not wrapped in a List type).
// Example usage: cat input1.yaml | go run main.go config.yaml input2.yaml input3.yaml -

Reading from files is not the standard way to read resources, can we drop it?

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with dropping it as an option, but I think it should be always enabled. It doesn't interfere with the primary (fn specification) way of running the program, and it adds a new capability: you can use the same program directly. That is really powerful. It means you can build a generator plugin that is both distributed on its own for simple use case and pluggable into any Kustomization. I personally have a use case like that. I also find standalone mode very handy for testing any function.

e := func() error {
in, err := ioutil.ReadAll(os.Stdin)
if err != nil {
return fmt.Errorf("unable to read from stdin: %v", err)
}
out, err := framework.Run(p, in)
if err != nil {
switch err.(type) {
case framework.Results, *framework.Results, framework.Result, *framework.Result:
// do nothing
default:
return err
}
}
// If there is an error and the error is Result(s), we don't return the
// error immediately. We write out to stdout before returning any error.
_, er := os.Stdout.Write(out)
if er != nil {
return fmt.Errorf("unable to write to stdout: %v", er)
}
return err
}()
if e != nil {
framework.Log(e)
}
return e
}
34 changes: 34 additions & 0 deletions kyaml/fn/framework/example_filter_GVK_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package framework_test

import (
"os"

"sigs.k8s.io/kustomize/kyaml/fn/framework"
"sigs.k8s.io/kustomize/kyaml/fn/framework/command"
)

// This example implements a function that reads the desired replicas from the
// functionConfig and updates the replicas field for all deployments.

func Example_filterGVK() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Just so we're all on the same page about what's already possible, an API-oriented version of the same thing would look like the code below. Personally I prefer to write my functions this way, because it is only slightly more code and it is much clearer about the function's API, validation rules and processing steps.

func Example_filterGVK() {
	if err := command.AsMain(&ReplicaUpdater{}); err != nil {
		os.Exit(1)
	}
}

type ReplicaUpdater struct {
	Replicas *int `yaml:"replicas" json:"replicas"`
}

func (u *ReplicaUpdater) Validate() error {
	if u.Replicas == nil {
		return framework.ErrMissingFnConfig{RequiredPaths: []string{".Replicas"}}
	}
	return nil
}

func (u *ReplicaUpdater) Process(rl *framework.ResourceList) error {
	if err := framework.LoadFunctionConfig(rl.FunctionConfig, u); err != nil {
		return errors.Wrap(err)
	}
	for i := range rl.Items {
		if framework.GVKMatcher("apps/v1/Deployment").Match(rl.Items[i]) {
			rl.Items[i].SetOrDie(u.Replicas, "spec", "replicas")
		}
	}
	return nil
}

And since this processor doesn't do anything special outside the filtering logic, here's the same thing again using SimpleProcessor:

func Example_filterGVK() {
	updater := &ReplicaUpdater{}
	if err := command.AsMain(framework.SimpleProcessor{Config: updater, Filter: updater}); err != nil {
		os.Exit(1)
	}
}

type ReplicaUpdater struct {
	Replicas *int `yaml:"replicas" json:"replicas"`
}

func (u *ReplicaUpdater) Validate() error {
	if u.Replicas == nil {
		return framework.ErrMissingFnConfig{RequiredPaths: []string{".Replicas"}}
	}
	return nil
}

func (u *ReplicaUpdater) Filter(items []*yaml.RNode) ([]*yaml.RNode, error) {
	for i := range items {
		if framework.GVKMatcher("apps/v1/Deployment").Match(items[i]) {
			items[i].SetOrDie(u.Replicas, "spec", "replicas")
		}
	}
	return items, nil
}

Writing that out, it is a little weird to have to provide the updater to both fields... maybe a nice optimization could be that if a Config is provided, a Filter isn't, and Config is a filter, we use it automatically. WDYT about that enhancement?

if err := command.AsMain(framework.ResourceListProcessorFunc(updateReplicas)); err != nil {
os.Exit(1)
}
}

// updateReplicas sets a field in resources selecting by GVK.
func updateReplicas(rl *framework.ResourceList) error {
if rl.FunctionConfig == nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: what matters is not the function config missing, but rather it having a replicas field specifically. I would suggest making GetNestedInt handle the nil case so all function authors don't have to. And then returning the typed error (ideally with the required path in it!) at L26 instead.

return framework.ErrMissingFnConfig{}
}
replicas, found, err := rl.FunctionConfig.GetNestedInt("replicas")
if !found || err != nil {
return err
}
for i, obj := range rl.Items {
if obj.GetApiVersion() == "apps/v1" && obj.GetKind() == "Deployment" {
Copy link
Contributor

Choose a reason for hiding this comment

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

Use the framework's matchers for this: framework.GVKMatcher("apps/v1/Deployment").Match(items[i])
(same for other similar places--we have lots of stock matchers written, and even ways to combine them easily).

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for bringing this up. GVKMatcher().Match() works here.
But thinking more about it, why do we make it return a ResourceTemplateMatcher? The signature is func GVKMatcher(names ...string) ResourceTemplateMatcher and it gives me the impression that it's not what I want, since I don't need templating at all.
We (Google) have built many KRM functions. But we haven't use this ResourceTemplateMatcher at all. I wonder what is the use cases for that? And do we really need it? ResourceMatcher seems to be good enough.

Copy link
Contributor

Choose a reason for hiding this comment

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

I haven't used the template aspect either. It was something I preserved from the previous iteration of the framework because of the test coverage. If y'all don't use it either I'm open to 🔥 it.

rl.Items[i].SetOrDie(replicas, "spec", "replicas")
}
}
mengqiy marked this conversation as resolved.
Show resolved Hide resolved
return nil
}
87 changes: 87 additions & 0 deletions kyaml/fn/framework/example_generator_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
package framework_test

import (
"fmt"
"io/ioutil"
"net/http"
"os"

"sigs.k8s.io/kustomize/kyaml/fn/framework"
"sigs.k8s.io/kustomize/kyaml/fn/framework/command"
"sigs.k8s.io/kustomize/kyaml/yaml"
)

// This function generates Graphana configuration in the form of ConfigMap. It
// accepts Revision and ID as input.

func Example_generator() {
if err := command.AsMain(framework.ResourceListProcessorFunc(generate)); err != nil {
os.Exit(1)
}
}

// generate generates a ConfigMap.
func generate(rl *framework.ResourceList) error {
if rl.FunctionConfig == nil {
return framework.ErrMissingFnConfig{}
}

revision, foundRevision, err := rl.FunctionConfig.GetNestedString("data", "revision")
if err != nil {
return fmt.Errorf("failed to find field revision: %w", err)
}
id, foundId, err := rl.FunctionConfig.GetNestedString("data", "id")
if err != nil {
return fmt.Errorf("failed to find field id: %w", err)
}
if !foundRevision || !foundId {
return nil
}
js, err := fetchDashboard(revision, id)
if err != nil {
return fmt.Errorf("fetch dashboard: %v", err)
}

// corev1.ConfigMap should be used here. But we can't use it here due to dependency restriction in the kustomize repo.
cm := ConfigMap{
ResourceMeta: yaml.ResourceMeta{
TypeMeta: yaml.TypeMeta{
APIVersion: "v1",
Kind: "ConfigMap",
},
ObjectMeta: yaml.ObjectMeta{
NameMeta: yaml.NameMeta{
Name: fmt.Sprintf("%v-gen", rl.FunctionConfig.GetName()),
Namespace: rl.FunctionConfig.GetNamespace(),
},
Labels: map[string]string{
"grafana_dashboard": "true",
},
},
},
Data: map[string]string{
fmt.Sprintf("%v.json", rl.FunctionConfig.GetName()): fmt.Sprintf("%q", js),
},
}
return rl.UpsertObjectToItems(cm, nil, false)
}

func fetchDashboard(revision, id string) (string, error) {
url := fmt.Sprintf("https://grafana.com/api/dashboards/%s/revisions/%s/download", id, revision)
resp, err := http.Get(url)
if err != nil {
return "", err
}
defer resp.Body.Close()
b, err := ioutil.ReadAll(resp.Body)
if err != nil {
return "", err
}
return string(b), nil
}

// ConfigMap is a copy of corev1.ConfigMap.
type ConfigMap struct {
yaml.ResourceMeta `json:",inline" yaml:",inline"`
Data map[string]string `json:"data,omitempty" yaml:"data,omitempty"`
}
63 changes: 63 additions & 0 deletions kyaml/fn/framework/example_logger_injector_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
package framework_test

import (
"fmt"
"os"

"sigs.k8s.io/kustomize/kyaml/fn/framework"
"sigs.k8s.io/kustomize/kyaml/fn/framework/command"
"sigs.k8s.io/kustomize/kyaml/yaml"
)

// In this example, we implement a function that injects a logger as a sidecar
// container in workload APIs.

func Example_loggeInjector() {
if err := command.AsMain(framework.ResourceListProcessorFunc(injectLogger)); err != nil {
os.Exit(1)
}
}

// injectLogger injects a logger container into the workload API resources.
// generate implements the goframework.KRMFunction interface.
func injectLogger(rl *framework.ResourceList) error {
var li LoggerInjection
if err := rl.FunctionConfig.As(&li); err != nil {
return err
}
for i, obj := range rl.Items {
if obj.GetApiVersion() == "apps/v1" && (obj.GetKind() == "Deployment" || obj.GetKind() == "StatefulSet" || obj.GetKind() == "DaemonSet" || obj.GetKind() == "ReplicaSet") {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment about taking advantage of the framework's matchers for this.

Btw, have you seen TestTemplateProcessor_ContainerPatchTemplates_MultipleWorkloadKinds, which shows how to use TemplateProcessor for a very similar use case? That processor handles the matter of where the container path is for you, to make it easy to support all the workload types. It also makes it so you don't need to worry about the found/new case, since it uses SMP.

var container Container
found, err := obj.Get(&container, "spec", "template", "spec", "containers", fmt.Sprintf("[name=%v]", li.ContainerName))
if err != nil {
return err
}
if found {
container.Image = li.ImageName
} else {
container = Container{
Name: li.ContainerName,
Image: li.ImageName,
}
}
if err = rl.Items[i].Set(container, "spec", "template", "spec", "containers", fmt.Sprintf("[name=%v]", li.ContainerName)); err != nil {
return err
}
}
}
return nil
}

// LoggerInjection is type definition of the functionConfig.
type LoggerInjection struct {
yaml.ResourceMeta `json:",inline" yaml:",inline"`

ContainerName string `json:"containerName" yaml:"containerName"`
ImageName string `json:"imageName" yaml:"imageName"`
}

// Container is a copy of corev1.Container.
type Container struct {
Name string `json:"name" yaml:"name"`
Image string `json:"image,omitempty" yaml:"image,omitempty"`
}
38 changes: 38 additions & 0 deletions kyaml/fn/framework/example_mutate_comments_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
package framework_test

import (
"os"
"strings"

"sigs.k8s.io/kustomize/kyaml/fn/framework"
"sigs.k8s.io/kustomize/kyaml/fn/framework/command"
)

// In this example, we mutate line comments for field metadata.name.
mengqiy marked this conversation as resolved.
Show resolved Hide resolved
// Some function may want to store some information in the comments (e.g.
// apply-setters function: https://catalog.kpt.dev/apply-setters/v0.2/)

func Example_dMutateComments() {
if err := command.AsMain(framework.ResourceListProcessorFunc(mutateComments)); err != nil {
os.Exit(1)
}
}

func mutateComments(rl *framework.ResourceList) error {
for i := range rl.Items {
lineComment, err := rl.Items[i].GetLineComment("metadata", "name")
if err != nil {
return err
}

if strings.TrimSpace(lineComment) == "" {
lineComment = "# bar-system"
} else {
lineComment = strings.Replace(lineComment, "foo", "bar", -1)
}
if err = rl.Items[i].SetLineComment(lineComment, "metadata", "name"); err != nil {
return err
}
}
return nil
}
29 changes: 29 additions & 0 deletions kyaml/fn/framework/example_read_field_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package framework_test

import (
"os"

"sigs.k8s.io/kustomize/kyaml/fn/framework"
"sigs.k8s.io/kustomize/kyaml/fn/framework/command"
)

// In this example, we read a field from the input object and print it to the log.
mengqiy marked this conversation as resolved.
Show resolved Hide resolved

func Example_aReadField() {
if err := command.AsMain(framework.ResourceListProcessorFunc(readField)); err != nil {
os.Exit(1)
}
}

func readField(rl *framework.ResourceList) error {
for _, obj := range rl.Items {
if obj.GetApiVersion() == "apps/v1" && obj.GetKind() == "Deployment" {
replicas, found, err := obj.GetNestedInt("spec", "replicas")
if !found || err != nil {
return err
}
framework.Logf("replicas is %v\n", replicas)
}
}
return nil
}
33 changes: 33 additions & 0 deletions kyaml/fn/framework/example_read_functionConfig_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package framework_test

import (
"os"

"sigs.k8s.io/kustomize/kyaml/fn/framework"
"sigs.k8s.io/kustomize/kyaml/fn/framework/command"
"sigs.k8s.io/kustomize/kyaml/yaml"
)

// In this example, we convert the functionConfig as strong typed object and then
// read a field from the functionConfig object.

func Example_bReadFunctionConfig() {
if err := command.AsMain(framework.ResourceListProcessorFunc(readFunctionConfig)); err != nil {
os.Exit(1)
}
}

func readFunctionConfig(rl *framework.ResourceList) error {
var sr SetReplicas
if err := rl.FunctionConfig.As(&sr); err != nil {
return err
}
framework.Logf("desired replicas is %v\n", sr.DesiredReplicas)
return nil
}

// SetReplicas is the type definition of the functionConfig
type SetReplicas struct {
yaml.ResourceIdentifier `json:",inline" yaml:",inline"`
DesiredReplicas int `json:"desiredReplicas,omitempty" yaml:"desiredReplicas,omitempty"`
}
25 changes: 25 additions & 0 deletions kyaml/fn/framework/example_set_field_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package framework_test

import (
"os"

"sigs.k8s.io/kustomize/kyaml/fn/framework"
"sigs.k8s.io/kustomize/kyaml/fn/framework/command"
)

// In this example, we read a field from the input object and print it to the log.
mengqiy marked this conversation as resolved.
Show resolved Hide resolved

func Example_cSetField() {
if err := command.AsMain(framework.ResourceListProcessorFunc(setField)); err != nil {
os.Exit(1)
}
}

func setField(rl *framework.ResourceList) error {
for _, obj := range rl.Items {
if obj.GetKind() == "Deployment" && obj.GetName() == "nginx" {
return obj.Set(10, "spec", "replicas")
}
}
return nil
}
33 changes: 33 additions & 0 deletions kyaml/fn/framework/example_validator_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package framework_test

import (
"os"

"sigs.k8s.io/kustomize/kyaml/fn/framework"
"sigs.k8s.io/kustomize/kyaml/fn/framework/command"
)

// This example implements a function that validate resources to ensure
// spec.template.spec.securityContext.runAsNonRoot is set in workload APIs.

func Example_validator() {
if err := command.AsMain(framework.ResourceListProcessorFunc(validator)); err != nil {
os.Exit(1)
}
}

func validator(rl *framework.ResourceList) error {
var results framework.Results
for _, obj := range rl.Items {
if obj.GetApiVersion() == "apps/v1" && (obj.GetKind() == "Deployment" || obj.GetKind() == "StatefulSet" || obj.GetKind() == "DaemonSet" || obj.GetKind() == "ReplicaSet") {
runAsNonRoot, _, err := obj.GetNestedBool("spec", "template", "spec", "securityContext", "runAsNonRoot")
if err != nil {
return framework.ErrorConfigObjectResult(err, obj)
}
if !runAsNonRoot {
results = append(results, framework.ConfigObjectResult("`spec.template.spec.securityContext.runAsNonRoot` must be set to true", obj, framework.Error))
}
}
}
return results
}
Loading