K8s: Add error to GetRestConfig (grafana#101147)

K8s: Add error to RestConfigProvider return values
tipee-sa · Feb 21, 2025 · 9e80b0f · 9e80b0f
1 parent 2010c66
commit 9e80b0f
Show file tree

Hide file tree

Showing 8 changed files with 55 additions and 52 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -159,15 +159,15 @@ 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 {
 					return &tokenExhangeRoundTripper{te: exchangeClient, rt: rt}
 				},
 				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
@@ -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
@@ -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) {