diff --git a/pkg/manager/hcp_proxy.go b/pkg/manager/hcp_proxy.go index bc08bd7a..5abda4c0 100644 --- a/pkg/manager/hcp_proxy.go +++ b/pkg/manager/hcp_proxy.go @@ -385,7 +385,7 @@ func (p *hcpProxy) handleDiscovery(w http.ResponseWriter, r *http.Request) { "singularName": "hostedcluster", "namespaced": true, "kind": "HostedCluster", - "verbs": []string{"create", "delete", "get"}, + "verbs": []string{"create", "delete", "deletecollection", "get", "list"}, }, { // Alias subresource: same as GET|PUT /{name} but with an explicit /resources suffix. @@ -406,7 +406,22 @@ func (p *hcpProxy) handleRoute(w http.ResponseWriter, r *http.Request) { remaining := strings.TrimPrefix(r.URL.Path, prefix) parts := strings.Split(remaining, "/") - hostingCluster, err := sanitizeProxyName(r.URL.Query().Get("hostingCluster")) + hostingClusterParam := r.URL.Query().Get("hostingCluster") + + // When hostingCluster is completely absent, the Kubernetes namespace + // controller may be sending collection-level GET (list) or DELETE + // (delete-collection) requests during namespace cleanup. Return an + // empty HostedClusterList so namespace deletion is not blocked. + // POST (create) still requires a spoke target → fall through to 400. + if hostingClusterParam == "" { + isCollection := len(parts) == 3 && parts[0] == "namespaces" && parts[2] == hcpProxyResource + if isCollection && (r.Method == http.MethodGet || r.Method == http.MethodDelete) { + p.handleEmptyCollection(w, r) + return + } + } + + hostingCluster, err := sanitizeProxyName(hostingClusterParam) if err != nil { writeJSONError(w, "hostingCluster query parameter is required and must be a valid DNS-1123 subdomain", @@ -479,6 +494,22 @@ func (p *hcpProxy) dispatchNamed(w http.ResponseWriter, r *http.Request, nsRaw, } } +// handleEmptyCollection returns an empty success response for collection-level +// operations (LIST / DELETE-collection) that arrive without a hostingCluster +// query parameter. The Kubernetes namespace controller sends these during +// namespace cleanup to enumerate and remove all resources of every registered +// API type. Since the proxy does not store resources locally (it proxies to +// spoke clusters identified by hostingCluster), an empty list is correct. +func (p *hcpProxy) handleEmptyCollection(w http.ResponseWriter, r *http.Request) { + w.Header().Set(headerContentType, contentTypeJSON) + _ = json.NewEncoder(w).Encode(map[string]interface{}{ + "apiVersion": hcpProxyAPIGroup + "/" + hcpProxyAPIVersion, + "kind": "HostedClusterList", + "metadata": map[string]interface{}{"resourceVersion": ""}, + "items": []interface{}{}, + }) +} + // checkSpokeHealth verifies that the named ManagedCluster is Available. func (p *hcpProxy) checkSpokeHealth(ctx context.Context, spokeName string) error { mc := &clusterv1.ManagedCluster{} diff --git a/pkg/manager/hcp_proxy_test.go b/pkg/manager/hcp_proxy_test.go index 7ce032b1..dcf76cb5 100644 --- a/pkg/manager/hcp_proxy_test.go +++ b/pkg/manager/hcp_proxy_test.go @@ -317,18 +317,69 @@ func Test_handleDiscovery_WhenVersionPath_ItShouldReturnAPIResourceList(t *testi assert.Len(t, resources, 2) first := resources[0].(map[string]interface{}) assert.Equal(t, hcpProxyResource, first["name"]) + verbs := first["verbs"].([]interface{}) + assert.Contains(t, verbs, "list") + assert.Contains(t, verbs, "deletecollection") second := resources[1].(map[string]interface{}) assert.Equal(t, hcpProxyResource+"/resources", second["name"]) } // --- handleRoute --- -func Test_handleRoute_WhenMissingHostingCluster_ItShouldReturn400(t *testing.T) { +func Test_handleRoute_WhenMissingHostingCluster_OnNamedEndpoint_ItShouldReturn400(t *testing.T) { + p := newTestProxy(t) + w := httptest.NewRecorder() + path := "/apis/" + hcpProxyAPIGroup + "/" + hcpProxyAPIVersion + "/namespaces/clusters/hostedclusters/my-hc" + r := httptest.NewRequest(http.MethodGet, path, nil) // no ?hostingCluster + p.handleRoute(w, r) + assert.Equal(t, http.StatusBadRequest, w.Code) +} + +func Test_handleRoute_WhenMissingHostingCluster_OnCollectionGET_ItShouldReturnEmptyList(t *testing.T) { p := newTestProxy(t) w := httptest.NewRecorder() path := "/apis/" + hcpProxyAPIGroup + "/" + hcpProxyAPIVersion + "/namespaces/clusters/hostedclusters" r := httptest.NewRequest(http.MethodGet, path, nil) // no ?hostingCluster p.handleRoute(w, r) + + assert.Equal(t, http.StatusOK, w.Code) + var doc map[string]interface{} + require.NoError(t, json.Unmarshal(w.Body.Bytes(), &doc)) + assert.Equal(t, "HostedClusterList", doc["kind"]) + items := doc["items"].([]interface{}) + assert.Empty(t, items) +} + +func Test_handleRoute_WhenMissingHostingCluster_OnCollectionDELETE_ItShouldReturnEmptyList(t *testing.T) { + p := newTestProxy(t) + w := httptest.NewRecorder() + path := "/apis/" + hcpProxyAPIGroup + "/" + hcpProxyAPIVersion + "/namespaces/clusters/hostedclusters" + r := httptest.NewRequest(http.MethodDelete, path, nil) // no ?hostingCluster + p.handleRoute(w, r) + + assert.Equal(t, http.StatusOK, w.Code) + var doc map[string]interface{} + require.NoError(t, json.Unmarshal(w.Body.Bytes(), &doc)) + assert.Equal(t, "HostedClusterList", doc["kind"]) + items := doc["items"].([]interface{}) + assert.Empty(t, items) +} + +func Test_handleRoute_WhenMissingHostingCluster_OnCollectionPOST_ItShouldReturn400(t *testing.T) { + p := newTestProxy(t) + w := httptest.NewRecorder() + path := "/apis/" + hcpProxyAPIGroup + "/" + hcpProxyAPIVersion + "/namespaces/clusters/hostedclusters" + r := httptest.NewRequest(http.MethodPost, path, nil) // no ?hostingCluster + p.handleRoute(w, r) + assert.Equal(t, http.StatusBadRequest, w.Code) +} + +func Test_handleRoute_WhenInvalidHostingCluster_OnCollection_ItShouldReturn400(t *testing.T) { + p := newTestProxy(t) + w := httptest.NewRecorder() + path := "/apis/" + hcpProxyAPIGroup + "/" + hcpProxyAPIVersion + "/namespaces/clusters/hostedclusters?hostingCluster=../evil" + r := httptest.NewRequest(http.MethodDelete, path, nil) + p.handleRoute(w, r) assert.Equal(t, http.StatusBadRequest, w.Code) } diff --git a/test/e2e/hcp_proxy_test.go b/test/e2e/hcp_proxy_test.go index ae6050d0..6b1fe9b8 100644 --- a/test/e2e/hcp_proxy_test.go +++ b/test/e2e/hcp_proxy_test.go @@ -144,13 +144,17 @@ var _ = ginkgo.Describe("HCP Proxy", func() { gomega.Expect(names).To(gomega.ContainElements("hostedclusters", "hostedclusters/resources")) }) - ginkgo.It("should return 400 when hostingCluster is missing from a spoke request", func() { + ginkgo.It("should return empty list when collection GET is missing hostingCluster", func() { client := insecureHTTPClient() url := proxyURL(proxyHost, "/apis/"+hcpProxyAPIGroup+"/"+hcpProxyAPIVersion+"/namespaces/clusters/hostedclusters") resp, err := client.Get(url) // no ?hostingCluster gomega.Expect(err).ToNot(gomega.HaveOccurred()) defer resp.Body.Close() - gomega.Expect(resp.StatusCode).To(gomega.Equal(http.StatusBadRequest)) + gomega.Expect(resp.StatusCode).To(gomega.Equal(http.StatusOK)) + var doc map[string]interface{} + gomega.Expect(json.NewDecoder(resp.Body).Decode(&doc)).To(gomega.Succeed()) + gomega.Expect(doc["kind"]).To(gomega.Equal("HostedClusterList")) + gomega.Expect(doc["items"]).To(gomega.BeEmpty()) }) ginkgo.It("should return 503 when the hosting cluster does not exist", func() { @@ -368,21 +372,21 @@ var _ = ginkgo.Describe("HCP Proxy", func() { "hcp.ocm.io should appear in server API groups") }) - ginkgo.It("should return 400 via the APIService route when hostingCluster is absent", func() { + ginkgo.It("should return empty list via the APIService route when hostingCluster is absent", func() { ginkgo.By("Making raw REST call to /apis/hcp.ocm.io/v1alpha1/namespaces/clusters/hostedclusters") restClient, err := util.NewKubeClient() gomega.Expect(err).ToNot(gomega.HaveOccurred()) - // The proxy returns 400 because hostingCluster is not set; - // the kube-apiserver may wrap this as a 400 or 503. - // Either way the call should not succeed with 200. + // Collection GET without hostingCluster returns an empty + // HostedClusterList so the namespace controller can complete + // cleanup without being blocked. gomega.Eventually(func() int { var statusCode int restClient.CoreV1().RESTClient().Get(). AbsPath("/apis/hcp.ocm.io/v1alpha1/namespaces/clusters/hostedclusters"). Do(ctx).StatusCode(&statusCode) return statusCode - }, eventuallyTimeout, eventuallyInterval).ShouldNot(gomega.Equal(http.StatusOK)) + }, eventuallyTimeout, eventuallyInterval).Should(gomega.Equal(http.StatusOK)) }) })