From 9e80b0f91307229cc5683b2e99495fc1ccc6f703 Mon Sep 17 00:00:00 2001 From: Todd Treece <360020+toddtreece@users.noreply.github.com> Date: Fri, 21 Feb 2025 11:07:13 -0500 Subject: [PATCH] K8s: Add error to GetRestConfig (#101147) K8s: Add error to RestConfigProvider return values --- pkg/registry/apps/apps.go | 10 ++-- .../apiserver/builder/runner/runner.go | 8 +-- pkg/services/apiserver/client/client.go | 57 +++++++++---------- pkg/services/apiserver/client/client_mock.go | 4 +- pkg/services/apiserver/service.go | 10 ++-- pkg/services/authz/rbac.go | 4 +- pkg/services/authz/rbac/store/folder_store.go | 10 +++- .../folderimpl/folder_unifiedstorage_test.go | 4 +- 8 files changed, 55 insertions(+), 52 deletions(-) diff --git a/pkg/registry/apps/apps.go b/pkg/registry/apps/apps.go index 6a59bccfaee..40e6d8c830b 100644 --- a/pkg/registry/apps/apps.go +++ b/pkg/registry/apps/apps.go @@ -34,13 +34,13 @@ func ProvideRegistryServiceSink( investigationAppProvider *investigations.InvestigationsAppProvider, advisorAppProvider *advisor.AdvisorAppProvider, ) (*Service, error) { - cfgWrapper := func(ctx context.Context) *rest.Config { - cfg := restConfigProvider.GetRestConfig(ctx) - if cfg == nil { - return nil + cfgWrapper := func(ctx context.Context) (*rest.Config, error) { + cfg, err := restConfigProvider.GetRestConfig(ctx) + if err != nil { + return nil, err } cfg.APIPath = "/apis" - return cfg + return cfg, nil } cfg := runner.RunnerConfig{ diff --git a/pkg/services/apiserver/builder/runner/runner.go b/pkg/services/apiserver/builder/runner/runner.go index 617429998a0..d7eb5f4e83d 100644 --- a/pkg/services/apiserver/builder/runner/runner.go +++ b/pkg/services/apiserver/builder/runner/runner.go @@ -12,7 +12,7 @@ import ( ) type RunnerConfig struct { - RestConfigGetter func(context.Context) *rest.Config + RestConfigGetter func(context.Context) (*rest.Config, error) APIRegistrar builder.APIRegistrar } @@ -49,9 +49,9 @@ func (r *APIGroupRunner) Run(ctx context.Context) error { func (r *APIGroupRunner) Init(ctx context.Context) error { defer close(r.initialized) - restConfig := r.config.RestConfigGetter(ctx) - if restConfig == nil { - return fmt.Errorf("rest config is nil") + restConfig, err := r.config.RestConfigGetter(ctx) + if err != nil { + return err } for i := range r.groups { appConfig := app.Config{ diff --git a/pkg/services/apiserver/client/client.go b/pkg/services/apiserver/client/client.go index 7987d09be8c..021dc5d53eb 100644 --- a/pkg/services/apiserver/client/client.go +++ b/pkg/services/apiserver/client/client.go @@ -11,11 +11,10 @@ import ( v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/client-go/dynamic" - "k8s.io/client-go/rest" - k8sUser "k8s.io/apiserver/pkg/authentication/user" k8sRequest "k8s.io/apiserver/pkg/endpoints/request" + "k8s.io/client-go/dynamic" + "k8s.io/client-go/rest" "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/infra/log" @@ -46,13 +45,13 @@ var _ K8sHandler = (*k8sHandler)(nil) type k8sHandler struct { namespacer request.NamespaceMapper gvr schema.GroupVersionResource - restConfig func(context.Context) *rest.Config + restConfig func(context.Context) (*rest.Config, error) searcher resource.ResourceIndexClient userService user.Service } func NewK8sHandler(dual dualwrite.Service, namespacer request.NamespaceMapper, gvr schema.GroupVersionResource, - restConfig func(context.Context) *rest.Config, dashStore dashboards.Store, userSvc user.Service, resourceClient resource.ResourceClient, sorter sort.Service) K8sHandler { + restConfig func(context.Context) (*rest.Config, error), dashStore dashboards.Store, userSvc user.Service, resourceClient resource.ResourceClient, sorter sort.Service) K8sHandler { legacySearcher := legacysearcher.NewDashboardSearchClient(dashStore, sorter) searchClient := resource.NewSearchClient(dual, gvr.GroupResource(), resourceClient, legacySearcher) @@ -79,9 +78,9 @@ func (h *k8sHandler) Get(ctx context.Context, name string, orgID int64, options defer cancel() } - client, ok := h.getClient(newCtx, orgID) - if !ok { - return nil, nil + client, err := h.getClient(newCtx, orgID) + if err != nil { + return nil, err } return client.Get(newCtx, name, options, subresource...) @@ -97,9 +96,9 @@ func (h *k8sHandler) Create(ctx context.Context, obj *unstructured.Unstructured, defer cancel() } - client, ok := h.getClient(newCtx, orgID) - if !ok { - return nil, nil + client, err := h.getClient(newCtx, orgID) + if err != nil { + return nil, err } return client.Create(newCtx, obj, v1.CreateOptions{}) @@ -115,9 +114,9 @@ func (h *k8sHandler) Update(ctx context.Context, obj *unstructured.Unstructured, defer cancel() } - client, ok := h.getClient(newCtx, orgID) - if !ok { - return nil, nil + client, err := h.getClient(newCtx, orgID) + if err != nil { + return nil, err } return client.Update(newCtx, obj, v1.UpdateOptions{}) @@ -133,9 +132,9 @@ func (h *k8sHandler) Delete(ctx context.Context, name string, orgID int64, optio defer cancel() } - client, ok := h.getClient(newCtx, orgID) - if !ok { - return nil + client, err := h.getClient(newCtx, orgID) + if err != nil { + return err } return client.Delete(newCtx, name, options) @@ -151,9 +150,9 @@ func (h *k8sHandler) DeleteCollection(ctx context.Context, orgID int64) error { defer cancel() } - client, ok := h.getClient(newCtx, orgID) - if !ok { - return fmt.Errorf("could not get k8s client") + client, err := h.getClient(newCtx, orgID) + if err != nil { + return err } return client.DeleteCollection(newCtx, v1.DeleteOptions{}, v1.ListOptions{}) @@ -169,9 +168,9 @@ func (h *k8sHandler) List(ctx context.Context, orgID int64, options v1.ListOptio defer cancel() } - client, ok := h.getClient(newCtx, orgID) - if !ok { - return nil, fmt.Errorf("could not get k8s client") + client, err := h.getClient(newCtx, orgID) + if err != nil { + return nil, err } return client.List(newCtx, options) @@ -226,18 +225,18 @@ func (h *k8sHandler) GetUserFromMeta(ctx context.Context, userMeta string) (*use return u, err } -func (h *k8sHandler) getClient(ctx context.Context, orgID int64) (dynamic.ResourceInterface, bool) { - cfg := h.restConfig(ctx) - if cfg == nil { - return nil, false +func (h *k8sHandler) getClient(ctx context.Context, orgID int64) (dynamic.ResourceInterface, error) { + cfg, err := h.restConfig(ctx) + if err != nil { + return nil, err } dyn, err := dynamic.NewForConfig(cfg) if err != nil { - return nil, false + return nil, fmt.Errorf("could not create dynamic client: %w", err) } - return dyn.Resource(h.gvr).Namespace(h.GetNamespace(orgID)), true + return dyn.Resource(h.gvr).Namespace(h.GetNamespace(orgID)), nil } func (h *k8sHandler) getK8sContext(ctx context.Context) (context.Context, context.CancelFunc, error) { diff --git a/pkg/services/apiserver/client/client_mock.go b/pkg/services/apiserver/client/client_mock.go index 57b6c4183f8..2c84a80e0be 100644 --- a/pkg/services/apiserver/client/client_mock.go +++ b/pkg/services/apiserver/client/client_mock.go @@ -94,6 +94,6 @@ type MockTestRestConfig struct { cfg *rest.Config } -func (r MockTestRestConfig) GetRestConfig(ctx context.Context) *rest.Config { - return r.cfg +func (r MockTestRestConfig) GetRestConfig(ctx context.Context) (*rest.Config, error) { + return r.cfg, nil } diff --git a/pkg/services/apiserver/service.go b/pkg/services/apiserver/service.go index 6cac47ab76a..dd9d39874e8 100644 --- a/pkg/services/apiserver/service.go +++ b/pkg/services/apiserver/service.go @@ -92,7 +92,7 @@ func init() { // The client Config gets initialized during the first call to // ProvideService. // Any call to GetRestConfig will block until we have a restConfig available -func GetRestConfig(ctx context.Context) *clientrest.Config { +func GetRestConfig(ctx context.Context) (*clientrest.Config, error) { <-ready return restConfig.GetRestConfig(ctx) } @@ -104,7 +104,7 @@ type Service interface { } type RestConfigProvider interface { - GetRestConfig(context.Context) *clientrest.Config + GetRestConfig(context.Context) (*clientrest.Config, error) } type DirectRestConfigProvider interface { @@ -244,11 +244,11 @@ func ProvideService( return s, nil } -func (s *service) GetRestConfig(ctx context.Context) *clientrest.Config { +func (s *service) GetRestConfig(ctx context.Context) (*clientrest.Config, error) { if err := s.NamedService.AwaitRunning(ctx); err != nil { - return nil + return nil, fmt.Errorf("unable to get rest config: %w", err) } - return s.restConfig + return s.restConfig, nil } func (s *service) IsDisabled() bool { diff --git a/pkg/services/authz/rbac.go b/pkg/services/authz/rbac.go index a589a5b99a5..f83a2b4d4a7 100644 --- a/pkg/services/authz/rbac.go +++ b/pkg/services/authz/rbac.go @@ -159,7 +159,7 @@ func RegisterRBACAuthZService( if folderAPIURL == "" { folderStore = store.NewSQLFolderStore(db, tracer) } else { - folderStore = store.NewAPIFolderStore(tracer, func(ctx context.Context) *rest.Config { + folderStore = store.NewAPIFolderStore(tracer, func(ctx context.Context) (*rest.Config, error) { return &rest.Config{ Host: folderAPIURL, WrapTransport: func(rt http.RoundTripper) http.RoundTripper { @@ -167,7 +167,7 @@ func RegisterRBACAuthZService( }, QPS: 50, Burst: 100, - } + }, nil }) } diff --git a/pkg/services/authz/rbac/store/folder_store.go b/pkg/services/authz/rbac/store/folder_store.go index 078cb2f4c61..a8ed3d2a1f0 100644 --- a/pkg/services/authz/rbac/store/folder_store.go +++ b/pkg/services/authz/rbac/store/folder_store.go @@ -102,13 +102,13 @@ func (s *SQLFolderStore) ListFolders(ctx context.Context, ns types.NamespaceInfo var _ FolderStore = (*APIFolderStore)(nil) -func NewAPIFolderStore(tracer tracing.Tracer, configProvider func(ctx context.Context) *rest.Config) *APIFolderStore { +func NewAPIFolderStore(tracer tracing.Tracer, configProvider func(ctx context.Context) (*rest.Config, error)) *APIFolderStore { return &APIFolderStore{tracer, configProvider} } type APIFolderStore struct { tracer tracing.Tracer - configProvider func(ctx context.Context) *rest.Config + configProvider func(ctx context.Context) (*rest.Config, error) } func (s *APIFolderStore) ListFolders(ctx context.Context, ns types.NamespaceInfo) ([]Folder, error) { @@ -150,7 +150,11 @@ func (s *APIFolderStore) ListFolders(ctx context.Context, ns types.NamespaceInfo } func (s *APIFolderStore) client(ctx context.Context, namespace string) (dynamic.ResourceInterface, error) { - client, err := dynamic.NewForConfig(s.configProvider(ctx)) + cfg, err := s.configProvider(ctx) + if err != nil { + return nil, err + } + client, err := dynamic.NewForConfig(cfg) if err != nil { return nil, err } diff --git a/pkg/services/folder/folderimpl/folder_unifiedstorage_test.go b/pkg/services/folder/folderimpl/folder_unifiedstorage_test.go index 244f11043f8..722e1fb2b87 100644 --- a/pkg/services/folder/folderimpl/folder_unifiedstorage_test.go +++ b/pkg/services/folder/folderimpl/folder_unifiedstorage_test.go @@ -48,10 +48,10 @@ type rcp struct { Host string } -func (r rcp) GetRestConfig(ctx context.Context) *clientrest.Config { +func (r rcp) GetRestConfig(ctx context.Context) (*clientrest.Config, error) { return &clientrest.Config{ Host: r.Host, - } + }, nil } func TestIntegrationFolderServiceViaUnifiedStorage(t *testing.T) {