diff --git a/cmd/kubectl-unbounded/app/net/detail_client.go b/cmd/kubectl-unbounded/app/net/detail_client.go index ae4f8abf6..c0f9342f2 100644 --- a/cmd/kubectl-unbounded/app/net/detail_client.go +++ b/cmd/kubectl-unbounded/app/net/detail_client.go @@ -9,6 +9,7 @@ import ( "fmt" "net/http" "net/url" + "strings" "time" "k8s.io/apimachinery/pkg/util/validation" @@ -186,8 +187,14 @@ func requestStatusViaAggregatedAPI(ctx context.Context, client *kubernetes.Clien return nil, fmt.Errorf("invalid controller status path %q", path) } + aggregatedPath := target.Path + if strings.HasPrefix(aggregatedPath, "/status/node/") && strings.HasSuffix(aggregatedPath, "/details") { + aggregatedPath = "/nodes/" + strings.TrimPrefix(aggregatedPath, "/status/node/") + } + request := client.CoreV1().RESTClient().Verb(method). - AbsPath("/apis/status.net.unbounded-cloud.io/v1alpha1" + target.Path) + AbsPath("/apis/status.net.unbounded-cloud.io/v1alpha1" + aggregatedPath) + for key, values := range target.Query() { for _, value := range values { request.Param(key, value) diff --git a/cmd/kubectl-unbounded/app/net/detail_client_test.go b/cmd/kubectl-unbounded/app/net/detail_client_test.go index 44d3fd78a..a068baf9d 100644 --- a/cmd/kubectl-unbounded/app/net/detail_client_test.go +++ b/cmd/kubectl-unbounded/app/net/detail_client_test.go @@ -55,7 +55,7 @@ func TestStatusRequestAggregatedTransport(t *testing.T) { for _, method := range []string{http.MethodPost, http.MethodGet} { t.Run(method, func(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.Method != method || r.URL.Path != "/apis/status.net.unbounded-cloud.io/v1alpha1/status/node/node-a/details" { + if r.Method != method || r.URL.Path != "/apis/status.net.unbounded-cloud.io/v1alpha1/nodes/node-a/details" { t.Errorf("unexpected request %s %s", r.Method, r.URL) } diff --git a/cmd/kubectl-unbounded/app/net/node_detail_command_test.go b/cmd/kubectl-unbounded/app/net/node_detail_command_test.go index 85f7a76ba..18553f9fc 100644 --- a/cmd/kubectl-unbounded/app/net/node_detail_command_test.go +++ b/cmd/kubectl-unbounded/app/net/node_detail_command_test.go @@ -43,7 +43,7 @@ func TestNodeShowRequestsNamedDetails(t *testing.T) { } _, _ = io.WriteString(w, `{"nodeSummaries":[{"name":"node-a"},{"name":"peer","siteName":"site","k8sReady":"Ready"}]}`) - case "/apis/status.net.unbounded-cloud.io/v1alpha1/status/node/node-a/details": + case "/apis/status.net.unbounded-cloud.io/v1alpha1/nodes/node-a/details": if r.Method != http.MethodPost { t.Errorf("cached show must POST once, got %s", r.Method) } @@ -113,7 +113,7 @@ func TestNodeShowFailureDoesNotFallBackToFullStatus(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { calls.Add(1) - if !strings.HasSuffix(r.URL.Path, "/node/node-a/details") || r.Method != http.MethodPost { + if !strings.HasSuffix(r.URL.Path, "/nodes/node-a/details") || r.Method != http.MethodPost { t.Errorf("unexpected fallback request: %s %s", r.Method, r.URL) } diff --git a/cmd/unbounded-net-controller/detail_aggregated_test.go b/cmd/unbounded-net-controller/detail_aggregated_test.go new file mode 100644 index 000000000..60b323f0d --- /dev/null +++ b/cmd/unbounded-net-controller/detail_aggregated_test.go @@ -0,0 +1,103 @@ +// Copyright (c) Microsoft Corporation. +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "crypto/tls" + "crypto/x509" + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" + + statusv1alpha1 "github.com/Azure/unbounded/internal/net/status/v1alpha1" + "github.com/Azure/unbounded/internal/net/webhook" +) + +func TestAggregatedDetailsUsesTrustedProxyAndSharedLifecycle(t *testing.T) { + certPEM, _, caPEM, err := webhook.GenerateClientAuthCertificateForTest("front-proxy-client") + if err != nil { + t.Fatal(err) + } + + cert, err := x509.ParseCertificate(mustParseCertPEM(t, certPEM)) + if err != nil { + t.Fatal(err) + } + + server := testWebhookServerForPush(t, caPEM) + manager := testDetailRequests(t, nodeDetailRequestHooks{}) + health := &healthState{detailRequests: manager, registerAggregatedAPIServer: true} + health.isLeader.Store(true) + + mux := http.NewServeMux() + registerStatusHandlers(mux, health, true, server, nil, nil) + + path := "/apis/status.net.unbounded-cloud.io/v1alpha1/nodes/node/details" + + send := func(method, path, body string, trusted bool) *httptest.ResponseRecorder { + t.Helper() + + request := httptest.NewRequest(method, path, strings.NewReader(body)) + request.Header.Set("X-Remote-User", "viewer") + + if trusted { + request.TLS = &tls.ConnectionState{PeerCertificates: []*x509.Certificate{cert}} + } + + recorder := httptest.NewRecorder() + mux.ServeHTTP(recorder, request) + + return recorder + } + + for _, method := range []string{http.MethodPost, http.MethodGet} { + if response := send(method, path, "{}", false); response.Code != http.StatusForbidden { + t.Fatalf("spoofed front-proxy header accepted: %d", response.Code) + } + } + + response := send(http.MethodPost, path, `{"forceRefresh":true}`, true) + if response.Code != http.StatusAccepted { + t.Fatalf("aggregated request failed: %d %s", response.Code, response.Body.String()) + } + + var pending statusv1alpha1.NodeDetailResult + if err := json.Unmarshal(response.Body.Bytes(), &pending); err != nil { + t.Fatal(err) + } + + if err := manager.Complete("node", pending.RequestID, testDetailStatus()); err != nil { + t.Fatal(err) + } + + for _, resultPath := range []string{path, "/status/node/node/details"} { + response = send(http.MethodGet, resultPath+"?requestId="+pending.RequestID, "", true) + + var result statusv1alpha1.NodeDetailResult + if err := json.Unmarshal(response.Body.Bytes(), &result); err != nil { + t.Fatal(err) + } + + if response.Code != http.StatusOK || result.State != statusv1alpha1.NodeDetailComplete || + result.RequestID != pending.RequestID || result.Details == nil || result.Details.Status.NodeInfo.Name != "node" { + t.Fatalf("paths do not share detail lifecycle: %d %+v", response.Code, result) + } + } + + if response = send(http.MethodDelete, path, "", true); response.Code != http.StatusMethodNotAllowed { + t.Fatalf("unsupported method accepted: %d", response.Code) + } + + disabledMux := http.NewServeMux() + registerStatusHandlers(disabledMux, &healthState{}, false, server, nil, nil) + + disabled := httptest.NewRecorder() + disabledMux.ServeHTTP(disabled, httptest.NewRequest(http.MethodPost, path, strings.NewReader("{}"))) + + if disabled.Code != http.StatusNotFound { + t.Fatalf("aggregated route exposed when disabled: %d", disabled.Code) + } +} diff --git a/cmd/unbounded-net-controller/detail_api.go b/cmd/unbounded-net-controller/detail_api.go index f99b9da30..d56ca7cbe 100644 --- a/cmd/unbounded-net-controller/detail_api.go +++ b/cmd/unbounded-net-controller/detail_api.go @@ -17,7 +17,7 @@ import ( ) func registerNodeDetailHandlers(mux *http.ServeMux, health *healthState, requireAuth bool, webhookServer *webhookpkg.Server, authorizer *dashboardAuthorizer, issuer *authn.TokenIssuer) { - mux.HandleFunc("/status/node/{name}/details", func(w http.ResponseWriter, r *http.Request) { + handler := func(w http.ResponseWriter, r *http.Request) { if !authorizeDashboardOrAggregated(requireAuth, issuer, authorizer, webhookServer, r) { http.Error(w, "Unauthorized", http.StatusUnauthorized) @@ -90,7 +90,20 @@ func registerNodeDetailHandlers(mux *http.ServeMux, health *healthState, require } writeNodeDetailResult(w, nodeDetailHTTPStatus(result.State), result) - }) + } + mux.HandleFunc("/status/node/{name}/details", handler) + + if health.registerAggregatedAPIServer { + mux.HandleFunc("/apis/status.net.unbounded-cloud.io/v1alpha1/nodes/{name}/details", func(w http.ResponseWriter, r *http.Request) { + if !webhookServer.IsTrustedAggregatedRequest(r) { + http.Error(w, "Forbidden", http.StatusForbidden) + + return + } + + handler(w, r) + }) + } } func nodeDetailHTTPStatus(state statusv1alpha1.NodeDetailState) int { diff --git a/deploy/net/controller/10-status-viewer.yaml.tmpl b/deploy/net/controller/10-status-viewer.yaml.tmpl index bb6393b76..c43ca862d 100644 --- a/deploy/net/controller/10-status-viewer.yaml.tmpl +++ b/deploy/net/controller/10-status-viewer.yaml.tmpl @@ -11,6 +11,9 @@ metadata: app.kubernetes.io/component: controller rbac.authorization.k8s.io/aggregate-to-view: "true" rules: + - apiGroups: ["status.net.unbounded-cloud.io"] + resources: ["nodes/details"] + verbs: ["get", "create"] - apiGroups: ["status.net.unbounded-cloud.io"] resources: ["status"] resourceNames: ["dashboard", "json"] diff --git a/docs/net/operations.md b/docs/net/operations.md index 1fb177d6c..526b78254 100644 --- a/docs/net/operations.md +++ b/docs/net/operations.md @@ -332,9 +332,11 @@ The request deadline governs pending work; snapshot expiry governs completed data. Reads never extend either lifetime. A pending response can include the failed pull's error while still waiting for a POST-delivered reply. -For aggregated access, prefix these paths with -`/apis/status.net.unbounded-cloud.io/v1alpha1`. Do not assume a raw controller -URL bypasses viewer authentication. +For aggregated detail requests, use +`/apis/status.net.unbounded-cloud.io/v1alpha1/nodes//details` with the +same methods, body, and query. This maps to the `nodes/details` subresource, +authorized by the status-viewer role without granting node publication access. +Do not assume a raw controller URL bypasses viewer authentication. #### Gateway Health diff --git a/internal/net/webhook/server.go b/internal/net/webhook/server.go index 1b7ea03e2..1ad1b336c 100644 --- a/internal/net/webhook/server.go +++ b/internal/net/webhook/server.go @@ -163,7 +163,7 @@ func (s *Server) registerAggregatedDiscoveryHandlers() { } w.Header().Set("Content-Type", "application/json") - _, _ = w.Write([]byte(`{"kind":"APIResourceList","apiVersion":"v1","groupVersion":"status.net.unbounded-cloud.io/v1alpha1","resources":[{"name":"status/push","singularName":"","namespaced":false,"kind":"NodeStatusPush","verbs":["create"]},{"name":"status/nodews","singularName":"","namespaced":false,"kind":"NodeStatusStream","verbs":["get"]},{"name":"status/json","singularName":"","namespaced":false,"kind":"ClusterStatus","verbs":["get"]},{"name":"token/node","singularName":"","namespaced":false,"kind":"TokenRequest","verbs":["create"]},{"name":"token/viewer","singularName":"","namespaced":false,"kind":"TokenRequest","verbs":["create"]}]}`)) //nolint:errcheck + _, _ = w.Write([]byte(`{"kind":"APIResourceList","apiVersion":"v1","groupVersion":"status.net.unbounded-cloud.io/v1alpha1","resources":[{"name":"status/push","singularName":"","namespaced":false,"kind":"NodeStatusPush","verbs":["create"]},{"name":"status/nodews","singularName":"","namespaced":false,"kind":"NodeStatusStream","verbs":["get"]},{"name":"status/json","singularName":"","namespaced":false,"kind":"ClusterStatus","verbs":["get"]},{"name":"nodes/details","singularName":"","namespaced":false,"kind":"NodeDetails","verbs":["get","create"]},{"name":"token/node","singularName":"","namespaced":false,"kind":"TokenRequest","verbs":["create"]},{"name":"token/viewer","singularName":"","namespaced":false,"kind":"TokenRequest","verbs":["create"]}]}`)) //nolint:errcheck }) }