Skip to content

Commit

Permalink
Add a WithValueTranslator option to Reconciller.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
porridge committed Nov 9, 2021
1 parent 1be4e1a commit eaf6277
Show file tree
Hide file tree
Showing 4 changed files with 94 additions and 5 deletions.
9 changes: 9 additions & 0 deletions pkg/reconciler/internal/values/values.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package values

import (
"context"
"fmt"
"os"

Expand Down Expand Up @@ -68,3 +69,11 @@ func (v *Values) ApplyOverrides(in map[string]string) error {
}

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) {
internalValues, err := FromUnstructured(u)
if err != nil {
return chartutil.Values{}, err
}
return internalValues.Map(), err
})
27 changes: 22 additions & 5 deletions pkg/reconciler/reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ const uninstallFinalizer = "uninstall-helm-release"
type Reconciler struct {
client client.Client
actionClientGetter helmclient.ActionClientGetter
valueTranslator values.Translator
valueMapper values.Mapper
eventRecorder record.EventRecorder
preHooks []hook.PreHook
Expand Down Expand Up @@ -375,8 +376,20 @@ 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.
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.
func WithValueMapper(m values.Mapper) Option {
return func(r *Reconciler) error {
r.valueMapper = m
Expand Down Expand Up @@ -471,7 +484,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)),
Expand Down Expand Up @@ -534,15 +547,16 @@ 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)
func (r *Reconciler) getValues(ctx context.Context, obj *unstructured.Unstructured) (chartutil.Values, error) {
vals, err := r.valueTranslator.Translate(ctx, obj)
if err != nil {
return chartutil.Values{}, err
}
crVals := internalvalues.New(vals)
if err := crVals.ApplyOverrides(r.overrideValues); err != nil {
return chartutil.Values{}, err
}
vals := r.valueMapper.Map(crVals.Map())
vals = r.valueMapper.Map(crVals.Map())
vals, err = chartutil.CoalesceValues(r.chrt, vals)
if err != nil {
return chartutil.Values{}, err
Expand Down Expand Up @@ -761,6 +775,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
}
Expand Down
49 changes: 49 additions & 0 deletions pkg/reconciler/reconciler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,16 @@ var _ = Describe("Reconciler", func() {
Expect(r.valueMapper.Map(chartutil.Values{})).To(Equal(chartutil.Values{"mapped": true}))
})
})
var _ = Describe("WithValueTranslator", func() {
It("should set the reconciler value translator", func() {
translator := values.TranslatorFunc(func(ctx context.Context, u *unstructured.Unstructured) (chartutil.Values, error) {
return chartutil.Values{"translated": true}, nil
})
Expect(WithValueTranslator(translator)(r)).To(Succeed())
Expect(r.valueTranslator).NotTo(BeNil())
Expect(r.valueTranslator.Translate(context.Background(), &unstructured.Unstructured{})).To(Equal(chartutil.Values{"translated": true}))
})
})
})

var _ = Describe("Reconcile", func() {
Expand Down Expand Up @@ -824,6 +834,45 @@ var _ = Describe("Reconciler", func() {
})
})
})
When("value translator fails", func() {
BeforeEach(func() {
r.valueTranslator = values.TranslatorFunc(func(ctx context.Context, u *unstructured.Unstructured) (chartutil.Values, error) {
return nil, errors.New("translation failure")
})
})
It("returns an error", func() {
By("reconciling unsuccessfully", func() {
res, err := r.Reconcile(ctx, req)
Expect(res).To(Equal(reconcile.Result{}))
Expect(err.Error()).To(ContainSubstring("translation failure"))
})

By("getting the CR", func() {
Expect(mgr.GetAPIReader().Get(ctx, objKey, obj)).To(Succeed())
})

By("verifying the CR status", func() {
objStat := &objStatus{}
Expect(runtime.DefaultUnstructuredConverter.FromUnstructured(obj.Object, objStat)).To(Succeed())
Expect(objStat.Status.Conditions.IsTrueFor(conditions.TypeInitialized)).To(BeTrue())
Expect(objStat.Status.Conditions.IsTrueFor(conditions.TypeIrreconcilable)).To(BeTrue())
Expect(objStat.Status.Conditions.IsTrueFor(conditions.TypeDeployed)).To(BeTrue())
Expect(objStat.Status.Conditions.IsUnknownFor(conditions.TypeReleaseFailed)).To(BeTrue())

c := objStat.Status.Conditions.GetCondition(conditions.TypeIrreconcilable)
Expect(c).NotTo(BeNil())
Expect(c.Reason).To(Equal(conditions.ReasonErrorGettingValues))
Expect(c.Message).To(ContainSubstring("translation failure"))

Expect(objStat.Status.DeployedRelease.Name).To(Equal(currentRelease.Name))
Expect(objStat.Status.DeployedRelease.Manifest).To(Equal(currentRelease.Manifest))
})

By("verifying the uninstall finalizer is not present on the CR", func() {
Expect(controllerutil.ContainsFinalizer(obj, uninstallFinalizer)).To(BeTrue())
})
})
})
When("requested CR release is not deployed", func() {
var actionConf *action.Configuration
BeforeEach(func() {
Expand Down
14 changes: 14 additions & 0 deletions pkg/values/values.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,13 @@ limitations under the License.
package values

import (
"context"
"helm.sh/helm/v3/pkg/chartutil"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
)

// TODO: Consider deprecating Mapper and overrides in favour of Translator.

type Mapper interface {
Map(chartutil.Values) chartutil.Values
}
Expand All @@ -29,3 +33,13 @@ type MapperFunc func(chartutil.Values) chartutil.Values
func (m MapperFunc) Map(v chartutil.Values) chartutil.Values {
return m(v)
}

type Translator interface {
Translate(ctx context.Context, unstructured *unstructured.Unstructured) (chartutil.Values, error)
}

type TranslatorFunc func(context.Context, *unstructured.Unstructured) (chartutil.Values, error)

func (t TranslatorFunc) Translate(ctx context.Context, u *unstructured.Unstructured) (chartutil.Values, error) {
return t(ctx, u)
}

0 comments on commit eaf6277

Please sign in to comment.