Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 33 additions & 2 deletions pkg/manager/hcp_proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"},
Comment thread
coderabbitai[bot] marked this conversation as resolved.
},
{
// Alias subresource: same as GET|PUT /{name} but with an explicit /resources suffix.
Expand All @@ -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",
Expand Down Expand Up @@ -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{}{
Comment thread
kurwang marked this conversation as resolved.
"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{}
Expand Down
53 changes: 52 additions & 1 deletion pkg/manager/hcp_proxy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -317,18 +317,69 @@
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"])
}
Comment thread
kurwang marked this conversation as resolved.

// --- 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"

Check warning on line 380 in pkg/manager/hcp_proxy_test.go

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Split this 126 characters long line (which is greater than 120 authorized).

See more on https://sonarcloud.io/project/issues?id=open-cluster-management_hypershift-addon-operator&issues=AZ_IkFUqxguKkLu-QGzZ&open=AZ_IkFUqxguKkLu-QGzZ&pullRequest=773
r := httptest.NewRequest(http.MethodDelete, path, nil)
p.handleRoute(w, r)
assert.Equal(t, http.StatusBadRequest, w.Code)
}

Expand Down
18 changes: 11 additions & 7 deletions test/e2e/hcp_proxy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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))
})
})

Expand Down
Loading