From 5721772b1c2f4bf2feb347b3881c558422f7b04c Mon Sep 17 00:00:00 2001 From: Amin Date: Fri, 24 Jul 2026 21:56:45 +0330 Subject: [PATCH] fix: allow tenant owners to create namespaces with create-if-missing clients A tenant owner running `helm install --create-namespace` fails every time with a 403 on a namespace-scoped resource, and the namespace is never created (projectcapsule/capsule#1895). Two distinct problems are at play. First, Kubernetes authorizes namespace-scoped requests via RoleBindings that live inside the target namespace, so a namespace that does not exist yet can never have one. A tenant owner probing a resource there therefore always gets 403 Forbidden, never the 404 Not Found a cluster-wide privileged caller receives for the same request. Clients using the idiomatic "GET, and create it if 404" pattern -- Helm's `--create-namespace` among them -- treat that 403 as fatal and abort before they ever attempt to create the namespace. Second, once creation does proceed, Capsule provisions the owner's RoleBindings asynchronously, after the Namespace object has already been persisted and returned. A client acting immediately inside the namespace it was just handed can still race that provisioning. Both are addressed in the reverse proxy's ModifyResponse hook: - maskForbiddenForMissingNamespace rewrites 403 to 404 when a namespace-scoped GET-by-name failed only because the namespace does not exist. It is deliberately narrow so it cannot become a cross-tenant existence oracle: it applies only when the namespace name falls within the naming space of a tenant the caller actually owns, and only when the namespace is confirmed absent. A caller can thus only learn that a namespace they are entitled to create does not exist yet -- something they can already determine. Every other 403 is passed through untouched. - waitForOwnerCreationResponse holds a successful namespace-create response until the owner's RoleBinding lands, covering both the collection POST and the server-side-apply PATCH that Helm 4 uses by default. Dry-run creates are skipped, since they persist nothing to wait for. The wait is bounded end-to-end by a context deadline and fails open: the namespace is already created by that point, so a timeout releases the response rather than misreporting success as an error. No new RBAC is required; capsule-proxy already has the necessary read access. Refs: projectcapsule/capsule#1895 --- internal/webserver/webserver.go | 367 +++++++++++++++++++++++++++++++- 1 file changed, 365 insertions(+), 2 deletions(-) diff --git a/internal/webserver/webserver.go b/internal/webserver/webserver.go index f5921d97..d7113c53 100644 --- a/internal/webserver/webserver.go +++ b/internal/webserver/webserver.go @@ -4,6 +4,7 @@ package webserver import ( + "bytes" "context" "crypto/tls" "encoding/json" @@ -19,6 +20,7 @@ import ( "os" "regexp" "slices" + "strconv" "strings" "time" @@ -27,18 +29,24 @@ import ( "github.com/gorilla/mux" pkgerrors "github.com/pkg/errors" capsulev1beta2 "github.com/projectcapsule/capsule/api/v1beta2" + capsulemeta "github.com/projectcapsule/capsule/pkg/api/meta" capsulerbac "github.com/projectcapsule/capsule/pkg/api/rbac" "golang.org/x/net/http/httpguts" authenticationv1 "k8s.io/api/authentication/v1" authorizationv1 "k8s.io/api/authorization/v1" corev1 "k8s.io/api/core/v1" + rbacv1 "k8s.io/api/rbac/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/runtime/serializer" "k8s.io/apimachinery/pkg/runtime/serializer/protobuf" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apiserver/pkg/authentication/serviceaccount" "k8s.io/client-go/discovery" + "k8s.io/client-go/kubernetes" "k8s.io/component-base/featuregate" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/certwatcher" @@ -70,6 +78,32 @@ import ( "github.com/projectcapsule/capsule-proxy/internal/webserver/middleware" ) +const ( + // namespaceReadinessTimeout bounds how long the proxy will delay a namespace-create + // response while waiting for Capsule to finish provisioning the requesting tenant + // owner's RoleBinding in the new namespace. Capsule provisions this RBAC + // asynchronously after the Namespace object is already persisted and returned to the + // client, so a caller that immediately acts within the namespace it was just handed + // (as Helm's `--create-namespace` does) can otherwise be told the namespace exists + // while still getting 403s inside it. See + // https://github.com/projectcapsule/capsule/issues/1895. + // + // On timeout the response is released as-is: the namespace was already created + // successfully, so failing the response at this point would misreport a successful + // creation as an error. The wait is bounded end-to-end by deriving a context with this + // deadline, so even a slow or hung API-server List cannot exceed it. + namespaceReadinessTimeout = 3 * time.Second + namespaceReadinessPollInterval = 25 * time.Millisecond + + // namespaceReadinessSettleBuffer is an extra wait applied after the first matching + // RoleBinding is seen. Capsule creates one RoleBinding per ClusterRole configured on the + // tenant owner (e.g. a tenant owner with clusterRoles [cluster-admin, + // capsule-namespace-deleter] gets two), and they don't necessarily land atomically — + // waiting for only the first one can still release the response before the others (and + // whatever specific permission the caller's very next request needs) have caught up. + namespaceReadinessSettleBuffer = 250 * time.Millisecond +) + func NewKubeFilter( opts options.ListenerOpts, srv options.ServerOptions, @@ -104,7 +138,18 @@ func NewKubeFilter( codecFactory := serializer.NewCodecFactory(scheme) universalDecoder := codecFactory.UniversalDeserializer() - return &kubeFilter{ + // rbacClient is best-effort: it backs the namespace-readiness gate + // (waitForOwnerRoleBinding). If it can't be built we log and continue with a nil client + // — the gate degrades to a no-op (see waitForOwnerRoleBinding's nil check) rather than + // taking down proxy startup for a feature that is itself designed to fail open. + rbacClient, err := kubernetes.NewForConfig(mgr.GetConfig()) + if err != nil { + ctrl.Log.WithName("proxy").Error(err, "cannot create kubernetes clientset for namespace RBAC readiness checks; namespace-creation readiness gating disabled") + + rbacClient = nil + } + + kf := &kubeFilter{ mgr: mgr, gates: gates, reader: clientOverride, @@ -124,12 +169,17 @@ func NewKubeFilter( serverOptions: srv, log: ctrl.Log.WithName("proxy"), roleBindingsReflector: rbReflector, + rbacClient: rbacClient, protoEncoder: protoEncoder, universalDecoder: universalDecoder, scheme: scheme, trustedProxyCIDRs: opts.TrustedProxyCIDRs(), xfcc_header: opts.XFCCHeader(), - }, nil + } + + reverseProxy.ModifyResponse = kf.gateNamespaceCreationResponse + + return kf, nil } type kubeFilter struct { @@ -148,6 +198,7 @@ type kubeFilter struct { serverOptions options.ServerOptions log logr.Logger roleBindingsReflector *controllers.RoleBindingReflector + rbacClient kubernetes.Interface gates featuregate.FeatureGate xfcc_header string trustedProxyCIDRs []*net.IPNet @@ -496,6 +547,318 @@ func (n *kubeFilter) impersonateHandler(writer http.ResponseWriter, request *htt } } +// gateNamespaceCreationResponse is the single ModifyResponse entry point for both fixes +// addressing https://github.com/projectcapsule/capsule/issues/1895: +// +// - maskForbiddenForMissingNamespace rewrites a 403 into the 404 a privileged caller would +// see, when a tenant owner's namespace-scoped GET fails only because the target +// namespace doesn't exist yet. Without this, clients with idiomatic "GET, if 404 then +// create" workflows (Helm's `--create-namespace` among them) treat the 403 as a fatal +// error and abort before ever attempting to create the namespace. +// - waitForOwnerCreationResponse delays a successful namespace-create response until the +// tenant owner's RoleBinding has actually landed in the new namespace, closing the +// separate (and separately real) race between namespace creation and Capsule's async +// RBAC provisioning for whatever request comes immediately after. +func (n *kubeFilter) gateNamespaceCreationResponse(resp *http.Response) error { + if resp.StatusCode == http.StatusForbidden { + return n.maskForbiddenForMissingNamespace(resp) + } + + return n.waitForOwnerCreationResponse(resp) +} + +// namespacedResourcePathPattern matches a GET-by-name request for a namespaced resource: +// /api/v1/namespaces/{ns}/{resource}/{name} or +// /apis/{group}/{version}/namespaces/{ns}/{resource}/{name}. It intentionally does not match +// collection requests (LIST, no trailing {name}) or subresources (an extra path segment +// after {name}, e.g. .../pods/{name}/log). +var namespacedResourcePathPattern = regexp.MustCompile(`^(?:/api/v1|/apis/[^/]+/[^/]+)/namespaces/([^/]+)/([^/]+)/([^/]+)$`) + +// maskForbiddenForMissingNamespace rewrites a 403 into the 404 a privileged user would see +// when a tenant owner's namespace-scoped GET fails only because the target namespace +// doesn't exist yet. Kubernetes RBAC authorizes namespace-scoped requests using RoleBindings +// that live inside that namespace, so a namespace that hasn't been created yet can never +// have one — meaning any tenant owner probing a resource there always gets 403 Forbidden, +// never the 404 Not Found a cluster-wide-privileged caller gets for the same nonexistent +// namespace. +// +// The rewrite is deliberately narrow, so it never becomes a cross-tenant existence oracle: +// it applies only when the target namespace name falls within the namespace-naming space of +// a tenant the caller actually owns (see namespaceClaimableByOwner) AND the namespace is +// confirmed not to exist. A caller can therefore only ever learn "a namespace I'm entitled +// to create doesn't exist yet" — information they already have (they can list their own +// tenant's namespaces). A 403 for any namespace outside the caller's own tenants, or for one +// that genuinely exists, is passed through unchanged. +func (n *kubeFilter) maskForbiddenForMissingNamespace(resp *http.Response) error { + if resp.Request.Method != http.MethodGet { + return nil + } + + username := resp.Request.Header.Get(authenticationv1.ImpersonateUserHeader) + if username == "" { + return nil + } + + match := namespacedResourcePathPattern.FindStringSubmatch(resp.Request.URL.Path) + if match == nil { + return nil + } + + namespaceName, resource, name := match[1], match[2], match[3] + + groups := resp.Request.Header.Values(authenticationv1.ImpersonateGroupHeader) + + // Ownership gate: only mask for a namespace the caller could legitimately create as one + // of their own tenants' namespaces. Without this, the 403-vs-404 distinction would let + // any tenant owner probe the existence of arbitrary namespaces cluster-wide. + if !n.namespaceClaimableByOwner(resp.Request.Context(), username, groups, namespaceName) { + return nil + } + + var namespaceObj corev1.Namespace + if err := n.reader.Get(resp.Request.Context(), client.ObjectKey{Name: namespaceName}, &namespaceObj); err == nil || !apierrors.IsNotFound(err) { + // Namespace exists, or we couldn't tell — this is a genuine permission denial, + // leave it as-is. + return nil + } + + status := apierrors.NewNotFound(schema.GroupResource{Resource: resource}, name).Status() + + body, err := json.Marshal(status) + if err != nil { + return nil //nolint:nilerr // best-effort; fall back to the original 403 on marshal failure + } + + _ = resp.Body.Close() + resp.Body = io.NopCloser(bytes.NewReader(body)) + resp.StatusCode = http.StatusNotFound + resp.Status = fmt.Sprintf("%d %s", http.StatusNotFound, http.StatusText(http.StatusNotFound)) + resp.ContentLength = int64(len(body)) + resp.Header.Set("Content-Length", strconv.Itoa(len(body))) + resp.Header.Set("Content-Type", "application/json") + + n.log.V(4).Info("masked forbidden as not-found for missing namespace", "namespace", namespaceName, "resource", resource, "name", name, "username", username) + + return nil +} + +// namespaceClaimableByOwner reports whether namespaceName falls within the namespace-naming +// space of a tenant owned by the caller — i.e. it equals an owned tenant's name or is +// prefixed by "-". This mirrors Capsule's own forceTenantPrefix boundary and is the +// only relationship the proxy can establish for a namespace that does not exist yet (it has +// no owner reference or tenant label to consult). Callers with no owned tenants, or probing +// a name outside all of their tenants' prefixes, get false. +func (n *kubeFilter) namespaceClaimableByOwner(ctx context.Context, username string, groups []string, namespaceName string) bool { + proxyTenants, err := n.getTenantsForOwner(ctx, username, groups) + if err != nil { + n.log.V(4).Info("cannot resolve tenant owners while masking namespace lookup", "namespace", namespaceName, "error", err.Error()) + + return false + } + + for _, pt := range proxyTenants { + tenantName := pt.Tenant.GetName() + if namespaceName == tenantName || strings.HasPrefix(namespaceName, tenantName+"-") { + return true + } + } + + return false +} + +// waitForOwnerCreationResponse delays a successful namespace-create response until the +// requesting tenant owner's RoleBinding has actually landed in the new namespace, or until +// namespaceReadinessTimeout elapses. See the namespaceReadinessTimeout doc comment and +// https://github.com/projectcapsule/capsule/issues/1895 for why this exists. +// +// This has to match both the classic collection-POST create (`POST /api/v1/namespaces`) +// and a Server-Side Apply create (`PATCH /api/v1/namespaces/`) — Helm 4 defaults +// `--server-side` to true, so `helm install --create-namespace` goes through the SSA path, +// not a plain POST. Kubernetes returns 201 for both when the object didn't previously exist, +// which is what distinguishes a genuine creation (something to gate) from an update to an +// existing namespace (RBAC already settled, nothing to wait for) on the same PATCH verb. +func (n *kubeFilter) waitForOwnerCreationResponse(resp *http.Response) error { + if resp.StatusCode != http.StatusCreated { + return nil + } + + if resp.Request.Method != http.MethodPost && resp.Request.Method != http.MethodPatch { + return nil + } + + if !isNamespaceCollectionOrObjectPath(resp.Request.URL.Path) { + return nil + } + + // A server-side dry-run create (`?dryRun=All`) returns 201 but persists nothing — no + // Namespace and no RoleBinding are actually created — so there is nothing to wait for. + // Without this, every dry-run namespace create would block for the full readiness + // timeout polling for a RoleBinding that will never appear. + if resp.Request.URL.Query().Get("dryRun") != "" { + return nil + } + + username := resp.Request.Header.Get(authenticationv1.ImpersonateUserHeader) + groups := resp.Request.Header.Values(authenticationv1.ImpersonateGroupHeader) + + if username == "" { + // Not an impersonated request (e.g. a call authenticated with the proxy's own + // bearer token) — nothing to gate. + return nil + } + + body, err := io.ReadAll(resp.Body) + if err != nil { + return nil //nolint:nilerr // best-effort gate; never fail a response that already succeeded upstream + } + + _ = resp.Body.Close() + resp.Body = io.NopCloser(bytes.NewReader(body)) + + var namespace corev1.Namespace + if err := json.Unmarshal(body, &namespace); err != nil || namespace.Name == "" { + return nil + } + + tenantName := namespace.Labels[capsulemeta.TenantLabel] + if tenantName == "" { + // Not a Capsule tenant namespace. + return nil + } + + if !n.isTenantOwner(resp.Request.Context(), tenantName, username, groups) { + // Whoever created this namespace isn't a registered owner of its tenant (e.g. a + // cluster administrator using capsule-proxy directly). Capsule only provisions an + // owner RoleBinding for registered tenant owners, so there's nothing to wait for + // and waiting here would only add latency to a request that already works. + return nil + } + + n.waitForOwnerRoleBinding(resp.Request.Context(), username, groups, namespace.Name) + + return nil +} + +// isNamespaceCollectionOrObjectPath reports whether path is the namespaces collection +// endpoint ("/api/v1/namespaces") or a single namespace's object endpoint +// ("/api/v1/namespaces/") — not a subresource of it (e.g. .../status, .../finalize). +func isNamespaceCollectionOrObjectPath(path string) bool { + const collectionPath = "/api/v1/namespaces" + + if path == collectionPath { + return true + } + + name, ok := strings.CutPrefix(path, collectionPath+"/") + + return ok && name != "" && !strings.Contains(name, "/") +} + +func (n *kubeFilter) isTenantOwner(ctx context.Context, tenantName, username string, groups []string) bool { + proxyTenants, err := n.getTenantsForOwner(ctx, username, groups) + if err != nil { + n.log.V(4).Info("cannot resolve tenant owners while gating namespace creation", "tenant", tenantName, "error", err.Error()) + + return false + } + + for _, pt := range proxyTenants { + if pt.Tenant.GetName() == tenantName { + return true + } + } + + return false +} + +// waitForOwnerRoleBinding polls the API server directly for a RoleBinding in namespace +// whose subjects include username or one of groups. It intentionally doesn't use the +// optional roleBindingsReflector (disabled by default via --enable-reflector=false): this +// gate needs to work regardless of that setting, and a namespace-scoped List is cheap +// enough to issue synchronously here — it's bounded to at most namespaceReadinessTimeout / +// namespaceReadinessPollInterval calls, and only for genuine tenant-owner namespace +// creations (see isTenantOwner above). +// +// The whole wait is bounded by a context deadline derived from namespaceReadinessTimeout, so +// that even a single slow or hung List call cannot hold the client's response open past the +// documented bound: once the deadline passes, the in-flight List is cancelled and the +// response is released (fail-open). +func (n *kubeFilter) waitForOwnerRoleBinding(ctx context.Context, username string, groups []string, namespace string) { + if n.rbacClient == nil { + return + } + + start := time.Now() + + ctx, cancel := context.WithTimeout(ctx, namespaceReadinessTimeout) + defer cancel() + + groupSet := sets.New(groups...) + + for { + ready, err := n.hasMatchingRoleBinding(ctx, username, groupSet, namespace) + if err != nil { + // Includes context deadline exceeded — fail open, the namespace is already + // created and the response must not be held any longer. + n.log.V(4).Info("stopped waiting for owner RoleBinding readiness", "namespace", namespace, "username", username, "waited", time.Since(start), "reason", err.Error()) + + return + } + + if ready { + // A tenant owner can have more than one ClusterRole configured, and Capsule + // creates one RoleBinding per ClusterRole — they don't necessarily land in the + // same instant. Give any siblings a short window to catch up too, rather than + // releasing the response the moment the very first one appears. Cap the buffer + // at the remaining deadline so it can't push total latency past the bound. + settle := namespaceReadinessSettleBuffer + if remaining := time.Until(start.Add(namespaceReadinessTimeout)); remaining < settle { + settle = remaining + } + + if settle > 0 { + time.Sleep(settle) + } + + n.log.V(5).Info("owner RoleBinding ready", "namespace", namespace, "username", username, "waited", time.Since(start)) + + return + } + + select { + case <-ctx.Done(): + n.log.Info("timed out waiting for owner RoleBinding to be provisioned; releasing namespace-create response anyway", "namespace", namespace, "username", username, "timeout", namespaceReadinessTimeout) + + return + case <-time.After(namespaceReadinessPollInterval): + } + } +} + +func (n *kubeFilter) hasMatchingRoleBinding(ctx context.Context, username string, groups sets.Set[string], namespace string) (bool, error) { + list, err := n.rbacClient.RbacV1().RoleBindings(namespace).List(ctx, metav1.ListOptions{}) + if err != nil { + return false, err + } + + for _, rb := range list.Items { + for _, subject := range rb.Subjects { + switch subject.Kind { + case rbacv1.UserKind: + if subject.Name == username { + return true, nil + } + case rbacv1.GroupKind: + if groups.Has(subject.Name) { + return true, nil + } + } + } + } + + return false, nil +} + func (n *kubeFilter) ownerFromCapsuleToProxySetting(owners capsulerbac.OwnerListSpec) []v1beta1.OwnerSpec { out := make([]v1beta1.OwnerSpec, 0, len(owners))