From e68897180fbc612ac2a2d9346de27b4b4a46d1d5 Mon Sep 17 00:00:00 2001 From: Joe Lanford Date: Wed, 23 Nov 2022 08:41:10 -0500 Subject: [PATCH] Add various functional options to ActionConfigGetter and ActionClientGetter constructors (#196) --- pkg/client/actionclient.go | 108 +++++++++++++++++-- pkg/client/actionclient_test.go | 181 +++++++++++++++++++++++++++++++- pkg/client/actionconfig.go | 75 ++++++++++--- pkg/client/actionconfig_test.go | 130 +++++++++++++++++++++++ pkg/reconciler/reconciler.go | 5 +- 5 files changed, 472 insertions(+), 27 deletions(-) diff --git a/pkg/client/actionclient.go b/pkg/client/actionclient.go index 4be6b9b9..ce8429aa 100644 --- a/pkg/client/actionclient.go +++ b/pkg/client/actionclient.go @@ -61,13 +61,71 @@ type GetOption func(*action.Get) error type InstallOption func(*action.Install) error type UpgradeOption func(*action.Upgrade) error type UninstallOption func(*action.Uninstall) error +type RollbackOption func(*action.Rollback) error -func NewActionClientGetter(acg ActionConfigGetter) ActionClientGetter { - return &actionClientGetter{acg} +type ActionClientGetterOption func(*actionClientGetter) error + +func AppendGetOptions(opts ...GetOption) ActionClientGetterOption { + return func(getter *actionClientGetter) error { + getter.defaultGetOpts = append(getter.defaultGetOpts, opts...) + return nil + } +} + +func AppendInstallOptions(opts ...InstallOption) ActionClientGetterOption { + return func(getter *actionClientGetter) error { + getter.defaultInstallOpts = append(getter.defaultInstallOpts, opts...) + return nil + } +} + +func AppendUpgradeOptions(opts ...UpgradeOption) ActionClientGetterOption { + return func(getter *actionClientGetter) error { + getter.defaultUpgradeOpts = append(getter.defaultUpgradeOpts, opts...) + return nil + } +} + +func AppendUninstallOptions(opts ...UninstallOption) ActionClientGetterOption { + return func(getter *actionClientGetter) error { + getter.defaultUninstallOpts = append(getter.defaultUninstallOpts, opts...) + return nil + } +} + +func AppendInstallFailureUninstallOptions(opts ...UninstallOption) ActionClientGetterOption { + return func(getter *actionClientGetter) error { + getter.installFailureUninstallOpts = append(getter.installFailureUninstallOpts, opts...) + return nil + } +} +func AppendUpgradeFailureRollbackOptions(opts ...RollbackOption) ActionClientGetterOption { + return func(getter *actionClientGetter) error { + getter.upgradeFailureRollbackOpts = append(getter.upgradeFailureRollbackOpts, opts...) + return nil + } +} + +func NewActionClientGetter(acg ActionConfigGetter, opts ...ActionClientGetterOption) (ActionClientGetter, error) { + actionClientGetter := &actionClientGetter{acg: acg} + for _, opt := range opts { + if err := opt(actionClientGetter); err != nil { + return nil, err + } + } + return actionClientGetter, nil } type actionClientGetter struct { acg ActionConfigGetter + + defaultGetOpts []GetOption + defaultInstallOpts []InstallOption + defaultUpgradeOpts []UpgradeOption + defaultUninstallOpts []UninstallOption + + installFailureUninstallOpts []UninstallOption + upgradeFailureRollbackOpts []RollbackOption } var _ ActionClientGetter = &actionClientGetter{} @@ -83,9 +141,18 @@ func (hcg *actionClientGetter) ActionClientFor(obj client.Object) (ActionInterfa } postRenderer := DefaultPostRendererFunc(rm, actionConfig.KubeClient, obj) return &actionClient{ - conf: actionConfig, - defaultInstallOpts: []InstallOption{WithInstallPostRenderer(postRenderer)}, - defaultUpgradeOpts: []UpgradeOption{WithUpgradePostRenderer(postRenderer)}, + conf: actionConfig, + + // For the install and upgrade options, we put the post renderer first in the list + // on purpose because we want user-provided defaults to be able to override the + // post-renderer that we automatically configure for the client. + defaultGetOpts: hcg.defaultGetOpts, + defaultInstallOpts: append([]InstallOption{WithInstallPostRenderer(postRenderer)}, hcg.defaultInstallOpts...), + defaultUpgradeOpts: append([]UpgradeOption{WithUpgradePostRenderer(postRenderer)}, hcg.defaultUpgradeOpts...), + defaultUninstallOpts: hcg.defaultUninstallOpts, + + installFailureUninstallOpts: hcg.installFailureUninstallOpts, + upgradeFailureRollbackOpts: hcg.upgradeFailureRollbackOpts, }, nil } @@ -96,6 +163,9 @@ type actionClient struct { defaultInstallOpts []InstallOption defaultUpgradeOpts []UpgradeOption defaultUninstallOpts []UninstallOption + + installFailureUninstallOpts []UninstallOption + upgradeFailureRollbackOpts []RollbackOption } var _ ActionInterface = &actionClient{} @@ -137,7 +207,7 @@ func (c *actionClient) Install(name, namespace string, chrt *chart.Chart, vals m // // Only return an error about a rollback failure if the failure was // caused by something other than the release not being found. - _, uninstallErr := c.Uninstall(name) + _, uninstallErr := c.uninstall(name, c.installFailureUninstallOpts...) if uninstallErr != nil && !errors.Is(uninstallErr, driver.ErrReleaseNotFound) { return nil, fmt.Errorf("uninstall failed: %v: original install error: %w", uninstallErr, err) } @@ -158,16 +228,18 @@ func (c *actionClient) Upgrade(name, namespace string, chrt *chart.Chart, vals m rel, err := upgrade.Run(name, chrt, vals) if err != nil { if rel != nil { - rollback := action.NewRollback(c.conf) - rollback.Force = true - rollback.MaxHistory = upgrade.MaxHistory + rollbackOpts := append([]RollbackOption{func(rollback *action.Rollback) error { + rollback.Force = true + rollback.MaxHistory = upgrade.MaxHistory + return nil + }}, c.upgradeFailureRollbackOpts...) // As of Helm 2.13, if Upgrade returns a non-nil release, that // means the release was also recorded in the release store. // Therefore, we should perform the rollback when we have a non-nil // release. Any rollback error here would be unexpected, so always // log both the update and rollback errors. - rollbackErr := rollback.Run(name) + rollbackErr := c.rollback(name, rollbackOpts...) if rollbackErr != nil { return nil, fmt.Errorf("rollback failed: %v: original upgrade error: %w", rollbackErr, err) } @@ -177,9 +249,23 @@ func (c *actionClient) Upgrade(name, namespace string, chrt *chart.Chart, vals m return rel, nil } +func (c *actionClient) rollback(name string, opts ...RollbackOption) error { + rollback := action.NewRollback(c.conf) + for _, o := range opts { + if err := o(rollback); err != nil { + return err + } + } + return rollback.Run(name) +} + func (c *actionClient) Uninstall(name string, opts ...UninstallOption) (*release.UninstallReleaseResponse, error) { + return c.uninstall(name, concat(c.defaultUninstallOpts, opts...)...) +} + +func (c *actionClient) uninstall(name string, opts ...UninstallOption) (*release.UninstallReleaseResponse, error) { uninstall := action.NewUninstall(c.conf) - for _, o := range concat(c.defaultUninstallOpts, opts...) { + for _, o := range opts { if err := o(uninstall); err != nil { return nil, err } diff --git a/pkg/client/actionclient_test.go b/pkg/client/actionclient_test.go index dc7b18cd..9f405d7f 100644 --- a/pkg/client/actionclient_test.go +++ b/pkg/client/actionclient_test.go @@ -20,6 +20,7 @@ import ( "context" "errors" "strconv" + "time" "github.com/go-logr/logr" . "github.com/onsi/ginkgo/v2" @@ -37,6 +38,7 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime/schema" apitypes "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/rand" "k8s.io/cli-runtime/pkg/resource" "k8s.io/utils/pointer" "sigs.k8s.io/controller-runtime/pkg/client" @@ -61,7 +63,178 @@ var _ = Describe("ActionClient", func() { It("should return a valid ActionConfigGetter", func() { actionConfigGetter, err := NewActionConfigGetter(cfg, rm, logr.Discard()) Expect(err).ShouldNot(HaveOccurred()) - Expect(NewActionClientGetter(actionConfigGetter)).NotTo(BeNil()) + acg, err := NewActionClientGetter(actionConfigGetter) + Expect(err).To(BeNil()) + Expect(acg).NotTo(BeNil()) + }) + + When("options are specified", func() { + expectErr := errors.New("expect this error") + + var ( + actionConfigGetter ActionConfigGetter + obj client.Object + ) + BeforeEach(func() { + var err error + actionConfigGetter, err = NewActionConfigGetter(cfg, rm, logr.Discard()) + Expect(err).ShouldNot(HaveOccurred()) + obj = testutil.BuildTestCR(gvk) + }) + + It("should get clients with custom get options", func() { + expectVersion := rand.Int() + acg, err := NewActionClientGetter(actionConfigGetter, AppendGetOptions( + func(get *action.Get) error { + get.Version = expectVersion + return nil + }, + func(get *action.Get) error { + Expect(get.Version).To(Equal(expectVersion)) + return expectErr + }, + )) + Expect(err).To(BeNil()) + Expect(acg).NotTo(BeNil()) + + ac, err := acg.ActionClientFor(obj) + Expect(err).To(BeNil()) + Expect(ac).NotTo(BeNil()) + + _, err = ac.Get(obj.GetName()) + Expect(err).To(MatchError(expectErr)) + }) + It("should get clients with custom install options", func() { + acg, err := NewActionClientGetter(actionConfigGetter, AppendInstallOptions( + func(install *action.Install) error { + install.Description = mockTestDesc + return nil + }, + func(install *action.Install) error { + Expect(install.Description).To(Equal(mockTestDesc)) + return expectErr + }, + )) + Expect(err).To(BeNil()) + Expect(acg).NotTo(BeNil()) + + ac, err := acg.ActionClientFor(obj) + Expect(err).To(BeNil()) + Expect(ac).NotTo(BeNil()) + + _, err = ac.Install(obj.GetName(), obj.GetNamespace(), &chrt, chartutil.Values{}) + Expect(err).To(MatchError(expectErr)) + }) + It("should get clients with custom upgrade options", func() { + acg, err := NewActionClientGetter(actionConfigGetter, AppendUpgradeOptions( + func(upgrade *action.Upgrade) error { + upgrade.Description = mockTestDesc + return nil + }, + func(upgrade *action.Upgrade) error { + Expect(upgrade.Description).To(Equal(mockTestDesc)) + return expectErr + }, + )) + Expect(err).To(BeNil()) + Expect(acg).NotTo(BeNil()) + + ac, err := acg.ActionClientFor(obj) + Expect(err).To(BeNil()) + Expect(ac).NotTo(BeNil()) + + _, err = ac.Upgrade(obj.GetName(), obj.GetNamespace(), &chrt, chartutil.Values{}) + Expect(err).To(MatchError(expectErr)) + }) + It("should get clients with custom uninstall options", func() { + acg, err := NewActionClientGetter(actionConfigGetter, AppendUninstallOptions( + func(uninstall *action.Uninstall) error { + uninstall.Description = mockTestDesc + return nil + }, + func(uninstall *action.Uninstall) error { + Expect(uninstall.Description).To(Equal(mockTestDesc)) + return expectErr + }, + )) + Expect(err).To(BeNil()) + Expect(acg).NotTo(BeNil()) + + ac, err := acg.ActionClientFor(obj) + Expect(err).To(BeNil()) + Expect(ac).NotTo(BeNil()) + + _, err = ac.Uninstall(obj.GetName()) + Expect(err).To(MatchError(expectErr)) + }) + It("should get clients with custom install failure uninstall options", func() { + acg, err := NewActionClientGetter(actionConfigGetter, AppendInstallFailureUninstallOptions( + func(uninstall *action.Uninstall) error { + uninstall.Description = mockTestDesc + return nil + }, + func(uninstall *action.Uninstall) error { + Expect(uninstall.Description).To(Equal(mockTestDesc)) + return expectErr + }, + )) + Expect(err).To(BeNil()) + Expect(acg).NotTo(BeNil()) + + ac, err := acg.ActionClientFor(obj) + Expect(err).To(BeNil()) + Expect(ac).NotTo(BeNil()) + + _, err = ac.Install(obj.GetName(), obj.GetNamespace(), &chrt, chartutil.Values{}, func(install *action.Install) error { + // Force the installatiom to fail by using an impossibly short wait. + // When the installation fails, the failure uninstall logic is attempted. + install.Wait = true + install.Timeout = time.Nanosecond * 1 + return nil + }) + Expect(err).To(MatchError(ContainSubstring(expectErr.Error()))) + + // Uninstall the chart to cleanup for other tests. + _, err = ac.Uninstall(obj.GetName()) + Expect(err).To(BeNil()) + }) + It("should get clients with custom upgrade failure rollback options", func() { + expectMaxHistory := rand.Int() + acg, err := NewActionClientGetter(actionConfigGetter, AppendUpgradeFailureRollbackOptions( + func(rollback *action.Rollback) error { + rollback.MaxHistory = expectMaxHistory + return nil + }, + func(rollback *action.Rollback) error { + Expect(rollback.MaxHistory).To(Equal(expectMaxHistory)) + return expectErr + }, + )) + Expect(err).To(BeNil()) + Expect(acg).NotTo(BeNil()) + + ac, err := acg.ActionClientFor(obj) + Expect(err).To(BeNil()) + Expect(ac).NotTo(BeNil()) + + // Install the chart so that we can try an upgrade. + rel, err := ac.Install(obj.GetName(), obj.GetNamespace(), &chrt, chartutil.Values{}) + Expect(err).To(BeNil()) + Expect(rel).NotTo(BeNil()) + + _, err = ac.Upgrade(obj.GetName(), obj.GetNamespace(), &chrt, chartutil.Values{}, func(upgrade *action.Upgrade) error { + // Force the upgrade to fail by using an impossibly short wait. + // When the upgrade fails, the rollback logic is attempted. + upgrade.Wait = true + upgrade.Timeout = time.Nanosecond * 1 + return nil + }) + Expect(err).To(MatchError(ContainSubstring(expectErr.Error()))) + + // Uninstall the chart to cleanup for other tests. + _, err = ac.Uninstall(obj.GetName()) + Expect(err).To(BeNil()) + }) }) }) @@ -88,7 +261,8 @@ var _ = Describe("ActionClient", func() { It("should return a valid ActionClient", func() { actionConfGetter, err := NewActionConfigGetter(cfg, rm, logr.Discard()) Expect(err).ShouldNot(HaveOccurred()) - acg := NewActionClientGetter(actionConfGetter) + acg, err := NewActionClientGetter(actionConfGetter) + Expect(err).To(BeNil()) ac, err := acg.ActionClientFor(obj) Expect(err).To(BeNil()) Expect(ac).NotTo(BeNil()) @@ -107,7 +281,8 @@ var _ = Describe("ActionClient", func() { actionConfigGetter, err := NewActionConfigGetter(cfg, rm, logr.Discard()) Expect(err).ShouldNot(HaveOccurred()) - acg := NewActionClientGetter(actionConfigGetter) + acg, err := NewActionClientGetter(actionConfigGetter) + Expect(err).To(BeNil()) ac, err = acg.ActionClientFor(obj) Expect(err).To(BeNil()) diff --git a/pkg/client/actionconfig.go b/pkg/client/actionconfig.go index 21b89d02..51ae0f16 100644 --- a/pkg/client/actionconfig.go +++ b/pkg/client/actionconfig.go @@ -20,8 +20,6 @@ import ( "context" "fmt" - "k8s.io/client-go/kubernetes" - "github.com/go-logr/logr" "helm.sh/helm/v3/pkg/action" "helm.sh/helm/v3/pkg/kube" @@ -30,6 +28,7 @@ import ( corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes" v1 "k8s.io/client-go/kubernetes/typed/core/v1" "k8s.io/client-go/rest" "sigs.k8s.io/controller-runtime/pkg/client" @@ -39,7 +38,7 @@ type ActionConfigGetter interface { ActionConfigFor(obj client.Object) (*action.Configuration, error) } -func NewActionConfigGetter(cfg *rest.Config, rm meta.RESTMapper, log logr.Logger) (ActionConfigGetter, error) { +func NewActionConfigGetter(cfg *rest.Config, rm meta.RESTMapper, log logr.Logger, opts ...ActionConfigGetterOption) (ActionConfigGetter, error) { rcg := newRESTClientGetter(cfg, rm, "") // Setup the debug log function that Helm will use debugLog := func(format string, v ...interface{}) { @@ -56,29 +55,78 @@ func NewActionConfigGetter(cfg *rest.Config, rm meta.RESTMapper, log logr.Logger return nil, fmt.Errorf("creating kubernetes client set: %w", err) } - return &actionConfigGetter{ + acg := &actionConfigGetter{ kubeClient: kc, kubeClientSet: kcs, debugLog: debugLog, restClientGetter: rcg.restClientGetter, - }, nil + } + for _, o := range opts { + o(acg) + } + if acg.objectToClientNamespace == nil { + acg.objectToClientNamespace = getObjectNamespace + } + if acg.objectToStorageNamespace == nil { + acg.objectToStorageNamespace = getObjectNamespace + } + return acg, nil } var _ ActionConfigGetter = &actionConfigGetter{} +type ActionConfigGetterOption func(getter *actionConfigGetter) + +type ObjectToStringMapper func(client.Object) (string, error) + +func ClientNamespaceMapper(m ObjectToStringMapper) ActionConfigGetterOption { + return func(getter *actionConfigGetter) { + getter.objectToClientNamespace = m + } +} + +func StorageNamespaceMapper(m ObjectToStringMapper) ActionConfigGetterOption { + return func(getter *actionConfigGetter) { + getter.objectToStorageNamespace = m + } +} + +func DisableStorageOwnerRefInjection(v bool) ActionConfigGetterOption { + return func(getter *actionConfigGetter) { + getter.disableStorageOwnerRefInjection = v + } +} + +func getObjectNamespace(obj client.Object) (string, error) { + return obj.GetNamespace(), nil +} + type actionConfigGetter struct { kubeClient *kube.Client kubeClientSet kubernetes.Interface debugLog func(string, ...interface{}) restClientGetter *restClientGetter + + objectToClientNamespace ObjectToStringMapper + objectToStorageNamespace ObjectToStringMapper + disableStorageOwnerRefInjection bool } func (acg *actionConfigGetter) ActionConfigFor(obj client.Object) (*action.Configuration, error) { - ownerRef := metav1.NewControllerRef(obj, obj.GetObjectKind().GroupVersionKind()) - d := driver.NewSecrets(&ownerRefSecretClient{ - SecretInterface: acg.kubeClientSet.CoreV1().Secrets(obj.GetNamespace()), - refs: []metav1.OwnerReference{*ownerRef}, - }) + storageNs, err := acg.objectToStorageNamespace(obj) + if err != nil { + return nil, fmt.Errorf("get storage namespace from object: %v", err) + } + + secretClient := acg.kubeClientSet.CoreV1().Secrets(storageNs) + if !acg.disableStorageOwnerRefInjection { + ownerRef := metav1.NewControllerRef(obj, obj.GetObjectKind().GroupVersionKind()) + secretClient = &ownerRefSecretClient{ + SecretInterface: secretClient, + refs: []metav1.OwnerReference{*ownerRef}, + } + } + d := driver.NewSecrets(secretClient) // Also, use the debug log for the storage driver d.Log = acg.debugLog @@ -87,10 +135,13 @@ func (acg *actionConfigGetter) ActionConfigFor(obj client.Object) (*action.Confi s := storage.Init(d) kubeClient := *acg.kubeClient - kubeClient.Namespace = obj.GetNamespace() + kubeClient.Namespace, err = acg.objectToClientNamespace(obj) + if err != nil { + return nil, fmt.Errorf("get client namespace from object: %v", err) + } return &action.Configuration{ - RESTClientGetter: acg.restClientGetter.ForNamespace(obj.GetNamespace()), + RESTClientGetter: acg.restClientGetter.ForNamespace(kubeClient.Namespace), Releases: s, KubeClient: &kubeClient, Log: acg.debugLog, diff --git a/pkg/client/actionconfig_test.go b/pkg/client/actionconfig_test.go index ad00ea55..6af6d87d 100644 --- a/pkg/client/actionconfig_test.go +++ b/pkg/client/actionconfig_test.go @@ -17,9 +17,22 @@ limitations under the License. package client import ( + "bytes" + "context" + "fmt" + "github.com/go-logr/logr" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "helm.sh/helm/v3/pkg/action" + "helm.sh/helm/v3/pkg/kube" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/rand" + "k8s.io/cli-runtime/pkg/resource" + clientgoscheme "k8s.io/client-go/kubernetes/scheme" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/apiutil" @@ -28,11 +41,128 @@ import ( var _ = Describe("ActionConfig", func() { var _ = Describe("NewActionConfigGetter", func() { + var rm meta.RESTMapper + + BeforeEach(func() { + var err error + rm, err = apiutil.NewDiscoveryRESTMapper(cfg) + Expect(err).To(BeNil()) + }) + It("should return a valid ActionConfigGetter", func() { acg, err := NewActionConfigGetter(cfg, nil, logr.Discard()) Expect(err).ShouldNot(HaveOccurred()) Expect(acg).NotTo(BeNil()) }) + + When("passing options", func() { + var ( + obj client.Object + cl client.Client + ) + + BeforeEach(func() { + obj = testutil.BuildTestCR(gvk) + + var err error + cl, err = client.New(cfg, client.Options{Scheme: clientgoscheme.Scheme}) + Expect(err).To(BeNil()) + }) + + It("should use a custom client namespace", func() { + clientNs := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: fmt.Sprintf("client-%s", rand.String(8))}} + clientNsMapper := func(_ client.Object) (string, error) { return clientNs.Name, nil } + acg, err := NewActionConfigGetter(cfg, rm, logr.Discard(), + ClientNamespaceMapper(clientNsMapper), + ) + Expect(err).To(BeNil()) + ac, err := acg.ActionConfigFor(obj) + Expect(err).To(BeNil()) + Expect(ac.KubeClient.(*kube.Client).Namespace).To(Equal(clientNs.Name)) + Expect(ac.RESTClientGetter.(*namespacedRCG).namespaceConfig.Namespace()).To(Equal(clientNs.Name)) + resources, err := ac.KubeClient.Build(bytes.NewBufferString(`--- +apiVersion: v1 +kind: ServiceAccount +metadata: + name: sa`), false) + Expect(err).To(BeNil()) + Expect(resources.Visit(func(info *resource.Info, err error) error { + Expect(err).To(BeNil()) + Expect(info.Namespace).To(Equal(clientNs.Name)) + return nil + })).To(Succeed()) + }) + + It("should use a custom storage namespace", func() { + storageNs := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: fmt.Sprintf("storage-%s", rand.String(8))}} + storageNsMapper := func(_ client.Object) (string, error) { return storageNs.Name, nil } + acg, err := NewActionConfigGetter(cfg, rm, logr.Discard(), + StorageNamespaceMapper(storageNsMapper), + ) + Expect(err).To(BeNil()) + + ac, err := acg.ActionConfigFor(obj) + Expect(err).To(BeNil()) + + By("Creating the storage namespace") + Expect(cl.Create(context.Background(), storageNs)).To(Succeed()) + + By("Installing a release") + i := action.NewInstall(ac) + i.ReleaseName = fmt.Sprintf("release-name-%s", rand.String(8)) + i.Namespace = obj.GetNamespace() + rel, err := i.Run(&chrt, nil) + Expect(err).To(BeNil()) + Expect(rel.Namespace).To(Equal(obj.GetNamespace())) + + By("Verifying the release secret is created in the storage namespace") + secretKey := types.NamespacedName{ + Namespace: storageNs.Name, + Name: fmt.Sprintf("sh.helm.release.v1.%s.v1", i.ReleaseName), + } + secret := &corev1.Secret{} + Expect(cl.Get(context.Background(), secretKey, secret)).To(Succeed()) + Expect(secret.OwnerReferences).To(HaveLen(1)) + + By("Uninstalling the release") + _, err = action.NewUninstall(ac).Run(i.ReleaseName) + Expect(err).To(BeNil()) + + By("Deleting the storage namespace") + Expect(cl.Delete(context.Background(), storageNs)).To(Succeed()) + }) + + It("should disable storage owner ref injection", func() { + acg, err := NewActionConfigGetter(cfg, rm, logr.Discard(), + DisableStorageOwnerRefInjection(true), + ) + Expect(err).To(BeNil()) + + ac, err := acg.ActionConfigFor(obj) + Expect(err).To(BeNil()) + + By("Installing a release") + i := action.NewInstall(ac) + i.ReleaseName = fmt.Sprintf("release-name-%s", rand.String(8)) + i.Namespace = obj.GetNamespace() + rel, err := i.Run(&chrt, nil) + Expect(err).To(BeNil()) + Expect(rel.Namespace).To(Equal(obj.GetNamespace())) + + By("Verifying the release secret has no owner references") + secretKey := types.NamespacedName{ + Namespace: obj.GetNamespace(), + Name: fmt.Sprintf("sh.helm.release.v1.%s.v1", i.ReleaseName), + } + secret := &corev1.Secret{} + Expect(cl.Get(context.Background(), secretKey, secret)).To(Succeed()) + Expect(secret.OwnerReferences).To(HaveLen(0)) + + By("Uninstalling the release") + _, err = action.NewUninstall(ac).Run(i.ReleaseName) + Expect(err).To(BeNil()) + }) + }) }) var _ = Describe("GetActionConfig", func() { diff --git a/pkg/reconciler/reconciler.go b/pkg/reconciler/reconciler.go index 31b7edfe..1debb28d 100644 --- a/pkg/reconciler/reconciler.go +++ b/pkg/reconciler/reconciler.go @@ -881,7 +881,10 @@ func (r *Reconciler) addDefaults(mgr ctrl.Manager, controllerName string) error if err != nil { return fmt.Errorf("creating action config getter: %w", err) } - r.actionClientGetter = helmclient.NewActionClientGetter(actionConfigGetter) + r.actionClientGetter, err = helmclient.NewActionClientGetter(actionConfigGetter) + if err != nil { + return fmt.Errorf("creating action client getter: %v", err) + } } if r.eventRecorder == nil { r.eventRecorder = mgr.GetEventRecorderFor(controllerName)