From 3a3be8c7046d2d56e4720a2bcb6b8b8509e02879 Mon Sep 17 00:00:00 2001 From: "Dr. Stefan Schimanski" Date: Wed, 1 May 2024 16:10:32 +0200 Subject: [PATCH] controlplane: add generic storage construction Signed-off-by: Dr. Stefan Schimanski --- pkg/controlplane/apiserver/apis.go | 52 ++++++++++++++ pkg/controlplane/instance.go | 112 ++++++++++++++--------------- pkg/controlplane/instance_test.go | 88 ++++++++++++++++++++--- 3 files changed, 186 insertions(+), 66 deletions(-) diff --git a/pkg/controlplane/apiserver/apis.go b/pkg/controlplane/apiserver/apis.go index c33180d5944..1ccfe787466 100644 --- a/pkg/controlplane/apiserver/apis.go +++ b/pkg/controlplane/apiserver/apis.go @@ -22,7 +22,20 @@ import ( "k8s.io/apiserver/pkg/registry/generic" genericapiserver "k8s.io/apiserver/pkg/server" serverstorage "k8s.io/apiserver/pkg/server/storage" + "k8s.io/client-go/discovery" "k8s.io/klog/v2" + svmrest "k8s.io/kubernetes/pkg/registry/storagemigration/rest" + + admissionregistrationrest "k8s.io/kubernetes/pkg/registry/admissionregistration/rest" + apiserverinternalrest "k8s.io/kubernetes/pkg/registry/apiserverinternal/rest" + authenticationrest "k8s.io/kubernetes/pkg/registry/authentication/rest" + authorizationrest "k8s.io/kubernetes/pkg/registry/authorization/rest" + certificatesrest "k8s.io/kubernetes/pkg/registry/certificates/rest" + coordinationrest "k8s.io/kubernetes/pkg/registry/coordination/rest" + corerest "k8s.io/kubernetes/pkg/registry/core/rest" + eventsrest "k8s.io/kubernetes/pkg/registry/events/rest" + flowcontrolrest "k8s.io/kubernetes/pkg/registry/flowcontrol/rest" + rbacrest "k8s.io/kubernetes/pkg/registry/rbac/rest" ) // RESTStorageProvider is a factory type for REST storage. @@ -31,6 +44,45 @@ type RESTStorageProvider interface { NewRESTStorage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (genericapiserver.APIGroupInfo, error) } +// NewCoreGenericConfig returns a new core rest generic config. +func (c *CompletedConfig) NewCoreGenericConfig() *corerest.GenericConfig { + return &corerest.GenericConfig{ + StorageFactory: c.Extra.StorageFactory, + EventTTL: c.Extra.EventTTL, + LoopbackClientConfig: c.Generic.LoopbackClientConfig, + ServiceAccountIssuer: c.Extra.ServiceAccountIssuer, + ExtendExpiration: c.Extra.ExtendExpiration, + ServiceAccountMaxExpiration: c.Extra.ServiceAccountMaxExpiration, + APIAudiences: c.Generic.Authentication.APIAudiences, + Informers: c.Extra.VersionedInformers, + } +} + +// GenericStorageProviders returns a set of APIs for a generic control plane. +// They ought to be a subset of those served by kube-apiserver. +func (c *CompletedConfig) GenericStorageProviders(discovery discovery.DiscoveryInterface) ([]RESTStorageProvider, error) { + // The order here is preserved in discovery. + // If resources with identical names exist in more than one of these groups (e.g. "deployments.apps"" and "deployments.extensions"), + // the order of this list determines which group an unqualified resource name (e.g. "deployments") should prefer. + // This priority order is used for local discovery, but it ends up aggregated in `k8s.io/kubernetes/cmd/kube-apiserver/app/aggregator.go + // with specific priorities. + // TODO: describe the priority all the way down in the RESTStorageProviders and plumb it back through the various discovery + // handlers that we have. + return []RESTStorageProvider{ + c.NewCoreGenericConfig(), + apiserverinternalrest.StorageProvider{}, + authenticationrest.RESTStorageProvider{Authenticator: c.Generic.Authentication.Authenticator, APIAudiences: c.Generic.Authentication.APIAudiences}, + authorizationrest.RESTStorageProvider{Authorizer: c.Generic.Authorization.Authorizer, RuleResolver: c.Generic.RuleResolver}, + certificatesrest.RESTStorageProvider{}, + coordinationrest.RESTStorageProvider{}, + rbacrest.RESTStorageProvider{Authorizer: c.Generic.Authorization.Authorizer}, + svmrest.RESTStorageProvider{}, + flowcontrolrest.RESTStorageProvider{InformerFactory: c.Generic.SharedInformerFactory}, + admissionregistrationrest.RESTStorageProvider{Authorizer: c.Generic.Authorization.Authorizer, DiscoveryClient: discovery}, + eventsrest.RESTStorageProvider{TTL: c.EventTTL}, + }, nil +} + // InstallAPIs will install the APIs for the restStorageProviders if they are enabled. func (s *Server) InstallAPIs(restStorageProviders ...RESTStorageProvider) error { nonLegacy := []*genericapiserver.APIGroupInfo{} diff --git a/pkg/controlplane/instance.go b/pkg/controlplane/instance.go index f74aba809b6..d51f15ec982 100644 --- a/pkg/controlplane/instance.go +++ b/pkg/controlplane/instance.go @@ -58,6 +58,7 @@ import ( genericapiserver "k8s.io/apiserver/pkg/server" serverstorage "k8s.io/apiserver/pkg/server/storage" utilfeature "k8s.io/apiserver/pkg/util/feature" + clientdiscovery "k8s.io/client-go/discovery" "k8s.io/client-go/kubernetes" corev1client "k8s.io/client-go/kubernetes/typed/core/v1" discoveryclient "k8s.io/client-go/kubernetes/typed/discovery/v1" @@ -322,67 +323,11 @@ func (c CompletedConfig) New(delegationTarget genericapiserver.DelegationTarget) return nil, err } - // TODO: update to a version that caches success but will recheck on failure, unlike memcache discovery - discoveryClientForAdmissionRegistration := client.Discovery() - - legacyRESTStorageProvider, err := corerest.New(corerest.Config{ - GenericConfig: corerest.GenericConfig{ - StorageFactory: c.ControlPlane.Extra.StorageFactory, - EventTTL: c.ControlPlane.Extra.EventTTL, - LoopbackClientConfig: c.ControlPlane.Generic.LoopbackClientConfig, - ServiceAccountIssuer: c.ControlPlane.Extra.ServiceAccountIssuer, - ExtendExpiration: c.ControlPlane.Extra.ExtendExpiration, - ServiceAccountMaxExpiration: c.ControlPlane.Extra.ServiceAccountMaxExpiration, - APIAudiences: c.ControlPlane.Generic.Authentication.APIAudiences, - Informers: c.ControlPlane.Extra.VersionedInformers, - }, - Proxy: corerest.ProxyConfig{ - Transport: c.ControlPlane.Extra.ProxyTransport, - KubeletClientConfig: c.Extra.KubeletClientConfig, - }, - Services: corerest.ServicesConfig{ - ClusterIPRange: c.Extra.ServiceIPRange, - SecondaryClusterIPRange: c.Extra.SecondaryServiceIPRange, - NodePortRange: c.Extra.ServiceNodePortRange, - IPRepairInterval: c.Extra.RepairServicesInterval, - }, - }) + restStorageProviders, err := c.StorageProviders(client.Discovery()) if err != nil { return nil, err } - // The order here is preserved in discovery. - // If resources with identical names exist in more than one of these groups (e.g. "deployments.apps"" and "deployments.extensions"), - // the order of this list determines which group an unqualified resource name (e.g. "deployments") should prefer. - // This priority order is used for local discovery, but it ends up aggregated in `k8s.io/kubernetes/cmd/kube-apiserver/app/aggregator.go - // with specific priorities. - // TODO: describe the priority all the way down in the RESTStorageProviders and plumb it back through the various discovery - // handlers that we have. - restStorageProviders := []controlplaneapiserver.RESTStorageProvider{ - legacyRESTStorageProvider, - apiserverinternalrest.StorageProvider{}, - authenticationrest.RESTStorageProvider{Authenticator: c.ControlPlane.Generic.Authentication.Authenticator, APIAudiences: c.ControlPlane.Generic.Authentication.APIAudiences}, - authorizationrest.RESTStorageProvider{Authorizer: c.ControlPlane.Generic.Authorization.Authorizer, RuleResolver: c.ControlPlane.Generic.RuleResolver}, - autoscalingrest.RESTStorageProvider{}, - batchrest.RESTStorageProvider{}, - certificatesrest.RESTStorageProvider{}, - coordinationrest.RESTStorageProvider{}, - discoveryrest.StorageProvider{}, - networkingrest.RESTStorageProvider{}, - noderest.RESTStorageProvider{}, - policyrest.RESTStorageProvider{}, - rbacrest.RESTStorageProvider{Authorizer: c.ControlPlane.Generic.Authorization.Authorizer}, - schedulingrest.RESTStorageProvider{}, - storagerest.RESTStorageProvider{}, - svmrest.RESTStorageProvider{}, - flowcontrolrest.RESTStorageProvider{InformerFactory: c.ControlPlane.Generic.SharedInformerFactory}, - // keep apps after extensions so legacy clients resolve the extensions versions of shared resource names. - // See https://github.com/kubernetes/kubernetes/issues/42392 - appsrest.StorageProvider{}, - admissionregistrationrest.RESTStorageProvider{Authorizer: c.ControlPlane.Generic.Authorization.Authorizer, DiscoveryClient: discoveryClientForAdmissionRegistration}, - eventsrest.RESTStorageProvider{TTL: c.ControlPlane.EventTTL}, - resourcerest.RESTStorageProvider{}, - } if err := s.ControlPlane.InstallAPIs(restStorageProviders...); err != nil { return nil, err } @@ -426,6 +371,59 @@ func (c CompletedConfig) New(delegationTarget genericapiserver.DelegationTarget) } return s, nil + +} + +func (c CompletedConfig) StorageProviders(discovery clientdiscovery.DiscoveryInterface) ([]controlplaneapiserver.RESTStorageProvider, error) { + legacyRESTStorageProvider, err := corerest.New(corerest.Config{ + GenericConfig: *c.ControlPlane.NewCoreGenericConfig(), + Proxy: corerest.ProxyConfig{ + Transport: c.ControlPlane.Extra.ProxyTransport, + KubeletClientConfig: c.Extra.KubeletClientConfig, + }, + Services: corerest.ServicesConfig{ + ClusterIPRange: c.Extra.ServiceIPRange, + SecondaryClusterIPRange: c.Extra.SecondaryServiceIPRange, + NodePortRange: c.Extra.ServiceNodePortRange, + IPRepairInterval: c.Extra.RepairServicesInterval, + }, + }) + if err != nil { + return nil, err + } + + // The order here is preserved in discovery. + // If resources with identical names exist in more than one of these groups (e.g. "deployments.apps"" and "deployments.extensions"), + // the order of this list determines which group an unqualified resource name (e.g. "deployments") should prefer. + // This priority order is used for local discovery, but it ends up aggregated in `k8s.io/kubernetes/cmd/kube-apiserver/app/aggregator.go + // with specific priorities. + // TODO: describe the priority all the way down in the RESTStorageProviders and plumb it back through the various discovery + // handlers that we have. + return []controlplaneapiserver.RESTStorageProvider{ + legacyRESTStorageProvider, + apiserverinternalrest.StorageProvider{}, + authenticationrest.RESTStorageProvider{Authenticator: c.ControlPlane.Generic.Authentication.Authenticator, APIAudiences: c.ControlPlane.Generic.Authentication.APIAudiences}, + authorizationrest.RESTStorageProvider{Authorizer: c.ControlPlane.Generic.Authorization.Authorizer, RuleResolver: c.ControlPlane.Generic.RuleResolver}, + autoscalingrest.RESTStorageProvider{}, + batchrest.RESTStorageProvider{}, + certificatesrest.RESTStorageProvider{}, + coordinationrest.RESTStorageProvider{}, + discoveryrest.StorageProvider{}, + networkingrest.RESTStorageProvider{}, + noderest.RESTStorageProvider{}, + policyrest.RESTStorageProvider{}, + rbacrest.RESTStorageProvider{Authorizer: c.ControlPlane.Generic.Authorization.Authorizer}, + schedulingrest.RESTStorageProvider{}, + storagerest.RESTStorageProvider{}, + svmrest.RESTStorageProvider{}, + flowcontrolrest.RESTStorageProvider{InformerFactory: c.ControlPlane.Generic.SharedInformerFactory}, + // keep apps after extensions so legacy clients resolve the extensions versions of shared resource names. + // See https://github.com/kubernetes/kubernetes/issues/42392 + appsrest.StorageProvider{}, + admissionregistrationrest.RESTStorageProvider{Authorizer: c.ControlPlane.Generic.Authorization.Authorizer, DiscoveryClient: discovery}, + eventsrest.RESTStorageProvider{TTL: c.ControlPlane.EventTTL}, + resourcerest.RESTStorageProvider{}, + }, nil } var ( diff --git a/pkg/controlplane/instance_test.go b/pkg/controlplane/instance_test.go index 15d9e22e8f4..d4c415101bc 100644 --- a/pkg/controlplane/instance_test.go +++ b/pkg/controlplane/instance_test.go @@ -29,6 +29,8 @@ import ( "testing" "github.com/stretchr/testify/assert" + autoscalingrest "k8s.io/kubernetes/pkg/registry/autoscaling/rest" + resourcerest "k8s.io/kubernetes/pkg/registry/resource/rest" autoscalingapiv2beta1 "k8s.io/api/autoscaling/v2beta1" autoscalingapiv2beta2 "k8s.io/api/autoscaling/v2beta2" @@ -69,9 +71,17 @@ import ( generatedopenapi "k8s.io/kubernetes/pkg/generated/openapi" "k8s.io/kubernetes/pkg/kubeapiserver" kubeletclient "k8s.io/kubernetes/pkg/kubelet/client" + appsrest "k8s.io/kubernetes/pkg/registry/apps/rest" + batchrest "k8s.io/kubernetes/pkg/registry/batch/rest" certificatesrest "k8s.io/kubernetes/pkg/registry/certificates/rest" corerest "k8s.io/kubernetes/pkg/registry/core/rest" + discoveryrest "k8s.io/kubernetes/pkg/registry/discovery/rest" + networkingrest "k8s.io/kubernetes/pkg/registry/networking/rest" + noderest "k8s.io/kubernetes/pkg/registry/node/rest" + policyrest "k8s.io/kubernetes/pkg/registry/policy/rest" "k8s.io/kubernetes/pkg/registry/registrytest" + schedulingrest "k8s.io/kubernetes/pkg/registry/scheduling/rest" + storagerest "k8s.io/kubernetes/pkg/registry/storage/rest" ) // setUp is a convenience function for setting up for (most) tests. @@ -159,12 +169,7 @@ func TestLegacyRestStorageStrategies(t *testing.T) { defer etcdserver.Terminate(t) storageProvider, err := corerest.New(corerest.Config{ - GenericConfig: corerest.GenericConfig{ - StorageFactory: apiserverCfg.ControlPlane.Extra.StorageFactory, - EventTTL: apiserverCfg.ControlPlane.Extra.EventTTL, - LoopbackClientConfig: apiserverCfg.ControlPlane.Generic.LoopbackClientConfig, - Informers: apiserverCfg.ControlPlane.Extra.VersionedInformers, - }, + GenericConfig: *apiserverCfg.ControlPlane.NewCoreGenericConfig(), Proxy: corerest.ProxyConfig{ Transport: apiserverCfg.ControlPlane.Extra.ProxyTransport, KubeletClientConfig: apiserverCfg.Extra.KubeletClientConfig, @@ -206,15 +211,16 @@ func TestCertificatesRestStorageStrategies(t *testing.T) { } } -func newInstance(t *testing.T) (*Instance, *etcd3testing.EtcdTestServer, Config, *assert.Assertions) { +func newInstance(t *testing.T) (*Instance, *etcd3testing.EtcdTestServer, CompletedConfig, *assert.Assertions) { etcdserver, config, assert := setUp(t) - apiserver, err := config.Complete().New(genericapiserver.NewEmptyDelegate()) + completed := config.Complete() + apiserver, err := completed.New(genericapiserver.NewEmptyDelegate()) if err != nil { t.Fatalf("Error in bringing up the master: %v", err) } - return apiserver, etcdserver, config, assert + return apiserver, etcdserver, completed, assert } // TestVersion tests /version @@ -480,3 +486,67 @@ func TestNewBetaResourcesEnabledByDefault(t *testing.T) { t.Errorf("no new beta resources can be enabled by default, see https://github.com/kubernetes/enhancements/blob/0ad0fc8269165ca300d05ca51c7ce190a79976a5/keps/sig-architecture/3136-beta-apis-off-by-default/README.md: %v", gvr) } } + +// TestGenericStorageProviders is a smoke test that ensures that the kube +// storage providers and the generic storage providers don't unexpectedly +// divert, i.e. the later is an equally ordered subset. +func TestGenericStorageProviders(t *testing.T) { + _, config, _ := setUp(t) + completed := config.Complete() + + // create kube storage providers + client, err := kubernetes.NewForConfig(config.ControlPlane.Generic.LoopbackClientConfig) + if err != nil { + t.Fatal(err) + } + kube, err := completed.StorageProviders(client.Discovery()) + if err != nil { + t.Fatal(err) + } + + // create generic storage providers. These should be an equally ordered subset + generic, err := completed.ControlPlane.GenericStorageProviders(client.Discovery()) + if err != nil { + t.Fatal(err) + } + + g := 0 // generic index + for k := range kube { + kt := reflect.TypeOf(kube[k]) + var gt reflect.Type + if g < len(generic) { + gt = reflect.TypeOf(generic[g]) + } + + // special case: we identify full core and generic core + if kt.Kind() == reflect.Ptr && kt.Elem().PkgPath() == reflect.TypeOf(corerest.Config{}).PkgPath() { + kt = reflect.TypeOf(&corerest.GenericConfig{}) + } + + if kt == gt { + g++ + continue + } + + switch kube[k].(type) { + case autoscalingrest.RESTStorageProvider, + batchrest.RESTStorageProvider, + discoveryrest.StorageProvider, + networkingrest.RESTStorageProvider, + noderest.RESTStorageProvider, + policyrest.RESTStorageProvider, + schedulingrest.RESTStorageProvider, + storagerest.RESTStorageProvider, + appsrest.StorageProvider, + resourcerest.RESTStorageProvider: + // all these are non-generic, but kube specific + continue + default: + t.Errorf("Unexpected, uncategorized storage %T from %s. Put into the list above for kube-specific APIs, or into GenericStorageProviders for generic APIs", kube[k], kt.PkgPath()) + } + } + + if g != len(generic) { + t.Errorf("Unexpected, generic APIs found: %#v", generic[g:]) + } +}