diff --git a/.github/labeler.yml b/.github/labeler.yml index 164dc38..c169049 100644 --- a/.github/labeler.yml +++ b/.github/labeler.yml @@ -1,3 +1,5 @@ +upstream-triage: + - "./*" area/main-binary: - 'main.go' - 'internal/**/*' diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9b97925..13c7342 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,11 +1,7 @@ name: CI on: - push: - branches: - - '**' - pull_request: - branches: [ main ] + - push jobs: diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index cb4d112..2cf5b1f 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -1,13 +1,14 @@ name: Deploy -on: - push: - branches: - - '**' - tags: - - 'v*' - pull_request: - branches: [ main ] +# Disabled as we don't need docker images to use the helm-operator as a library. +#on: +# push: +# branches: +# - '**' +# tags: +# - 'v*' +# pull_request: +# branches: [ main ] jobs: goreleaser: diff --git a/README.md b/README.md index 0a8e4ac..4a6c54d 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,47 @@ # helm-operator -[![Build Status](https://github.com/joelanford/helm-operator/workflows/CI/badge.svg?branch=master) -[![Coverage Status](https://coveralls.io/repos/github/joelanford/helm-operator/badge.svg?branch=master)](https://coveralls.io/github/joelanford/helm-operator?branch=master) +![Build Status](https://github.com/stackrox/helm-operator/workflows/CI/badge.svg?branch=main) + +Experimental refactoring of the operator-framework's helm operator. + +## Why a fork? + +The Helm operator type automates Helm chart operations +by mapping the [values](https://helm.sh/docs/chart_template_guide/values_files/) of a Helm chart exactly to a +`CustomResourceDefinition` and defining its watched resources in a `watches.yaml` +[configuration](https://sdk.operatorframework.io/docs/building-operators/helm/tutorial/#watch-the-nginx-cr) file. + +For creating a [Level II+](https://sdk.operatorframework.io/docs/advanced-topics/operator-capabilities/operator-capabilities/) operator +that reuses an already existing Helm chart, we need a [hybrid](https://github.com/operator-framework/operator-sdk/issues/670) +between the Go and Helm operator types. + +The hybrid approach allows adding customizations to the Helm operator, such as: +- value mapping based on cluster state, or +- executing code in specific events. + +## Quick start + +- Add this module as a replace directive to your `go.mod`: + + ``` + go mod edit -replace=github.com/joelanford/helm-operator=github.com/stackrox/helm-operator@main + ``` + + For example: + + ```go + chart, err := loader.Load("path/to/chart") + if err != nil { + panic(err) + } + + reconciler := reconciler.New( + reconciler.WithChart(*chart), + reconciler.WithGroupVersionKind(gvk), + ) + + if err := reconciler.SetupWithManager(mgr); err != nil { + panic(fmt.Sprintf("unable to create reconciler: %s", err)) + } + ``` -Experimental refactoring of the operator-framework's helm operator diff --git a/internal/cmd/run/cmd.go b/internal/cmd/run/cmd.go index 84ccee9..9131cf2 100644 --- a/internal/cmd/run/cmd.go +++ b/internal/cmd/run/cmd.go @@ -169,7 +169,7 @@ func (r *run) run(cmd *cobra.Command) { os.Exit(1) } - if err := r.SetupWithManager(mgr); err != nil { + if err := r.SetupWithManager(mgr, reconciler.SetupOpts{}); err != nil { log.Error(err, "unable to create controller", "controller", "Helm") os.Exit(1) } diff --git a/pkg/annotation/annotation.go b/pkg/annotation/annotation.go index 329ef1e..b286e31 100644 --- a/pkg/annotation/annotation.go +++ b/pkg/annotation/annotation.go @@ -14,6 +14,30 @@ See the License for the specific language governing permissions and limitations under the License. */ +// Package annotation allows to set custom install, upgrade or uninstall options on custom resource objects with annotations. +// To create custom annotations implement the Install, Upgrade or Uninstall interface. +// +// Example: +// +// To disable hooks based on annotations the InstallDisableHooks is passed to the reconciler as an option. +// +// r, err := reconciler.New( +// reconciler.WithChart(*w.Chart), +// reconciler.WithGroupVersionKind(w.GroupVersionKind), +// reconciler.WithInstallAnnotations(annotation.InstallDisableHooks{}), +// ) +// +// If the reconciler detects an annotation named "helm.sdk.operatorframework.io/install-disable-hooks" +// on the watched custom resource, it sets the install.DisableHooks option to the annotations value. For more information +// take a look at the InstallDisableHooks.InstallOption method. +// +// kind: OperatorHelmKind +// apiVersion: test.example.com/v1 +// metadata: +// name: nginx-sample +// annotations: +// "helm.sdk.operatorframework.io/install-disable-hooks": true +// package annotation import ( @@ -30,27 +54,24 @@ var ( DefaultUninstallAnnotations = []Uninstall{UninstallDescription{}, UninstallDisableHooks{}} ) +// Install configures an install annotation. type Install interface { Name() string InstallOption(string) helmclient.InstallOption } +// Upgrade configures an upgrade annotation. type Upgrade interface { Name() string UpgradeOption(string) helmclient.UpgradeOption } +// Uninstall configures an uninstall annotation. type Uninstall interface { Name() string UninstallOption(string) helmclient.UninstallOption } -type InstallDisableHooks struct { - CustomName string -} - -var _ Install = &InstallDisableHooks{} - const ( defaultDomain = "helm.sdk.operatorframework.io" defaultInstallDisableHooksName = defaultDomain + "/install-disable-hooks" @@ -64,6 +85,12 @@ const ( defaultUninstallDescriptionName = defaultDomain + "/uninstall-description" ) +type InstallDisableHooks struct { + CustomName string +} + +var _ Install = &InstallDisableHooks{} + func (i InstallDisableHooks) Name() string { if i.CustomName != "" { return i.CustomName diff --git a/pkg/client/actionclient.go b/pkg/client/actionclient.go index e6315fe..c7a5d9a 100644 --- a/pkg/client/actionclient.go +++ b/pkg/client/actionclient.go @@ -62,6 +62,7 @@ type ActionInterface interface { Get(name string, opts ...GetOption) (*release.Release, error) Install(name, namespace string, chrt *chart.Chart, vals map[string]interface{}, opts ...InstallOption) (*release.Release, error) Upgrade(name, namespace string, chrt *chart.Chart, vals map[string]interface{}, opts ...UpgradeOption) (*release.Release, error) + MarkFailed(release *release.Release, reason string) error Uninstall(name string, opts ...UninstallOption) (*release.UninstallReleaseResponse, error) Reconcile(rel *release.Release) error } @@ -163,6 +164,7 @@ func (c *actionClient) Upgrade(name, namespace string, chrt *chart.Chart, vals m if rel != nil { rollback := action.NewRollback(c.conf) rollback.Force = true + rollback.MaxHistory = upgrade.MaxHistory // As of Helm 2.13, if Upgrade returns a non-nil release, that // means the release was also recorded in the release store. @@ -179,6 +181,14 @@ func (c *actionClient) Upgrade(name, namespace string, chrt *chart.Chart, vals m return rel, nil } +func (c *actionClient) MarkFailed(rel *release.Release, reason string) error { + infoCopy := *rel.Info + releaseCopy := *rel + releaseCopy.Info = &infoCopy + releaseCopy.SetStatus(release.StatusFailed, reason) + return c.conf.Releases.Update(&releaseCopy) +} + func (c *actionClient) Uninstall(name string, opts ...UninstallOption) (*release.UninstallReleaseResponse, error) { uninstall := action.NewUninstall(c.conf) for _, o := range opts { diff --git a/pkg/extensions/types.go b/pkg/extensions/types.go new file mode 100644 index 0000000..fe449f8 --- /dev/null +++ b/pkg/extensions/types.go @@ -0,0 +1,17 @@ +package extensions + +import ( + "context" + + "github.com/go-logr/logr" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" +) + +// UpdateStatusFunc is a function that updates an unstructured status. If the status has been modified, +// true must be returned, false otherwise. +type UpdateStatusFunc func(*unstructured.Unstructured) bool + +// ReconcileExtension is an arbitrary extension that can be implemented to run either before +// or after the main Helm reconciliation action. +// An error returned by a ReconcileExtension will cause the Reconcile to fail, unlike a hook error. +type ReconcileExtension func(context.Context, *unstructured.Unstructured, func(UpdateStatusFunc), logr.Logger) error diff --git a/pkg/reconciler/internal/conditions/conditions.go b/pkg/reconciler/internal/conditions/conditions.go index 892aee3..a37735b 100644 --- a/pkg/reconciler/internal/conditions/conditions.go +++ b/pkg/reconciler/internal/conditions/conditions.go @@ -41,6 +41,7 @@ const ( ReasonUpgradeError = status.ConditionReason("UpgradeError") ReasonReconcileError = status.ConditionReason("ReconcileError") ReasonUninstallError = status.ConditionReason("UninstallError") + ReasonPendingError = status.ConditionReason("PendingError") ) func Initialized(stat corev1.ConditionStatus, reason status.ConditionReason, message interface{}) status.Condition { diff --git a/pkg/reconciler/internal/fake/actionclient.go b/pkg/reconciler/internal/fake/actionclient.go index 410c4de..2bd6022 100644 --- a/pkg/reconciler/internal/fake/actionclient.go +++ b/pkg/reconciler/internal/fake/actionclient.go @@ -48,17 +48,19 @@ func (hcg *fakeActionClientGetter) ActionClientFor(_ crclient.Object) (client.Ac } type ActionClient struct { - Gets []GetCall - Installs []InstallCall - Upgrades []UpgradeCall - Uninstalls []UninstallCall - Reconciles []ReconcileCall - - HandleGet func() (*release.Release, error) - HandleInstall func() (*release.Release, error) - HandleUpgrade func() (*release.Release, error) - HandleUninstall func() (*release.UninstallReleaseResponse, error) - HandleReconcile func() error + Gets []GetCall + Installs []InstallCall + Upgrades []UpgradeCall + MarkFaileds []MarkFailedCall + Uninstalls []UninstallCall + Reconciles []ReconcileCall + + HandleGet func() (*release.Release, error) + HandleInstall func() (*release.Release, error) + HandleUpgrade func() (*release.Release, error) + HandleMarkFailed func() error + HandleUninstall func() (*release.UninstallReleaseResponse, error) + HandleReconcile func() error } func NewActionClient() ActionClient { @@ -109,6 +111,11 @@ type UpgradeCall struct { Opts []client.UpgradeOption } +type MarkFailedCall struct { + Release *release.Release + Reason string +} + type UninstallCall struct { Name string Opts []client.UninstallOption @@ -133,6 +140,11 @@ func (c *ActionClient) Upgrade(name, namespace string, chrt *chart.Chart, vals m return c.HandleUpgrade() } +func (c *ActionClient) MarkFailed(rel *release.Release, reason string) error { + c.MarkFaileds = append(c.MarkFaileds, MarkFailedCall{rel, reason}) + return c.HandleMarkFailed() +} + func (c *ActionClient) Uninstall(name string, opts ...client.UninstallOption) (*release.UninstallReleaseResponse, error) { c.Uninstalls = append(c.Uninstalls, UninstallCall{name, opts}) return c.HandleUninstall() diff --git a/pkg/reconciler/internal/hook/hook.go b/pkg/reconciler/internal/hook/hook.go index 8299739..9b2a305 100644 --- a/pkg/reconciler/internal/hook/hook.go +++ b/pkg/reconciler/internal/hook/hook.go @@ -37,18 +37,22 @@ import ( "github.com/joelanford/helm-operator/pkg/manifestutil" ) -func NewDependentResourceWatcher(c controller.Controller, rm meta.RESTMapper) hook.PostHook { +func NewDependentResourceWatcher(c controller.Controller, rm meta.RESTMapper, watchReleaseResources bool, extraWatches ...schema.GroupVersionKind) hook.PostHook { return &dependentResourceWatcher{ - controller: c, - restMapper: rm, - m: sync.Mutex{}, - watches: make(map[schema.GroupVersionKind]struct{}), + controller: c, + restMapper: rm, + watchReleaseResources: watchReleaseResources, + extraWatches: extraWatches, + m: sync.Mutex{}, + watches: make(map[schema.GroupVersionKind]struct{}), } } type dependentResourceWatcher struct { - controller controller.Controller - restMapper meta.RESTMapper + controller controller.Controller + restMapper meta.RESTMapper + watchReleaseResources bool + extraWatches []schema.GroupVersionKind m sync.Mutex watches map[schema.GroupVersionKind]struct{} @@ -58,16 +62,31 @@ func (d *dependentResourceWatcher) Exec(owner *unstructured.Unstructured, rel re // using predefined functions for filtering events dependentPredicate := predicate.DependentPredicateFuncs() - resources := releaseutil.SplitManifests(rel.Manifest) - d.m.Lock() - defer d.m.Unlock() - for _, r := range resources { - var obj unstructured.Unstructured - err := yaml.Unmarshal([]byte(r), &obj) - if err != nil { - return err + var allWatches []unstructured.Unstructured + if d.watchReleaseResources { + resources := releaseutil.SplitManifests(rel.Manifest) + for _, r := range resources { + var obj unstructured.Unstructured + err := yaml.Unmarshal([]byte(r), &obj) + if err != nil { + return err + } + + allWatches = append(allWatches, obj) } + } + // For extra watches, we only support same namespace + for _, extraWatchGVK := range d.extraWatches { + var obj unstructured.Unstructured + obj.SetGroupVersionKind(extraWatchGVK) + obj.SetNamespace(owner.GetNamespace()) + allWatches = append(allWatches, obj) + } + + d.m.Lock() + defer d.m.Unlock() + for _, obj := range allWatches { depGVK := obj.GroupVersionKind() if _, ok := d.watches[depGVK]; ok || depGVK.Empty() { continue diff --git a/pkg/reconciler/internal/hook/hook_test.go b/pkg/reconciler/internal/hook/hook_test.go index 6861e1c..dcda5f6 100644 --- a/pkg/reconciler/internal/hook/hook_test.go +++ b/pkg/reconciler/internal/hook/hook_test.go @@ -66,7 +66,7 @@ var _ = Describe("Hook", func() { rel = &release.Release{ Manifest: strings.Join([]string{rsOwnerNamespace}, "---\n"), } - drw = internalhook.NewDependentResourceWatcher(c, rm) + drw = internalhook.NewDependentResourceWatcher(c, rm, true) }) It("should fail with an invalid release manifest", func() { rel.Manifest = "---\nfoobar" @@ -110,7 +110,7 @@ var _ = Describe("Hook", func() { rel = &release.Release{ Manifest: strings.Join([]string{clusterRole, clusterRole, rsOwnerNamespace, rsOwnerNamespace}, "---\n"), } - drw = internalhook.NewDependentResourceWatcher(c, rm) + drw = internalhook.NewDependentResourceWatcher(c, rm, true) Expect(drw.Exec(owner, *rel, log)).To(Succeed()) Expect(c.WatchCalls).To(HaveLen(2)) Expect(c.WatchCalls[0].Handler).To(BeAssignableToTypeOf(&handler.EnqueueRequestForOwner{})) @@ -133,7 +133,7 @@ var _ = Describe("Hook", func() { rel = &release.Release{ Manifest: strings.Join([]string{rsOwnerNamespace, ssOtherNamespace}, "---\n"), } - drw = internalhook.NewDependentResourceWatcher(c, rm) + drw = internalhook.NewDependentResourceWatcher(c, rm, true) Expect(drw.Exec(owner, *rel, log)).To(Succeed()) Expect(c.WatchCalls).To(HaveLen(2)) Expect(c.WatchCalls[0].Handler).To(BeAssignableToTypeOf(&handler.EnqueueRequestForOwner{})) @@ -144,7 +144,7 @@ var _ = Describe("Hook", func() { rel = &release.Release{ Manifest: strings.Join([]string{clusterRole, clusterRoleBinding}, "---\n"), } - drw = internalhook.NewDependentResourceWatcher(c, rm) + drw = internalhook.NewDependentResourceWatcher(c, rm, true) Expect(drw.Exec(owner, *rel, log)).To(Succeed()) Expect(c.WatchCalls).To(HaveLen(2)) Expect(c.WatchCalls[0].Handler).To(BeAssignableToTypeOf(&handler.EnqueueRequestForOwner{})) @@ -154,7 +154,7 @@ var _ = Describe("Hook", func() { rel = &release.Release{ Manifest: strings.Join([]string{rsOwnerNamespaceWithKeep, ssOtherNamespaceWithKeep, clusterRoleWithKeep, clusterRoleBindingWithKeep}, "---\n"), } - drw = internalhook.NewDependentResourceWatcher(c, rm) + drw = internalhook.NewDependentResourceWatcher(c, rm, true) Expect(drw.Exec(owner, *rel, log)).To(Succeed()) Expect(c.WatchCalls).To(HaveLen(4)) Expect(c.WatchCalls[0].Handler).To(BeAssignableToTypeOf(&sdkhandler.EnqueueRequestForAnnotation{})) @@ -181,7 +181,7 @@ var _ = Describe("Hook", func() { rel = &release.Release{ Manifest: strings.Join([]string{rsOwnerNamespace}, "---\n"), } - drw = internalhook.NewDependentResourceWatcher(c, rm) + drw = internalhook.NewDependentResourceWatcher(c, rm, true) Expect(drw.Exec(owner, *rel, log)).To(Succeed()) Expect(c.WatchCalls).To(HaveLen(1)) Expect(c.WatchCalls[0].Handler).To(BeAssignableToTypeOf(&handler.EnqueueRequestForOwner{})) @@ -190,7 +190,7 @@ var _ = Describe("Hook", func() { rel = &release.Release{ Manifest: strings.Join([]string{clusterRole}, "---\n"), } - drw = internalhook.NewDependentResourceWatcher(c, rm) + drw = internalhook.NewDependentResourceWatcher(c, rm, true) Expect(drw.Exec(owner, *rel, log)).To(Succeed()) Expect(c.WatchCalls).To(HaveLen(1)) Expect(c.WatchCalls[0].Handler).To(BeAssignableToTypeOf(&sdkhandler.EnqueueRequestForAnnotation{})) @@ -199,7 +199,7 @@ var _ = Describe("Hook", func() { rel = &release.Release{ Manifest: strings.Join([]string{ssOtherNamespace}, "---\n"), } - drw = internalhook.NewDependentResourceWatcher(c, rm) + drw = internalhook.NewDependentResourceWatcher(c, rm, true) Expect(drw.Exec(owner, *rel, log)).To(Succeed()) Expect(c.WatchCalls).To(HaveLen(1)) Expect(c.WatchCalls[0].Handler).To(BeAssignableToTypeOf(&sdkhandler.EnqueueRequestForAnnotation{})) @@ -208,7 +208,7 @@ var _ = Describe("Hook", func() { rel = &release.Release{ Manifest: strings.Join([]string{rsOwnerNamespaceWithKeep, ssOtherNamespaceWithKeep, clusterRoleWithKeep}, "---\n"), } - drw = internalhook.NewDependentResourceWatcher(c, rm) + drw = internalhook.NewDependentResourceWatcher(c, rm, true) Expect(drw.Exec(owner, *rel, log)).To(Succeed()) Expect(c.WatchCalls).To(HaveLen(3)) Expect(c.WatchCalls[0].Handler).To(BeAssignableToTypeOf(&sdkhandler.EnqueueRequestForAnnotation{})) diff --git a/pkg/reconciler/internal/updater/updater.go b/pkg/reconciler/internal/updater/updater.go index 590b5be..54ca71f 100644 --- a/pkg/reconciler/internal/updater/updater.go +++ b/pkg/reconciler/internal/updater/updater.go @@ -26,6 +26,7 @@ import ( "k8s.io/client-go/util/retry" "sigs.k8s.io/controller-runtime/pkg/client" + "github.com/joelanford/helm-operator/pkg/extensions" "github.com/joelanford/helm-operator/pkg/internal/sdk/controllerutil" "github.com/joelanford/helm-operator/pkg/internal/sdk/status" ) @@ -53,6 +54,21 @@ func (u *Updater) UpdateStatus(fs ...UpdateStatusFunc) { u.updateStatusFuncs = append(u.updateStatusFuncs, fs...) } +func (u *Updater) UpdateStatusCustom(f extensions.UpdateStatusFunc) { + updateFn := func(status *helmAppStatus) bool { + status.updateStatusObject() + + unstructuredStatus := unstructured.Unstructured{Object: status.StatusObject} + if !f(&unstructuredStatus) { + return false + } + _ = runtime.DefaultUnstructuredConverter.FromUnstructured(unstructuredStatus.Object, status) + status.StatusObject = unstructuredStatus.Object + return true + } + u.UpdateStatus(updateFn) +} + func (u *Updater) Apply(ctx context.Context, obj *unstructured.Unstructured) error { backoff := retry.DefaultRetry @@ -66,11 +82,8 @@ func (u *Updater) Apply(ctx context.Context, obj *unstructured.Unstructured) err needsStatusUpdate = f(st) || needsStatusUpdate } if needsStatusUpdate { - uSt, err := runtime.DefaultUnstructuredConverter.ToUnstructured(st) - if err != nil { - return err - } - obj.Object["status"] = uSt + st.updateStatusObject() + obj.Object["status"] = st.StatusObject return u.client.Status().Update(ctx, obj) } return nil @@ -149,10 +162,25 @@ func RemoveDeployedRelease() UpdateStatusFunc { } type helmAppStatus struct { + StatusObject map[string]interface{} `json:"-"` + Conditions status.Conditions `json:"conditions"` DeployedRelease *helmAppRelease `json:"deployedRelease,omitempty"` } +func (s *helmAppStatus) updateStatusObject() { + unstructuredHelmAppStatus, _ := runtime.DefaultUnstructuredConverter.ToUnstructured(s) + if s.StatusObject == nil { + s.StatusObject = make(map[string]interface{}) + } + s.StatusObject["conditions"] = unstructuredHelmAppStatus["conditions"] + if deployedRelease := unstructuredHelmAppStatus["deployedRelease"]; deployedRelease != nil { + s.StatusObject["deployedRelease"] = deployedRelease + } else { + delete(s.StatusObject, "deployedRelease") + } +} + type helmAppRelease struct { Name string `json:"name,omitempty"` Manifest string `json:"manifest,omitempty"` @@ -175,6 +203,7 @@ func statusFor(obj *unstructured.Unstructured) *helmAppStatus { case map[string]interface{}: out := &helmAppStatus{} _ = runtime.DefaultUnstructuredConverter.FromUnstructured(s, out) + out.StatusObject = s return out default: return &helmAppStatus{} diff --git a/pkg/reconciler/internal/updater/updater_test.go b/pkg/reconciler/internal/updater/updater_test.go index 7df9d30..a52f2dc 100644 --- a/pkg/reconciler/internal/updater/updater_test.go +++ b/pkg/reconciler/internal/updater/updater_test.go @@ -86,6 +86,71 @@ var _ = Describe("Updater", func() { Expect((obj.Object["status"].(map[string]interface{}))["conditions"]).To(HaveLen(1)) Expect(obj.GetResourceVersion()).NotTo(Equal(resourceVersion)) }) + + It("should support a mix of standard and custom status updates", func() { + u.UpdateStatus(EnsureCondition(conditions.Deployed(corev1.ConditionTrue, "", ""))) + u.UpdateStatusCustom(func(uSt *unstructured.Unstructured) bool { + Expect(unstructured.SetNestedMap(uSt.Object, map[string]interface{}{"bar": "baz"}, "foo")).To(Succeed()) + return true + }) + u.UpdateStatus(EnsureCondition(conditions.Irreconcilable(corev1.ConditionFalse, "", ""))) + u.UpdateStatusCustom(func(uSt *unstructured.Unstructured) bool { + Expect(unstructured.SetNestedField(uSt.Object, "quux", "foo", "qux")).To(Succeed()) + return true + }) + u.UpdateStatus(EnsureCondition(conditions.Initialized(corev1.ConditionTrue, "", ""))) + + Expect(u.Apply(context.TODO(), obj)).To(Succeed()) + Expect(client.Get(context.TODO(), types.NamespacedName{Namespace: "testNamespace", Name: "testDeployment"}, obj)).To(Succeed()) + Expect((obj.Object["status"].(map[string]interface{}))["conditions"]).To(HaveLen(3)) + _, found, err := unstructured.NestedFieldNoCopy(obj.Object, "status", "deployedRelease") + Expect(found).To(BeFalse()) + Expect(err).To(Not(HaveOccurred())) + + val, found, err := unstructured.NestedString(obj.Object, "status", "foo", "bar") + Expect(val).To(Equal("baz")) + Expect(found).To(BeTrue()) + Expect(err).To(Not(HaveOccurred())) + + val, found, err = unstructured.NestedString(obj.Object, "status", "foo", "qux") + Expect(val).To(Equal("quux")) + Expect(found).To(BeTrue()) + Expect(err).To(Not(HaveOccurred())) + }) + + It("should preserve any custom status across multiple apply calls", func() { + u.UpdateStatusCustom(func(uSt *unstructured.Unstructured) bool { + Expect(unstructured.SetNestedMap(uSt.Object, map[string]interface{}{"bar": "baz"}, "foo")).To(Succeed()) + return true + }) + Expect(u.Apply(context.TODO(), obj)).To(Succeed()) + + Expect(client.Get(context.TODO(), types.NamespacedName{Namespace: "testNamespace", Name: "testDeployment"}, obj)).To(Succeed()) + + _, found, err := unstructured.NestedFieldNoCopy(obj.Object, "status", "deployedRelease") + Expect(found).To(BeFalse()) + Expect(err).To(Not(HaveOccurred())) + + val, found, err := unstructured.NestedString(obj.Object, "status", "foo", "bar") + Expect(val).To(Equal("baz")) + Expect(found).To(BeTrue()) + Expect(err).To(Succeed()) + + u.UpdateStatus(EnsureCondition(conditions.Deployed(corev1.ConditionTrue, "", ""))) + Expect(u.Apply(context.TODO(), obj)).To(Succeed()) + + Expect(client.Get(context.TODO(), types.NamespacedName{Namespace: "testNamespace", Name: "testDeployment"}, obj)).To(Succeed()) + Expect((obj.Object["status"].(map[string]interface{}))["conditions"]).To(HaveLen(1)) + + _, found, err = unstructured.NestedFieldNoCopy(obj.Object, "status", "deployedRelease") + Expect(found).To(BeFalse()) + Expect(err).To(Not(HaveOccurred())) + + val, found, err = unstructured.NestedString(obj.Object, "status", "foo", "bar") + Expect(val).To(Equal("baz")) + Expect(found).To(BeTrue()) + Expect(err).To(Succeed()) + }) }) }) @@ -241,8 +306,9 @@ var _ = Describe("statusFor", func() { }) It("should handle map[string]interface{}", func() { - obj.Object["status"] = map[string]interface{}{} - Expect(statusFor(obj)).To(Equal(&helmAppStatus{})) + uSt := map[string]interface{}{} + obj.Object["status"] = uSt + Expect(statusFor(obj)).To(Equal(&helmAppStatus{StatusObject: uSt})) }) It("should handle arbitrary types", func() { diff --git a/pkg/reconciler/internal/values/values.go b/pkg/reconciler/internal/values/values.go index 5eef1b3..b188c20 100644 --- a/pkg/reconciler/internal/values/values.go +++ b/pkg/reconciler/internal/values/values.go @@ -17,6 +17,7 @@ limitations under the License. package values import ( + "context" "fmt" "os" @@ -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 +}) diff --git a/pkg/reconciler/reconciler.go b/pkg/reconciler/reconciler.go index abecaa9..20c0110 100644 --- a/pkg/reconciler/reconciler.go +++ b/pkg/reconciler/reconciler.go @@ -46,6 +46,7 @@ import ( "github.com/joelanford/helm-operator/pkg/annotation" helmclient "github.com/joelanford/helm-operator/pkg/client" + "github.com/joelanford/helm-operator/pkg/extensions" "github.com/joelanford/helm-operator/pkg/hook" "github.com/joelanford/helm-operator/pkg/internal/sdk/controllerutil" "github.com/joelanford/helm-operator/pkg/reconciler/internal/conditions" @@ -61,18 +62,27 @@ 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 postHooks []hook.PostHook + preExtensions []extensions.ReconcileExtension + postExtensions []extensions.ReconcileExtension + log logr.Logger gvk *schema.GroupVersionKind chrt *chart.Chart overrideValues map[string]string skipDependentWatches bool + extraDependentWatches []schema.GroupVersionKind maxConcurrentReconciles int reconcilePeriod time.Duration + markFailedAfter time.Duration + maxHistory int + + stripManifestFromStatus bool annotSetupOnce sync.Once annotations map[string]struct{} @@ -115,6 +125,10 @@ func (r *Reconciler) setupAnnotationMaps() { r.uninstallAnnotations = make(map[string]annotation.Uninstall) } +type SetupOpts struct { + DisableSetupScheme bool +} + // SetupWithManager configures a controller for the Reconciler and registers // watches. It also uses the passed Manager to initialize default values for the // Reconciler and sets up the manager's scheme with the Reconciler's configured @@ -122,11 +136,13 @@ func (r *Reconciler) setupAnnotationMaps() { // // If an error occurs setting up the Reconciler with the manager, it is // returned. -func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error { +func (r *Reconciler) SetupWithManager(mgr ctrl.Manager, opts SetupOpts) error { controllerName := fmt.Sprintf("%v-controller", strings.ToLower(r.gvk.Kind)) r.addDefaults(mgr, controllerName) - r.setupScheme(mgr) + if !opts.DisableSetupScheme { + r.setupScheme(mgr) + } c, err := controller.New(controllerName, mgr, controller.Options{Reconciler: r, MaxConcurrentReconciles: r.maxConcurrentReconciles}) if err != nil { @@ -252,6 +268,27 @@ func SkipDependentWatches(skip bool) Option { } } +// WithExtraDependentWatches is an option that configures whether the reconciler +// will watch objects of the given kind. These objects will be watched even if +// SkipDependentWatches is set to true. +func WithExtraDependentWatches(gvks ...schema.GroupVersionKind) Option { + return func(r *Reconciler) error { + r.extraDependentWatches = append(r.extraDependentWatches, gvks...) + return nil + } +} + +// StripManifestFromStatus is an Option that configures whether the manifest +// should be removed from the automatically populated status. +// This is recommended if the manifest might return sensitive data (i.e., +// secrets). +func StripManifestFromStatus(strip bool) Option { + return func(r *Reconciler) error { + r.stripManifestFromStatus = strip + return nil + } +} + // WithMaxConcurrentReconciles is an Option that configures the number of // concurrent reconciles that the controller will run. // @@ -280,6 +317,30 @@ func WithReconcilePeriod(rp time.Duration) Option { } } +// WithMaxReleaseHistory specifies the maximum size of the Helm release history maintained +// on upgrades/rollbacks. Zero (default) means unlimited. +func WithMaxReleaseHistory(maxHistory int) Option { + return func(r *Reconciler) error { + if maxHistory < 0 { + return errors.New("maximum Helm release history size must not be negative") + } + r.maxHistory = maxHistory + return nil + } +} + +// WithMarkFailedAfter specifies the duration after which the reconciler will mark a release in a pending (locked) +// state as false in order to allow rolling forward. +func WithMarkFailedAfter(duration time.Duration) Option { + return func(r *Reconciler) error { + if duration < 0 { + return errors.New("auto-rollback after duration must not be negative") + } + r.markFailedAfter = duration + return nil + } +} + // WithInstallAnnotations is an Option that configures Install annotations // to enable custom action.Install fields to be set based on the value of // annotations found in the custom resource watched by this reconciler. @@ -353,6 +414,20 @@ func WithPreHook(h hook.PreHook) Option { } } +// WithPreExtension is an Option that configures the reconciler to run the given +// extension before performing any reconciliation steps (including values translation). +// An error returned from the extension will cause the reconciliation to fail. +// This should be preferred to WithPreHook in most cases, except for when the logic +// depends on the translated Helm values. +// The extension will be invoked with the raw object state; meaning it needs to be careful +// to check for existence of the deletionTimestamp field. +func WithPreExtension(e extensions.ReconcileExtension) Option { + return func(r *Reconciler) error { + r.preExtensions = append(r.preExtensions, e) + return nil + } +} + // WithPostHook is an Option that configures the reconciler to run the given // PostHook just after performing any non-uninstall release actions. func WithPostHook(h hook.PostHook) Option { @@ -362,8 +437,36 @@ func WithPostHook(h hook.PostHook) Option { } } +// WithPostExtension is an Option that configures the reconciler to run the given +// extension after performing any reconciliation steps (including uninstall of the release, +// but not removal of the finalizer). +// An error returned from the extension will cause the reconciliation to fail, which might +// prevent the finalizer from getting removed. +// This should be preferred to WithPostHook in most cases, except for when the logic +// depends on the translated Helm values. +// The extension will be invoked with the raw object state; meaning it needs to be careful +// to check for existence of the deletionTimestamp field. +func WithPostExtension(e extensions.ReconcileExtension) Option { + return func(r *Reconciler) error { + r.postExtensions = append(r.postExtensions, e) + return nil + } +} + +// 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 @@ -449,16 +552,26 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (res ctrl. if errors.Is(err, driver.ErrReleaseNotFound) { u.UpdateStatus(updater.EnsureCondition(conditions.Deployed(corev1.ConditionFalse, "", ""))) } else if err == nil { - ensureDeployedRelease(&u, rel) + r.ensureDeployedRelease(&u, rel) } u.UpdateStatus(updater.EnsureCondition(conditions.Initialized(corev1.ConditionTrue, "", ""))) + for _, ext := range r.preExtensions { + if err := ext(ctx, obj, u.UpdateStatusCustom, r.log); err != nil { + u.UpdateStatus( + updater.EnsureCondition(conditions.Irreconcilable(corev1.ConditionTrue, conditions.ReasonReconcileError, err)), + updater.EnsureConditionUnknown(conditions.TypeReleaseFailed), + ) + return ctrl.Result{}, err + } + } + if obj.GetDeletionTimestamp() != nil { err := r.handleDeletion(ctx, actionClient, obj, log) 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)), @@ -477,6 +590,10 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (res ctrl. ) return ctrl.Result{}, err } + if state == statePending { + return r.handlePending(actionClient, rel, &u, log) + } + u.UpdateStatus(updater.EnsureCondition(conditions.Irreconcilable(corev1.ConditionFalse, "", ""))) for _, h := range r.preHooks { @@ -512,7 +629,17 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (res ctrl. } } - ensureDeployedRelease(&u, rel) + for _, ext := range r.postExtensions { + if err := ext(ctx, obj, u.UpdateStatusCustom, r.log); err != nil { + u.UpdateStatus( + updater.EnsureCondition(conditions.Irreconcilable(corev1.ConditionTrue, conditions.ReasonReconcileError, err)), + updater.EnsureConditionUnknown(conditions.TypeReleaseFailed), + ) + return ctrl.Result{}, err + } + } + + r.ensureDeployedRelease(&u, rel) u.UpdateStatus( updater.EnsureCondition(conditions.ReleaseFailed(corev1.ConditionFalse, "", "")), updater.EnsureCondition(conditions.Irreconcilable(corev1.ConditionFalse, "", "")), @@ -521,15 +648,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 @@ -543,6 +671,7 @@ const ( stateNeedsInstall helmReleaseState = "needs install" stateNeedsUpgrade helmReleaseState = "needs upgrade" stateUnchanged helmReleaseState = "unchanged" + statePending helmReleaseState = "pending" stateError helmReleaseState = "error" ) @@ -566,7 +695,7 @@ func (r *Reconciler) handleDeletion(ctx context.Context, actionClient helmclient err = applyErr } }() - return r.doUninstall(actionClient, &uninstallUpdater, obj, log) + return r.doUninstall(ctx, actionClient, &uninstallUpdater, obj, log) }(); err != nil { return err } @@ -591,7 +720,17 @@ func (r *Reconciler) getReleaseState(client helmclient.ActionInterface, obj meta return nil, stateNeedsInstall, nil } + if currentRelease.Info != nil && currentRelease.Info.Status.IsPending() { + return currentRelease, statePending, nil + } + var opts []helmclient.UpgradeOption + if r.maxHistory > 0 { + opts = append(opts, func(u *action.Upgrade) error { + u.MaxHistory = r.maxHistory + return nil + }) + } for name, annot := range r.upgradeAnnotations { if v, ok := obj.GetAnnotations()[name]; ok { opts = append(opts, annot.UpgradeOption(v)) @@ -636,6 +775,12 @@ func (r *Reconciler) doInstall(actionClient helmclient.ActionInterface, u *updat func (r *Reconciler) doUpgrade(actionClient helmclient.ActionInterface, u *updater.Updater, obj *unstructured.Unstructured, vals map[string]interface{}, log logr.Logger) (*release.Release, error) { var opts []helmclient.UpgradeOption + if r.maxHistory > 0 { + opts = append(opts, func(u *action.Upgrade) error { + u.MaxHistory = r.maxHistory + return nil + }) + } for name, annot := range r.upgradeAnnotations { if v, ok := obj.GetAnnotations()[name]; ok { opts = append(opts, annot.UpgradeOption(v)) @@ -656,6 +801,35 @@ func (r *Reconciler) doUpgrade(actionClient helmclient.ActionInterface, u *updat return rel, nil } +func (r *Reconciler) handlePending(actionClient helmclient.ActionInterface, rel *release.Release, u *updater.Updater, log logr.Logger) (ctrl.Result, error) { + err := r.doHandlePending(actionClient, rel, log) + if err == nil { + err = errors.New("unknown error handling pending release") + } + u.UpdateStatus( + updater.EnsureCondition(conditions.Irreconcilable(corev1.ConditionTrue, conditions.ReasonPendingError, err))) + return ctrl.Result{}, err +} + +func (r *Reconciler) doHandlePending(actionClient helmclient.ActionInterface, rel *release.Release, log logr.Logger) error { + if r.markFailedAfter <= 0 { + return errors.New("Release is in a pending (locked) state and cannot be modified. User intervention is required.") + } + if rel.Info == nil || rel.Info.LastDeployed.IsZero() { + return errors.New("Release is in a pending (locked) state and lacks 'last deployed' timestamp. User intervention is required.") + } + if pendingSince := time.Since(rel.Info.LastDeployed.Time); pendingSince < r.markFailedAfter { + return fmt.Errorf("Release is in a pending (locked) state and cannot currently be modified. Release will be marked failed to allow a roll-forward in %v.", r.markFailedAfter-pendingSince) + } + + log.Info("Marking release as failed", "releaseName", rel.Name) + err := actionClient.MarkFailed(rel, fmt.Sprintf("operator marked pending (locked) release as failed after state did not change for %v", r.markFailedAfter)) + if err != nil { + return fmt.Errorf("Failed to mark pending (locked) release as failed: %w", err) + } + return fmt.Errorf("marked release %s as failed to allow upgrade to succeed in next reconcile attempt", rel.Name) +} + func (r *Reconciler) reportOverrideEvents(obj runtime.Object) { for k, v := range r.overrideValues { r.eventRecorder.Eventf(obj, "Warning", "ValueOverridden", @@ -683,7 +857,7 @@ func (r *Reconciler) doReconcile(actionClient helmclient.ActionInterface, u *upd return nil } -func (r *Reconciler) doUninstall(actionClient helmclient.ActionInterface, u *updater.Updater, obj *unstructured.Unstructured, log logr.Logger) error { +func (r *Reconciler) doUninstall(ctx context.Context, actionClient helmclient.ActionInterface, u *updater.Updater, obj *unstructured.Unstructured, log logr.Logger) error { var opts []helmclient.UninstallOption for name, annot := range r.uninstallAnnotations { if v, ok := obj.GetAnnotations()[name]; ok { @@ -703,6 +877,17 @@ func (r *Reconciler) doUninstall(actionClient helmclient.ActionInterface, u *upd } else { log.Info("Release uninstalled", "name", resp.Release.Name, "version", resp.Release.Version) } + + for _, ext := range r.postExtensions { + if err := ext(ctx, obj, u.UpdateStatusCustom, r.log); err != nil { + u.UpdateStatus( + updater.EnsureCondition(conditions.Irreconcilable(corev1.ConditionTrue, conditions.ReasonReconcileError, err)), + updater.EnsureConditionUnknown(conditions.TypeReleaseFailed), + ) + return err + } + } + u.Update(updater.RemoveFinalizer(uninstallFinalizer)) u.UpdateStatus( updater.EnsureCondition(conditions.ReleaseFailed(corev1.ConditionFalse, "", "")), @@ -736,6 +921,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 } @@ -773,13 +961,13 @@ func (r *Reconciler) setupWatches(mgr ctrl.Manager, c controller.Controller) err return err } - if !r.skipDependentWatches { - r.postHooks = append([]hook.PostHook{internalhook.NewDependentResourceWatcher(c, mgr.GetRESTMapper())}, r.postHooks...) + if !r.skipDependentWatches || len(r.extraDependentWatches) > 0 { + r.postHooks = append([]hook.PostHook{internalhook.NewDependentResourceWatcher(c, mgr.GetRESTMapper(), !r.skipDependentWatches, r.extraDependentWatches...)}, r.postHooks...) } return nil } -func ensureDeployedRelease(u *updater.Updater, rel *release.Release) { +func (r *Reconciler) ensureDeployedRelease(u *updater.Updater, rel *release.Release) { reason := conditions.ReasonInstallSuccessful message := "release was successfully installed" if rel.Version > 1 { @@ -789,6 +977,13 @@ func ensureDeployedRelease(u *updater.Updater, rel *release.Release) { if rel.Info != nil && len(rel.Info.Notes) > 0 { message = rel.Info.Notes } + + if r.stripManifestFromStatus { + relCopy := *rel + relCopy.Manifest = "" + rel = &relCopy + } + u.Update(updater.EnsureFinalizer(uninstallFinalizer)) u.UpdateStatus( updater.EnsureCondition(conditions.Deployed(corev1.ConditionTrue, reason, message)), diff --git a/pkg/reconciler/reconciler_test.go b/pkg/reconciler/reconciler_test.go index 96768e2..f6ea2df 100644 --- a/pkg/reconciler/reconciler_test.go +++ b/pkg/reconciler/reconciler_test.go @@ -154,6 +154,16 @@ var _ = Describe("Reconciler", func() { Expect(r.skipDependentWatches).To(Equal(true)) }) }) + var _ = Describe("StripManifestFromStatus", func() { + It("should set to false", func() { + Expect(StripManifestFromStatus(false)(r)).To(Succeed()) + Expect(r.stripManifestFromStatus).To(Equal(false)) + }) + It("should set to true", func() { + Expect(StripManifestFromStatus(true)(r)).To(Succeed()) + Expect(r.stripManifestFromStatus).To(Equal(true)) + }) + }) var _ = Describe("WithMaxConcurrentReconciles", func() { It("should set the reconciler max concurrent reconciled", func() { Expect(WithMaxConcurrentReconciles(1)(r)).To(Succeed()) @@ -173,6 +183,19 @@ var _ = Describe("Reconciler", func() { Expect(WithReconcilePeriod(-time.Nanosecond)(r)).NotTo(Succeed()) }) }) + var _ = Describe("WithMaxReleaseHistory", func() { + It("should set the max history size", func() { + Expect(WithMaxReleaseHistory(10)(r)).To(Succeed()) + Expect(r.maxHistory).To(Equal(10)) + }) + It("should allow setting the history to unlimited", func() { + Expect(WithMaxReleaseHistory(0)(r)).To(Succeed()) + Expect(r.maxHistory).To(Equal(0)) + }) + It("should fail if value is less than 0", func() { + Expect(WithMaxReleaseHistory(-1)(r)).NotTo(Succeed()) + }) + }) var _ = Describe("WithInstallAnnotations", func() { It("should set multiple reconciler install annotations", func() { a1 := annotation.InstallDisableHooks{CustomName: "my.domain/custom-name1"} @@ -397,7 +420,7 @@ var _ = Describe("Reconciler", func() { }), ) Expect(err).To(BeNil()) - Expect(r.SetupWithManager(mgr)).To(Succeed()) + Expect(r.SetupWithManager(mgr, SetupOpts{})).To(Succeed()) ac, err = r.actionClientGetter.ActionClientFor(obj) Expect(err).To(BeNil()) diff --git a/pkg/values/values.go b/pkg/values/values.go index 46e1941..e1c094f 100644 --- a/pkg/values/values.go +++ b/pkg/values/values.go @@ -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 } @@ -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) +}