From 2539912a2245a53f6612100a32af96dd71a2ad4f Mon Sep 17 00:00:00 2001 From: wojtekt Date: Fri, 26 Jul 2019 15:48:37 +0200 Subject: [PATCH] Stop setting SelfLink in kube-apiserver. --- .../test/integration/basic_test.go | 5 ++ .../test/integration/subresources_test.go | 12 ++- .../apiserver/pkg/endpoints/apiserver_test.go | 77 ++++++++++--------- .../apiserver/pkg/endpoints/handlers/rest.go | 6 ++ .../pkg/endpoints/patchhandler_test.go | 6 +- .../apiserver/pkg/features/kube_features.go | 2 +- test/e2e/apimachinery/table_conversion.go | 2 - .../integration/apiserver/apply/apply_test.go | 9 ++- test/integration/client/BUILD | 3 + test/integration/client/client_test.go | 5 ++ 10 files changed, 80 insertions(+), 47 deletions(-) diff --git a/staging/src/k8s.io/apiextensions-apiserver/test/integration/basic_test.go b/staging/src/k8s.io/apiextensions-apiserver/test/integration/basic_test.go index f910bf4edc5..a370c9d8a7a 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/test/integration/basic_test.go +++ b/staging/src/k8s.io/apiextensions-apiserver/test/integration/basic_test.go @@ -35,7 +35,10 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/apimachinery/pkg/watch" + "k8s.io/apiserver/pkg/features" + utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/client-go/dynamic" + featuregatetesting "k8s.io/component-base/featuregate/testing" ) func TestServerUp(t *testing.T) { @@ -624,6 +627,8 @@ func TestSameNameDiffNamespace(t *testing.T) { } func TestSelfLink(t *testing.T) { + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.RemoveSelfLink, false)() + tearDown, apiExtensionClient, dynamicClient, err := fixtures.StartDefaultServerWithClients(t) if err != nil { t.Fatal(err) diff --git a/staging/src/k8s.io/apiextensions-apiserver/test/integration/subresources_test.go b/staging/src/k8s.io/apiextensions-apiserver/test/integration/subresources_test.go index 9a465861e0b..d6e227e0b2c 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/test/integration/subresources_test.go +++ b/staging/src/k8s.io/apiextensions-apiserver/test/integration/subresources_test.go @@ -31,6 +31,8 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" + "k8s.io/apiserver/pkg/features" + utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/client-go/dynamic" apiextensionsv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" @@ -340,10 +342,12 @@ func TestScaleSubresource(t *testing.T) { t.Fatalf("Scale.Status.Selector: expected: %v, got: %v", "bar", gottenScale.Status.Selector) } - // check self link - expectedSelfLink := fmt.Sprintf("/apis/mygroup.example.com/%s/namespaces/not-the-default/noxus/foo/scale", v.Name) - if gottenScale.GetSelfLink() != expectedSelfLink { - t.Fatalf("Scale.Metadata.SelfLink: expected: %v, got: %v", expectedSelfLink, gottenScale.GetSelfLink()) + if !utilfeature.DefaultFeatureGate.Enabled(features.RemoveSelfLink) { + // check self link + expectedSelfLink := fmt.Sprintf("/apis/mygroup.example.com/%s/namespaces/not-the-default/noxus/foo/scale", v.Name) + if gottenScale.GetSelfLink() != expectedSelfLink { + t.Fatalf("Scale.Metadata.SelfLink: expected: %v, got: %v", expectedSelfLink, gottenScale.GetSelfLink()) + } } // update the scale object diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/apiserver_test.go b/staging/src/k8s.io/apiserver/pkg/endpoints/apiserver_test.go index f52b9303fa2..15786c002d0 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/apiserver_test.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/apiserver_test.go @@ -1144,9 +1144,8 @@ func TestList(t *testing.T) { t.Logf("%d: body: %s", i, string(body)) continue } - // TODO: future, restore get links - if !selfLinker.called { - t.Errorf("%d: never set self link", i) + if utilfeature.DefaultFeatureGate.Enabled(features.RemoveSelfLink) == selfLinker.called { + t.Errorf("%d: unexpected selfLinker.called: %v", i, selfLinker.called) } if !simpleStorage.namespacePresent { t.Errorf("%d: namespace not set", i) @@ -1279,9 +1278,8 @@ func TestListCompression(t *testing.T) { t.Logf("%d: body: %s", i, string(body)) continue } - // TODO: future, restore get links - if !selfLinker.called { - t.Errorf("%d: never set self link", i) + if utilfeature.DefaultFeatureGate.Enabled(features.RemoveSelfLink) == selfLinker.called { + t.Errorf("%d: unexpected selfLinker.called: %v", i, selfLinker.called) } if !simpleStorage.namespacePresent { t.Errorf("%d: namespace not set", i) @@ -1399,12 +1397,14 @@ func TestNonEmptyList(t *testing.T) { if listOut.Items[0].Other != simpleStorage.list[0].Other { t.Errorf("Unexpected data: %#v, %s", listOut.Items[0], string(body)) } - if listOut.SelfLink != "/"+prefix+"/"+testGroupVersion.Group+"/"+testGroupVersion.Version+"/simple" { - t.Errorf("unexpected list self link: %#v", listOut) - } - expectedSelfLink := "/" + prefix + "/" + testGroupVersion.Group + "/" + testGroupVersion.Version + "/namespaces/other/simple/something" - if listOut.Items[0].ObjectMeta.SelfLink != expectedSelfLink { - t.Errorf("Unexpected data: %#v, %s", listOut.Items[0].ObjectMeta.SelfLink, expectedSelfLink) + if !utilfeature.DefaultFeatureGate.Enabled(features.RemoveSelfLink) { + if listOut.SelfLink != "/"+prefix+"/"+testGroupVersion.Group+"/"+testGroupVersion.Version+"/simple" { + t.Errorf("unexpected list self link: %#v", listOut) + } + expectedSelfLink := "/" + prefix + "/" + testGroupVersion.Group + "/" + testGroupVersion.Version + "/namespaces/other/simple/something" + if listOut.Items[0].ObjectMeta.SelfLink != expectedSelfLink { + t.Errorf("Unexpected data: %#v, %s", listOut.Items[0].ObjectMeta.SelfLink, expectedSelfLink) + } } } @@ -1449,16 +1449,20 @@ func TestSelfLinkSkipsEmptyName(t *testing.T) { if listOut.Items[0].Other != simpleStorage.list[0].Other { t.Errorf("Unexpected data: %#v, %s", listOut.Items[0], string(body)) } - if listOut.SelfLink != "/"+prefix+"/"+testGroupVersion.Group+"/"+testGroupVersion.Version+"/simple" { - t.Errorf("unexpected list self link: %#v", listOut) - } - expectedSelfLink := "" - if listOut.Items[0].ObjectMeta.SelfLink != expectedSelfLink { - t.Errorf("Unexpected data: %#v, %s", listOut.Items[0].ObjectMeta.SelfLink, expectedSelfLink) + if !utilfeature.DefaultFeatureGate.Enabled(features.RemoveSelfLink) { + if listOut.SelfLink != "/"+prefix+"/"+testGroupVersion.Group+"/"+testGroupVersion.Version+"/simple" { + t.Errorf("unexpected list self link: %#v", listOut) + } + expectedSelfLink := "" + if listOut.Items[0].ObjectMeta.SelfLink != expectedSelfLink { + t.Errorf("Unexpected data: %#v, %s", listOut.Items[0].ObjectMeta.SelfLink, expectedSelfLink) + } } } func TestRootSelfLink(t *testing.T) { + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.RemoveSelfLink, false)() + storage := map[string]rest.Storage{} simpleStorage := GetWithOptionsRootRESTStorage{ SimpleTypedStorage: &SimpleTypedStorage{ @@ -1596,8 +1600,8 @@ func TestExport(t *testing.T) { t.Errorf("Expected: exported, saw: %s", itemOut.Other) } - if !selfLinker.called { - t.Errorf("Never set self link") + if utilfeature.DefaultFeatureGate.Enabled(features.RemoveSelfLink) == selfLinker.called { + t.Errorf("unexpected selfLinker.called: %v", selfLinker.called) } } @@ -1635,8 +1639,8 @@ func TestGet(t *testing.T) { if itemOut.Name != simpleStorage.item.Name { t.Errorf("Unexpected data: %#v, expected %#v (%s)", itemOut, simpleStorage.item, string(body)) } - if !selfLinker.called { - t.Errorf("Never set self link") + if utilfeature.DefaultFeatureGate.Enabled(features.RemoveSelfLink) == selfLinker.called { + t.Errorf("unexpected selfLinker.called: %v", selfLinker.called) } } @@ -1715,6 +1719,7 @@ func BenchmarkGetNoCompression(b *testing.B) { } b.StopTimer() } + func TestGetCompression(t *testing.T) { storage := map[string]rest.Storage{} simpleStorage := SimpleRESTStorage{ @@ -1781,8 +1786,8 @@ func TestGetCompression(t *testing.T) { if itemOut.Name != simpleStorage.item.Name { t.Errorf("Unexpected data: %#v, expected %#v (%s)", itemOut, simpleStorage.item, string(body)) } - if !selfLinker.called { - t.Errorf("Never set self link") + if utilfeature.DefaultFeatureGate.Enabled(features.RemoveSelfLink) == selfLinker.called { + t.Errorf("unexpected selfLinker.called: %v", selfLinker.called) } } } @@ -2762,8 +2767,8 @@ func TestGetAlternateSelfLink(t *testing.T) { if itemOut.Name != simpleStorage.item.Name { t.Errorf("Unexpected data: %#v, expected %#v (%s)", itemOut, simpleStorage.item, string(body)) } - if !selfLinker.called { - t.Errorf("Never set self link") + if utilfeature.DefaultFeatureGate.Enabled(features.RemoveSelfLink) == selfLinker.called { + t.Errorf("unexpected selfLinker.called: %v", selfLinker.called) } } @@ -2800,8 +2805,8 @@ func TestGetNamespaceSelfLink(t *testing.T) { if itemOut.Name != simpleStorage.item.Name { t.Errorf("Unexpected data: %#v, expected %#v (%s)", itemOut, simpleStorage.item, string(body)) } - if !selfLinker.called { - t.Errorf("Never set self link") + if utilfeature.DefaultFeatureGate.Enabled(features.RemoveSelfLink) == selfLinker.called { + t.Errorf("unexpected selfLinker.called: %v", selfLinker.called) } } @@ -3342,8 +3347,8 @@ func TestUpdate(t *testing.T) { if simpleStorage.updated == nil || simpleStorage.updated.Name != item.Name { t.Errorf("Unexpected update value %#v, expected %#v.", simpleStorage.updated, item) } - if !selfLinker.called { - t.Errorf("Never set self link") + if utilfeature.DefaultFeatureGate.Enabled(features.RemoveSelfLink) == selfLinker.called { + t.Errorf("unexpected selfLinker.called: %v", selfLinker.called) } } @@ -4010,8 +4015,8 @@ func TestCreate(t *testing.T) { if response.StatusCode != http.StatusCreated { t.Errorf("Unexpected status: %d, Expected: %d, %#v", response.StatusCode, http.StatusOK, response) } - if !selfLinker.called { - t.Errorf("Never set self link") + if utilfeature.DefaultFeatureGate.Enabled(features.RemoveSelfLink) == selfLinker.called { + t.Errorf("unexpected selfLinker.called: %v", selfLinker.called) } } @@ -4080,8 +4085,8 @@ func TestCreateYAML(t *testing.T) { if response.StatusCode != http.StatusCreated { t.Errorf("Unexpected status: %d, Expected: %d, %#v", response.StatusCode, http.StatusOK, response) } - if !selfLinker.called { - t.Errorf("Never set self link") + if utilfeature.DefaultFeatureGate.Enabled(features.RemoveSelfLink) == selfLinker.called { + t.Errorf("unexpected selfLinker.called: %v", selfLinker.called) } } @@ -4140,8 +4145,8 @@ func TestCreateInNamespace(t *testing.T) { if response.StatusCode != http.StatusCreated { t.Errorf("Unexpected status: %d, Expected: %d, %#v", response.StatusCode, http.StatusOK, response) } - if !selfLinker.called { - t.Errorf("Never set self link") + if utilfeature.DefaultFeatureGate.Enabled(features.RemoveSelfLink) == selfLinker.called { + t.Errorf("unexpected selfLinker.called: %v", selfLinker.called) } } diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest.go index 073a32109b7..8fb854a3c09 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest.go @@ -328,6 +328,12 @@ func checkName(obj runtime.Object, name, namespace string, namer ScopeNamer) err // interfaces func setObjectSelfLink(ctx context.Context, obj runtime.Object, req *http.Request, namer ScopeNamer) error { if utilfeature.DefaultFeatureGate.Enabled(features.RemoveSelfLink) { + // Ensure that for empty lists we don't return items. + if meta.IsListType(obj) && meta.LenList(obj) == 0 { + if err := meta.SetList(obj, []runtime.Object{}); err != nil { + return err + } + } return nil } diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/patchhandler_test.go b/staging/src/k8s.io/apiserver/pkg/endpoints/patchhandler_test.go index d3e198f1fed..43c7c8bc26b 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/patchhandler_test.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/patchhandler_test.go @@ -25,7 +25,9 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" genericapitesting "k8s.io/apiserver/pkg/endpoints/testing" + "k8s.io/apiserver/pkg/features" "k8s.io/apiserver/pkg/registry/rest" + utilfeature "k8s.io/apiserver/pkg/util/feature" ) func TestPatch(t *testing.T) { @@ -67,8 +69,8 @@ func TestPatch(t *testing.T) { if simpleStorage.updated == nil || simpleStorage.updated.Labels["foo"] != "bar" { t.Errorf("Unexpected update value %#v, expected %#v.", simpleStorage.updated, item) } - if !selfLinker.called { - t.Errorf("Never set self link") + if utilfeature.DefaultFeatureGate.Enabled(features.RemoveSelfLink) == selfLinker.called { + t.Errorf("unexpected selfLinker.called: %v", selfLinker.called) } } diff --git a/staging/src/k8s.io/apiserver/pkg/features/kube_features.go b/staging/src/k8s.io/apiserver/pkg/features/kube_features.go index a407c3492d9..ccb9e24ffae 100644 --- a/staging/src/k8s.io/apiserver/pkg/features/kube_features.go +++ b/staging/src/k8s.io/apiserver/pkg/features/kube_features.go @@ -159,7 +159,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS StorageVersionHash: {Default: true, PreRelease: featuregate.Beta}, WatchBookmark: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, APIPriorityAndFairness: {Default: false, PreRelease: featuregate.Alpha}, - RemoveSelfLink: {Default: false, PreRelease: featuregate.Alpha}, + RemoveSelfLink: {Default: true, PreRelease: featuregate.Beta}, SelectorIndex: {Default: true, PreRelease: featuregate.Beta}, WarningHeaders: {Default: true, PreRelease: featuregate.Beta}, } diff --git a/test/e2e/apimachinery/table_conversion.go b/test/e2e/apimachinery/table_conversion.go index a5f337cd56c..66dce347972 100644 --- a/test/e2e/apimachinery/table_conversion.go +++ b/test/e2e/apimachinery/table_conversion.go @@ -111,7 +111,6 @@ var _ = SIGDescribe("Servers with support for Table transformation", func() { framework.ExpectNoError(err, "failed to get pod templates in Table form in namespace: %s", ns) framework.ExpectEqual(len(pagedTable.Rows), 2) framework.ExpectNotEqual(pagedTable.ResourceVersion, "") - framework.ExpectNotEqual(pagedTable.SelfLink, "") framework.ExpectNotEqual(pagedTable.Continue, "") framework.ExpectEqual(pagedTable.Rows[0].Cells[0], "template-0000") framework.ExpectEqual(pagedTable.Rows[1].Cells[0], "template-0001") @@ -138,7 +137,6 @@ var _ = SIGDescribe("Servers with support for Table transformation", func() { framework.ExpectEqual(len(table.Rows[0].Cells), len(table.ColumnDefinitions)) framework.ExpectEqual(table.ColumnDefinitions[0].Name, "Name") framework.ExpectNotEqual(table.ResourceVersion, "") - framework.ExpectNotEqual(table.SelfLink, "") out := printTable(table) gomega.Expect(out).To(gomega.MatchRegexp("^NAME\\s")) diff --git a/test/integration/apiserver/apply/apply_test.go b/test/integration/apiserver/apply/apply_test.go index f86b87d7944..3af6f308272 100644 --- a/test/integration/apiserver/apply/apply_test.go +++ b/test/integration/apiserver/apply/apply_test.go @@ -686,11 +686,16 @@ func TestApplyManagedFields(t *testing.T) { t.Fatalf("Failed to marshal object: %v", err) } + selfLink := "" + if !utilfeature.DefaultFeatureGate.Enabled(genericfeatures.RemoveSelfLink) { + selfLink = ` + "selfLink": "` + accessor.GetSelfLink() + `",` + } + expected := []byte(`{ "metadata": { "name": "test-cm", - "namespace": "default", - "selfLink": "` + accessor.GetSelfLink() + `", + "namespace": "default",` + selfLink + ` "uid": "` + string(accessor.GetUID()) + `", "resourceVersion": "` + accessor.GetResourceVersion() + `", "creationTimestamp": "` + accessor.GetCreationTimestamp().UTC().Format(time.RFC3339) + `", diff --git a/test/integration/client/BUILD b/test/integration/client/BUILD index b237151e0ed..22c833884dd 100644 --- a/test/integration/client/BUILD +++ b/test/integration/client/BUILD @@ -30,11 +30,14 @@ go_test( "//staging/src/k8s.io/apimachinery/pkg/types:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/wait:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/watch:go_default_library", + "//staging/src/k8s.io/apiserver/pkg/features:go_default_library", + "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library", "//staging/src/k8s.io/client-go/dynamic:go_default_library", "//staging/src/k8s.io/client-go/kubernetes:go_default_library", "//staging/src/k8s.io/client-go/kubernetes/scheme:go_default_library", "//staging/src/k8s.io/client-go/transport:go_default_library", "//staging/src/k8s.io/client-go/util/cert:go_default_library", + "//staging/src/k8s.io/component-base/featuregate/testing:go_default_library", "//staging/src/k8s.io/component-base/version:go_default_library", "//test/integration/framework:go_default_library", "//test/utils:go_default_library", diff --git a/test/integration/client/client_test.go b/test/integration/client/client_test.go index 987bae8396e..8e7f562e388 100644 --- a/test/integration/client/client_test.go +++ b/test/integration/client/client_test.go @@ -36,7 +36,10 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/apimachinery/pkg/watch" + "k8s.io/apiserver/pkg/features" + utilfeature "k8s.io/apiserver/pkg/util/feature" clientset "k8s.io/client-go/kubernetes" + featuregatetesting "k8s.io/component-base/featuregate/testing" "k8s.io/component-base/version" kubeapiservertesting "k8s.io/kubernetes/cmd/kube-apiserver/app/testing" @@ -790,6 +793,8 @@ func runSelfLinkTestOnNamespace(t *testing.T, c clientset.Interface, namespace s } func TestSelfLinkOnNamespace(t *testing.T) { + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.RemoveSelfLink, false)() + result := kubeapiservertesting.StartTestServerOrDie(t, nil, []string{"--disable-admission-plugins", "ServiceAccount"}, framework.SharedEtcd()) defer result.TearDownFn()