add flexibility around use of post-renderer (#195)

Author: joelanford

.github/workflows/ci.yml

@@ -76,7 +76,7 @@ jobs:
     - name: Set up Go
       uses: actions/setup-go@v3
       with:
-        go-version: ~1.17
+        go-version: ~1.18
       id: go
 
     - name: Check out code into the Go module directory

.github/workflows/deploy.yml

@@ -28,7 +28,7 @@ jobs:
     - name: Install Go
       uses: actions/setup-go@v3
       with:
-        go-version: ~1.17
+        go-version: ~1.18
 
     - name: Create release
       run: |

Dockerfile

@@ -1,5 +1,5 @@
 # Build the manager binary
-FROM --platform=$BUILDPLATFORM golang:1.17 as builder
+FROM --platform=$BUILDPLATFORM golang:1.18 as builder
 ARG TARGETOS
 ARG TARGETARCH
 

go.mod

@@ -1,6 +1,6 @@
 module github.com/operator-framework/helm-operator-plugins
 
-go 1.17
+go 1.18
 
 require (
 	github.com/blang/semver/v4 v4.0.0

go.sum

@@ -22,30 +22,21 @@ import (
 	"errors"
 	"fmt"
 
-	sdkhandler "github.com/operator-framework/operator-lib/handler"
 	"gomodules.xyz/jsonpatch/v2"
 	"helm.sh/helm/v3/pkg/action"
 	"helm.sh/helm/v3/pkg/chart"
-	"helm.sh/helm/v3/pkg/kube"
 	helmkube "helm.sh/helm/v3/pkg/kube"
-	"helm.sh/helm/v3/pkg/postrender"
 	"helm.sh/helm/v3/pkg/release"
 	"helm.sh/helm/v3/pkg/storage/driver"
 	apiextv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	apiextv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
-	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
 	apitypes "k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/strategicpatch"
 	"k8s.io/cli-runtime/pkg/resource"
 	"sigs.k8s.io/controller-runtime/pkg/client"
-	"sigs.k8s.io/yaml"
-
-	"github.com/operator-framework/helm-operator-plugins/internal/sdk/controllerutil"
-	"github.com/operator-framework/helm-operator-plugins/pkg/manifestutil"
 )
 
 type ActionClientGetter interface {
@@ -90,20 +81,28 @@ func (hcg *actionClientGetter) ActionClientFor(obj client.Object) (ActionInterfa
 	if err != nil {
 		return nil, err
 	}
-	postRenderer := createPostRenderer(rm, actionConfig.KubeClient, obj)
-	return &actionClient{actionConfig, postRenderer}, nil
+	postRenderer := DefaultPostRendererFunc(rm, actionConfig.KubeClient, obj)
+	return &actionClient{
+		conf:               actionConfig,
+		defaultInstallOpts: []InstallOption{WithInstallPostRenderer(postRenderer)},
+		defaultUpgradeOpts: []UpgradeOption{WithUpgradePostRenderer(postRenderer)},
+	}, nil
 }
 
 type actionClient struct {
-	conf         *action.Configuration
-	postRenderer postrender.PostRenderer
+	conf *action.Configuration
+
+	defaultGetOpts       []GetOption
+	defaultInstallOpts   []InstallOption
+	defaultUpgradeOpts   []UpgradeOption
+	defaultUninstallOpts []UninstallOption
 }
 
 var _ ActionInterface = &actionClient{}
 
 func (c *actionClient) Get(name string, opts ...GetOption) (*release.Release, error) {
 	get := action.NewGet(c.conf)
-	for _, o := range opts {
+	for _, o := range concat(c.defaultGetOpts, opts...) {
 		if err := o(get); err != nil {
 			return nil, err
 		}
@@ -113,8 +112,7 @@ func (c *actionClient) Get(name string, opts ...GetOption) (*release.Release, er
 
 func (c *actionClient) Install(name, namespace string, chrt *chart.Chart, vals map[string]interface{}, opts ...InstallOption) (*release.Release, error) {
 	install := action.NewInstall(c.conf)
-	install.PostRenderer = c.postRenderer
-	for _, o := range opts {
+	for _, o := range concat(c.defaultInstallOpts, opts...) {
 		if err := o(install); err != nil {
 			return nil, err
 		}
@@ -151,8 +149,7 @@ func (c *actionClient) Install(name, namespace string, chrt *chart.Chart, vals m
 
 func (c *actionClient) Upgrade(name, namespace string, chrt *chart.Chart, vals map[string]interface{}, opts ...UpgradeOption) (*release.Release, error) {
 	upgrade := action.NewUpgrade(c.conf)
-	upgrade.PostRenderer = c.postRenderer
-	for _, o := range opts {
+	for _, o := range concat(c.defaultUpgradeOpts, opts...) {
 		if err := o(upgrade); err != nil {
 			return nil, err
 		}
@@ -182,7 +179,7 @@ func (c *actionClient) Upgrade(name, namespace string, chrt *chart.Chart, vals m
 
 func (c *actionClient) Uninstall(name string, opts ...UninstallOption) (*release.UninstallReleaseResponse, error) {
 	uninstall := action.NewUninstall(c.conf)
-	for _, o := range opts {
+	for _, o := range concat(c.defaultUninstallOpts, opts...) {
 		if err := o(uninstall); err != nil {
 			return nil, err
 		}
@@ -305,56 +302,9 @@ func createJSONMergePatch(existingJSON, expectedJSON []byte) ([]byte, error) {
 	return json.Marshal(patchOps)
 }
 
-func createPostRenderer(rm meta.RESTMapper, kubeClient kube.Interface, owner client.Object) postrender.PostRenderer {
-	return &ownerPostRenderer{rm, kubeClient, owner}
-}
-
-type ownerPostRenderer struct {
-	rm         meta.RESTMapper
-	kubeClient kube.Interface
-	owner      client.Object
-}
-
-func (pr *ownerPostRenderer) Run(in *bytes.Buffer) (*bytes.Buffer, error) {
-	resourceList, err := pr.kubeClient.Build(in, false)
-	if err != nil {
-		return nil, err
-	}
-	out := bytes.Buffer{}
-
-	err = resourceList.Visit(func(r *resource.Info, err error) error {
-		if err != nil {
-			return err
-		}
-		objMap, err := runtime.DefaultUnstructuredConverter.ToUnstructured(r.Object)
-		if err != nil {
-			return err
-		}
-		u := &unstructured.Unstructured{Object: objMap}
-		useOwnerRef, err := controllerutil.SupportsOwnerReference(pr.rm, pr.owner, u)
-		if err != nil {
-			return err
-		}
-		if useOwnerRef && !manifestutil.HasResourcePolicyKeep(u.GetAnnotations()) {
-			ownerRef := metav1.NewControllerRef(pr.owner, pr.owner.GetObjectKind().GroupVersionKind())
-			ownerRefs := append(u.GetOwnerReferences(), *ownerRef)
-			u.SetOwnerReferences(ownerRefs)
-		} else {
-			if err := sdkhandler.SetOwnerAnnotations(pr.owner, u); err != nil {
-				return err
-			}
-		}
-		outData, err := yaml.Marshal(u.Object)
-		if err != nil {
-			return err
-		}
-		if _, err := out.WriteString("---\n" + string(outData)); err != nil {
-			return err
-		}
-		return nil
-	})
-	if err != nil {
-		return nil, err
-	}
-	return &out, nil
+func concat[T any](s1 []T, s2 ...T) []T {
+	out := make([]T, 0, len(s1)+len(s2))
+	out = append(out, s1...)
+	out = append(out, s2...)
+	return out
 }

pkg/client/actionclient.go

@@ -17,7 +17,6 @@ limitations under the License.
 package client
 
 import (
-	"bytes"
 	"context"
 	"errors"
 	"strconv"
@@ -27,7 +26,6 @@ import (
 	. "github.com/onsi/gomega"
 	"helm.sh/helm/v3/pkg/action"
 	"helm.sh/helm/v3/pkg/chartutil"
-	"helm.sh/helm/v3/pkg/kube"
 	"helm.sh/helm/v3/pkg/release"
 	"helm.sh/helm/v3/pkg/releaseutil"
 	"helm.sh/helm/v3/pkg/storage/driver"
@@ -40,6 +38,7 @@ import (
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	apitypes "k8s.io/apimachinery/pkg/types"
 	"k8s.io/cli-runtime/pkg/resource"
+	"k8s.io/utils/pointer"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
 	"sigs.k8s.io/yaml"
@@ -141,7 +140,7 @@ var _ = Describe("ActionClient", func() {
 						Expect(err).To(BeNil())
 						Expect(rel).NotTo(BeNil())
 					})
-					verifyRelease(cl, obj.GetNamespace(), rel)
+					verifyRelease(cl, obj, rel)
 				})
 				It("should uninstall a failed install", func() {
 					By("failing to install the release", func() {
@@ -209,7 +208,7 @@ var _ = Describe("ActionClient", func() {
 						Expect(err).To(BeNil())
 						Expect(rel).NotTo(BeNil())
 					})
-					verifyRelease(cl, obj.GetNamespace(), rel)
+					verifyRelease(cl, obj, rel)
 				})
 				When("using an option function that returns an error", func() {
 					It("should fail", func() {
@@ -253,7 +252,7 @@ var _ = Describe("ActionClient", func() {
 						Expect(err).To(BeNil())
 						Expect(rel).NotTo(BeNil())
 					})
-					verifyRelease(cl, obj.GetNamespace(), rel)
+					verifyRelease(cl, obj, rel)
 				})
 				It("should rollback a failed upgrade", func() {
 					By("failing to install the release", func() {
@@ -265,7 +264,7 @@ var _ = Describe("ActionClient", func() {
 					tmp := *installedRelease
 					rollbackRelease := &tmp
 					rollbackRelease.Version = installedRelease.Version + 2
-					verifyRelease(cl, obj.GetNamespace(), rollbackRelease)
+					verifyRelease(cl, obj, rollbackRelease)
 				})
 				When("using an option function that returns an error", func() {
 					It("should fail", func() {
@@ -305,7 +304,7 @@ var _ = Describe("ActionClient", func() {
 						err := ac.Reconcile(installedRelease)
 						Expect(err).To(BeNil())
 					})
-					verifyRelease(cl, obj.GetNamespace(), installedRelease)
+					verifyRelease(cl, obj, installedRelease)
 				})
 				It("should re-create deleted resources", func() {
 					By("deleting the manifest resources", func() {
@@ -319,7 +318,7 @@ var _ = Describe("ActionClient", func() {
 						err := ac.Reconcile(installedRelease)
 						Expect(err).To(BeNil())
 					})
-					verifyRelease(cl, obj.GetNamespace(), installedRelease)
+					verifyRelease(cl, obj, installedRelease)
 				})
 				It("should patch changed resources", func() {
 					By("changing manifest resources", func() {
@@ -344,7 +343,7 @@ var _ = Describe("ActionClient", func() {
 						err := ac.Reconcile(installedRelease)
 						Expect(err).To(BeNil())
 					})
-					verifyRelease(cl, obj.GetNamespace(), installedRelease)
+					verifyRelease(cl, obj, installedRelease)
 				})
 			})
 		})
@@ -516,34 +515,6 @@ var _ = Describe("ActionClient", func() {
 			Expect(patchType).To(Equal(apitypes.StrategicMergePatchType))
 		})
 	})
-
-	var _ = Describe("ownerPostRenderer", func() {
-		var (
-			pr    ownerPostRenderer
-			owner client.Object
-		)
-
-		BeforeEach(func() {
-			rm, err := apiutil.NewDynamicRESTMapper(cfg)
-			Expect(err).To(BeNil())
-
-			owner = newTestUnstructured([]interface{}{
-				map[string]interface{}{
-					"name": "test1",
-				},
-			})
-			pr = ownerPostRenderer{
-				owner:      owner,
-				rm:         rm,
-				kubeClient: kube.New(newRESTClientGetter(cfg, rm, owner.GetNamespace())),
-			}
-		})
-
-		It("fails on invalid input", func() {
-			_, err := pr.Run(bytes.NewBufferString("test"))
-			Expect(err).NotTo(BeNil())
-		})
-	})
 })
 
 func manifestToObjects(manifest string) []client.Object {
@@ -557,10 +528,10 @@ func manifestToObjects(manifest string) []client.Object {
 	return objs
 }
 
-func verifyRelease(cl client.Client, ns string, rel *release.Release) {
+func verifyRelease(cl client.Client, owner client.Object, rel *release.Release) {
 	By("verifying release secret exists at release version", func() {
 		releaseSecrets := &v1.SecretList{}
-		err := cl.List(context.TODO(), releaseSecrets, client.InNamespace(ns), client.MatchingLabels{"owner": "helm", "name": rel.Name})
+		err := cl.List(context.TODO(), releaseSecrets, client.InNamespace(owner.GetNamespace()), client.MatchingLabels{"owner": "helm", "name": rel.Name})
 		Expect(err).To(BeNil())
 		Expect(releaseSecrets.Items).To(HaveLen(rel.Version))
 		Expect(releaseSecrets.Items[rel.Version-1].Type).To(Equal(v1.SecretType("helm.sh/release.v1")))
@@ -578,6 +549,17 @@ func verifyRelease(cl client.Client, ns string, rel *release.Release) {
 			key := client.ObjectKeyFromObject(obj)
 			err := cl.Get(context.TODO(), key, obj)
 			Expect(err).To(BeNil())
+			Expect(obj.GetOwnerReferences()).To(HaveLen(1))
+			Expect(obj.GetOwnerReferences()[0]).To(Equal(
+				metav1.OwnerReference{
+					APIVersion:         owner.GetObjectKind().GroupVersionKind().GroupVersion().String(),
+					Kind:               owner.GetObjectKind().GroupVersionKind().Kind,
+					Name:               owner.GetName(),
+					UID:                owner.GetUID(),
+					Controller:         pointer.Bool(true),
+					BlockOwnerDeletion: pointer.Bool(true),
+				}),
+			)
 		}
 	})
 }