Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

api server library: moving API registration logic to generic api server #19040

Merged
merged 1 commit into from
Jan 4, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 4 additions & 4 deletions api/swagger-spec/resourceListing.json
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
{
"swaggerVersion": "1.2",
"apis": [
{
"path": "/api/v1",
"description": "API at /api/v1"
},
{
"path": "/version",
"description": "git code version from which this is built"
},
{
"path": "/api/v1",
"description": "API at /api/v1"
},
{
"path": "/api",
"description": "get available API versions"
Expand Down
2 changes: 1 addition & 1 deletion pkg/apiserver/api_installer.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ func (a *APIInstaller) Install(ws *restful.WebService) (apiResources []unversion
for _, path := range paths {
apiResource, err := a.registerResourceHandlers(path, a.group.Storage[path], ws, proxyHandler)
if err != nil {
errors = append(errors, err)
errors = append(errors, fmt.Errorf("error in registering resource: %s, %v", path, err))
}
if apiResource != nil {
apiResources = append(apiResources, *apiResource)
Expand Down
126 changes: 120 additions & 6 deletions pkg/genericapiserver/genericapiserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package genericapiserver

import (
"crypto/tls"
"fmt"
"net"
"net/http"
"net/http/pprof"
Expand All @@ -27,7 +28,9 @@ import (

"k8s.io/kubernetes/pkg/admission"
"k8s.io/kubernetes/pkg/api"
"k8s.io/kubernetes/pkg/api/latest"
"k8s.io/kubernetes/pkg/api/rest"
"k8s.io/kubernetes/pkg/api/unversioned"
"k8s.io/kubernetes/pkg/apiserver"
"k8s.io/kubernetes/pkg/auth/authenticator"
"k8s.io/kubernetes/pkg/auth/authorizer"
Expand Down Expand Up @@ -128,6 +131,21 @@ type APIGroupVersionOverride struct {
ResourceOverrides map[string]bool
}

// Info about an API group.
type APIGroupInfo struct {
GroupMeta latest.GroupMeta
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fact that this type is in latest but it shows up in this class is indicative it should not be in latest - it probably belongs in the same location as GroupVersionKind (which today is unversioned, but should be a different package in the future).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. How about I move it to unversioned now and then we can move all types away from unversioned together?
Or if you have a better place in mind, I can move it there directly.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a little different from GroupVersionKind, because TypeMeta.APIVersion would be of type GroupVersionKind, so we would need to keep a declaration of GroupVersionKind in the unversioned package. GroupMeta is not part of the API, so it doesn't belong to the unversioned package.

How about do a type GroupMeta latest.GroupMeta in the genericapiserver package?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes thats right.
Adding type GroupMeta latest.GroupMeta doesnt help much.
I will try to come up with a better place for GroupMeta and move it in another PR.

// Info about the resources in this group. Its a map from version to resource to the storage.
VersionedResourcesStorageMap map[string]map[string]rest.Storage
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

map[string] implies there is no ordering in the versions, but there is. This should really be []structThatIncludesMapStringToStorage

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes there is no ordering in the versions.
GroupMeta has a preferred group version and a list of all group versions.

// True, if this is the legacy group ("/v1").
IsLegacyGroup bool
// OptionsExternalVersion controls the APIVersion used for common objects in the
// schema like api.Status, api.DeleteOptions, and api.ListOptions. Other implementors may
// define a version "v1beta1" but want to use the Kubernetes "v1" internal objects.
// If nil, defaults to groupMeta.GroupVersion.
// TODO: Remove this when https://github.com/kubernetes/kubernetes/issues/19018 is fixed.
OptionsExternalVersion *unversioned.GroupVersion
}

// Config is a structure used to configure a GenericAPIServer.
type Config struct {
StorageDestinations StorageDestinations
Expand Down Expand Up @@ -229,8 +247,8 @@ type GenericAPIServer struct {
enableSwaggerSupport bool
enableProfiling bool
enableWatchCache bool
ApiPrefix string
ApiGroupPrefix string
APIPrefix string
APIGroupPrefix string
corsAllowedOriginList []string
authenticator authenticator.Request
authorizer authorizer.Authorizer
Expand Down Expand Up @@ -351,8 +369,8 @@ func New(c *Config) *GenericAPIServer {
enableSwaggerSupport: c.EnableSwaggerSupport,
enableProfiling: c.EnableProfiling,
enableWatchCache: c.EnableWatchCache,
ApiPrefix: c.APIPrefix,
ApiGroupPrefix: c.APIGroupPrefix,
APIPrefix: c.APIPrefix,
APIGroupPrefix: c.APIGroupPrefix,
corsAllowedOriginList: c.CorsAllowedOriginList,
authenticator: c.Authenticator,
authorizer: c.Authorizer,
Expand Down Expand Up @@ -397,8 +415,8 @@ func New(c *Config) *GenericAPIServer {

func (s *GenericAPIServer) NewRequestInfoResolver() *apiserver.RequestInfoResolver {
return &apiserver.RequestInfoResolver{
sets.NewString(strings.Trim(s.ApiPrefix, "/"), strings.Trim(s.ApiGroupPrefix, "/")), // all possible API prefixes
sets.NewString(strings.Trim(s.ApiPrefix, "/")), // APIPrefixes that won't have groups (legacy)
sets.NewString(strings.Trim(s.APIPrefix, "/"), strings.Trim(s.APIGroupPrefix, "/")), // all possible API prefixes
sets.NewString(strings.Trim(s.APIPrefix, "/")), // APIPrefixes that won't have groups (legacy)
}
}

Expand Down Expand Up @@ -504,6 +522,102 @@ func (s *GenericAPIServer) init(c *Config) {
}
}

// Exposes the given group versions in API.
func (s *GenericAPIServer) InstallAPIGroups(groupsInfo []APIGroupInfo) error {
for _, apiGroupInfo := range groupsInfo {
if err := s.installAPIGroup(&apiGroupInfo); err != nil {
return err
}
}
return nil
}

func (s *GenericAPIServer) installAPIGroup(apiGroupInfo *APIGroupInfo) error {
apiPrefix := s.APIGroupPrefix
if apiGroupInfo.IsLegacyGroup {
apiPrefix = s.APIPrefix
}

// Install REST handlers for all the versions in this group.
apiVersions := []string{}
for _, groupVersion := range apiGroupInfo.GroupMeta.GroupVersions {
apiVersions = append(apiVersions, groupVersion.Version)

apiGroupVersion, err := s.getAPIGroupVersion(apiGroupInfo, groupVersion, apiPrefix)
if err != nil {
return err
}
if apiGroupInfo.OptionsExternalVersion != nil {
apiGroupVersion.OptionsExternalVersion = apiGroupInfo.OptionsExternalVersion
}

if err := apiGroupVersion.InstallREST(s.HandlerContainer); err != nil {
return fmt.Errorf("Unable to setup API %v: %v", apiGroupInfo, err)
}
}
// Install the version handler.
if apiGroupInfo.IsLegacyGroup {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Legacy group has to be parameterized to accept a prefix - because we have two legacy groups, Kube /api and OpenShift /oapi.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Never mind, I see that you parameterize s.ApiPrefix

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes.

// Add a handler at /api to enumerate the supported api versions.
apiserver.AddApiWebService(s.HandlerContainer, apiPrefix, apiVersions)
} else {
// Add a handler at /apis/<groupName> to enumerate all versions supported by this group.
apiVersionsForDiscovery := []unversioned.GroupVersionForDiscovery{}
for _, groupVersion := range apiGroupInfo.GroupMeta.GroupVersions {
apiVersionsForDiscovery = append(apiVersionsForDiscovery, unversioned.GroupVersionForDiscovery{
GroupVersion: groupVersion.String(),
Version: groupVersion.Version,
})
}
preferedVersionForDiscovery := unversioned.GroupVersionForDiscovery{
GroupVersion: apiGroupInfo.GroupMeta.GroupVersion.String(),
Version: apiGroupInfo.GroupMeta.GroupVersion.Version,
}
apiGroup := unversioned.APIGroup{
Name: apiGroupInfo.GroupMeta.GroupVersion.Group,
Versions: apiVersionsForDiscovery,
PreferredVersion: preferedVersionForDiscovery,
}
apiserver.AddGroupWebService(s.HandlerContainer, apiPrefix+"/"+apiGroup.Name, apiGroup)
}
apiserver.InstallServiceErrorHandler(s.HandlerContainer, s.NewRequestInfoResolver(), apiVersions)
return nil
}

func (s *GenericAPIServer) getAPIGroupVersion(apiGroupInfo *APIGroupInfo, groupVersion unversioned.GroupVersion, apiPrefix string) (*apiserver.APIGroupVersion, error) {
storage := make(map[string]rest.Storage)
for k, v := range apiGroupInfo.VersionedResourcesStorageMap[groupVersion.Version] {
storage[strings.ToLower(k)] = v
}
version, err := s.newAPIGroupVersion(apiGroupInfo.GroupMeta, groupVersion)
version.Root = apiPrefix
version.Storage = storage
return version, err
}

func (s *GenericAPIServer) newAPIGroupVersion(groupMeta latest.GroupMeta, groupVersion unversioned.GroupVersion) (*apiserver.APIGroupVersion, error) {
versionInterface, err := groupMeta.InterfacesFor(groupVersion)
if err != nil {
return nil, err
}
return &apiserver.APIGroupVersion{
RequestInfoResolver: s.NewRequestInfoResolver(),

Creater: api.Scheme,
Convertor: api.Scheme,
Typer: api.Scheme,

GroupVersion: groupVersion,
Linker: groupMeta.SelfLinker,
Mapper: groupMeta.RESTMapper,
Codec: versionInterface.Codec,

Admit: s.AdmissionControl,
Context: s.RequestContextMapper,

MinRequestTimeout: s.MinRequestTimeout,
}, nil
}

// InstallSwaggerAPI installs the /swaggerapi/ endpoint to allow schema discovery
// and traversal. It is optional to allow consumers of the Kubernetes GenericAPIServer to
// register their own web services into the Kubernetes mux prior to initialization
Expand Down
57 changes: 55 additions & 2 deletions pkg/genericapiserver/genericapiserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,13 @@ import (
"fmt"
"net"
"net/http"
"net/http/httptest"
"testing"

"k8s.io/kubernetes/pkg/api"
"k8s.io/kubernetes/pkg/api/latest"
"k8s.io/kubernetes/pkg/api/rest"
"k8s.io/kubernetes/pkg/apis/extensions"
"k8s.io/kubernetes/pkg/apiserver"
etcdtesting "k8s.io/kubernetes/pkg/storage/etcd/testing"
"k8s.io/kubernetes/pkg/util"
Expand Down Expand Up @@ -57,8 +62,8 @@ func TestNew(t *testing.T) {
assert.Equal(s.enableUISupport, config.EnableUISupport)
assert.Equal(s.enableSwaggerSupport, config.EnableSwaggerSupport)
assert.Equal(s.enableProfiling, config.EnableProfiling)
assert.Equal(s.ApiPrefix, config.APIPrefix)
assert.Equal(s.ApiGroupPrefix, config.APIGroupPrefix)
assert.Equal(s.APIPrefix, config.APIPrefix)
assert.Equal(s.APIGroupPrefix, config.APIGroupPrefix)
assert.Equal(s.corsAllowedOriginList, config.CorsAllowedOriginList)
assert.Equal(s.authenticator, config.Authenticator)
assert.Equal(s.authorizer, config.Authorizer)
Expand All @@ -80,6 +85,54 @@ func TestNew(t *testing.T) {
assert.Equal(s.ProxyTransport.(*http.Transport).TLSClientConfig, config.ProxyTLSClientConfig)
}

// Verifies that AddGroupVersions works as expected.
func TestInstallAPIGroups(t *testing.T) {
_, etcdserver, config, assert := setUp(t)
defer etcdserver.Terminate(t)

config.ProxyDialer = func(network, addr string) (net.Conn, error) { return nil, nil }
config.ProxyTLSClientConfig = &tls.Config{}
config.APIPrefix = "/apiPrefix"
config.APIGroupPrefix = "/apiGroupPrefix"

s := New(&config)
apiGroupMeta := latest.GroupOrDie(api.GroupName)
extensionsGroupMeta := latest.GroupOrDie(extensions.GroupName)
apiGroupsInfo := []APIGroupInfo{
{
// legacy group version
GroupMeta: *apiGroupMeta,
VersionedResourcesStorageMap: map[string]map[string]rest.Storage{},
IsLegacyGroup: true,
},
{
// extensions group version
GroupMeta: *extensionsGroupMeta,
VersionedResourcesStorageMap: map[string]map[string]rest.Storage{},
OptionsExternalVersion: &apiGroupMeta.GroupVersion,
},
}
s.InstallAPIGroups(apiGroupsInfo)

server := httptest.NewServer(s.HandlerContainer.ServeMux)
validPaths := []string{
// "/api"
config.APIPrefix,
// "/api/v1"
config.APIPrefix + "/" + apiGroupMeta.GroupVersion.Version,
// "/apis/extensions"
config.APIGroupPrefix + "/" + extensionsGroupMeta.GroupVersion.Group,
// "/apis/extensions/v1beta1"
config.APIGroupPrefix + "/" + extensionsGroupMeta.GroupVersion.String(),
}
for _, path := range validPaths {
_, err := http.Get(server.URL + path)
if !assert.NoError(err) {
t.Errorf("unexpected error: %v, for path: %s", err, path)
}
}
}

// TestNewHandlerContainer verifies that NewHandlerContainer uses the
// mux provided
func TestNewHandlerContainer(t *testing.T) {
Expand Down