feat: improve mutate efficiency and logic (#48)

* feat: added improvements to child resource mutation

This adds inline functionality to mutate resources.  Prior to this change, the mutation functionality
would loop through all resources for each child resource to perform mutation.  This resulted in a lot
of if/then statements and really ugly code.  This new change mutates the resources inline and also
passes the request with context, and the reconciler so that cluster lookups and storage of data on the
context may happen between resources.

Also Fixes #17, which now makes the header comments more legible and strips the ugly start/end tags.

Signed-off-by: Dustin Scott <[email protected]>

* feat: move names into its own package

This will prevent import cycle conflicts when attempting to retrieve names from
packages.

Signed-off-by: Dustin Scott <[email protected]>

* feat: Fixes #29, create source file names with intelligence

This commit addresses an issue that creates very long file names for each manifest.  This
instead prefers the file name, and works its way back through the directory structure until a
unique name is found.  If the names cannot be deduplicated, then a count is appended to the
end.

Signed-off-by: Dustin Scott <[email protected]>

* feat: decrease mutate file name length

Signed-off-by: Dustin Scott <[email protected]>

* refactor: change from names to constants package

Signed-off-by: Dustin Scott <[email protected]>

* chore: fix linter errors

Signed-off-by: Dustin Scott <[email protected]>

* fix: backwards logic for checking out of range file names

Signed-off-by: Dustin Scott <[email protected]>

* chore: rename constants.go to names.go to make file more specific

Signed-off-by: Dustin Scott <[email protected]>

* chore: add pod disruption budget alias

Signed-off-by: Dustin Scott <[email protected]>

* chore: add webhook aliases

Signed-off-by: Dustin Scott <[email protected]>

internal/plugins/workload/v1/scaffolds/api.go

@@ -222,14 +222,30 @@ func (s *apiScaffolder) scaffoldAPI(
 		return fmt.Errorf("%w; %s", err, ErrScaffoldAPIResources)
 	}
 
-	// child resource definition files
+	// scaffolds the child resource definition files
 	// these are the resources defined in the static yaml manifests
 	for _, manifest := range *workload.GetManifests() {
 		if err := scaffold.Execute(
 			&resources.Definition{Builder: workload, Manifest: manifest},
 		); err != nil {
 			return fmt.Errorf("%w; %s", err, ErrScaffoldAPIChildResources)
 		}
+
+		// update the child resource mutation for each child resource
+		for i := range manifest.ChildResources {
+			if err := scaffold.Execute(
+				&resources.Mutate{Builder: workload, ChildResource: manifest.ChildResources[i]},
+			); err != nil {
+				return fmt.Errorf("%w; %s", err, ErrScaffoldAPIChildResources)
+			}
+		}
+	}
+
+	// scaffold the child resource naming helpers
+	if err := scaffold.Execute(
+		&resources.Constants{Builder: workload},
+	); err != nil {
+		return fmt.Errorf("%w; %s", err, ErrScaffoldAPIResources)
 	}
 
 	return nil

internal/plugins/workload/v1/scaffolds/templates/api/resources/constants.go

@@ -0,0 +1,74 @@
+// Copyright 2022 Nukleros
+// SPDX-License-Identifier: MIT
+
+package resources
+
+import (
+	"fmt"
+	"path/filepath"
+
+	"sigs.k8s.io/kubebuilder/v3/pkg/machinery"
+
+	"github.com/vmware-tanzu-labs/operator-builder/internal/workload/v1/kinds"
+)
+
+var _ machinery.Template = &Constants{}
+
+// Types scaffolds the child resource Constants files.
+type Constants struct {
+	machinery.TemplateMixin
+	machinery.BoilerplateMixin
+	machinery.RepositoryMixin
+	machinery.ResourceMixin
+
+	// input fields
+	Builder kinds.WorkloadBuilder
+
+	// template fields
+	ConstantStrings []string
+}
+
+func (f *Constants) SetTemplateDefaults() error {
+	f.Path = filepath.Join(
+		"apis",
+		f.Resource.Group,
+		f.Resource.Version,
+		f.Builder.GetPackageName(),
+		"constants",
+		"names.go",
+	)
+
+	children := kinds.GetWorkloadChildren(f.Builder)
+
+	for i := range children {
+		child := children[i]
+
+		if child.NameConstant() != "" {
+			f.ConstantStrings = append(
+				f.ConstantStrings,
+				fmt.Sprintf("%s = %q", child.UniqueName, child.NameConstant()),
+			)
+		}
+	}
+
+	f.TemplateBody = NamesTemplate
+	f.IfExistsAction = machinery.OverwriteFile
+
+	return nil
+}
+
+const NamesTemplate = `{{ .Boilerplate }}
+
+package constants
+
+{{ if .Builder.HasChildResources }}
+// this package includes the constants which include the resource names.  it is a standalone
+// package to prevent import cycle errors when attempting to reference the names from other
+// packages (e.g. mutate).
+const (
+	{{ range .ConstantStrings }}
+	{{- . }}
+	{{ end }}
+)
+{{ end }}
+`

internal/plugins/workload/v1/scaffolds/templates/api/resources/definition.go

@@ -65,7 +65,10 @@ import (
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
+	"github.com/nukleros/operator-builder-tools/pkg/controller/workload"
+
 	{{ .Resource.ImportAlias }} "{{ .Resource.Path }}"
+	"{{ .Resource.Path }}/{{ .Builder.GetPackageName }}/mutate"
 	{{- if .Builder.IsComponent }}
 	{{ .Builder.GetCollection.Spec.API.Group }}{{ .Builder.GetCollection.Spec.API.Version }} "{{ .Repo }}/apis/{{ .Builder.GetCollection.Spec.API.Group }}/{{ .Builder.GetCollection.Spec.API.Version }}"
 	{{ end -}}
@@ -75,29 +78,29 @@ import (
 {{ range .RBAC }}
 {{- .ToMarker }}
 {{ end }}
-{{ if ne .NameConstant "" }}const {{ .UniqueName }} = "{{ .NameConstant }}"{{ end }}
-
-// {{ .CreateFuncName }} creates the {{ .Name }} {{ .Kind }} resource.
+// {{ .CreateFuncName }} creates the {{ .Kind }} resource with name {{ .NameComment }}.
 func {{ .CreateFuncName }} (
 	parent *{{ $.Resource.ImportAlias }}.{{ $.Resource.Kind }},
 	{{ if $.Builder.IsComponent -}}
 	collection *{{ $.Builder.GetCollection.Spec.API.Group }}{{ $.Builder.GetCollection.Spec.API.Version }}.{{ $.Builder.GetCollection.Spec.API.Kind }},
 	{{ end -}}
+	reconciler workload.Reconciler,
+	req *workload.Request,
 ) ([]client.Object, error) {
 
 	{{- if ne .IncludeCode "" }}{{ .IncludeCode }}{{ end }}
 
-	resourceObjs := []client.Object{}
-
 	{{- .SourceCode }}
 
 	{{ if not $.Builder.IsClusterScoped }}
 	resourceObj.SetNamespace(parent.Namespace)
 	{{ end }}
 
-	resourceObjs = append(resourceObjs, resourceObj)
-
-	return resourceObjs, nil
+	{{ if $.Builder.IsComponent -}}
+	return mutate.{{ .MutateFuncName }}(resourceObj, parent, collection, reconciler, req)
+	{{ else -}}
+	return mutate.{{ .MutateFuncName }}(resourceObj, parent, reconciler, req)
+	{{ end -}}
 }
 {{ end }}
 `

internal/plugins/workload/v1/scaffolds/templates/api/resources/mutate.go

@@ -0,0 +1,83 @@
+// Copyright 2022 Nukleros
+// SPDX-License-Identifier: MIT
+
+package resources
+
+import (
+	"path/filepath"
+
+	"sigs.k8s.io/kubebuilder/v3/pkg/machinery"
+
+	"github.com/vmware-tanzu-labs/operator-builder/internal/workload/v1/kinds"
+	"github.com/vmware-tanzu-labs/operator-builder/internal/workload/v1/manifests"
+)
+
+var (
+	_ machinery.Template = &Mutate{}
+)
+
+// Mutate scaffolds the root command file for the companion CLI.
+type Mutate struct {
+	machinery.TemplateMixin
+	machinery.BoilerplateMixin
+	machinery.RepositoryMixin
+	machinery.ResourceMixin
+
+	// input variables
+	Builder       kinds.WorkloadBuilder
+	ChildResource manifests.ChildResource
+}
+
+func (f *Mutate) SetTemplateDefaults() error {
+	// set interface variables
+	f.Path = filepath.Join(
+		"apis",
+		f.Resource.Group,
+		f.Resource.Version,
+		f.Builder.GetPackageName(),
+		"mutate",
+		f.ChildResource.MutateFileName(),
+	)
+
+	f.TemplateBody = MutateTemplate
+
+	return nil
+}
+
+// GetIfExistsAction implements file.Builder interface.
+func (*Mutate) GetIfExistsAction() machinery.IfExistsAction {
+	return machinery.SkipFile
+}
+
+//nolint:lll
+const MutateTemplate = `{{ .Boilerplate }}
+
+package mutate
+
+import (
+	"sigs.k8s.io/controller-runtime/pkg/client"
+
+	"github.com/nukleros/operator-builder-tools/pkg/controller/workload"
+
+	{{ .Resource.ImportAlias }} "{{ .Resource.Path }}"
+	{{- if .Builder.IsComponent }}
+	{{ .Builder.GetCollection.Spec.API.Group }}{{ .Builder.GetCollection.Spec.API.Version }} "{{ .Repo }}/apis/{{ .Builder.GetCollection.Spec.API.Group }}/{{ .Builder.GetCollection.Spec.API.Version }}"
+	{{ end -}}
+)
+
+// {{ .ChildResource.MutateFuncName }} mutates the {{ .ChildResource.Kind }} resource with name {{ .ChildResource.NameComment }}.
+func {{ .ChildResource.MutateFuncName }} (
+	original client.Object,
+	parent *{{ .Resource.ImportAlias }}.{{ .Resource.Kind }}, {{ if .Builder.IsComponent }}collection *{{ .Builder.GetCollection.Spec.API.Group }}{{ .Builder.GetCollection.Spec.API.Version }}.{{ .Builder.GetCollection.Spec.API.Kind }},{{ end }}
+	reconciler workload.Reconciler, req *workload.Request,
+) ([]client.Object, error) {
+	// if either the reconciler or request are found to be nil, return the base object.
+	if reconciler == nil || req == nil {
+		return []client.Object{original}, nil
+	}
+
+	// mutation logic goes here
+
+	return []client.Object{original}, nil
+}
+`

internal/plugins/workload/v1/scaffolds/templates/api/resources/resources.go

@@ -97,21 +97,25 @@ func Sample(requiredOnly bool) string {
 func Generate(
 	workloadObj {{ .Resource.ImportAlias }}.{{ .Resource.Kind }},
 	collectionObj {{ .Builder.GetCollection.Spec.API.Group }}{{ .Builder.GetCollection.Spec.API.Version }}.{{ .Builder.GetCollection.Spec.API.Kind }},
-) ([]client.Object, error) {
 {{ else if .Builder.IsCollection -}}
-func Generate(collectionObj {{ .Builder.GetCollection.Spec.API.Group }}{{ .Builder.GetCollection.Spec.API.Version }}.{{ .Builder.GetCollection.Spec.API.Kind }}) ([]client.Object, error) {
+func Generate(
+	collectionObj {{ .Builder.GetCollection.Spec.API.Group }}{{ .Builder.GetCollection.Spec.API.Version }}.{{ .Builder.GetCollection.Spec.API.Kind }},
 {{ else -}}
-func Generate(workloadObj {{ .Resource.ImportAlias }}.{{ .Resource.Kind }}) ([]client.Object, error) {
+func Generate(
+	workloadObj {{ .Resource.ImportAlias }}.{{ .Resource.Kind }},
 {{ end -}}
+	reconciler workload.Reconciler,
+	req *workload.Request,
+) ([]client.Object, error) {
 	resourceObjects := []client.Object{}
 
 	for _, f := range CreateFuncs {
 		{{ if .Builder.IsComponent -}}
-		resources, err := f(&workloadObj, &collectionObj)
+		resources, err := f(&workloadObj, &collectionObj, reconciler, req)
 		{{ else if .Builder.IsCollection -}}
-		resources, err := f(&collectionObj)
+		resources, err := f(&collectionObj, reconciler, req)
 		{{ else -}}
-		resources, err := f(&workloadObj)
+		resources, err := f(&workloadObj, reconciler, req)
 		{{ end }}
 		if err != nil {
 			return nil, err
@@ -153,11 +157,11 @@ func GenerateForCLI(
 	{{ end }}
 
 	{{ if .Builder.IsComponent }}
-	return Generate(workloadObj, collectionObj)
+	return Generate(workloadObj, collectionObj, nil, nil)
 	{{ else if .Builder.IsCollection }}
-	return Generate(collectionObj)
+	return Generate(collectionObj, nil, nil)
 	{{ else }}
-	return Generate(workloadObj)
+	return Generate(workloadObj, nil, nil)
 	{{ end -}}
 }
 {{ end }}
@@ -170,6 +174,8 @@ var CreateFuncs = []func(
 	{{ if $.Builder.IsComponent -}}
 	*{{ .Builder.GetCollection.Spec.API.Group }}{{ .Builder.GetCollection.Spec.API.Version }}.{{ .Builder.GetCollection.Spec.API.Kind }},
 	{{ end -}}
+	workload.Reconciler,
+	*workload.Request,
 ) ([]client.Object, error) {
 	{{ range .CreateFuncNames }}
 		{{- . -}},
@@ -189,6 +195,8 @@ var InitFuncs = []func(
 	{{ if $.Builder.IsComponent -}}
 	*{{ .Builder.GetCollection.Spec.API.Group }}{{ .Builder.GetCollection.Spec.API.Version }}.{{ .Builder.GetCollection.Spec.API.Kind }},
 	{{ end -}}
+	workload.Reconciler,
+	*workload.Request,
 ) ([]client.Object, error) {
 	{{ range .InitFuncNames }}
 		{{- . -}},

internal/plugins/workload/v1/scaffolds/templates/controller/controller.go

@@ -343,34 +343,12 @@ func (r *{{ .Resource.Kind }}Reconciler) EnqueueRequestOnCollectionChange(req *w
 // GetResources resources runs the methods to properly construct the resources in memory.
 func (r *{{ .Resource.Kind }}Reconciler) GetResources(req *workload.Request) ([]client.Object, error) {
 	{{- if .Builder.HasChildResources }}
-	resourceObjects := []client.Object{}
-
 	component, {{ if .Builder.IsComponent }}collection,{{ end }} err := {{ .Builder.GetPackageName }}.ConvertWorkload(req.Workload{{ if .Builder.IsComponent }}, req.Collection{{ end }})
 	if err != nil {
 		return nil, err
 	}
 
-	// create resources in memory
-	resources, err := {{ .Builder.GetPackageName }}.Generate(*component{{ if .Builder.IsComponent }}, *collection{{ end }})
-	if err != nil {
-		return nil, err
-	}
-
-	// run through the mutation functions to mutate the resources
-	for _, resource := range resources {
-		mutatedResources, skip, err := r.Mutate(req, resource)
-		if err != nil {
-			return []client.Object{}, err
-		}
-
-		if skip {
-			continue
-		}
-
-		resourceObjects = append(resourceObjects, mutatedResources...)
-	}
-
-	return resourceObjects, nil
+	return {{ .Builder.GetPackageName }}.Generate(*component{{ if .Builder.IsComponent }}, *collection{{ end }}, r, req)
 {{- else -}}
 	return []client.Object{}, nil
 {{ end -}}
@@ -417,6 +395,7 @@ func (r *{{ .Resource.Kind }}Reconciler) CheckReady(req *workload.Request) (bool
 }
 
 // Mutate will run the mutate function for the workload.
+// WARN: this will be deprecated in the future.  See apis/group/version/kind/mutate*
 func (r *{{ .Resource.Kind }}Reconciler) Mutate(
 	req *workload.Request,
 	object client.Object,

internal/plugins/workload/v1/scaffolds/templates/test/e2e/workloads.go

@@ -101,7 +101,7 @@ func {{ .TesterName }}ChildrenFuncs(tester *E2ETest) error {
 		return fmt.Errorf("error in workload conversion; %w", err)
 	}
 
-	resourceObjects, err := {{ .Builder.GetPackageName }}.Generate(*workload{{ if .Builder.IsComponent }}, *collection){{ else }}){{ end }}
+	resourceObjects, err := {{ .Builder.GetPackageName }}.Generate(*workload{{ if .Builder.IsComponent }}, *collection, nil, nil){{ else }}, nil, nil){{ end }}
 	if err != nil {
 		return fmt.Errorf("unable to create objects in memory; %w", err)
 	}