From b6a020148b1189dccf061d332c23a88b0e0031c8 Mon Sep 17 00:00:00 2001 From: Charandas Date: Wed, 13 Mar 2024 16:54:30 -0700 Subject: [PATCH] K8s: disallow MT storage functionality for Aggregator builders (#84408) --- pkg/registry/apis/apis.go | 2 - .../apiserver/aggregator/aggregator.go | 31 ++++++++++++++-- pkg/services/apiserver/aggregator/config.go | 37 +++++++++++++++++-- pkg/services/apiserver/options/aggregator.go | 18 +-------- pkg/services/apiserver/service.go | 5 ++- 5 files changed, 67 insertions(+), 26 deletions(-) diff --git a/pkg/registry/apis/apis.go b/pkg/registry/apis/apis.go index afc747ccdb9..90b7de08755 100644 --- a/pkg/registry/apis/apis.go +++ b/pkg/registry/apis/apis.go @@ -14,7 +14,6 @@ import ( "github.com/grafana/grafana/pkg/registry/apis/playlist" "github.com/grafana/grafana/pkg/registry/apis/query" "github.com/grafana/grafana/pkg/registry/apis/scope" - "github.com/grafana/grafana/pkg/registry/apis/service" ) var ( @@ -35,7 +34,6 @@ func ProvideRegistryServiceSink( _ *folders.FolderAPIBuilder, _ *peakq.PeakQAPIBuilder, _ *scope.ScopeAPIBuilder, - _ *service.ServiceAPIBuilder, _ *query.QueryAPIBuilder, ) *Service { return &Service{} diff --git a/pkg/services/apiserver/aggregator/aggregator.go b/pkg/services/apiserver/aggregator/aggregator.go index 1a3bca1c4e3..55dc9ab78f8 100644 --- a/pkg/services/apiserver/aggregator/aggregator.go +++ b/pkg/services/apiserver/aggregator/aggregator.go @@ -23,6 +23,7 @@ import ( "time" servicev0alpha1 "github.com/grafana/grafana/pkg/apis/service/v0alpha1" + "github.com/grafana/grafana/pkg/registry/apis/service" "gopkg.in/yaml.v3" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" @@ -37,11 +38,13 @@ import ( v1 "k8s.io/kube-aggregator/pkg/apis/apiregistration/v1" v1helper "k8s.io/kube-aggregator/pkg/apis/apiregistration/v1/helper" aggregatorapiserver "k8s.io/kube-aggregator/pkg/apiserver" + aggregatorscheme "k8s.io/kube-aggregator/pkg/apiserver/scheme" apiregistrationclientset "k8s.io/kube-aggregator/pkg/client/clientset_generated/clientset" apiregistrationclient "k8s.io/kube-aggregator/pkg/client/clientset_generated/clientset/typed/apiregistration/v1" apiregistrationInformers "k8s.io/kube-aggregator/pkg/client/informers/externalversions/apiregistration/v1" "k8s.io/kube-aggregator/pkg/controllers/autoregister" + "github.com/grafana/grafana/pkg/apiserver/builder" servicev0alpha1applyconfiguration "github.com/grafana/grafana/pkg/generated/applyconfiguration/service/v0alpha1" serviceclientset "github.com/grafana/grafana/pkg/generated/clientset/versioned" informersv0alpha1 "github.com/grafana/grafana/pkg/generated/informers/externalversions" @@ -131,9 +134,15 @@ func CreateAggregatorConfig(commandOptions *options.Options, sharedConfig generi return nil, err } + serviceAPIBuilder := service.NewServiceAPIBuilder() + if err := serviceAPIBuilder.InstallSchema(aggregatorscheme.Scheme); err != nil { + return nil, err + } + APIVersionPriorities[serviceAPIBuilder.GetGroupVersion()] = Priority{Group: 15000, Version: int32(1)} + // Exit early, if no remote services file is configured if commandOptions.AggregatorOptions.RemoteServicesFile == "" { - return NewConfig(aggregatorConfig, sharedInformerFactory, nil), nil + return NewConfig(aggregatorConfig, sharedInformerFactory, []builder.APIGroupBuilder{serviceAPIBuilder}, nil), nil } _, err = readCABundlePEM(commandOptions.AggregatorOptions.APIServiceCABundleFile, commandOptions.ExtraOptions.DevMode) @@ -157,11 +166,16 @@ func CreateAggregatorConfig(commandOptions *options.Options, sharedConfig generi serviceClientSet: serviceClient, } - return NewConfig(aggregatorConfig, sharedInformerFactory, remoteServicesConfig), nil + return NewConfig(aggregatorConfig, sharedInformerFactory, []builder.APIGroupBuilder{serviceAPIBuilder}, remoteServicesConfig), nil } -func CreateAggregatorServer(aggregatorConfig *aggregatorapiserver.Config, sharedInformerFactory informersv0alpha1.SharedInformerFactory, remoteServicesConfig *RemoteServicesConfig, delegateAPIServer genericapiserver.DelegationTarget) (*aggregatorapiserver.APIAggregator, error) { +func CreateAggregatorServer(config *Config, delegateAPIServer genericapiserver.DelegationTarget) (*aggregatorapiserver.APIAggregator, error) { + aggregatorConfig := config.KubeAggregatorConfig + sharedInformerFactory := config.Informers + remoteServicesConfig := config.RemoteServicesConfig + completedConfig := aggregatorConfig.Complete() + aggregatorServer, err := completedConfig.NewWithDelegate(delegateAPIServer) if err != nil { return nil, err @@ -174,6 +188,7 @@ func CreateAggregatorServer(aggregatorConfig *aggregatorapiserver.Config, shared } autoRegistrationController := autoregister.NewAutoRegisterController(aggregatorServer.APIRegistrationInformers.Apiregistration().V1().APIServices(), apiRegistrationClient) + apiServices := apiServicesToRegister(delegateAPIServer, autoRegistrationController) // Imbue all builtin group-priorities onto the aggregated discovery @@ -252,6 +267,16 @@ func CreateAggregatorServer(aggregatorConfig *aggregatorapiserver.Config, shared return nil }) + for _, b := range config.Builders { + serviceAPIGroupInfo, err := b.GetAPIGroupInfo(aggregatorscheme.Scheme, aggregatorscheme.Codecs, aggregatorConfig.GenericConfig.RESTOptionsGetter, false) + if err != nil { + return nil, err + } + if err := aggregatorServer.GenericAPIServer.InstallAPIGroup(serviceAPIGroupInfo); err != nil { + return nil, err + } + } + return aggregatorServer, nil } diff --git a/pkg/services/apiserver/aggregator/config.go b/pkg/services/apiserver/aggregator/config.go index 47e3d79129b..e50f9383165 100644 --- a/pkg/services/apiserver/aggregator/config.go +++ b/pkg/services/apiserver/aggregator/config.go @@ -1,9 +1,16 @@ package aggregator import ( + openapinamer "k8s.io/apiserver/pkg/endpoints/openapi" + genericapiserver "k8s.io/apiserver/pkg/server" + aggregatorapiserver "k8s.io/kube-aggregator/pkg/apiserver" + aggregatorscheme "k8s.io/kube-aggregator/pkg/apiserver/scheme" + aggregatoropenapi "k8s.io/kube-aggregator/pkg/generated/openapi" + "k8s.io/kube-openapi/pkg/common" + + "github.com/grafana/grafana/pkg/apiserver/builder" serviceclientset "github.com/grafana/grafana/pkg/generated/clientset/versioned" informersv0alpha1 "github.com/grafana/grafana/pkg/generated/informers/externalversions" - aggregatorapiserver "k8s.io/kube-aggregator/pkg/apiserver" ) type RemoteService struct { @@ -25,13 +32,37 @@ type Config struct { KubeAggregatorConfig *aggregatorapiserver.Config Informers informersv0alpha1.SharedInformerFactory RemoteServicesConfig *RemoteServicesConfig + // Builders contain prerequisite api groups for aggregator to function correctly e.g. ExternalName + // Since the main APIServer delegate supports storage implementations that intend to be multi-tenant + // Aggregator builders that we don't intend to use multi-tenant storage are kept in aggregator's + // Delegate, one which is configured explicitly to use file storage only + Builders []builder.APIGroupBuilder } -// remoteServices may be nil, when not using aggregation -func NewConfig(aggregator *aggregatorapiserver.Config, informers informersv0alpha1.SharedInformerFactory, remoteServices *RemoteServicesConfig) *Config { +// remoteServices may be nil when not using aggregation +func NewConfig(aggregator *aggregatorapiserver.Config, informers informersv0alpha1.SharedInformerFactory, builders []builder.APIGroupBuilder, remoteServices *RemoteServicesConfig) *Config { + getMergedOpenAPIDefinitions := func(ref common.ReferenceCallback) map[string]common.OpenAPIDefinition { + aggregatorAPIs := aggregatoropenapi.GetOpenAPIDefinitions(ref) + builderAPIs := builder.GetOpenAPIDefinitions(builders)(ref) + + for k, v := range builderAPIs { + aggregatorAPIs[k] = v + } + + return aggregatorAPIs + } + + // Add OpenAPI config, which depends on builders + namer := openapinamer.NewDefinitionNamer(aggregatorscheme.Scheme) + aggregator.GenericConfig.OpenAPIV3Config = genericapiserver.DefaultOpenAPIV3Config(getMergedOpenAPIDefinitions, namer) + aggregator.GenericConfig.OpenAPIV3Config.Info.Title = "Kubernetes" + aggregator.GenericConfig.OpenAPIConfig = genericapiserver.DefaultOpenAPIConfig(getMergedOpenAPIDefinitions, namer) + aggregator.GenericConfig.OpenAPIConfig.Info.Title = "Kubernetes" + return &Config{ aggregator, informers, remoteServices, + builders, } } diff --git a/pkg/services/apiserver/options/aggregator.go b/pkg/services/apiserver/options/aggregator.go index a9b00097810..911b6c9c951 100644 --- a/pkg/services/apiserver/options/aggregator.go +++ b/pkg/services/apiserver/options/aggregator.go @@ -1,11 +1,12 @@ package options import ( + servicev0alpha1 "github.com/grafana/grafana/pkg/apis/service/v0alpha1" + filestorage "github.com/grafana/grafana/pkg/apiserver/storage/file" "github.com/spf13/pflag" v1 "k8s.io/api/apps/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" - openapinamer "k8s.io/apiserver/pkg/endpoints/openapi" genericfeatures "k8s.io/apiserver/pkg/features" genericapiserver "k8s.io/apiserver/pkg/server" "k8s.io/apiserver/pkg/server/options" @@ -14,11 +15,6 @@ import ( apiregistrationv1beta1 "k8s.io/kube-aggregator/pkg/apis/apiregistration/v1beta1" aggregatorapiserver "k8s.io/kube-aggregator/pkg/apiserver" aggregatorscheme "k8s.io/kube-aggregator/pkg/apiserver/scheme" - aggregatoropenapi "k8s.io/kube-aggregator/pkg/generated/openapi" - "k8s.io/kube-openapi/pkg/common" - - servicev0alpha1 "github.com/grafana/grafana/pkg/apis/service/v0alpha1" - filestorage "github.com/grafana/grafana/pkg/apiserver/storage/file" ) // AggregatorServerOptions contains the state for the aggregator apiserver @@ -34,11 +30,6 @@ func NewAggregatorServerOptions() *AggregatorServerOptions { return &AggregatorServerOptions{} } -func (o *AggregatorServerOptions) getMergedOpenAPIDefinitions(ref common.ReferenceCallback) map[string]common.OpenAPIDefinition { - aggregatorAPIs := aggregatoropenapi.GetOpenAPIDefinitions(ref) - return aggregatorAPIs -} - func (o *AggregatorServerOptions) AddFlags(fs *pflag.FlagSet) { if o == nil { return @@ -109,11 +100,6 @@ func (o *AggregatorServerOptions) ApplyTo(aggregatorConfig *aggregatorapiserver. aggregatorConfig.ExtraConfig.ProxyClientCertFile = o.ProxyClientCertFile aggregatorConfig.ExtraConfig.ProxyClientKeyFile = o.ProxyClientKeyFile - namer := openapinamer.NewDefinitionNamer(aggregatorscheme.Scheme) - genericConfig.OpenAPIV3Config = genericapiserver.DefaultOpenAPIV3Config(o.getMergedOpenAPIDefinitions, namer) - genericConfig.OpenAPIV3Config.Info.Title = "Kubernetes" - genericConfig.OpenAPIConfig = genericapiserver.DefaultOpenAPIConfig(o.getMergedOpenAPIDefinitions, namer) - genericConfig.OpenAPIConfig.Info.Title = "Kubernetes" genericConfig.PostStartHooks = map[string]genericapiserver.PostStartHookConfigEntry{} // These hooks use v1 informers, which are not available in the grafana aggregator. diff --git a/pkg/services/apiserver/service.go b/pkg/services/apiserver/service.go index e6d32610dbe..3315c62fabb 100644 --- a/pkg/services/apiserver/service.go +++ b/pkg/services/apiserver/service.go @@ -193,6 +193,7 @@ func (s *service) start(ctx context.Context) error { groupVersions := make([]schema.GroupVersion, 0, len(builders)) // Install schemas + initialSize := len(aggregator.APIVersionPriorities) for i, b := range builders { groupVersions = append(groupVersions, b.GetGroupVersion()) if err := b.InstallSchema(Scheme); err != nil { @@ -201,7 +202,7 @@ func (s *service) start(ctx context.Context) error { if s.features.IsEnabledGlobally(featuremgmt.FlagKubernetesAggregator) { // set the priority for the group+version - aggregator.APIVersionPriorities[b.GetGroupVersion()] = aggregator.Priority{Group: 15000, Version: int32(i + 1)} + aggregator.APIVersionPriorities[b.GetGroupVersion()] = aggregator.Priority{Group: 15000, Version: int32(i + initialSize)} } auth := b.GetAuthorizer() @@ -378,7 +379,7 @@ func (s *service) startAggregator( return nil, err } - aggregatorServer, err := aggregator.CreateAggregatorServer(aggregatorConfig.KubeAggregatorConfig, aggregatorConfig.Informers, aggregatorConfig.RemoteServicesConfig, server) + aggregatorServer, err := aggregator.CreateAggregatorServer(aggregatorConfig, server) if err != nil { return nil, err }