Deserialization/Type Confusion in Kubernetes client usage (unstructured -> typed)

diff --git a/pkg/server/wire_gen.go b/pkg/server/wire_gen.go index 01f1bcff7def1..8b1f668717978 100644 --- a/pkg/server/wire_gen.go +++ b/pkg/server/wire_gen.go @@ -442,7 +442,8 @@ func Initialize(ctx context.Context, cfg *setting.Cfg, opts Options, apiOpts api if err != nil { return nil, err } - userimplService, err := userimpl.ProvideService(sqlStore, orgService, cfg, teamimplService, cacheService, tracingService, quotaService, bundleregistryService, eventualRestConfigProvider) + clientGenerator := apiserver.ProvideClientGenerator(eventualRestConfigProvider) + userimplService, err := userimpl.ProvideService(sqlStore, orgService, cfg, teamimplService, cacheService, tracingService, quotaService, bundleregistryService, clientGenerator) if err != nil { return nil, err } @@ -768,7 +769,6 @@ func Initialize(ctx context.Context, cfg *setting.Cfg, opts Options, apiOpts api apiAPI := api3.ProvideApi(cfg, featureToggles, starService, eventualRestConfigProvider) anonUserLimitValidatorImpl := validator2.ProvideAnonUserLimitValidator() anonDeviceService := anonimpl.ProvideAnonymousDeviceService(usageStats, authnService, sqlStore, cfg, orgService, serverLockService, accessControl, routeRegisterImpl, anonUserLimitValidatorImpl) - clientGenerator := apiserver.ProvideClientGenerator(eventualRestConfigProvider) signingkeysimplService, err := signingkeysimpl.ProvideEmbeddedSigningKeysService(sqlStore, secretsService, remoteCache, routeRegisterImpl) if err != nil { return nil, err @@ -1141,7 +1141,8 @@ func InitializeForTest(ctx context.Context, t sqlutil.ITestDB, testingT interfac if err != nil { return nil, err } - userimplService, err := userimpl.ProvideService(sqlStore, orgService, cfg, teamimplService, cacheService, tracingService, quotaService, bundleregistryService, eventualRestConfigProvider) + clientGenerator := apiserver.ProvideClientGenerator(eventualRestConfigProvider) + userimplService, err := userimpl.ProvideService(sqlStore, orgService, cfg, teamimplService, cacheService, tracingService, quotaService, bundleregistryService, clientGenerator) if err != nil { return nil, err } @@ -1469,7 +1470,6 @@ func InitializeForTest(ctx context.Context, t sqlutil.ITestDB, testingT interfac apiAPI := api3.ProvideApi(cfg, featureToggles, starService, eventualRestConfigProvider) anonUserLimitValidatorImpl := validator2.ProvideAnonUserLimitValidator() anonDeviceService := anonimpl.ProvideAnonymousDeviceService(usageStats, authnService, sqlStore, cfg, orgService, serverLockService, accessControl, routeRegisterImpl, anonUserLimitValidatorImpl) - clientGenerator := apiserver.ProvideClientGenerator(eventualRestConfigProvider) signingkeysimplService, err := signingkeysimpl.ProvideEmbeddedSigningKeysService(sqlStore, secretsService, remoteCache, routeRegisterImpl) if err != nil { return nil, err @@ -1761,7 +1761,8 @@ func InitializeForCLI(ctx context.Context, cfg *setting.Cfg) (Runner, error) { return Runner{}, err } cacheService := localcache.ProvideService() - userimplService, err := userimpl.ProvideService(sqlStore, orgService, cfg, teamimplService, cacheService, tracingService, quotaService, bundleregistryService, eventualRestConfigProvider) + clientGenerator := apiserver.ProvideClientGenerator(eventualRestConfigProvider) + userimplService, err := userimpl.ProvideService(sqlStore, orgService, cfg, teamimplService, cacheService, tracingService, quotaService, bundleregistryService, clientGenerator) if err != nil { return Runner{}, err } diff --git a/pkg/services/apiserver/wireset.go b/pkg/services/apiserver/wireset.go index daaf337bc2339..393b775019e03 100644 --- a/pkg/services/apiserver/wireset.go +++ b/pkg/services/apiserver/wireset.go @@ -16,3 +16,13 @@ var WireSet = wire.NewSet( wire.Bind(new(builder.APIRegistrar), new(*service)), ProvideClientGenerator, ) + +// BaseCLIWireSet provides the minimal set needed by CLI runners that don't start +// the full apiserver: the concrete rest config provider (bound to both interfaces) +// and the typed ClientGenerator — without ProvideService or its builder metrics. +var BaseCLIWireSet = wire.NewSet( + ProvideEventualRestConfigProvider, + wire.Bind(new(RestConfigProvider), new(*eventualRestConfigProvider)), + wire.Bind(new(DirectRestConfigProvider), new(*eventualRestConfigProvider)), + ProvideClientGenerator, +) diff --git a/pkg/services/user/userimpl/user.go b/pkg/services/user/userimpl/user.go index c17f5ffb06aad..a11d7b8f7705d 100644 --- a/pkg/services/user/userimpl/user.go +++ b/pkg/services/user/userimpl/user.go @@ -4,6 +4,7 @@ import ( "context" "errors" + "github.com/grafana/grafana-app-sdk/resource" "github.com/open-feature/go-sdk/openfeature" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/codes" @@ -14,7 +15,6 @@ import ( "github.com/grafana/grafana/pkg/infra/localcache" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/tracing" - "github.com/grafana/grafana/pkg/services/apiserver" "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/org" "github.com/grafana/grafana/pkg/services/quota" @@ -31,6 +31,7 @@ type Service struct { openFeatureClient *openfeature.Client logger log.Logger tracer tracing.Tracer + cfg *setting.Cfg } var _ user.Service = (*Service)(nil) @@ -41,13 +42,13 @@ func ProvideService(db db.DB, teamService team.Service, cacheService *localcache.CacheService, tracer tracing.Tracer, quotaService quota.Service, bundleRegistry supportbundles.Service, - configProvider apiserver.DirectRestConfigProvider) (*Service, error) { + clientGenerator resource.ClientGenerator) (*Service, error) { legacyService, err := NewLegacyService(db, orgService, cfg, teamService, cacheService, tracer, quotaService, bundleRegistry) if err != nil { return nil, err } - k8sService := userk8s.NewUserK8sService(log.New("user.k8s"), cfg, configProvider, tracer) + k8sService := userk8s.NewUserK8sService(log.New("user.k8s"), cfg, clientGenerator, tracer) return &Service{ legacyService: legacyService, @@ -55,6 +56,7 @@ func ProvideService(db db.DB, openFeatureClient: openfeature.NewDefaultClient(), logger: log.New("user"), tracer: tracer, + cfg: cfg, }, nil } @@ -186,19 +188,17 @@ func (s *Service) GetSignedInUser(ctx context.Context, cmd *user.GetSignedInUser ctxLogger := s.logger.FromContext(ctx) if s.isKubernetesUserServiceEnabled(ctx) { - if hasOrgID(ctx) || cmd.OrgID > 0 { - result, err := s.k8sService.GetSignedInUser(ctx, cmd) - if err == nil { - span.SetAttributes(attribute.Bool("fallback_to_legacy", false)) - return result, nil - } - // Fall back to legacy is needed on the following cases: - // - When the user is not found in k8s. It happens with integration tests where the user is created in the legacy store but not in k8s. - // - When there is no request context (e.g. calls from the k8s API server handler). - ctxLogger.Warn("k8s GetSignedInUser failed, falling back to legacy", "userID", cmd.UserID, "err", err) - } else { - ctxLogger.Warn("no orgID in context, falling back to legacy", "method", "GetSignedInUser") + k8sCmd := *cmd + if !hasOrgID(ctx) && k8sCmd.OrgID == 0 { + k8sCmd.OrgID = s.cfg.DefaultOrgID() + } + + result, err := s.k8sService.GetSignedInUser(ctx, &k8sCmd) + if err == nil { + span.SetAttributes(attribute.Bool("fallback_to_legacy", false)) + return result, nil } + ctxLogger.Warn("k8s GetSignedInUser failed, falling back to legacy", "userID", cmd.UserID, "err", err) } span.SetAttributes(attribute.Bool("fallback_to_legacy", true)) diff --git a/pkg/services/user/userk8s/user.go b/pkg/services/user/userk8s/user.go index 9f762dc3a67ea..88fdf8b874208 100644 --- a/pkg/services/user/userk8s/user.go +++ b/pkg/services/user/userk8s/user.go @@ -7,71 +7,48 @@ import ( "strings" "time" + "github.com/grafana/grafana-app-sdk/resource" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/codes" "go.opentelemetry.io/otel/trace" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/apimachinery/pkg/fields" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/client-go/dynamic" iamv0alpha1 "github.com/grafana/grafana/apps/iam/pkg/apis/iam/v0alpha1" "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/apimachinery/utils" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/tracing" - "github.com/grafana/grafana/pkg/services/apiserver" "github.com/grafana/grafana/pkg/services/apiserver/endpoints/request" - "github.com/grafana/grafana/pkg/services/contexthandler" "github.com/grafana/grafana/pkg/services/user" "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/util" ) -var userGVR = schema.GroupVersionResource{ - Group: iamv0alpha1.APIGroup, - Version: iamv0alpha1.APIVersion, - Resource: "users", -} - type UserK8sService struct { logger log.Logger namespaceMapper request.NamespaceMapper - configProvider apiserver.DirectRestConfigProvider + clientGenerator resource.ClientGenerator config *setting.Cfg tracer tracing.Tracer } var _ user.Service = (*UserK8sService)(nil) -func NewUserK8sService(logger log.Logger, cfg *setting.Cfg, configProvider apiserver.DirectRestConfigProvider, tracer tracing.Tracer) *UserK8sService { +func NewUserK8sService(logger log.Logger, cfg *setting.Cfg, clientGenerator resource.ClientGenerator, tracer tracing.Tracer) *UserK8sService { return &UserK8sService{ logger: logger, namespaceMapper: request.GetNamespaceMapper(cfg), - configProvider: configProvider, + clientGenerator: clientGenerator, config: cfg, tracer: tracer, } } -func (s *UserK8sService) getClient(ctx context.Context, namespace string) (dynamic.ResourceInterface, error) { - if s.configProvider == nil { - return nil, errors.New("config provider not initialized") +func (s *UserK8sService) getUserClient() (*iamv0alpha1.UserClient, error) { + if s.clientGenerator == nil { + return nil, errors.New("client generator not initialized") } - - reqCtx := contexthandler.FromContext(ctx) - if reqCtx == nil { - return nil, errors.New("no request context") - } - - dyn, err := dynamic.NewForConfig(s.configProvider.GetDirectRestConfig(reqCtx)) - if err != nil { - return nil, err - } - - return dyn.Resource(userGVR).Namespace(namespace), nil + return iamv0alpha1.NewUserClientFromGenerator(s.clientGenerator) } func (s *UserK8sService) Create(ctx context.Context, cmd *user.CreateUserCommand) (*user.User, error) { @@ -91,7 +68,7 @@ func (s *UserK8sService) Create(ctx context.Context, cmd *user.CreateUserCommand namespace := s.namespaceMapper(orgID) span.SetAttributes(attribute.Int64("orgID", orgID)) - client, err := s.getClient(ctx, namespace) + client, err := s.getUserClient() if err != nil { ctxLogger.Error("failed to get k8s client", "namespace", namespace, "err", err) return nil, err @@ -111,11 +88,7 @@ func (s *UserK8sService) Create(ctx context.Context, cmd *user.CreateUserCommand cmd.Email = cmd.Login } - k8sUser := iamv0alpha1.User{ - TypeMeta: metav1.TypeMeta{ - APIVersion: iamv0alpha1.GroupVersion.Identifier(), - Kind: "User", - }, + k8sUser := &iamv0alpha1.User{ ObjectMeta: metav1.ObjectMeta{ Name: uid, Namespace: namespace, @@ -132,24 +105,14 @@ func (s *UserK8sService) Create(ctx context.Context, cmd *user.CreateUserCommand }, } - unstructuredObj, err := runtime.DefaultUnstructuredConverter.ToUnstructured(&k8sUser) - if err != nil { - return nil, err - } - - result, err := client.Create(ctx, &unstructured.Unstructured{Object: unstructuredObj}, metav1.CreateOptions{}) + created, err := client.Create(ctx, k8sUser, resource.CreateOptions{}) if err != nil { ctxLogger.Error("k8s user create failed", "namespace", namespace, "orgID", orgID, "login", cmd.Login, "err", err) span.RecordError(err) return nil, err } - var created iamv0alpha1.User - if err := runtime.DefaultUnstructuredConverter.FromUnstructured(result.Object, &created); err != nil { - return nil, err - } - - return toUser(&created, orgID), nil + return toUser(created, orgID), nil } func (s *UserK8sService) CreateServiceAccount(ctx context.Context, cmd *user.CreateUserCommand) (*user.User, error) { @@ -179,7 +142,7 @@ func (s *UserK8sService) GetByID(ctx context.Context, cmd *user.GetUserByIDQuery namespace := s.namespaceMapper(orgID) span.SetAttributes(attribute.Int64("orgID", orgID)) - client, err := s.getClient(ctx, namespace) + client, err := s.getUserClient() if err != nil { ctxLogger.Error("failed to get k8s client", "namespace", namespace, "err", err) span.RecordError(err) @@ -228,14 +191,14 @@ func (s *UserK8sService) GetByLogin(ctx context.Context, cmd *user.GetUserByLogi namespace := s.namespaceMapper(orgID) span.SetAttributes(attribute.Int64("orgID", orgID)) - client, err := s.getClient(ctx, namespace) + client, err := s.getUserClient() if err != nil { ctxLogger.Error("failed to get k8s client", "namespace", namespace, "err", err) return nil, err } if strings.Contains(loginOrEmail, "@") { - u, err := s.getByFieldSelector(ctx, ctxLogger, client, "spec.email", loginOrEmail, orgID) + u, err := s.getByFieldSelector(ctx, ctxLogger, client, "spec.email", loginOrEmail, namespace, orgID) if err != nil && !errors.Is(err, user.ErrUserNotFound) { span.RecordError(err) return nil, err @@ -245,7 +208,7 @@ func (s *UserK8sService) GetByLogin(ctx context.Context, cmd *user.GetUserByLogi } } - u, err := s.getByFieldSelector(ctx, ctxLogger, client, "spec.login", loginOrEmail, orgID) + u, err := s.getByFieldSelector(ctx, ctxLogger, client, "spec.login", loginOrEmail, namespace, orgID) if err != nil { span.RecordError(err) return nil, err @@ -271,13 +234,13 @@ func (s *UserK8sService) GetByEmail(ctx context.Context, cmd *user.GetUserByEmai namespace := s.namespaceMapper(orgID) span.SetAttributes(attribute.Int64("orgID", orgID)) - client, err := s.getClient(ctx, namespace) + client, err := s.getUserClient() if err != nil { ctxLogger.Error("failed to get k8s client", "namespace", namespace, "err", err) return nil, err } - u, err := s.getByFieldSelector(ctx, ctxLogger, client, "spec.email", strings.ToLower(cmd.Email), orgID) + u, err := s.getByFieldSelector(ctx, ctxLogger, client, "spec.email", strings.ToLower(cmd.Email), namespace, orgID) if err != nil { span.RecordError(err) return nil, err @@ -305,7 +268,7 @@ func (s *UserK8sService) Update(ctx context.Context, cmd *user.UpdateUserCommand namespace := s.namespaceMapper(orgID) span.SetAt ... [truncated]