Merge pull request #17225 from kubernetes/revert-17175-gv-apigv

pkg/api/unversioned/group_version.go

@@ -56,15 +56,6 @@ func ParseGroupVersion(gv string) (GroupVersion, error) {
 	}
 }
 
-func ParseGroupVersionOrDie(gv string) GroupVersion {
-	ret, err := ParseGroupVersion(gv)
-	if err != nil {
-		panic(err)
-	}
-
-	return ret
-}
-
 // MarshalJSON implements the json.Marshaller interface.
 func (gv GroupVersion) MarshalJSON() ([]byte, error) {
 	s := gv.String()

pkg/apiserver/api_installer.go

@@ -95,7 +95,7 @@ func (a *APIInstaller) NewWebService() *restful.WebService {
 	// TODO: change to restful.MIME_JSON when we set content type in client
 	ws.Consumes("*/*")
 	ws.Produces(restful.MIME_JSON)
-	ws.ApiVersion(a.group.GroupVersion.String())
+	ws.ApiVersion(a.group.Version)
 
 	return ws
 }
@@ -104,9 +104,9 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag
 	admit := a.group.Admit
 	context := a.group.Context
 
-	serverGroupVersion := a.group.GroupVersion
-	if a.group.ServerGroupVersion != nil {
-		serverGroupVersion = *a.group.ServerGroupVersion
+	serverVersion := a.group.ServerVersion
+	if len(serverVersion) == 0 {
+		serverVersion = a.group.Version
 	}
 
 	var resource, subresource string
@@ -126,13 +126,13 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag
 	if err != nil {
 		return nil, err
 	}
-	versionedPtr, err := a.group.Creater.New(a.group.GroupVersion.String(), kind)
+	versionedPtr, err := a.group.Creater.New(a.group.Version, kind)
 	if err != nil {
 		return nil, err
 	}
 	versionedObject := indirectArbitraryPointer(versionedPtr)
 
-	mapping, err := a.group.Mapper.RESTMapping(kind, a.group.GroupVersion.String())
+	mapping, err := a.group.Mapper.RESTMapping(kind, a.group.Version)
 	if err != nil {
 		return nil, err
 	}
@@ -148,7 +148,7 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag
 		if err != nil {
 			return nil, err
 		}
-		parentMapping, err := a.group.Mapper.RESTMapping(parentKind, a.group.GroupVersion.String())
+		parentMapping, err := a.group.Mapper.RESTMapping(parentKind, a.group.Version)
 		if err != nil {
 			return nil, err
 		}
@@ -181,14 +181,14 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag
 	if isLister {
 		list := lister.NewList()
 		_, listKind, err := a.group.Typer.ObjectVersionAndKind(list)
-		versionedListPtr, err := a.group.Creater.New(a.group.GroupVersion.String(), listKind)
+		versionedListPtr, err := a.group.Creater.New(a.group.Version, listKind)
 		if err != nil {
 			return nil, err
 		}
 		versionedList = indirectArbitraryPointer(versionedListPtr)
 	}
 
-	versionedListOptions, err := a.group.Creater.New(serverGroupVersion.String(), "ListOptions")
+	versionedListOptions, err := a.group.Creater.New(serverVersion, "ListOptions")
 	if err != nil {
 		return nil, err
 	}
@@ -196,7 +196,7 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag
 	var versionedDeleterObject interface{}
 	switch {
 	case isGracefulDeleter:
-		objectPtr, err := a.group.Creater.New(serverGroupVersion.String(), "DeleteOptions")
+		objectPtr, err := a.group.Creater.New(serverVersion, "DeleteOptions")
 		if err != nil {
 			return nil, err
 		}
@@ -206,7 +206,7 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag
 		gracefulDeleter = rest.GracefulDeleteAdapter{Deleter: deleter}
 	}
 
-	versionedStatusPtr, err := a.group.Creater.New(serverGroupVersion.String(), "Status")
+	versionedStatusPtr, err := a.group.Creater.New(serverVersion, "Status")
 	if err != nil {
 		return nil, err
 	}
@@ -224,7 +224,7 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag
 		if err != nil {
 			return nil, err
 		}
-		versionedGetOptions, err = a.group.Creater.New(serverGroupVersion.String(), getOptionsKind)
+		versionedGetOptions, err = a.group.Creater.New(serverVersion, getOptionsKind)
 		if err != nil {
 			return nil, err
 		}
@@ -245,7 +245,7 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag
 			if err != nil {
 				return nil, err
 			}
-			versionedConnectOptions, err = a.group.Creater.New(serverGroupVersion.String(), connectOptionsKind)
+			versionedConnectOptions, err = a.group.Creater.New(serverVersion, connectOptionsKind)
 		}
 	}
 
@@ -379,8 +379,8 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag
 		Creater:          a.group.Creater,
 		Convertor:        a.group.Convertor,
 		Codec:            mapping.Codec,
-		APIVersion:       a.group.GroupVersion.String(),
-		ServerAPIVersion: serverGroupVersion.String(),
+		APIVersion:       a.group.Version,
+		ServerAPIVersion: serverVersion,
 		Resource:         resource,
 		Subresource:      subresource,
 		Kind:             kind,

pkg/apiserver/apiserver.go

@@ -79,8 +79,9 @@ type Mux interface {
 type APIGroupVersion struct {
 	Storage map[string]rest.Storage
 
-	Root         string
-	GroupVersion unversioned.GroupVersion
+	Root string
+	// TODO: caesarxuchao: Version actually contains "group/version", refactor it to avoid confusion.
+	Version string
 
 	// RequestInfoResolver is used to parse URLs for the legacy proxy handler.  Don't use this for anything else
 	// TODO: refactor proxy handler to use sub resources
@@ -90,8 +91,9 @@ type APIGroupVersion struct {
 	// 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
 	// empty, defaults to Version.
-	// TODO this seems suspicious.  Is this actually just "unversioned" now?
-	ServerGroupVersion *unversioned.GroupVersion
+	// TODO: caesarxuchao: ServerVersion actually contains "group/version",
+	// refactor it to avoid confusion.
+	ServerVersion string
 
 	Mapper meta.RESTMapper
 
@@ -124,7 +126,8 @@ func (g *APIGroupVersion) InstallREST(container *restful.Container) error {
 	installer := g.newInstaller()
 	ws := installer.NewWebService()
 	apiResources, registrationErrors := installer.Install(ws)
-	AddSupportedResourcesWebService(ws, g.GroupVersion, apiResources)
+	// TODO: g.Version only contains "version" now, it will contain "group/version" in the near future.
+	AddSupportedResourcesWebService(ws, g.Version, apiResources)
 	container.Add(ws)
 	return utilerrors.NewAggregate(registrationErrors)
 }
@@ -148,13 +151,14 @@ func (g *APIGroupVersion) UpdateREST(container *restful.Container) error {
 		return apierrors.NewInternalError(fmt.Errorf("unable to find an existing webservice for prefix %s", installer.prefix))
 	}
 	apiResources, registrationErrors := installer.Install(ws)
-	AddSupportedResourcesWebService(ws, g.GroupVersion, apiResources)
+	// TODO: g.Version only contains "version" now, it will contain "group/version" in the near future.
+	AddSupportedResourcesWebService(ws, g.Version, apiResources)
 	return utilerrors.NewAggregate(registrationErrors)
 }
 
 // newInstaller is a helper to create the installer.  Used by InstallREST and UpdateREST.
 func (g *APIGroupVersion) newInstaller() *APIInstaller {
-	prefix := path.Join(g.Root, g.GroupVersion.Group, g.GroupVersion.Version)
+	prefix := path.Join(g.Root, g.Version)
 	installer := &APIInstaller{
 		group:             g,
 		info:              g.RequestInfoResolver,
@@ -283,7 +287,7 @@ func AddGroupWebService(container *restful.Container, path string, group unversi
 
 // Adds a service to return the supported resources, E.g., a such web service
 // will be registered at /apis/extensions/v1.
-func AddSupportedResourcesWebService(ws *restful.WebService, groupVersion unversioned.GroupVersion, apiResources []unversioned.APIResource) {
+func AddSupportedResourcesWebService(ws *restful.WebService, groupVersion string, apiResources []unversioned.APIResource) {
 	resourceHandler := SupportedResourcesHandler(groupVersion, apiResources)
 	ws.Route(ws.GET("/").To(resourceHandler).
 		Doc("get available resources").
@@ -324,10 +328,10 @@ func GroupHandler(group unversioned.APIGroup) restful.RouteFunction {
 }
 
 // SupportedResourcesHandler returns a handler which will list the provided resources as available.
-func SupportedResourcesHandler(groupVersion unversioned.GroupVersion, apiResources []unversioned.APIResource) restful.RouteFunction {
+func SupportedResourcesHandler(groupVersion string, apiResources []unversioned.APIResource) restful.RouteFunction {
 	return func(req *restful.Request, resp *restful.Response) {
 		// TODO: use restful's Response methods
-		writeJSON(http.StatusOK, api.Codec, &unversioned.APIResourceList{GroupVersion: groupVersion.String(), APIResources: apiResources}, resp.ResponseWriter, true)
+		writeJSON(http.StatusOK, api.Codec, &unversioned.APIResourceList{GroupVersion: groupVersion, APIResources: apiResources}, resp.ResponseWriter, true)
 	}
 }
 

pkg/apiserver/proxy_test.go

@@ -95,7 +95,7 @@ func TestProxy(t *testing.T) {
 			server           *httptest.Server
 			proxyTestPattern string
 		}{
-			{namespaceServer, "/" + prefix + "/" + newGroupVersion.Group + "/" + newGroupVersion.Version + "/proxy/namespaces/" + item.reqNamespace + "/foo/id" + item.path},
+			{namespaceServer, "/api/version2/proxy/namespaces/" + item.reqNamespace + "/foo/id" + item.path},
 		}
 
 		for _, serverPattern := range serverPatterns {
@@ -212,7 +212,7 @@ func TestProxyUpgrade(t *testing.T) {
 		server := httptest.NewServer(namespaceHandler)
 		defer server.Close()
 
-		ws, err := websocket.Dial("ws://"+server.Listener.Addr().String()+"/"+prefix+"/"+newGroupVersion.Group+"/"+newGroupVersion.Version+"/proxy/namespaces/myns/foo/123", "", "http://127.0.0.1/")
+		ws, err := websocket.Dial("ws://"+server.Listener.Addr().String()+"/api/version2/proxy/namespaces/myns/foo/123", "", "http://127.0.0.1/")
 		if err != nil {
 			t.Errorf("%s: websocket dial err: %s", k, err)
 			continue
@@ -276,7 +276,7 @@ func TestRedirectOnMissingTrailingSlash(t *testing.T) {
 		server := httptest.NewServer(handler)
 		defer server.Close()
 
-		proxyTestPattern := "/" + prefix + "/" + newGroupVersion.Group + "/" + newGroupVersion.Version + "/proxy/namespaces/ns/foo/id" + item.path
+		proxyTestPattern := "/api/version2/proxy/namespaces/ns/foo/id" + item.path
 		req, err := http.NewRequest(
 			"GET",
 			server.URL+proxyTestPattern+"?"+item.query,

pkg/apiserver/watch_test.go

@@ -62,7 +62,7 @@ func TestWatchWebsocket(t *testing.T) {
 
 	dest, _ := url.Parse(server.URL)
 	dest.Scheme = "ws" // Required by websocket, though the server never sees it.
-	dest.Path = "/" + prefix + "/" + testGroupVersion.Group + "/" + testGroupVersion.Version + "/watch/simples"
+	dest.Path = "/api/version/watch/simples"
 	dest.RawQuery = ""
 
 	ws, err := websocket.Dial(dest.String(), "", "http://localhost")
@@ -114,7 +114,7 @@ func TestWatchHTTP(t *testing.T) {
 	client := http.Client{}
 
 	dest, _ := url.Parse(server.URL)
-	dest.Path = "/" + prefix + "/" + testGroupVersion.Group + "/" + testGroupVersion.Version + "/watch/simples"
+	dest.Path = "/api/version/watch/simples"
 	dest.RawQuery = ""
 
 	request, err := http.NewRequest("GET", dest.String(), nil)
@@ -178,8 +178,8 @@ func TestWatchParamParsing(t *testing.T) {
 
 	dest, _ := url.Parse(server.URL)
 
-	rootPath := "/" + prefix + "/" + testVersion + "/watch/simples"
-	namespacedPath := "/" + prefix + "/" + testVersion + "/watch/namespaces/other/simpleroots"
+	rootPath := "/api/" + testVersion + "/watch/simples"
+	namespacedPath := "/api/" + testVersion + "/watch/namespaces/other/simpleroots"
 
 	table := []struct {
 		path            string
@@ -286,7 +286,7 @@ func TestWatchProtocolSelection(t *testing.T) {
 	client := http.Client{}
 
 	dest, _ := url.Parse(server.URL)
-	dest.Path = "/" + prefix + "/" + testGroupVersion.Group + "/" + testGroupVersion.Version + "/watch/simples"
+	dest.Path = "/api/version/watch/simples"
 	dest.RawQuery = ""
 
 	table := []struct {
@@ -358,7 +358,7 @@ func TestWatchHTTPTimeout(t *testing.T) {
 
 	// Setup a client
 	dest, _ := url.Parse(s.URL)
-	dest.Path = "/" + prefix + "/" + newVersion + "/simple"
+	dest.Path = "/api/" + newVersion + "/simple"
 	dest.RawQuery = "watch=true"
 
 	req, _ := http.NewRequest("GET", dest.String(), nil)

pkg/master/master.go

@@ -670,8 +670,8 @@ func (m *Master) init(c *Config) {
 		}
 		expAPIVersions := []unversioned.GroupVersionForDiscovery{
 			{
-				GroupVersion: expVersion.GroupVersion.String(),
-				Version:      expVersion.GroupVersion.Version,
+				GroupVersion: expVersion.Version,
+				Version:      apiutil.GetVersion(expVersion.Version),
 			},
 		}
 		storageVersion, found := c.StorageVersions[g.Group]
@@ -685,7 +685,7 @@ func (m *Master) init(c *Config) {
 		}
 		apiserver.AddGroupWebService(m.handlerContainer, c.APIGroupPrefix+"/"+latest.GroupOrDie("extensions").Group, group)
 		allGroups = append(allGroups, group)
-		apiserver.InstallServiceErrorHandler(m.handlerContainer, m.newRequestInfoResolver(), []string{expVersion.GroupVersion.String()})
+		apiserver.InstallServiceErrorHandler(m.handlerContainer, m.newRequestInfoResolver(), []string{expVersion.Version})
 	}
 
 	// This should be done after all groups are registered
@@ -892,7 +892,7 @@ func (m *Master) api_v1() *apiserver.APIGroupVersion {
 	}
 	version := m.defaultAPIGroupVersion()
 	version.Storage = storage
-	version.GroupVersion = unversioned.GroupVersion{Version: "v1"}
+	version.Version = "v1"
 	version.Codec = v1.Codec
 	return version
 }
@@ -1005,7 +1005,7 @@ func (m *Master) InstallThirdPartyResource(rsrc *expapi.ThirdPartyResource) erro
 	}
 	apiserver.AddGroupWebService(m.handlerContainer, path, apiGroup)
 	m.addThirdPartyResourceStorage(path, thirdparty.Storage[strings.ToLower(kind)+"s"].(*thirdpartyresourcedataetcd.REST))
-	apiserver.InstallServiceErrorHandler(m.handlerContainer, m.newRequestInfoResolver(), []string{thirdparty.GroupVersion.String()})
+	apiserver.InstallServiceErrorHandler(m.handlerContainer, m.newRequestInfoResolver(), []string{thirdparty.Version})
 	return nil
 }
 
@@ -1018,22 +1018,20 @@ func (m *Master) thirdpartyapi(group, kind, version string) *apiserver.APIGroupV
 		strings.ToLower(kind) + "s": resourceStorage,
 	}
 
-	serverGroupVersion := unversioned.ParseGroupVersionOrDie(latest.GroupOrDie("").GroupVersion)
-
 	return &apiserver.APIGroupVersion{
 		Root:                apiRoot,
-		GroupVersion:        unversioned.GroupVersion{Group: group, Version: version},
+		Version:             apiutil.GetGroupVersion(group, version),
 		RequestInfoResolver: m.newRequestInfoResolver(),
 
 		Creater:   thirdpartyresourcedata.NewObjectCreator(group, version, api.Scheme),
 		Convertor: api.Scheme,
 		Typer:     api.Scheme,
 
-		Mapper:             thirdpartyresourcedata.NewMapper(latest.GroupOrDie("extensions").RESTMapper, kind, version, group),
-		Codec:              thirdpartyresourcedata.NewCodec(latest.GroupOrDie("extensions").Codec, kind),
-		Linker:             latest.GroupOrDie("extensions").SelfLinker,
-		Storage:            storage,
-		ServerGroupVersion: &serverGroupVersion,
+		Mapper:        thirdpartyresourcedata.NewMapper(latest.GroupOrDie("extensions").RESTMapper, kind, version, group),
+		Codec:         thirdpartyresourcedata.NewCodec(latest.GroupOrDie("extensions").Codec, kind),
+		Linker:        latest.GroupOrDie("extensions").SelfLinker,
+		Storage:       storage,
+		ServerVersion: latest.GroupOrDie("").GroupVersion,
 
 		Context: m.requestContextMapper,
 
@@ -1108,7 +1106,6 @@ func (m *Master) experimental(c *Config) *apiserver.APIGroupVersion {
 	}
 
 	extensionsGroup := latest.GroupOrDie("extensions")
-	serverGroupVersion := unversioned.ParseGroupVersionOrDie(latest.GroupOrDie("").GroupVersion)
 
 	return &apiserver.APIGroupVersion{
 		Root:                m.apiGroupPrefix,
@@ -1118,12 +1115,12 @@ func (m *Master) experimental(c *Config) *apiserver.APIGroupVersion {
 		Convertor: api.Scheme,
 		Typer:     api.Scheme,
 
-		Mapper:             extensionsGroup.RESTMapper,
-		Codec:              extensionsGroup.Codec,
-		Linker:             extensionsGroup.SelfLinker,
-		Storage:            storage,
-		GroupVersion:       unversioned.ParseGroupVersionOrDie(extensionsGroup.GroupVersion),
-		ServerGroupVersion: &serverGroupVersion,
+		Mapper:        extensionsGroup.RESTMapper,
+		Codec:         extensionsGroup.Codec,
+		Linker:        extensionsGroup.SelfLinker,
+		Storage:       storage,
+		Version:       extensionsGroup.GroupVersion,
+		ServerVersion: latest.GroupOrDie("").GroupVersion,
 
 		Admit:   m.admissionControl,
 		Context: m.requestContextMapper,

pkg/master/master_test.go

@@ -175,7 +175,7 @@ func TestFindExternalAddress(t *testing.T) {
 func TestApi_v1(t *testing.T) {
 	master, _, assert := setUp(t)
 	version := master.api_v1()
-	assert.Equal(unversioned.GroupVersion{Version: "v1"}, version.GroupVersion, "Version was not v1: %s", version.GroupVersion)
+	assert.Equal("v1", version.Version, "Version was not v1: %s", version.Version)
 	assert.Equal(v1.Codec, version.Codec, "version.Codec was not for v1: %s", version.Codec)
 	for k, v := range master.storage {
 		assert.Contains(version.Storage, v, "Value %s not found (key: %s)", k, v)
@@ -322,14 +322,12 @@ func TestDefaultAPIGroupVersion(t *testing.T) {
 func TestExpapi(t *testing.T) {
 	master, config, assert := setUp(t)
 
-	extensionsGroupMeta := latest.GroupOrDie("extensions")
-
 	expAPIGroup := master.experimental(&config)
 	assert.Equal(expAPIGroup.Root, master.apiGroupPrefix)
-	assert.Equal(expAPIGroup.Mapper, extensionsGroupMeta.RESTMapper)
-	assert.Equal(expAPIGroup.Codec, extensionsGroupMeta.Codec)
-	assert.Equal(expAPIGroup.Linker, extensionsGroupMeta.SelfLinker)
-	assert.Equal(expAPIGroup.GroupVersion, unversioned.GroupVersion{Group: extensionsGroupMeta.Group, Version: extensionsGroupMeta.Version})
+	assert.Equal(expAPIGroup.Mapper, latest.GroupOrDie("extensions").RESTMapper)
+	assert.Equal(expAPIGroup.Codec, latest.GroupOrDie("extensions").Codec)
+	assert.Equal(expAPIGroup.Linker, latest.GroupOrDie("extensions").SelfLinker)
+	assert.Equal(expAPIGroup.Version, latest.GroupOrDie("extensions").GroupVersion)
 }
 
 // TestGetNodeAddresses verifies that proper results are returned