Merge pull request #124658 from sttts/sttts-controlplane-storage

Step 10 – controlplane: add generic storage construction

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{}

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
+	restStorageProviders, err := c.StorageProviders(client.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,
-		},
-	})
 	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 (

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{
+		GenericConfig: *apiserverCfg.ControlPlane.NewCoreGenericConfig(),
-			StorageFactory:       apiserverCfg.ControlPlane.Extra.StorageFactory,
-			EventTTL:             apiserverCfg.ControlPlane.Extra.EventTTL,
-			LoopbackClientConfig: apiserverCfg.ControlPlane.Generic.LoopbackClientConfig,
-			Informers:            apiserverCfg.ControlPlane.Extra.VersionedInformers,
-		},
 		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:])
+	}
+}