Merge pull request #46112 from sttts/sttts-unversioned-to-meta

Automatic merge from submit-queue

apimachinery: move unversioned registration to metav1

Follow-up from the discussions in https://github.com/kubernetes/kubernetes/pull/43027:

We need `Status` as unversioned type which is hardcoded to `GroupVersion{Group: "", Version: "v1"}`. If the core group is not in the scheme, we miss `Status`.

Fixing https://github.com/kubernetes/kubernetes/issues/47030.

pkg/api/register.go

@@ -50,10 +50,6 @@ const GroupName = ""
 // SchemeGroupVersion is group version used to register these objects
 var SchemeGroupVersion = schema.GroupVersion{Group: GroupName, Version: runtime.APIVersionInternal}
 
-// Unversioned is group version for unversioned API objects
-// TODO: this should be v1 probably
-var Unversioned = schema.GroupVersion{Group: "", Version: "v1"}
-
 // ParameterCodec handles versioning of objects that are converted to query parameters.
 var ParameterCodec = runtime.NewParameterCodec(Scheme)
 
@@ -123,13 +119,5 @@ func addKnownTypes(scheme *runtime.Scheme) error {
 		&ConfigMapList{},
 	)
 
-	// Register Unversioned types under their own special group
-	scheme.AddUnversionedTypes(Unversioned,
-		&metav1.Status{},
-		&metav1.APIVersions{},
-		&metav1.APIGroupList{},
-		&metav1.APIGroup{},
-		&metav1.APIResourceList{},
-	)
 	return nil
 }

pkg/controller/garbagecollector/metaonly/metaonly.go

@@ -52,14 +52,14 @@ func NewMetadataCodecFactory() serializer.CodecFactory {
 		if kind.Version == runtime.APIVersionInternal {
 			continue
 		}
-		if kind == api.Unversioned.WithKind("Status") {
+		if kind == metav1.Unversioned.WithKind("Status") {
 			// this is added below as unversioned
 			continue
 		}
 		metaOnlyObject := gvkToMetadataOnlyObject(kind)
 		scheme.AddKnownTypeWithName(kind, metaOnlyObject)
 	}
-	scheme.AddUnversionedTypes(api.Unversioned, &metav1.Status{})
+	scheme.AddUnversionedTypes(metav1.Unversioned, &metav1.Status{})
 	return serializer.NewCodecFactory(scheme)
 }
 

staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/register.go

@@ -27,6 +27,10 @@ const GroupName = "meta.k8s.io"
 // SchemeGroupVersion is group version used to register these objects
 var SchemeGroupVersion = schema.GroupVersion{Group: GroupName, Version: "v1"}
 
+// Unversioned is group version for unversioned API objects
+// TODO: this should be v1 probably
+var Unversioned = schema.GroupVersion{Group: "", Version: "v1"}
+
 // WatchEventKind is name reserved for serializing watch events.
 const WatchEventKind = "WatchEvent"
 
@@ -56,6 +60,15 @@ func AddToGroupVersion(scheme *runtime.Scheme, groupVersion schema.GroupVersion)
 		Convert_versioned_Event_to_versioned_InternalEvent,
 	)
 
+	// Register Unversioned types under their own special group
+	scheme.AddUnversionedTypes(Unversioned,
+		&Status{},
+		&APIVersions{},
+		&APIGroupList{},
+		&APIGroup{},
+		&APIResourceList{},
+	)
+
 	// register manually. This usually goes through the SchemeBuilder, which we cannot use here.
 	scheme.AddGeneratedDeepCopyFuncs(GetGeneratedDeepCopyFuncs()...)
 	AddConversionFuncs(scheme)

staging/src/k8s.io/apimachinery/pkg/runtime/scheme.go

@@ -151,8 +151,8 @@ func (s *Scheme) AddUnversionedTypes(version schema.GroupVersion, types ...Objec
 		t := reflect.TypeOf(obj).Elem()
 		gvk := version.WithKind(t.Name())
 		s.unversionedTypes[t] = gvk
-		if _, ok := s.unversionedKinds[gvk.Kind]; ok {
-			panic(fmt.Sprintf("%v has already been registered as unversioned kind %q - kind name must be unique", reflect.TypeOf(t), gvk.Kind))
+		if old, ok := s.unversionedKinds[gvk.Kind]; ok && t != old {
+			panic(fmt.Sprintf("%v.%v has already been registered as unversioned kind %q - kind name must be unique", old.PkgPath(), old.Name(), gvk))
 		}
 		s.unversionedKinds[gvk.Kind] = t
 	}

staging/src/k8s.io/apimachinery/pkg/runtime/scheme_test.go

@@ -570,6 +570,7 @@ func TestAddKnownTypesIdemPotent(t *testing.T) {
 		t.Errorf("expected only one type after double registration with custom name")
 	}
 
+	s.AddUnversionedTypes(gv, &InternalSimple{})
 	s.AddUnversionedTypes(gv, &InternalSimple{})
 	if len(s.KnownTypes(gv)) != 1 {
 		t.Errorf("expected only one %v type after double registration with custom name", gv)
@@ -587,6 +588,11 @@ func TestAddKnownTypesIdemPotent(t *testing.T) {
 	}
 }
 
+// EmbeddableTypeMeta passes GetObjectKind to the type which embeds it.
+type EmbeddableTypeMeta runtime.TypeMeta
+
+func (tm *EmbeddableTypeMeta) GetObjectKind() schema.ObjectKind { return (*runtime.TypeMeta)(tm) }
+
 func TestConflictingAddKnownTypes(t *testing.T) {
 	s := runtime.NewScheme()
 	gv := schema.GroupVersion{Group: "foo", Version: "v1"}
@@ -612,7 +618,14 @@ func TestConflictingAddKnownTypes(t *testing.T) {
 				panicked <- true
 			}
 		}()
+
 		s.AddUnversionedTypes(gv, &InternalSimple{})
+
+		// redefine InternalSimple with the same name, but obviously as a different type
+		type InternalSimple struct {
+			EmbeddableTypeMeta `json:",inline"`
+			TestString         string `json:"testString"`
+		}
 		s.AddUnversionedTypes(gv, &InternalSimple{})
 		panicked <- false
 	}()

staging/src/k8s.io/client-go/pkg/api/register.go

@@ -50,10 +50,6 @@ const GroupName = ""
 // SchemeGroupVersion is group version used to register these objects
 var SchemeGroupVersion = schema.GroupVersion{Group: GroupName, Version: runtime.APIVersionInternal}
 
-// Unversioned is group version for unversioned API objects
-// TODO: this should be v1 probably
-var Unversioned = schema.GroupVersion{Group: "", Version: "v1"}
-
 // ParameterCodec handles versioning of objects that are converted to query parameters.
 var ParameterCodec = runtime.NewParameterCodec(Scheme)
 
@@ -123,13 +119,5 @@ func addKnownTypes(scheme *runtime.Scheme) error {
 		&ConfigMapList{},
 	)
 
-	// Register Unversioned types under their own special group
-	scheme.AddUnversionedTypes(Unversioned,
-		&metav1.Status{},
-		&metav1.APIVersions{},
-		&metav1.APIGroupList{},
-		&metav1.APIGroup{},
-		&metav1.APIResourceList{},
-	)
 	return nil
 }

test/integration/master/master_test.go

@@ -101,6 +101,41 @@ func TestEmptyList(t *testing.T) {
 	}
 }
 
+func TestStatus(t *testing.T) {
+	_, s, closeFn := framework.RunAMaster(nil)
+	defer closeFn()
+
+	u := s.URL + "/apis/batch/v1/namespaces/default/jobs/foo"
+	resp, err := http.Get(u)
+	if err != nil {
+		t.Fatalf("unexpected error getting %s: %v", u, err)
+	}
+	if resp.StatusCode != http.StatusNotFound {
+		t.Fatalf("got status %v instead of 404", resp.StatusCode)
+	}
+	defer resp.Body.Close()
+	data, _ := ioutil.ReadAll(resp.Body)
+	decodedData := map[string]interface{}{}
+	if err := json.Unmarshal(data, &decodedData); err != nil {
+		t.Logf("body: %s", string(data))
+		t.Fatalf("got error decoding data: %v", err)
+	}
+	t.Logf("body: %s", string(data))
+
+	if got, expected := decodedData["apiVersion"], "v1"; got != expected {
+		t.Errorf("unexpected apiVersion %q, expected %q", got, expected)
+	}
+	if got, expected := decodedData["kind"], "Status"; got != expected {
+		t.Errorf("unexpected kind %q, expected %q", got, expected)
+	}
+	if got, expected := decodedData["status"], "Failure"; got != expected {
+		t.Errorf("unexpected status %q, expected %q", got, expected)
+	}
+	if got, expected := decodedData["code"], float64(404); got != expected {
+		t.Errorf("unexpected code %v, expected %v", got, expected)
+	}
+}
+
 func TestWatchSucceedsWithoutArgs(t *testing.T) {
 	_, s, closeFn := framework.RunAMaster(nil)
 	defer closeFn()