Add a WithValueTranslator option to Reconciller. (#114)

A Translator is a way to produces helm values based on the fetched custom
resource itself (unlike `Mapper` which can only see `Values`).

This way the code which converts the custom resource to Helm values can first
convert an `Unstructured` into a regular struct, and then rely on Go type
safety rather than work with a tree of maps from `string` to `interface{}`.

Thanks to having access to a `Context`, the code can also safely access the
network, for example in order to retrieve other resources from the k8s cluster,
when they are referenced by the custom resource.

Author: porridge

pkg/reconciler/internal/values/values.go

@@ -17,21 +17,37 @@ limitations under the License.
 package values
 
 import (
+	"context"
 	"fmt"
-	"os"
-
 	"helm.sh/helm/v3/pkg/chartutil"
 	"helm.sh/helm/v3/pkg/strvals"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"os"
 
 	"github.com/operator-framework/helm-operator-plugins/pkg/values"
 )
 
-type Values struct {
-	m map[string]interface{}
+var DefaultMapper = values.MapperFunc(func(v chartutil.Values) chartutil.Values { return v })
+
+var DefaultTranslator = values.TranslatorFunc(func(ctx context.Context, u *unstructured.Unstructured) (chartutil.Values, error) {
+	return getSpecMap(u)
+})
+
+func ApplyOverrides(overrideValues map[string]string, obj *unstructured.Unstructured) error {
+	specMap, err := getSpecMap(obj)
+	if err != nil {
+		return err
+	}
+	for inK, inV := range overrideValues {
+		val := fmt.Sprintf("%s=%s", inK, os.ExpandEnv(inV))
+		if err := strvals.ParseInto(val, specMap); err != nil {
+			return err
+		}
+	}
+	return nil
 }
 
-func FromUnstructured(obj *unstructured.Unstructured) (*Values, error) {
+func getSpecMap(obj *unstructured.Unstructured) (map[string]interface{}, error) {
 	if obj == nil || obj.Object == nil {
 		return nil, fmt.Errorf("nil object")
 	}
@@ -43,28 +59,5 @@ func FromUnstructured(obj *unstructured.Unstructured) (*Values, error) {
 	if !ok {
 		return nil, fmt.Errorf("spec must be a map")
 	}
-	return New(specMap), nil
-}
-
-func New(m map[string]interface{}) *Values {
-	return &Values{m: m}
-}
-
-func (v *Values) Map() map[string]interface{} {
-	if v == nil {
-		return nil
-	}
-	return v.m
+	return specMap, nil
 }
-
-func (v *Values) ApplyOverrides(in map[string]string) error {
-	for inK, inV := range in {
-		val := fmt.Sprintf("%s=%s", inK, os.ExpandEnv(inV))
-		if err := strvals.ParseInto(val, v.m); err != nil {
-			return err
-		}
-	}
-	return nil
-}
-
-var DefaultMapper = values.MapperFunc(func(v chartutil.Values) chartutil.Values { return v })

pkg/reconciler/internal/values/values_test.go

@@ -17,6 +17,7 @@ limitations under the License.
 package values_test
 
 import (
+	"context"
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 	"helm.sh/helm/v3/pkg/chartutil"
@@ -25,73 +26,50 @@ import (
 	. "github.com/operator-framework/helm-operator-plugins/pkg/reconciler/internal/values"
 )
 
-var _ = Describe("Values", func() {
-	var _ = Describe("FromUnstructured", func() {
-		It("should error with nil object", func() {
-			u := &unstructured.Unstructured{}
-			v, err := FromUnstructured(u)
-			Expect(v).To(BeNil())
-			Expect(err).NotTo(BeNil())
-		})
+var _ = Describe("ApplyOverrides", func() {
+	var u *unstructured.Unstructured
 
-		It("should error with missing spec", func() {
-			u := &unstructured.Unstructured{Object: map[string]interface{}{}}
-			v, err := FromUnstructured(u)
-			Expect(v).To(BeNil())
-			Expect(err).NotTo(BeNil())
+	When("Unstructured object is invalid", func() {
+		It("should error with nil unstructured", func() {
+			u = nil
+			Expect(ApplyOverrides(nil, u)).NotTo(BeNil())
 		})
 
-		It("should error with non-map spec", func() {
-			u := &unstructured.Unstructured{Object: map[string]interface{}{"spec": 0}}
-			v, err := FromUnstructured(u)
-			Expect(v).To(BeNil())
-			Expect(err).NotTo(BeNil())
+		It("should error with nil object", func() {
+			u = &unstructured.Unstructured{}
+			Expect(ApplyOverrides(nil, u)).NotTo(BeNil())
 		})
 
-		It("should succeed with valid spec", func() {
-			values := New(map[string]interface{}{"foo": "bar"})
-			u := &unstructured.Unstructured{Object: map[string]interface{}{"spec": values.Map()}}
-			Expect(FromUnstructured(u)).To(Equal(values))
+		It("should error with missing spec", func() {
+			u = &unstructured.Unstructured{Object: map[string]interface{}{}}
+			Expect(ApplyOverrides(nil, u)).NotTo(BeNil())
 		})
-	})
 
-	var _ = Describe("New", func() {
-		It("should return new values", func() {
-			m := map[string]interface{}{"foo": "bar"}
-			v := New(m)
-			Expect(v.Map()).To(Equal(m))
+		It("should error with non-map spec", func() {
+			u = &unstructured.Unstructured{Object: map[string]interface{}{"spec": 0}}
+			Expect(ApplyOverrides(nil, u)).NotTo(BeNil())
 		})
 	})
 
-	var _ = Describe("Map", func() {
-		It("should return nil with nil values", func() {
-			var v *Values
-			Expect(v.Map()).To(BeNil())
-		})
+	When("Unstructured object is valid", func() {
 
-		It("should return values as a map", func() {
-			m := map[string]interface{}{"foo": "bar"}
-			v := New(m)
-			Expect(v.Map()).To(Equal(m))
+		BeforeEach(func() {
+			u = &unstructured.Unstructured{Object: map[string]interface{}{"spec": map[string]interface{}{}}}
 		})
-	})
 
-	var _ = Describe("ApplyOverrides", func() {
 		It("should succeed with empty values", func() {
-			v := New(map[string]interface{}{})
-			Expect(v.ApplyOverrides(map[string]string{"foo": "bar"})).To(Succeed())
-			Expect(v.Map()).To(Equal(map[string]interface{}{"foo": "bar"}))
+			Expect(ApplyOverrides(map[string]string{"foo": "bar"}, u)).To(Succeed())
+			Expect(u.Object).To(Equal(map[string]interface{}{"spec": map[string]interface{}{"foo": "bar"}}))
 		})
 
-		It("should succeed with empty values", func() {
-			v := New(map[string]interface{}{"foo": "bar"})
-			Expect(v.ApplyOverrides(map[string]string{"foo": "baz"})).To(Succeed())
-			Expect(v.Map()).To(Equal(map[string]interface{}{"foo": "baz"}))
+		It("should succeed with non-empty values", func() {
+			u.Object["spec"].(map[string]interface{})["foo"] = "bar"
+			Expect(ApplyOverrides(map[string]string{"foo": "baz"}, u)).To(Succeed())
+			Expect(u.Object).To(Equal(map[string]interface{}{"spec": map[string]interface{}{"foo": "baz"}}))
 		})
 
 		It("should fail with invalid overrides", func() {
-			v := New(map[string]interface{}{"foo": "bar"})
-			Expect(v.ApplyOverrides(map[string]string{"foo[": "test"})).ToNot(BeNil())
+			Expect(ApplyOverrides(map[string]string{"foo[": "test"}, u)).ToNot(BeNil())
 		})
 	})
 })
@@ -103,3 +81,20 @@ var _ = Describe("DefaultMapper", func() {
 		Expect(out).To(Equal(in))
 	})
 })
+
+var _ = Describe("DefaultTranslator", func() {
+	var m map[string]interface{}
+
+	It("returns empty spec untouched", func() {
+		m = map[string]interface{}{}
+	})
+
+	It("returns filled spec untouched", func() {
+		m = map[string]interface{}{"something": 0}
+	})
+
+	AfterEach(func() {
+		u := &unstructured.Unstructured{Object: map[string]interface{}{"spec": m}}
+		Expect(DefaultTranslator.Translate(context.Background(), u)).To(Equal(chartutil.Values(m)))
+	})
+})

pkg/reconciler/reconciler.go

@@ -63,7 +63,8 @@ const uninstallFinalizer = "uninstall-helm-release"
 type Reconciler struct {
 	client             client.Client
 	actionClientGetter helmclient.ActionClientGetter
-	valueMapper        values.Mapper
+	valueTranslator    values.Translator
+	valueMapper        values.Mapper // nolint:staticcheck
 	eventRecorder      record.EventRecorder
 	preHooks           []hook.PreHook
 	postHooks          []hook.PostHook
@@ -234,8 +235,8 @@ func WithOverrideValues(overrides map[string]string) Option {
 		// Validate that overrides can be parsed and applied
 		// so that we fail fast during operator setup rather
 		// than during the first reconciliation.
-		m := internalvalues.New(map[string]interface{}{})
-		if err := m.ApplyOverrides(overrides); err != nil {
+		obj := &unstructured.Unstructured{Object: map[string]interface{}{"spec": map[string]interface{}{}}}
+		if err := internalvalues.ApplyOverrides(overrides, obj); err != nil {
 			return err
 		}
 
@@ -378,8 +379,36 @@ func WithPostHook(h hook.PostHook) Option {
 	}
 }
 
+// WithValueTranslator is an Option that configures a function that translates a
+// custom resource to the values passed to Helm.
+// Use this if you need to customize the logic that translates your custom resource to Helm values.
+// If you wish to, you can convert the Unstructured that is passed to your Translator to your own
+// Custom Resource struct like this:
+//
+//   import "k8s.io/apimachinery/pkg/runtime"
+//   foo := your.Foo{}
+//   if err = runtime.DefaultUnstructuredConverter.FromUnstructured(u.Object, &foo); err != nil {
+//     return nil, err
+//   }
+//   // work with the type-safe foo
+//
+// Alternatively, your translator can also work similarly to a Mapper, by accessing the spec with:
+//
+//   u.Object["spec"].(map[string]interface{})
+func WithValueTranslator(t values.Translator) Option {
+	return func(r *Reconciler) error {
+		r.valueTranslator = t
+		return nil
+	}
+}
+
 // WithValueMapper is an Option that configures a function that maps values
-// from a custom resource spec to the values passed to Helm
+// from a custom resource spec to the values passed to Helm.
+// Use this if you want to apply a transformation on the values obtained from your custom resource, before
+// they are passed to Helm.
+//
+// Deprecated: Use WithValueTranslator instead.
+// WithValueMapper will be removed in a future release.
 func WithValueMapper(m values.Mapper) Option {
 	return func(r *Reconciler) error {
 		r.valueMapper = m
@@ -483,7 +512,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (res ctrl.
 		return ctrl.Result{}, err
 	}
 
-	vals, err := r.getValues(obj)
+	vals, err := r.getValues(ctx, obj)
 	if err != nil {
 		u.UpdateStatus(
 			updater.EnsureCondition(conditions.Irreconcilable(corev1.ConditionTrue, conditions.ReasonErrorGettingValues, err)),
@@ -546,15 +575,15 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (res ctrl.
 	return ctrl.Result{RequeueAfter: r.reconcilePeriod}, nil
 }
 
-func (r *Reconciler) getValues(obj *unstructured.Unstructured) (chartutil.Values, error) {
-	crVals, err := internalvalues.FromUnstructured(obj)
-	if err != nil {
+func (r *Reconciler) getValues(ctx context.Context, obj *unstructured.Unstructured) (chartutil.Values, error) {
+	if err := internalvalues.ApplyOverrides(r.overrideValues, obj); err != nil {
 		return chartutil.Values{}, err
 	}
-	if err := crVals.ApplyOverrides(r.overrideValues); err != nil {
+	vals, err := r.valueTranslator.Translate(ctx, obj)
+	if err != nil {
 		return chartutil.Values{}, err
 	}
-	vals := r.valueMapper.Map(crVals.Map())
+	vals = r.valueMapper.Map(vals)
 	vals, err = chartutil.CoalesceValues(r.chrt, vals)
 	if err != nil {
 		return chartutil.Values{}, err
@@ -773,6 +802,9 @@ func (r *Reconciler) addDefaults(mgr ctrl.Manager, controllerName string) {
 	if r.eventRecorder == nil {
 		r.eventRecorder = mgr.GetEventRecorderFor(controllerName)
 	}
+	if r.valueTranslator == nil {
+		r.valueTranslator = internalvalues.DefaultTranslator
+	}
 	if r.valueMapper == nil {
 		r.valueMapper = internalvalues.DefaultMapper
 	}