diff --git a/.github/scripts/free-disk-space.sh b/.github/scripts/free-disk-space.sh new file mode 100755 index 000000000..eedc5159d --- /dev/null +++ b/.github/scripts/free-disk-space.sh @@ -0,0 +1,23 @@ +#!/usr/bin/env bash +# Reclaim disk on GitHub-hosted Ubuntu runners before heavy image builds. +set -euo pipefail + +echo "::group::Disk before" +df -h / +echo "::endgroup::" + +sudo rm -rf \ + /usr/share/dotnet \ + /usr/local/lib/android \ + /opt/ghc \ + /opt/hostedtoolcache/CodeQL \ + /usr/local/share/boost \ + /usr/share/swift \ + /usr/local/.ghcup || true + +sudo docker image prune -af || true +sudo docker builder prune -af || true + +echo "::group::Disk after" +df -h / +echo "::endgroup::" diff --git a/.github/workflows/e2e.yml b/.github/workflows/e2e.yml index e1766f2ba..b50fd227b 100644 --- a/.github/workflows/e2e.yml +++ b/.github/workflows/e2e.yml @@ -32,6 +32,8 @@ jobs: if: ${{ env.ACT }} # this step only runs locally when using the https://github.com/nektos/act to debug the e2e - name: Checkout uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + - name: Free disk space + run: bash .github/scripts/free-disk-space.sh - name: Setup Go uses: actions/setup-go@924ae3a1cded613372ab5595356fb5720e22ba16 # v6 with: @@ -57,6 +59,8 @@ jobs: if: ${{ env.ACT }} # this step only runs locally when using the https://github.com/nektos/act to debug the e2e - name: Checkout uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + - name: Free disk space + run: bash .github/scripts/free-disk-space.sh - name: Setup Go uses: actions/setup-go@924ae3a1cded613372ab5595356fb5720e22ba16 # v6 with: @@ -82,6 +86,8 @@ jobs: if: ${{ env.ACT }} # this step only runs locally when using the https://github.com/nektos/act to debug the e2e - name: Checkout uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + - name: Free disk space + run: bash .github/scripts/free-disk-space.sh - name: Setup Go uses: actions/setup-go@924ae3a1cded613372ab5595356fb5720e22ba16 # v6 with: @@ -107,6 +113,8 @@ jobs: if: ${{ env.ACT }} # this step only runs locally when using the https://github.com/nektos/act to debug the e2e - name: Checkout uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + - name: Free disk space + run: bash .github/scripts/free-disk-space.sh - name: Setup Go uses: actions/setup-go@924ae3a1cded613372ab5595356fb5720e22ba16 # v6 with: diff --git a/pkg/placement/debugger/debugger.go b/pkg/placement/debugger/debugger.go index 627989940..baf52097d 100644 --- a/pkg/placement/debugger/debugger.go +++ b/pkg/placement/debugger/debugger.go @@ -80,16 +80,15 @@ func (d *Debugger) Handler(w http.ResponseWriter, r *http.Request) { var placement *clusterv1beta1.Placement var err error - // Support both GET (fetch from API) and POST (accept JSON body) - if r.Method == http.MethodPost { + switch r.Method { + case http.MethodPost: // POST: Parse Placement from request body placement, err = d.parsePlacementFromBody(r) if err != nil { d.reportErr(w, http.StatusBadRequest, err) return } - } else { - // GET: Fetch Placement from API (original behavior) + case http.MethodGet: namespace, name, err := d.parsePath(r.URL.Path) if err != nil { d.reportErr(w, http.StatusBadRequest, err) @@ -109,9 +108,12 @@ func (d *Debugger) Handler(w http.ResponseWriter, r *http.Request) { } return } + default: + d.reportErr(w, http.StatusMethodNotAllowed, fmt.Errorf("method %s not allowed", r.Method)) + return } - // Check if user has permission to create placements in this namespace + // Check if user has permission to access placements in this namespace if err := d.checkPermission(r, placement.Namespace); err != nil { d.reportPermissionErr(w, err) return @@ -209,11 +211,24 @@ func (d *Debugger) reportPermissionErr(w http.ResponseWriter, err error) { statusCode = http.StatusUnauthorized case strings.Contains(msg, "does not have permission"): statusCode = http.StatusForbidden + case strings.Contains(msg, "unsupported method"): + statusCode = http.StatusMethodNotAllowed } d.reportErr(w, statusCode, err) } -// checkPermission checks if the user has permission to create placements in the namespace using SAR +func placementPermissionVerb(r *http.Request) (string, error) { + switch r.Method { + case http.MethodPost: + return "create", nil + case http.MethodGet: + return "get", nil + default: + return "", fmt.Errorf("unsupported method %s", r.Method) + } +} + +// checkPermission checks if the user has permission to access placements in the namespace using SAR func (d *Debugger) checkPermission(r *http.Request, namespace string) error { // Get user from request context (authenticated by GenericAPIServer) ctx := r.Context() @@ -229,7 +244,12 @@ func (d *Debugger) checkPermission(r *http.Request, namespace string) error { extra[k] = authorizationv1.ExtraValue(v) } - // Create SubjectAccessReview to check if user can create placements + verb, err := placementPermissionVerb(r) + if err != nil { + return err + } + + // Create SubjectAccessReview to check if user can access placements sar := &authorizationv1.SubjectAccessReview{ Spec: authorizationv1.SubjectAccessReviewSpec{ User: username, @@ -237,7 +257,7 @@ func (d *Debugger) checkPermission(r *http.Request, namespace string) error { Extra: extra, ResourceAttributes: &authorizationv1.ResourceAttributes{ Namespace: namespace, - Verb: "create", + Verb: verb, Group: "cluster.open-cluster-management.io", Version: "v1beta1", Resource: "placements", @@ -254,8 +274,8 @@ func (d *Debugger) checkPermission(r *http.Request, namespace string) error { } if !result.Status.Allowed { - return fmt.Errorf("user does not have permission to create placements in namespace %s: %s", - namespace, result.Status.Reason) + return fmt.Errorf("user does not have permission to %s placements in namespace %s: %s", + verb, namespace, result.Status.Reason) } return nil diff --git a/pkg/placement/debugger/debugger_test.go b/pkg/placement/debugger/debugger_test.go index 5d1ec58d4..32687fe61 100644 --- a/pkg/placement/debugger/debugger_test.go +++ b/pkg/placement/debugger/debugger_test.go @@ -230,10 +230,20 @@ func TestReportPermissionErr(t *testing.T) { expectStatus: http.StatusUnauthorized, }, { - name: "forbidden", + name: "forbidden create", err: fmt.Errorf("user does not have permission to create placements in namespace test: denied"), expectStatus: http.StatusForbidden, }, + { + name: "forbidden get", + err: fmt.Errorf("user does not have permission to get placements in namespace test: denied"), + expectStatus: http.StatusForbidden, + }, + { + name: "unsupported method", + err: fmt.Errorf("unsupported method DELETE"), + expectStatus: http.StatusMethodNotAllowed, + }, { name: "internal", err: fmt.Errorf("failed to check permissions: boom"), @@ -301,6 +311,27 @@ func TestDebuggerHandlerReportErr(t *testing.T) { expectStatus: http.StatusNotFound, errorContains: "does-not-exist", }, + { + name: "DELETE method not allowed", + method: http.MethodDelete, + pathSuffix: placementNamespace + "/" + placementName, + expectStatus: http.StatusMethodNotAllowed, + errorContains: "method DELETE not allowed", + }, + { + name: "PUT method not allowed", + method: http.MethodPut, + pathSuffix: placementNamespace + "/" + placementName, + expectStatus: http.StatusMethodNotAllowed, + errorContains: "method PUT not allowed", + }, + { + name: "PATCH method not allowed", + method: http.MethodPatch, + pathSuffix: placementNamespace + "/" + placementName, + expectStatus: http.StatusMethodNotAllowed, + errorContains: "method PATCH not allowed", + }, { name: "GET placement lister failure", initObjs: validObjs, @@ -769,6 +800,8 @@ func TestDebuggerPermissionCheck(t *testing.T) { cases := []struct { name string + method string + body []byte injectUser bool userInfo *user.DefaultInfo sarAllowed bool @@ -793,12 +826,33 @@ func TestDebuggerPermissionCheck(t *testing.T) { expectStatusCode: http.StatusOK, }, { - name: "User without permission - should reject", + name: "GET user without permission - should reject with get verb", + method: http.MethodGet, injectUser: true, sarAllowed: false, expectError: true, expectStatusCode: http.StatusForbidden, - errorContains: "does not have permission", + errorContains: "does not have permission to get placements", + verifySARRequest: func(t *testing.T, sar *authorizationv1.SubjectAccessReview) { + if sar.Spec.ResourceAttributes.Verb != "get" { + t.Errorf("Expected SAR verb get, got %s", sar.Spec.ResourceAttributes.Verb) + } + }, + }, + { + name: "POST user without permission - should reject with create verb", + method: http.MethodPost, + body: []byte(`{"apiVersion":"cluster.open-cluster-management.io/v1beta1","kind":"Placement","metadata":{"name":"test-placement","namespace":"test-ns"},"spec":{"numberOfClusters":1}}`), + injectUser: true, + sarAllowed: false, + expectError: true, + expectStatusCode: http.StatusForbidden, + errorContains: "does not have permission to create placements", + verifySARRequest: func(t *testing.T, sar *authorizationv1.SubjectAccessReview) { + if sar.Spec.ResourceAttributes.Verb != "create" { + t.Errorf("Expected SAR verb create, got %s", sar.Spec.ResourceAttributes.Verb) + } + }, }, { name: "SAR API call fails", @@ -899,8 +953,31 @@ func TestDebuggerPermissionCheck(t *testing.T) { defer server.Close() // Send request - url := fmt.Sprintf("%s%s%s/%s", server.URL, DebugPath, placementNamespace, placementName) - res, err := http.Get(url) + method := c.method + if method == "" { + method = http.MethodGet + } + + var url string + var body io.Reader + if method == http.MethodPost { + url = fmt.Sprintf("%s%s", server.URL, DebugPath) + if len(c.body) > 0 { + body = bytes.NewBuffer(c.body) + } + } else { + url = fmt.Sprintf("%s%s%s/%s", server.URL, DebugPath, placementNamespace, placementName) + } + + req, err := http.NewRequest(method, url, body) + if err != nil { + t.Fatalf("Failed to create request: %v", err) + } + if method == http.MethodPost { + req.Header.Set("Content-Type", "application/json") + } + + res, err := http.DefaultClient.Do(req) if err != nil { t.Fatalf("Unexpected error: %v", err) } diff --git a/test/integration/placement/debugger_test.go b/test/integration/placement/debugger_test.go index d97c1fb3a..0c9e6db1f 100644 --- a/test/integration/placement/debugger_test.go +++ b/test/integration/placement/debugger_test.go @@ -92,7 +92,7 @@ var _ = ginkgo.Describe("DebugService", func() { _, err := kubeClient.CoreV1().ServiceAccounts(namespace).Create(context.Background(), sa, metav1.CreateOptions{}) gomega.Expect(err).ToNot(gomega.HaveOccurred()) - // Create Role with permission to create placements + // Create Role with permission to get placements (GET debug endpoint checks get verb) role := &rbacv1.Role{ ObjectMeta: metav1.ObjectMeta{ Name: roleName, @@ -102,7 +102,7 @@ var _ = ginkgo.Describe("DebugService", func() { { APIGroups: []string{"cluster.open-cluster-management.io"}, Resources: []string{"placements"}, - Verbs: []string{"create"}, + Verbs: []string{"get"}, }, }, } @@ -258,8 +258,9 @@ var _ = ginkgo.Describe("DebugService", func() { gomega.Expect(err).ToNot(gomega.HaveOccurred()) ginkgo.By("Verify request was rejected due to lack of permission") + gomega.Expect(resp.StatusCode).To(gomega.Equal(http.StatusForbidden)) gomega.Expect(result.Error).ToNot(gomega.BeEmpty(), "Should have permission error") - gomega.Expect(result.Error).To(gomega.ContainSubstring("does not have permission")) + gomega.Expect(result.Error).To(gomega.ContainSubstring("does not have permission to get placements")) }) ginkgo.It("Should accept POST request with placement JSON and valid token", func() {