diff --git a/pkg/client/actionclient_test.go b/pkg/client/actionclient_test.go index f1dd3a2f..c0fe448c 100644 --- a/pkg/client/actionclient_test.go +++ b/pkg/client/actionclient_test.go @@ -59,7 +59,8 @@ const mockTestDesc = "Test Description" var _ = Describe("ActionClient", func() { var ( - rm meta.RESTMapper + rm meta.RESTMapper + sch *runtime.Scheme ) BeforeEach(func() { var err error @@ -69,10 +70,12 @@ var _ = Describe("ActionClient", func() { rm, err = apiutil.NewDynamicRESTMapper(cfg, httpClient) Expect(err).ToNot(HaveOccurred()) + + sch = runtime.NewScheme() }) var _ = Describe("NewActionClientGetter", func() { It("should return a valid ActionConfigGetter", func() { - actionConfigGetter, err := NewActionConfigGetter(cfg, rm) + actionConfigGetter, err := NewActionConfigGetter(cfg, rm, sch) Expect(err).ShouldNot(HaveOccurred()) acg, err := NewActionClientGetter(actionConfigGetter) Expect(err).ToNot(HaveOccurred()) @@ -89,7 +92,7 @@ var _ = Describe("ActionClient", func() { ) BeforeEach(func() { var err error - actionConfigGetter, err = NewActionConfigGetter(cfg, rm) + actionConfigGetter, err = NewActionConfigGetter(cfg, rm, sch) Expect(err).ShouldNot(HaveOccurred()) dc, err := discovery.NewDiscoveryClientForConfig(cfg) Expect(err).ShouldNot(HaveOccurred()) @@ -310,7 +313,7 @@ var _ = Describe("ActionClient", func() { obj = testutil.BuildTestCR(gvk) }) It("should return a valid ActionClient", func() { - actionConfGetter, err := NewActionConfigGetter(cfg, rm) + actionConfGetter, err := NewActionConfigGetter(cfg, rm, sch) Expect(err).ShouldNot(HaveOccurred()) acg, err := NewActionClientGetter(actionConfGetter) Expect(err).ToNot(HaveOccurred()) @@ -330,7 +333,7 @@ var _ = Describe("ActionClient", func() { BeforeEach(func() { obj = testutil.BuildTestCR(gvk) - actionConfigGetter, err := NewActionConfigGetter(cfg, rm) + actionConfigGetter, err := NewActionConfigGetter(cfg, rm, sch) Expect(err).ShouldNot(HaveOccurred()) acg, err := NewActionClientGetter(actionConfigGetter) Expect(err).ToNot(HaveOccurred()) diff --git a/pkg/client/actionconfig.go b/pkg/client/actionconfig.go index 120febf1..888ba351 100644 --- a/pkg/client/actionconfig.go +++ b/pkg/client/actionconfig.go @@ -28,18 +28,22 @@ import ( corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/discovery" "k8s.io/client-go/discovery/cached/memory" v1 "k8s.io/client-go/kubernetes/typed/core/v1" "k8s.io/client-go/rest" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/apiutil" ) +const StorageNamespaceKey = "helm-secret-namespace" + type ActionConfigGetter interface { ActionConfigFor(ctx context.Context, obj client.Object) (*action.Configuration, error) } -func NewActionConfigGetter(baseRestConfig *rest.Config, rm meta.RESTMapper, opts ...ActionConfigGetterOption) (ActionConfigGetter, error) { +func NewActionConfigGetter(baseRestConfig *rest.Config, rm meta.RESTMapper, scheme *runtime.Scheme, opts ...ActionConfigGetterOption) (ActionConfigGetter, error) { dc, err := discovery.NewDiscoveryClientForConfig(baseRestConfig) if err != nil { return nil, fmt.Errorf("create discovery client: %v", err) @@ -50,6 +54,7 @@ func NewActionConfigGetter(baseRestConfig *rest.Config, rm meta.RESTMapper, opts baseRestConfig: baseRestConfig, restMapper: rm, discoveryClient: cdc, + scheme: scheme, } for _, o := range opts { o(acg) @@ -99,12 +104,14 @@ func RestConfigMapper(f func(context.Context, client.Object, *rest.Config) (*res } func getObjectNamespace(obj client.Object) (string, error) { + return obj.GetNamespace(), nil } type actionConfigGetter struct { baseRestConfig *rest.Config restMapper meta.RESTMapper + scheme *runtime.Scheme discoveryClient discovery.CachedDiscoveryInterface objectToClientNamespace ObjectToStringMapper @@ -119,6 +126,19 @@ func (acg *actionConfigGetter) ActionConfigFor(ctx context.Context, obj client.O return nil, fmt.Errorf("get storage namespace for object: %v", err) } + isNamespaced, err := apiutil.IsObjectNamespaced(obj, acg.scheme, acg.restMapper) + if err != nil { + return nil, fmt.Errorf("failed to determine scope of the object: %v", err) + } + + if storageNs == "" || !isNamespaced { + ns, ok := ctx.Value(StorageNamespaceKey).(string) + if !ok || ns == "" { + return nil, fmt.Errorf("empty namespace for creating helm secret") + } + storageNs = ns + } + restConfig, err := acg.objectToRestConfig(ctx, obj, acg.baseRestConfig) if err != nil { return nil, fmt.Errorf("get rest config for object: %v", err) diff --git a/pkg/client/actionconfig_test.go b/pkg/client/actionconfig_test.go index 6a3ee470..490d888e 100644 --- a/pkg/client/actionconfig_test.go +++ b/pkg/client/actionconfig_test.go @@ -29,6 +29,7 @@ import ( "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" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/rand" "k8s.io/cli-runtime/pkg/resource" @@ -42,8 +43,10 @@ import ( var _ = Describe("ActionConfig", func() { var _ = Describe("NewActionConfigGetter", func() { - var rm meta.RESTMapper - + var ( + rm meta.RESTMapper + sch *runtime.Scheme + ) BeforeEach(func() { var err error @@ -52,10 +55,12 @@ var _ = Describe("ActionConfig", func() { rm, err = apiutil.NewDynamicRESTMapper(cfg, httpClient) Expect(err).ToNot(HaveOccurred()) + + sch = runtime.NewScheme() }) It("should return a valid ActionConfigGetter", func() { - acg, err := NewActionConfigGetter(cfg, nil) + acg, err := NewActionConfigGetter(cfg, nil, nil) Expect(err).ShouldNot(HaveOccurred()) Expect(acg).NotTo(BeNil()) }) @@ -77,7 +82,7 @@ var _ = Describe("ActionConfig", func() { 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, ClientNamespaceMapper(clientNsMapper)) + acg, err := NewActionConfigGetter(cfg, rm, sch, ClientNamespaceMapper(clientNsMapper)) Expect(err).ToNot(HaveOccurred()) ac, err := acg.ActionConfigFor(context.Background(), obj) Expect(err).ToNot(HaveOccurred()) @@ -99,7 +104,7 @@ metadata: 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, StorageNamespaceMapper(storageNsMapper)) + acg, err := NewActionConfigGetter(cfg, rm, sch, StorageNamespaceMapper(storageNsMapper)) Expect(err).ToNot(HaveOccurred()) ac, err := acg.ActionConfigFor(context.Background(), obj) @@ -134,7 +139,7 @@ metadata: }) It("should disable storage owner ref injection", func() { - acg, err := NewActionConfigGetter(cfg, rm, DisableStorageOwnerRefInjection(true)) + acg, err := NewActionConfigGetter(cfg, rm, sch, DisableStorageOwnerRefInjection(true)) Expect(err).ToNot(HaveOccurred()) ac, err := acg.ActionConfigFor(context.Background(), obj) @@ -168,7 +173,7 @@ metadata: BearerToken: obj.GetName(), }, nil } - acg, err := NewActionConfigGetter(cfg, rm, RestConfigMapper(restConfigMapper)) + acg, err := NewActionConfigGetter(cfg, rm, sch, RestConfigMapper(restConfigMapper)) Expect(err).ToNot(HaveOccurred()) testObject := func(name string) client.Object { @@ -199,8 +204,9 @@ metadata: rm, err := apiutil.NewDynamicRESTMapper(cfg, httpClient) Expect(err).ToNot(HaveOccurred()) + sch := runtime.NewScheme() - acg, err := NewActionConfigGetter(cfg, rm) + acg, err := NewActionConfigGetter(cfg, rm, sch) Expect(err).ShouldNot(HaveOccurred()) ac, err := acg.ActionConfigFor(context.Background(), obj) Expect(err).ToNot(HaveOccurred()) diff --git a/pkg/reconciler/reconciler.go b/pkg/reconciler/reconciler.go index 272c8c75..e5439117 100644 --- a/pkg/reconciler/reconciler.go +++ b/pkg/reconciler/reconciler.go @@ -917,7 +917,7 @@ func (r *Reconciler) addDefaults(mgr ctrl.Manager, controllerName string) error r.log = ctrl.Log.WithName("controllers").WithName("Helm") } if r.actionClientGetter == nil { - actionConfigGetter, err := helmclient.NewActionConfigGetter(mgr.GetConfig(), mgr.GetRESTMapper()) + actionConfigGetter, err := helmclient.NewActionConfigGetter(mgr.GetConfig(), mgr.GetRESTMapper(), mgr.GetScheme()) if err != nil { return fmt.Errorf("creating action config getter: %w", err) } diff --git a/pkg/reconciler/reconciler_test.go b/pkg/reconciler/reconciler_test.go index 4a5e9455..e6a020d6 100644 --- a/pkg/reconciler/reconciler_test.go +++ b/pkg/reconciler/reconciler_test.go @@ -977,7 +977,7 @@ var _ = Describe("Reconciler", func() { var actionConf *action.Configuration BeforeEach(func() { By("getting the current release and config", func() { - acg, err := helmclient.NewActionConfigGetter(mgr.GetConfig(), mgr.GetRESTMapper()) + acg, err := helmclient.NewActionConfigGetter(mgr.GetConfig(), mgr.GetRESTMapper(), mgr.GetScheme()) Expect(err).ShouldNot(HaveOccurred()) actionConf, err = acg.ActionConfigFor(ctx, obj) Expect(err).ToNot(HaveOccurred())