From 14d59e2950f1043dd542b8e0abaf6636bfddd517 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. Kubernetes-commit: 2539912a2245a53f6612100a32af96dd71a2ad4f --- pkg/endpoints/apiserver_test.go | 77 ++++++++++++++++-------------- pkg/endpoints/handlers/rest.go | 6 +++ pkg/endpoints/patchhandler_test.go | 6 ++- pkg/features/kube_features.go | 2 +- 4 files changed, 52 insertions(+), 39 deletions(-) diff --git a/pkg/endpoints/apiserver_test.go b/pkg/endpoints/apiserver_test.go index f52b9303f..15786c002 100644 --- a/pkg/endpoints/apiserver_test.go +++ b/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/pkg/endpoints/handlers/rest.go b/pkg/endpoints/handlers/rest.go index 073a32109..8fb854a3c 100644 --- a/pkg/endpoints/handlers/rest.go +++ b/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/pkg/endpoints/patchhandler_test.go b/pkg/endpoints/patchhandler_test.go index d3e198f1f..43c7c8bc2 100644 --- a/pkg/endpoints/patchhandler_test.go +++ b/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/pkg/features/kube_features.go b/pkg/features/kube_features.go index a407c3492..ccb9e24ff 100644 --- a/pkg/features/kube_features.go +++ b/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}, }